Fix atomic GeoIP restoration across existing set limits and await startup migration

README.md

@@ -2,7 +2,7 @@
 
 DebTik is a Debian/Proxmox router appliance with an embedded web interface, REST API and RouterOS-style CLI. It manages network interfaces, routing, firewall/NAT, DHCP/DNS, WireGuard, IPsec, PPP, VRRP and AeroTUN. AeroTUN v3 uses pinned peer identities and encrypted tunnels; it works with native TUN or a veth fallback when `/dev/net/tun` is unavailable.
 
-Current release: **1.3.0**. See [functionality corrections and verification](docs/functionality-fixes.md) for network persistence, firewall isolation/IPv6, service error handling and certificate renewal changes.
+Current release: **1.3.1**. See [functionality corrections and verification](docs/functionality-fixes.md) for network persistence, firewall isolation/IPv6, service error handling and certificate renewal changes.
 
 ## Install
 
@@ -21,10 +21,10 @@ The internal Git server uses a private CA. Trust that CA before cloning, or repl
 ```sh
 git -c http.sslVerify=false \
   -c 'http.pinnedpubkey=sha256//u4xyt0njDzn4BAQnS6OTGrXBxVE8vMg5BSNIuryUsMo=' \
-  clone --depth 1 --branch v1.3.0 https://gogs.eparmak.local/eparmak/DebTik.git
+  clone --depth 1 --branch v1.3.1 https://gogs.eparmak.local/eparmak/DebTik.git
 ```
 
-This verifies the server's pinned key for this command; it does not change global Git settings. Reverify the pin if the server's TLS key changes. For a reproducible fresh installation, `v1.3.0` contains the complete dependency installer and native Linux firewall corrections.
+This verifies the server's pinned key for this command; it does not change global Git settings. Reverify the pin if the server's TLS key changes. For a reproducible fresh installation, `v1.3.1` contains the complete dependency installer and native Linux firewall corrections.
 
 `install.sh` installs and verifies the dependencies listed in [Dependencies](docs/dependencies.md), including Go, WireGuard, firewall, BIRD, DHCP/DNS, PPP clients and servers, strongSwan plugins, diagnostics and the RSTP daemon. It builds `proxmox-router` and `debtik-cli`, installs the updater helpers, and starts `proxmox-router.service`. It can be run again; existing `/etc/proxmox-router` files and tunnel keys are preserved. Previous binaries are saved as `/usr/local/bin/*.previous`. For native TUN in an LXC container, enable `/dev/net/tun` on the Proxmox host; otherwise AeroTUN uses veth.
 

VERSION

@@ -1 +1 @@
-1.3.0
+1.3.1

docs/firewall-native.md

@@ -89,3 +89,8 @@ Verification covers real private kernel namespaces, rollback, both IP families,
 packet/counter behavior, dynamic expiration/preservation, GeoIP in-use guards,
 request validation, browser form submissions, legacy/custom-chain handling and
 XSS regression. Production firewall edits are not used for destructive tests.
+
+Version 1.3.1 also restores existing country sets whose creation limits differ
+from the current defaults, using atomic swap without recreating the existing
+set. Startup exposes `is_restoring`; migration verification waits for all
+country restores and checks `last_error` before declaring success.

internal/firewall/geoip.go

@@ -34,23 +34,25 @@ type GeoIPConfig struct {
 
 // GeoIPManager handles country IP databases, ipset sync, and auto-update scheduling
 type GeoIPManager struct {
-	mu         sync.RWMutex
-	config     GeoIPConfig
-	countries  map[string]*GeoCountryInfo
-	timer      *time.Timer
-	stopCh     chan struct{}
-	isUpdating bool
-	lastError  string
-	referenced func(string) bool
-	syncMu     sync.Mutex
+	mu          sync.RWMutex
+	config      GeoIPConfig
+	countries   map[string]*GeoCountryInfo
+	timer       *time.Timer
+	stopCh      chan struct{}
+	restoreDone chan struct{}
+	isUpdating  bool
+	lastError   string
+	referenced  func(string) bool
+	syncMu      sync.Mutex
 }
 
 func NewGeoIPManager() *GeoIPManager {
 	_ = os.MkdirAll(GeoIPZonesDir, 0755)
 
 	m := &GeoIPManager{
-		countries: make(map[string]*GeoCountryInfo),
-		stopCh:    make(chan struct{}),
+		countries:   make(map[string]*GeoCountryInfo),
+		stopCh:      make(chan struct{}),
+		restoreDone: make(chan struct{}),
 	}
 
 	// 1. Initialize country metadata
@@ -68,6 +70,7 @@ func NewGeoIPManager() *GeoIPManager {
 
 	// Restore cached enabled countries without hiding a failed restore.
 	go func() {
+		defer close(m.restoreDone)
 		m.mu.RLock()
 		var enabled []string
 		for code, c := range m.countries {
@@ -255,6 +258,15 @@ func (m *GeoIPManager) GetStatus() map[string]any {
 		}
 	}
 
+	restoring := false
+	if m.restoreDone != nil {
+		select {
+		case <-m.restoreDone:
+		default:
+			restoring = true
+		}
+	}
+
 	return map[string]any{
 		"auto_update":       m.config.AutoUpdate,
 		"interval_hours":    m.config.IntervalHours,
@@ -264,6 +276,7 @@ func (m *GeoIPManager) GetStatus() map[string]any {
 		"enabled_count":     enabledCount,
 		"total_subnets":     totalSubnets,
 		"is_updating":       m.isUpdating,
+		"is_restoring":      restoring,
 		"last_error":        m.lastError,
 	}
 }

internal/firewall/ipset_atomic.go

@@ -50,20 +50,31 @@ func replaceSet(name, family string, addresses []string) error {
 		}
 	}
 	digest := sha256.Sum256([]byte(family + "\n" + strings.Join(clean, "\n")))
-	if setDigests[name] == digest {
-		out, err := exec.Command("ipset", "list", name, "-terse").Output()
-		if err == nil && strings.Contains(string(out), "Number of entries: "+strconv.Itoa(len(clean))+"\n") {
-			return nil
-		}
+	header, inspectErr := exec.Command("ipset", "list", name, "-terse").CombinedOutput()
+	exists := inspectErr == nil
+	if inspectErr != nil && !strings.Contains(string(header), "does not exist") {
+		return fmt.Errorf("cannot inspect ipset %s: %s: %w", name, header, inspectErr)
+	}
+	if exists && (!strings.Contains(string(header), "Type: hash:net\n") || !strings.Contains(string(header), "family "+family+" ")) {
+		return fmt.Errorf("set %s has an incompatible type/family; existing data was preserved", name)
+	}
+	if exists && setDigests[name] == digest && strings.Contains(string(header), "Number of entries: "+strconv.Itoa(len(clean))+"\n") {
+		return nil
 	}
+
 	h := sha256.Sum256([]byte(name))
 	temp := fmt.Sprintf("dt_tmp_%x", h[:8])
+	// A previous version may have left a temporary set with different limits.
+	_ = exec.Command("ipset", "destroy", temp).Run()
 	var restore strings.Builder
-	fmt.Fprintf(&restore, "create %s hash:net family %s maxelem 1048576 -exist\nflush %s\n", temp, family, temp)
+	fmt.Fprintf(&restore, "create %s hash:net family %s maxelem 1048576\n", temp, family)
 	for _, address := range clean {
 		fmt.Fprintf(&restore, "add %s %s -exist\n", temp, address)
 	}
-	fmt.Fprintf(&restore, "create %s hash:net family %s maxelem 1048576 -exist\nswap %s %s\ndestroy %s\n", name, family, temp, name, temp)
+	if !exists {
+		fmt.Fprintf(&restore, "create %s hash:net family %s maxelem 1048576\n", name, family)
+	}
+	fmt.Fprintf(&restore, "swap %s %s\ndestroy %s\n", temp, name, temp)
 	cmd := exec.Command("ipset", "restore")
 	cmd.Stdin = strings.NewReader(restore.String())
 	if out, err := cmd.CombinedOutput(); err != nil {

internal/firewall/owned_test.go

@@ -5,6 +5,7 @@ import (
 	"os/exec"
 	"strings"
 	"testing"
+	"time"
 )
 
 func TestOwnedPayloadPreservesForeignRulesAndPolicies(t *testing.T) {
@@ -144,6 +145,15 @@ func TestProductionFirewallMigration(t *testing.T) {
 	restore("iptables-restore", fixture+"/iptables.save")
 	restore("ip6tables-restore", fixture+"/ip6tables.save")
 	m := NewFirewallManager()
+	select {
+	case <-m.GeoIP().restoreDone:
+	case <-time.After(30 * time.Second):
+		t.Fatal("GeoIP restore did not finish")
+	}
+	if last := m.GeoIP().GetStatus()["last_error"]; last != "" {
+		t.Fatal("production GeoIP restore failed", last)
+	}
+
 	if err := m.Reapply(); err != nil {
 		t.Fatal("production migration failed", err)
 	}