Merge remote-tracking branch 'origin/main' into embedded-vnc

This commit is contained in:
Viktor Liu
2026-09-23 15:05:30 +02:00
15 changed files with 609 additions and 30 deletions
+84 -14
View File
@@ -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 := &registryConfigurator{
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.
+129
View File
@@ -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 := &registryConfigurator{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 := &registryConfigurator{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")
}
+5 -1
View File
@@ -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)
}
@@ -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:
@@ -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
+68
View File
@@ -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 {
+78
View File
@@ -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)
+3
View File
@@ -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
+45
View File
@@ -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()
@@ -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)
+1 -1
View File
@@ -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"`
+1 -1
View File
@@ -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"`
+7 -1
View File
@@ -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()
+116 -8
View File
@@ -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
}
@@ -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)
}