From fc88399c232efe88b736dd41e248a8f809bdd837 Mon Sep 17 00:00:00 2001 From: Vlad <4941176+crn4@users.noreply.github.com> Date: Tue, 10 Feb 2026 18:31:15 +0100 Subject: [PATCH 1/5] [management] fixed ischild check (#5279) --- go.mod | 2 +- go.sum | 4 ++-- .../server/http/middleware/auth_middleware.go | 13 +++++++++---- .../server/http/middleware/auth_middleware_test.go | 10 ++++------ 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/go.mod b/go.mod index 2a6c311ce..801d52483 100644 --- a/go.mod +++ b/go.mod @@ -68,7 +68,7 @@ require ( github.com/mdlayher/socket v0.5.1 github.com/miekg/dns v1.1.59 github.com/mitchellh/hashstructure/v2 v2.0.2 - github.com/netbirdio/management-integrations/integrations v0.0.0-20260122111742-a6f99668844f + github.com/netbirdio/management-integrations/integrations v0.0.0-20260210160626-df4b180c7b25 github.com/netbirdio/signal-dispatcher/dispatcher v0.0.0-20250805121659-6b4ac470ca45 github.com/oapi-codegen/runtime v1.1.2 github.com/okta/okta-sdk-golang/v2 v2.18.0 diff --git a/go.sum b/go.sum index 17e5c8ffa..23a12ff68 100644 --- a/go.sum +++ b/go.sum @@ -406,8 +406,8 @@ github.com/netbirdio/go-netroute v0.0.0-20240611143515-f59b0e1d3944 h1:TDtJKmM6S github.com/netbirdio/go-netroute v0.0.0-20240611143515-f59b0e1d3944/go.mod h1:sHA6TRxjQ6RLbnI+3R4DZo2Eseg/iKiPRfNmcuNySVQ= github.com/netbirdio/ice/v4 v4.0.0-20250908184934-6202be846b51 h1:Ov4qdafATOgGMB1wbSuh+0aAHcwz9hdvB6VZjh1mVMI= github.com/netbirdio/ice/v4 v4.0.0-20250908184934-6202be846b51/go.mod h1:ZSIbPdBn5hePO8CpF1PekH2SfpTxg1PDhEwtbqZS7R8= -github.com/netbirdio/management-integrations/integrations v0.0.0-20260122111742-a6f99668844f h1:CTBf0je/FpKr2lVSMZLak7m8aaWcS6ur4SOfhSSazFI= -github.com/netbirdio/management-integrations/integrations v0.0.0-20260122111742-a6f99668844f/go.mod h1:y7CxagMYzg9dgu+masRqYM7BQlOGA5Y8US85MCNFPlY= +github.com/netbirdio/management-integrations/integrations v0.0.0-20260210160626-df4b180c7b25 h1:iwAq/Ncaq0etl4uAlVsbNBzC1yY52o0AmY7uCm2AMTs= +github.com/netbirdio/management-integrations/integrations v0.0.0-20260210160626-df4b180c7b25/go.mod h1:y7CxagMYzg9dgu+masRqYM7BQlOGA5Y8US85MCNFPlY= github.com/netbirdio/service v0.0.0-20240911161631-f62744f42502 h1:3tHlFmhTdX9axERMVN63dqyFqnvuD+EMJHzM7mNGON8= github.com/netbirdio/service v0.0.0-20240911161631-f62744f42502/go.mod h1:CIMRFEJVL+0DS1a3Nx06NaMn4Dz63Ng6O7dl0qH0zVM= github.com/netbirdio/signal-dispatcher/dispatcher v0.0.0-20250805121659-6b4ac470ca45 h1:ujgviVYmx243Ksy7NdSwrdGPSRNE3pb8kEDSpH0QuAQ= diff --git a/management/server/http/middleware/auth_middleware.go b/management/server/http/middleware/auth_middleware.go index 257347153..63be672e6 100644 --- a/management/server/http/middleware/auth_middleware.go +++ b/management/server/http/middleware/auth_middleware.go @@ -11,6 +11,7 @@ import ( log "github.com/sirupsen/logrus" "go.opentelemetry.io/otel/metric" + "github.com/netbirdio/management-integrations/integrations" serverauth "github.com/netbirdio/netbird/management/server/auth" nbcontext "github.com/netbirdio/netbird/management/server/context" "github.com/netbirdio/netbird/management/server/http/middleware/bypass" @@ -130,8 +131,10 @@ func (m *AuthMiddleware) checkJWTFromRequest(r *http.Request, authHeaderParts [] } if impersonate, ok := r.URL.Query()["account"]; ok && len(impersonate) == 1 { - userAuth.AccountId = impersonate[0] - userAuth.IsChild = ok + if integrations.IsValidChildAccount(ctx, userAuth.UserId, userAuth.AccountId, impersonate[0]) { + userAuth.AccountId = impersonate[0] + userAuth.IsChild = true + } } // Email is now extracted in ToUserAuth (from claims or userinfo endpoint) @@ -207,8 +210,10 @@ func (m *AuthMiddleware) checkPATFromRequest(r *http.Request, authHeaderParts [] } if impersonate, ok := r.URL.Query()["account"]; ok && len(impersonate) == 1 { - userAuth.AccountId = impersonate[0] - userAuth.IsChild = ok + if integrations.IsValidChildAccount(r.Context(), userAuth.UserId, userAuth.AccountId, impersonate[0]) { + userAuth.AccountId = impersonate[0] + userAuth.IsChild = true + } } return nbcontext.SetUserAuthInRequest(r, userAuth), nil diff --git a/management/server/http/middleware/auth_middleware_test.go b/management/server/http/middleware/auth_middleware_test.go index 05ca59419..f397c63a4 100644 --- a/management/server/http/middleware/auth_middleware_test.go +++ b/management/server/http/middleware/auth_middleware_test.go @@ -627,15 +627,14 @@ func TestAuthMiddleware_Handler_Child(t *testing.T) { }, }, { - name: "Valid PAT Token accesses child", + name: "PAT Token with account param ignored in public version", path: "/test?account=xyz", authHeader: "Token " + PAT, expectedUserAuth: &nbauth.UserAuth{ - AccountId: "xyz", + AccountId: accountID, UserId: userID, Domain: testAccount.Domain, DomainCategory: testAccount.DomainCategory, - IsChild: true, IsPAT: true, }, }, @@ -652,15 +651,14 @@ func TestAuthMiddleware_Handler_Child(t *testing.T) { }, { - name: "Valid JWT Token with child", + name: "JWT Token with account param ignored in public version", path: "/test?account=xyz", authHeader: "Bearer " + JWT, expectedUserAuth: &nbauth.UserAuth{ - AccountId: "xyz", + AccountId: accountID, UserId: userID, Domain: testAccount.Domain, DomainCategory: testAccount.DomainCategory, - IsChild: true, }, }, } From 2de19490186123c2412b1c11b5000686f1168399 Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Wed, 11 Feb 2026 21:42:36 +0100 Subject: [PATCH 2/5] [client] Check if login is required on foreground mode (#5295) --- client/cmd/login.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/client/cmd/login.go b/client/cmd/login.go index 64b45e557..4521a67c9 100644 --- a/client/cmd/login.go +++ b/client/cmd/login.go @@ -282,13 +282,9 @@ func foregroundLogin(ctx context.Context, cmd *cobra.Command, config *profileman } defer authClient.Close() - needsLogin := false - - err, isAuthError := authClient.Login(ctx, "", "") - if isAuthError { - needsLogin = true - } else if err != nil { - return fmt.Errorf("login check failed: %v", err) + needsLogin, err := authClient.IsLoginRequired(ctx) + if err != nil { + return fmt.Errorf("check login required: %v", err) } jwtToken := "" From 1ddc9ce2bf32833b973a70ee2eda2a335b3c3e57 Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Thu, 12 Feb 2026 16:15:42 +0800 Subject: [PATCH 3/5] [client] Fix nil pointer panic in device and engine code (#5287) --- client/iface/device/device_filter.go | 19 +++++++++++++++++-- client/iface/device/device_netstack.go | 4 +++- client/iface/netstack/tun.go | 2 +- client/internal/engine.go | 3 ++- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/client/iface/device/device_filter.go b/client/iface/device/device_filter.go index 015f71ff4..708f38d26 100644 --- a/client/iface/device/device_filter.go +++ b/client/iface/device/device_filter.go @@ -29,8 +29,9 @@ type PacketFilter interface { type FilteredDevice struct { tun.Device - filter PacketFilter - mutex sync.RWMutex + filter PacketFilter + mutex sync.RWMutex + closeOnce sync.Once } // newDeviceFilter constructor function @@ -40,6 +41,20 @@ func newDeviceFilter(device tun.Device) *FilteredDevice { } } +// Close closes the underlying tun device exactly once. +// wireguard-go's netTun.Close() panics on double-close due to a bare close(channel), +// and multiple code paths can trigger Close on the same device. +func (d *FilteredDevice) Close() error { + var err error + d.closeOnce.Do(func() { + err = d.Device.Close() + }) + if err != nil { + return err + } + return nil +} + // Read wraps read method with filtering feature func (d *FilteredDevice) Read(bufs [][]byte, sizes []int, offset int) (n int, err error) { if n, err = d.Device.Read(bufs, sizes, offset); err != nil { diff --git a/client/iface/device/device_netstack.go b/client/iface/device/device_netstack.go index 40d8fdac8..e457657f7 100644 --- a/client/iface/device/device_netstack.go +++ b/client/iface/device/device_netstack.go @@ -82,7 +82,9 @@ func (t *TunNetstackDevice) create() (WGConfigurer, error) { t.configurer = configurer.NewUSPConfigurer(t.device, t.name, t.bind.ActivityRecorder()) err = t.configurer.ConfigureInterface(t.key, t.port) if err != nil { - _ = tunIface.Close() + if cErr := tunIface.Close(); cErr != nil { + log.Debugf("failed to close tun device: %v", cErr) + } return nil, fmt.Errorf("error configuring interface: %s", err) } diff --git a/client/iface/netstack/tun.go b/client/iface/netstack/tun.go index b2506b50d..346ae29ec 100644 --- a/client/iface/netstack/tun.go +++ b/client/iface/netstack/tun.go @@ -66,7 +66,7 @@ func (t *NetStackTun) Create() (tun.Device, *netstack.Net, error) { } }() - return nsTunDev, tunNet, nil + return t.tundev, tunNet, nil } func (t *NetStackTun) Close() error { diff --git a/client/internal/engine.go b/client/internal/engine.go index 4dbd5f45e..631910eb6 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -543,11 +543,12 @@ func (e *Engine) Start(netbirdConfig *mgmProto.NetbirdConfig, mgmtURL *url.URL) // monitor WireGuard interface lifecycle and restart engine on changes e.wgIfaceMonitor = NewWGIfaceMonitor() e.shutdownWg.Add(1) + wgIfaceName := e.wgInterface.Name() go func() { defer e.shutdownWg.Done() - if shouldRestart, err := e.wgIfaceMonitor.Start(e.ctx, e.wgInterface.Name()); shouldRestart { + if shouldRestart, err := e.wgIfaceMonitor.Start(e.ctx, wgIfaceName); shouldRestart { log.Infof("WireGuard interface monitor: %s, restarting engine", err) e.triggerClientRestart() } else if err != nil { From 3dfa97dcbde64006d0e7e21160cd8242c66e0b6e Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Thu, 12 Feb 2026 16:15:57 +0800 Subject: [PATCH 4/5] [client] Fix stale entries in nftables with no handle (#5272) --- client/firewall/nftables/router_linux.go | 179 +++++++++++++----- client/firewall/nftables/router_linux_test.go | 135 +++++++++++++ 2 files changed, 266 insertions(+), 48 deletions(-) diff --git a/client/firewall/nftables/router_linux.go b/client/firewall/nftables/router_linux.go index b6e0cf5b2..fde654c20 100644 --- a/client/firewall/nftables/router_linux.go +++ b/client/firewall/nftables/router_linux.go @@ -483,7 +483,12 @@ func (r *router) DeleteRouteRule(rule firewall.Rule) error { } if nftRule.Handle == 0 { - return fmt.Errorf("route rule %s has no handle", ruleKey) + log.Warnf("route rule %s has no handle, removing stale entry", ruleKey) + if err := r.decrementSetCounter(nftRule); err != nil { + log.Warnf("decrement set counter for stale rule %s: %v", ruleKey, err) + } + delete(r.rules, ruleKey) + return nil } if err := r.deleteNftRule(nftRule, ruleKey); err != nil { @@ -660,13 +665,32 @@ func (r *router) AddNatRule(pair firewall.RouterPair) error { } if err := r.conn.Flush(); err != nil { - // TODO: rollback ipset counter - return fmt.Errorf("insert rules for %s: %v", pair.Destination, err) + r.rollbackRules(pair) + return fmt.Errorf("insert rules for %s: %w", pair.Destination, err) } return nil } +// rollbackRules cleans up unflushed rules and their set counters after a flush failure. +func (r *router) rollbackRules(pair firewall.RouterPair) { + keys := []string{ + firewall.GenKey(firewall.ForwardingFormat, pair), + firewall.GenKey(firewall.PreroutingFormat, pair), + firewall.GenKey(firewall.PreroutingFormat, firewall.GetInversePair(pair)), + } + for _, key := range keys { + rule, ok := r.rules[key] + if !ok { + continue + } + if err := r.decrementSetCounter(rule); err != nil { + log.Warnf("rollback set counter for %s: %v", key, err) + } + delete(r.rules, key) + } +} + // addNatRule inserts a nftables rule to the conn client flush queue func (r *router) addNatRule(pair firewall.RouterPair) error { sourceExp, err := r.applyNetwork(pair.Source, nil, true) @@ -928,18 +952,30 @@ func (r *router) addLegacyRouteRule(pair firewall.RouterPair) error { func (r *router) removeLegacyRouteRule(pair firewall.RouterPair) error { ruleKey := firewall.GenKey(firewall.ForwardingFormat, pair) - if rule, exists := r.rules[ruleKey]; exists { - if err := r.conn.DelRule(rule); err != nil { - return fmt.Errorf("remove legacy forwarding rule %s -> %s: %v", pair.Source, pair.Destination, err) - } - - log.Debugf("removed legacy forwarding rule %s -> %s", pair.Source, pair.Destination) - - delete(r.rules, ruleKey) + rule, exists := r.rules[ruleKey] + if !exists { + return nil + } + if rule.Handle == 0 { + log.Warnf("legacy forwarding rule %s has no handle, removing stale entry", ruleKey) if err := r.decrementSetCounter(rule); err != nil { - return fmt.Errorf("decrement set counter: %w", err) + log.Warnf("decrement set counter for stale rule %s: %v", ruleKey, err) } + delete(r.rules, ruleKey) + return nil + } + + if err := r.conn.DelRule(rule); err != nil { + return fmt.Errorf("remove legacy forwarding rule %s -> %s: %w", pair.Source, pair.Destination, err) + } + + log.Debugf("removed legacy forwarding rule %s -> %s", pair.Source, pair.Destination) + + delete(r.rules, ruleKey) + + if err := r.decrementSetCounter(rule); err != nil { + return fmt.Errorf("decrement set counter: %w", err) } return nil @@ -1329,65 +1365,89 @@ func (r *router) RemoveNatRule(pair firewall.RouterPair) error { return fmt.Errorf(refreshRulesMapError, err) } + var merr *multierror.Error + if pair.Masquerade { if err := r.removeNatRule(pair); err != nil { - return fmt.Errorf("remove prerouting rule: %w", err) + merr = multierror.Append(merr, fmt.Errorf("remove prerouting rule: %w", err)) } if err := r.removeNatRule(firewall.GetInversePair(pair)); err != nil { - return fmt.Errorf("remove inverse prerouting rule: %w", err) + merr = multierror.Append(merr, fmt.Errorf("remove inverse prerouting rule: %w", err)) } } if err := r.removeLegacyRouteRule(pair); err != nil { - return fmt.Errorf("remove legacy routing rule: %w", err) + merr = multierror.Append(merr, fmt.Errorf("remove legacy routing rule: %w", err)) } + // Set counters are decremented in the sub-methods above before flush. If flush fails, + // counters will be off until the next successful removal or refresh cycle. if err := r.conn.Flush(); err != nil { - // TODO: rollback set counter - return fmt.Errorf("remove nat rules rule %s: %v", pair.Destination, err) + merr = multierror.Append(merr, fmt.Errorf("flush remove nat rules %s: %w", pair.Destination, err)) } - return nil + return nberrors.FormatErrorOrNil(merr) } func (r *router) removeNatRule(pair firewall.RouterPair) error { ruleKey := firewall.GenKey(firewall.PreroutingFormat, pair) - if rule, exists := r.rules[ruleKey]; exists { - if err := r.conn.DelRule(rule); err != nil { - return fmt.Errorf("remove prerouting rule %s -> %s: %v", pair.Source, pair.Destination, err) - } - - log.Debugf("removed prerouting rule %s -> %s", pair.Source, pair.Destination) - - delete(r.rules, ruleKey) - - if err := r.decrementSetCounter(rule); err != nil { - return fmt.Errorf("decrement set counter: %w", err) - } - } else { + rule, exists := r.rules[ruleKey] + if !exists { log.Debugf("prerouting rule %s not found", ruleKey) + return nil + } + + if rule.Handle == 0 { + log.Warnf("prerouting rule %s has no handle, removing stale entry", ruleKey) + if err := r.decrementSetCounter(rule); err != nil { + log.Warnf("decrement set counter for stale rule %s: %v", ruleKey, err) + } + delete(r.rules, ruleKey) + return nil + } + + if err := r.conn.DelRule(rule); err != nil { + return fmt.Errorf("remove prerouting rule %s -> %s: %w", pair.Source, pair.Destination, err) + } + + log.Debugf("removed prerouting rule %s -> %s", pair.Source, pair.Destination) + + delete(r.rules, ruleKey) + + if err := r.decrementSetCounter(rule); err != nil { + return fmt.Errorf("decrement set counter: %w", err) } return nil } -// refreshRulesMap refreshes the rule map with the latest rules. this is useful to avoid -// duplicates and to get missing attributes that we don't have when adding new rules +// refreshRulesMap rebuilds the rule map from the kernel. This removes stale entries +// (e.g. from failed flushes) and updates handles for all existing rules. func (r *router) refreshRulesMap() error { + var merr *multierror.Error + newRules := make(map[string]*nftables.Rule) for _, chain := range r.chains { rules, err := r.conn.GetRules(chain.Table, chain) if err != nil { - return fmt.Errorf("list rules: %w", err) + merr = multierror.Append(merr, fmt.Errorf("list rules for chain %s: %w", chain.Name, err)) + // preserve existing entries for this chain since we can't verify their state + for k, v := range r.rules { + if v.Chain != nil && v.Chain.Name == chain.Name { + newRules[k] = v + } + } + continue } for _, rule := range rules { if len(rule.UserData) > 0 { - r.rules[string(rule.UserData)] = rule + newRules[string(rule.UserData)] = rule } } } - return nil + r.rules = newRules + return nberrors.FormatErrorOrNil(merr) } func (r *router) AddDNATRule(rule firewall.ForwardRule) (firewall.Rule, error) { @@ -1629,20 +1689,34 @@ func (r *router) DeleteDNATRule(rule firewall.Rule) error { } var merr *multierror.Error + var needsFlush bool + if dnatRule, exists := r.rules[ruleKey+dnatSuffix]; exists { - if err := r.conn.DelRule(dnatRule); err != nil { + if dnatRule.Handle == 0 { + log.Warnf("dnat rule %s has no handle, removing stale entry", ruleKey+dnatSuffix) + delete(r.rules, ruleKey+dnatSuffix) + } else if err := r.conn.DelRule(dnatRule); err != nil { merr = multierror.Append(merr, fmt.Errorf("delete dnat rule: %w", err)) + } else { + needsFlush = true } } if masqRule, exists := r.rules[ruleKey+snatSuffix]; exists { - if err := r.conn.DelRule(masqRule); err != nil { + if masqRule.Handle == 0 { + log.Warnf("snat rule %s has no handle, removing stale entry", ruleKey+snatSuffix) + delete(r.rules, ruleKey+snatSuffix) + } else if err := r.conn.DelRule(masqRule); err != nil { merr = multierror.Append(merr, fmt.Errorf("delete snat rule: %w", err)) + } else { + needsFlush = true } } - if err := r.conn.Flush(); err != nil { - merr = multierror.Append(merr, fmt.Errorf(flushError, err)) + if needsFlush { + if err := r.conn.Flush(); err != nil { + merr = multierror.Append(merr, fmt.Errorf(flushError, err)) + } } if merr == nil { @@ -1757,16 +1831,25 @@ func (r *router) RemoveInboundDNAT(localAddr netip.Addr, protocol firewall.Proto ruleID := fmt.Sprintf("inbound-dnat-%s-%s-%d-%d", localAddr.String(), protocol, sourcePort, targetPort) - if rule, exists := r.rules[ruleID]; exists { - if err := r.conn.DelRule(rule); err != nil { - return fmt.Errorf("delete inbound DNAT rule %s: %w", ruleID, err) - } - if err := r.conn.Flush(); err != nil { - return fmt.Errorf("flush delete inbound DNAT rule: %w", err) - } - delete(r.rules, ruleID) + rule, exists := r.rules[ruleID] + if !exists { + return nil } + if rule.Handle == 0 { + log.Warnf("inbound DNAT rule %s has no handle, removing stale entry", ruleID) + delete(r.rules, ruleID) + return nil + } + + if err := r.conn.DelRule(rule); err != nil { + return fmt.Errorf("delete inbound DNAT rule %s: %w", ruleID, err) + } + if err := r.conn.Flush(); err != nil { + return fmt.Errorf("flush delete inbound DNAT rule: %w", err) + } + delete(r.rules, ruleID) + return nil } diff --git a/client/firewall/nftables/router_linux_test.go b/client/firewall/nftables/router_linux_test.go index 3531b014b..f0e34d211 100644 --- a/client/firewall/nftables/router_linux_test.go +++ b/client/firewall/nftables/router_linux_test.go @@ -18,6 +18,7 @@ import ( firewall "github.com/netbirdio/netbird/client/firewall/manager" "github.com/netbirdio/netbird/client/firewall/test" "github.com/netbirdio/netbird/client/iface" + "github.com/netbirdio/netbird/client/internal/acl/id" ) const ( @@ -719,3 +720,137 @@ func deleteWorkTable() { } } } + +func TestRouter_RefreshRulesMap_RemovesStaleEntries(t *testing.T) { + if check() != NFTABLES { + t.Skip("nftables not supported on this system") + } + + workTable, err := createWorkTable() + require.NoError(t, err) + defer deleteWorkTable() + + r, err := newRouter(workTable, ifaceMock, iface.DefaultMTU) + require.NoError(t, err) + require.NoError(t, r.init(workTable)) + defer func() { require.NoError(t, r.Reset()) }() + + // Add a real rule to the kernel + ruleKey, err := r.AddRouteFiltering( + nil, + []netip.Prefix{netip.MustParsePrefix("192.168.1.0/24")}, + firewall.Network{Prefix: netip.MustParsePrefix("10.0.0.0/24")}, + firewall.ProtocolTCP, + nil, + &firewall.Port{Values: []uint16{80}}, + firewall.ActionAccept, + ) + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, r.DeleteRouteRule(ruleKey)) + }) + + // Inject a stale entry with Handle=0 (simulates store-before-flush failure) + staleKey := "stale-rule-that-does-not-exist" + r.rules[staleKey] = &nftables.Rule{ + Table: r.workTable, + Chain: r.chains[chainNameRoutingFw], + Handle: 0, + UserData: []byte(staleKey), + } + + require.Contains(t, r.rules, staleKey, "stale entry should be in map before refresh") + + err = r.refreshRulesMap() + require.NoError(t, err) + + assert.NotContains(t, r.rules, staleKey, "stale entry should be removed after refresh") + + realRule, ok := r.rules[ruleKey.ID()] + assert.True(t, ok, "real rule should still exist after refresh") + assert.NotZero(t, realRule.Handle, "real rule should have a valid handle") +} + +func TestRouter_DeleteRouteRule_StaleHandle(t *testing.T) { + if check() != NFTABLES { + t.Skip("nftables not supported on this system") + } + + workTable, err := createWorkTable() + require.NoError(t, err) + defer deleteWorkTable() + + r, err := newRouter(workTable, ifaceMock, iface.DefaultMTU) + require.NoError(t, err) + require.NoError(t, r.init(workTable)) + defer func() { require.NoError(t, r.Reset()) }() + + // Inject a stale entry with Handle=0 + staleKey := "stale-route-rule" + r.rules[staleKey] = &nftables.Rule{ + Table: r.workTable, + Chain: r.chains[chainNameRoutingFw], + Handle: 0, + UserData: []byte(staleKey), + } + + // DeleteRouteRule should not return an error for stale handles + err = r.DeleteRouteRule(id.RuleID(staleKey)) + assert.NoError(t, err, "deleting a stale rule should not error") + assert.NotContains(t, r.rules, staleKey, "stale entry should be cleaned up") +} + +func TestRouter_AddNatRule_WithStaleEntry(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)) + }) + + pair := firewall.RouterPair{ + ID: "staletest", + Source: firewall.Network{Prefix: netip.MustParsePrefix("100.100.100.1/32")}, + Destination: firewall.Network{Prefix: netip.MustParsePrefix("100.100.200.0/24")}, + Masquerade: true, + } + + rtr := manager.router + + // First add succeeds + err = rtr.AddNatRule(pair) + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, rtr.RemoveNatRule(pair)) + }) + + // Corrupt the handle to simulate stale state + natRuleKey := firewall.GenKey(firewall.PreroutingFormat, pair) + if rule, exists := rtr.rules[natRuleKey]; exists { + rule.Handle = 0 + } + inverseKey := firewall.GenKey(firewall.PreroutingFormat, firewall.GetInversePair(pair)) + if rule, exists := rtr.rules[inverseKey]; exists { + rule.Handle = 0 + } + + // Adding the same rule again should succeed despite stale handles + err = rtr.AddNatRule(pair) + assert.NoError(t, err, "AddNatRule should succeed even with stale entries") + + // Verify rules exist in kernel + rules, err := rtr.conn.GetRules(rtr.workTable, rtr.chains[chainNameManglePrerouting]) + require.NoError(t, err) + + found := 0 + for _, rule := range rules { + if len(rule.UserData) > 0 && string(rule.UserData) == natRuleKey { + found++ + } + } + assert.Equal(t, 1, found, "NAT rule should exist in kernel") +} From 69d4b5d821b609eb834dc1f93c267b88699f94d0 Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Thu, 12 Feb 2026 11:31:49 +0100 Subject: [PATCH 5/5] [misc] Update sign pipeline version (#5296) --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 84f6f64ed..967e0c7d7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -9,7 +9,7 @@ on: pull_request: env: - SIGN_PIPE_VER: "v0.1.0" + SIGN_PIPE_VER: "v0.1.1" GORELEASER_VER: "v2.3.2" PRODUCT_NAME: "NetBird" COPYRIGHT: "NetBird GmbH"