diff --git a/client/firewall/nftables/chains_linux.go b/client/firewall/nftables/chains_linux.go index 745f2da54..71f0c60f2 100644 --- a/client/firewall/nftables/chains_linux.go +++ b/client/firewall/nftables/chains_linux.go @@ -606,7 +606,12 @@ func (r *family) Flush() error { return nil } -func (r *family) createPreroutingRule(expressions []expr.Any, userData []byte) *nftables.Rule { +// queuePreroutingRule builds the prerouting mangle rule that marks +// redirected traffic and queues it on the connection without flushing, +// so the caller can commit it in the same transaction as the rule it +// pairs with. Returns nil when the prerouting chain is absent, in which +// case nothing is queued. +func (r *family) queuePreroutingRule(expressions []expr.Any, userData []byte) *nftables.Rule { if r.chainPrerouting == nil { log.Warn("prerouting chain is not created") return nil @@ -651,19 +656,12 @@ func (r *family) createPreroutingRule(expressions []expr.Any, userData []byte) * }, ) - nfRule := r.conn.AddRule(&nftables.Rule{ + return r.conn.AddRule(&nftables.Rule{ Table: r.workTable, Chain: r.chainPrerouting, Exprs: preroutingExprs, UserData: userData, }) - - if err := r.conn.Flush(); err != nil { - log.Errorf("failed to flush mangle rule %s: %v", string(userData), err) - return nil - } - - return nfRule } func (r *family) createDefaultChains() (err error) { diff --git a/client/firewall/nftables/dnat_linux.go b/client/firewall/nftables/dnat_linux.go index b4cf3536a..7f73e11c6 100644 --- a/client/firewall/nftables/dnat_linux.go +++ b/client/firewall/nftables/dnat_linux.go @@ -39,7 +39,11 @@ func (r *family) AddDNATRule(rule firewall.ForwardRule) (firewall.Rule, error) { return nil, err } - r.addDnatMasq(rule, protoNum, ruleID) + if err := r.addDnatMasq(rule, protoNum, ruleID); err != nil { + r.releaseForwarding() + delete(r.rules, ruleID+dnatSuffix) + return nil, err + } // Unlike iptables, there's no point in adding "out" rules in the forward chain here as our policy is ACCEPT. // To overcome DROP policies in other chains, we'd have to add rules to the chains there. @@ -75,7 +79,11 @@ func (r *family) addDnatRedirect(rule firewall.ForwardRule, protoNum uint8, rule Len: 2, }, } - dnatExprs = append(dnatExprs, applyPort(&rule.DestinationPort, false)...) + portExprs, err := r.applyPort(&rule.DestinationPort, false) + if err != nil { + return fmt.Errorf("apply destination port: %w", err) + } + dnatExprs = append(dnatExprs, portExprs...) // shifted translated port is not supported in nftables, so we hand this over to xtables if rule.TranslatedPort.IsRange && len(rule.TranslatedPort.Values) == 2 { @@ -209,7 +217,12 @@ func (r *family) addXTablesRedirect(dnatExprs []expr.Any, ruleID firewall.RuleID return nil } -func (r *family) addDnatMasq(rule firewall.ForwardRule, protoNum uint8, ruleID firewall.RuleID) { +func (r *family) addDnatMasq(rule firewall.ForwardRule, protoNum uint8, ruleID firewall.RuleID) error { + portExprs, err := r.applyPort(&rule.TranslatedPort, false) + if err != nil { + return fmt.Errorf("apply translated port: %w", err) + } + masqExprs := []expr.Any{ &expr.Meta{Key: expr.MetaKeyOIFNAME, Register: 1}, &expr.Cmp{ @@ -236,7 +249,7 @@ func (r *family) addDnatMasq(rule firewall.ForwardRule, protoNum uint8, ruleID f }, } - masqExprs = append(masqExprs, applyPort(&rule.TranslatedPort, false)...) + masqExprs = append(masqExprs, portExprs...) masqExprs = append(masqExprs, &expr.Masq{}) masqRule := &nftables.Rule{ @@ -247,6 +260,8 @@ func (r *family) addDnatMasq(rule firewall.ForwardRule, protoNum uint8, ruleID f } r.conn.AddRule(masqRule) r.rules[ruleID+snatSuffix] = masqRule + + return nil } func (r *family) DeleteDNATRule(rule firewall.Rule) error { diff --git a/client/firewall/nftables/family_linux.go b/client/firewall/nftables/family_linux.go index 5a0d46475..314f4a1a2 100644 --- a/client/firewall/nftables/family_linux.go +++ b/client/firewall/nftables/family_linux.go @@ -102,7 +102,7 @@ type family struct { mtu uint16 } -func newFamily(workTable *nftables.Table, wgIface iFaceMapper, mtu uint16) (*family, error) { +func newFamily(workTable *nftables.Table, wgIface iFaceMapper, mtu uint16) *family { r := &family{ conn: &nftables.Conn{}, workTable: workTable, @@ -127,7 +127,7 @@ func newFamily(workTable *nftables.Table, wgIface iFaceMapper, mtu uint16) (*fam log.Debugf("ip filter table not found: %v", err) } - return r, nil + return r } func (r *family) init(workTable *nftables.Table) error { diff --git a/client/firewall/nftables/filter_linux.go b/client/firewall/nftables/filter_linux.go index 4c6f63a26..e040da4fc 100644 --- a/client/firewall/nftables/filter_linux.go +++ b/client/firewall/nftables/filter_linux.go @@ -70,6 +70,24 @@ func (r *family) AddFilterRule( } userData := []byte(ruleID) + + // Build the paired prerouting mangle rule before flushing so both + // rules commit in one transaction. An anonymous port set binds to + // exactly one rule, so the mangle rule needs its own expression list + // with fresh sets, not a clone of the main rule's. Guard on the + // prerouting chain first: building the expressions queues the port + // set, so skipping the build when there is no chain to bind it to + // keeps an unbound set out of the connection batch. + var mangleRule *nftables.Rule + if !isRoute && r.chainPrerouting != nil { + mangleExprs, err := r.buildPeerFilterExprs(srcExprs, proto, sPort, dPort) + if err != nil { + r.dropNetworkMatch(exprs) + return nil, fmt.Errorf("build mangle rule: %w", err) + } + mangleRule = r.queuePreroutingRule(mangleExprs, userData) + } + nftRule := &nftables.Rule{ Table: r.workTable, Chain: chain, @@ -87,12 +105,10 @@ func (r *family) AddFilterRule( } rule := &Rule{ - nftRule: nftRule, - sources: sources, - id: ruleID, - } - if !isRoute { - rule.mangleRule = r.createPreroutingRule(exprs, userData) + nftRule: nftRule, + mangleRule: mangleRule, + sources: sources, + id: ruleID, } r.filters[ruleID] = rule @@ -128,8 +144,12 @@ func (r *family) buildPeerFilterExprs( ) } exprs = append(exprs, srcExprs...) - exprs = append(exprs, applyPort(sPort, true)...) - exprs = append(exprs, applyPort(dPort, false)...) + + portExprs, err := r.applyPorts(sPort, dPort) + if err != nil { + return nil, err + } + exprs = append(exprs, portExprs...) return exprs, nil } @@ -159,8 +179,13 @@ func (r *family) buildRouteFilterExprs( &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{protoNum}}, ) - exprs = append(exprs, applyPort(sPort, true)...) - exprs = append(exprs, applyPort(dPort, false)...) + + portExprs, err := r.applyPorts(sPort, dPort) + if err != nil { + r.dropNetworkMatch(destExprs) + return nil, err + } + exprs = append(exprs, portExprs...) } exprs = append(exprs, &expr.Counter{}) @@ -191,15 +216,21 @@ func (r *family) DeleteFilterRule(rule firewall.Rule) error { // A freshly added rule carries no handle until it is read back from // the kernel, and Flush only refreshes the peer chains. Pull live // handles for this rule's chain before deciding it is stale so route - // rules (which Flush never refreshes) can actually be deleted. + // rules (which Flush never refreshes) can actually be deleted. A + // refresh failure aborts the delete without touching tracking state, + // so the caller can retry while the rule may still exist in the kernel. if pr.nftRule.Handle == 0 { if err := r.refreshRuleHandles(pr.nftRule.Chain, false); err != nil { - log.Warnf("refresh handles for chain %s: %v", pr.nftRule.Chain.Name, err) + return fmt.Errorf("refresh handles for chain %s: %w", pr.nftRule.Chain.Name, err) } - if pr.mangleRule != nil { - if err := r.refreshRuleHandles(r.chainPrerouting, true); err != nil { - log.Warnf("refresh mangle handles: %v", err) - } + } + // Refresh the mangle handle independently: the main rule's handle can + // be populated while the prerouting refresh during Flush failed, and + // gating the mangle refresh on the main handle would leak the mangle + // rule on delete. + if pr.mangleRule != nil && pr.mangleRule.Handle == 0 { + if err := r.refreshRuleHandles(r.chainPrerouting, true); err != nil { + return fmt.Errorf("refresh mangle handles: %w", err) } } @@ -285,6 +316,87 @@ func (r *family) applyNetwork( return nil, nil } +// applyPort builds the transport-header port match. A single value +// compares directly, a range uses a range expression, and multiple +// values go through an anonymous constant set: consecutive cmp +// expressions AND together, so chained equality comparisons could +// never match more than one port. The set is queued on the +// connection and committed by the caller's flush together with the +// rule that binds it. +func (r *family) applyPort(port *firewall.Port, isSource bool) ([]expr.Any, error) { + if port == nil || len(port.Values) == 0 { + return nil, nil + } + + // dst port + offset := uint32(2) + if isSource { + // src port + offset = 0 + } + + exprs := []expr.Any{ + &expr.Payload{ + DestRegister: 1, + Base: expr.PayloadBaseTransportHeader, + Offset: offset, + Len: 2, + }, + } + + switch { + case port.IsRange && len(port.Values) == 2: + exprs = append(exprs, &expr.Range{ + Op: expr.CmpOpEq, + Register: 1, + FromData: binaryutil.BigEndian.PutUint16(port.Values[0]), + ToData: binaryutil.BigEndian.PutUint16(port.Values[1]), + }) + case len(port.Values) == 1: + exprs = append(exprs, &expr.Cmp{ + Op: expr.CmpOpEq, + Register: 1, + Data: binaryutil.BigEndian.PutUint16(port.Values[0]), + }) + default: + set := &nftables.Set{ + Anonymous: true, + Constant: true, + Table: r.workTable, + KeyType: nftables.TypeInetService, + } + elements := make([]nftables.SetElement, 0, len(port.Values)) + for _, p := range port.Values { + elements = append(elements, nftables.SetElement{Key: binaryutil.BigEndian.PutUint16(p)}) + } + if err := r.conn.AddSet(set, elements); err != nil { + return nil, fmt.Errorf("add anonymous port set: %w", err) + } + exprs = append(exprs, &expr.Lookup{ + SourceRegister: 1, + SetID: set.ID, + SetName: set.Name, + }) + } + + return exprs, nil +} + +// applyPorts builds the source then destination port matches. +func (r *family) applyPorts(sPort, dPort *firewall.Port) ([]expr.Any, error) { + sPortExprs, err := r.applyPort(sPort, true) + if err != nil { + return nil, fmt.Errorf("apply source port: %w", err) + } + + dPortExprs, err := r.applyPort(dPort, false) + if err != nil { + return nil, fmt.Errorf("apply destination port: %w", err) + } + + return append(sPortExprs, dPortExprs...), nil +} + // prefixMatchExprs is the family-aware match sequence for a CIDR // prefix. /0 returns nil; a host prefix (full bit length for the // family) skips the bitwise step since the mask is all-ones. Shared @@ -332,58 +444,6 @@ func prefixMatchExprs(af addrFamily, prefix netip.Prefix, isSource bool) []expr. } } -func applyPort(port *firewall.Port, isSource bool) []expr.Any { - if port == nil { - return nil - } - - var exprs []expr.Any - - // src - offset := uint32(2) - if isSource { - // dst - offset = 0 - } - - exprs = append(exprs, &expr.Payload{ - DestRegister: 1, - Base: expr.PayloadBaseTransportHeader, - Offset: offset, - Len: 2, - }) - - if port.IsRange && len(port.Values) == 2 { - exprs = append(exprs, - &expr.Range{ - Op: expr.CmpOpEq, - Register: 1, - FromData: binaryutil.BigEndian.PutUint16(port.Values[0]), - ToData: binaryutil.BigEndian.PutUint16(port.Values[1]), - }, - ) - } else { - for i, p := range port.Values { - if i > 0 { - exprs = append(exprs, &expr.Bitwise{ - SourceRegister: 1, - DestRegister: 1, - Len: 4, - Mask: []byte{0x00, 0x00, 0xff, 0xff}, - Xor: []byte{0x00, 0x00, 0x00, 0x00}, - }) - } - exprs = append(exprs, &expr.Cmp{ - Op: expr.CmpOpEq, - Register: 1, - Data: binaryutil.BigEndian.PutUint16(p), - }) - } - } - - return exprs -} - func getCtNewExprs() []expr.Any { return []expr.Any{ &expr.Ct{ diff --git a/client/firewall/nftables/interface_allower_linux.go b/client/firewall/nftables/interface_allower_linux.go index d208d874d..e7232e2bf 100644 --- a/client/firewall/nftables/interface_allower_linux.go +++ b/client/firewall/nftables/interface_allower_linux.go @@ -35,10 +35,7 @@ type InterfaceAllower struct { func NewInterfaceAllower(wgIface iFaceMapper, mtu uint16) (*InterfaceAllower, error) { tableName := getTableName() - family4, err := newFamily(&nftables.Table{Name: tableName, Family: nftables.TableFamilyIPv4}, wgIface, mtu) - if err != nil { - return nil, fmt.Errorf("create family: %w", err) - } + family4 := newFamily(&nftables.Table{Name: tableName, Family: nftables.TableFamilyIPv4}, wgIface, mtu) // Probe nftables availability before committing to this backend. if _, err := family4.conn.ListChainsOfTableFamily(nftables.TableFamilyINet); err != nil { @@ -48,11 +45,7 @@ func NewInterfaceAllower(wgIface iFaceMapper, mtu uint16) (*InterfaceAllower, er a := &InterfaceAllower{family4: family4} if wgIface.Address().HasIPv6() { - family6, err := newFamily(&nftables.Table{Name: tableName, Family: nftables.TableFamilyIPv6}, wgIface, mtu) - if err != nil { - return nil, fmt.Errorf("create v6 family: %w", err) - } - a.family6 = family6 + a.family6 = newFamily(&nftables.Table{Name: tableName, Family: nftables.TableFamilyIPv6}, wgIface, mtu) } a.extMonitor = newExternalChainMonitor(a) diff --git a/client/firewall/nftables/manager_linux.go b/client/firewall/nftables/manager_linux.go index e00b0fd1a..f8153a177 100644 --- a/client/firewall/nftables/manager_linux.go +++ b/client/firewall/nftables/manager_linux.go @@ -71,16 +71,10 @@ func Create(wgIface iFaceMapper, mtu uint16) (*Manager, error) { tableName := getTableName() workTable := &nftables.Table{Name: tableName, Family: nftables.TableFamilyIPv4} - var err error - m.family4, err = newFamily(workTable, wgIface, mtu) - if err != nil { - return nil, fmt.Errorf("create family: %w", err) - } + m.family4 = newFamily(workTable, wgIface, mtu) if wgIface.Address().HasIPv6() { - if err := m.createIPv6Components(tableName, wgIface, mtu); err != nil { - return nil, fmt.Errorf("create IPv6 firewall: %w", err) - } + m.createIPv6Components(tableName, wgIface, mtu) } m.extMonitor = newExternalChainMonitor(m) @@ -88,20 +82,14 @@ func Create(wgIface iFaceMapper, mtu uint16) (*Manager, error) { return m, nil } -func (m *Manager) createIPv6Components(tableName string, wgIface iFaceMapper, mtu uint16) error { +func (m *Manager) createIPv6Components(tableName string, wgIface iFaceMapper, mtu uint16) { workTable6 := &nftables.Table{Name: tableName, Family: nftables.TableFamilyIPv6} - var err error - m.family6, err = newFamily(workTable6, wgIface, mtu) - if err != nil { - return fmt.Errorf("create v6 family: %w", err) - } + m.family6 = newFamily(workTable6, wgIface, mtu) // Share the same IP forwarding state with the v4 router, since // EnableIPForwarding controls both v4 and v6 sysctls. m.family6.ipFwdState = m.family4.ipFwdState - - return nil } // hasIPv6 reports whether the manager has IPv6 components initialized. @@ -265,14 +253,14 @@ func (m *Manager) AddFilterRule( } // DeleteFilterRule removes a filtering rule. The owning family is found -// by id, refreshing from the kernel if the in-memory caches miss so a -// stale cache cannot leak the rule. family.DeleteFilterRule is idempotent -// when the id is absent. +// by id in the in-memory filter maps, which are the only tracking for +// filter rules. family.DeleteFilterRule is idempotent when the id is +// absent. func (m *Manager) DeleteFilterRule(rule firewall.Rule) error { m.mutex.Lock() defer m.mutex.Unlock() - fam, err := m.familyForRuleID(rule.ID(), (*family).hasRule) + fam, err := m.familyForRuleID(rule.ID(), (*family).hasRule, false) if err != nil { return err } @@ -280,16 +268,21 @@ func (m *Manager) DeleteFilterRule(rule firewall.Rule) error { } // familyForRuleID picks the family holding the rule with the given id, using -// the supplied lookup. If the cached maps disagree (or both miss), it refreshes -// from the kernel once and re-checks before falling back to the v4 family. -func (m *Manager) familyForRuleID(id firewall.RuleID, has func(*family, firewall.RuleID) bool) (*family, error) { +// the supplied lookup. With refresh set, a miss in both cached maps reloads +// the NAT/DNAT rule maps from the kernel once and re-checks before falling +// back to the v4 family. Filter rules are tracked only in memory and have no +// kernel-backed reload, so their callers pass refresh as false. +func (m *Manager) familyForRuleID(id firewall.RuleID, has func(*family, firewall.RuleID) bool, refresh bool) (*family, error) { if has(m.family4, id) { return m.family4, nil } - if m.hasIPv6() && has(m.family6, id) { + if !m.hasIPv6() { + return m.family4, nil + } + if has(m.family6, id) { return m.family6, nil } - if !m.hasIPv6() { + if !refresh { return m.family4, nil } if err := m.family4.refreshRulesMap(); err != nil { @@ -463,7 +456,7 @@ func (m *Manager) Flush() error { if m.hasIPv6() { if err := m.family6.Flush(); err != nil { - return fmt.Errorf("flush v6 acl: %w", err) + return fmt.Errorf("flush v6 family: %w", err) } } @@ -493,7 +486,7 @@ func (m *Manager) DeleteDNATRule(rule firewall.Rule) error { m.mutex.Lock() defer m.mutex.Unlock() - r, err := m.familyForRuleID(rule.ID(), (*family).hasDNATRule) + r, err := m.familyForRuleID(rule.ID(), (*family).hasDNATRule, true) if err != nil { return err } diff --git a/client/firewall/nftables/manager_linux_test.go b/client/firewall/nftables/manager_linux_test.go index 809445ed2..e9077afba 100644 --- a/client/firewall/nftables/manager_linux_test.go +++ b/client/firewall/nftables/manager_linux_test.go @@ -601,6 +601,73 @@ func TestNftablesManagerCompatibilityWithIptablesForWildcardSource(t *testing.T) verifyIptablesOutput(t, stdout, stderr) } +func TestNftablesManagerMultiPortFilter(t *testing.T) { + if check() != NFTABLES { + t.Skip("nftables not supported on this system") + } + + manager, err := Create(ifaceMock, iface.DefaultMTU) + require.NoError(t, err) + require.NoError(t, manager.Init(nil)) + + t.Cleanup(func() { + require.NoError(t, manager.Close(nil), "failed to reset manager state") + }) + + ip := netip.MustParseAddr("100.96.0.1") + + rule, err := manager.AddFilterRule(nil, pfx(ip.AsSlice()), fw.Network{}, fw.ProtocolTCP, nil, &fw.Port{Values: []uint16{80, 443}}, fw.ActionAccept) + require.NoError(t, err, "failed to add multi-port rule") + + testClient := &nftables.Conn{} + rules, err := testClient.GetRules(manager.family4.workTable, manager.family4.chainInputRules) + require.NoError(t, err, "failed to get rules") + + var lookup *expr.Lookup + for _, kernelRule := range rules { + if string(kernelRule.UserData) != string(rule.ID()) { + continue + } + for _, e := range kernelRule.Exprs { + if l, ok := e.(*expr.Lookup); ok { + lookup = l + } + } + } + require.NotNil(t, lookup, "multi-port rule must match ports via a set lookup") + + sets, err := testClient.GetSets(manager.family4.workTable) + require.NoError(t, err, "failed to get sets") + + var portSet *nftables.Set + for _, s := range sets { + if s.Name == lookup.SetName { + portSet = s + } + } + require.NotNil(t, portSet, "anonymous port set not found in kernel") + + portSet.Table = manager.family4.workTable + elements, err := testClient.GetSetElements(portSet) + require.NoError(t, err, "failed to get set elements") + + ports := make(map[uint16]bool) + for _, e := range elements { + require.Len(t, e.Key, 2, "port set element key should be 2 bytes") + ports[binary.BigEndian.Uint16(e.Key)] = true + } + require.True(t, ports[80], "port set should contain port 80") + require.True(t, ports[443], "port set should contain port 443") + + require.NoError(t, manager.DeleteFilterRule(rule), "failed to delete rule") + + rules, err = testClient.GetRules(manager.family4.workTable, manager.family4.chainInputRules) + require.NoError(t, err, "failed to get rules after delete") + for _, kernelRule := range rules { + require.NotEqual(t, string(rule.ID()), string(kernelRule.UserData), "rule should be removed from kernel") + } +} + func compareExprsIgnoringCounters(t *testing.T, got, want []expr.Any) { t.Helper() require.Equal(t, len(got), len(want), "expression count mismatch") diff --git a/client/firewall/nftables/router_linux_test.go b/client/firewall/nftables/router_linux_test.go index 83d8a015c..fc2974a8d 100644 --- a/client/firewall/nftables/router_linux_test.go +++ b/client/firewall/nftables/router_linux_test.go @@ -200,8 +200,7 @@ func TestRouter_AddRouteFiltering(t *testing.T) { defer deleteWorkTable() - r, err := newFamily(workTable, ifaceMock, iface.DefaultMTU) - require.NoError(t, err, "Failed to create family") + r := newFamily(workTable, ifaceMock, iface.DefaultMTU) require.NoError(t, r.init(workTable)) defer func(r *family) { @@ -367,8 +366,7 @@ func TestNftablesCreateIpSet(t *testing.T) { defer deleteWorkTable() - r, err := newFamily(workTable, ifaceMock, iface.DefaultMTU) - require.NoError(t, err, "Failed to create family") + r := newFamily(workTable, ifaceMock, iface.DefaultMTU) require.NoError(t, r.init(workTable)) defer func() { @@ -523,8 +521,7 @@ func TestNftablesUpdateSetMergesOverlapping(t *testing.T) { require.NoError(t, err, "create work table") defer deleteWorkTable() - r, err := newFamily(workTable, ifaceMock, iface.DefaultMTU) - require.NoError(t, err, "create family") + r := newFamily(workTable, ifaceMock, iface.DefaultMTU) require.NoError(t, r.init(workTable)) defer func() { require.NoError(t, r.Reset(), "reset family") @@ -554,8 +551,7 @@ func TestNftablesCreateIpSet_IPv6(t *testing.T) { require.NoError(t, err, "Failed to create v6 work table") defer deleteWorkTableIPv6() - r, err := newFamily(workTable, ifaceMock, iface.DefaultMTU) - require.NoError(t, err, "Failed to create family") + r := newFamily(workTable, ifaceMock, iface.DefaultMTU) require.NoError(t, r.init(workTable)) defer func() { require.NoError(t, r.Reset(), "Failed to reset family") @@ -897,8 +893,7 @@ func TestRouter_RefreshRulesMap_RemovesStaleEntries(t *testing.T) { require.NoError(t, err) defer deleteWorkTable() - r, err := newFamily(workTable, ifaceMock, iface.DefaultMTU) - require.NoError(t, err) + r := newFamily(workTable, ifaceMock, iface.DefaultMTU) require.NoError(t, r.init(workTable)) defer func() { require.NoError(t, r.Reset()) }() @@ -952,8 +947,7 @@ func TestRouter_DeleteRouteRule_RemovesKernelRule(t *testing.T) { require.NoError(t, err) defer deleteWorkTable() - r, err := newFamily(workTable, ifaceMock, iface.DefaultMTU) - require.NoError(t, err) + r := newFamily(workTable, ifaceMock, iface.DefaultMTU) require.NoError(t, r.init(workTable)) defer func() { require.NoError(t, r.Reset()) }() @@ -996,8 +990,7 @@ func TestRouter_DeleteRouteRule_StaleHandle(t *testing.T) { require.NoError(t, err) defer deleteWorkTable() - r, err := newFamily(workTable, ifaceMock, iface.DefaultMTU) - require.NoError(t, err) + r := newFamily(workTable, ifaceMock, iface.DefaultMTU) require.NoError(t, r.init(workTable)) defer func() { require.NoError(t, r.Reset()) }() diff --git a/client/firewall/nftables/routing_linux.go b/client/firewall/nftables/routing_linux.go index b7f3c4e12..c526102ee 100644 --- a/client/firewall/nftables/routing_linux.go +++ b/client/firewall/nftables/routing_linux.go @@ -79,6 +79,7 @@ func (r *family) addNatRule(pair firewall.RouterPair) error { destExp, err := r.applyNetwork(pair.Destination, nil, false) if err != nil { + r.dropNetworkMatch(sourceExp) return fmt.Errorf("apply destination: %w", err) } @@ -126,6 +127,8 @@ func (r *family) addNatRule(pair firewall.RouterPair) error { if _, exists := r.rules[ruleID]; exists { if err := r.removeNatRule(pair); err != nil { + r.dropNetworkMatch(sourceExp) + r.dropNetworkMatch(destExp) return fmt.Errorf("remove prerouting rule: %w", err) } } @@ -302,6 +305,7 @@ func (r *family) addLegacyRouteRule(pair firewall.RouterPair) error { destExp, err := r.applyNetwork(pair.Destination, nil, false) if err != nil { + r.dropNetworkMatch(sourceExp) return fmt.Errorf("apply destination: %w", err) } @@ -317,6 +321,8 @@ func (r *family) addLegacyRouteRule(pair firewall.RouterPair) error { if _, exists := r.rules[ruleID]; exists { if err := r.removeLegacyRouteRule(pair); err != nil { + r.dropNetworkMatch(sourceExp) + r.dropNetworkMatch(destExp) return fmt.Errorf("remove legacy routing rule: %w", err) } }