fix(net): track gateway transit flows from its routed CIDRs

cmd/unbounded-net-node/site_watch_reconcile.go

@@ -2201,7 +2201,15 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf
 			// site underlay (e.g. a service ClusterIP DNAT'd by a remote
 			// node to an in-site host-network pod) leave eth0 and need
 			// MASQUERADE in nat/POSTROUTING, which requires conntrack.
-			if err := state.notrackManager.ReconcileCIDRs(state.nodePodCIDRs, siteRouting.gatewayReturnCIDRs, supernets); err != nil {
+			// Flows sourced from this gateway's routedCidrs are tracked too:
+			// their replies come back through this gateway to a tracked
+			// local destination.
+			var sourceCIDRs []string
+			for _, pool := range gatewayPoolsForNode {
+				sourceCIDRs = append(sourceCIDRs, pool.Spec.RoutedCidrs...)
+			}
+
+			if err := state.notrackManager.ReconcileCIDRs(state.nodePodCIDRs, siteRouting.gatewayReturnCIDRs, sourceCIDRs, supernets); err != nil {
 				return fmt.Errorf("reconcile notrack CIDRs: %w", err)
 			}
 		} else {

internal/net/netlink/mss_clamp_manager_test.go

@@ -14,9 +14,12 @@ type fakeIPTables struct {
 	deleteJumpErr      error
 	deletedCurrentJump bool
 	deletedLegacyJump  bool
+	appended           [][]string
 }
 
-func (f *fakeIPTables) Append(_, _ string, _ ...string) error {
+func (f *fakeIPTables) Append(_, _ string, rulespec ...string) error {
+	f.appended = append(f.appended, rulespec)
+
 	return nil
 }
 

internal/net/netlink/notrack_manager.go

@@ -30,6 +30,8 @@ const (
 //
 //	-m addrtype --dst-type LOCAL -j RETURN   (keep conntrack for node itself)
 //	-d <podCIDR> -j RETURN                   (keep conntrack for local pods)
+//	-d <returnCIDR> -j RETURN                (keep conntrack for the local site)
+//	-s <sourceCIDR> -j RETURN                (and for flows from what this gateway routes)
 //	-d <supernet> -j CT --notrack            (skip conntrack for transit)
 //	(implicit RETURN)                         (anything else: normal conntrack)
 //
@@ -47,6 +49,7 @@ type NotrackManager struct {
 	// Track current CIDR sets to avoid unnecessary rebuilds.
 	currentPodCIDRs    string
 	currentReturnCIDRs string
+	currentSourceCIDRs string
 	currentSupernets   string
 }
 
@@ -175,57 +178,65 @@ func (m *NotrackManager) removeInterfaceForFamily(ipt *iptables.IPTables, family
 }
 
 // ReconcileCIDRs rebuilds the UNBOUNDED-NOTRACK chain contents if any of
-// podCIDRs, returnCIDRs, or supernets have changed since the last call.
-// It installs (in order):
+// podCIDRs, returnCIDRs, sourceCIDRs, or supernets have changed since the
+// last call. It installs (in order):
 //   - addrtype LOCAL RETURN (always first)
 //   - per-podCIDR RETURN rules (this node's own pod CIDRs)
 //   - per-returnCIDR RETURN rules (e.g. the gateway's own site NodeCidr,
 //     where transit packets must remain conntrack-tracked so MASQUERADE
 //     can SNAT overlay sources to the underlay-facing eth0 address)
+//   - per-sourceCIDR RETURN rules matched by source (the CIDRs this gateway
+//     routes for its pools, whose return traffic comes back through it)
 //   - per-supernet CT --notrack rules
 //
 // CIDRs are automatically sorted into IPv4 and IPv6 buckets.
-func (m *NotrackManager) ReconcileCIDRs(podCIDRs, returnCIDRs, supernets []string) error {
+func (m *NotrackManager) ReconcileCIDRs(podCIDRs, returnCIDRs, sourceCIDRs, supernets []string) error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
 	// Deduplicate and sort for stable comparison.
 	podCIDRs = dedupeAndSort(podCIDRs)
 	returnCIDRs = dedupeAndSort(returnCIDRs)
+	sourceCIDRs = dedupeAndSort(sourceCIDRs)
 	supernets = dedupeAndSort(supernets)
 
 	podKey := strings.Join(podCIDRs, ",")
 	returnKey := strings.Join(returnCIDRs, ",")
+	sourceKey := strings.Join(sourceCIDRs, ",")
 	superKey := strings.Join(supernets, ",")
 
-	if podKey == m.currentPodCIDRs && returnKey == m.currentReturnCIDRs && superKey == m.currentSupernets {
+	if podKey == m.currentPodCIDRs && returnKey == m.currentReturnCIDRs &&
+		sourceKey == m.currentSourceCIDRs && superKey == m.currentSupernets {
 		return nil
 	}
 
-	klog.V(2).Infof("NotrackManager: CIDRs changed, rebuilding chain (podCIDRs=%d, returnCIDRs=%d, supernets=%d)", len(podCIDRs), len(returnCIDRs), len(supernets))
+	klog.V(2).Infof("NotrackManager: CIDRs changed, rebuilding chain (podCIDRs=%d, returnCIDRs=%d, sourceCIDRs=%d, supernets=%d)",
+		len(podCIDRs), len(returnCIDRs), len(sourceCIDRs), len(supernets))
 
 	v4Pods, v6Pods := splitByFamily(podCIDRs)
 	v4Returns, v6Returns := splitByFamily(returnCIDRs)
+	v4Sources, v6Sources := splitByFamily(sourceCIDRs)
 	v4Supers, v6Supers := splitByFamily(supernets)
 
-	if err := m.reconcileFamily(m.ipt4, "IPv4", m.ctSupported4, v4Pods, v4Returns, v4Supers); err != nil {
+	if err := m.reconcileFamily(m.ipt4, "IPv4", m.ctSupported4, v4Pods, v4Returns, v4Sources, v4Supers); err != nil {
 		return fmt.Errorf("failed to reconcile IPv4 notrack rules: %w", err)
 	}
 
 	if m.ipt6 != nil {
-		if err := m.reconcileFamily(m.ipt6, "IPv6", m.ctSupported6, v6Pods, v6Returns, v6Supers); err != nil {
+		if err := m.reconcileFamily(m.ipt6, "IPv6", m.ctSupported6, v6Pods, v6Returns, v6Sources, v6Supers); err != nil {
 			klog.Warningf("NotrackManager: failed to reconcile IPv6 notrack rules: %v", err)
 		}
 	}
 
 	m.currentPodCIDRs = podKey
 	m.currentReturnCIDRs = returnKey
+	m.currentSourceCIDRs = sourceKey
 	m.currentSupernets = superKey
 
 	return nil
 }
 
-func (m *NotrackManager) reconcileFamily(ipt *iptables.IPTables, family string, ctSupported bool, podCIDRs, returnCIDRs, supernets []string) error {
+func (m *NotrackManager) reconcileFamily(ipt iptablesClient, family string, ctSupported bool, podCIDRs, returnCIDRs, sourceCIDRs, supernets []string) error {
 	if !ctSupported {
 		return nil
 	}
@@ -272,6 +283,21 @@ func (m *NotrackManager) reconcileFamily(ipt *iptables.IPTables, family string,
 		}
 	}
 
+	// RETURN by source for each sourceCIDR. A request from these CIDRs
+	// enters the tunnel here and its reply comes back through this gateway
+	// to a returnCIDR, where it is tracked; tracking the request too keeps
+	// the reply from being INVALID and dropped by KUBE-FORWARD. Sources whose
+	// replies may bypass the gateway (e.g. node IPs) are left untracked.
+	for _, cidr := range sourceCIDRs {
+		rule := []string{
+			"-s", cidr,
+			"-m", "comment", "--comment", notrackComment, "-j", "RETURN",
+		}
+		if err := ipt.Append("raw", notrackChain, rule...); err != nil {
+			return fmt.Errorf("failed to add %s sourceCIDR RETURN for %s: %w", family, cidr, err)
+		}
+	}
+
 	// CT --notrack for each supernet.
 	for _, cidr := range supernets {
 		rule := []string{
@@ -283,14 +309,14 @@ func (m *NotrackManager) reconcileFamily(ipt *iptables.IPTables, family string,
 		}
 	}
 
-	klog.V(2).Infof("NotrackManager: rebuilt %s chain (%d podCIDR RETURNs, %d returnCIDR RETURNs, %d supernet NOTRACKs)",
-		family, len(podCIDRs), len(returnCIDRs), len(supernets))
+	klog.V(2).Infof("NotrackManager: rebuilt %s chain (%d podCIDR RETURNs, %d returnCIDR RETURNs, %d sourceCIDR RETURNs, %d supernet NOTRACKs)",
+		family, len(podCIDRs), len(returnCIDRs), len(sourceCIDRs), len(supernets))
 
 	return nil
 }
 
 // ensureChain creates the UNBOUNDED-NOTRACK chain if it does not exist.
-func (m *NotrackManager) ensureChain(ipt *iptables.IPTables, family string) error {
+func (m *NotrackManager) ensureChain(ipt iptablesClient, family string) error {
 	exists, err := ipt.ChainExists("raw", notrackChain)
 	if err != nil {
 		return fmt.Errorf("failed to check if %s chain exists: %w", family, err)
@@ -321,6 +347,7 @@ func (m *NotrackManager) Cleanup() {
 
 	m.currentPodCIDRs = ""
 	m.currentReturnCIDRs = ""
+	m.currentSourceCIDRs = ""
 	m.currentSupernets = ""
 
 	klog.V(2).Info("NotrackManager: cleaned up notrack chain and rules")

internal/net/netlink/notrack_manager_test.go

@@ -105,3 +105,69 @@ func TestNotrackJumpRuleSpec(t *testing.T) {
 		}
 	}
 }
+
+// TestReconcileFamilyTracksRoutedSources covers a non-gateway node in the
+// gateway's site reaching a remote site through the gateway. A request from
+// the gateway's routedCidrs arrives on the tunnel with a local source and its
+// reply comes back with a local destination; both must stay tracked so the
+// reply is not INVALID. Node IPs are not routed through the gateway, so their
+// replies may bypass it and they must stay untracked by source.
+func TestReconcileFamilyTracksRoutedSources(t *testing.T) {
+	t.Parallel()
+
+	const (
+		podCIDR    = "10.244.1.0/24"
+		podSubnet  = "10.0.2.0/23"
+		nodeSubnet = "10.0.0.0/27"
+		remotePods = "192.168.20.0/24"
+	)
+
+	ipt := &fakeIPTables{}
+	m := &NotrackManager{}
+
+	err := m.reconcileFamily(ipt, "IPv4", true,
+		[]string{podCIDR},
+		[]string{podSubnet, nodeSubnet},
+		[]string{podSubnet},
+		[]string{remotePods},
+	)
+	if err != nil {
+		t.Fatalf("reconcileFamily: %v", err)
+	}
+
+	index := func(match, cidr, target string) int {
+		for i, rule := range ipt.appended {
+			if len(rule) > 1 && rule[0] == match && rule[1] == cidr && rule[len(rule)-1] == target {
+				return i
+			}
+		}
+
+		return -1
+	}
+
+	notrack := index("-d", remotePods, "--notrack")
+	if notrack < 0 {
+		t.Fatalf("missing supernet NOTRACK rule: %v", ipt.appended)
+	}
+
+	for _, want := range []struct{ match, cidr string }{
+		{"-d", podSubnet},
+		{"-d", nodeSubnet},
+		{"-s", podSubnet},
+	} {
+		i := index(want.match, want.cidr, "RETURN")
+		if i < 0 {
+			t.Fatalf("missing %s %s RETURN rule: %v", want.match, want.cidr, ipt.appended)
+		}
+
+		if i > notrack {
+			t.Fatalf("%s %s RETURN at %d comes after NOTRACK at %d", want.match, want.cidr, i, notrack)
+		}
+	}
+
+	for _, cidr := range []string{nodeSubnet, podCIDR} {
+		if index("-s", cidr, "RETURN") >= 0 {
+			t.Fatalf("%s must not be tracked by source: %v", cidr, ipt.appended)
+		}
+	}
+}