From 211a26019a0a5b96d35d758faaec7b7ad5fcf24e Mon Sep 17 00:00:00 2001 From: Pascal Fischer <32096965+pascal-fischer@users.noreply.github.com> Date: Mon, 22 Jun 2026 19:42:04 +0200 Subject: [PATCH 1/3] [management] validate meta change against posture checks (#6510) --- management/internals/shared/grpc/server.go | 2 +- management/server/account.go | 8 +- management/server/account/manager.go | 4 +- management/server/account/manager_mock.go | 16 +-- management/server/account_test.go | 16 +-- management/server/mock_server/account_mock.go | 12 +- management/server/peer.go | 51 +++---- management/server/peer/peer.go | 124 +++++++++++++++--- management/server/posture/checks.go | 29 ++++ management/server/store/sql_store.go | 22 ---- management/server/store/sql_store_test.go | 50 ------- management/server/store/store.go | 1 - management/server/store/store_mock.go | 14 -- management/server/types/peer.go | 3 + 14 files changed, 192 insertions(+), 160 deletions(-) diff --git a/management/internals/shared/grpc/server.go b/management/internals/shared/grpc/server.go index 7283cae6c..8ee7722be 100644 --- a/management/internals/shared/grpc/server.go +++ b/management/internals/shared/grpc/server.go @@ -1205,7 +1205,7 @@ func (s *Server) SyncMeta(ctx context.Context, req *proto.EncryptedMessage) (*pr return nil, msg } - err = s.accountManager.SyncPeerMeta(ctx, peerKey.String(), extractPeerMeta(ctx, syncMetaReq.GetMeta())) + err = s.accountManager.SyncPeerMeta(ctx, peerKey.String(), extractPeerMeta(ctx, syncMetaReq.GetMeta()), realIP) if err != nil { return nil, mapError(ctx, err) } diff --git a/management/server/account.go b/management/server/account.go index f58c797b7..34220ed3f 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -1889,12 +1889,12 @@ func domainIsUpToDate(domain string, domainCategory string, userAuth auth.UserAu // concurrent stream that started earlier loses the optimistic-lock race // in MarkPeerConnected and bails without writing. func (am *DefaultAccountManager) SyncAndMarkPeer(ctx context.Context, accountID string, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP, syncTime time.Time) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, int64, error) { - peer, netMap, postureChecks, dnsfwdPort, err := am.SyncPeer(ctx, types.PeerSync{WireGuardPubKey: peerPubKey, Meta: meta}, accountID) + peer, netMap, postureChecks, dnsfwdPort, err := am.SyncPeer(ctx, types.PeerSync{WireGuardPubKey: peerPubKey, Meta: meta, RealIP: realIP}, accountID) if err != nil { return nil, nil, nil, 0, fmt.Errorf("error syncing peer: %w", err) } - if err := am.MarkPeerConnected(ctx, peerPubKey, realIP, accountID, syncTime.UnixNano(), netMap); err != nil { + if err := am.MarkPeerConnected(ctx, peerPubKey, accountID, syncTime.UnixNano(), netMap); err != nil { log.WithContext(ctx).Warnf("failed marking peer as connected %s %v", peerPubKey, err) } @@ -1914,13 +1914,13 @@ func (am *DefaultAccountManager) OnPeerDisconnected(ctx context.Context, account return nil } -func (am *DefaultAccountManager) SyncPeerMeta(ctx context.Context, peerPubKey string, meta nbpeer.PeerSystemMeta) error { +func (am *DefaultAccountManager) SyncPeerMeta(ctx context.Context, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP) error { accountID, err := am.Store.GetAccountIDByPeerPubKey(ctx, peerPubKey) if err != nil { return err } - _, _, _, _, err = am.SyncPeer(ctx, types.PeerSync{WireGuardPubKey: peerPubKey, Meta: meta, UpdateAccountPeers: true}, accountID) + _, _, _, _, err = am.SyncPeer(ctx, types.PeerSync{WireGuardPubKey: peerPubKey, Meta: meta, RealIP: realIP, UpdateAccountPeers: true}, accountID) if err != nil { return err } diff --git a/management/server/account/manager.go b/management/server/account/manager.go index 784e432f6..1e738c274 100644 --- a/management/server/account/manager.go +++ b/management/server/account/manager.go @@ -62,7 +62,7 @@ type Manager interface { GetUserFromUserAuth(ctx context.Context, userAuth auth.UserAuth) (*types.User, error) ListUsers(ctx context.Context, accountID string) ([]*types.User, error) GetPeers(ctx context.Context, accountID, userID, nameFilter, ipFilter string) ([]*nbpeer.Peer, error) - MarkPeerConnected(ctx context.Context, peerKey string, realIP net.IP, accountID string, sessionStartedAt int64, nmap *types.NetworkMap) error + MarkPeerConnected(ctx context.Context, peerKey string, accountID string, sessionStartedAt int64, nmap *types.NetworkMap) error MarkPeerDisconnected(ctx context.Context, peerKey string, accountID string, sessionStartedAt int64) error DeletePeer(ctx context.Context, accountID, peerID, userID string) error UpdatePeer(ctx context.Context, accountID, userID string, p *nbpeer.Peer) (*nbpeer.Peer, error) @@ -123,7 +123,7 @@ type Manager interface { GetValidatedPeers(ctx context.Context, accountID string) (map[string]struct{}, map[string]string, error) SyncAndMarkPeer(ctx context.Context, accountID string, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP, syncTime time.Time) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, int64, error) OnPeerDisconnected(ctx context.Context, accountID string, peerPubKey string, streamStartTime time.Time) error - SyncPeerMeta(ctx context.Context, peerPubKey string, meta nbpeer.PeerSystemMeta) error + SyncPeerMeta(ctx context.Context, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP) error FindExistingPostureCheck(accountID string, checks *posture.ChecksDefinition) (*posture.Checks, error) GetAccountIDForPeerKey(ctx context.Context, peerKey string) (string, error) GetAccountSettings(ctx context.Context, accountID string, userID string) (*types.Settings, error) diff --git a/management/server/account/manager_mock.go b/management/server/account/manager_mock.go index 145e6e00f..274e4c683 100644 --- a/management/server/account/manager_mock.go +++ b/management/server/account/manager_mock.go @@ -1323,17 +1323,17 @@ func (mr *MockManagerMockRecorder) ExtendPeerSession(ctx, peerPubKey, userID int } // MarkPeerConnected mocks base method. -func (m *MockManager) MarkPeerConnected(ctx context.Context, peerKey string, realIP net.IP, accountID string, sessionStartedAt int64, nmap *types.NetworkMap) error { +func (m *MockManager) MarkPeerConnected(ctx context.Context, peerKey string, accountID string, sessionStartedAt int64, nmap *types.NetworkMap) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "MarkPeerConnected", ctx, peerKey, realIP, accountID, sessionStartedAt, nmap) + ret := m.ctrl.Call(m, "MarkPeerConnected", ctx, peerKey, accountID, sessionStartedAt, nmap) ret0, _ := ret[0].(error) return ret0 } // MarkPeerConnected indicates an expected call of MarkPeerConnected. -func (mr *MockManagerMockRecorder) MarkPeerConnected(ctx, peerKey, realIP, accountID, sessionStartedAt, nmap interface{}) *gomock.Call { +func (mr *MockManagerMockRecorder) MarkPeerConnected(ctx, peerKey, accountID, sessionStartedAt, nmap interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MarkPeerConnected", reflect.TypeOf((*MockManager)(nil).MarkPeerConnected), ctx, peerKey, realIP, accountID, sessionStartedAt, nmap) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MarkPeerConnected", reflect.TypeOf((*MockManager)(nil).MarkPeerConnected), ctx, peerKey, accountID, sessionStartedAt, nmap) } // MarkPeerDisconnected mocks base method. @@ -1586,17 +1586,17 @@ func (mr *MockManagerMockRecorder) SyncPeer(ctx, sync, accountID interface{}) *g } // SyncPeerMeta mocks base method. -func (m *MockManager) SyncPeerMeta(ctx context.Context, peerPubKey string, meta peer.PeerSystemMeta) error { +func (m *MockManager) SyncPeerMeta(ctx context.Context, peerPubKey string, meta peer.PeerSystemMeta, realIP net.IP) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SyncPeerMeta", ctx, peerPubKey, meta) + ret := m.ctrl.Call(m, "SyncPeerMeta", ctx, peerPubKey, meta, realIP) ret0, _ := ret[0].(error) return ret0 } // SyncPeerMeta indicates an expected call of SyncPeerMeta. -func (mr *MockManagerMockRecorder) SyncPeerMeta(ctx, peerPubKey, meta interface{}) *gomock.Call { +func (mr *MockManagerMockRecorder) SyncPeerMeta(ctx, peerPubKey, meta, realIP interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SyncPeerMeta", reflect.TypeOf((*MockManager)(nil).SyncPeerMeta), ctx, peerPubKey, meta) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SyncPeerMeta", reflect.TypeOf((*MockManager)(nil).SyncPeerMeta), ctx, peerPubKey, meta, realIP) } // SyncUserJWTGroups mocks base method. diff --git a/management/server/account_test.go b/management/server/account_test.go index 2e26ac222..e99e5861f 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -1836,7 +1836,7 @@ func TestDefaultAccountManager_UpdatePeer_PeerLoginExpiration(t *testing.T) { accountID, err := manager.GetAccountIDByUserID(context.Background(), auth.UserAuth{UserId: userID}) require.NoError(t, err, "unable to get the account") - err = manager.MarkPeerConnected(context.Background(), key.PublicKey().String(), nil, accountID, time.Now().UTC().UnixNano(), nil) + err = manager.MarkPeerConnected(context.Background(), key.PublicKey().String(), accountID, time.Now().UTC().UnixNano(), nil) require.NoError(t, err, "unable to mark peer connected") _, err = manager.UpdateAccountSettings(context.Background(), accountID, userID, &types.Settings{ @@ -1907,7 +1907,7 @@ func TestDefaultAccountManager_MarkPeerConnected_PeerLoginExpiration(t *testing. require.NoError(t, err, "unable to get the account") // when we mark peer as connected, the peer login expiration routine should trigger - err = manager.MarkPeerConnected(context.Background(), key.PublicKey().String(), nil, accountID, time.Now().UTC().UnixNano(), nil) + err = manager.MarkPeerConnected(context.Background(), key.PublicKey().String(), accountID, time.Now().UTC().UnixNano(), nil) require.NoError(t, err, "unable to mark peer connected") failed := waitTimeout(wg, time.Second) @@ -1935,7 +1935,7 @@ func TestDefaultAccountManager_OnPeerDisconnected_LastSeenCheck(t *testing.T) { t.Run("disconnect peer when session token matches", func(t *testing.T) { streamStartTime := time.Now().UTC() - err = manager.MarkPeerConnected(context.Background(), peerPubKey, nil, accountID, streamStartTime.UnixNano(), nil) + err = manager.MarkPeerConnected(context.Background(), peerPubKey, accountID, streamStartTime.UnixNano(), nil) require.NoError(t, err, "unable to mark peer connected") peer, err := manager.Store.GetPeerByPeerPubKey(context.Background(), store.LockingStrengthNone, peerPubKey) @@ -1956,7 +1956,7 @@ func TestDefaultAccountManager_OnPeerDisconnected_LastSeenCheck(t *testing.T) { t.Run("skip disconnect when stored session is newer (zombie stream protection)", func(t *testing.T) { // Newer stream wins on connect (sets SessionStartedAt = now ns). streamStartTime := time.Now().UTC() - err = manager.MarkPeerConnected(context.Background(), peerPubKey, nil, accountID, streamStartTime.UnixNano(), nil) + err = manager.MarkPeerConnected(context.Background(), peerPubKey, accountID, streamStartTime.UnixNano(), nil) require.NoError(t, err, "unable to mark peer connected") peer, err := manager.Store.GetPeerByPeerPubKey(context.Background(), store.LockingStrengthNone, peerPubKey) @@ -1980,7 +1980,7 @@ func TestDefaultAccountManager_OnPeerDisconnected_LastSeenCheck(t *testing.T) { t.Run("skip stale connect when stored session is newer (blocked goroutine protection)", func(t *testing.T) { node2SyncTime := time.Now().UTC() - err = manager.MarkPeerConnected(context.Background(), peerPubKey, nil, accountID, node2SyncTime.UnixNano(), nil) + err = manager.MarkPeerConnected(context.Background(), peerPubKey, accountID, node2SyncTime.UnixNano(), nil) require.NoError(t, err, "node 2 should connect peer") peer, err := manager.Store.GetPeerByPeerPubKey(context.Background(), store.LockingStrengthNone, peerPubKey) @@ -1990,7 +1990,7 @@ func TestDefaultAccountManager_OnPeerDisconnected_LastSeenCheck(t *testing.T) { "SessionStartedAt should equal node2SyncTime token") node1StaleSyncTime := node2SyncTime.Add(-1 * time.Minute) - err = manager.MarkPeerConnected(context.Background(), peerPubKey, nil, accountID, node1StaleSyncTime.UnixNano(), nil) + err = manager.MarkPeerConnected(context.Background(), peerPubKey, accountID, node1StaleSyncTime.UnixNano(), nil) require.NoError(t, err, "stale connect should not return error") peer, err = manager.Store.GetPeerByPeerPubKey(context.Background(), store.LockingStrengthNone, peerPubKey) @@ -2052,7 +2052,7 @@ func TestDefaultAccountManager_MarkPeerConnected_ConcurrentRace(t *testing.T) { defer done.Done() ready.Done() start.Wait() - errs <- manager.MarkPeerConnected(context.Background(), peerPubKey, nil, accountID, token, nil) + errs <- manager.MarkPeerConnected(context.Background(), peerPubKey, accountID, token, nil) }() } @@ -2093,7 +2093,7 @@ func TestDefaultAccountManager_UpdateAccountSettings_PeerLoginExpiration(t *test account, err := manager.Store.GetAccount(context.Background(), accountID) require.NoError(t, err, "unable to get the account") - err = manager.MarkPeerConnected(context.Background(), key.PublicKey().String(), nil, accountID, time.Now().UTC().UnixNano(), nil) + err = manager.MarkPeerConnected(context.Background(), key.PublicKey().String(), accountID, time.Now().UTC().UnixNano(), nil) require.NoError(t, err, "unable to mark peer connected") wg := &sync.WaitGroup{} diff --git a/management/server/mock_server/account_mock.go b/management/server/mock_server/account_mock.go index f81139f24..071e3771b 100644 --- a/management/server/mock_server/account_mock.go +++ b/management/server/mock_server/account_mock.go @@ -39,7 +39,7 @@ type MockAccountManager struct { GetUserFromUserAuthFunc func(ctx context.Context, userAuth auth.UserAuth) (*types.User, error) ListUsersFunc func(ctx context.Context, accountID string) ([]*types.User, error) GetPeersFunc func(ctx context.Context, accountID, userID, nameFilter, ipFilter string) ([]*nbpeer.Peer, error) - MarkPeerConnectedFunc func(ctx context.Context, peerKey string, realIP net.IP, accountID string, sessionStartedAt int64, nmap *types.NetworkMap) error + MarkPeerConnectedFunc func(ctx context.Context, peerKey string, accountID string, sessionStartedAt int64, nmap *types.NetworkMap) error MarkPeerDisconnectedFunc func(ctx context.Context, peerKey string, accountID string, sessionStartedAt int64) error SyncAndMarkPeerFunc func(ctx context.Context, accountID string, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP, syncTime time.Time) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, int64, error) DeletePeerFunc func(ctx context.Context, accountID, peerKey, userID string) error @@ -114,7 +114,7 @@ type MockAccountManager struct { GetIdpManagerFunc func() idp.Manager UpdateIntegratedValidatorFunc func(ctx context.Context, accountID, userID, validator string, groups []string) error GroupValidationFunc func(ctx context.Context, accountId string, groups []string) (bool, error) - SyncPeerMetaFunc func(ctx context.Context, peerPubKey string, meta nbpeer.PeerSystemMeta) error + SyncPeerMetaFunc func(ctx context.Context, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP) error FindExistingPostureCheckFunc func(accountID string, checks *posture.ChecksDefinition) (*posture.Checks, error) GetAccountIDForPeerKeyFunc func(ctx context.Context, peerKey string) (string, error) GetAccountByIDFunc func(ctx context.Context, accountID string, userID string) (*types.Account, error) @@ -345,9 +345,9 @@ func (am *MockAccountManager) GetAccountIDByUserID(ctx context.Context, userAuth } // MarkPeerConnected mock implementation of MarkPeerConnected from server.AccountManager interface -func (am *MockAccountManager) MarkPeerConnected(ctx context.Context, peerKey string, realIP net.IP, accountID string, sessionStartedAt int64, nmap *types.NetworkMap) error { +func (am *MockAccountManager) MarkPeerConnected(ctx context.Context, peerKey string, accountID string, sessionStartedAt int64, nmap *types.NetworkMap) error { if am.MarkPeerConnectedFunc != nil { - return am.MarkPeerConnectedFunc(ctx, peerKey, realIP, accountID, sessionStartedAt, nmap) + return am.MarkPeerConnectedFunc(ctx, peerKey, accountID, sessionStartedAt, nmap) } return status.Errorf(codes.Unimplemented, "method MarkPeerConnected is not implemented") } @@ -975,9 +975,9 @@ func (am *MockAccountManager) GroupValidation(ctx context.Context, accountId str } // SyncPeerMeta mocks SyncPeerMeta of the AccountManager interface -func (am *MockAccountManager) SyncPeerMeta(ctx context.Context, peerPubKey string, meta nbpeer.PeerSystemMeta) error { +func (am *MockAccountManager) SyncPeerMeta(ctx context.Context, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP) error { if am.SyncPeerMetaFunc != nil { - return am.SyncPeerMetaFunc(ctx, peerPubKey, meta) + return am.SyncPeerMetaFunc(ctx, peerPubKey, meta, realIP) } return status.Errorf(codes.Unimplemented, "method SyncPeerMeta is not implemented") } diff --git a/management/server/peer.go b/management/server/peer.go index f219d761c..91fafa830 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -74,7 +74,7 @@ func (am *DefaultAccountManager) GetPeers(ctx context.Context, accountID, userID // // Disconnects use MarkPeerDisconnected and require the session to match // exactly; see PeerStatus.SessionStartedAt for the protocol. -func (am *DefaultAccountManager) MarkPeerConnected(ctx context.Context, peerPubKey string, realIP net.IP, accountID string, sessionStartedAt int64, nmap *types.NetworkMap) error { +func (am *DefaultAccountManager) MarkPeerConnected(ctx context.Context, peerPubKey string, accountID string, sessionStartedAt int64, nmap *types.NetworkMap) error { start := time.Now() defer func() { am.metrics.AccountManagerMetrics().RecordPeerStatusUpdateDuration(telemetry.PeerStatusConnect, time.Since(start)) @@ -102,10 +102,6 @@ func (am *DefaultAccountManager) MarkPeerConnected(ctx context.Context, peerPubK } am.metrics.AccountManagerMetrics().CountPeerStatusUpdate(telemetry.PeerStatusConnect, telemetry.PeerStatusApplied) - if am.geo != nil && realIP != nil { - am.updatePeerLocationIfChanged(ctx, accountID, peer, realIP) - } - if err = am.schedulePeerExpirations(ctx, accountID, peer); err != nil { return err } @@ -195,24 +191,28 @@ func (am *DefaultAccountManager) MarkPeerDisconnected(ctx context.Context, peerP return nil } -// updatePeerLocationIfChanged refreshes the geolocation on a separate -// row update, only when the connection IP actually changed. Geo lookups -// are expensive so we skip same-IP reconnects. -func (am *DefaultAccountManager) updatePeerLocationIfChanged(ctx context.Context, accountID string, peer *nbpeer.Peer, realIP net.IP) { +// resolvePeerLocation looks up the geo location for realIP, returning nil when +// there is nothing to apply: geo disabled, no real IP, the IP is unchanged from +// what the peer already has, or the lookup failed. Geo lookups are skipped on +// same-IP reconnects since they are comparatively expensive. The returned value +// is applied by Peer.UpdateMetaIfNew so the change is persisted by its peer save. +func (am *DefaultAccountManager) resolvePeerLocation(ctx context.Context, peer *nbpeer.Peer, realIP net.IP) *nbpeer.Location { + if am.geo == nil || realIP == nil { + return nil + } if peer.Location.ConnectionIP != nil && peer.Location.ConnectionIP.Equal(realIP) { - return + return nil } location, err := am.geo.Lookup(realIP) if err != nil { log.WithContext(ctx).Warnf("failed to get location for peer %s realip: [%s]: %v", peer.ID, realIP.String(), err) - return + return nil } - peer.Location.ConnectionIP = realIP - peer.Location.CountryCode = location.Country.ISOCode - peer.Location.CityName = location.City.Names.En - peer.Location.GeoNameID = location.City.GeonameID - if err := am.Store.SavePeerLocation(ctx, accountID, peer); err != nil { - log.WithContext(ctx).Warnf("could not store location for peer %s: %s", peer.ID, err) + return &nbpeer.Location{ + ConnectionIP: realIP, + CountryCode: location.Country.ISOCode, + CityName: location.City.Names.En, + GeoNameID: location.City.GeonameID, } } @@ -980,7 +980,8 @@ func getPeerIPDNSLabel(ip netip.Addr, peerHostName string) (string, error) { // SyncPeer checks whether peer is eligible for receiving NetworkMap (authenticated) and returns its NetworkMap if eligible func (am *DefaultAccountManager) SyncPeer(ctx context.Context, sync types.PeerSync, accountID string) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, int64, error) { var peer *nbpeer.Peer - var updated, versionChanged, ipv6CapabilityChanged bool + var ipv6CapabilityChanged bool + var metaDiff nbpeer.MetaDiff var err error settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthNone, accountID) @@ -1010,9 +1011,10 @@ func (am *DefaultAccountManager) SyncPeer(ctx context.Context, sync types.PeerSy } oldHasIPv6Cap := peer.HasCapability(nbpeer.PeerCapabilityIPv6Overlay) - updated, versionChanged = peer.UpdateMetaIfNew(ctx, sync.Meta) + newLocation := am.resolvePeerLocation(ctx, peer, sync.RealIP) + metaDiff = peer.UpdateMetaIfNew(ctx, sync.Meta, newLocation) ipv6CapabilityChanged = oldHasIPv6Cap != peer.HasCapability(nbpeer.PeerCapabilityIPv6Overlay) - if updated { + if metaDiff.Updated() { am.metrics.AccountManagerMetrics().CountPeerMetUpdate() log.WithContext(ctx).Tracef("peer %s metadata updated", peer.ID) if err = transaction.SavePeer(ctx, accountID, peer); err != nil { @@ -1040,9 +1042,10 @@ func (am *DefaultAccountManager) SyncPeer(ctx context.Context, sync types.PeerSy return nil, nil, nil, 0, err } - if isStatusChanged || sync.UpdateAccountPeers || ipv6CapabilityChanged || (updated && (len(resPostureChecks) > 0 || versionChanged)) { + metaDiffAffectsPosture := posture.AffectsPosture(&metaDiff, resPostureChecks) + if isStatusChanged || sync.UpdateAccountPeers || ipv6CapabilityChanged || metaDiffAffectsPosture || metaDiff.VersionChanged || metaDiff.Hostname { changedPeerIDs := []string{peer.ID} - affectedPeerIDs := am.syncPeerAffectedPeers(ctx, accountID, peer.ID, nmap, peerNotValid, updated, len(resPostureChecks) > 0) + affectedPeerIDs := am.syncPeerAffectedPeers(ctx, accountID, peer.ID, nmap, peerNotValid, metaDiffAffectsPosture) if err = am.networkMapController.OnPeersUpdated(ctx, accountID, changedPeerIDs, affectedPeerIDs); err != nil { return nil, nil, nil, 0, fmt.Errorf("notify network map controller of peer update: %w", err) } @@ -1059,8 +1062,8 @@ func (am *DefaultAccountManager) SyncPeer(ctx context.Context, sync types.PeerSy // metadata change that flips a posture result removes this peer from others' // maps asymmetrically; that case (and an invalid peer, whose map is empty) falls // back to the resolver. -func (am *DefaultAccountManager) syncPeerAffectedPeers(ctx context.Context, accountID, peerID string, nmap *types.NetworkMap, peerNotValid, metaUpdated, hasPostureChecks bool) []string { - if peerNotValid || (metaUpdated && hasPostureChecks) { +func (am *DefaultAccountManager) syncPeerAffectedPeers(ctx context.Context, accountID, peerID string, nmap *types.NetworkMap, peerNotValid, metaChangeAffectedPosture bool) []string { + if peerNotValid || metaChangeAffectedPosture { return am.resolveAffectedPeersForPeerChanges(ctx, am.Store, accountID, []string{peerID}) } return affectedPeerIDsFromNetworkMap(nmap, peerID) diff --git a/management/server/peer/peer.go b/management/server/peer/peer.go index 591ac074e..4a846ebdd 100644 --- a/management/server/peer/peer.go +++ b/management/server/peer/peer.go @@ -256,14 +256,18 @@ func (p *Peer) Copy() *Peer { } } -// UpdateMetaIfNew updates peer's system metadata if new information is provided -// returns true if meta was updated, false otherwise -func (p *Peer) UpdateMetaIfNew(ctx context.Context, meta PeerSystemMeta) (updated, versionChanged bool) { +// UpdateMetaIfNew updates peer's system metadata and connection geo location if +// new information is provided. newLocation is the geo location resolved from the +// peer's current connection IP, or nil when there is nothing to apply (geo +// disabled, no real IP, or the IP is unchanged); the caller owns the expensive +// lookup and the same-IP guard. It returns a MetaDiff describing what changed; +// diff.Updated() reports whether the peer needs to be persisted. +func (p *Peer) UpdateMetaIfNew(ctx context.Context, meta PeerSystemMeta, newLocation *Location) MetaDiff { if meta.isEmpty() { - return updated, versionChanged + return MetaDiff{} } - versionChanged = p.Meta.WtVersion != meta.WtVersion + versionChanged := p.Meta.WtVersion != meta.WtVersion // Avoid overwriting UIVersion if the update was triggered sole by the CLI client if meta.UIVersion == "" { @@ -272,97 +276,177 @@ func (p *Peer) UpdateMetaIfNew(ctx context.Context, meta PeerSystemMeta) (update oldVersion := p.Meta.WtVersion - diff := metaDiff(p.Meta, meta) - if len(diff) != 0 { + diff := diffMeta(p.Meta, meta) + if diff.Any() { p.Meta = meta - updated = true + } + diff.VersionChanged = versionChanged + + locationInfo := "" + if newLocation != nil { + p.Location = *newLocation + diff.LocationChanged = true + locationInfo = fmt.Sprintf("location changed to %s, ", newLocation.ConnectionIP) } versionInfo := "" - if versionChanged { + if diff.VersionChanged { versionInfo = fmt.Sprintf("version changed: %s -> %s, ", oldVersion, meta.WtVersion) } - if len(diff) > 0 || versionChanged { + if diff.Any() || diff.VersionChanged || diff.LocationChanged { log.WithContext(ctx). - Debugf("peer meta updated, %s%d field(s) changed: %s", versionInfo, len(diff), strings.Join(diff, ", ")) + Debugf("peer meta updated, %s%s%d field(s) changed: %s", versionInfo, locationInfo, len(diff.Changed), strings.Join(diff.Changed, ", ")) } - return updated, versionChanged + return diff +} + +// MetaDiff records which PeerSystemMeta fields differ between two metas. Each bool +// maps to a single struct field, except Environment, which is split into Cloud and +// Platform. Changed holds the human-readable `field: -> ` entries so the +// existing log line and isEqual can be derived from the same comparison. +// +// VersionChanged and LocationChanged sit outside the per-meta-field set: +// VersionChanged tracks the WireGuard client version specifically (compared before +// the UIVersion fixup, to signal client upgrades) and LocationChanged tracks the +// peer's connection geo location, which lives on Peer rather than PeerSystemMeta. +// Neither contributes an entry to Changed, so the field-coverage accounting stays +// driven purely by the PeerSystemMeta comparison. +type MetaDiff struct { + Hostname bool + GoOS bool + Kernel bool + KernelVersion bool + Core bool + Platform bool + OS bool + OSVersion bool + WtVersion bool + UIVersion bool + SystemSerialNumber bool + SystemProductName bool + SystemManufacturer bool + EnvironmentCloud bool + EnvironmentPlatform bool + Flags bool + Capabilities bool + NetworkAddresses bool + Files bool + + VersionChanged bool + LocationChanged bool + + Changed []string +} + +// Any reports whether any PeerSystemMeta field changed. +func (d MetaDiff) Any() bool { + return len(d.Changed) != 0 +} + +// Updated reports whether the peer needs to be persisted: any meta field changed +// or the geo location changed. The version flag alone does not imply a write, +// since a version change is also reflected in the WtVersion meta field. +func (d MetaDiff) Updated() bool { + return d.Any() || d.LocationChanged || d.VersionChanged } -// metaDiff returns a human-readable list of the fields that differ between the -// old and new meta, each formatted as `field: -> `. It is the single -// source of truth for meta comparison: isEqual reports equality as an empty -// diff, so the log line can never disagree with the change decision. Slices are -// cloned before sorting, so callers' meta is not mutated. func metaDiff(oldMeta, newMeta PeerSystemMeta) []string { - var diff []string + return diffMeta(oldMeta, newMeta).Changed +} + +// diffMeta compares two metas field by field, returning both a per-field flag set +// (for callers that need to know exactly what changed, e.g. matching against +// posture checks) and the human-readable Changed list. It is the single source of +// truth for meta comparison: isEqual reports equality as an empty diff, so the log +// line, the change decision, and the flags can never disagree. +func diffMeta(oldMeta, newMeta PeerSystemMeta) MetaDiff { + var d MetaDiff add := func(field string, oldVal, newVal any) { - diff = append(diff, fmt.Sprintf("%s: %v -> %v", field, oldVal, newVal)) + d.Changed = append(d.Changed, fmt.Sprintf("%s: %v -> %v", field, oldVal, newVal)) } if oldMeta.Hostname != newMeta.Hostname { + d.Hostname = true add("hostname", oldMeta.Hostname, newMeta.Hostname) } if oldMeta.GoOS != newMeta.GoOS { + d.GoOS = true add("goos", oldMeta.GoOS, newMeta.GoOS) } if oldMeta.Kernel != newMeta.Kernel { + d.Kernel = true add("kernel", oldMeta.Kernel, newMeta.Kernel) } if oldMeta.KernelVersion != newMeta.KernelVersion { + d.KernelVersion = true add("kernel_version", oldMeta.KernelVersion, newMeta.KernelVersion) } if oldMeta.Core != newMeta.Core { + d.Core = true add("core", oldMeta.Core, newMeta.Core) } if oldMeta.Platform != newMeta.Platform { + d.Platform = true add("platform", oldMeta.Platform, newMeta.Platform) } if oldMeta.OS != newMeta.OS { + d.OS = true add("os", oldMeta.OS, newMeta.OS) } if oldMeta.OSVersion != newMeta.OSVersion { + d.OSVersion = true add("os_version", oldMeta.OSVersion, newMeta.OSVersion) } if oldMeta.WtVersion != newMeta.WtVersion { + d.WtVersion = true add("wt_version", oldMeta.WtVersion, newMeta.WtVersion) } if oldMeta.UIVersion != newMeta.UIVersion { + d.UIVersion = true add("ui_version", oldMeta.UIVersion, newMeta.UIVersion) } if oldMeta.SystemSerialNumber != newMeta.SystemSerialNumber { + d.SystemSerialNumber = true add("system_serial_number", oldMeta.SystemSerialNumber, newMeta.SystemSerialNumber) } if oldMeta.SystemProductName != newMeta.SystemProductName { + d.SystemProductName = true add("system_product_name", oldMeta.SystemProductName, newMeta.SystemProductName) } if oldMeta.SystemManufacturer != newMeta.SystemManufacturer { + d.SystemManufacturer = true add("system_manufacturer", oldMeta.SystemManufacturer, newMeta.SystemManufacturer) } if oldMeta.Environment.Cloud != newMeta.Environment.Cloud { + d.EnvironmentCloud = true add("environment_cloud", oldMeta.Environment.Cloud, newMeta.Environment.Cloud) } if oldMeta.Environment.Platform != newMeta.Environment.Platform { + d.EnvironmentPlatform = true add("environment_platform", oldMeta.Environment.Platform, newMeta.Environment.Platform) } if !oldMeta.Flags.isEqual(newMeta.Flags) { + d.Flags = true add("flags", fmt.Sprintf("%+v", oldMeta.Flags), fmt.Sprintf("%+v", newMeta.Flags)) } if !capabilitiesEqual(oldMeta.Capabilities, newMeta.Capabilities) { + d.Capabilities = true add("capabilities", oldMeta.Capabilities, newMeta.Capabilities) } if !sameMultiset(oldMeta.NetworkAddresses, newMeta.NetworkAddresses) { + d.NetworkAddresses = true add("network_addresses", fmt.Sprintf("%v", oldMeta.NetworkAddresses), fmt.Sprintf("%v", newMeta.NetworkAddresses)) } if !sameMultiset(oldMeta.Files, newMeta.Files) { + d.Files = true add("files", fmt.Sprintf("%v", oldMeta.Files), fmt.Sprintf("%v", newMeta.Files)) } - return diff + return d } // sameMultiset reports whether two slices contain the same elements with the diff --git a/management/server/posture/checks.go b/management/server/posture/checks.go index f0bbbc32e..6a98edb99 100644 --- a/management/server/posture/checks.go +++ b/management/server/posture/checks.go @@ -7,6 +7,7 @@ import ( "regexp" "github.com/hashicorp/go-version" + nbpeer "github.com/netbirdio/netbird/management/server/peer" "github.com/netbirdio/netbird/shared/management/http/api" "github.com/netbirdio/netbird/shared/management/status" @@ -51,6 +52,34 @@ type Checks struct { Checks ChecksDefinition `gorm:"serializer:json"` } +// AffectsPosture reports whether the peer metadata changes described by diff can +// alter the outcome of any of the given posture checks. It maps each check kind to +// the metadata fields it inspects, so an unrelated change (e.g. a hostname update) +// does not force a posture re-evaluation. +func AffectsPosture(diff *nbpeer.MetaDiff, checks []*Checks) bool { + if diff == nil { + return false + } + for _, c := range checks { + if c.Checks.ProcessCheck != nil && diff.Files { + return true + } + if c.Checks.OSVersionCheck != nil && (diff.OSVersion || diff.OS || diff.KernelVersion) { + return true + } + if c.Checks.NBVersionCheck != nil && diff.WtVersion { + return true + } + if c.Checks.GeoLocationCheck != nil && diff.LocationChanged { + return true + } + if c.Checks.PeerNetworkRangeCheck != nil && diff.NetworkAddresses { + return true + } + } + return false +} + // ChecksDefinition contains definition of actual check type ChecksDefinition struct { NBVersionCheck *NBVersionCheck `json:",omitempty"` diff --git a/management/server/store/sql_store.go b/management/server/store/sql_store.go index 7d22905dd..8bc4bcd7d 100644 --- a/management/server/store/sql_store.go +++ b/management/server/store/sql_store.go @@ -581,28 +581,6 @@ func (s *SqlStore) MarkPeerDisconnectedIfSameSession(ctx context.Context, accoun return result.RowsAffected > 0, nil } -func (s *SqlStore) SavePeerLocation(ctx context.Context, accountID string, peerWithLocation *nbpeer.Peer) error { - // To maintain data integrity, we create a copy of the peer's location to prevent unintended updates to other fields. - var peerCopy nbpeer.Peer - // Since the location field has been migrated to JSON serialization, - // updating the struct ensures the correct data format is inserted into the database. - peerCopy.Location = peerWithLocation.Location - - result := s.db.Model(&nbpeer.Peer{}). - Where(accountAndIDQueryCondition, accountID, peerWithLocation.ID). - Updates(peerCopy) - - if result.Error != nil { - return status.Errorf(status.Internal, "failed to save peer locations to store: %v", result.Error) - } - - if result.RowsAffected == 0 { - return status.Errorf(status.NotFound, peerNotFoundFMT, peerWithLocation.ID) - } - - return nil -} - // ApproveAccountPeers marks all peers that currently require approval in the given account as approved. func (s *SqlStore) ApproveAccountPeers(ctx context.Context, accountID string) (int, error) { result := s.db.Model(&nbpeer.Peer{}). diff --git a/management/server/store/sql_store_test.go b/management/server/store/sql_store_test.go index ac136987e..92784af83 100644 --- a/management/server/store/sql_store_test.go +++ b/management/server/store/sql_store_test.go @@ -618,56 +618,6 @@ func TestSqlStore_SavePeerStatus(t *testing.T) { assert.WithinDurationf(t, newStatus.LastSeen, actual.LastSeen.UTC(), time.Millisecond, "LastSeen should be equal") } -func TestSqlStore_SavePeerLocation(t *testing.T) { - store, cleanUp, err := NewTestStoreFromSQL(context.Background(), "../testdata/store.sql", t.TempDir()) - t.Cleanup(cleanUp) - assert.NoError(t, err) - - account, err := store.GetAccount(context.Background(), "bf1c8084-ba50-4ce7-9439-34653001fc3b") - require.NoError(t, err) - - peer := &nbpeer.Peer{ - AccountID: account.Id, - ID: "testpeer", - Location: nbpeer.Location{ - ConnectionIP: net.ParseIP("0.0.0.0"), - CountryCode: "YY", - CityName: "City", - GeoNameID: 1, - }, - CreatedAt: time.Now().UTC(), - Meta: nbpeer.PeerSystemMeta{}, - } - // error is expected as peer is not in store yet - err = store.SavePeerLocation(context.Background(), account.Id, peer) - assert.Error(t, err) - - account.Peers[peer.ID] = peer - err = store.SaveAccount(context.Background(), account) - require.NoError(t, err) - - peer.Location.ConnectionIP = net.ParseIP("35.1.1.1") - peer.Location.CountryCode = "DE" - peer.Location.CityName = "Berlin" - peer.Location.GeoNameID = 2950159 - - err = store.SavePeerLocation(context.Background(), account.Id, account.Peers[peer.ID]) - assert.NoError(t, err) - - account, err = store.GetAccount(context.Background(), account.Id) - require.NoError(t, err) - - actual := account.Peers[peer.ID].Location - assert.Equal(t, peer.Location, actual) - - peer.ID = "non-existing-peer" - err = store.SavePeerLocation(context.Background(), account.Id, peer) - assert.Error(t, err) - parsedErr, ok := status.FromError(err) - require.True(t, ok) - require.Equal(t, status.NotFound, parsedErr.Type(), "should return not found error") -} - func Test_TestGetAccountByPrivateDomain(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("The SQLite store is not properly supported by Windows yet") diff --git a/management/server/store/store.go b/management/server/store/store.go index 31f1fea86..066ab285d 100644 --- a/management/server/store/store.go +++ b/management/server/store/store.go @@ -185,7 +185,6 @@ type Store interface { // recorded by the database. Returns true when the update happened, // false when a newer session has taken over. MarkPeerDisconnectedIfSameSession(ctx context.Context, accountID, peerID string, sessionStartedAt int64) (bool, error) - SavePeerLocation(ctx context.Context, accountID string, peer *nbpeer.Peer) error ApproveAccountPeers(ctx context.Context, accountID string) (int, error) DeletePeer(ctx context.Context, accountID string, peerID string) error diff --git a/management/server/store/store_mock.go b/management/server/store/store_mock.go index 706c03f1b..fdd2d0900 100644 --- a/management/server/store/store_mock.go +++ b/management/server/store/store_mock.go @@ -2968,20 +2968,6 @@ func (mr *MockStoreMockRecorder) SavePeer(ctx, accountID, peer interface{}) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SavePeer", reflect.TypeOf((*MockStore)(nil).SavePeer), ctx, accountID, peer) } -// SavePeerLocation mocks base method. -func (m *MockStore) SavePeerLocation(ctx context.Context, accountID string, peer *peer.Peer) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SavePeerLocation", ctx, accountID, peer) - ret0, _ := ret[0].(error) - return ret0 -} - -// SavePeerLocation indicates an expected call of SavePeerLocation. -func (mr *MockStoreMockRecorder) SavePeerLocation(ctx, accountID, peer interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SavePeerLocation", reflect.TypeOf((*MockStore)(nil).SavePeerLocation), ctx, accountID, peer) -} - // SavePeerStatus mocks base method. func (m *MockStore) SavePeerStatus(ctx context.Context, accountID, peerID string, status peer.PeerStatus) error { m.ctrl.T.Helper() diff --git a/management/server/types/peer.go b/management/server/types/peer.go index 15d343793..885d67bba 100644 --- a/management/server/types/peer.go +++ b/management/server/types/peer.go @@ -12,6 +12,9 @@ type PeerSync struct { WireGuardPubKey string // Meta is the system information passed by peer, must be always present Meta nbpeer.PeerSystemMeta + // RealIP is the peer's connection IP, used to refresh its geo location. + // May be nil when the request has no associated connection IP. + RealIP net.IP // UpdateAccountPeers indicate updating account peers, // which occurs when the peer's metadata is updated UpdateAccountPeers bool From 2ebf26006a6c6be1c9b824012a57224d6eb53c96 Mon Sep 17 00:00:00 2001 From: Pascal Fischer <32096965+pascal-fischer@users.noreply.github.com> Date: Mon, 22 Jun 2026 19:54:38 +0200 Subject: [PATCH 2/3] [management] empty file check in nmap on other posturechecks (#6511) --- management/internals/shared/grpc/server.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/management/internals/shared/grpc/server.go b/management/internals/shared/grpc/server.go index 8ee7722be..1d734dae7 100644 --- a/management/internals/shared/grpc/server.go +++ b/management/internals/shared/grpc/server.go @@ -1254,7 +1254,10 @@ func (s *Server) Logout(ctx context.Context, req *proto.EncryptedMessage) (*prot func toProtocolChecks(ctx context.Context, postureChecks []*posture.Checks) []*proto.Checks { protoChecks := make([]*proto.Checks, 0, len(postureChecks)) for _, postureCheck := range postureChecks { - protoChecks = append(protoChecks, toProtocolCheck(postureCheck)) + check := toProtocolCheck(postureCheck) + if check != nil { + protoChecks = append(protoChecks, check) + } } return protoChecks @@ -1278,5 +1281,9 @@ func toProtocolCheck(postureCheck *posture.Checks) *proto.Checks { } } + if len(protoCheck.Files) == 0 { + return nil + } + return protoCheck } From 51b6f6291b9f82101980e773e22649deddb73b7b Mon Sep 17 00:00:00 2001 From: Riccardo Manfrin <3090891+riccardomanfrin@users.noreply.github.com> Date: Mon, 22 Jun 2026 22:01:49 +0200 Subject: [PATCH 3/3] Fixup debug config (#6514) --- client/cmd/debug.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/cmd/debug.go b/client/cmd/debug.go index bc7b0e98c..57e75f663 100644 --- a/client/cmd/debug.go +++ b/client/cmd/debug.go @@ -130,7 +130,7 @@ func debugConfigDump(cmd *cobra.Command, _ []string) error { client := proto.NewDaemonServiceClient(conn) resp, err := client.GetConfig(cmd.Context(), &proto.GetConfigRequest{ - ProfileName: activeProf.Name, + ProfileName: string(activeProf.ID), Username: currUser.Username, }) if err != nil {