diff --git a/client/internal/dns/host_windows.go b/client/internal/dns/host_windows.go index 948000a3d..6462d0c37 100644 --- a/client/internal/dns/host_windows.go +++ b/client/internal/dns/host_windows.go @@ -124,19 +124,9 @@ func newHostManager(wgInterface WGIface) (*registryConfigurator, error) { return nil, err } - var useGPO bool - k, err := registry.OpenKey(registry.LOCAL_MACHINE, GPODNSPolicyConfigRoot, registry.QUERY_VALUE) - if err != nil { - log.Debugf("failed to open GPO DNS policy root: %v", err) - } else { - closer(k) - useGPO = true - log.Infof("detected GPO DNS policy configuration, using policy store") - } - configurator := ®istryConfigurator{ guid: guid, - gpo: useGPO, + gpo: useGPOPolicyStore(), } origNameservers, err := configurator.captureOriginalNameservers() @@ -576,14 +566,22 @@ func (r *registryConfigurator) setInterfaceRegistryKeyStringValue(key, value str return nil } +// deleteInterfaceRegistryKeyProperty removes a value from the interface key. +// A value that is already gone, or an interface key that is, is not an error: +// the caller asked for the value not to be there, and a cleanup that runs twice +// has to reach its later steps on the second run as well. func (r *registryConfigurator) deleteInterfaceRegistryKeyProperty(propertyKey string) error { regKey, err := r.getInterfaceRegistryKey() - if err != nil { + switch { + case errors.Is(err, registry.ErrNotExist), errors.Is(err, syscall.ERROR_PATH_NOT_FOUND): + log.Debugf("interface key of %s does not exist, nothing to delete %s from", r.guid, propertyKey) + return nil + case err != nil: return fmt.Errorf("get interface registry key: %w", err) } defer closer(regKey) - if err := regKey.DeleteValue(propertyKey); err != nil { + if err := regKey.DeleteValue(propertyKey); err != nil && !errors.Is(err, registry.ErrNotExist) { return fmt.Errorf("delete registry key %s: %w", propertyKey, err) } return nil @@ -612,7 +610,12 @@ func (r *registryConfigurator) restoreHostDNS() error { go r.flushDNSCache() - return nil + // Last, and only on the way out, once no rule of ours is left: during a + // session the store is where the rules of this run live, and emptying it + // mid-session would have the next rule recreate it anyway. Propagated so a + // failure keeps the shutdown state for the next run to retry, rather than + // leaving the store to hold up every rule change from here on. + return removeEmptyGPOPolicyStore() } // removeDNSMatchPolicies deletes every NRPT rule this client may have created, @@ -651,6 +654,73 @@ func (r *registryConfigurator) restoreUncleanShutdownDNS() error { return r.restoreHostDNS() } +// useGPOPolicyStore reports whether NRPT rules have to go into the group policy +// store, and clears an empty one out of the way first. +// +// The order is the point. A store left empty by an earlier run would otherwise +// decide this run too, sending its rules somewhere the resolver only reads when +// the policy engine next applies DNS client policy. Removing it before the +// choice is made leaves the local store authoritative for the whole session, +// including the first one after an upgrade. +func useGPOPolicyStore() bool { + if err := removeEmptyGPOPolicyStore(); err != nil { + // Nothing to retry against here: the worst case is the run going + // through the group policy store, which is where it would have gone + // before this check existed. + log.Warnf("%v", err) + } + + k, err := registry.OpenKey(registry.LOCAL_MACHINE, GPODNSPolicyConfigRoot, registry.QUERY_VALUE) + if err != nil { + log.Debugf("failed to open GPO DNS policy root: %v", err) + return false + } + closer(k) + + log.Infof("detected GPO DNS policy configuration, using policy store") + return true +} + +// removeEmptyGPOPolicyStore deletes the group policy DnsPolicyConfig key once +// nothing is left in it. The key survives the deletion of the last rule it +// held, and the client treats its presence as "group policy configures the +// NRPT", so an empty one left behind keeps every later run writing rules there. +// Rules in that store reach the resolver only when the policy engine next +// applies DNS client policy, and a rule this client writes belongs to no GPO, +// so nothing schedules that application: both adding and removing a rule are +// held up by a minute or more, and for a removal that is a catch-all rule +// resolving every name over an interface that no longer exists. With the store +// absent the local one is authoritative and a change applies at once. +// +// A store that still holds rules, values or subkeys of somebody else's is left +// alone. +func removeEmptyGPOPolicyStore() error { + k, err := registry.OpenKey(registry.LOCAL_MACHINE, GPODNSPolicyConfigRoot, registry.QUERY_VALUE) + switch { + case errors.Is(err, registry.ErrNotExist), errors.Is(err, syscall.ERROR_PATH_NOT_FOUND): + return nil + case err != nil: + return fmt.Errorf("open HKEY_LOCAL_MACHINE\\%s: %w", GPODNSPolicyConfigRoot, err) + } + + info, err := k.Stat() + closer(k) + if err != nil { + return fmt.Errorf("stat HKEY_LOCAL_MACHINE\\%s: %w", GPODNSPolicyConfigRoot, err) + } + + if info.SubKeyCount != 0 || info.ValueCount != 0 { + return nil + } + + if err := registry.DeleteKey(registry.LOCAL_MACHINE, GPODNSPolicyConfigRoot); err != nil { + return fmt.Errorf("delete empty HKEY_LOCAL_MACHINE\\%s: %w", GPODNSPolicyConfigRoot, err) + } + + log.Infof("removed the empty GPO DNS policy store, leaving the local one authoritative") + return nil +} + // listNRPTRuleKeys returns the names of our NRPT rule keys under a policy store // root. An absent root holds nothing to clean up, which is the normal state of // the GPO store on a machine without DNS Client policy. diff --git a/client/internal/dns/host_windows_test.go b/client/internal/dns/host_windows_test.go index 7aef64590..353f6adbc 100644 --- a/client/internal/dns/host_windows_test.go +++ b/client/internal/dns/host_windows_test.go @@ -8,6 +8,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/sys/windows/registry" + + "github.com/netbirdio/netbird/client/internal/winregistry" ) // TestNRPTEntriesCleanupOnConfigChange tests that old NRPT entries are properly cleaned up @@ -405,3 +407,130 @@ func TestNRPTDomainBatching(t *testing.T) { }) } } + +// TestRemoveEmptyGPOPolicyStore verifies that cleanup takes the GPO policy +// store itself with it once our rules are gone, since the store existing keeps +// the local one from being applied, and that a store with somebody else's rule +// in it is left alone. +func TestRemoveEmptyGPOPolicyStore(t *testing.T) { + if testing.Short() { + t.Skip("skipping registry integration test in short mode") + } + + t.Cleanup(func() { cleanupRegistryKeys(t) }) + cleanupRegistryKeys(t) + + testIP := netip.MustParseAddr("100.64.0.1") + cfg := ®istryConfigurator{gpo: true} + + // a store holding a rule of ours is kept, because the rule is still applied + require.NoError(t, cfg.addDNSMatchPolicy([]string{".example.com"}, testIP)) + exists, err := registryKeyExists(gpoDnsPolicyConfigMatchPath + "-0") + require.NoError(t, err) + require.True(t, exists, "Should write the rule to the GPO policy store") + + require.NoError(t, removeEmptyGPOPolicyStore()) + exists, err = registryKeyExists(GPODNSPolicyConfigRoot) + require.NoError(t, err) + assert.True(t, exists, "Should keep a policy store that still holds a rule") + + // once the rules are gone the store goes with them + require.NoError(t, cfg.removeDNSMatchPolicies()) + require.NoError(t, removeEmptyGPOPolicyStore()) + + exists, err = registryKeyExists(GPODNSPolicyConfigRoot) + require.NoError(t, err) + assert.False(t, exists, "Should remove the GPO policy store once it is empty") + + // A store is not ours to remove while somebody else has a rule in it. The + // rule is written volatile like our own: the rules above created the parent + // chain volatile, and Windows refuses a stable subkey under a volatile + // parent. + foreignRule := GPODNSPolicyConfigRoot + `\{2A3B4C5D-6E7F-4041-8283-84858687888A}` + foreignKey, _, err := winregistry.CreateVolatileKey(registry.LOCAL_MACHINE, foreignRule, registry.SET_VALUE) + require.NoError(t, err, "Should create a foreign GPO rule") + foreignKey.Close() + t.Cleanup(func() { + _ = registry.DeleteKey(registry.LOCAL_MACHINE, foreignRule) + _ = registry.DeleteKey(registry.LOCAL_MACHINE, GPODNSPolicyConfigRoot) + }) + + require.NoError(t, cfg.removeDNSMatchPolicies()) + require.NoError(t, removeEmptyGPOPolicyStore()) + + exists, err = registryKeyExists(foreignRule) + require.NoError(t, err) + assert.True(t, exists, "Should not remove a foreign rule") + exists, err = registryKeyExists(GPODNSPolicyConfigRoot) + require.NoError(t, err) + assert.True(t, exists, "Should keep a policy store that still holds a foreign rule") +} + +// TestDeleteInterfaceRegistryKeyPropertyTwice verifies that removing a value +// that is already gone, or one on an interface key that is, reports success. +// Teardown runs again after a failed cleanup, and the steps that follow this +// one have to be reached on that second run. +func TestDeleteInterfaceRegistryKeyPropertyTwice(t *testing.T) { + if testing.Short() { + t.Skip("skipping registry integration test in short mode") + } + + testGUID := "{12345678-1234-1234-1234-123456789ABC}" + interfacePath := InterfaceConfigPath + `\` + testGUID + testKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, interfacePath, registry.SET_VALUE) + require.NoError(t, err, "Should create test interface registry key") + testKey.Close() + t.Cleanup(func() { + _ = registry.DeleteKey(registry.LOCAL_MACHINE, interfacePath) + }) + + cfg := ®istryConfigurator{guid: testGUID} + + require.NoError(t, cfg.setInterfaceRegistryKeyStringValue(interfaceConfigSearchListKey, "example.com")) + require.NoError(t, cfg.deleteInterfaceRegistryKeyProperty(interfaceConfigSearchListKey)) + assert.NoError(t, cfg.deleteInterfaceRegistryKeyProperty(interfaceConfigSearchListKey), + "Should report success for a value that is already gone") + + // and with the interface key itself gone, as it is once the adapter is + require.NoError(t, registry.DeleteKey(registry.LOCAL_MACHINE, interfacePath)) + assert.NoError(t, cfg.deleteInterfaceRegistryKeyProperty(interfaceConfigSearchListKey), + "Should report success when the interface key does not exist") +} + +// TestUseGPOPolicyStoreClearsEmptyStore verifies that the store is cleared +// before it is consulted, so an empty one left by an earlier run does not send +// this run's rules to the group policy store. A store somebody else has a rule +// in still decides where the rules go. +func TestUseGPOPolicyStoreClearsEmptyStore(t *testing.T) { + if testing.Short() { + t.Skip("skipping registry integration test in short mode") + } + + t.Cleanup(func() { cleanupRegistryKeys(t) }) + cleanupRegistryKeys(t) + + // the leftover an earlier run used to keep, which the client read as + // "group policy configures the NRPT" for every run after it + emptyStore, _, err := winregistry.CreateVolatileKey(registry.LOCAL_MACHINE, GPODNSPolicyConfigRoot, registry.SET_VALUE) + require.NoError(t, err, "Should create the GPO policy store") + emptyStore.Close() + + assert.False(t, useGPOPolicyStore(), "An empty store should not decide where the rules go") + exists, err := registryKeyExists(GPODNSPolicyConfigRoot) + require.NoError(t, err) + assert.False(t, exists, "Should clear the empty store before consulting it") + + foreignRule := GPODNSPolicyConfigRoot + `\{2A3B4C5D-6E7F-4041-8283-84858687888A}` + foreignKey, _, err := winregistry.CreateVolatileKey(registry.LOCAL_MACHINE, foreignRule, registry.SET_VALUE) + require.NoError(t, err, "Should create a foreign GPO rule") + foreignKey.Close() + t.Cleanup(func() { + _ = registry.DeleteKey(registry.LOCAL_MACHINE, foreignRule) + _ = registry.DeleteKey(registry.LOCAL_MACHINE, GPODNSPolicyConfigRoot) + }) + + assert.True(t, useGPOPolicyStore(), "A store holding a rule should decide where the rules go") + exists, err = registryKeyExists(GPODNSPolicyConfigRoot) + require.NoError(t, err) + assert.True(t, exists, "Should keep a store that holds a rule") +} diff --git a/client/internal/engine.go b/client/internal/engine.go index 78e4dcbc9..a8d6bc0f6 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -1071,7 +1071,11 @@ func (e *Engine) handleSync(update *mgmProto.SyncResponse) error { // back to empty if the FQDN doesn't have the expected shape. dnsName = extractDNSDomainFromFQDN(pc.GetFqdn()) } - result, err := nbnetworkmap.EnvelopeToNetworkMap(e.ctx, envelope, localKey, dnsName) + // With the firewall disabled there is no ACL manager to program, so + // RoutesFirewallRules would be built and then dropped. On a peer that + // routes many network resources that is the single most expensive + // step of the sync. + result, err := nbnetworkmap.EnvelopeToNetworkMap(e.ctx, envelope, localKey, dnsName, e.config.DisableFirewall) if err != nil { return fmt.Errorf("decode network map envelope: %w", err) } diff --git a/management/internals/controllers/network_map/nmaptest/runner.go b/management/internals/controllers/network_map/nmaptest/runner.go index 0d6ac9c18..ffce6483e 100644 --- a/management/internals/controllers/network_map/nmaptest/runner.go +++ b/management/internals/controllers/network_map/nmaptest/runner.go @@ -245,7 +245,7 @@ func computeMode(t *testing.T, ctx context.Context, mode Mode, nmData *networkma peerGroups := maps.Keys(nmData.GetPeerGroups(peerID)) resp := mgmtgrpc.ToComponentSyncResponse(ctx, nil, nil, nil, peer, nil, nil, components, nil, dnsDomain, nil, nmData.AccountSettings, nil, peerGroups, dnsFwdPort) - res, err := networkmap.EnvelopeToNetworkMap(ctx, resp.NetworkMapEnvelope, peer.Key, dnsDomain) + res, err := networkmap.EnvelopeToNetworkMap(ctx, resp.NetworkMapEnvelope, peer.Key, dnsDomain, false) require.NoError(t, err, "expand envelope") return res.NetworkMap default: diff --git a/management/server/networks/resources/types/resource.go b/management/server/networks/resources/types/resource.go index 4cf7f7ea3..bb33e00eb 100644 --- a/management/server/networks/resources/types/resource.go +++ b/management/server/networks/resources/types/resource.go @@ -32,7 +32,7 @@ type NetworkResource struct { ID string `gorm:"primaryKey"` NetworkID string `gorm:"index"` AccountID string `gorm:"index"` - PublicID string `json:"-"` + PublicID string `json:"-" gorm:"index"` Name string Description string Type NetworkResourceType diff --git a/management/server/store/sql_store.go b/management/server/store/sql_store.go index 6cf74649b..a63219930 100644 --- a/management/server/store/sql_store.go +++ b/management/server/store/sql_store.go @@ -58,6 +58,7 @@ const ( keyQueryCondition = "key = ?" mysqlKeyQueryCondition = "`key` = ?" accountAndIDQueryCondition = "account_id = ? and id = ?" + accountAndAnyIDQueryCondition = "account_id = ? and (id = ? or public_id = ?)" accountAndPeerIDQueryCondition = "account_id = ? and peer_id = ?" accountAndIDsQueryCondition = "account_id = ? AND id IN ?" accountIDCondition = "account_id = ?" @@ -4069,6 +4070,30 @@ func (s *SqlStore) GetPolicyByID(ctx context.Context, lockStrength LockingStreng return policy, nil } +// GetPolicyByIDOrPublicID retrieves a policy by either its ID or its PublicID. Peers report +// whichever of the two the network map they were served carries, so callers resolving a +// peer-reported reference cannot know upfront which namespace it belongs to. +func (s *SqlStore) GetPolicyByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, policyID string) (*types.Policy, error) { + tx := s.db + if lockStrength != LockingStrengthNone { + tx = tx.Clauses(clause.Locking{Strength: string(lockStrength)}) + } + + var policy *types.Policy + + result := tx.Preload(clause.Associations). + Take(&policy, accountAndAnyIDQueryCondition, accountID, policyID, policyID) + if err := result.Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, status.NewPolicyNotFoundError(policyID) + } + log.WithContext(ctx).Errorf("failed to get policy from store: %s", err) + return nil, status.Errorf(status.Internal, "failed to get policy from store") + } + + return policy, nil +} + func (s *SqlStore) CreatePolicy(ctx context.Context, policy *types.Policy) error { result := s.db.Create(policy) if result.Error != nil { @@ -4254,6 +4279,27 @@ func (s *SqlStore) GetRouteByID(ctx context.Context, lockStrength LockingStrengt return route, nil } +// GetRouteByIDOrPublicID retrieves a route by either its ID or its PublicID. See +// GetPolicyByIDOrPublicID for why peer-reported references need both. +func (s *SqlStore) GetRouteByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID string, routeID string) (*route.Route, error) { + tx := s.db + if lockStrength != LockingStrengthNone { + tx = tx.Clauses(clause.Locking{Strength: string(lockStrength)}) + } + + var route *route.Route + result := tx.Take(&route, accountAndAnyIDQueryCondition, accountID, routeID, routeID) + if err := result.Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, status.NewRouteNotFoundError(routeID) + } + log.WithContext(ctx).Errorf("failed to get route from the store: %s", err) + return nil, status.Errorf(status.Internal, "failed to get route from store") + } + + return route, nil +} + // SaveRoute saves a route to the database. func (s *SqlStore) SaveRoute(ctx context.Context, route *route.Route) error { result := s.db.Save(route) @@ -4648,6 +4694,28 @@ func (s *SqlStore) GetNetworkResourceByID(ctx context.Context, lockStrength Lock return netResources, nil } +// GetNetworkResourceByIDOrPublicID retrieves a network resource by either its ID or its +// PublicID. See GetPolicyByIDOrPublicID for why peer-reported references need both. +func (s *SqlStore) GetNetworkResourceByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, resourceID string) (*resourceTypes.NetworkResource, error) { + tx := s.db + if lockStrength != LockingStrengthNone { + tx = tx.Clauses(clause.Locking{Strength: string(lockStrength)}) + } + + var netResources *resourceTypes.NetworkResource + result := tx. + Take(&netResources, accountAndAnyIDQueryCondition, accountID, resourceID, resourceID) + if result.Error != nil { + if errors.Is(result.Error, gorm.ErrRecordNotFound) { + return nil, status.NewNetworkResourceNotFoundError(resourceID) + } + log.WithContext(ctx).Errorf("failed to get network resource from store: %v", result.Error) + return nil, status.Errorf(status.Internal, "failed to get network resource from store") + } + + return netResources, nil +} + func (s *SqlStore) GetNetworkResourceByName(ctx context.Context, lockStrength LockingStrength, accountID, resourceName string) (*resourceTypes.NetworkResource, error) { tx := s.db if lockStrength != LockingStrengthNone { diff --git a/management/server/store/sql_store_test.go b/management/server/store/sql_store_test.go index 7aef2fe5a..8ecf973c2 100644 --- a/management/server/store/sql_store_test.go +++ b/management/server/store/sql_store_test.go @@ -1975,6 +1975,32 @@ func TestSqlStore_GetPolicyByID(t *testing.T) { } } +func TestSqlStore_GetPolicyByIDOrPublicID(t *testing.T) { + store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/store.sql", t.TempDir()) + t.Cleanup(cleanup) + require.NoError(t, err) + + accountID := "bf1c8084-ba50-4ce7-9439-34653001fc3b" + policyID := "cs1tnh0hhcjnqoiuebf0" + + policy, err := store.GetPolicyByID(context.Background(), LockingStrengthNone, accountID, policyID) + require.NoError(t, err) + require.NotEmpty(t, policy.PublicID) + + for _, id := range []string{policyID, policy.PublicID} { + policy, err := store.GetPolicyByIDOrPublicID(context.Background(), LockingStrengthNone, accountID, id) + require.NoError(t, err) + require.Equal(t, policyID, policy.ID) + } + + policy, err = store.GetPolicyByIDOrPublicID(context.Background(), LockingStrengthNone, accountID, "non-existing") + require.Error(t, err) + sErr, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, sErr.Type(), status.NotFound) + require.Nil(t, policy) +} + func TestSqlStore_CreatePolicy(t *testing.T) { store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/store.sql", t.TempDir()) t.Cleanup(cleanup) @@ -2634,6 +2660,32 @@ func TestSqlStore_GetNetworkResourceByID(t *testing.T) { } } +func TestSqlStore_GetNetworkResourceByIDOrPublicID(t *testing.T) { + store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/store.sql", t.TempDir()) + t.Cleanup(cleanup) + require.NoError(t, err) + + accountID := "bf1c8084-ba50-4ce7-9439-34653001fc3b" + netResourceID := "ctc4nci7qv9061u6ilfg" + + netResource, err := store.GetNetworkResourceByID(context.Background(), LockingStrengthNone, accountID, netResourceID) + require.NoError(t, err) + require.NotEmpty(t, netResource.PublicID) + + for _, id := range []string{netResourceID, netResource.PublicID} { + netResource, err := store.GetNetworkResourceByIDOrPublicID(context.Background(), LockingStrengthNone, accountID, id) + require.NoError(t, err) + require.Equal(t, netResourceID, netResource.ID) + } + + netResource, err = store.GetNetworkResourceByIDOrPublicID(context.Background(), LockingStrengthNone, accountID, "non-existing") + require.Error(t, err) + sErr, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, sErr.Type(), status.NotFound) + require.Nil(t, netResource) +} + func TestSqlStore_SaveNetworkResource(t *testing.T) { store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/store.sql", t.TempDir()) t.Cleanup(cleanup) @@ -3759,6 +3811,32 @@ func TestSqlStore_GetRouteByID(t *testing.T) { } } +func TestSqlStore_GetRouteByIDOrPublicID(t *testing.T) { + store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/extended-store.sql", t.TempDir()) + t.Cleanup(cleanup) + require.NoError(t, err) + + accountID := "bf1c8084-ba50-4ce7-9439-34653001fc3b" + routeID := "ct03t427qv97vmtmglog" + + route, err := store.GetRouteByID(context.Background(), LockingStrengthNone, accountID, routeID) + require.NoError(t, err) + require.NotEmpty(t, route.PublicID) + + for _, id := range []string{routeID, route.PublicID} { + route, err := store.GetRouteByIDOrPublicID(context.Background(), LockingStrengthNone, accountID, id) + require.NoError(t, err) + require.Equal(t, routeID, string(route.ID)) + } + + route, err = store.GetRouteByIDOrPublicID(context.Background(), LockingStrengthNone, accountID, "non-existing") + require.Error(t, err) + sErr, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, sErr.Type(), status.NotFound) + require.Nil(t, route) +} + func TestSqlStore_SaveRoute(t *testing.T) { store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/extended-store.sql", t.TempDir()) t.Cleanup(cleanup) diff --git a/management/server/store/store.go b/management/server/store/store.go index 97da95b4c..b6368f47f 100644 --- a/management/server/store/store.go +++ b/management/server/store/store.go @@ -138,6 +138,7 @@ type Store interface { GetAccountPolicies(ctx context.Context, lockStrength LockingStrength, accountID string) ([]*types.Policy, error) GetPolicyByID(ctx context.Context, lockStrength LockingStrength, accountID, policyID string) (*types.Policy, error) + GetPolicyByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, policyID string) (*types.Policy, error) CreatePolicy(ctx context.Context, policy *types.Policy) error SavePolicy(ctx context.Context, policy *types.Policy) error DeletePolicy(ctx context.Context, accountID, policyID string) error @@ -208,6 +209,7 @@ type Store interface { GetAccountRoutes(ctx context.Context, lockStrength LockingStrength, accountID string) ([]*route.Route, error) GetRouteByID(ctx context.Context, lockStrength LockingStrength, accountID, routeID string) (*route.Route, error) + GetRouteByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, routeID string) (*route.Route, error) SaveRoute(ctx context.Context, route *route.Route) error DeleteRoute(ctx context.Context, accountID, routeID string) error @@ -248,6 +250,7 @@ type Store interface { GetNetworkResourcesByNetID(ctx context.Context, lockStrength LockingStrength, accountID, netID string) ([]*resourceTypes.NetworkResource, error) GetNetworkResourcesByAccountID(ctx context.Context, lockStrength LockingStrength, accountID string) ([]*resourceTypes.NetworkResource, error) GetNetworkResourceByID(ctx context.Context, lockStrength LockingStrength, accountID, resourceID string) (*resourceTypes.NetworkResource, error) + GetNetworkResourceByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, resourceID string) (*resourceTypes.NetworkResource, error) GetNetworkResourceByName(ctx context.Context, lockStrength LockingStrength, accountID, resourceName string) (*resourceTypes.NetworkResource, error) SaveNetworkResource(ctx context.Context, resource *resourceTypes.NetworkResource) error DeleteNetworkResource(ctx context.Context, accountID, resourceID string) error diff --git a/management/server/store/store_mock.go b/management/server/store/store_mock.go index 399a07a19..460ba712b 100644 --- a/management/server/store/store_mock.go +++ b/management/server/store/store_mock.go @@ -2166,6 +2166,21 @@ func (mr *MockStoreMockRecorder) GetNetworkResourceByID(ctx, lockStrength, accou return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetworkResourceByID", reflect.TypeOf((*MockStore)(nil).GetNetworkResourceByID), ctx, lockStrength, accountID, resourceID) } +// GetNetworkResourceByIDOrPublicID mocks base method. +func (m *MockStore) GetNetworkResourceByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, resourceID string) (*types0.NetworkResource, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNetworkResourceByIDOrPublicID", ctx, lockStrength, accountID, resourceID) + ret0, _ := ret[0].(*types0.NetworkResource) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetNetworkResourceByIDOrPublicID indicates an expected call of GetNetworkResourceByIDOrPublicID. +func (mr *MockStoreMockRecorder) GetNetworkResourceByIDOrPublicID(ctx, lockStrength, accountID, resourceID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetworkResourceByIDOrPublicID", reflect.TypeOf((*MockStore)(nil).GetNetworkResourceByIDOrPublicID), ctx, lockStrength, accountID, resourceID) +} + // GetNetworkResourceByName mocks base method. func (m *MockStore) GetNetworkResourceByName(ctx context.Context, lockStrength LockingStrength, accountID, resourceName string) (*types0.NetworkResource, error) { m.ctrl.T.Helper() @@ -2496,6 +2511,21 @@ func (mr *MockStoreMockRecorder) GetPolicyByID(ctx, lockStrength, accountID, pol return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPolicyByID", reflect.TypeOf((*MockStore)(nil).GetPolicyByID), ctx, lockStrength, accountID, policyID) } +// GetPolicyByIDOrPublicID mocks base method. +func (m *MockStore) GetPolicyByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, policyID string) (*types3.Policy, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPolicyByIDOrPublicID", ctx, lockStrength, accountID, policyID) + ret0, _ := ret[0].(*types3.Policy) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetPolicyByIDOrPublicID indicates an expected call of GetPolicyByIDOrPublicID. +func (mr *MockStoreMockRecorder) GetPolicyByIDOrPublicID(ctx, lockStrength, accountID, policyID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPolicyByIDOrPublicID", reflect.TypeOf((*MockStore)(nil).GetPolicyByIDOrPublicID), ctx, lockStrength, accountID, policyID) +} + // GetPolicyRulesByResourceID mocks base method. func (m *MockStore) GetPolicyRulesByResourceID(ctx context.Context, lockStrength LockingStrength, accountID, peerID string) ([]*types3.PolicyRule, error) { m.ctrl.T.Helper() @@ -2676,6 +2706,21 @@ func (mr *MockStoreMockRecorder) GetRouteByID(ctx, lockStrength, accountID, rout return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRouteByID", reflect.TypeOf((*MockStore)(nil).GetRouteByID), ctx, lockStrength, accountID, routeID) } +// GetRouteByIDOrPublicID mocks base method. +func (m *MockStore) GetRouteByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, routeID string) (*route.Route, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRouteByIDOrPublicID", ctx, lockStrength, accountID, routeID) + ret0, _ := ret[0].(*route.Route) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetRouteByIDOrPublicID indicates an expected call of GetRouteByIDOrPublicID. +func (mr *MockStoreMockRecorder) GetRouteByIDOrPublicID(ctx, lockStrength, accountID, routeID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRouteByIDOrPublicID", reflect.TypeOf((*MockStore)(nil).GetRouteByIDOrPublicID), ctx, lockStrength, accountID, routeID) +} + // GetRoutingPeerNetworks mocks base method. func (m *MockStore) GetRoutingPeerNetworks(ctx context.Context, accountID, peerID string) ([]string, error) { m.ctrl.T.Helper() diff --git a/management/server/types/networkmap_components_test.go b/management/server/types/networkmap_components_test.go index f6d542609..9e76de775 100644 --- a/management/server/types/networkmap_components_test.go +++ b/management/server/types/networkmap_components_test.go @@ -175,6 +175,63 @@ func TestNetworkMapComponents_NetworkResourceRoutes_RouterPeer(t *testing.T) { assert.NotEmpty(t, nm.RoutesFirewallRules, "router peer should have route firewall rules for the resource") } +// A receiver without a firewall asks Calculate to skip the route firewall +// rules. Everything the rest of the sync consumes — routes, peers, peer +// firewall rules — must come out unchanged. +func TestNetworkMapComponents_SkipRouteFirewallRules(t *testing.T) { + ctx := context.Background() + account := createComponentTestAccount() + + // The shared fixture leaves peer-router-1 out of every peer ACL, so its + // FirewallRules would be empty and the comparison below vacuous. Give the + // router a policy of its own. + account.Policies = append(account.Policies, &types.Policy{ + ID: "policy-router", Name: "Router connectivity", Enabled: true, + Rules: []*types.PolicyRule{{ + ID: "rule-router", Name: "Allow all <-> router", Enabled: true, + Action: types.PolicyTrafficActionAccept, Protocol: types.PolicyRuleProtocolALL, + Bidirectional: true, + Sources: []string{"group-all"}, Destinations: []string{"group-all"}, + }}, + }) + + validated := allPeersValidated(account) + + components := account.GetPeerNetworkMapComponents( + ctx, + "peer-router-1", + account.GetPeersCustomZone(ctx, "netbird.io"), + nil, + validated, + account.GetResourcePoliciesMap(), + account.GetResourceRoutersMap(), + account.GetActiveGroupUsers(), + ) + + full := components.Calculate(ctx) + require.NotEmpty(t, full.RoutesFirewallRules, "baseline: router peer must get route firewall rules") + require.NotEmpty(t, full.FirewallRules, "baseline: router peer must get peer firewall rules") + + components.SkipRouteFirewallRules = true + skipped := components.Calculate(ctx) + + assert.Empty(t, skipped.RoutesFirewallRules, "route firewall rules must not be computed when skipped") + assert.ElementsMatch(t, routeNetworks(full.Routes), routeNetworks(skipped.Routes), + "skipping route firewall rules must not change the routes") + assert.ElementsMatch(t, peerIDs(full.Peers), peerIDs(skipped.Peers), + "skipping route firewall rules must not change the peers to connect") + assert.Equal(t, full.FirewallRules, skipped.FirewallRules, + "peer firewall rules are unrelated and must come out unchanged") +} + +func routeNetworks(routes []*nmdata.Route) []string { + networks := make([]string, 0, len(routes)) + for _, r := range routes { + networks = append(networks, r.Network.String()) + } + return networks +} + func TestNetworkMapComponents_NetworkResourceRoutes_UnrelatedPeer(t *testing.T) { account := createComponentTestAccount() validated := allPeersValidated(account) diff --git a/management/server/types/policy.go b/management/server/types/policy.go index 0f7298d18..9786d17b6 100644 --- a/management/server/types/policy.go +++ b/management/server/types/policy.go @@ -29,7 +29,7 @@ type Policy struct { // ID of the policy' ID string `gorm:"primaryKey"` - PublicID string `json:"-"` + PublicID string `json:"-" gorm:"index"` // AccountID is a reference to Account that this object belongs AccountID string `json:"-" gorm:"index"` diff --git a/route/route.go b/route/route.go index 3bdb0a3a1..ef9a39ef7 100644 --- a/route/route.go +++ b/route/route.go @@ -95,7 +95,7 @@ type Route struct { ID ID `gorm:"primaryKey"` // AccountID is a reference to Account that this object belongs AccountID string `gorm:"index"` - PublicID string `json:"-"` + PublicID string `json:"-" gorm:"index"` // Network and Domains are mutually exclusive Network netip.Prefix `gorm:"serializer:json"` Domains domain.List `gorm:"serializer:json"` diff --git a/shared/management/networkmap/envelope.go b/shared/management/networkmap/envelope.go index 9d2293fb3..2c016af53 100644 --- a/shared/management/networkmap/envelope.go +++ b/shared/management/networkmap/envelope.go @@ -35,7 +35,12 @@ type EnvelopeResult struct { // // dnsName is the account's DNS domain ("netbird.cloud" etc.); used when // rebuilding the per-peer FQDNs that proto.RemotePeerConfig carries. -func EnvelopeToNetworkMap(ctx context.Context, env *proto.NetworkMapEnvelope, localPeerKey, dnsName string) (*EnvelopeResult, error) { +// +// skipRouteFirewallRules leaves RoutesFirewallRules empty. Callers that have +// no firewall to program pass true: the rules are the most expensive part of +// Calculate on a peer that routes many network resources, and nothing reads +// them afterwards. +func EnvelopeToNetworkMap(ctx context.Context, env *proto.NetworkMapEnvelope, localPeerKey, dnsName string, skipRouteFirewallRules bool) (*EnvelopeResult, error) { components, err := DecodeEnvelope(ctx, env) if err != nil { return nil, fmt.Errorf("decode envelope: %w", err) @@ -53,6 +58,7 @@ func EnvelopeToNetworkMap(ctx context.Context, env *proto.NetworkMapEnvelope, lo return nil, fmt.Errorf("receiving peer (wg_key prefix %q) not found among %d decoded peers — components have no PeerID, Calculate would return empty", trimKey(localPeerKey), len(components.Peers)) } components.PeerID = canonicalKey + components.SkipRouteFirewallRules = skipRouteFirewallRules includeIPv6 := localPeer.SupportsIPv6() && localPeer.IPv6.IsValid() useSourcePrefixes := localPeer.SupportsSourcePrefixes() diff --git a/shared/management/networkmap/envelope_test.go b/shared/management/networkmap/envelope_test.go index e205b65ae..03dbe23f7 100644 --- a/shared/management/networkmap/envelope_test.go +++ b/shared/management/networkmap/envelope_test.go @@ -10,6 +10,7 @@ import ( "strconv" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" goproto "google.golang.org/protobuf/proto" @@ -39,7 +40,7 @@ func TestEnvelopeToNetworkMap_RoundTrip(t *testing.T) { var decoded proto.NetworkMapEnvelope require.NoError(t, goproto.Unmarshal(wire, &decoded), "unmarshal envelope") - result, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), &decoded, localPeerKey, "netbird.cloud") + result, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), &decoded, localPeerKey, "netbird.cloud", false) require.NoError(t, err, "EnvelopeToNetworkMap") require.NotNil(t, result) require.NotNil(t, result.NetworkMap, "decoded NetworkMap must be non-nil") @@ -80,7 +81,7 @@ func TestCalculate_FirewallRuleProtocol_NeverNetbirdSSH(t *testing.T) { var decoded proto.NetworkMapEnvelope require.NoError(t, goproto.Unmarshal(wire, &decoded)) - result, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), &decoded, localPeerKey, "netbird.cloud") + result, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), &decoded, localPeerKey, "netbird.cloud", false) require.NoError(t, err) require.NotEmpty(t, result.NetworkMap.FirewallRules, "ssh policy should produce firewall rules") for i, fr := range result.NetworkMap.FirewallRules { @@ -181,13 +182,13 @@ func roundTripComponents(t *testing.T, c *types.NetworkMapComponents, localPeerK } func TestEnvelopeToNetworkMap_NilEnvelope(t *testing.T) { - _, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), nil, "key", "netbird.cloud") + _, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), nil, "key", "netbird.cloud", false) require.Error(t, err, "nil envelope must produce an error rather than panic") } func TestEnvelopeToNetworkMap_FullPayloadMissing(t *testing.T) { env := &proto.NetworkMapEnvelope{} - _, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), env, "key", "netbird.cloud") + _, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), env, "key", "netbird.cloud", false) require.Error(t, err, "envelope with no Full payload must produce an error") } @@ -219,7 +220,7 @@ func TestDecodeEnvelope_MalformedWgKeyPeerSkipped(t *testing.T) { var decoded proto.NetworkMapEnvelope require.NoError(t, goproto.Unmarshal(wire, &decoded), "unmarshal envelope") - result, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), &decoded, localPeerKey, "netbird.cloud") + result, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), &decoded, localPeerKey, "netbird.cloud", false) require.NoError(t, err, "EnvelopeToNetworkMap must tolerate one bad peer key") require.NotNil(t, result) require.NotNil(t, result.Components) @@ -288,7 +289,7 @@ func TestEnvelopeRoundTrip_AllGroupShortCircuitParity(t *testing.T) { var decodedEnv proto.NetworkMapEnvelope require.NoError(t, goproto.Unmarshal(wire, &decodedEnv), "unmarshal envelope") - result, err := nbnetworkmap.EnvelopeToNetworkMap(ctx, &decodedEnv, peers["peer-T"].Key, "netbird.cloud") + result, err := nbnetworkmap.EnvelopeToNetworkMap(ctx, &decodedEnv, peers["peer-T"].Key, "netbird.cloud", false) require.NoError(t, err, "EnvelopeToNetworkMap") clientNM := result.NetworkMap @@ -346,7 +347,7 @@ func TestEnvelopeToNetworkMap_EmptyComponents(t *testing.T) { var decoded proto.NetworkMapEnvelope require.NoError(t, goproto.Unmarshal(wire, &decoded), "unmarshal envelope") - result, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), &decoded, localPeerKey, "netbird.cloud") + result, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), &decoded, localPeerKey, "netbird.cloud", false) require.NoError(t, err, "EnvelopeToNetworkMap must degrade gracefully on empty components") require.Equal(t, uint64(7), result.NetworkMap.Serial) require.Empty(t, result.NetworkMap.RemotePeers, "unvalidated peer connects to nobody") @@ -369,7 +370,7 @@ func TestEnvelopeToNetworkMap_MissingNetwork(t *testing.T) { var decoded proto.NetworkMapEnvelope require.NoError(t, goproto.Unmarshal(wire, &decoded), "unmarshal envelope") - result, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), &decoded, localPeerKey, "netbird.cloud") + result, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), &decoded, localPeerKey, "netbird.cloud", false) require.NoError(t, err, "a missing AccountNetwork must not panic the client") require.NotNil(t, result.Components.Network) require.NotEmpty(t, result.NetworkMap.RemotePeers, "the rest of the snapshot stays usable") @@ -446,3 +447,110 @@ func randomWgKey(t *testing.T) string { require.NoError(t, err) return base64.StdEncoding.EncodeToString(raw[:]) } + +// TestEnvelopeToNetworkMap_SkipRouteFirewallRules covers the flag end to end, +// through the envelope rather than by poking Calculate directly. The +// RoutesFirewallRulesIsEmpty derivation is the part that matters: the client's +// legacy-management probe reads an empty rule list together with that bit, so +// skipping the rules must set it rather than leave it false. +func TestEnvelopeToNetworkMap_SkipRouteFirewallRules(t *testing.T) { + ctx := context.Background() + c, routerKey := buildRoutedResourceComponents(t) + + envelope := mgmtgrpc.EncodeNetworkMapEnvelope(mgmtgrpc.ComponentsEnvelopeInput{ + Components: c, + DNSDomain: "netbird.cloud", + }) + wire, err := goproto.Marshal(envelope) + require.NoError(t, err, "marshal envelope") + var decoded proto.NetworkMapEnvelope + require.NoError(t, goproto.Unmarshal(wire, &decoded), "unmarshal envelope") + + full, err := nbnetworkmap.EnvelopeToNetworkMap(ctx, &decoded, routerKey, "netbird.cloud", false) + require.NoError(t, err, "EnvelopeToNetworkMap without skip") + require.NotEmpty(t, full.NetworkMap.RoutesFirewallRules, + "baseline: the router peer must receive route firewall rules") + require.False(t, full.NetworkMap.RoutesFirewallRulesIsEmpty, + "baseline: the empty bit must be false when rules are present") + + var decodedSkip proto.NetworkMapEnvelope + require.NoError(t, goproto.Unmarshal(wire, &decodedSkip), "unmarshal envelope") + skipped, err := nbnetworkmap.EnvelopeToNetworkMap(ctx, &decodedSkip, routerKey, "netbird.cloud", true) + require.NoError(t, err, "EnvelopeToNetworkMap with skip") + + assert.Empty(t, skipped.NetworkMap.RoutesFirewallRules, + "route firewall rules must not be computed when skipped") + assert.True(t, skipped.NetworkMap.RoutesFirewallRulesIsEmpty, + "the empty bit must be derived from the skipped list, or the client misreads it as legacy management") + assert.Len(t, skipped.NetworkMap.Routes, len(full.NetworkMap.Routes), + "skipping route firewall rules must not change the routes") + assert.Len(t, skipped.NetworkMap.RemotePeers, len(full.NetworkMap.RemotePeers), + "skipping route firewall rules must not change the remote peers") +} + +// buildRoutedResourceComponents returns components in which the local peer is +// the routing peer for one enabled network resource, reachable by a second +// peer through a resource policy — the minimum shape that yields a non-empty +// RoutesFirewallRules. It also returns the local peer's WG key. +func buildRoutedResourceComponents(t *testing.T) (*types.NetworkMapComponents, string) { + t.Helper() + + routerKey := randomWgKey(t) + peers := map[string]*nmdata.Peer{ + "peer-R": { + ID: "peer-R", Key: routerKey, DNSLabel: "router", + IP: netip.AddrFrom4([4]byte{100, 64, 0, 1}), + Meta: nmdata.PeerSystemMeta{WtVersion: "0.40.0"}, + }, + "peer-S": { + ID: "peer-S", Key: randomWgKey(t), DNSLabel: "source", + IP: netip.AddrFrom4([4]byte{100, 64, 0, 2}), + Meta: nmdata.PeerSystemMeta{WtVersion: "0.40.0"}, + }, + } + + resourcePolicy := &nmdata.Policy{ + ID: "pol-res", PublicID: "10", Enabled: true, + Rules: []*nmdata.PolicyRule{{ + ID: "rule-res", + Enabled: true, + Action: string(types.PolicyTrafficActionAccept), + Protocol: string(types.PolicyRuleProtocolALL), + Sources: []string{"g-src"}, + }}, + } + + c := &types.NetworkMapComponents{ + PeerID: "peer-R", + Network: &nmdata.Network{ + Identifier: "net-routed-resource", + Net: net.IPNet{IP: net.IP{100, 64, 0, 0}, Mask: net.CIDRMask(10, 32)}, + Serial: 1, + }, + AccountSettings: &nmdata.AccountSettingsInfo{}, + DNSSettings: &nmdata.DNSSettings{}, + Peers: peers, + Groups: map[string]*nmdata.Group{ + "g-src": {PublicID: "1", Name: "sources", Peers: []string{"peer-S"}}, + "g-routers": {PublicID: "2", Name: "routers", Peers: []string{"peer-R"}}, + }, + NetworkResources: []*nmdata.NetworkResource{{ + ID: "res-1", NetworkID: "netid-1", PublicID: "100", Name: "res1", + Type: "subnet", + Prefix: netip.MustParsePrefix("10.200.0.0/24"), + Enabled: true, + }}, + RoutersMap: map[string]map[string]*nmdata.NetworkRouter{ + "netid-1": {"peer-R": { + PublicID: "200", PeerGroups: []string{"g-routers"}, Metric: 9999, Enabled: true, + }}, + }, + ResourcePoliciesMap: map[string][]*nmdata.Policy{ + "res-1": {resourcePolicy}, + }, + Policies: []*nmdata.Policy{resourcePolicy}, + NetworkXIDToPublicID: map[string]string{"netid-1": "1"}, + } + + return c, routerKey +} diff --git a/shared/management/types/networkmap_components.go b/shared/management/types/networkmap_components.go index 2940ed492..5f8fd10ed 100644 --- a/shared/management/types/networkmap_components.go +++ b/shared/management/types/networkmap_components.go @@ -58,6 +58,13 @@ type NetworkMapComponents struct { // domain targets. ForceRoutingPeerDNSResolution bool + // SkipRouteFirewallRules drops the route firewall rule computation from + // Calculate. A receiver without a firewall manager never reads + // RoutesFirewallRules, and on a routing peer with many network resources + // building them dominates the cost of a sync. Defaults to false so the + // management server keeps producing them. + SkipRouteFirewallRules bool + routesByPeerOnce sync.Once routesByPeerIdx map[string][]routeIndexEntry @@ -150,11 +157,15 @@ func (c *NetworkMapComponents) Calculate(ctx context.Context) *NetworkMap { includeIPv6 = p.SupportsIPv6() && p.IPv6.IsValid() } routesUpdate := filterAndExpandRoutes(c.getRoutesToSync(targetPeerID, peersToConnect, peerGroups), includeIPv6) - routesFirewallRules := c.getPeerRoutesFirewallRules(ctx, targetPeerID, includeIPv6) + + var routesFirewallRules []*RouteFirewallRule + if !c.SkipRouteFirewallRules { + routesFirewallRules = c.getPeerRoutesFirewallRules(ctx, targetPeerID, includeIPv6) + } isRouter, networkResourcesRoutes, sourcePeers := c.getNetworkResourcesRoutesToSync(targetPeerID) var networkResourcesFirewallRules []*RouteFirewallRule - if isRouter { + if isRouter && !c.SkipRouteFirewallRules { networkResourcesFirewallRules = c.getPeerNetworkResourceFirewallRules(ctx, targetPeerID, networkResourcesRoutes, includeIPv6) }