From 22678bce7fd62e625c1dee1695bca5ed9ed6b326 Mon Sep 17 00:00:00 2001 From: Pascal Fischer <32096965+pascal-fischer@users.noreply.github.com> Date: Wed, 2 Jul 2025 18:13:10 +0200 Subject: [PATCH 01/10] [management] add uniqueness constraint for peer ip and label and optimize generation (#4042) --- management/server/account.go | 31 +- management/server/account_test.go | 20 +- management/server/migration/migration.go | 39 ++ management/server/peer.go | 394 +++++++++--------- management/server/peer/peer.go | 4 +- management/server/peer_test.go | 144 ++++++- management/server/store/file_store.go | 2 +- management/server/store/sql_store.go | 36 +- management/server/store/sql_store_test.go | 132 ++++-- management/server/store/store.go | 39 +- management/server/testdata/store.sql | 2 +- .../testdata/store_with_expired_peers.sql | 2 +- management/server/types/network.go | 64 ++- 13 files changed, 616 insertions(+), 293 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 3b7359502..8a80aefb6 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -106,6 +106,18 @@ type DefaultAccountManager struct { disableDefaultPolicy bool } +func isUniqueConstraintError(err error) bool { + switch { + case strings.Contains(err.Error(), "(SQLSTATE 23505)"), + strings.Contains(err.Error(), "Error 1062 (23000)"), + strings.Contains(err.Error(), "UNIQUE constraint failed"): + return true + + default: + return false + } +} + // getJWTGroupsChanges calculates the changes needed to sync a user's JWT groups. // Returns a bool indicating if there are changes in the JWT group membership, the updated user AutoGroups, // newly groups to create and an error if any occurred. @@ -1661,25 +1673,6 @@ func (am *DefaultAccountManager) handleUserPeer(ctx context.Context, transaction return false, nil } -func (am *DefaultAccountManager) getFreeDNSLabel(ctx context.Context, s store.Store, accountID string, peerHostName string) (string, error) { - existingLabels, err := s.GetPeerLabelsInAccount(ctx, store.LockingStrengthShare, accountID) - if err != nil { - return "", fmt.Errorf("failed to get peer dns labels: %w", err) - } - - labelMap := ConvertSliceToMap(existingLabels) - newLabel, err := types.GetPeerHostLabel(peerHostName, labelMap) - if err != nil { - return "", fmt.Errorf("failed to get new host label: %w", err) - } - - if newLabel == "" { - return "", fmt.Errorf("failed to get new host label: %w", err) - } - - return newLabel, nil -} - func (am *DefaultAccountManager) GetAccountSettings(ctx context.Context, accountID string, userID string) (*types.Settings, error) { allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Settings, operations.Read) if err != nil { diff --git a/management/server/account_test.go b/management/server/account_test.go index 7f319b81e..60353389f 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -2623,11 +2623,11 @@ func TestAccount_SetJWTGroups(t *testing.T) { account := &types.Account{ Id: "accountID", Peers: map[string]*nbpeer.Peer{ - "peer1": {ID: "peer1", Key: "key1", UserID: "user1"}, - "peer2": {ID: "peer2", Key: "key2", UserID: "user1"}, - "peer3": {ID: "peer3", Key: "key3", UserID: "user1"}, - "peer4": {ID: "peer4", Key: "key4", UserID: "user2"}, - "peer5": {ID: "peer5", Key: "key5", UserID: "user2"}, + "peer1": {ID: "peer1", Key: "key1", UserID: "user1", IP: net.IP{1, 1, 1, 1}, DNSLabel: "peer1.domain.test"}, + "peer2": {ID: "peer2", Key: "key2", UserID: "user1", IP: net.IP{2, 2, 2, 2}, DNSLabel: "peer2.domain.test"}, + "peer3": {ID: "peer3", Key: "key3", UserID: "user1", IP: net.IP{3, 3, 3, 3}, DNSLabel: "peer3.domain.test"}, + "peer4": {ID: "peer4", Key: "key4", UserID: "user2", IP: net.IP{4, 4, 4, 4}, DNSLabel: "peer4.domain.test"}, + "peer5": {ID: "peer5", Key: "key5", UserID: "user2", IP: net.IP{5, 5, 5, 5}, DNSLabel: "peer5.domain.test"}, }, Groups: map[string]*types.Group{ "group1": {ID: "group1", Name: "group1", Issued: types.GroupIssuedAPI, Peers: []string{}}, @@ -3147,11 +3147,11 @@ func BenchmarkLoginPeer_NewPeer(b *testing.B) { minMsPerOpCICD float64 maxMsPerOpCICD float64 }{ - {"Small", 50, 5, 7, 20, 10, 80}, + {"Small", 50, 5, 7, 20, 5, 80}, {"Medium", 500, 100, 5, 40, 30, 140}, {"Large", 5000, 200, 80, 120, 140, 390}, - {"Small single", 50, 10, 7, 20, 10, 80}, - {"Medium single", 500, 10, 5, 40, 20, 85}, + {"Small single", 50, 10, 7, 20, 6, 80}, + {"Medium single", 500, 10, 5, 40, 15, 85}, {"Large 5", 5000, 15, 80, 120, 80, 200}, } @@ -3343,11 +3343,11 @@ func TestPropagateUserGroupMemberships(t *testing.T) { account, err := manager.GetOrCreateAccountByUser(ctx, initiatorId, domain) require.NoError(t, err) - peer1 := &nbpeer.Peer{ID: "peer1", AccountID: account.Id, UserID: initiatorId} + peer1 := &nbpeer.Peer{ID: "peer1", AccountID: account.Id, UserID: initiatorId, IP: net.IP{1, 1, 1, 1}, DNSLabel: "peer1.domain.test"} err = manager.Store.AddPeerToAccount(ctx, store.LockingStrengthUpdate, peer1) require.NoError(t, err) - peer2 := &nbpeer.Peer{ID: "peer2", AccountID: account.Id, UserID: initiatorId} + peer2 := &nbpeer.Peer{ID: "peer2", AccountID: account.Id, UserID: initiatorId, IP: net.IP{2, 2, 2, 2}, DNSLabel: "peer2.domain.test"} err = manager.Store.AddPeerToAccount(ctx, store.LockingStrengthUpdate, peer2) require.NoError(t, err) diff --git a/management/server/migration/migration.go b/management/server/migration/migration.go index c8a852e0a..ab11be731 100644 --- a/management/server/migration/migration.go +++ b/management/server/migration/migration.go @@ -373,3 +373,42 @@ func DropIndex[T any](ctx context.Context, db *gorm.DB, indexName string) error log.WithContext(ctx).Infof("dropped index %s from table %T", indexName, model) return nil } + +func CreateIndexIfNotExists[T any](ctx context.Context, db *gorm.DB, indexName string, columns ...string) error { + var model T + + stmt := &gorm.Statement{DB: db} + if err := stmt.Parse(&model); err != nil { + return fmt.Errorf("failed to parse model schema: %w", err) + } + tableName := stmt.Schema.Table + dialect := db.Dialector.Name() + + var columnClause string + if dialect == "mysql" { + var withLength []string + for _, col := range columns { + if col == "ip" || col == "dns_label" { + withLength = append(withLength, fmt.Sprintf("%s(64)", col)) + } else { + withLength = append(withLength, col) + } + } + columnClause = strings.Join(withLength, ", ") + } else { + columnClause = strings.Join(columns, ", ") + } + + createStmt := fmt.Sprintf("CREATE UNIQUE INDEX %s ON %s (%s)", indexName, tableName, columnClause) + if dialect == "postgres" || dialect == "sqlite" { + createStmt = strings.Replace(createStmt, "CREATE UNIQUE INDEX", "CREATE UNIQUE INDEX IF NOT EXISTS", 1) + } + + log.WithContext(ctx).Infof("executing index creation: %s", createStmt) + if err := db.Exec(createStmt).Error; err != nil { + return fmt.Errorf("failed to create index %s: %w", indexName, err) + } + + log.WithContext(ctx).Infof("successfully created index %s on table %s", indexName, tableName) + return nil +} diff --git a/management/server/peer.go b/management/server/peer.go index 254048a96..2c1d8f64c 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -15,13 +15,14 @@ import ( log "github.com/sirupsen/logrus" "golang.org/x/exp/maps" + nbdns "github.com/netbirdio/netbird/dns" "github.com/netbirdio/netbird/management/domain" "github.com/netbirdio/netbird/management/server/geolocation" + "github.com/netbirdio/netbird/management/server/idp" routerTypes "github.com/netbirdio/netbird/management/server/networks/routers/types" "github.com/netbirdio/netbird/management/server/permissions/modules" "github.com/netbirdio/netbird/management/server/permissions/operations" - "github.com/netbirdio/netbird/management/server/idp" "github.com/netbirdio/netbird/management/server/posture" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" @@ -234,14 +235,10 @@ func (am *DefaultAccountManager) UpdatePeer(ctx context.Context, accountID, user } if peer.Name != update.Name { - existingLabels, err := getPeerDNSLabels(ctx, transaction, accountID) + var newLabel string + newLabel, err = getPeerIPDNSLabel(ctx, transaction, peer.IP, accountID, update.Name) if err != nil { - return err - } - - newLabel, err := types.GetPeerHostLabel(update.Name, existingLabels) - if err != nil { - return err + return fmt.Errorf("failed to get free DNS label: %w", err) } peer.Name = update.Name @@ -463,208 +460,232 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, setupKey, userID s upperKey := strings.ToUpper(setupKey) hashedKey := sha256.Sum256([]byte(upperKey)) encodedHashedKey := b64.StdEncoding.EncodeToString(hashedKey[:]) - var accountID string - var err error - addedByUser := false - if len(userID) > 0 { - addedByUser = true - accountID, err = am.Store.GetAccountIDByUserID(ctx, store.LockingStrengthShare, userID) - } else { - accountID, err = am.Store.GetAccountIDBySetupKey(ctx, encodedHashedKey) - } - if err != nil { - return nil, nil, nil, status.Errorf(status.NotFound, "failed adding new peer: account not found") - } - - unlock := am.Store.AcquireWriteLockByUID(ctx, accountID) - defer func() { - if unlock != nil { - unlock() - } - }() + addedByUser := len(userID) > 0 // This is a handling for the case when the same machine (with the same WireGuard pub key) tries to register twice. // Such case is possible when AddPeer function takes long time to finish after AcquireWriteLockByUID (e.g., database is slow) // and the peer disconnects with a timeout and tries to register again. // We just check if this machine has been registered before and reject the second registration. // The connecting peer should be able to recover with a retry. - _, err = am.Store.GetPeerByPeerPubKey(ctx, store.LockingStrengthShare, peer.Key) + _, err := am.Store.GetPeerByPeerPubKey(ctx, store.LockingStrengthNone, peer.Key) if err == nil { return nil, nil, nil, status.Errorf(status.PreconditionFailed, "peer has been already registered") } opEvent := &activity.Event{ Timestamp: time.Now().UTC(), - AccountID: accountID, } var newPeer *nbpeer.Peer var updateAccountPeers bool - err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { - var setupKeyID string - var setupKeyName string - var ephemeral bool - var groupsToAdd []string - var allowExtraDNSLabels bool - if addedByUser { - user, err := transaction.GetUserByUserID(ctx, store.LockingStrengthUpdate, userID) - if err != nil { - return fmt.Errorf("failed to get user groups: %w", err) - } - groupsToAdd = user.AutoGroups - opEvent.InitiatorID = userID - opEvent.Activity = activity.PeerAddedByUser - } else { - // Validate the setup key - sk, err := transaction.GetSetupKeyBySecret(ctx, store.LockingStrengthUpdate, encodedHashedKey) - if err != nil { - return fmt.Errorf("failed to get setup key: %w", err) - } - - if !sk.IsValid() { - return status.Errorf(status.PreconditionFailed, "couldn't add peer: setup key is invalid") - } - - opEvent.InitiatorID = sk.Id - opEvent.Activity = activity.PeerAddedWithSetupKey - groupsToAdd = sk.AutoGroups - ephemeral = sk.Ephemeral - setupKeyID = sk.Id - setupKeyName = sk.Name - allowExtraDNSLabels = sk.AllowExtraDNSLabels - - if !sk.AllowExtraDNSLabels && len(peer.ExtraDNSLabels) > 0 { - return status.Errorf(status.PreconditionFailed, "couldn't add peer: setup key doesn't allow extra DNS labels") - } - } - - if (strings.ToLower(peer.Meta.Hostname) == "iphone" || strings.ToLower(peer.Meta.Hostname) == "ipad") && userID != "" { - if am.idpManager != nil { - userdata, err := am.idpManager.GetUserDataByID(ctx, userID, idp.AppMetadata{WTAccountID: accountID}) - if err == nil && userdata != nil { - peer.Meta.Hostname = fmt.Sprintf("%s-%s", peer.Meta.Hostname, strings.Split(userdata.Email, "@")[0]) - } - } - } - - freeLabel, err := am.getFreeDNSLabel(ctx, transaction, accountID, peer.Meta.Hostname) + var setupKeyID string + var setupKeyName string + var ephemeral bool + var groupsToAdd []string + var allowExtraDNSLabels bool + var accountID string + if addedByUser { + user, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, userID) if err != nil { - return fmt.Errorf("failed to get free DNS label: %w", err) + return nil, nil, nil, fmt.Errorf("failed to get user groups: %w", err) } - - freeIP, err := getFreeIP(ctx, transaction, accountID) + groupsToAdd = user.AutoGroups + opEvent.InitiatorID = userID + opEvent.Activity = activity.PeerAddedByUser + accountID = user.AccountID + } else { + // Validate the setup key + sk, err := am.Store.GetSetupKeyBySecret(ctx, store.LockingStrengthNone, encodedHashedKey) if err != nil { - return fmt.Errorf("failed to get free IP: %w", err) + return nil, nil, nil, fmt.Errorf("failed to get setup key: %w", err) } - if err := domain.ValidateDomainsList(peer.ExtraDNSLabels); err != nil { - return status.Errorf(status.InvalidArgument, "invalid extra DNS labels: %v", err) + // we will check key twice for early return + if !sk.IsValid() { + return nil, nil, nil, status.Errorf(status.PreconditionFailed, "couldn't add peer: setup key is invalid") } - registrationTime := time.Now().UTC() - newPeer = &nbpeer.Peer{ - ID: xid.New().String(), - AccountID: accountID, - Key: peer.Key, - IP: freeIP, - Meta: peer.Meta, - Name: peer.Meta.Hostname, - DNSLabel: freeLabel, - UserID: userID, - Status: &nbpeer.PeerStatus{Connected: false, LastSeen: registrationTime}, - SSHEnabled: false, - SSHKey: peer.SSHKey, - LastLogin: ®istrationTime, - CreatedAt: registrationTime, - LoginExpirationEnabled: addedByUser, - Ephemeral: ephemeral, - Location: peer.Location, - InactivityExpirationEnabled: addedByUser, - ExtraDNSLabels: peer.ExtraDNSLabels, - AllowExtraDNSLabels: allowExtraDNSLabels, - } - settings, err := transaction.GetAccountSettings(ctx, store.LockingStrengthShare, accountID) - if err != nil { - return fmt.Errorf("failed to get account settings: %w", err) - } + opEvent.InitiatorID = sk.Id + opEvent.Activity = activity.PeerAddedWithSetupKey + groupsToAdd = sk.AutoGroups + ephemeral = sk.Ephemeral + setupKeyID = sk.Id + setupKeyName = sk.Name + allowExtraDNSLabels = sk.AllowExtraDNSLabels + accountID = sk.AccountID - opEvent.TargetID = newPeer.ID - opEvent.Meta = newPeer.EventMeta(am.GetDNSDomain(settings)) - if !addedByUser { - opEvent.Meta["setup_key_name"] = setupKeyName + if !sk.AllowExtraDNSLabels && len(peer.ExtraDNSLabels) > 0 { + return nil, nil, nil, status.Errorf(status.PreconditionFailed, "couldn't add peer: setup key doesn't allow extra DNS labels") } + } + opEvent.AccountID = accountID - if am.geo != nil && newPeer.Location.ConnectionIP != nil { - location, err := am.geo.Lookup(newPeer.Location.ConnectionIP) - if err != nil { - log.WithContext(ctx).Warnf("failed to get location for new peer realip: [%s]: %v", newPeer.Location.ConnectionIP.String(), err) - } else { - newPeer.Location.CountryCode = location.Country.ISOCode - newPeer.Location.CityName = location.City.Names.En - newPeer.Location.GeoNameID = location.City.GeonameID + if (strings.ToLower(peer.Meta.Hostname) == "iphone" || strings.ToLower(peer.Meta.Hostname) == "ipad") && userID != "" { + if am.idpManager != nil { + userdata, err := am.idpManager.GetUserDataByID(ctx, userID, idp.AppMetadata{WTAccountID: accountID}) + if err == nil && userdata != nil { + peer.Meta.Hostname = fmt.Sprintf("%s-%s", peer.Meta.Hostname, strings.Split(userdata.Email, "@")[0]) } } + } - newPeer = am.integratedPeerValidator.PreparePeer(ctx, accountID, newPeer, groupsToAdd, settings.Extra) - - err = transaction.AddPeerToAllGroup(ctx, store.LockingStrengthUpdate, accountID, newPeer.ID) - if err != nil { - return fmt.Errorf("failed adding peer to All group: %w", err) - } - - if len(groupsToAdd) > 0 { - for _, g := range groupsToAdd { - err = transaction.AddPeerToGroup(ctx, store.LockingStrengthUpdate, accountID, newPeer.ID, g) - if err != nil { - return err - } - } - } - - err = transaction.AddPeerToAccount(ctx, store.LockingStrengthUpdate, newPeer) - if err != nil { - return fmt.Errorf("failed to add peer to account: %w", err) - } - - err = transaction.IncrementNetworkSerial(ctx, store.LockingStrengthUpdate, accountID) - if err != nil { - return fmt.Errorf("failed to increment network serial: %w", err) - } - - if addedByUser { - err := transaction.SaveUserLastLogin(ctx, accountID, userID, newPeer.GetLastLogin()) - if err != nil { - log.WithContext(ctx).Debugf("failed to update user last login: %v", err) - } - } else { - err = transaction.IncrementSetupKeyUsage(ctx, setupKeyID) - if err != nil { - return fmt.Errorf("failed to increment setup key usage: %w", err) - } - } - - updateAccountPeers, err = isPeerInActiveGroup(ctx, transaction, accountID, newPeer.ID) - if err != nil { - return err - } - - log.WithContext(ctx).Debugf("Peer %s added to account %s", newPeer.ID, accountID) - return nil - }) + if err := domain.ValidateDomainsList(peer.ExtraDNSLabels); err != nil { + return nil, nil, nil, status.Errorf(status.InvalidArgument, "invalid extra DNS labels: %v", err) + } + registrationTime := time.Now().UTC() + newPeer = &nbpeer.Peer{ + ID: xid.New().String(), + AccountID: accountID, + Key: peer.Key, + Meta: peer.Meta, + Name: peer.Meta.Hostname, + UserID: userID, + Status: &nbpeer.PeerStatus{Connected: false, LastSeen: registrationTime}, + SSHEnabled: false, + SSHKey: peer.SSHKey, + LastLogin: ®istrationTime, + CreatedAt: registrationTime, + LoginExpirationEnabled: addedByUser, + Ephemeral: ephemeral, + Location: peer.Location, + InactivityExpirationEnabled: addedByUser, + ExtraDNSLabels: peer.ExtraDNSLabels, + AllowExtraDNSLabels: allowExtraDNSLabels, + } + settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthNone, accountID) if err != nil { + return nil, nil, nil, fmt.Errorf("failed to get account settings: %w", err) + } + + if am.geo != nil && newPeer.Location.ConnectionIP != nil { + location, err := am.geo.Lookup(newPeer.Location.ConnectionIP) + if err != nil { + log.WithContext(ctx).Warnf("failed to get location for new peer realip: [%s]: %v", newPeer.Location.ConnectionIP.String(), err) + } else { + newPeer.Location.CountryCode = location.Country.ISOCode + newPeer.Location.CityName = location.City.Names.En + newPeer.Location.GeoNameID = location.City.GeonameID + } + } + + newPeer = am.integratedPeerValidator.PreparePeer(ctx, accountID, newPeer, groupsToAdd, settings.Extra) + + network, err := am.Store.GetAccountNetwork(ctx, store.LockingStrengthNone, accountID) + if err != nil { + return nil, nil, nil, fmt.Errorf("failed getting network: %w", err) + } + + maxAttempts := 10 + for attempt := 1; attempt <= maxAttempts; attempt++ { + var freeIP net.IP + freeIP, err = types.AllocateRandomPeerIP(network.Net) + if err != nil { + return nil, nil, nil, fmt.Errorf("failed to get free IP: %w", err) + } + + var freeLabel string + freeLabel, err = getPeerIPDNSLabel(ctx, am.Store, freeIP, accountID, peer.Meta.Hostname) + if err != nil { + return nil, nil, nil, fmt.Errorf("failed to get free DNS label: %w", err) + } + + newPeer.DNSLabel = freeLabel + newPeer.IP = freeIP + + unlock := am.Store.AcquireWriteLockByUID(ctx, accountID) + defer func() { + if unlock != nil { + unlock() + } + }() + + err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + err = transaction.AddPeerToAccount(ctx, store.LockingStrengthUpdate, newPeer) + if err != nil { + return err + } + + err = transaction.AddPeerToAllGroup(ctx, store.LockingStrengthUpdate, accountID, newPeer.ID) + if err != nil { + return fmt.Errorf("failed adding peer to All group: %w", err) + } + + if len(groupsToAdd) > 0 { + for _, g := range groupsToAdd { + err = transaction.AddPeerToGroup(ctx, store.LockingStrengthUpdate, accountID, newPeer.ID, g) + if err != nil { + return err + } + } + } + + if addedByUser { + err := transaction.SaveUserLastLogin(ctx, accountID, userID, newPeer.GetLastLogin()) + if err != nil { + log.WithContext(ctx).Debugf("failed to update user last login: %v", err) + } + } else { + sk, err := transaction.GetSetupKeyBySecret(ctx, store.LockingStrengthUpdate, encodedHashedKey) + if err != nil { + return fmt.Errorf("failed to get setup key: %w", err) + } + + // we validate at the end to not block the setup key for too long + if !sk.IsValid() { + return status.Errorf(status.PreconditionFailed, "couldn't add peer: setup key is invalid") + } + + err = transaction.IncrementSetupKeyUsage(ctx, setupKeyID) + if err != nil { + return fmt.Errorf("failed to increment setup key usage: %w", err) + } + } + + err = transaction.IncrementNetworkSerial(ctx, store.LockingStrengthUpdate, accountID) + if err != nil { + return fmt.Errorf("failed to increment network serial: %w", err) + } + + log.WithContext(ctx).Debugf("Peer %s added to account %s", newPeer.ID, accountID) + return nil + }) + if err == nil { + unlock() + unlock = nil + break + } + + if isUniqueConstraintError(err) { + unlock() + unlock = nil + log.WithContext(ctx).Debugf("Failed to add peer in attempt %d, retrying: %v", attempt, err) + continue + } + return nil, nil, nil, fmt.Errorf("failed to add peer to database: %w", err) } + if err != nil { + return nil, nil, nil, fmt.Errorf("failed to add peer to database after %d attempts: %w", maxAttempts, err) + } + + updateAccountPeers, err = isPeerInActiveGroup(ctx, am.Store, accountID, newPeer.ID) + if err != nil { + updateAccountPeers = true + } if newPeer == nil { return nil, nil, nil, fmt.Errorf("new peer is nil") } - am.StoreEvent(ctx, opEvent.InitiatorID, opEvent.TargetID, opEvent.AccountID, opEvent.Activity, opEvent.Meta) + opEvent.TargetID = newPeer.ID + opEvent.Meta = newPeer.EventMeta(am.GetDNSDomain(settings)) + if !addedByUser { + opEvent.Meta["setup_key_name"] = setupKeyName + } - unlock() - unlock = nil + am.StoreEvent(ctx, opEvent.InitiatorID, opEvent.TargetID, opEvent.AccountID, opEvent.Activity, opEvent.Meta) if updateAccountPeers { am.BufferUpdateAccountPeers(ctx, accountID) @@ -673,23 +694,21 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, setupKey, userID s return am.getValidatedPeerWithMap(ctx, false, accountID, newPeer) } -func getFreeIP(ctx context.Context, transaction store.Store, accountID string) (net.IP, error) { - takenIps, err := transaction.GetTakenIPs(ctx, store.LockingStrengthShare, accountID) +func getPeerIPDNSLabel(ctx context.Context, tx store.Store, ip net.IP, accountID, peerHostName string) (string, error) { + ip = ip.To4() + + dnsName, err := nbdns.GetParsedDomainLabel(peerHostName) if err != nil { - return nil, fmt.Errorf("failed to get taken IPs: %w", err) + return "", fmt.Errorf("failed to parse peer host name %s: %w", peerHostName, err) } - network, err := transaction.GetAccountNetwork(ctx, store.LockingStrengthUpdate, accountID) + _, err = tx.GetPeerIdByLabel(ctx, store.LockingStrengthNone, accountID, dnsName) if err != nil { - return nil, fmt.Errorf("failed getting network: %w", err) + //nolint:nilerr + return dnsName, nil } - nextIp, err := types.AllocatePeerIP(network.Net, takenIps) - if err != nil { - return nil, fmt.Errorf("failed to allocate new peer ip: %w", err) - } - - return nextIp, nil + return fmt.Sprintf("%s-%d-%d", dnsName, ip[2], ip[3]), nil } // SyncPeer checks whether peer is eligible for receiving NetworkMap (authenticated) and returns its NetworkMap if eligible @@ -1477,19 +1496,6 @@ func getPeerGroupIDs(ctx context.Context, transaction store.Store, accountID str return groupIDs, err } -func getPeerDNSLabels(ctx context.Context, transaction store.Store, accountID string) (types.LookupMap, error) { - dnsLabels, err := transaction.GetPeerLabelsInAccount(ctx, store.LockingStrengthShare, accountID) - if err != nil { - return nil, err - } - - existingLabels := make(types.LookupMap) - for _, label := range dnsLabels { - existingLabels[label] = struct{}{} - } - return existingLabels, nil -} - // IsPeerInActiveGroup checks if the given peer is part of a group that is used // in an active DNS, route, or ACL configuration. func isPeerInActiveGroup(ctx context.Context, transaction store.Store, accountID, peerID string) (bool, error) { diff --git a/management/server/peer/peer.go b/management/server/peer/peer.go index 8ce1dfb4e..f7140e254 100644 --- a/management/server/peer/peer.go +++ b/management/server/peer/peer.go @@ -20,14 +20,14 @@ type Peer struct { // WireGuard public key Key string `gorm:"index"` // IP address of the Peer - IP net.IP `gorm:"serializer:json"` + IP net.IP `gorm:"serializer:json"` // uniqueness index per accountID (check migrations) // Meta is a Peer system meta data Meta PeerSystemMeta `gorm:"embedded;embeddedPrefix:meta_"` // Name is peer's name (machine name) Name string // DNSLabel is the parsed peer name for domain resolution. It is used to form an FQDN by appending the account's // domain to the peer label. e.g. peer-dns-label.netbird.cloud - DNSLabel string + DNSLabel string // uniqueness index per accountID (check migrations) // Status peer's management connection status Status *PeerStatus `gorm:"embedded;embeddedPrefix:peer_status_"` // The user ID that registered the peer diff --git a/management/server/peer_test.go b/management/server/peer_test.go index 775385a29..3edf7e82c 100644 --- a/management/server/peer_test.go +++ b/management/server/peer_test.go @@ -10,7 +10,9 @@ import ( "net/netip" "os" "runtime" + "strconv" "strings" + "sync" "testing" "time" @@ -19,6 +21,7 @@ import ( log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/exp/maps" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" "github.com/netbirdio/netbird/management/server/integrations/port_forwarding" @@ -1391,7 +1394,7 @@ func Test_RegisterPeerBySetupKey(t *testing.T) { name: "Absent setup key", existingSetupKeyID: "AAAAAAAA-38F5-4553-B31E-DD66C696CEBB", expectAddPeerError: true, - expectedErrorMsgSubstring: "failed adding new peer: account not found", + expectedErrorMsgSubstring: "failed to get setup key: setup key not found", }, } @@ -2057,10 +2060,14 @@ func Test_DeletePeer(t *testing.T) { "peer1": { ID: "peer1", AccountID: accountID, + IP: net.IP{1, 1, 1, 1}, + DNSLabel: "peer1.test", }, "peer2": { ID: "peer2", AccountID: accountID, + IP: net.IP{2, 2, 2, 2}, + DNSLabel: "peer2.test", }, } account.Groups = map[string]*types.Group{ @@ -2090,3 +2097,138 @@ func Test_DeletePeer(t *testing.T) { assert.NotContains(t, group.Peers, "peer1") } + +func Test_IsUniqueConstraintError(t *testing.T) { + tests := []struct { + name string + engine types.Engine + }{ + { + name: "PostgreSQL uniqueness error", + engine: types.PostgresStoreEngine, + }, + { + name: "MySQL uniqueness error", + engine: types.MysqlStoreEngine, + }, + { + name: "SQLite uniqueness error", + engine: types.SqliteStoreEngine, + }, + } + + peer := &nbpeer.Peer{ + ID: "test-peer-id", + AccountID: "bf1c8084-ba50-4ce7-9439-34653001fc3b", + DNSLabel: "test-peer-dns-label", + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("NETBIRD_STORE_ENGINE", string(tt.engine)) + s, cleanup, err := store.NewTestStoreFromSQL(context.Background(), "testdata/extended-store.sql", t.TempDir()) + if err != nil { + t.Fatalf("Error when creating store: %s", err) + } + t.Cleanup(cleanup) + + err = s.AddPeerToAccount(context.Background(), store.LockingStrengthUpdate, peer) + assert.NoError(t, err) + + err = s.AddPeerToAccount(context.Background(), store.LockingStrengthUpdate, peer) + result := isUniqueConstraintError(err) + assert.True(t, result) + }) + } +} + +func Test_AddPeer(t *testing.T) { + t.Setenv("NETBIRD_STORE_ENGINE", string(types.PostgresStoreEngine)) + manager, err := createManager(t) + if err != nil { + t.Fatal(err) + return + } + + accountID := "testaccount" + userID := "testuser" + + _, err = createAccount(manager, accountID, userID, "domain.com") + if err != nil { + t.Fatal("error creating account") + return + } + + setupKey, err := manager.CreateSetupKey(context.Background(), accountID, "test-key", types.SetupKeyReusable, time.Hour, nil, 10000, userID, false, false) + if err != nil { + t.Fatal("error creating setup key") + return + } + + const totalPeers = 300 // totalPeers / differentHostnames should be less than 10 (due to concurrent retries) + const differentHostnames = 50 + + var wg sync.WaitGroup + errs := make(chan error, totalPeers+differentHostnames) + start := make(chan struct{}) + for i := 0; i < totalPeers; i++ { + wg.Add(1) + hostNameID := i % differentHostnames + + go func(i int) { + defer wg.Done() + + newPeer := &nbpeer.Peer{ + Key: "key" + strconv.Itoa(i), + Meta: nbpeer.PeerSystemMeta{Hostname: "peer" + strconv.Itoa(hostNameID), GoOS: "linux"}, + } + + <-start + + _, _, _, err := manager.AddPeer(context.Background(), setupKey.Key, "", newPeer) + if err != nil { + errs <- fmt.Errorf("AddPeer failed for peer %d: %w", i, err) + return + } + + }(i) + } + startTime := time.Now() + + close(start) + wg.Wait() + close(errs) + + t.Logf("time since start: %s", time.Since(startTime)) + + for err := range errs { + t.Fatal(err) + } + + account, err := manager.Store.GetAccount(context.Background(), accountID) + if err != nil { + t.Fatalf("Failed to get account %s: %v", accountID, err) + } + + assert.Equal(t, totalPeers, len(account.Peers), "Expected %d peers in account %s, got %d", totalPeers, accountID, len(account.Peers)) + + seenIP := make(map[string]bool) + for _, p := range account.Peers { + ipStr := p.IP.String() + if seenIP[ipStr] { + t.Fatalf("Duplicate IP found in account %s: %s", accountID, ipStr) + } + seenIP[ipStr] = true + } + + seenLabel := make(map[string]bool) + for _, p := range account.Peers { + if seenLabel[p.DNSLabel] { + t.Fatalf("Duplicate Label found in account %s: %s", accountID, p.DNSLabel) + } + seenLabel[p.DNSLabel] = true + } + + assert.Equal(t, totalPeers, maps.Values(account.SetupKeys)[0].UsedTimes) + assert.Equal(t, uint64(totalPeers), account.Network.Serial) +} diff --git a/management/server/store/file_store.go b/management/server/store/file_store.go index 3b95164f5..d5d9337ca 100644 --- a/management/server/store/file_store.go +++ b/management/server/store/file_store.go @@ -156,7 +156,7 @@ func restore(ctx context.Context, file string) (*FileStore, error) { allGroup, err := account.GetGroupAll() if err != nil { - log.WithContext(ctx).Errorf("unable to find the All group, this should happen only when migrate from a version that didn't support groups. Error: %v", err) + log.WithContext(ctx).Errorf("unable to find the All group, this should happen only when migratePreAuto from a version that didn't support groups. Error: %v", err) // if the All group didn't exist we probably don't have routes to update continue } diff --git a/management/server/store/sql_store.go b/management/server/store/sql_store.go index 72a73a57a..197255ab6 100644 --- a/management/server/store/sql_store.go +++ b/management/server/store/sql_store.go @@ -92,8 +92,8 @@ func NewSqlStore(ctx context.Context, db *gorm.DB, storeEngine types.Engine, met return &SqlStore{db: db, storeEngine: storeEngine, metrics: metrics, installationPK: 1}, nil } - if err := migrate(ctx, db); err != nil { - return nil, fmt.Errorf("migrate: %w", err) + if err := migratePreAuto(ctx, db); err != nil { + return nil, fmt.Errorf("migratePreAuto: %w", err) } err = db.AutoMigrate( &types.SetupKey{}, &nbpeer.Peer{}, &types.User{}, &types.PersonalAccessToken{}, &types.Group{}, @@ -102,7 +102,10 @@ func NewSqlStore(ctx context.Context, db *gorm.DB, storeEngine types.Engine, met &networkTypes.Network{}, &routerTypes.NetworkRouter{}, &resourceTypes.NetworkResource{}, ) if err != nil { - return nil, fmt.Errorf("auto migrate: %w", err) + return nil, fmt.Errorf("auto migratePreAuto: %w", err) + } + if err := migratePostAuto(ctx, db); err != nil { + return nil, fmt.Errorf("migratePostAuto: %w", err) } return &SqlStore{db: db, storeEngine: storeEngine, metrics: metrics, installationPK: 1}, nil @@ -967,7 +970,7 @@ func (s *SqlStore) GetTakenIPs(ctx context.Context, lockStrength LockingStrength return ips, nil } -func (s *SqlStore) GetPeerLabelsInAccount(ctx context.Context, lockStrength LockingStrength, accountID string) ([]string, error) { +func (s *SqlStore) GetPeerLabelsInAccount(ctx context.Context, lockStrength LockingStrength, accountID string, dnsLabel string) ([]string, error) { tx := s.db if lockStrength != LockingStrengthNone { tx = tx.Clauses(clause.Locking{Strength: string(lockStrength)}) @@ -975,7 +978,7 @@ func (s *SqlStore) GetPeerLabelsInAccount(ctx context.Context, lockStrength Lock var labels []string result := tx.Model(&nbpeer.Peer{}). - Where("account_id = ?", accountID). + Where("account_id = ? AND dns_label LIKE ?", accountID, dnsLabel+"%"). Pluck("dns_label", &labels) if result.Error != nil { @@ -1254,7 +1257,7 @@ func (s *SqlStore) GetSetupKeyBySecret(ctx context.Context, lockStrength Locking if result.Error != nil { if errors.Is(result.Error, gorm.ErrRecordNotFound) { - return nil, status.NewSetupKeyNotFoundError(key) + return nil, status.Errorf(status.PreconditionFailed, "setup key not found") } log.WithContext(ctx).Errorf("failed to get setup key by secret from store: %v", result.Error) return nil, status.Errorf(status.Internal, "failed to get setup key by secret from store") @@ -2546,6 +2549,27 @@ func (s *SqlStore) GetPeerByIP(ctx context.Context, lockStrength LockingStrength return &peer, nil } +func (s *SqlStore) GetPeerIdByLabel(ctx context.Context, lockStrength LockingStrength, accountID string, hostname string) (string, error) { + tx := s.db.WithContext(ctx) + if lockStrength != LockingStrengthNone { + tx = tx.Clauses(clause.Locking{Strength: string(lockStrength)}) + } + + var peerID string + result := tx.Model(&nbpeer.Peer{}). + Select("id"). + // Where(" = ?", hostname). + Where("account_id = ? AND dns_label = ?", accountID, hostname). + Limit(1). + Scan(&peerID) + + if peerID == "" { + return "", gorm.ErrRecordNotFound + } + + return peerID, result.Error +} + func (s *SqlStore) CountAccountsByPrivateDomain(ctx context.Context, domain string) (int64, error) { var count int64 result := s.db.Model(&types.Account{}). diff --git a/management/server/store/sql_store_test.go b/management/server/store/sql_store_test.go index f187be8c7..928486ab4 100644 --- a/management/server/store/sql_store_test.go +++ b/management/server/store/sql_store_test.go @@ -10,6 +10,7 @@ import ( "net/netip" "os" "runtime" + "sort" "sync" "testing" "time" @@ -630,7 +631,7 @@ func TestMigrate(t *testing.T) { t.Cleanup(cleanUp) assert.NoError(t, err) - err = migrate(context.Background(), store.(*SqlStore).db) + err = migratePreAuto(context.Background(), store.(*SqlStore).db) require.NoError(t, err, "Migration should not fail on empty db") _, ipnet, err := net.ParseCIDR("10.0.0.0/24") @@ -685,10 +686,10 @@ func TestMigrate(t *testing.T) { err = store.(*SqlStore).db.Save(rt).Error require.NoError(t, err, "Failed to insert Gob data") - err = migrate(context.Background(), store.(*SqlStore).db) + err = migratePreAuto(context.Background(), store.(*SqlStore).db) require.NoError(t, err, "Migration should not fail on gob populated db") - err = migrate(context.Background(), store.(*SqlStore).db) + err = migratePreAuto(context.Background(), store.(*SqlStore).db) require.NoError(t, err, "Migration should not fail on migrated db") err = store.(*SqlStore).db.Delete(rt).Where("id = ?", "route1").Error @@ -704,10 +705,10 @@ func TestMigrate(t *testing.T) { err = store.(*SqlStore).db.Save(nRT).Error require.NoError(t, err, "Failed to insert json nil slice data") - err = migrate(context.Background(), store.(*SqlStore).db) + err = migratePreAuto(context.Background(), store.(*SqlStore).db) require.NoError(t, err, "Migration should not fail on json nil slice populated db") - err = migrate(context.Background(), store.(*SqlStore).db) + err = migratePreAuto(context.Background(), store.(*SqlStore).db) require.NoError(t, err, "Migration should not fail on migrated db") } @@ -950,6 +951,7 @@ func TestSqlite_GetTakenIPs(t *testing.T) { peer1 := &nbpeer.Peer{ ID: "peer1", AccountID: existingAccountID, + DNSLabel: "peer1", IP: net.IP{1, 1, 1, 1}, } err = store.AddPeerToAccount(context.Background(), LockingStrengthUpdate, peer1) @@ -961,8 +963,9 @@ func TestSqlite_GetTakenIPs(t *testing.T) { assert.Equal(t, []net.IP{ip1}, takenIPs) peer2 := &nbpeer.Peer{ - ID: "peer2", + ID: "peer1second", AccountID: existingAccountID, + DNSLabel: "peer1-1", IP: net.IP{2, 2, 2, 2}, } err = store.AddPeerToAccount(context.Background(), LockingStrengthUpdate, peer2) @@ -972,49 +975,100 @@ func TestSqlite_GetTakenIPs(t *testing.T) { require.NoError(t, err) ip2 := net.IP{2, 2, 2, 2}.To16() assert.Equal(t, []net.IP{ip1, ip2}, takenIPs) - } func TestSqlite_GetPeerLabelsInAccount(t *testing.T) { - t.Setenv("NETBIRD_STORE_ENGINE", string(types.SqliteStoreEngine)) - store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/extended-store.sql", t.TempDir()) - if err != nil { - return - } - t.Cleanup(cleanup) + runTestForAllEngines(t, "../testdata/extended-store.sql", func(t *testing.T, store Store) { + existingAccountID := "bf1c8084-ba50-4ce7-9439-34653001fc3b" + peerHostname := "peer1" - existingAccountID := "bf1c8084-ba50-4ce7-9439-34653001fc3b" + _, err := store.GetAccount(context.Background(), existingAccountID) + require.NoError(t, err) - _, err = store.GetAccount(context.Background(), existingAccountID) - require.NoError(t, err) + labels, err := store.GetPeerLabelsInAccount(context.Background(), LockingStrengthShare, existingAccountID, peerHostname) + require.NoError(t, err) + assert.Equal(t, []string{}, labels) - labels, err := store.GetPeerLabelsInAccount(context.Background(), LockingStrengthShare, existingAccountID) - require.NoError(t, err) - assert.Equal(t, []string{}, labels) + peer1 := &nbpeer.Peer{ + ID: "peer1", + AccountID: existingAccountID, + DNSLabel: "peer1", + IP: net.IP{1, 1, 1, 1}, + } + err = store.AddPeerToAccount(context.Background(), LockingStrengthUpdate, peer1) + require.NoError(t, err) - peer1 := &nbpeer.Peer{ - ID: "peer1", - AccountID: existingAccountID, - DNSLabel: "peer1.domain.test", - } - err = store.AddPeerToAccount(context.Background(), LockingStrengthUpdate, peer1) - require.NoError(t, err) + labels, err = store.GetPeerLabelsInAccount(context.Background(), LockingStrengthShare, existingAccountID, peerHostname) + require.NoError(t, err) + assert.Equal(t, []string{"peer1"}, labels) - labels, err = store.GetPeerLabelsInAccount(context.Background(), LockingStrengthShare, existingAccountID) - require.NoError(t, err) - assert.Equal(t, []string{"peer1.domain.test"}, labels) + peer2 := &nbpeer.Peer{ + ID: "peer1second", + AccountID: existingAccountID, + DNSLabel: "peer1-1", + IP: net.IP{2, 2, 2, 2}, + } + err = store.AddPeerToAccount(context.Background(), LockingStrengthUpdate, peer2) + require.NoError(t, err) - peer2 := &nbpeer.Peer{ - ID: "peer2", - AccountID: existingAccountID, - DNSLabel: "peer2.domain.test", - } - err = store.AddPeerToAccount(context.Background(), LockingStrengthUpdate, peer2) - require.NoError(t, err) + labels, err = store.GetPeerLabelsInAccount(context.Background(), LockingStrengthShare, existingAccountID, peerHostname) + require.NoError(t, err) - labels, err = store.GetPeerLabelsInAccount(context.Background(), LockingStrengthShare, existingAccountID) - require.NoError(t, err) - assert.Equal(t, []string{"peer1.domain.test", "peer2.domain.test"}, labels) + expected := []string{"peer1", "peer1-1"} + sort.Strings(expected) + sort.Strings(labels) + assert.Equal(t, expected, labels) + }) +} + +func Test_AddPeerWithSameDnsLabel(t *testing.T) { + runTestForAllEngines(t, "../testdata/extended-store.sql", func(t *testing.T, store Store) { + existingAccountID := "bf1c8084-ba50-4ce7-9439-34653001fc3b" + + _, err := store.GetAccount(context.Background(), existingAccountID) + require.NoError(t, err) + + peer1 := &nbpeer.Peer{ + ID: "peer1", + AccountID: existingAccountID, + DNSLabel: "peer1.domain.test", + } + err = store.AddPeerToAccount(context.Background(), LockingStrengthUpdate, peer1) + require.NoError(t, err) + + peer2 := &nbpeer.Peer{ + ID: "peer1second", + AccountID: existingAccountID, + DNSLabel: "peer1.domain.test", + } + err = store.AddPeerToAccount(context.Background(), LockingStrengthUpdate, peer2) + require.Error(t, err) + }) +} + +func Test_AddPeerWithSameIP(t *testing.T) { + runTestForAllEngines(t, "../testdata/extended-store.sql", func(t *testing.T, store Store) { + existingAccountID := "bf1c8084-ba50-4ce7-9439-34653001fc3b" + + _, err := store.GetAccount(context.Background(), existingAccountID) + require.NoError(t, err) + + peer1 := &nbpeer.Peer{ + ID: "peer1", + AccountID: existingAccountID, + IP: net.IP{1, 1, 1, 1}, + } + err = store.AddPeerToAccount(context.Background(), LockingStrengthUpdate, peer1) + require.NoError(t, err) + + peer2 := &nbpeer.Peer{ + ID: "peer1second", + AccountID: existingAccountID, + IP: net.IP{1, 1, 1, 1}, + } + err = store.AddPeerToAccount(context.Background(), LockingStrengthUpdate, peer2) + require.Error(t, err) + }) } func TestSqlite_GetAccountNetwork(t *testing.T) { diff --git a/management/server/store/store.go b/management/server/store/store.go index f66130ad3..30ff1549d 100644 --- a/management/server/store/store.go +++ b/management/server/store/store.go @@ -117,7 +117,7 @@ type Store interface { SavePostureChecks(ctx context.Context, lockStrength LockingStrength, postureCheck *posture.Checks) error DeletePostureChecks(ctx context.Context, lockStrength LockingStrength, accountID, postureChecksID string) error - GetPeerLabelsInAccount(ctx context.Context, lockStrength LockingStrength, accountId string) ([]string, error) + GetPeerLabelsInAccount(ctx context.Context, lockStrength LockingStrength, accountId string, hostname string) ([]string, error) AddPeerToAllGroup(ctx context.Context, lockStrength LockingStrength, accountID string, peerID string) error AddPeerToGroup(ctx context.Context, lockStrength LockingStrength, accountId string, peerId string, groupID string) error GetPeerGroups(ctx context.Context, lockStrength LockingStrength, accountId string, peerId string) ([]*types.Group, error) @@ -193,6 +193,7 @@ type Store interface { SaveNetworkResource(ctx context.Context, lockStrength LockingStrength, resource *resourceTypes.NetworkResource) error DeleteNetworkResource(ctx context.Context, lockStrength LockingStrength, accountID, resourceID string) error GetPeerByIP(ctx context.Context, lockStrength LockingStrength, accountID string, ip net.IP) (*nbpeer.Peer, error) + GetPeerIdByLabel(ctx context.Context, lockStrength LockingStrength, accountID string, hostname string) (string, error) } const ( @@ -234,9 +235,9 @@ func getStoreEngine(ctx context.Context, dataDir string, kind types.Engine) type if util.FileExists(jsonStoreFile) && !util.FileExists(sqliteStoreFile) { log.WithContext(ctx).Warnf("unsupported store engine specified, but found %s. Automatically migrating to SQLite.", jsonStoreFile) - // Attempt to migrate from JSON store to SQLite + // Attempt to migratePreAuto from JSON store to SQLite if err := MigrateFileStoreToSqlite(ctx, dataDir); err != nil { - log.WithContext(ctx).Errorf("failed to migrate filestore to SQLite: %v", err) + log.WithContext(ctx).Errorf("failed to migratePreAuto filestore to SQLite: %v", err) kind = types.FileStoreEngine } } @@ -280,9 +281,9 @@ func checkFileStoreEngine(kind types.Engine, dataDir string) error { return nil } -// migrate migrates the SQLite database to the latest schema -func migrate(ctx context.Context, db *gorm.DB) error { - migrations := getMigrations(ctx) +// migratePreAuto migrates the SQLite database to the latest schema +func migratePreAuto(ctx context.Context, db *gorm.DB) error { + migrations := getMigrationsPreAuto(ctx) for _, m := range migrations { if err := m(db); err != nil { @@ -293,7 +294,7 @@ func migrate(ctx context.Context, db *gorm.DB) error { return nil } -func getMigrations(ctx context.Context) []migrationFunc { +func getMigrationsPreAuto(ctx context.Context) []migrationFunc { return []migrationFunc{ func(db *gorm.DB) error { return migration.MigrateFieldFromGobToJSON[types.Account, net.IPNet](ctx, db, "network_net") @@ -329,6 +330,28 @@ func getMigrations(ctx context.Context) []migrationFunc { return migration.DropIndex[routerTypes.NetworkRouter](ctx, db, "idx_network_routers_id") }, } +} // migratePostAuto migrates the SQLite database to the latest schema +func migratePostAuto(ctx context.Context, db *gorm.DB) error { + migrations := getMigrationsPostAuto(ctx) + + for _, m := range migrations { + if err := m(db); err != nil { + return err + } + } + + return nil +} + +func getMigrationsPostAuto(ctx context.Context) []migrationFunc { + return []migrationFunc{ + func(db *gorm.DB) error { + return migration.CreateIndexIfNotExists[nbpeer.Peer](ctx, db, "idx_account_ip", "account_id", "ip") + }, + func(db *gorm.DB) error { + return migration.CreateIndexIfNotExists[nbpeer.Peer](ctx, db, "idx_account_dnslabel", "account_id", "dns_label") + }, + } } // NewTestStoreFromSQL is only used in tests. It will create a test database base of the store engine set in env. @@ -577,7 +600,7 @@ func MigrateFileStoreToSqlite(ctx context.Context, dataDir string) error { sqliteStoreAccounts := len(store.GetAllAccounts(ctx)) if fsStoreAccounts != sqliteStoreAccounts { - return fmt.Errorf("failed to migrate accounts from file to sqlite. Expected accounts: %d, got: %d", + return fmt.Errorf("failed to migratePreAuto accounts from file to sqlite. Expected accounts: %d, got: %d", fsStoreAccounts, sqliteStoreAccounts) } diff --git a/management/server/testdata/store.sql b/management/server/testdata/store.sql index 41b8fa2f7..4b126c618 100644 --- a/management/server/testdata/store.sql +++ b/management/server/testdata/store.sql @@ -52,4 +52,4 @@ INSERT INTO policy_rules VALUES('cs387mkv2d4bgq41b6n0','cs1tnh0hhcjnqoiuebf0','D INSERT INTO network_routers VALUES('ctc20ji7qv9ck2sebc80','ct286bi7qv930dsrrug0','bf1c8084-ba50-4ce7-9439-34653001fc3b','cs1tnh0hhcjnqoiuebeg',NULL,0,0); INSERT INTO network_resources VALUES ('ctc4nci7qv9061u6ilfg','ct286bi7qv930dsrrug0','bf1c8084-ba50-4ce7-9439-34653001fc3b','Host','192.168.1.1'); INSERT INTO networks VALUES('ct286bi7qv930dsrrug0','bf1c8084-ba50-4ce7-9439-34653001fc3b','Test Network','Test Network'); -INSERT INTO peers VALUES('ct286bi7qv930dsrrug0','bf1c8084-ba50-4ce7-9439-34653001fc3b','','','192.168.0.0','','','','','','','','','','','','','','','','','test','test','2023-01-01 00:00:00+00:00',0,0,0,'a23efe53-63fb-11ec-90d6-0242ac120003','',0,0,'2023-01-01 00:00:00+00:00','2023-01-01 00:00:00+00:00',0,'','','',0); +INSERT INTO peers VALUES('ct286bi7qv930dsrrug0','bf1c8084-ba50-4ce7-9439-34653001fc3b','','','"192.168.0.0"','','','','','','','','','','','','','','','','','test','test','2023-01-01 00:00:00+00:00',0,0,0,'a23efe53-63fb-11ec-90d6-0242ac120003','',0,0,'2023-01-01 00:00:00+00:00','2023-01-01 00:00:00+00:00',0,'','','',0); diff --git a/management/server/testdata/store_with_expired_peers.sql b/management/server/testdata/store_with_expired_peers.sql index 5990a0625..f2ef56a23 100644 --- a/management/server/testdata/store_with_expired_peers.sql +++ b/management/server/testdata/store_with_expired_peers.sql @@ -30,7 +30,7 @@ INSERT INTO setup_keys VALUES('','bf1c8084-ba50-4ce7-9439-34653001fc3b','A2C8E62 INSERT INTO peers VALUES('cfvprsrlo1hqoo49ohog','bf1c8084-ba50-4ce7-9439-34653001fc3b','5rvhvriKJZ3S9oxYToVj5TzDM9u9y8cxg7htIMWlYAg=','72546A29-6BC8-4311-BCFC-9CDBF33F1A48','"100.64.114.31"','f2a34f6a4731','linux','Linux','11','unknown','Debian GNU/Linux','','0.12.0','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'f2a34f6a4731','f2a34f6a4731','2023-03-02 09:21:02.189035775+01:00',0,0,0,'','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAILzUUSYG/LGnV8zarb2SGN+tib/PZ+M7cL4WtTzUrTpk',0,1,0,'2023-03-01 19:48:19.817799698+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0); INSERT INTO peers VALUES('cg05lnblo1hkg2j514p0','bf1c8084-ba50-4ce7-9439-34653001fc3b','RlSy2vzoG2HyMBTUImXOiVhCBiiBa5qD5xzMxkiFDW4=','','"100.64.39.54"','expiredhost','linux','Linux','22.04','x86_64','Ubuntu','','development','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'expiredhost','expiredhost','2023-03-02 09:19:57.276717255+01:00',0,1,0,'edafee4e-63fb-11ec-90d6-0242ac120003','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIMbK5ZXJsGOOWoBT4OmkPtgdPZe2Q7bDuS/zjn2CZxhK',0,1,0,'2023-03-02 09:14:21.791679181+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0); INSERT INTO peers VALUES('cg3161rlo1hs9cq94gdg','bf1c8084-ba50-4ce7-9439-34653001fc3b','mVABSKj28gv+JRsf7e0NEGKgSOGTfU/nPB2cpuG56HU=','','"100.64.117.96"','testhost','linux','Linux','22.04','x86_64','Ubuntu','','development','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'testhost','testhost','2023-03-06 18:21:27.252010027+01:00',0,0,0,'edafee4e-63fb-11ec-90d6-0242ac120003','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAINWvvUkFFcrj48CWTkNUb/do/n52i1L5dH4DhGu+4ZuM',0,0,0,'2023-03-07 09:02:47.442857106+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0); -INSERT INTO peers VALUES('csrnkiq7qv9d8aitqd50','bf1c8084-ba50-4ce7-9439-34653001fc3b','mVABSKj28gv+JRsf7e0NEGKgSOGTfU/nPB2cpuG56HU=','','"100.64.117.96"','testhost','linux','Linux','22.04','x86_64','Ubuntu','','development','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'testhost','testhost','2023-03-06 18:21:27.252010027+01:00',0,0,0,'f4f6d672-63fb-11ec-90d6-0242ac120003','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAINWvvUkFFcrj48CWTkNUb/do/n52i1L5dH4DhGu+4ZuM',0,0,1,'2023-03-07 09:02:47.442857106+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0); +INSERT INTO peers VALUES('csrnkiq7qv9d8aitqd50','bf1c8084-ba50-4ce7-9439-34653001fc3b','mVABSKj28gv+JRsf7e0NEGKgSOGTfU/nPB2cpuG56HU=','','"100.64.117.97"','testhost','linux','Linux','22.04','x86_64','Ubuntu','','development','','',NULL,'','','','{"Cloud":"","Platform":""}',NULL,'testhost','testhost-1','2023-03-06 18:21:27.252010027+01:00',0,0,0,'f4f6d672-63fb-11ec-90d6-0242ac120003','ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAINWvvUkFFcrj48CWTkNUb/do/n52i1L5dH4DhGu+4ZuM',0,0,1,'2023-03-07 09:02:47.442857106+01:00','2024-10-02 17:00:32.527947+02:00',0,'""','','',0); INSERT INTO users VALUES('f4f6d672-63fb-11ec-90d6-0242ac120003','bf1c8084-ba50-4ce7-9439-34653001fc3b','user',0,0,'','[]',0,NULL,'2024-10-02 17:00:32.528196+02:00','api',0,''); INSERT INTO users VALUES('edafee4e-63fb-11ec-90d6-0242ac120003','bf1c8084-ba50-4ce7-9439-34653001fc3b','admin',0,0,'','[]',0,NULL,'2024-10-02 17:00:32.528196+02:00','api',0,''); INSERT INTO installations VALUES(1,''); diff --git a/management/server/types/network.go b/management/server/types/network.go index 00082bb41..eb8415264 100644 --- a/management/server/types/network.go +++ b/management/server/types/network.go @@ -1,6 +1,7 @@ package types import ( + "encoding/binary" "math/rand" "net" "sync" @@ -161,24 +162,65 @@ func (n *Network) Copy() *Network { // This method considers already taken IPs and reuses IPs if there are gaps in takenIps // E.g. if ipNet=100.30.0.0/16 and takenIps=[100.30.0.1, 100.30.0.4] then the result would be 100.30.0.2 or 100.30.0.3 func AllocatePeerIP(ipNet net.IPNet, takenIps []net.IP) (net.IP, error) { - takenIPMap := make(map[string]struct{}) - takenIPMap[ipNet.IP.String()] = struct{}{} + baseIP := ipToUint32(ipNet.IP.Mask(ipNet.Mask)) + totalIPs := uint32(1 << SubnetSize) + + taken := make(map[uint32]struct{}, len(takenIps)+1) + taken[baseIP] = struct{}{} // reserve network IP + taken[baseIP+totalIPs-1] = struct{}{} // reserve broadcast IP + for _, ip := range takenIps { - takenIPMap[ip.String()] = struct{}{} + taken[ipToUint32(ip)] = struct{}{} } - ips, _ := generateIPs(&ipNet, takenIPMap) + rng := rand.New(rand.NewSource(time.Now().UnixNano())) + maxAttempts := (int(totalIPs) - len(taken)) / 100 - if len(ips) == 0 { - return nil, status.Errorf(status.PreconditionFailed, "failed allocating new IP for the ipNet %s - network is out of IPs", ipNet.String()) + for i := 0; i < maxAttempts; i++ { + offset := uint32(rng.Intn(int(totalIPs-2))) + 1 + candidate := baseIP + offset + if _, exists := taken[candidate]; !exists { + return uint32ToIP(candidate), nil + } } - // pick a random IP - s := rand.NewSource(time.Now().Unix()) - r := rand.New(s) - intn := r.Intn(len(ips)) + for offset := uint32(1); offset < totalIPs-1; offset++ { + candidate := baseIP + offset + if _, exists := taken[candidate]; !exists { + return uint32ToIP(candidate), nil + } + } - return ips[intn], nil + return nil, status.Errorf(status.PreconditionFailed, "network %s is out of IPs", ipNet.String()) +} + +func AllocateRandomPeerIP(ipNet net.IPNet) (net.IP, error) { + baseIP := ipToUint32(ipNet.IP.Mask(ipNet.Mask)) + + ones, bits := ipNet.Mask.Size() + hostBits := bits - ones + + totalIPs := uint32(1 << hostBits) + + rng := rand.New(rand.NewSource(time.Now().UnixNano())) + offset := uint32(rng.Intn(int(totalIPs-2))) + 1 + + candidate := baseIP + offset + return uint32ToIP(candidate), nil +} + +func ipToUint32(ip net.IP) uint32 { + ip = ip.To4() + if len(ip) < 4 { + return 0 + } + return binary.BigEndian.Uint32(ip) +} + +func uint32ToIP(n uint32) net.IP { + ip := make(net.IP, 4) + binary.BigEndian.PutUint32(ip, n) + return ip } // generateIPs generates a list of all possible IPs of the given network excluding IPs specified in the exclusion list From 57961afe9557ecc651a2f40c53243b8aaf0a2681 Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Wed, 2 Jul 2025 18:40:07 +0200 Subject: [PATCH 02/10] [doc] Add forum link (#4093) * Add forum link * Add forum link --- README.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 1d2a976c2..c3b365694 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,9 @@
+ + +
@@ -29,13 +32,13 @@
See Documentation
- Join our Slack channel + Join our Slack channel or our Community forum

- - New: NetBird Kubernetes Operator + + New: NetBird terraform provider

From 551cb4e467d44d9b83b7c636d33e985590329081 Mon Sep 17 00:00:00 2001 From: Pascal Fischer <32096965+pascal-fischer@users.noreply.github.com> Date: Wed, 2 Jul 2025 20:04:28 +0200 Subject: [PATCH 03/10] [management] expect specific error types on registration with setup key (#4094) --- management/server/peer.go | 6 +++--- management/server/peer_test.go | 11 ++++++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/management/server/peer.go b/management/server/peer.go index 2c1d8f64c..50967cfb9 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -488,7 +488,7 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, setupKey, userID s if addedByUser { user, err := am.Store.GetUserByUserID(ctx, store.LockingStrengthNone, userID) if err != nil { - return nil, nil, nil, fmt.Errorf("failed to get user groups: %w", err) + return nil, nil, nil, status.Errorf(status.NotFound, "failed adding new peer: user not found") } groupsToAdd = user.AutoGroups opEvent.InitiatorID = userID @@ -498,12 +498,12 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, setupKey, userID s // Validate the setup key sk, err := am.Store.GetSetupKeyBySecret(ctx, store.LockingStrengthNone, encodedHashedKey) if err != nil { - return nil, nil, nil, fmt.Errorf("failed to get setup key: %w", err) + return nil, nil, nil, status.Errorf(status.NotFound, "couldn't add peer: setup key is invalid") } // we will check key twice for early return if !sk.IsValid() { - return nil, nil, nil, status.Errorf(status.PreconditionFailed, "couldn't add peer: setup key is invalid") + return nil, nil, nil, status.Errorf(status.NotFound, "couldn't add peer: setup key is invalid") } opEvent.InitiatorID = sk.Id diff --git a/management/server/peer_test.go b/management/server/peer_test.go index 3edf7e82c..31439d670 100644 --- a/management/server/peer_test.go +++ b/management/server/peer_test.go @@ -27,6 +27,7 @@ import ( "github.com/netbirdio/netbird/management/server/integrations/port_forwarding" "github.com/netbirdio/netbird/management/server/permissions" "github.com/netbirdio/netbird/management/server/settings" + "github.com/netbirdio/netbird/management/server/status" "github.com/netbirdio/netbird/management/server/util" @@ -1376,6 +1377,7 @@ func Test_RegisterPeerBySetupKey(t *testing.T) { existingSetupKeyID string expectedGroupIDsInAccount []string expectAddPeerError bool + errorType status.Type expectedErrorMsgSubstring string }{ { @@ -1388,13 +1390,15 @@ func Test_RegisterPeerBySetupKey(t *testing.T) { name: "Failed registration with setup key not allowing extra DNS labels", existingSetupKeyID: "A2C8E62B-38F5-4553-B31E-DD66C696CEBB", expectAddPeerError: true, + errorType: status.PreconditionFailed, expectedErrorMsgSubstring: "setup key doesn't allow extra DNS labels", }, { name: "Absent setup key", existingSetupKeyID: "AAAAAAAA-38F5-4553-B31E-DD66C696CEBB", expectAddPeerError: true, - expectedErrorMsgSubstring: "failed to get setup key: setup key not found", + errorType: status.NotFound, + expectedErrorMsgSubstring: "couldn't add peer: setup key is invalid", }, } @@ -1419,6 +1423,11 @@ func Test_RegisterPeerBySetupKey(t *testing.T) { if tc.expectAddPeerError { require.Error(t, err, "Expected an error when adding peer with setup key: %s", tc.existingSetupKeyID) assert.Contains(t, err.Error(), tc.expectedErrorMsgSubstring, "Error message mismatch") + e, ok := status.FromError(err) + if !ok { + t.Fatal("Failed to map error") + } + assert.Equal(t, e.Type(), tc.errorType) return } From 2c81cf2c1ea3a55466090fbf9741880c861ace3a Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Thu, 3 Jul 2025 09:01:32 +0200 Subject: [PATCH 04/10] [management] Add account onboarding (#4084) This PR introduces a new onboarding feature to handle such flows in the dashboard by defining an AccountOnboarding model, persisting it in the store, exposing CRUD operations in the manager and HTTP handlers, and updating API schemas and tests accordingly. Add AccountOnboarding struct and embed it in Account Extend Store and DefaultAccountManager with onboarding methods and SQL migrations Update HTTP handlers, API types, OpenAPI spec, and add end-to-end tests --- management/server/account.go | 69 +++++++ management/server/account/manager.go | 2 + management/server/account_test.go | 71 +++++++ management/server/http/api/openapi.yml | 19 ++ management/server/http/api/types.gen.go | 17 +- .../handlers/accounts/accounts_handler.go | 32 ++- .../accounts/accounts_handler_test.go | 44 +++- management/server/mock_server/account_mock.go | 193 ++++++++++-------- management/server/status/error.go | 5 + management/server/store/sql_store.go | 28 ++- management/server/store/sql_store_test.go | 74 +++++++ management/server/store/store.go | 2 + management/server/testdata/store.sql | 4 +- management/server/types/account.go | 19 +- 14 files changed, 476 insertions(+), 103 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 8a80aefb6..cd0c933f0 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -1204,6 +1204,71 @@ func (am *DefaultAccountManager) GetAccountMeta(ctx context.Context, accountID s return am.Store.GetAccountMeta(ctx, store.LockingStrengthShare, accountID) } +// GetAccountOnboarding retrieves the onboarding information for a specific account. +func (am *DefaultAccountManager) GetAccountOnboarding(ctx context.Context, accountID string, userID string) (*types.AccountOnboarding, error) { + allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Accounts, operations.Read) + if err != nil { + return nil, status.NewPermissionValidationError(err) + } + + if !allowed { + return nil, status.NewPermissionDeniedError() + } + + onboarding, err := am.Store.GetAccountOnboarding(ctx, accountID) + if err != nil && err.Error() != status.NewAccountOnboardingNotFoundError(accountID).Error() { + log.Errorf("failed to get account onboarding for accountssssssss %s: %v", accountID, err) + return nil, err + } + + if onboarding == nil { + onboarding = &types.AccountOnboarding{ + AccountID: accountID, + } + } + + return onboarding, nil +} + +func (am *DefaultAccountManager) UpdateAccountOnboarding(ctx context.Context, accountID, userID string, newOnboarding *types.AccountOnboarding) (*types.AccountOnboarding, error) { + allowed, err := am.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Settings, operations.Update) + if err != nil { + return nil, fmt.Errorf("failed to validate user permissions: %w", err) + } + + if !allowed { + return nil, status.NewPermissionDeniedError() + } + + oldOnboarding, err := am.Store.GetAccountOnboarding(ctx, accountID) + if err != nil && err.Error() != status.NewAccountOnboardingNotFoundError(accountID).Error() { + return nil, fmt.Errorf("failed to get account onboarding: %w", err) + } + + if oldOnboarding == nil { + oldOnboarding = &types.AccountOnboarding{ + AccountID: accountID, + } + } + + if newOnboarding == nil { + return oldOnboarding, nil + } + + if oldOnboarding.IsEqual(*newOnboarding) { + log.WithContext(ctx).Debugf("no changes in onboarding for account %s", accountID) + return oldOnboarding, nil + } + + newOnboarding.AccountID = accountID + err = am.Store.SaveAccountOnboarding(ctx, newOnboarding) + if err != nil { + return nil, fmt.Errorf("failed to update account onboarding: %w", err) + } + + return newOnboarding, nil +} + func (am *DefaultAccountManager) GetAccountIDFromUserAuth(ctx context.Context, userAuth nbcontext.UserAuth) (string, string, error) { if userAuth.UserId == "" { return "", "", errors.New(emptyUserID) @@ -1726,6 +1791,10 @@ func newAccountWithId(ctx context.Context, accountID, userID, domain string, dis PeerInactivityExpiration: types.DefaultPeerInactivityExpiration, RoutingPeerDNSResolutionEnabled: true, }, + Onboarding: types.AccountOnboarding{ + OnboardingFlowPending: true, + SignupFormPending: true, + }, } if err := acc.AddAllGroup(disableDefaultPolicy); err != nil { diff --git a/management/server/account/manager.go b/management/server/account/manager.go index de5031c03..ed17fa5ec 100644 --- a/management/server/account/manager.go +++ b/management/server/account/manager.go @@ -39,6 +39,7 @@ type Manager interface { GetSetupKey(ctx context.Context, accountID, userID, keyID string) (*types.SetupKey, error) GetAccountByID(ctx context.Context, accountID string, userID string) (*types.Account, error) GetAccountMeta(ctx context.Context, accountID string, userID string) (*types.AccountMeta, error) + GetAccountOnboarding(ctx context.Context, accountID string, userID string) (*types.AccountOnboarding, error) AccountExists(ctx context.Context, accountID string) (bool, error) GetAccountIDByUserID(ctx context.Context, userID, domain string) (string, error) GetAccountIDFromUserAuth(ctx context.Context, userAuth nbcontext.UserAuth) (string, string, error) @@ -89,6 +90,7 @@ type Manager interface { SaveDNSSettings(ctx context.Context, accountID string, userID string, dnsSettingsToSave *types.DNSSettings) error GetPeer(ctx context.Context, accountID, peerID, userID string) (*nbpeer.Peer, error) UpdateAccountSettings(ctx context.Context, accountID, userID string, newSettings *types.Settings) (*types.Settings, error) + UpdateAccountOnboarding(ctx context.Context, accountID, userID string, newOnboarding *types.AccountOnboarding) (*types.AccountOnboarding, error) LoginPeer(ctx context.Context, login types.PeerLogin) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, error) // used by peer gRPC API SyncPeer(ctx context.Context, sync types.PeerSync, accountID string) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, error) // used by peer gRPC API GetAllConnectedPeers() (map[string]struct{}, error) diff --git a/management/server/account_test.go b/management/server/account_test.go index 60353389f..fcd40b082 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -3448,3 +3448,74 @@ func TestPropagateUserGroupMemberships(t *testing.T) { } }) } + +func TestDefaultAccountManager_GetAccountOnboarding(t *testing.T) { + manager, err := createManager(t) + require.NoError(t, err) + + account, err := manager.GetOrCreateAccountByUser(context.Background(), userID, "") + require.NoError(t, err) + + t.Run("should return account onboarding when onboarding exist", func(t *testing.T) { + onboarding, err := manager.GetAccountOnboarding(context.Background(), account.Id, userID) + require.NoError(t, err) + require.NotNil(t, onboarding) + assert.Equal(t, account.Id, onboarding.AccountID) + assert.Equal(t, true, onboarding.OnboardingFlowPending) + assert.Equal(t, true, onboarding.SignupFormPending) + if onboarding.UpdatedAt.IsZero() { + t.Errorf("Onboarding was not retrieved from the store") + } + }) + + t.Run("should return account onboarding when onboard don't exist", func(t *testing.T) { + account.Id = "with-zero-onboarding" + account.Onboarding = types.AccountOnboarding{} + err = manager.Store.SaveAccount(context.Background(), account) + require.NoError(t, err) + onboarding, err := manager.GetAccountOnboarding(context.Background(), account.Id, userID) + require.NoError(t, err) + require.NotNil(t, onboarding) + _, err = manager.Store.GetAccountOnboarding(context.Background(), account.Id) + require.Error(t, err, "should return error when onboarding is not set") + }) +} + +func TestDefaultAccountManager_UpdateAccountOnboarding(t *testing.T) { + manager, err := createManager(t) + require.NoError(t, err) + + account, err := manager.GetOrCreateAccountByUser(context.Background(), userID, "") + require.NoError(t, err) + + onboarding := &types.AccountOnboarding{ + OnboardingFlowPending: true, + SignupFormPending: true, + } + + t.Run("update onboarding with no change", func(t *testing.T) { + updated, err := manager.UpdateAccountOnboarding(context.Background(), account.Id, userID, onboarding) + require.NoError(t, err) + assert.Equal(t, onboarding.OnboardingFlowPending, updated.OnboardingFlowPending) + assert.Equal(t, onboarding.SignupFormPending, updated.SignupFormPending) + if updated.UpdatedAt.IsZero() { + t.Errorf("Onboarding was updated in the store") + } + }) + + onboarding.OnboardingFlowPending = false + onboarding.SignupFormPending = false + + t.Run("update onboarding", func(t *testing.T) { + updated, err := manager.UpdateAccountOnboarding(context.Background(), account.Id, userID, onboarding) + require.NoError(t, err) + require.NotNil(t, updated) + assert.Equal(t, onboarding.OnboardingFlowPending, updated.OnboardingFlowPending) + assert.Equal(t, onboarding.SignupFormPending, updated.SignupFormPending) + }) + + t.Run("update onboarding with no onboarding", func(t *testing.T) { + _, err = manager.UpdateAccountOnboarding(context.Background(), account.Id, userID, nil) + require.NoError(t, err) + }) +} diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml index 1c5ca9b04..f8c2b9854 100644 --- a/management/server/http/api/openapi.yml +++ b/management/server/http/api/openapi.yml @@ -60,6 +60,8 @@ components: description: Account creator type: string example: google-oauth2|277474792786460067937 + onboarding: + $ref: '#/components/schemas/AccountOnboarding' required: - id - settings @@ -67,6 +69,21 @@ components: - domain_category - created_at - created_by + - onboarding + AccountOnboarding: + type: object + properties: + signup_form_pending: + description: Indicates whether the account signup form is pending + type: boolean + example: true + onboarding_flow_pending: + description: Indicates whether the account onboarding flow is pending + type: boolean + example: false + required: + - signup_form_pending + - onboarding_flow_pending AccountSettings: type: object properties: @@ -153,6 +170,8 @@ components: properties: settings: $ref: '#/components/schemas/AccountSettings' + onboarding: + $ref: '#/components/schemas/AccountOnboarding' required: - settings User: diff --git a/management/server/http/api/types.gen.go b/management/server/http/api/types.gen.go index d27fd2a57..a9f17aab4 100644 --- a/management/server/http/api/types.gen.go +++ b/management/server/http/api/types.gen.go @@ -250,8 +250,9 @@ type Account struct { DomainCategory string `json:"domain_category"` // Id Account ID - Id string `json:"id"` - Settings AccountSettings `json:"settings"` + Id string `json:"id"` + Onboarding AccountOnboarding `json:"onboarding"` + Settings AccountSettings `json:"settings"` } // AccountExtraSettings defines model for AccountExtraSettings. @@ -266,9 +267,19 @@ type AccountExtraSettings struct { PeerApprovalEnabled bool `json:"peer_approval_enabled"` } +// AccountOnboarding defines model for AccountOnboarding. +type AccountOnboarding struct { + // OnboardingFlowPending Indicates whether the account onboarding flow is pending + OnboardingFlowPending bool `json:"onboarding_flow_pending"` + + // SignupFormPending Indicates whether the account signup form is pending + SignupFormPending bool `json:"signup_form_pending"` +} + // AccountRequest defines model for AccountRequest. type AccountRequest struct { - Settings AccountSettings `json:"settings"` + Onboarding *AccountOnboarding `json:"onboarding,omitempty"` + Settings AccountSettings `json:"settings"` } // AccountSettings defines model for AccountSettings. diff --git a/management/server/http/handlers/accounts/accounts_handler.go b/management/server/http/handlers/accounts/accounts_handler.go index dfc782b3f..ab59434d1 100644 --- a/management/server/http/handlers/accounts/accounts_handler.go +++ b/management/server/http/handlers/accounts/accounts_handler.go @@ -59,7 +59,13 @@ func (h *handler) getAllAccounts(w http.ResponseWriter, r *http.Request) { return } - resp := toAccountResponse(accountID, settings, meta) + onboarding, err := h.accountManager.GetAccountOnboarding(r.Context(), accountID, userID) + if err != nil { + util.WriteError(r.Context(), err, w) + return + } + + resp := toAccountResponse(accountID, settings, meta, onboarding) util.WriteJSONObject(r.Context(), w, []*api.Account{resp}) } @@ -126,6 +132,20 @@ func (h *handler) updateAccount(w http.ResponseWriter, r *http.Request) { settings.LazyConnectionEnabled = *req.Settings.LazyConnectionEnabled } + var onboarding *types.AccountOnboarding + if req.Onboarding != nil { + onboarding = &types.AccountOnboarding{ + OnboardingFlowPending: req.Onboarding.OnboardingFlowPending, + SignupFormPending: req.Onboarding.SignupFormPending, + } + } + + updatedOnboarding, err := h.accountManager.UpdateAccountOnboarding(r.Context(), accountID, userID, onboarding) + if err != nil { + util.WriteError(r.Context(), err, w) + return + } + updatedSettings, err := h.accountManager.UpdateAccountSettings(r.Context(), accountID, userID, settings) if err != nil { util.WriteError(r.Context(), err, w) @@ -138,7 +158,7 @@ func (h *handler) updateAccount(w http.ResponseWriter, r *http.Request) { return } - resp := toAccountResponse(accountID, updatedSettings, meta) + resp := toAccountResponse(accountID, updatedSettings, meta, updatedOnboarding) util.WriteJSONObject(r.Context(), w, &resp) } @@ -167,7 +187,7 @@ func (h *handler) deleteAccount(w http.ResponseWriter, r *http.Request) { util.WriteJSONObject(r.Context(), w, util.EmptyObject{}) } -func toAccountResponse(accountID string, settings *types.Settings, meta *types.AccountMeta) *api.Account { +func toAccountResponse(accountID string, settings *types.Settings, meta *types.AccountMeta, onboarding *types.AccountOnboarding) *api.Account { jwtAllowGroups := settings.JWTAllowGroups if jwtAllowGroups == nil { jwtAllowGroups = []string{} @@ -188,6 +208,11 @@ func toAccountResponse(accountID string, settings *types.Settings, meta *types.A DnsDomain: &settings.DNSDomain, } + apiOnboarding := api.AccountOnboarding{ + OnboardingFlowPending: onboarding.OnboardingFlowPending, + SignupFormPending: onboarding.SignupFormPending, + } + if settings.Extra != nil { apiSettings.Extra = &api.AccountExtraSettings{ PeerApprovalEnabled: settings.Extra.PeerApprovalEnabled, @@ -203,5 +228,6 @@ func toAccountResponse(accountID string, settings *types.Settings, meta *types.A CreatedBy: meta.CreatedBy, Domain: meta.Domain, DomainCategory: meta.DomainCategory, + Onboarding: apiOnboarding, } } diff --git a/management/server/http/handlers/accounts/accounts_handler_test.go b/management/server/http/handlers/accounts/accounts_handler_test.go index a18798743..dbf0c22bc 100644 --- a/management/server/http/handlers/accounts/accounts_handler_test.go +++ b/management/server/http/handlers/accounts/accounts_handler_test.go @@ -54,6 +54,18 @@ func initAccountsTestData(t *testing.T, account *types.Account) *handler { GetAccountMetaFunc: func(ctx context.Context, accountID string, userID string) (*types.AccountMeta, error) { return account.GetMeta(), nil }, + GetAccountOnboardingFunc: func(ctx context.Context, accountID string, userID string) (*types.AccountOnboarding, error) { + return &types.AccountOnboarding{ + OnboardingFlowPending: true, + SignupFormPending: true, + }, nil + }, + UpdateAccountOnboardingFunc: func(ctx context.Context, accountID, userID string, onboarding *types.AccountOnboarding) (*types.AccountOnboarding, error) { + return &types.AccountOnboarding{ + OnboardingFlowPending: true, + SignupFormPending: true, + }, nil + }, }, settingsManager: settingsMockManager, } @@ -117,7 +129,7 @@ func TestAccounts_AccountsHandler(t *testing.T) { expectedBody: true, requestType: http.MethodPut, requestPath: "/api/accounts/" + accountID, - requestBody: bytes.NewBufferString("{\"settings\": {\"peer_login_expiration\": 15552000,\"peer_login_expiration_enabled\": true}}"), + requestBody: bytes.NewBufferString("{\"settings\": {\"peer_login_expiration\": 15552000,\"peer_login_expiration_enabled\": true},\"onboarding\": {\"onboarding_flow_pending\": true,\"signup_form_pending\": true}}"), expectedStatus: http.StatusOK, expectedSettings: api.AccountSettings{ PeerLoginExpiration: 15552000, @@ -139,7 +151,7 @@ func TestAccounts_AccountsHandler(t *testing.T) { expectedBody: true, requestType: http.MethodPut, requestPath: "/api/accounts/" + accountID, - requestBody: bytes.NewBufferString("{\"settings\": {\"peer_login_expiration\": 15552000,\"peer_login_expiration_enabled\": false,\"jwt_groups_enabled\":true,\"jwt_groups_claim_name\":\"roles\",\"jwt_allow_groups\":[\"test\"],\"regular_users_view_blocked\":true}}"), + requestBody: bytes.NewBufferString("{\"settings\": {\"peer_login_expiration\": 15552000,\"peer_login_expiration_enabled\": false,\"jwt_groups_enabled\":true,\"jwt_groups_claim_name\":\"roles\",\"jwt_allow_groups\":[\"test\"],\"regular_users_view_blocked\":true},\"onboarding\": {\"onboarding_flow_pending\": true,\"signup_form_pending\": true}}"), expectedStatus: http.StatusOK, expectedSettings: api.AccountSettings{ PeerLoginExpiration: 15552000, @@ -161,7 +173,7 @@ func TestAccounts_AccountsHandler(t *testing.T) { expectedBody: true, requestType: http.MethodPut, requestPath: "/api/accounts/" + accountID, - requestBody: bytes.NewBufferString("{\"settings\": {\"peer_login_expiration\": 554400,\"peer_login_expiration_enabled\": true,\"jwt_groups_enabled\":true,\"jwt_groups_claim_name\":\"groups\",\"groups_propagation_enabled\":true,\"regular_users_view_blocked\":true}}"), + requestBody: bytes.NewBufferString("{\"settings\": {\"peer_login_expiration\": 554400,\"peer_login_expiration_enabled\": true,\"jwt_groups_enabled\":true,\"jwt_groups_claim_name\":\"groups\",\"groups_propagation_enabled\":true,\"regular_users_view_blocked\":true},\"onboarding\": {\"onboarding_flow_pending\": true,\"signup_form_pending\": true}}"), expectedStatus: http.StatusOK, expectedSettings: api.AccountSettings{ PeerLoginExpiration: 554400, @@ -178,12 +190,34 @@ func TestAccounts_AccountsHandler(t *testing.T) { expectedArray: false, expectedID: accountID, }, + { + name: "PutAccount OK without onboarding", + expectedBody: true, + requestType: http.MethodPut, + requestPath: "/api/accounts/" + accountID, + requestBody: bytes.NewBufferString("{\"settings\": {\"peer_login_expiration\": 15552000,\"peer_login_expiration_enabled\": false,\"jwt_groups_enabled\":true,\"jwt_groups_claim_name\":\"roles\",\"jwt_allow_groups\":[\"test\"],\"regular_users_view_blocked\":true}}"), + expectedStatus: http.StatusOK, + expectedSettings: api.AccountSettings{ + PeerLoginExpiration: 15552000, + PeerLoginExpirationEnabled: false, + GroupsPropagationEnabled: br(false), + JwtGroupsClaimName: sr("roles"), + JwtGroupsEnabled: br(true), + JwtAllowGroups: &[]string{"test"}, + RegularUsersViewBlocked: true, + RoutingPeerDnsResolutionEnabled: br(false), + LazyConnectionEnabled: br(false), + DnsDomain: sr(""), + }, + expectedArray: false, + expectedID: accountID, + }, { name: "Update account failure with high peer_login_expiration more than 180 days", expectedBody: true, requestType: http.MethodPut, requestPath: "/api/accounts/" + accountID, - requestBody: bytes.NewBufferString("{\"settings\": {\"peer_login_expiration\": 15552001,\"peer_login_expiration_enabled\": true}}"), + requestBody: bytes.NewBufferString("{\"settings\": {\"peer_login_expiration\": 15552001,\"peer_login_expiration_enabled\": true},\"onboarding\": {\"onboarding_flow_pending\": true,\"signup_form_pending\": true}}"), expectedStatus: http.StatusUnprocessableEntity, expectedArray: false, }, @@ -192,7 +226,7 @@ func TestAccounts_AccountsHandler(t *testing.T) { expectedBody: true, requestType: http.MethodPut, requestPath: "/api/accounts/" + accountID, - requestBody: bytes.NewBufferString("{\"settings\": {\"peer_login_expiration\": 3599,\"peer_login_expiration_enabled\": true}}"), + requestBody: bytes.NewBufferString("{\"settings\": {\"peer_login_expiration\": 3599,\"peer_login_expiration_enabled\": true},\"onboarding\": {\"onboarding_flow_pending\": true,\"signup_form_pending\": true}}"), expectedStatus: http.StatusUnprocessableEntity, expectedArray: false, }, diff --git a/management/server/mock_server/account_mock.go b/management/server/mock_server/account_mock.go index 3caa6744a..8837f9f50 100644 --- a/management/server/mock_server/account_mock.go +++ b/management/server/mock_server/account_mock.go @@ -30,94 +30,95 @@ type MockAccountManager struct { GetAccountFunc func(ctx context.Context, accountID string) (*types.Account, error) CreateSetupKeyFunc func(ctx context.Context, accountId string, keyName string, keyType types.SetupKeyType, expiresIn time.Duration, autoGroups []string, usageLimit int, userID string, ephemeral bool, allowExtraDNSLabels bool) (*types.SetupKey, error) - GetSetupKeyFunc func(ctx context.Context, accountID, userID, keyID string) (*types.SetupKey, error) - AccountExistsFunc func(ctx context.Context, accountID string) (bool, error) - GetAccountIDByUserIdFunc func(ctx context.Context, userId, domain string) (string, error) - GetUserFromUserAuthFunc func(ctx context.Context, userAuth nbcontext.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, connected bool, realIP net.IP) error - SyncAndMarkPeerFunc func(ctx context.Context, accountID string, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, error) - DeletePeerFunc func(ctx context.Context, accountID, peerKey, userID string) error - GetNetworkMapFunc func(ctx context.Context, peerKey string) (*types.NetworkMap, error) - GetPeerNetworkFunc func(ctx context.Context, peerKey string) (*types.Network, error) - AddPeerFunc func(ctx context.Context, setupKey string, userId string, peer *nbpeer.Peer) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, error) - GetGroupFunc func(ctx context.Context, accountID, groupID, userID string) (*types.Group, error) - GetAllGroupsFunc func(ctx context.Context, accountID, userID string) ([]*types.Group, error) - GetGroupByNameFunc func(ctx context.Context, accountID, groupName string) (*types.Group, error) - SaveGroupFunc func(ctx context.Context, accountID, userID string, group *types.Group, create bool) error - SaveGroupsFunc func(ctx context.Context, accountID, userID string, groups []*types.Group, create bool) error - DeleteGroupFunc func(ctx context.Context, accountID, userId, groupID string) error - DeleteGroupsFunc func(ctx context.Context, accountId, userId string, groupIDs []string) error - GroupAddPeerFunc func(ctx context.Context, accountID, groupID, peerID string) error - GroupDeletePeerFunc func(ctx context.Context, accountID, groupID, peerID string) error - GetPeerGroupsFunc func(ctx context.Context, accountID, peerID string) ([]*types.Group, error) - DeleteRuleFunc func(ctx context.Context, accountID, ruleID, userID string) error - GetPolicyFunc func(ctx context.Context, accountID, policyID, userID string) (*types.Policy, error) - SavePolicyFunc func(ctx context.Context, accountID, userID string, policy *types.Policy, create bool) (*types.Policy, error) - DeletePolicyFunc func(ctx context.Context, accountID, policyID, userID string) error - ListPoliciesFunc func(ctx context.Context, accountID, userID string) ([]*types.Policy, error) - GetUsersFromAccountFunc func(ctx context.Context, accountID, userID string) (map[string]*types.UserInfo, error) - UpdatePeerMetaFunc func(ctx context.Context, peerID string, meta nbpeer.PeerSystemMeta) error - UpdatePeerFunc func(ctx context.Context, accountID, userID string, peer *nbpeer.Peer) (*nbpeer.Peer, error) - CreateRouteFunc func(ctx context.Context, accountID string, prefix netip.Prefix, networkType route.NetworkType, domains domain.List, peer string, peerGroups []string, description string, netID route.NetID, masquerade bool, metric int, groups, accessControlGroupIDs []string, enabled bool, userID string, keepRoute bool) (*route.Route, error) - GetRouteFunc func(ctx context.Context, accountID string, routeID route.ID, userID string) (*route.Route, error) - SaveRouteFunc func(ctx context.Context, accountID string, userID string, route *route.Route) error - DeleteRouteFunc func(ctx context.Context, accountID string, routeID route.ID, userID string) error - ListRoutesFunc func(ctx context.Context, accountID, userID string) ([]*route.Route, error) - SaveSetupKeyFunc func(ctx context.Context, accountID string, key *types.SetupKey, userID string) (*types.SetupKey, error) - ListSetupKeysFunc func(ctx context.Context, accountID, userID string) ([]*types.SetupKey, error) - SaveUserFunc func(ctx context.Context, accountID, userID string, user *types.User) (*types.UserInfo, error) - SaveOrAddUserFunc func(ctx context.Context, accountID, userID string, user *types.User, addIfNotExists bool) (*types.UserInfo, error) - SaveOrAddUsersFunc func(ctx context.Context, accountID, initiatorUserID string, update []*types.User, addIfNotExists bool) ([]*types.UserInfo, error) - DeleteUserFunc func(ctx context.Context, accountID string, initiatorUserID string, targetUserID string) error - DeleteRegularUsersFunc func(ctx context.Context, accountID, initiatorUserID string, targetUserIDs []string, userInfos map[string]*types.UserInfo) error - CreatePATFunc func(ctx context.Context, accountID string, initiatorUserID string, targetUserId string, tokenName string, expiresIn int) (*types.PersonalAccessTokenGenerated, error) - DeletePATFunc func(ctx context.Context, accountID string, initiatorUserID string, targetUserId string, tokenID string) error - GetPATFunc func(ctx context.Context, accountID string, initiatorUserID string, targetUserId string, tokenID string) (*types.PersonalAccessToken, error) - GetAllPATsFunc func(ctx context.Context, accountID string, initiatorUserID string, targetUserId string) ([]*types.PersonalAccessToken, error) - GetNameServerGroupFunc func(ctx context.Context, accountID, userID, nsGroupID string) (*nbdns.NameServerGroup, error) - CreateNameServerGroupFunc func(ctx context.Context, accountID string, name, description string, nameServerList []nbdns.NameServer, groups []string, primary bool, domains []string, enabled bool, userID string, searchDomainsEnabled bool) (*nbdns.NameServerGroup, error) - SaveNameServerGroupFunc func(ctx context.Context, accountID, userID string, nsGroupToSave *nbdns.NameServerGroup) error - DeleteNameServerGroupFunc func(ctx context.Context, accountID, nsGroupID, userID string) error - ListNameServerGroupsFunc func(ctx context.Context, accountID string, userID string) ([]*nbdns.NameServerGroup, error) - CreateUserFunc func(ctx context.Context, accountID, userID string, key *types.UserInfo) (*types.UserInfo, error) - GetAccountIDFromUserAuthFunc func(ctx context.Context, userAuth nbcontext.UserAuth) (string, string, error) - DeleteAccountFunc func(ctx context.Context, accountID, userID string) error - GetDNSDomainFunc func(settings *types.Settings) string - StoreEventFunc func(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) - GetEventsFunc func(ctx context.Context, accountID, userID string) ([]*activity.Event, error) - GetDNSSettingsFunc func(ctx context.Context, accountID, userID string) (*types.DNSSettings, error) - SaveDNSSettingsFunc func(ctx context.Context, accountID, userID string, dnsSettingsToSave *types.DNSSettings) error - GetPeerFunc func(ctx context.Context, accountID, peerID, userID string) (*nbpeer.Peer, error) - UpdateAccountSettingsFunc func(ctx context.Context, accountID, userID string, newSettings *types.Settings) (*types.Settings, error) - LoginPeerFunc func(ctx context.Context, login types.PeerLogin) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, error) - SyncPeerFunc func(ctx context.Context, sync types.PeerSync, accountID string) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, error) - InviteUserFunc func(ctx context.Context, accountID string, initiatorUserID string, targetUserEmail string) error - GetAllConnectedPeersFunc func() (map[string]struct{}, error) - HasConnectedChannelFunc func(peerID string) bool - GetExternalCacheManagerFunc func() account.ExternalCacheManager - GetPostureChecksFunc func(ctx context.Context, accountID, postureChecksID, userID string) (*posture.Checks, error) - SavePostureChecksFunc func(ctx context.Context, accountID, userID string, postureChecks *posture.Checks, create bool) (*posture.Checks, error) - DeletePostureChecksFunc func(ctx context.Context, accountID, postureChecksID, userID string) error - ListPostureChecksFunc func(ctx context.Context, accountID, userID string) ([]*posture.Checks, error) - GetIdpManagerFunc func() idp.Manager - UpdateIntegratedValidatorGroupsFunc func(ctx context.Context, accountID string, userID 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 - 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) - GetUserByIDFunc func(ctx context.Context, id string) (*types.User, error) - GetAccountSettingsFunc func(ctx context.Context, accountID string, userID string) (*types.Settings, error) - DeleteSetupKeyFunc func(ctx context.Context, accountID, userID, keyID string) error - BuildUserInfosForAccountFunc func(ctx context.Context, accountID, initiatorUserID string, accountUsers []*types.User) (map[string]*types.UserInfo, error) - GetStoreFunc func() store.Store - UpdateToPrimaryAccountFunc func(ctx context.Context, accountId string) (*types.Account, error) - GetOwnerInfoFunc func(ctx context.Context, accountID string) (*types.UserInfo, error) - GetCurrentUserInfoFunc func(ctx context.Context, userAuth nbcontext.UserAuth) (*users.UserInfoWithPermissions, error) - GetAccountMetaFunc func(ctx context.Context, accountID, userID string) (*types.AccountMeta, error) - + GetSetupKeyFunc func(ctx context.Context, accountID, userID, keyID string) (*types.SetupKey, error) + AccountExistsFunc func(ctx context.Context, accountID string) (bool, error) + GetAccountIDByUserIdFunc func(ctx context.Context, userId, domain string) (string, error) + GetUserFromUserAuthFunc func(ctx context.Context, userAuth nbcontext.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, connected bool, realIP net.IP) error + SyncAndMarkPeerFunc func(ctx context.Context, accountID string, peerPubKey string, meta nbpeer.PeerSystemMeta, realIP net.IP) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, error) + DeletePeerFunc func(ctx context.Context, accountID, peerKey, userID string) error + GetNetworkMapFunc func(ctx context.Context, peerKey string) (*types.NetworkMap, error) + GetPeerNetworkFunc func(ctx context.Context, peerKey string) (*types.Network, error) + AddPeerFunc func(ctx context.Context, setupKey string, userId string, peer *nbpeer.Peer) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, error) + GetGroupFunc func(ctx context.Context, accountID, groupID, userID string) (*types.Group, error) + GetAllGroupsFunc func(ctx context.Context, accountID, userID string) ([]*types.Group, error) + GetGroupByNameFunc func(ctx context.Context, accountID, groupName string) (*types.Group, error) + SaveGroupFunc func(ctx context.Context, accountID, userID string, group *types.Group, create bool) error + SaveGroupsFunc func(ctx context.Context, accountID, userID string, groups []*types.Group, create bool) error + DeleteGroupFunc func(ctx context.Context, accountID, userId, groupID string) error + DeleteGroupsFunc func(ctx context.Context, accountId, userId string, groupIDs []string) error + GroupAddPeerFunc func(ctx context.Context, accountID, groupID, peerID string) error + GroupDeletePeerFunc func(ctx context.Context, accountID, groupID, peerID string) error + GetPeerGroupsFunc func(ctx context.Context, accountID, peerID string) ([]*types.Group, error) + DeleteRuleFunc func(ctx context.Context, accountID, ruleID, userID string) error + GetPolicyFunc func(ctx context.Context, accountID, policyID, userID string) (*types.Policy, error) + SavePolicyFunc func(ctx context.Context, accountID, userID string, policy *types.Policy, create bool) (*types.Policy, error) + DeletePolicyFunc func(ctx context.Context, accountID, policyID, userID string) error + ListPoliciesFunc func(ctx context.Context, accountID, userID string) ([]*types.Policy, error) + GetUsersFromAccountFunc func(ctx context.Context, accountID, userID string) (map[string]*types.UserInfo, error) + UpdatePeerMetaFunc func(ctx context.Context, peerID string, meta nbpeer.PeerSystemMeta) error + UpdatePeerFunc func(ctx context.Context, accountID, userID string, peer *nbpeer.Peer) (*nbpeer.Peer, error) + CreateRouteFunc func(ctx context.Context, accountID string, prefix netip.Prefix, networkType route.NetworkType, domains domain.List, peer string, peerGroups []string, description string, netID route.NetID, masquerade bool, metric int, groups, accessControlGroupIDs []string, enabled bool, userID string, keepRoute bool) (*route.Route, error) + GetRouteFunc func(ctx context.Context, accountID string, routeID route.ID, userID string) (*route.Route, error) + SaveRouteFunc func(ctx context.Context, accountID string, userID string, route *route.Route) error + DeleteRouteFunc func(ctx context.Context, accountID string, routeID route.ID, userID string) error + ListRoutesFunc func(ctx context.Context, accountID, userID string) ([]*route.Route, error) + SaveSetupKeyFunc func(ctx context.Context, accountID string, key *types.SetupKey, userID string) (*types.SetupKey, error) + ListSetupKeysFunc func(ctx context.Context, accountID, userID string) ([]*types.SetupKey, error) + SaveUserFunc func(ctx context.Context, accountID, userID string, user *types.User) (*types.UserInfo, error) + SaveOrAddUserFunc func(ctx context.Context, accountID, userID string, user *types.User, addIfNotExists bool) (*types.UserInfo, error) + SaveOrAddUsersFunc func(ctx context.Context, accountID, initiatorUserID string, update []*types.User, addIfNotExists bool) ([]*types.UserInfo, error) + DeleteUserFunc func(ctx context.Context, accountID string, initiatorUserID string, targetUserID string) error + DeleteRegularUsersFunc func(ctx context.Context, accountID, initiatorUserID string, targetUserIDs []string, userInfos map[string]*types.UserInfo) error + CreatePATFunc func(ctx context.Context, accountID string, initiatorUserID string, targetUserId string, tokenName string, expiresIn int) (*types.PersonalAccessTokenGenerated, error) + DeletePATFunc func(ctx context.Context, accountID string, initiatorUserID string, targetUserId string, tokenID string) error + GetPATFunc func(ctx context.Context, accountID string, initiatorUserID string, targetUserId string, tokenID string) (*types.PersonalAccessToken, error) + GetAllPATsFunc func(ctx context.Context, accountID string, initiatorUserID string, targetUserId string) ([]*types.PersonalAccessToken, error) + GetNameServerGroupFunc func(ctx context.Context, accountID, userID, nsGroupID string) (*nbdns.NameServerGroup, error) + CreateNameServerGroupFunc func(ctx context.Context, accountID string, name, description string, nameServerList []nbdns.NameServer, groups []string, primary bool, domains []string, enabled bool, userID string, searchDomainsEnabled bool) (*nbdns.NameServerGroup, error) + SaveNameServerGroupFunc func(ctx context.Context, accountID, userID string, nsGroupToSave *nbdns.NameServerGroup) error + DeleteNameServerGroupFunc func(ctx context.Context, accountID, nsGroupID, userID string) error + ListNameServerGroupsFunc func(ctx context.Context, accountID string, userID string) ([]*nbdns.NameServerGroup, error) + CreateUserFunc func(ctx context.Context, accountID, userID string, key *types.UserInfo) (*types.UserInfo, error) + GetAccountIDFromUserAuthFunc func(ctx context.Context, userAuth nbcontext.UserAuth) (string, string, error) + DeleteAccountFunc func(ctx context.Context, accountID, userID string) error + GetDNSDomainFunc func(settings *types.Settings) string + StoreEventFunc func(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) + GetEventsFunc func(ctx context.Context, accountID, userID string) ([]*activity.Event, error) + GetDNSSettingsFunc func(ctx context.Context, accountID, userID string) (*types.DNSSettings, error) + SaveDNSSettingsFunc func(ctx context.Context, accountID, userID string, dnsSettingsToSave *types.DNSSettings) error + GetPeerFunc func(ctx context.Context, accountID, peerID, userID string) (*nbpeer.Peer, error) + UpdateAccountSettingsFunc func(ctx context.Context, accountID, userID string, newSettings *types.Settings) (*types.Settings, error) + LoginPeerFunc func(ctx context.Context, login types.PeerLogin) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, error) + SyncPeerFunc func(ctx context.Context, sync types.PeerSync, accountID string) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, error) + InviteUserFunc func(ctx context.Context, accountID string, initiatorUserID string, targetUserEmail string) error + GetAllConnectedPeersFunc func() (map[string]struct{}, error) + HasConnectedChannelFunc func(peerID string) bool + GetExternalCacheManagerFunc func() account.ExternalCacheManager + GetPostureChecksFunc func(ctx context.Context, accountID, postureChecksID, userID string) (*posture.Checks, error) + SavePostureChecksFunc func(ctx context.Context, accountID, userID string, postureChecks *posture.Checks, create bool) (*posture.Checks, error) + DeletePostureChecksFunc func(ctx context.Context, accountID, postureChecksID, userID string) error + ListPostureChecksFunc func(ctx context.Context, accountID, userID string) ([]*posture.Checks, error) + GetIdpManagerFunc func() idp.Manager + UpdateIntegratedValidatorGroupsFunc func(ctx context.Context, accountID string, userID 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 + 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) + GetUserByIDFunc func(ctx context.Context, id string) (*types.User, error) + GetAccountSettingsFunc func(ctx context.Context, accountID string, userID string) (*types.Settings, error) + DeleteSetupKeyFunc func(ctx context.Context, accountID, userID, keyID string) error + BuildUserInfosForAccountFunc func(ctx context.Context, accountID, initiatorUserID string, accountUsers []*types.User) (map[string]*types.UserInfo, error) + GetStoreFunc func() store.Store + UpdateToPrimaryAccountFunc func(ctx context.Context, accountId string) (*types.Account, error) + GetOwnerInfoFunc func(ctx context.Context, accountID string) (*types.UserInfo, error) + GetCurrentUserInfoFunc func(ctx context.Context, userAuth nbcontext.UserAuth) (*users.UserInfoWithPermissions, error) + GetAccountMetaFunc func(ctx context.Context, accountID, userID string) (*types.AccountMeta, error) + GetAccountOnboardingFunc func(ctx context.Context, accountID, userID string) (*types.AccountOnboarding, error) + UpdateAccountOnboardingFunc func(ctx context.Context, accountID, userID string, onboarding *types.AccountOnboarding) (*types.AccountOnboarding, error) GetOrCreateAccountByPrivateDomainFunc func(ctx context.Context, initiatorId, domain string) (*types.Account, bool, error) } @@ -814,6 +815,22 @@ func (am *MockAccountManager) GetAccountMeta(ctx context.Context, accountID stri return nil, status.Errorf(codes.Unimplemented, "method GetAccountMeta is not implemented") } +// GetAccountOnboarding mocks GetAccountOnboarding of the AccountManager interface +func (am *MockAccountManager) GetAccountOnboarding(ctx context.Context, accountID string, userID string) (*types.AccountOnboarding, error) { + if am.GetAccountOnboardingFunc != nil { + return am.GetAccountOnboardingFunc(ctx, accountID, userID) + } + return nil, status.Errorf(codes.Unimplemented, "method GetAccountOnboarding is not implemented") +} + +// UpdateAccountOnboarding mocks UpdateAccountOnboarding of the AccountManager interface +func (am *MockAccountManager) UpdateAccountOnboarding(ctx context.Context, accountID string, userID string, onboarding *types.AccountOnboarding) (*types.AccountOnboarding, error) { + if am.UpdateAccountOnboardingFunc != nil { + return am.UpdateAccountOnboardingFunc(ctx, accountID, userID, onboarding) + } + return nil, status.Errorf(codes.Unimplemented, "method UpdateAccountOnboarding is not implemented") +} + // GetUserByID mocks GetUserByID of the AccountManager interface func (am *MockAccountManager) GetUserByID(ctx context.Context, id string) (*types.User, error) { if am.GetUserByIDFunc != nil { diff --git a/management/server/status/error.go b/management/server/status/error.go index 5a6f6d1a7..47c236e93 100644 --- a/management/server/status/error.go +++ b/management/server/status/error.go @@ -90,6 +90,11 @@ func NewAccountNotFoundError(accountKey string) error { return Errorf(NotFound, "account not found: %s", accountKey) } +// NewAccountOnboardingNotFoundError creates a new Error with NotFound type for a missing account onboarding +func NewAccountOnboardingNotFoundError(accountKey string) error { + return Errorf(NotFound, "account onboarding not found: %s", accountKey) +} + // NewPeerNotPartOfAccountError creates a new Error with PermissionDenied type for a peer not being part of an account func NewPeerNotPartOfAccountError() error { return Errorf(PermissionDenied, "peer is not part of this account") diff --git a/management/server/store/sql_store.go b/management/server/store/sql_store.go index 197255ab6..baee4ad28 100644 --- a/management/server/store/sql_store.go +++ b/management/server/store/sql_store.go @@ -99,7 +99,7 @@ func NewSqlStore(ctx context.Context, db *gorm.DB, storeEngine types.Engine, met &types.SetupKey{}, &nbpeer.Peer{}, &types.User{}, &types.PersonalAccessToken{}, &types.Group{}, &types.Account{}, &types.Policy{}, &types.PolicyRule{}, &route.Route{}, &nbdns.NameServerGroup{}, &installation{}, &types.ExtraSettings{}, &posture.Checks{}, &nbpeer.NetworkAddress{}, - &networkTypes.Network{}, &routerTypes.NetworkRouter{}, &resourceTypes.NetworkResource{}, + &networkTypes.Network{}, &routerTypes.NetworkRouter{}, &resourceTypes.NetworkResource{}, &types.AccountOnboarding{}, ) if err != nil { return nil, fmt.Errorf("auto migratePreAuto: %w", err) @@ -728,6 +728,32 @@ func (s *SqlStore) GetAccountMeta(ctx context.Context, lockStrength LockingStren return &accountMeta, nil } +// GetAccountOnboarding retrieves the onboarding information for a specific account. +func (s *SqlStore) GetAccountOnboarding(ctx context.Context, accountID string) (*types.AccountOnboarding, error) { + var accountOnboarding types.AccountOnboarding + result := s.db.Model(&accountOnboarding).First(&accountOnboarding, accountIDCondition, accountID) + if result.Error != nil { + if errors.Is(result.Error, gorm.ErrRecordNotFound) { + return nil, status.NewAccountOnboardingNotFoundError(accountID) + } + log.WithContext(ctx).Errorf("error when getting account onboarding %s from the store: %s", accountID, result.Error) + return nil, status.NewGetAccountFromStoreError(result.Error) + } + + return &accountOnboarding, nil +} + +// SaveAccountOnboarding updates the onboarding information for a specific account. +func (s *SqlStore) SaveAccountOnboarding(ctx context.Context, onboarding *types.AccountOnboarding) error { + result := s.db.Clauses(clause.OnConflict{UpdateAll: true}).Create(onboarding) + if result.Error != nil { + log.WithContext(ctx).Errorf("error when saving account onboarding %s in the store: %s", onboarding.AccountID, result.Error) + return status.Errorf(status.Internal, "error when saving account onboarding %s in the store: %s", onboarding.AccountID, result.Error) + } + + return nil +} + func (s *SqlStore) GetAccount(ctx context.Context, accountID string) (*types.Account, error) { start := time.Now() defer func() { diff --git a/management/server/store/sql_store_test.go b/management/server/store/sql_store_test.go index 928486ab4..738c5a28c 100644 --- a/management/server/store/sql_store_test.go +++ b/management/server/store/sql_store_test.go @@ -354,9 +354,16 @@ func TestSqlite_DeleteAccount(t *testing.T) { t.Errorf("expecting 1 Accounts to be stored after SaveAccount()") } + o, err := store.GetAccountOnboarding(context.Background(), account.Id) + require.NoError(t, err) + require.Equal(t, o.AccountID, account.Id) + err = store.DeleteAccount(context.Background(), account) require.NoError(t, err) + _, err = store.GetAccountOnboarding(context.Background(), account.Id) + require.Error(t, err, "expecting error after removing DeleteAccount when getting onboarding") + if len(store.GetAllAccounts(context.Background())) != 0 { t.Errorf("expecting 0 Accounts to be stored after DeleteAccount()") } @@ -414,12 +421,21 @@ func Test_GetAccount(t *testing.T) { account, err := store.GetAccount(context.Background(), id) require.NoError(t, err) require.Equal(t, id, account.Id, "account id should match") + require.Equal(t, false, account.Onboarding.OnboardingFlowPending) + + id = "9439-34653001fc3b-bf1c8084-ba50-4ce7" + + account, err = store.GetAccount(context.Background(), id) + require.NoError(t, err) + require.Equal(t, id, account.Id, "account id should match") + require.Equal(t, true, account.Onboarding.OnboardingFlowPending) _, err = store.GetAccount(context.Background(), "non-existing-account") 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") + }) } @@ -2096,6 +2112,7 @@ func newAccountWithId(ctx context.Context, accountID, userID, domain string) *ty PeerInactivityExpirationEnabled: false, PeerInactivityExpiration: types.DefaultPeerInactivityExpiration, }, + Onboarding: types.AccountOnboarding{SignupFormPending: true, OnboardingFlowPending: true}, } if err := acc.AddAllGroup(false); err != nil { @@ -3440,6 +3457,63 @@ func TestSqlStore_GetAccountMeta(t *testing.T) { require.Equal(t, time.Date(2024, time.October, 2, 14, 1, 38, 210000000, time.UTC), accountMeta.CreatedAt.UTC()) } +func TestSqlStore_GetAccountOnboarding(t *testing.T) { + store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/store.sql", t.TempDir()) + t.Cleanup(cleanup) + require.NoError(t, err) + + accountID := "9439-34653001fc3b-bf1c8084-ba50-4ce7" + a, err := store.GetAccount(context.Background(), accountID) + require.NoError(t, err) + t.Logf("Onboarding: %+v", a.Onboarding) + err = store.SaveAccount(context.Background(), a) + require.NoError(t, err) + onboarding, err := store.GetAccountOnboarding(context.Background(), accountID) + require.NoError(t, err) + require.NotNil(t, onboarding) + require.Equal(t, accountID, onboarding.AccountID) + require.Equal(t, time.Date(2024, time.October, 2, 14, 1, 38, 210000000, time.UTC), onboarding.CreatedAt.UTC()) +} + +func TestSqlStore_SaveAccountOnboarding(t *testing.T) { + store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/store.sql", t.TempDir()) + t.Cleanup(cleanup) + require.NoError(t, err) + t.Run("New onboarding should be saved correctly", func(t *testing.T) { + accountID := "bf1c8084-ba50-4ce7-9439-34653001fc3b" + onboarding := &types.AccountOnboarding{ + AccountID: accountID, + SignupFormPending: true, + OnboardingFlowPending: true, + } + + err = store.SaveAccountOnboarding(context.Background(), onboarding) + require.NoError(t, err) + + savedOnboarding, err := store.GetAccountOnboarding(context.Background(), accountID) + require.NoError(t, err) + require.Equal(t, onboarding.SignupFormPending, savedOnboarding.SignupFormPending) + require.Equal(t, onboarding.OnboardingFlowPending, savedOnboarding.OnboardingFlowPending) + }) + + t.Run("Existing onboarding should be updated correctly", func(t *testing.T) { + accountID := "9439-34653001fc3b-bf1c8084-ba50-4ce7" + onboarding, err := store.GetAccountOnboarding(context.Background(), accountID) + require.NoError(t, err) + + onboarding.OnboardingFlowPending = !onboarding.OnboardingFlowPending + onboarding.SignupFormPending = !onboarding.SignupFormPending + + err = store.SaveAccountOnboarding(context.Background(), onboarding) + require.NoError(t, err) + + savedOnboarding, err := store.GetAccountOnboarding(context.Background(), accountID) + require.NoError(t, err) + require.Equal(t, onboarding.SignupFormPending, savedOnboarding.SignupFormPending) + require.Equal(t, onboarding.OnboardingFlowPending, savedOnboarding.OnboardingFlowPending) + }) +} + func TestSqlStore_GetAnyAccountID(t *testing.T) { t.Run("should return account ID when accounts exist", func(t *testing.T) { store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/extended-store.sql", t.TempDir()) diff --git a/management/server/store/store.go b/management/server/store/store.go index 30ff1549d..b3254c4c9 100644 --- a/management/server/store/store.go +++ b/management/server/store/store.go @@ -52,6 +52,7 @@ type Store interface { GetAllAccounts(ctx context.Context) []*types.Account GetAccount(ctx context.Context, accountID string) (*types.Account, error) GetAccountMeta(ctx context.Context, lockStrength LockingStrength, accountID string) (*types.AccountMeta, error) + GetAccountOnboarding(ctx context.Context, accountID string) (*types.AccountOnboarding, error) AccountExists(ctx context.Context, lockStrength LockingStrength, id string) (bool, error) GetAccountDomainAndCategory(ctx context.Context, lockStrength LockingStrength, accountID string) (string, string, error) GetAccountByUser(ctx context.Context, userID string) (*types.Account, error) @@ -74,6 +75,7 @@ type Store interface { SaveDNSSettings(ctx context.Context, lockStrength LockingStrength, accountID string, settings *types.DNSSettings) error SaveAccountSettings(ctx context.Context, lockStrength LockingStrength, accountID string, settings *types.Settings) error CountAccountsByPrivateDomain(ctx context.Context, domain string) (int64, error) + SaveAccountOnboarding(ctx context.Context, onboarding *types.AccountOnboarding) error GetUserByPATID(ctx context.Context, lockStrength LockingStrength, patID string) (*types.User, error) GetUserByUserID(ctx context.Context, lockStrength LockingStrength, userID string) (*types.User, error) diff --git a/management/server/testdata/store.sql b/management/server/testdata/store.sql index 4b126c618..a21783857 100644 --- a/management/server/testdata/store.sql +++ b/management/server/testdata/store.sql @@ -1,4 +1,5 @@ CREATE TABLE `accounts` (`id` text,`created_by` text,`created_at` datetime,`domain` text,`domain_category` text,`is_domain_primary_account` numeric,`network_identifier` text,`network_net` text,`network_dns` text,`network_serial` integer,`dns_settings_disabled_management_groups` text,`settings_peer_login_expiration_enabled` numeric,`settings_peer_login_expiration` integer,`settings_regular_users_view_blocked` numeric,`settings_groups_propagation_enabled` numeric,`settings_jwt_groups_enabled` numeric,`settings_jwt_groups_claim_name` text,`settings_jwt_allow_groups` text,`settings_extra_peer_approval_enabled` numeric,`settings_extra_integrated_validator_groups` text,PRIMARY KEY (`id`)); +CREATE TABLE `account_onboardings` (`account_id` text, `created_at` datetime,`updated_at` datetime, `onboarding_flow_pending` numeric, `signup_form_pending` numeric, PRIMARY KEY (`account_id`)); CREATE TABLE `setup_keys` (`id` text,`account_id` text,`key` text,`name` text,`type` text,`created_at` datetime,`expires_at` datetime,`updated_at` datetime,`revoked` numeric,`used_times` integer,`last_used` datetime DEFAULT NULL,`auto_groups` text,`usage_limit` integer,`ephemeral` numeric,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_setup_keys_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); CREATE TABLE `peers` (`id` text,`account_id` text,`key` text,`setup_key` text,`ip` text,`meta_hostname` text,`meta_go_os` text,`meta_kernel` text,`meta_core` text,`meta_platform` text,`meta_os` text,`meta_os_version` text,`meta_wt_version` text,`meta_ui_version` text,`meta_kernel_version` text,`meta_network_addresses` text,`meta_system_serial_number` text,`meta_system_product_name` text,`meta_system_manufacturer` text,`meta_environment` text,`meta_files` text,`name` text,`dns_label` text,`peer_status_last_seen` datetime,`peer_status_connected` numeric,`peer_status_login_expired` numeric,`peer_status_requires_approval` numeric,`user_id` text,`ssh_key` text,`ssh_enabled` numeric,`login_expiration_enabled` numeric,`last_login` datetime,`created_at` datetime,`ephemeral` numeric,`location_connection_ip` text,`location_country_code` text,`location_city_name` text,`location_geo_name_id` integer,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_peers_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); CREATE TABLE `users` (`id` text,`account_id` text,`role` text,`is_service_user` numeric,`non_deletable` numeric,`service_user_name` text,`auto_groups` text,`blocked` numeric,`last_login` datetime,`created_at` datetime,`issued` text DEFAULT "api",`integration_ref_id` integer,`integration_ref_integration_type` text,PRIMARY KEY (`id`),CONSTRAINT `fk_accounts_users_g` FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`)); @@ -38,7 +39,8 @@ CREATE INDEX `idx_networks_id` ON `networks`(`id`); CREATE INDEX `idx_networks_account_id` ON `networks`(`account_id`); INSERT INTO accounts VALUES('bf1c8084-ba50-4ce7-9439-34653001fc3b','edafee4e-63fb-11ec-90d6-0242ac120003','2024-10-02 16:03:06.778746+02:00','test.com','private',1,'af1c8024-ha40-4ce2-9418-34653101fc3c','{"IP":"100.64.0.0","Mask":"//8AAA=="}','',0,'[]',0,86400000000000,0,0,0,'',NULL,NULL,NULL); -INSERT INTO "groups" VALUES('cs1tnh0hhcjnqoiuebeg','bf1c8084-ba50-4ce7-9439-34653001fc3b','All','api','[]',0,''); +INSERT INTO accounts VALUES('9439-34653001fc3b-bf1c8084-ba50-4ce7','90d6-0242ac120003-edafee4e-63fb-11ec','2024-10-02 16:01:38.210000+02:00','test2.com','private',1,'af1c8024-ha40-4ce2-9418-34653101fc3c','{"IP":"100.64.0.0","Mask":"//8AAA=="}','',0,'[]',0,86400000000000,0,0,0,'',NULL,NULL,NULL); +INSERT INTO account_onboardings VALUES('9439-34653001fc3b-bf1c8084-ba50-4ce7','2024-10-02 16:01:38.210000+02:00','2021-08-19 20:46:20.005936822+02:00',1,0);INSERT INTO "groups" VALUES('cs1tnh0hhcjnqoiuebeg','bf1c8084-ba50-4ce7-9439-34653001fc3b','All','api','[]',0,''); INSERT INTO setup_keys VALUES('','bf1c8084-ba50-4ce7-9439-34653001fc3b','A2C8E62B-38F5-4553-B31E-DD66C696CEBB','Default key','reusable','2021-08-19 20:46:20.005936822+02:00','2321-09-18 20:46:20.005936822+02:00','2021-08-19 20:46:20.005936822+02:00',0,0,NULL,'["cs1tnh0hhcjnqoiuebeg"]',0,0); INSERT INTO users VALUES('a23efe53-63fb-11ec-90d6-0242ac120003','bf1c8084-ba50-4ce7-9439-34653001fc3b','owner',0,0,'','[]',0,NULL,'2024-10-02 16:03:06.779156+02:00','api',0,''); INSERT INTO users VALUES('edafee4e-63fb-11ec-90d6-0242ac120003','bf1c8084-ba50-4ce7-9439-34653001fc3b','admin',0,0,'','[]',0,NULL,'2024-10-02 16:03:06.779156+02:00','api',0,''); diff --git a/management/server/types/account.go b/management/server/types/account.go index 5a62ee4c6..f0887be07 100644 --- a/management/server/types/account.go +++ b/management/server/types/account.go @@ -82,11 +82,11 @@ type Account struct { DNSSettings DNSSettings `gorm:"embedded;embeddedPrefix:dns_settings_"` PostureChecks []*posture.Checks `gorm:"foreignKey:AccountID;references:id"` // Settings is a dictionary of Account settings - Settings *Settings `gorm:"embedded;embeddedPrefix:settings_"` - + Settings *Settings `gorm:"embedded;embeddedPrefix:settings_"` Networks []*networkTypes.Network `gorm:"foreignKey:AccountID;references:id"` NetworkRouters []*routerTypes.NetworkRouter `gorm:"foreignKey:AccountID;references:id"` NetworkResources []*resourceTypes.NetworkResource `gorm:"foreignKey:AccountID;references:id"` + Onboarding AccountOnboarding `gorm:"foreignKey:AccountID;references:id;constraint:OnDelete:CASCADE"` } // Subclass used in gorm to only load network and not whole account @@ -104,6 +104,20 @@ type AccountSettings struct { Settings *Settings `gorm:"embedded;embeddedPrefix:settings_"` } +type AccountOnboarding struct { + AccountID string `gorm:"primaryKey"` + OnboardingFlowPending bool + SignupFormPending bool + CreatedAt time.Time + UpdatedAt time.Time +} + +// IsEqual compares two AccountOnboarding objects and returns true if they are equal +func (o AccountOnboarding) IsEqual(onboarding AccountOnboarding) bool { + return o.OnboardingFlowPending == onboarding.OnboardingFlowPending && + o.SignupFormPending == onboarding.SignupFormPending +} + // GetRoutesToSync returns the enabled routes for the peer ID and the routes // from the ACL peers that have distribution groups associated with the peer ID. // Please mind, that the returned route.Route objects will contain Peer.Key instead of Peer.ID. @@ -866,6 +880,7 @@ func (a *Account) Copy() *Account { Networks: nets, NetworkRouters: networkRouters, NetworkResources: networkResources, + Onboarding: a.Onboarding, } } From 9afbecb7acf0af5876bfd4c47e95fba3e15d4289 Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Thu, 3 Jul 2025 09:02:53 +0200 Subject: [PATCH 05/10] [client] Use unique sequence numbers for bsd routes (#4081) updates the route manager on Unix to use a unique, incrementing sequence number for each route message instead of a fixed value. Replace the static Seq: 1 with a call to r.getSeq() Add an atomic seq field and the getSeq method in SysOps --- client/internal/routemanager/systemops/systemops.go | 9 +++++++++ client/internal/routemanager/systemops/systemops_unix.go | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/client/internal/routemanager/systemops/systemops.go b/client/internal/routemanager/systemops/systemops.go index 8caf22f81..106c520da 100644 --- a/client/internal/routemanager/systemops/systemops.go +++ b/client/internal/routemanager/systemops/systemops.go @@ -5,6 +5,7 @@ import ( "net" "net/netip" "sync" + "sync/atomic" "github.com/netbirdio/netbird/client/iface/wgaddr" "github.com/netbirdio/netbird/client/internal/routemanager/notifier" @@ -52,6 +53,9 @@ type SysOps struct { mu sync.Mutex // notifier is used to notify the system of route changes (also used on mobile) notifier *notifier.Notifier + // seq is an atomic counter for generating unique sequence numbers for route messages + //nolint:unused // only used on BSD systems + seq atomic.Uint32 } func NewSysOps(wgInterface wgIface, notifier *notifier.Notifier) *SysOps { @@ -61,6 +65,11 @@ func NewSysOps(wgInterface wgIface, notifier *notifier.Notifier) *SysOps { } } +//nolint:unused // only used on BSD systems +func (r *SysOps) getSeq() int { + return int(r.seq.Add(1)) +} + func (r *SysOps) validateRoute(prefix netip.Prefix) error { addr := prefix.Addr() diff --git a/client/internal/routemanager/systemops/systemops_unix.go b/client/internal/routemanager/systemops/systemops_unix.go index f284e131b..46e5ca915 100644 --- a/client/internal/routemanager/systemops/systemops_unix.go +++ b/client/internal/routemanager/systemops/systemops_unix.go @@ -108,7 +108,7 @@ func (r *SysOps) buildRouteMessage(action int, prefix netip.Prefix, nexthop Next Type: action, Flags: unix.RTF_UP, Version: unix.RTM_VERSION, - Seq: 1, + Seq: r.getSeq(), } const numAddrs = unix.RTAX_NETMASK + 1 From c4ed11d447dfe5a7dffb209f6071aaa5b915c7ea Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Thu, 3 Jul 2025 16:22:18 +0200 Subject: [PATCH 06/10] [client] Avoid logging setup keys on error message (#3962) --- client/internal/login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/internal/login.go b/client/internal/login.go index bbf844eb3..53fa17d90 100644 --- a/client/internal/login.go +++ b/client/internal/login.go @@ -148,7 +148,7 @@ func registerPeer(ctx context.Context, serverPublicKey wgtypes.Key, client *mgm. ) loginResp, err := client.Register(serverPublicKey, validSetupKey.String(), jwtToken, info, pubSSHKey, config.DNSLabels) if err != nil { - log.Errorf("failed registering peer %v,%s", err, validSetupKey.String()) + log.Errorf("failed registering peer %v", err) return nil, err } From 996b8c600c94c35159ffab260470a4556cae0499 Mon Sep 17 00:00:00 2001 From: "Krzysztof Nazarewski (kdn)" Date: Thu, 3 Jul 2025 16:36:36 +0200 Subject: [PATCH 07/10] [management] replace `invalid user` with a clear error message about mismatched logins (#4097) --- management/server/peer.go | 4 ++-- management/server/status/error.go | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/management/server/peer.go b/management/server/peer.go index 50967cfb9..1dd390dd9 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -857,7 +857,7 @@ func (am *DefaultAccountManager) LoginPeer(ctx context.Context, login types.Peer if login.UserID != "" { if peer.UserID != login.UserID { log.Warnf("user mismatch when logging in peer %s: peer user %s, login user %s ", peer.ID, peer.UserID, login.UserID) - return status.Errorf(status.Unauthenticated, "invalid user") + return status.NewPeerLoginMismatchError() } changed, err := am.handleUserPeer(ctx, transaction, peer, settings) @@ -1106,7 +1106,7 @@ func checkAuth(ctx context.Context, loginUserID string, peer *nbpeer.Peer) error } if peer.UserID != loginUserID { log.WithContext(ctx).Warnf("user mismatch when logging in peer %s: peer user %s, login user %s ", peer.ID, peer.UserID, loginUserID) - return status.Errorf(status.Unauthenticated, "can't login with this credentials") + return status.NewPeerLoginMismatchError() } return nil } diff --git a/management/server/status/error.go b/management/server/status/error.go index 47c236e93..e3cc27b29 100644 --- a/management/server/status/error.go +++ b/management/server/status/error.go @@ -110,11 +110,16 @@ func NewUserBlockedError() error { return Errorf(PermissionDenied, "user is blocked") } -// NewPeerNotRegisteredError creates a new Error with NotFound type for a missing peer +// NewPeerNotRegisteredError creates a new Error with Unauthenticated type unregistered peer func NewPeerNotRegisteredError() error { return Errorf(Unauthenticated, "peer is not registered") } +// NewPeerLoginMismatchError creates a new Error with Unauthenticated type for a peer that is already registered for another user +func NewPeerLoginMismatchError() error { + return Errorf(Unauthenticated, "peer is already registered by a different User or a Setup Key") +} + // NewPeerLoginExpiredError creates a new Error with PermissionDenied type for an expired peer func NewPeerLoginExpiredError() error { return Errorf(PermissionDenied, "peer login has expired, please log in once more") From f603ddf35ec8729b98d384692727d6cb2e2898a6 Mon Sep 17 00:00:00 2001 From: Pedro Maia Costa <550684+pnmcosta@users.noreply.github.com> Date: Fri, 4 Jul 2025 08:44:08 +0100 Subject: [PATCH 08/10] management: fix store get account peers without lock (#4092) --- management/server/store/sql_store.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/management/server/store/sql_store.go b/management/server/store/sql_store.go index baee4ad28..e380a7da7 100644 --- a/management/server/store/sql_store.go +++ b/management/server/store/sql_store.go @@ -1439,7 +1439,11 @@ func (s *SqlStore) GetPeerGroups(ctx context.Context, lockStrength LockingStreng // GetAccountPeers retrieves peers for an account. func (s *SqlStore) GetAccountPeers(ctx context.Context, lockStrength LockingStrength, accountID, nameFilter, ipFilter string) ([]*nbpeer.Peer, error) { var peers []*nbpeer.Peer - query := s.db.Clauses(clause.Locking{Strength: string(lockStrength)}).Where(accountIDCondition, accountID) + tx := s.db + if lockStrength != LockingStrengthNone { + tx = tx.Clauses(clause.Locking{Strength: string(lockStrength)}) + } + query := tx.Where(accountIDCondition, accountID) if nameFilter != "" { query = query.Where("name LIKE ?", "%"+nameFilter+"%") From 8c09a55057ee30406af600d72bbd0ffb9b357667 Mon Sep 17 00:00:00 2001 From: Bethuel Mmbaga Date: Fri, 4 Jul 2025 10:51:58 +0300 Subject: [PATCH 09/10] [management] Log user id on account mismatch (#4101) --- management/server/user.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/server/user.go b/management/server/user.go index a1f1c46d5..7d8382978 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -550,7 +550,7 @@ func (am *DefaultAccountManager) SaveOrAddUsers(ctx context.Context, accountID, ctx, transaction, groupsMap, accountID, initiatorUserID, initiatorUser, update, addIfNotExists, settings, ) if err != nil { - return fmt.Errorf("failed to process user update: %w", err) + return fmt.Errorf("failed to process update for user %s: %w", update.Id, err) } usersToSave = append(usersToSave, updatedUser) addUserEvents = append(addUserEvents, userEvents...) From 77ec32dd6fc87f8498c4bff55dba12f31dc9b2e1 Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Fri, 4 Jul 2025 16:43:11 +0200 Subject: [PATCH 10/10] [client] Implement dns routes for Android (#3989) --- client/android/client.go | 8 +- .../uspfilter/{uspfilter.go => filter.go} | 54 ++- ...ter_bench_test.go => filter_bench_test.go} | 80 ++-- ...r_filter_test.go => filter_filter_test.go} | 6 +- .../{uspfilter_test.go => filter_test.go} | 14 +- client/firewall/uspfilter/nat.go | 408 +++++++++++++++++ client/firewall/uspfilter/nat_bench_test.go | 416 ++++++++++++++++++ client/firewall/uspfilter/nat_test.go | 145 ++++++ client/firewall/uspfilter/tracer.go | 2 +- client/iface/device/device_filter.go | 12 +- client/iface/device/device_filter_test.go | 4 +- client/iface/mocks/filter.go | 24 +- client/iface/mocks/iface/mocks/filter.go | 24 +- client/internal/dns/server_test.go | 2 +- client/internal/engine.go | 44 +- client/internal/routemanager/client/client.go | 42 +- .../routemanager/client/client_test.go | 7 +- client/internal/routemanager/common/params.go | 28 ++ .../routemanager/dnsinterceptor/handler.go | 306 ++++++++++--- client/internal/routemanager/dynamic/route.go | 25 +- client/internal/routemanager/fakeip/fakeip.go | 93 ++++ .../routemanager/fakeip/fakeip_test.go | 240 ++++++++++ client/internal/routemanager/manager.go | 71 ++- client/internal/routemanager/mock.go | 2 +- .../routemanager/notifier/notifier.go | 124 ------ .../routemanager/notifier/notifier_android.go | 127 ++++++ .../routemanager/notifier/notifier_ios.go | 80 ++++ .../routemanager/notifier/notifier_other.go | 36 ++ client/internal/routemanager/static/route.go | 9 +- 29 files changed, 2036 insertions(+), 397 deletions(-) rename client/firewall/uspfilter/{uspfilter.go => filter.go} (96%) rename client/firewall/uspfilter/{uspfilter_bench_test.go => filter_bench_test.go} (94%) rename client/firewall/uspfilter/{uspfilter_filter_test.go => filter_filter_test.go} (99%) rename client/firewall/uspfilter/{uspfilter_test.go => filter_test.go} (98%) create mode 100644 client/firewall/uspfilter/nat.go create mode 100644 client/firewall/uspfilter/nat_bench_test.go create mode 100644 client/firewall/uspfilter/nat_test.go create mode 100644 client/internal/routemanager/common/params.go create mode 100644 client/internal/routemanager/fakeip/fakeip.go create mode 100644 client/internal/routemanager/fakeip/fakeip_test.go delete mode 100644 client/internal/routemanager/notifier/notifier.go create mode 100644 client/internal/routemanager/notifier/notifier_android.go create mode 100644 client/internal/routemanager/notifier/notifier_ios.go create mode 100644 client/internal/routemanager/notifier/notifier_other.go diff --git a/client/android/client.go b/client/android/client.go index 3b8a5bd0f..a17439696 100644 --- a/client/android/client.go +++ b/client/android/client.go @@ -203,8 +203,10 @@ func (c *Client) Networks() *NetworkArray { continue } - if routes[0].IsDynamic() { - continue + r := routes[0] + netStr := r.Network.String() + if r.IsDynamic() { + netStr = r.Domains.SafeString() } peer, err := c.recorder.GetPeer(routes[0].Peer) @@ -214,7 +216,7 @@ func (c *Client) Networks() *NetworkArray { } network := Network{ Name: string(id), - Network: routes[0].Network.String(), + Network: netStr, Peer: peer.FQDN, Status: peer.ConnStatus.String(), } diff --git a/client/firewall/uspfilter/uspfilter.go b/client/firewall/uspfilter/filter.go similarity index 96% rename from client/firewall/uspfilter/uspfilter.go rename to client/firewall/uspfilter/filter.go index dcff92c61..7120d7d64 100644 --- a/client/firewall/uspfilter/uspfilter.go +++ b/client/firewall/uspfilter/filter.go @@ -104,6 +104,12 @@ type Manager struct { flowLogger nftypes.FlowLogger blockRule firewall.Rule + + // Internal 1:1 DNAT + dnatEnabled atomic.Bool + dnatMappings map[netip.Addr]netip.Addr + dnatMutex sync.RWMutex + dnatBiMap *biDNATMap } // decoder for packages @@ -189,6 +195,7 @@ func create(iface common.IFaceMapper, nativeFirewall firewall.Manager, disableSe flowLogger: flowLogger, netstack: netstack.IsEnabled(), localForwarding: enableLocalForwarding, + dnatMappings: make(map[netip.Addr]netip.Addr), } m.routingEnabled.Store(false) @@ -519,22 +526,6 @@ func (m *Manager) SetLegacyManagement(isLegacy bool) error { // Flush doesn't need to be implemented for this manager func (m *Manager) Flush() error { return nil } -// AddDNATRule adds a DNAT rule -func (m *Manager) AddDNATRule(rule firewall.ForwardRule) (firewall.Rule, error) { - if m.nativeFirewall == nil { - return nil, errNatNotSupported - } - return m.nativeFirewall.AddDNATRule(rule) -} - -// DeleteDNATRule deletes a DNAT rule -func (m *Manager) DeleteDNATRule(rule firewall.Rule) error { - if m.nativeFirewall == nil { - return errNatNotSupported - } - return m.nativeFirewall.DeleteDNATRule(rule) -} - // UpdateSet updates the rule destinations associated with the given set // by merging the existing prefixes with the new ones, then deduplicating. func (m *Manager) UpdateSet(set firewall.Set, prefixes []netip.Prefix) error { @@ -581,14 +572,14 @@ func (m *Manager) UpdateSet(set firewall.Set, prefixes []netip.Prefix) error { return nil } -// DropOutgoing filter outgoing packets -func (m *Manager) DropOutgoing(packetData []byte, size int) bool { - return m.processOutgoingHooks(packetData, size) +// FilterOutBound filters outgoing packets +func (m *Manager) FilterOutbound(packetData []byte, size int) bool { + return m.filterOutbound(packetData, size) } -// DropIncoming filter incoming packets -func (m *Manager) DropIncoming(packetData []byte, size int) bool { - return m.dropFilter(packetData, size) +// FilterInbound filters incoming packets +func (m *Manager) FilterInbound(packetData []byte, size int) bool { + return m.filterInbound(packetData, size) } // UpdateLocalIPs updates the list of local IPs @@ -596,7 +587,7 @@ func (m *Manager) UpdateLocalIPs() error { return m.localipmanager.UpdateLocalIPs(m.wgIface) } -func (m *Manager) processOutgoingHooks(packetData []byte, size int) bool { +func (m *Manager) filterOutbound(packetData []byte, size int) bool { d := m.decoders.Get().(*decoder) defer m.decoders.Put(d) @@ -618,8 +609,8 @@ func (m *Manager) processOutgoingHooks(packetData []byte, size int) bool { return true } - // for netflow we keep track even if the firewall is stateless m.trackOutbound(d, srcIP, dstIP, size) + m.translateOutboundDNAT(packetData, d) return false } @@ -723,9 +714,9 @@ func (m *Manager) udpHooksDrop(dport uint16, dstIP netip.Addr, packetData []byte return false } -// dropFilter implements filtering logic for incoming packets. +// filterInbound implements filtering logic for incoming packets. // If it returns true, the packet should be dropped. -func (m *Manager) dropFilter(packetData []byte, size int) bool { +func (m *Manager) filterInbound(packetData []byte, size int) bool { d := m.decoders.Get().(*decoder) defer m.decoders.Put(d) @@ -747,8 +738,15 @@ func (m *Manager) dropFilter(packetData []byte, size int) bool { return false } - // For all inbound traffic, first check if it matches a tracked connection. - // This must happen before any other filtering because the packets are statefully tracked. + if translated := m.translateInboundReverse(packetData, d); translated { + // Re-decode after translation to get original addresses + if err := d.parser.DecodeLayers(packetData, &d.decoded); err != nil { + m.logger.Error("Failed to re-decode packet after reverse DNAT: %v", err) + return true + } + srcIP, dstIP = m.extractIPs(d) + } + if m.stateful && m.isValidTrackedConnection(d, srcIP, dstIP, size) { return false } diff --git a/client/firewall/uspfilter/uspfilter_bench_test.go b/client/firewall/uspfilter/filter_bench_test.go similarity index 94% rename from client/firewall/uspfilter/uspfilter_bench_test.go rename to client/firewall/uspfilter/filter_bench_test.go index c03e60640..0cffcc1a7 100644 --- a/client/firewall/uspfilter/uspfilter_bench_test.go +++ b/client/firewall/uspfilter/filter_bench_test.go @@ -188,13 +188,13 @@ func BenchmarkCoreFiltering(b *testing.B) { // For stateful scenarios, establish the connection if sc.stateful { - manager.processOutgoingHooks(outbound, 0) + manager.filterOutbound(outbound, 0) } // Measure inbound packet processing b.ResetTimer() for i := 0; i < b.N; i++ { - manager.dropFilter(inbound, 0) + manager.filterInbound(inbound, 0) } }) } @@ -220,7 +220,7 @@ func BenchmarkStateScaling(b *testing.B) { for i := 0; i < count; i++ { outbound := generatePacket(b, srcIPs[i], dstIPs[i], uint16(1024+i), 80, layers.IPProtocolTCP) - manager.processOutgoingHooks(outbound, 0) + manager.filterOutbound(outbound, 0) } // Test packet @@ -228,11 +228,11 @@ func BenchmarkStateScaling(b *testing.B) { testIn := generatePacket(b, dstIPs[0], srcIPs[0], 80, 1024, layers.IPProtocolTCP) // First establish our test connection - manager.processOutgoingHooks(testOut, 0) + manager.filterOutbound(testOut, 0) b.ResetTimer() for i := 0; i < b.N; i++ { - manager.dropFilter(testIn, 0) + manager.filterInbound(testIn, 0) } }) } @@ -263,12 +263,12 @@ func BenchmarkEstablishmentOverhead(b *testing.B) { inbound := generatePacket(b, dstIP, srcIP, 80, 1024, layers.IPProtocolTCP) if sc.established { - manager.processOutgoingHooks(outbound, 0) + manager.filterOutbound(outbound, 0) } b.ResetTimer() for i := 0; i < b.N; i++ { - manager.dropFilter(inbound, 0) + manager.filterInbound(inbound, 0) } }) } @@ -426,25 +426,25 @@ func BenchmarkRoutedNetworkReturn(b *testing.B) { // For stateful cases and established connections if !strings.Contains(sc.name, "allow_non_wg") || (strings.Contains(sc.state, "established") || sc.state == "post_handshake") { - manager.processOutgoingHooks(outbound, 0) + manager.filterOutbound(outbound, 0) // For TCP post-handshake, simulate full handshake if sc.state == "post_handshake" { // SYN syn := generateTCPPacketWithFlags(b, srcIP, dstIP, 1024, 80, uint16(conntrack.TCPSyn)) - manager.processOutgoingHooks(syn, 0) + manager.filterOutbound(syn, 0) // SYN-ACK synack := generateTCPPacketWithFlags(b, dstIP, srcIP, 80, 1024, uint16(conntrack.TCPSyn|conntrack.TCPAck)) - manager.dropFilter(synack, 0) + manager.filterInbound(synack, 0) // ACK ack := generateTCPPacketWithFlags(b, srcIP, dstIP, 1024, 80, uint16(conntrack.TCPAck)) - manager.processOutgoingHooks(ack, 0) + manager.filterOutbound(ack, 0) } } b.ResetTimer() for i := 0; i < b.N; i++ { - manager.dropFilter(inbound, 0) + manager.filterInbound(inbound, 0) } }) } @@ -568,17 +568,17 @@ func BenchmarkLongLivedConnections(b *testing.B) { // Initial SYN syn := generateTCPPacketWithFlags(b, srcIPs[i], dstIPs[i], uint16(1024+i), 80, uint16(conntrack.TCPSyn)) - manager.processOutgoingHooks(syn, 0) + manager.filterOutbound(syn, 0) // SYN-ACK synack := generateTCPPacketWithFlags(b, dstIPs[i], srcIPs[i], 80, uint16(1024+i), uint16(conntrack.TCPSyn|conntrack.TCPAck)) - manager.dropFilter(synack, 0) + manager.filterInbound(synack, 0) // ACK ack := generateTCPPacketWithFlags(b, srcIPs[i], dstIPs[i], uint16(1024+i), 80, uint16(conntrack.TCPAck)) - manager.processOutgoingHooks(ack, 0) + manager.filterOutbound(ack, 0) } // Prepare test packets simulating bidirectional traffic @@ -599,9 +599,9 @@ func BenchmarkLongLivedConnections(b *testing.B) { // Simulate bidirectional traffic // First outbound data - manager.processOutgoingHooks(outPackets[connIdx], 0) + manager.filterOutbound(outPackets[connIdx], 0) // Then inbound response - this is what we're actually measuring - manager.dropFilter(inPackets[connIdx], 0) + manager.filterInbound(inPackets[connIdx], 0) } }) } @@ -700,19 +700,19 @@ func BenchmarkShortLivedConnections(b *testing.B) { p := patterns[connIdx] // Connection establishment - manager.processOutgoingHooks(p.syn, 0) - manager.dropFilter(p.synAck, 0) - manager.processOutgoingHooks(p.ack, 0) + manager.filterOutbound(p.syn, 0) + manager.filterInbound(p.synAck, 0) + manager.filterOutbound(p.ack, 0) // Data transfer - manager.processOutgoingHooks(p.request, 0) - manager.dropFilter(p.response, 0) + manager.filterOutbound(p.request, 0) + manager.filterInbound(p.response, 0) // Connection teardown - manager.processOutgoingHooks(p.finClient, 0) - manager.dropFilter(p.ackServer, 0) - manager.dropFilter(p.finServer, 0) - manager.processOutgoingHooks(p.ackClient, 0) + manager.filterOutbound(p.finClient, 0) + manager.filterInbound(p.ackServer, 0) + manager.filterInbound(p.finServer, 0) + manager.filterOutbound(p.ackClient, 0) } }) } @@ -760,15 +760,15 @@ func BenchmarkParallelLongLivedConnections(b *testing.B) { for i := 0; i < sc.connCount; i++ { syn := generateTCPPacketWithFlags(b, srcIPs[i], dstIPs[i], uint16(1024+i), 80, uint16(conntrack.TCPSyn)) - manager.processOutgoingHooks(syn, 0) + manager.filterOutbound(syn, 0) synack := generateTCPPacketWithFlags(b, dstIPs[i], srcIPs[i], 80, uint16(1024+i), uint16(conntrack.TCPSyn|conntrack.TCPAck)) - manager.dropFilter(synack, 0) + manager.filterInbound(synack, 0) ack := generateTCPPacketWithFlags(b, srcIPs[i], dstIPs[i], uint16(1024+i), 80, uint16(conntrack.TCPAck)) - manager.processOutgoingHooks(ack, 0) + manager.filterOutbound(ack, 0) } // Pre-generate test packets @@ -790,8 +790,8 @@ func BenchmarkParallelLongLivedConnections(b *testing.B) { counter++ // Simulate bidirectional traffic - manager.processOutgoingHooks(outPackets[connIdx], 0) - manager.dropFilter(inPackets[connIdx], 0) + manager.filterOutbound(outPackets[connIdx], 0) + manager.filterInbound(inPackets[connIdx], 0) } }) }) @@ -879,17 +879,17 @@ func BenchmarkParallelShortLivedConnections(b *testing.B) { p := patterns[connIdx] // Full connection lifecycle - manager.processOutgoingHooks(p.syn, 0) - manager.dropFilter(p.synAck, 0) - manager.processOutgoingHooks(p.ack, 0) + manager.filterOutbound(p.syn, 0) + manager.filterInbound(p.synAck, 0) + manager.filterOutbound(p.ack, 0) - manager.processOutgoingHooks(p.request, 0) - manager.dropFilter(p.response, 0) + manager.filterOutbound(p.request, 0) + manager.filterInbound(p.response, 0) - manager.processOutgoingHooks(p.finClient, 0) - manager.dropFilter(p.ackServer, 0) - manager.dropFilter(p.finServer, 0) - manager.processOutgoingHooks(p.ackClient, 0) + manager.filterOutbound(p.finClient, 0) + manager.filterInbound(p.ackServer, 0) + manager.filterInbound(p.finServer, 0) + manager.filterOutbound(p.ackClient, 0) } }) }) diff --git a/client/firewall/uspfilter/uspfilter_filter_test.go b/client/firewall/uspfilter/filter_filter_test.go similarity index 99% rename from client/firewall/uspfilter/uspfilter_filter_test.go rename to client/firewall/uspfilter/filter_filter_test.go index 318f86a87..b630c9e66 100644 --- a/client/firewall/uspfilter/uspfilter_filter_test.go +++ b/client/firewall/uspfilter/filter_filter_test.go @@ -462,7 +462,7 @@ func TestPeerACLFiltering(t *testing.T) { t.Run("Implicit DROP (no rules)", func(t *testing.T) { packet := createTestPacket(t, "100.10.0.1", "100.10.0.100", fw.ProtocolTCP, 12345, 443) - isDropped := manager.DropIncoming(packet, 0) + isDropped := manager.FilterInbound(packet, 0) require.True(t, isDropped, "Packet should be dropped when no rules exist") }) @@ -509,7 +509,7 @@ func TestPeerACLFiltering(t *testing.T) { }) packet := createTestPacket(t, tc.srcIP, tc.dstIP, tc.proto, tc.srcPort, tc.dstPort) - isDropped := manager.DropIncoming(packet, 0) + isDropped := manager.FilterInbound(packet, 0) require.Equal(t, tc.shouldBeBlocked, isDropped) }) } @@ -1233,7 +1233,7 @@ func TestRouteACLFiltering(t *testing.T) { srcIP := netip.MustParseAddr(tc.srcIP) dstIP := netip.MustParseAddr(tc.dstIP) - // testing routeACLsPass only and not DropIncoming, as routed packets are dropped after being passed + // testing routeACLsPass only and not FilterInbound, as routed packets are dropped after being passed // to the forwarder _, isAllowed := manager.routeACLsPass(srcIP, dstIP, tc.proto, tc.srcPort, tc.dstPort) require.Equal(t, tc.shouldPass, isAllowed) diff --git a/client/firewall/uspfilter/uspfilter_test.go b/client/firewall/uspfilter/filter_test.go similarity index 98% rename from client/firewall/uspfilter/uspfilter_test.go rename to client/firewall/uspfilter/filter_test.go index 88de1ddcd..5b5cd5a53 100644 --- a/client/firewall/uspfilter/uspfilter_test.go +++ b/client/firewall/uspfilter/filter_test.go @@ -321,7 +321,7 @@ func TestNotMatchByIP(t *testing.T) { return } - if m.dropFilter(buf.Bytes(), 0) { + if m.filterInbound(buf.Bytes(), 0) { t.Errorf("expected packet to be accepted") return } @@ -447,7 +447,7 @@ func TestProcessOutgoingHooks(t *testing.T) { require.NoError(t, err) // Test hook gets called - result := manager.processOutgoingHooks(buf.Bytes(), 0) + result := manager.filterOutbound(buf.Bytes(), 0) require.True(t, result) require.True(t, hookCalled) @@ -457,7 +457,7 @@ func TestProcessOutgoingHooks(t *testing.T) { err = gopacket.SerializeLayers(buf, opts, ipv4) require.NoError(t, err) - result = manager.processOutgoingHooks(buf.Bytes(), 0) + result = manager.filterOutbound(buf.Bytes(), 0) require.False(t, result) } @@ -553,7 +553,7 @@ func TestStatefulFirewall_UDPTracking(t *testing.T) { require.NoError(t, err) // Process outbound packet and verify connection tracking - drop := manager.DropOutgoing(outboundBuf.Bytes(), 0) + drop := manager.FilterOutbound(outboundBuf.Bytes(), 0) require.False(t, drop, "Initial outbound packet should not be dropped") // Verify connection was tracked @@ -620,7 +620,7 @@ func TestStatefulFirewall_UDPTracking(t *testing.T) { for _, cp := range checkPoints { time.Sleep(cp.sleep) - drop = manager.dropFilter(inboundBuf.Bytes(), 0) + drop = manager.filterInbound(inboundBuf.Bytes(), 0) require.Equal(t, cp.shouldAllow, !drop, cp.description) // If the connection should still be valid, verify it exists @@ -669,7 +669,7 @@ func TestStatefulFirewall_UDPTracking(t *testing.T) { } // Create a new outbound connection for invalid tests - drop = manager.processOutgoingHooks(outboundBuf.Bytes(), 0) + drop = manager.filterOutbound(outboundBuf.Bytes(), 0) require.False(t, drop, "Second outbound packet should not be dropped") for _, tc := range invalidCases { @@ -691,7 +691,7 @@ func TestStatefulFirewall_UDPTracking(t *testing.T) { require.NoError(t, err) // Verify the invalid packet is dropped - drop = manager.dropFilter(testBuf.Bytes(), 0) + drop = manager.filterInbound(testBuf.Bytes(), 0) require.True(t, drop, tc.description) }) } diff --git a/client/firewall/uspfilter/nat.go b/client/firewall/uspfilter/nat.go new file mode 100644 index 000000000..4539f7da5 --- /dev/null +++ b/client/firewall/uspfilter/nat.go @@ -0,0 +1,408 @@ +package uspfilter + +import ( + "encoding/binary" + "errors" + "fmt" + "net/netip" + + "github.com/google/gopacket/layers" + + firewall "github.com/netbirdio/netbird/client/firewall/manager" +) + +var ErrIPv4Only = errors.New("only IPv4 is supported for DNAT") + +func ipv4Checksum(header []byte) uint16 { + if len(header) < 20 { + return 0 + } + + var sum1, sum2 uint32 + + // Parallel processing - unroll and compute two sums simultaneously + sum1 += uint32(binary.BigEndian.Uint16(header[0:2])) + sum2 += uint32(binary.BigEndian.Uint16(header[2:4])) + sum1 += uint32(binary.BigEndian.Uint16(header[4:6])) + sum2 += uint32(binary.BigEndian.Uint16(header[6:8])) + sum1 += uint32(binary.BigEndian.Uint16(header[8:10])) + // Skip checksum field at [10:12] + sum2 += uint32(binary.BigEndian.Uint16(header[12:14])) + sum1 += uint32(binary.BigEndian.Uint16(header[14:16])) + sum2 += uint32(binary.BigEndian.Uint16(header[16:18])) + sum1 += uint32(binary.BigEndian.Uint16(header[18:20])) + + sum := sum1 + sum2 + + // Handle remaining bytes for headers > 20 bytes + for i := 20; i < len(header)-1; i += 2 { + sum += uint32(binary.BigEndian.Uint16(header[i : i+2])) + } + + if len(header)%2 == 1 { + sum += uint32(header[len(header)-1]) << 8 + } + + // Optimized carry fold - single iteration handles most cases + sum = (sum & 0xFFFF) + (sum >> 16) + if sum > 0xFFFF { + sum++ + } + + return ^uint16(sum) +} + +func icmpChecksum(data []byte) uint16 { + var sum1, sum2, sum3, sum4 uint32 + i := 0 + + // Process 16 bytes at once with 4 parallel accumulators + for i <= len(data)-16 { + sum1 += uint32(binary.BigEndian.Uint16(data[i : i+2])) + sum2 += uint32(binary.BigEndian.Uint16(data[i+2 : i+4])) + sum3 += uint32(binary.BigEndian.Uint16(data[i+4 : i+6])) + sum4 += uint32(binary.BigEndian.Uint16(data[i+6 : i+8])) + sum1 += uint32(binary.BigEndian.Uint16(data[i+8 : i+10])) + sum2 += uint32(binary.BigEndian.Uint16(data[i+10 : i+12])) + sum3 += uint32(binary.BigEndian.Uint16(data[i+12 : i+14])) + sum4 += uint32(binary.BigEndian.Uint16(data[i+14 : i+16])) + i += 16 + } + + sum := sum1 + sum2 + sum3 + sum4 + + // Handle remaining bytes + for i < len(data)-1 { + sum += uint32(binary.BigEndian.Uint16(data[i : i+2])) + i += 2 + } + + if len(data)%2 == 1 { + sum += uint32(data[len(data)-1]) << 8 + } + + sum = (sum & 0xFFFF) + (sum >> 16) + if sum > 0xFFFF { + sum++ + } + + return ^uint16(sum) +} + +type biDNATMap struct { + forward map[netip.Addr]netip.Addr + reverse map[netip.Addr]netip.Addr +} + +func newBiDNATMap() *biDNATMap { + return &biDNATMap{ + forward: make(map[netip.Addr]netip.Addr), + reverse: make(map[netip.Addr]netip.Addr), + } +} + +func (b *biDNATMap) set(original, translated netip.Addr) { + b.forward[original] = translated + b.reverse[translated] = original +} + +func (b *biDNATMap) delete(original netip.Addr) { + if translated, exists := b.forward[original]; exists { + delete(b.forward, original) + delete(b.reverse, translated) + } +} + +func (b *biDNATMap) getTranslated(original netip.Addr) (netip.Addr, bool) { + translated, exists := b.forward[original] + return translated, exists +} + +func (b *biDNATMap) getOriginal(translated netip.Addr) (netip.Addr, bool) { + original, exists := b.reverse[translated] + return original, exists +} + +func (m *Manager) AddInternalDNATMapping(originalAddr, translatedAddr netip.Addr) error { + if !originalAddr.IsValid() || !translatedAddr.IsValid() { + return fmt.Errorf("invalid IP addresses") + } + + if m.localipmanager.IsLocalIP(translatedAddr) { + return fmt.Errorf("cannot map to local IP: %s", translatedAddr) + } + + m.dnatMutex.Lock() + defer m.dnatMutex.Unlock() + + // Initialize both maps together if either is nil + if m.dnatMappings == nil || m.dnatBiMap == nil { + m.dnatMappings = make(map[netip.Addr]netip.Addr) + m.dnatBiMap = newBiDNATMap() + } + + m.dnatMappings[originalAddr] = translatedAddr + m.dnatBiMap.set(originalAddr, translatedAddr) + + if len(m.dnatMappings) == 1 { + m.dnatEnabled.Store(true) + } + + return nil +} + +// RemoveInternalDNATMapping removes a 1:1 IP address mapping +func (m *Manager) RemoveInternalDNATMapping(originalAddr netip.Addr) error { + m.dnatMutex.Lock() + defer m.dnatMutex.Unlock() + + if _, exists := m.dnatMappings[originalAddr]; !exists { + return fmt.Errorf("mapping not found for: %s", originalAddr) + } + + delete(m.dnatMappings, originalAddr) + m.dnatBiMap.delete(originalAddr) + if len(m.dnatMappings) == 0 { + m.dnatEnabled.Store(false) + } + + return nil +} + +// getDNATTranslation returns the translated address if a mapping exists +func (m *Manager) getDNATTranslation(addr netip.Addr) (netip.Addr, bool) { + if !m.dnatEnabled.Load() { + return addr, false + } + + m.dnatMutex.RLock() + translated, exists := m.dnatBiMap.getTranslated(addr) + m.dnatMutex.RUnlock() + return translated, exists +} + +// findReverseDNATMapping finds original address for return traffic +func (m *Manager) findReverseDNATMapping(translatedAddr netip.Addr) (netip.Addr, bool) { + if !m.dnatEnabled.Load() { + return translatedAddr, false + } + + m.dnatMutex.RLock() + original, exists := m.dnatBiMap.getOriginal(translatedAddr) + m.dnatMutex.RUnlock() + return original, exists +} + +// translateOutboundDNAT applies DNAT translation to outbound packets +func (m *Manager) translateOutboundDNAT(packetData []byte, d *decoder) bool { + if !m.dnatEnabled.Load() { + return false + } + + if len(packetData) < 20 || d.decoded[0] != layers.LayerTypeIPv4 { + return false + } + + dstIP := netip.AddrFrom4([4]byte{packetData[16], packetData[17], packetData[18], packetData[19]}) + + translatedIP, exists := m.getDNATTranslation(dstIP) + if !exists { + return false + } + + if err := m.rewritePacketDestination(packetData, d, translatedIP); err != nil { + m.logger.Error("Failed to rewrite packet destination: %v", err) + return false + } + + m.logger.Trace("DNAT: %s -> %s", dstIP, translatedIP) + return true +} + +// translateInboundReverse applies reverse DNAT to inbound return traffic +func (m *Manager) translateInboundReverse(packetData []byte, d *decoder) bool { + if !m.dnatEnabled.Load() { + return false + } + + if len(packetData) < 20 || d.decoded[0] != layers.LayerTypeIPv4 { + return false + } + + srcIP := netip.AddrFrom4([4]byte{packetData[12], packetData[13], packetData[14], packetData[15]}) + + originalIP, exists := m.findReverseDNATMapping(srcIP) + if !exists { + return false + } + + if err := m.rewritePacketSource(packetData, d, originalIP); err != nil { + m.logger.Error("Failed to rewrite packet source: %v", err) + return false + } + + m.logger.Trace("Reverse DNAT: %s -> %s", srcIP, originalIP) + return true +} + +// rewritePacketDestination replaces destination IP in the packet +func (m *Manager) rewritePacketDestination(packetData []byte, d *decoder, newIP netip.Addr) error { + if len(packetData) < 20 || d.decoded[0] != layers.LayerTypeIPv4 || !newIP.Is4() { + return ErrIPv4Only + } + + var oldDst [4]byte + copy(oldDst[:], packetData[16:20]) + newDst := newIP.As4() + + copy(packetData[16:20], newDst[:]) + + ipHeaderLen := int(d.ip4.IHL) * 4 + if ipHeaderLen < 20 || ipHeaderLen > len(packetData) { + return fmt.Errorf("invalid IP header length") + } + + binary.BigEndian.PutUint16(packetData[10:12], 0) + ipChecksum := ipv4Checksum(packetData[:ipHeaderLen]) + binary.BigEndian.PutUint16(packetData[10:12], ipChecksum) + + if len(d.decoded) > 1 { + switch d.decoded[1] { + case layers.LayerTypeTCP: + m.updateTCPChecksum(packetData, ipHeaderLen, oldDst[:], newDst[:]) + case layers.LayerTypeUDP: + m.updateUDPChecksum(packetData, ipHeaderLen, oldDst[:], newDst[:]) + case layers.LayerTypeICMPv4: + m.updateICMPChecksum(packetData, ipHeaderLen) + } + } + + return nil +} + +// rewritePacketSource replaces the source IP address in the packet +func (m *Manager) rewritePacketSource(packetData []byte, d *decoder, newIP netip.Addr) error { + if len(packetData) < 20 || d.decoded[0] != layers.LayerTypeIPv4 || !newIP.Is4() { + return ErrIPv4Only + } + + var oldSrc [4]byte + copy(oldSrc[:], packetData[12:16]) + newSrc := newIP.As4() + + copy(packetData[12:16], newSrc[:]) + + ipHeaderLen := int(d.ip4.IHL) * 4 + if ipHeaderLen < 20 || ipHeaderLen > len(packetData) { + return fmt.Errorf("invalid IP header length") + } + + binary.BigEndian.PutUint16(packetData[10:12], 0) + ipChecksum := ipv4Checksum(packetData[:ipHeaderLen]) + binary.BigEndian.PutUint16(packetData[10:12], ipChecksum) + + if len(d.decoded) > 1 { + switch d.decoded[1] { + case layers.LayerTypeTCP: + m.updateTCPChecksum(packetData, ipHeaderLen, oldSrc[:], newSrc[:]) + case layers.LayerTypeUDP: + m.updateUDPChecksum(packetData, ipHeaderLen, oldSrc[:], newSrc[:]) + case layers.LayerTypeICMPv4: + m.updateICMPChecksum(packetData, ipHeaderLen) + } + } + + return nil +} + +func (m *Manager) updateTCPChecksum(packetData []byte, ipHeaderLen int, oldIP, newIP []byte) { + tcpStart := ipHeaderLen + if len(packetData) < tcpStart+18 { + return + } + + checksumOffset := tcpStart + 16 + oldChecksum := binary.BigEndian.Uint16(packetData[checksumOffset : checksumOffset+2]) + newChecksum := incrementalUpdate(oldChecksum, oldIP, newIP) + binary.BigEndian.PutUint16(packetData[checksumOffset:checksumOffset+2], newChecksum) +} + +func (m *Manager) updateUDPChecksum(packetData []byte, ipHeaderLen int, oldIP, newIP []byte) { + udpStart := ipHeaderLen + if len(packetData) < udpStart+8 { + return + } + + checksumOffset := udpStart + 6 + oldChecksum := binary.BigEndian.Uint16(packetData[checksumOffset : checksumOffset+2]) + + if oldChecksum == 0 { + return + } + + newChecksum := incrementalUpdate(oldChecksum, oldIP, newIP) + binary.BigEndian.PutUint16(packetData[checksumOffset:checksumOffset+2], newChecksum) +} + +func (m *Manager) updateICMPChecksum(packetData []byte, ipHeaderLen int) { + icmpStart := ipHeaderLen + if len(packetData) < icmpStart+8 { + return + } + + icmpData := packetData[icmpStart:] + binary.BigEndian.PutUint16(icmpData[2:4], 0) + checksum := icmpChecksum(icmpData) + binary.BigEndian.PutUint16(icmpData[2:4], checksum) +} + +// incrementalUpdate performs incremental checksum update per RFC 1624 +func incrementalUpdate(oldChecksum uint16, oldBytes, newBytes []byte) uint16 { + sum := uint32(^oldChecksum) + + // Fast path for IPv4 addresses (4 bytes) - most common case + if len(oldBytes) == 4 && len(newBytes) == 4 { + sum += uint32(^binary.BigEndian.Uint16(oldBytes[0:2])) + sum += uint32(^binary.BigEndian.Uint16(oldBytes[2:4])) + sum += uint32(binary.BigEndian.Uint16(newBytes[0:2])) + sum += uint32(binary.BigEndian.Uint16(newBytes[2:4])) + } else { + // Fallback for other lengths + for i := 0; i < len(oldBytes)-1; i += 2 { + sum += uint32(^binary.BigEndian.Uint16(oldBytes[i : i+2])) + } + if len(oldBytes)%2 == 1 { + sum += uint32(^oldBytes[len(oldBytes)-1]) << 8 + } + + for i := 0; i < len(newBytes)-1; i += 2 { + sum += uint32(binary.BigEndian.Uint16(newBytes[i : i+2])) + } + if len(newBytes)%2 == 1 { + sum += uint32(newBytes[len(newBytes)-1]) << 8 + } + } + + sum = (sum & 0xFFFF) + (sum >> 16) + if sum > 0xFFFF { + sum++ + } + + return ^uint16(sum) +} + +// AddDNATRule adds a DNAT rule (delegates to native firewall for port forwarding) +func (m *Manager) AddDNATRule(rule firewall.ForwardRule) (firewall.Rule, error) { + if m.nativeFirewall == nil { + return nil, errNatNotSupported + } + return m.nativeFirewall.AddDNATRule(rule) +} + +// DeleteDNATRule deletes a DNAT rule (delegates to native firewall) +func (m *Manager) DeleteDNATRule(rule firewall.Rule) error { + if m.nativeFirewall == nil { + return errNatNotSupported + } + return m.nativeFirewall.DeleteDNATRule(rule) +} diff --git a/client/firewall/uspfilter/nat_bench_test.go b/client/firewall/uspfilter/nat_bench_test.go new file mode 100644 index 000000000..16dba682e --- /dev/null +++ b/client/firewall/uspfilter/nat_bench_test.go @@ -0,0 +1,416 @@ +package uspfilter + +import ( + "fmt" + "net/netip" + "testing" + + "github.com/google/gopacket" + "github.com/google/gopacket/layers" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/client/iface/device" +) + +// BenchmarkDNATTranslation measures the performance of DNAT operations +func BenchmarkDNATTranslation(b *testing.B) { + scenarios := []struct { + name string + proto layers.IPProtocol + setupDNAT bool + description string + }{ + { + name: "tcp_with_dnat", + proto: layers.IPProtocolTCP, + setupDNAT: true, + description: "TCP packet with DNAT translation enabled", + }, + { + name: "tcp_without_dnat", + proto: layers.IPProtocolTCP, + setupDNAT: false, + description: "TCP packet without DNAT (baseline)", + }, + { + name: "udp_with_dnat", + proto: layers.IPProtocolUDP, + setupDNAT: true, + description: "UDP packet with DNAT translation enabled", + }, + { + name: "udp_without_dnat", + proto: layers.IPProtocolUDP, + setupDNAT: false, + description: "UDP packet without DNAT (baseline)", + }, + { + name: "icmp_with_dnat", + proto: layers.IPProtocolICMPv4, + setupDNAT: true, + description: "ICMP packet with DNAT translation enabled", + }, + { + name: "icmp_without_dnat", + proto: layers.IPProtocolICMPv4, + setupDNAT: false, + description: "ICMP packet without DNAT (baseline)", + }, + } + + for _, sc := range scenarios { + b.Run(sc.name, func(b *testing.B) { + manager, err := Create(&IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, false, flowLogger) + require.NoError(b, err) + defer func() { + require.NoError(b, manager.Close(nil)) + }() + + // Set logger to error level to reduce noise during benchmarking + manager.SetLogLevel(log.ErrorLevel) + defer func() { + // Restore to info level after benchmark + manager.SetLogLevel(log.InfoLevel) + }() + + // Setup DNAT mapping if needed + originalIP := netip.MustParseAddr("192.168.1.100") + translatedIP := netip.MustParseAddr("10.0.0.100") + + if sc.setupDNAT { + err := manager.AddInternalDNATMapping(originalIP, translatedIP) + require.NoError(b, err) + } + + // Create test packets + srcIP := netip.MustParseAddr("172.16.0.1") + outboundPacket := generateDNATTestPacket(b, srcIP, originalIP, sc.proto, 12345, 80) + + // Pre-establish connection for reverse DNAT test + if sc.setupDNAT { + manager.filterOutbound(outboundPacket, 0) + } + + b.ResetTimer() + + // Benchmark outbound DNAT translation + b.Run("outbound", func(b *testing.B) { + for i := 0; i < b.N; i++ { + // Create fresh packet each time since translation modifies it + packet := generateDNATTestPacket(b, srcIP, originalIP, sc.proto, 12345, 80) + manager.filterOutbound(packet, 0) + } + }) + + // Benchmark inbound reverse DNAT translation + if sc.setupDNAT { + b.Run("inbound_reverse", func(b *testing.B) { + for i := 0; i < b.N; i++ { + // Create fresh packet each time since translation modifies it + packet := generateDNATTestPacket(b, translatedIP, srcIP, sc.proto, 80, 12345) + manager.filterInbound(packet, 0) + } + }) + } + }) + } +} + +// BenchmarkDNATConcurrency tests DNAT performance under concurrent load +func BenchmarkDNATConcurrency(b *testing.B) { + manager, err := Create(&IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, false, flowLogger) + require.NoError(b, err) + defer func() { + require.NoError(b, manager.Close(nil)) + }() + + // Set logger to error level to reduce noise during benchmarking + manager.SetLogLevel(log.ErrorLevel) + defer func() { + // Restore to info level after benchmark + manager.SetLogLevel(log.InfoLevel) + }() + + // Setup multiple DNAT mappings + numMappings := 100 + originalIPs := make([]netip.Addr, numMappings) + translatedIPs := make([]netip.Addr, numMappings) + + for i := 0; i < numMappings; i++ { + originalIPs[i] = netip.MustParseAddr(fmt.Sprintf("192.168.%d.%d", (i/254)+1, (i%254)+1)) + translatedIPs[i] = netip.MustParseAddr(fmt.Sprintf("10.0.%d.%d", (i/254)+1, (i%254)+1)) + err := manager.AddInternalDNATMapping(originalIPs[i], translatedIPs[i]) + require.NoError(b, err) + } + + srcIP := netip.MustParseAddr("172.16.0.1") + + // Pre-generate packets + outboundPackets := make([][]byte, numMappings) + inboundPackets := make([][]byte, numMappings) + for i := 0; i < numMappings; i++ { + outboundPackets[i] = generateDNATTestPacket(b, srcIP, originalIPs[i], layers.IPProtocolTCP, 12345, 80) + inboundPackets[i] = generateDNATTestPacket(b, translatedIPs[i], srcIP, layers.IPProtocolTCP, 80, 12345) + // Establish connections + manager.filterOutbound(outboundPackets[i], 0) + } + + b.ResetTimer() + + b.Run("concurrent_outbound", func(b *testing.B) { + b.RunParallel(func(pb *testing.PB) { + i := 0 + for pb.Next() { + idx := i % numMappings + packet := generateDNATTestPacket(b, srcIP, originalIPs[idx], layers.IPProtocolTCP, 12345, 80) + manager.filterOutbound(packet, 0) + i++ + } + }) + }) + + b.Run("concurrent_inbound", func(b *testing.B) { + b.RunParallel(func(pb *testing.PB) { + i := 0 + for pb.Next() { + idx := i % numMappings + packet := generateDNATTestPacket(b, translatedIPs[idx], srcIP, layers.IPProtocolTCP, 80, 12345) + manager.filterInbound(packet, 0) + i++ + } + }) + }) +} + +// BenchmarkDNATScaling tests how DNAT performance scales with number of mappings +func BenchmarkDNATScaling(b *testing.B) { + mappingCounts := []int{1, 10, 100, 1000} + + for _, count := range mappingCounts { + b.Run(fmt.Sprintf("mappings_%d", count), func(b *testing.B) { + manager, err := Create(&IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, false, flowLogger) + require.NoError(b, err) + defer func() { + require.NoError(b, manager.Close(nil)) + }() + + // Set logger to error level to reduce noise during benchmarking + manager.SetLogLevel(log.ErrorLevel) + defer func() { + // Restore to info level after benchmark + manager.SetLogLevel(log.InfoLevel) + }() + + // Setup DNAT mappings + for i := 0; i < count; i++ { + originalIP := netip.MustParseAddr(fmt.Sprintf("192.168.%d.%d", (i/254)+1, (i%254)+1)) + translatedIP := netip.MustParseAddr(fmt.Sprintf("10.0.%d.%d", (i/254)+1, (i%254)+1)) + err := manager.AddInternalDNATMapping(originalIP, translatedIP) + require.NoError(b, err) + } + + // Test with the last mapping added (worst case for lookup) + srcIP := netip.MustParseAddr("172.16.0.1") + lastOriginal := netip.MustParseAddr(fmt.Sprintf("192.168.%d.%d", ((count-1)/254)+1, ((count-1)%254)+1)) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + packet := generateDNATTestPacket(b, srcIP, lastOriginal, layers.IPProtocolTCP, 12345, 80) + manager.filterOutbound(packet, 0) + } + }) + } +} + +// generateDNATTestPacket creates a test packet for DNAT benchmarking +func generateDNATTestPacket(tb testing.TB, srcIP, dstIP netip.Addr, proto layers.IPProtocol, srcPort, dstPort uint16) []byte { + tb.Helper() + + ipv4 := &layers.IPv4{ + TTL: 64, + Version: 4, + SrcIP: srcIP.AsSlice(), + DstIP: dstIP.AsSlice(), + Protocol: proto, + } + + var transportLayer gopacket.SerializableLayer + switch proto { + case layers.IPProtocolTCP: + tcp := &layers.TCP{ + SrcPort: layers.TCPPort(srcPort), + DstPort: layers.TCPPort(dstPort), + SYN: true, + } + require.NoError(tb, tcp.SetNetworkLayerForChecksum(ipv4)) + transportLayer = tcp + case layers.IPProtocolUDP: + udp := &layers.UDP{ + SrcPort: layers.UDPPort(srcPort), + DstPort: layers.UDPPort(dstPort), + } + require.NoError(tb, udp.SetNetworkLayerForChecksum(ipv4)) + transportLayer = udp + case layers.IPProtocolICMPv4: + icmp := &layers.ICMPv4{ + TypeCode: layers.CreateICMPv4TypeCode(layers.ICMPv4TypeEchoRequest, 0), + } + transportLayer = icmp + } + + buf := gopacket.NewSerializeBuffer() + opts := gopacket.SerializeOptions{ComputeChecksums: true, FixLengths: true} + err := gopacket.SerializeLayers(buf, opts, ipv4, transportLayer, gopacket.Payload("test")) + require.NoError(tb, err) + return buf.Bytes() +} + +// BenchmarkChecksumUpdate specifically benchmarks checksum calculation performance +func BenchmarkChecksumUpdate(b *testing.B) { + // Create test data for checksum calculations + testData := make([]byte, 64) // Typical packet size for checksum testing + for i := range testData { + testData[i] = byte(i) + } + + b.Run("ipv4_checksum", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = ipv4Checksum(testData[:20]) // IPv4 header is typically 20 bytes + } + }) + + b.Run("icmp_checksum", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = icmpChecksum(testData) + } + }) + + b.Run("incremental_update", func(b *testing.B) { + oldBytes := []byte{192, 168, 1, 100} + newBytes := []byte{10, 0, 0, 100} + oldChecksum := uint16(0x1234) + + for i := 0; i < b.N; i++ { + _ = incrementalUpdate(oldChecksum, oldBytes, newBytes) + } + }) +} + +// BenchmarkDNATMemoryAllocations checks for memory allocations in DNAT operations +func BenchmarkDNATMemoryAllocations(b *testing.B) { + manager, err := Create(&IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, false, flowLogger) + require.NoError(b, err) + defer func() { + require.NoError(b, manager.Close(nil)) + }() + + // Set logger to error level to reduce noise during benchmarking + manager.SetLogLevel(log.ErrorLevel) + defer func() { + // Restore to info level after benchmark + manager.SetLogLevel(log.InfoLevel) + }() + + originalIP := netip.MustParseAddr("192.168.1.100") + translatedIP := netip.MustParseAddr("10.0.0.100") + srcIP := netip.MustParseAddr("172.16.0.1") + + err = manager.AddInternalDNATMapping(originalIP, translatedIP) + require.NoError(b, err) + + packet := generateDNATTestPacket(b, srcIP, originalIP, layers.IPProtocolTCP, 12345, 80) + + b.ResetTimer() + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + // Create fresh packet each time to isolate allocation testing + testPacket := make([]byte, len(packet)) + copy(testPacket, packet) + + // Parse the packet fresh each time to get a clean decoder + d := &decoder{decoded: []gopacket.LayerType{}} + d.parser = gopacket.NewDecodingLayerParser( + layers.LayerTypeIPv4, + &d.eth, &d.ip4, &d.ip6, &d.icmp4, &d.icmp6, &d.tcp, &d.udp, + ) + d.parser.IgnoreUnsupported = true + err = d.parser.DecodeLayers(testPacket, &d.decoded) + assert.NoError(b, err) + + manager.translateOutboundDNAT(testPacket, d) + } +} + +// BenchmarkDirectIPExtraction tests the performance improvement of direct IP extraction +func BenchmarkDirectIPExtraction(b *testing.B) { + // Create a test packet + srcIP := netip.MustParseAddr("172.16.0.1") + dstIP := netip.MustParseAddr("192.168.1.100") + packet := generateDNATTestPacket(b, srcIP, dstIP, layers.IPProtocolTCP, 12345, 80) + + b.Run("direct_byte_access", func(b *testing.B) { + for i := 0; i < b.N; i++ { + // Direct extraction from packet bytes + _ = netip.AddrFrom4([4]byte{packet[16], packet[17], packet[18], packet[19]}) + } + }) + + b.Run("decoder_extraction", func(b *testing.B) { + // Create decoder once for comparison + d := &decoder{decoded: []gopacket.LayerType{}} + d.parser = gopacket.NewDecodingLayerParser( + layers.LayerTypeIPv4, + &d.eth, &d.ip4, &d.ip6, &d.icmp4, &d.icmp6, &d.tcp, &d.udp, + ) + d.parser.IgnoreUnsupported = true + err := d.parser.DecodeLayers(packet, &d.decoded) + assert.NoError(b, err) + + for i := 0; i < b.N; i++ { + // Extract using decoder (traditional method) + dst, _ := netip.AddrFromSlice(d.ip4.DstIP) + _ = dst + } + }) +} + +// BenchmarkChecksumOptimizations compares optimized vs standard checksum implementations +func BenchmarkChecksumOptimizations(b *testing.B) { + // Create test IPv4 header (20 bytes) + header := make([]byte, 20) + for i := range header { + header[i] = byte(i) + } + // Clear checksum field + header[10] = 0 + header[11] = 0 + + b.Run("optimized_ipv4_checksum", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = ipv4Checksum(header) + } + }) + + // Test incremental checksum updates + oldIP := []byte{192, 168, 1, 100} + newIP := []byte{10, 0, 0, 100} + oldChecksum := uint16(0x1234) + + b.Run("optimized_incremental_update", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = incrementalUpdate(oldChecksum, oldIP, newIP) + } + }) +} diff --git a/client/firewall/uspfilter/nat_test.go b/client/firewall/uspfilter/nat_test.go new file mode 100644 index 000000000..710abd445 --- /dev/null +++ b/client/firewall/uspfilter/nat_test.go @@ -0,0 +1,145 @@ +package uspfilter + +import ( + "net/netip" + "testing" + + "github.com/google/gopacket" + "github.com/google/gopacket/layers" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/client/iface/device" +) + +// TestDNATTranslationCorrectness verifies DNAT translation works correctly +func TestDNATTranslationCorrectness(t *testing.T) { + manager, err := Create(&IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, false, flowLogger) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close(nil)) + }() + + originalIP := netip.MustParseAddr("192.168.1.100") + translatedIP := netip.MustParseAddr("10.0.0.100") + srcIP := netip.MustParseAddr("172.16.0.1") + + // Add DNAT mapping + err = manager.AddInternalDNATMapping(originalIP, translatedIP) + require.NoError(t, err) + + testCases := []struct { + name string + protocol layers.IPProtocol + srcPort uint16 + dstPort uint16 + }{ + {"TCP", layers.IPProtocolTCP, 12345, 80}, + {"UDP", layers.IPProtocolUDP, 12345, 53}, + {"ICMP", layers.IPProtocolICMPv4, 0, 0}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Test outbound DNAT translation + outboundPacket := generateDNATTestPacket(t, srcIP, originalIP, tc.protocol, tc.srcPort, tc.dstPort) + originalOutbound := make([]byte, len(outboundPacket)) + copy(originalOutbound, outboundPacket) + + // Process outbound packet (should translate destination) + translated := manager.translateOutboundDNAT(outboundPacket, parsePacket(t, outboundPacket)) + require.True(t, translated, "Outbound packet should be translated") + + // Verify destination IP was changed + dstIPAfter := netip.AddrFrom4([4]byte{outboundPacket[16], outboundPacket[17], outboundPacket[18], outboundPacket[19]}) + require.Equal(t, translatedIP, dstIPAfter, "Destination IP should be translated") + + // Test inbound reverse DNAT translation + inboundPacket := generateDNATTestPacket(t, translatedIP, srcIP, tc.protocol, tc.dstPort, tc.srcPort) + originalInbound := make([]byte, len(inboundPacket)) + copy(originalInbound, inboundPacket) + + // Process inbound packet (should reverse translate source) + reversed := manager.translateInboundReverse(inboundPacket, parsePacket(t, inboundPacket)) + require.True(t, reversed, "Inbound packet should be reverse translated") + + // Verify source IP was changed back to original + srcIPAfter := netip.AddrFrom4([4]byte{inboundPacket[12], inboundPacket[13], inboundPacket[14], inboundPacket[15]}) + require.Equal(t, originalIP, srcIPAfter, "Source IP should be reverse translated") + + // Test that checksums are recalculated correctly + if tc.protocol != layers.IPProtocolICMPv4 { + // For TCP/UDP, verify the transport checksum was updated + require.NotEqual(t, originalOutbound, outboundPacket, "Outbound packet should be modified") + require.NotEqual(t, originalInbound, inboundPacket, "Inbound packet should be modified") + } + }) + } +} + +// parsePacket helper to create a decoder for testing +func parsePacket(t testing.TB, packetData []byte) *decoder { + t.Helper() + d := &decoder{ + decoded: []gopacket.LayerType{}, + } + d.parser = gopacket.NewDecodingLayerParser( + layers.LayerTypeIPv4, + &d.eth, &d.ip4, &d.ip6, &d.icmp4, &d.icmp6, &d.tcp, &d.udp, + ) + d.parser.IgnoreUnsupported = true + + err := d.parser.DecodeLayers(packetData, &d.decoded) + require.NoError(t, err) + return d +} + +// TestDNATMappingManagement tests adding/removing DNAT mappings +func TestDNATMappingManagement(t *testing.T) { + manager, err := Create(&IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, false, flowLogger) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close(nil)) + }() + + originalIP := netip.MustParseAddr("192.168.1.100") + translatedIP := netip.MustParseAddr("10.0.0.100") + + // Test adding mapping + err = manager.AddInternalDNATMapping(originalIP, translatedIP) + require.NoError(t, err) + + // Verify mapping exists + result, exists := manager.getDNATTranslation(originalIP) + require.True(t, exists) + require.Equal(t, translatedIP, result) + + // Test reverse lookup + reverseResult, exists := manager.findReverseDNATMapping(translatedIP) + require.True(t, exists) + require.Equal(t, originalIP, reverseResult) + + // Test removing mapping + err = manager.RemoveInternalDNATMapping(originalIP) + require.NoError(t, err) + + // Verify mapping no longer exists + _, exists = manager.getDNATTranslation(originalIP) + require.False(t, exists) + + _, exists = manager.findReverseDNATMapping(translatedIP) + require.False(t, exists) + + // Test error cases + err = manager.AddInternalDNATMapping(netip.Addr{}, translatedIP) + require.Error(t, err, "Should reject invalid original IP") + + err = manager.AddInternalDNATMapping(originalIP, netip.Addr{}) + require.Error(t, err, "Should reject invalid translated IP") + + err = manager.RemoveInternalDNATMapping(originalIP) + require.Error(t, err, "Should error when removing non-existent mapping") +} diff --git a/client/firewall/uspfilter/tracer.go b/client/firewall/uspfilter/tracer.go index 53350797c..ef04f2700 100644 --- a/client/firewall/uspfilter/tracer.go +++ b/client/firewall/uspfilter/tracer.go @@ -401,7 +401,7 @@ func (m *Manager) addForwardingResult(trace *PacketTrace, action, remoteAddr str func (m *Manager) traceOutbound(packetData []byte, trace *PacketTrace) *PacketTrace { // will create or update the connection state - dropped := m.processOutgoingHooks(packetData, 0) + dropped := m.filterOutbound(packetData, 0) if dropped { trace.AddResult(StageCompleted, "Packet dropped by outgoing hook", false) } else { diff --git a/client/iface/device/device_filter.go b/client/iface/device/device_filter.go index 5a1a0e96a..015f71ff4 100644 --- a/client/iface/device/device_filter.go +++ b/client/iface/device/device_filter.go @@ -9,11 +9,11 @@ import ( // PacketFilter interface for firewall abilities type PacketFilter interface { - // DropOutgoing filter outgoing packets from host to external destinations - DropOutgoing(packetData []byte, size int) bool + // FilterOutbound filter outgoing packets from host to external destinations + FilterOutbound(packetData []byte, size int) bool - // DropIncoming filter incoming packets from external sources to host - DropIncoming(packetData []byte, size int) bool + // FilterInbound filter incoming packets from external sources to host + FilterInbound(packetData []byte, size int) bool // AddUDPPacketHook calls hook when UDP packet from given direction matched // @@ -54,7 +54,7 @@ func (d *FilteredDevice) Read(bufs [][]byte, sizes []int, offset int) (n int, er } for i := 0; i < n; i++ { - if filter.DropOutgoing(bufs[i][offset:offset+sizes[i]], sizes[i]) { + if filter.FilterOutbound(bufs[i][offset:offset+sizes[i]], sizes[i]) { bufs = append(bufs[:i], bufs[i+1:]...) sizes = append(sizes[:i], sizes[i+1:]...) n-- @@ -78,7 +78,7 @@ func (d *FilteredDevice) Write(bufs [][]byte, offset int) (int, error) { filteredBufs := make([][]byte, 0, len(bufs)) dropped := 0 for _, buf := range bufs { - if !filter.DropIncoming(buf[offset:], len(buf)) { + if !filter.FilterInbound(buf[offset:], len(buf)) { filteredBufs = append(filteredBufs, buf) dropped++ } diff --git a/client/iface/device/device_filter_test.go b/client/iface/device/device_filter_test.go index c90269e82..eef783542 100644 --- a/client/iface/device/device_filter_test.go +++ b/client/iface/device/device_filter_test.go @@ -146,7 +146,7 @@ func TestDeviceWrapperRead(t *testing.T) { tun.EXPECT().Write(mockBufs, 0).Return(0, nil) filter := mocks.NewMockPacketFilter(ctrl) - filter.EXPECT().DropIncoming(gomock.Any(), gomock.Any()).Return(true) + filter.EXPECT().FilterInbound(gomock.Any(), gomock.Any()).Return(true) wrapped := newDeviceFilter(tun) wrapped.filter = filter @@ -201,7 +201,7 @@ func TestDeviceWrapperRead(t *testing.T) { return 1, nil }) filter := mocks.NewMockPacketFilter(ctrl) - filter.EXPECT().DropOutgoing(gomock.Any(), gomock.Any()).Return(true) + filter.EXPECT().FilterOutbound(gomock.Any(), gomock.Any()).Return(true) wrapped := newDeviceFilter(tun) wrapped.filter = filter diff --git a/client/iface/mocks/filter.go b/client/iface/mocks/filter.go index 8cd2a1231..566068aa5 100644 --- a/client/iface/mocks/filter.go +++ b/client/iface/mocks/filter.go @@ -48,32 +48,32 @@ func (mr *MockPacketFilterMockRecorder) AddUDPPacketHook(arg0, arg1, arg2, arg3 return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddUDPPacketHook", reflect.TypeOf((*MockPacketFilter)(nil).AddUDPPacketHook), arg0, arg1, arg2, arg3) } -// DropIncoming mocks base method. -func (m *MockPacketFilter) DropIncoming(arg0 []byte, arg1 int) bool { +// FilterInbound mocks base method. +func (m *MockPacketFilter) FilterInbound(arg0 []byte, arg1 int) bool { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DropIncoming", arg0, arg1) + ret := m.ctrl.Call(m, "FilterInbound", arg0, arg1) ret0, _ := ret[0].(bool) return ret0 } -// DropIncoming indicates an expected call of DropIncoming. -func (mr *MockPacketFilterMockRecorder) DropIncoming(arg0 interface{}, arg1 any) *gomock.Call { +// FilterInbound indicates an expected call of FilterInbound. +func (mr *MockPacketFilterMockRecorder) FilterInbound(arg0 interface{}, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DropIncoming", reflect.TypeOf((*MockPacketFilter)(nil).DropIncoming), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FilterInbound", reflect.TypeOf((*MockPacketFilter)(nil).FilterInbound), arg0, arg1) } -// DropOutgoing mocks base method. -func (m *MockPacketFilter) DropOutgoing(arg0 []byte, arg1 int) bool { +// FilterOutbound mocks base method. +func (m *MockPacketFilter) FilterOutbound(arg0 []byte, arg1 int) bool { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DropOutgoing", arg0, arg1) + ret := m.ctrl.Call(m, "FilterOutbound", arg0, arg1) ret0, _ := ret[0].(bool) return ret0 } -// DropOutgoing indicates an expected call of DropOutgoing. -func (mr *MockPacketFilterMockRecorder) DropOutgoing(arg0 interface{}, arg1 any) *gomock.Call { +// FilterOutbound indicates an expected call of FilterOutbound. +func (mr *MockPacketFilterMockRecorder) FilterOutbound(arg0 interface{}, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DropOutgoing", reflect.TypeOf((*MockPacketFilter)(nil).DropOutgoing), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FilterOutbound", reflect.TypeOf((*MockPacketFilter)(nil).FilterOutbound), arg0, arg1) } // RemovePacketHook mocks base method. diff --git a/client/iface/mocks/iface/mocks/filter.go b/client/iface/mocks/iface/mocks/filter.go index 17e123abb..291ab9ab5 100644 --- a/client/iface/mocks/iface/mocks/filter.go +++ b/client/iface/mocks/iface/mocks/filter.go @@ -46,32 +46,32 @@ func (mr *MockPacketFilterMockRecorder) AddUDPPacketHook(arg0, arg1, arg2, arg3 return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddUDPPacketHook", reflect.TypeOf((*MockPacketFilter)(nil).AddUDPPacketHook), arg0, arg1, arg2, arg3) } -// DropIncoming mocks base method. -func (m *MockPacketFilter) DropIncoming(arg0 []byte) bool { +// FilterInbound mocks base method. +func (m *MockPacketFilter) FilterInbound(arg0 []byte) bool { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DropIncoming", arg0) + ret := m.ctrl.Call(m, "FilterInbound", arg0) ret0, _ := ret[0].(bool) return ret0 } -// DropIncoming indicates an expected call of DropIncoming. -func (mr *MockPacketFilterMockRecorder) DropIncoming(arg0 interface{}) *gomock.Call { +// FilterInbound indicates an expected call of FilterInbound. +func (mr *MockPacketFilterMockRecorder) FilterInbound(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DropIncoming", reflect.TypeOf((*MockPacketFilter)(nil).DropIncoming), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FilterInbound", reflect.TypeOf((*MockPacketFilter)(nil).FilterInbound), arg0) } -// DropOutgoing mocks base method. -func (m *MockPacketFilter) DropOutgoing(arg0 []byte) bool { +// FilterOutbound mocks base method. +func (m *MockPacketFilter) FilterOutbound(arg0 []byte) bool { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DropOutgoing", arg0) + ret := m.ctrl.Call(m, "FilterOutbound", arg0) ret0, _ := ret[0].(bool) return ret0 } -// DropOutgoing indicates an expected call of DropOutgoing. -func (mr *MockPacketFilterMockRecorder) DropOutgoing(arg0 interface{}) *gomock.Call { +// FilterOutbound indicates an expected call of FilterOutbound. +func (mr *MockPacketFilterMockRecorder) FilterOutbound(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DropOutgoing", reflect.TypeOf((*MockPacketFilter)(nil).DropOutgoing), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FilterOutbound", reflect.TypeOf((*MockPacketFilter)(nil).FilterOutbound), arg0) } // SetNetwork mocks base method. diff --git a/client/internal/dns/server_test.go b/client/internal/dns/server_test.go index 1cf59fb5b..21a9e2f2d 100644 --- a/client/internal/dns/server_test.go +++ b/client/internal/dns/server_test.go @@ -464,7 +464,7 @@ func TestDNSFakeResolverHandleUpdates(t *testing.T) { defer ctrl.Finish() packetfilter := pfmock.NewMockPacketFilter(ctrl) - packetfilter.EXPECT().DropOutgoing(gomock.Any(), gomock.Any()).AnyTimes() + packetfilter.EXPECT().FilterOutbound(gomock.Any(), gomock.Any()).AnyTimes() packetfilter.EXPECT().AddUDPPacketHook(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) packetfilter.EXPECT().RemovePacketHook(gomock.Any()) diff --git a/client/internal/engine.go b/client/internal/engine.go index 4ea6fbd94..74d84569a 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -383,7 +383,13 @@ func (e *Engine) Start() error { } e.stateManager.Start() - initialRoutes, dnsServer, err := e.newDnsServer() + initialRoutes, dnsConfig, dnsFeatureFlag, err := e.readInitialSettings() + if err != nil { + e.close() + return fmt.Errorf("read initial settings: %w", err) + } + + dnsServer, err := e.newDnsServer(dnsConfig) if err != nil { e.close() return fmt.Errorf("create dns server: %w", err) @@ -400,6 +406,7 @@ func (e *Engine) Start() error { InitialRoutes: initialRoutes, StateManager: e.stateManager, DNSServer: dnsServer, + DNSFeatureFlag: dnsFeatureFlag, PeerStore: e.peerStore, DisableClientRoutes: e.config.DisableClientRoutes, DisableServerRoutes: e.config.DisableServerRoutes, @@ -488,9 +495,9 @@ func (e *Engine) createFirewall() error { } func (e *Engine) initFirewall() error { - if err := e.routeManager.EnableServerRouter(e.firewall); err != nil { + if err := e.routeManager.SetFirewall(e.firewall); err != nil { e.close() - return fmt.Errorf("enable server router: %w", err) + return fmt.Errorf("set firewall: %w", err) } if e.config.BlockLANAccess { @@ -1009,8 +1016,6 @@ func (e *Engine) updateNetworkMap(networkMap *mgmProto.NetworkMap) error { log.Errorf("failed to update dns server, err: %v", err) } - dnsRouteFeatureFlag := toDNSFeatureFlag(networkMap) - // apply routes first, route related actions might depend on routing being enabled routes := toRoutes(networkMap.GetRoutes()) serverRoutes, clientRoutes := e.routeManager.ClassifyRoutes(routes) @@ -1021,6 +1026,7 @@ func (e *Engine) updateNetworkMap(networkMap *mgmProto.NetworkMap) error { log.Debugf("updated lazy connection manager with %d HA groups", len(clientRoutes)) } + dnsRouteFeatureFlag := toDNSFeatureFlag(networkMap) if err := e.routeManager.UpdateRoutes(serial, serverRoutes, clientRoutes, dnsRouteFeatureFlag); err != nil { log.Errorf("failed to update routes: %v", err) } @@ -1489,7 +1495,12 @@ func (e *Engine) close() { } } -func (e *Engine) readInitialSettings() ([]*route.Route, *nbdns.Config, error) { +func (e *Engine) readInitialSettings() ([]*route.Route, *nbdns.Config, bool, error) { + if runtime.GOOS != "android" { + // nolint:nilnil + return nil, nil, false, nil + } + info := system.GetInfo(e.ctx) info.SetFlags( e.config.RosenpassEnabled, @@ -1506,11 +1517,12 @@ func (e *Engine) readInitialSettings() ([]*route.Route, *nbdns.Config, error) { netMap, err := e.mgmClient.GetNetworkMap(info) if err != nil { - return nil, nil, err + return nil, nil, false, err } routes := toRoutes(netMap.GetRoutes()) dnsCfg := toDNSConfig(netMap.GetDNSConfig(), e.wgInterface.Address().Network) - return routes, &dnsCfg, nil + dnsFeatureFlag := toDNSFeatureFlag(netMap) + return routes, &dnsCfg, dnsFeatureFlag, nil } func (e *Engine) newWgIface() (*iface.WGIface, error) { @@ -1558,18 +1570,14 @@ func (e *Engine) wgInterfaceCreate() (err error) { return err } -func (e *Engine) newDnsServer() ([]*route.Route, dns.Server, error) { +func (e *Engine) newDnsServer(dnsConfig *nbdns.Config) (dns.Server, error) { // due to tests where we are using a mocked version of the DNS server if e.dnsServer != nil { - return nil, e.dnsServer, nil + return e.dnsServer, nil } switch runtime.GOOS { case "android": - routes, dnsConfig, err := e.readInitialSettings() - if err != nil { - return nil, nil, err - } dnsServer := dns.NewDefaultServerPermanentUpstream( e.ctx, e.wgInterface, @@ -1580,19 +1588,19 @@ func (e *Engine) newDnsServer() ([]*route.Route, dns.Server, error) { e.config.DisableDNS, ) go e.mobileDep.DnsReadyListener.OnReady() - return routes, dnsServer, nil + return dnsServer, nil case "ios": dnsServer := dns.NewDefaultServerIos(e.ctx, e.wgInterface, e.mobileDep.DnsManager, e.statusRecorder, e.config.DisableDNS) - return nil, dnsServer, nil + return dnsServer, nil default: dnsServer, err := dns.NewDefaultServer(e.ctx, e.wgInterface, e.config.CustomDNSAddress, e.statusRecorder, e.stateManager, e.config.DisableDNS) if err != nil { - return nil, nil, err + return nil, err } - return nil, dnsServer, nil + return dnsServer, nil } } diff --git a/client/internal/routemanager/client/client.go b/client/internal/routemanager/client/client.go index 46bff96db..0b8e161d2 100644 --- a/client/internal/routemanager/client/client.go +++ b/client/internal/routemanager/client/client.go @@ -10,11 +10,10 @@ import ( nbdns "github.com/netbirdio/netbird/client/internal/dns" "github.com/netbirdio/netbird/client/internal/peer" - "github.com/netbirdio/netbird/client/internal/peerstore" + "github.com/netbirdio/netbird/client/internal/routemanager/common" "github.com/netbirdio/netbird/client/internal/routemanager/dnsinterceptor" "github.com/netbirdio/netbird/client/internal/routemanager/dynamic" "github.com/netbirdio/netbird/client/internal/routemanager/iface" - "github.com/netbirdio/netbird/client/internal/routemanager/refcounter" "github.com/netbirdio/netbird/client/internal/routemanager/static" "github.com/netbirdio/netbird/client/proto" "github.com/netbirdio/netbird/route" @@ -553,41 +552,16 @@ func (w *Watcher) Stop() { w.currentChosenStatus = nil } -func HandlerFromRoute( - rt *route.Route, - routeRefCounter *refcounter.RouteRefCounter, - allowedIPsRefCounter *refcounter.AllowedIPsRefCounter, - dnsRouterInteval time.Duration, - statusRecorder *peer.Status, - wgInterface iface.WGIface, - dnsServer nbdns.Server, - peerStore *peerstore.Store, - useNewDNSRoute bool, -) RouteHandler { - switch handlerType(rt, useNewDNSRoute) { +func HandlerFromRoute(params common.HandlerParams) RouteHandler { + switch handlerType(params.Route, params.UseNewDNSRoute) { case handlerTypeDnsInterceptor: - return dnsinterceptor.New( - rt, - routeRefCounter, - allowedIPsRefCounter, - statusRecorder, - dnsServer, - wgInterface, - peerStore, - ) + return dnsinterceptor.New(params) case handlerTypeDynamic: - dns := nbdns.NewServiceViaMemory(wgInterface) - return dynamic.NewRoute( - rt, - routeRefCounter, - allowedIPsRefCounter, - dnsRouterInteval, - statusRecorder, - wgInterface, - fmt.Sprintf("%s:%d", dns.RuntimeIP(), dns.RuntimePort()), - ) + dns := nbdns.NewServiceViaMemory(params.WgInterface) + dnsAddr := fmt.Sprintf("%s:%d", dns.RuntimeIP(), dns.RuntimePort()) + return dynamic.NewRoute(params, dnsAddr) default: - return static.NewRoute(rt, routeRefCounter, allowedIPsRefCounter) + return static.NewRoute(params) } } diff --git a/client/internal/routemanager/client/client_test.go b/client/internal/routemanager/client/client_test.go index e7aff28b6..ec8e0e944 100644 --- a/client/internal/routemanager/client/client_test.go +++ b/client/internal/routemanager/client/client_test.go @@ -7,12 +7,12 @@ import ( "time" "github.com/netbirdio/netbird/client/internal/peer" + "github.com/netbirdio/netbird/client/internal/routemanager/common" "github.com/netbirdio/netbird/client/internal/routemanager/static" "github.com/netbirdio/netbird/route" ) func TestGetBestrouteFromStatuses(t *testing.T) { - testCases := []struct { name string statuses map[route.ID]routerPeerStatus @@ -811,9 +811,12 @@ func TestGetBestrouteFromStatuses(t *testing.T) { currentRoute = tc.existingRoutes[tc.currentRoute] } + params := common.HandlerParams{ + Route: &route.Route{Network: netip.MustParsePrefix("192.168.0.0/24")}, + } // create new clientNetwork client := &Watcher{ - handler: static.NewRoute(&route.Route{Network: netip.MustParsePrefix("192.168.0.0/24")}, nil, nil), + handler: static.NewRoute(params), routes: tc.existingRoutes, currentChosen: currentRoute, } diff --git a/client/internal/routemanager/common/params.go b/client/internal/routemanager/common/params.go new file mode 100644 index 000000000..def18411f --- /dev/null +++ b/client/internal/routemanager/common/params.go @@ -0,0 +1,28 @@ +package common + +import ( + "time" + + "github.com/netbirdio/netbird/client/firewall/manager" + "github.com/netbirdio/netbird/client/internal/dns" + "github.com/netbirdio/netbird/client/internal/peer" + "github.com/netbirdio/netbird/client/internal/peerstore" + "github.com/netbirdio/netbird/client/internal/routemanager/fakeip" + "github.com/netbirdio/netbird/client/internal/routemanager/iface" + "github.com/netbirdio/netbird/client/internal/routemanager/refcounter" + "github.com/netbirdio/netbird/route" +) + +type HandlerParams struct { + Route *route.Route + RouteRefCounter *refcounter.RouteRefCounter + AllowedIPsRefCounter *refcounter.AllowedIPsRefCounter + DnsRouterInterval time.Duration + StatusRecorder *peer.Status + WgInterface iface.WGIface + DnsServer dns.Server + PeerStore *peerstore.Store + UseNewDNSRoute bool + Firewall manager.Manager + FakeIPManager *fakeip.Manager +} diff --git a/client/internal/routemanager/dnsinterceptor/handler.go b/client/internal/routemanager/dnsinterceptor/handler.go index 66557e888..c7c3aeb0b 100644 --- a/client/internal/routemanager/dnsinterceptor/handler.go +++ b/client/internal/routemanager/dnsinterceptor/handler.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/netip" + "runtime" "strings" "sync" @@ -12,11 +13,14 @@ import ( log "github.com/sirupsen/logrus" nberrors "github.com/netbirdio/netbird/client/errors" + firewall "github.com/netbirdio/netbird/client/firewall/manager" "github.com/netbirdio/netbird/client/iface/wgaddr" nbdns "github.com/netbirdio/netbird/client/internal/dns" "github.com/netbirdio/netbird/client/internal/dnsfwd" "github.com/netbirdio/netbird/client/internal/peer" "github.com/netbirdio/netbird/client/internal/peerstore" + "github.com/netbirdio/netbird/client/internal/routemanager/common" + "github.com/netbirdio/netbird/client/internal/routemanager/fakeip" "github.com/netbirdio/netbird/client/internal/routemanager/refcounter" "github.com/netbirdio/netbird/management/domain" "github.com/netbirdio/netbird/route" @@ -24,6 +28,11 @@ import ( type domainMap map[domain.Domain][]netip.Prefix +type internalDNATer interface { + RemoveInternalDNATMapping(netip.Addr) error + AddInternalDNATMapping(netip.Addr, netip.Addr) error +} + type wgInterface interface { Name() string Address() wgaddr.Address @@ -40,26 +49,22 @@ type DnsInterceptor struct { interceptedDomains domainMap wgInterface wgInterface peerStore *peerstore.Store + firewall firewall.Manager + fakeIPManager *fakeip.Manager } -func New( - rt *route.Route, - routeRefCounter *refcounter.RouteRefCounter, - allowedIPsRefCounter *refcounter.AllowedIPsRefCounter, - statusRecorder *peer.Status, - dnsServer nbdns.Server, - wgInterface wgInterface, - peerStore *peerstore.Store, -) *DnsInterceptor { +func New(params common.HandlerParams) *DnsInterceptor { return &DnsInterceptor{ - route: rt, - routeRefCounter: routeRefCounter, - allowedIPsRefcounter: allowedIPsRefCounter, - statusRecorder: statusRecorder, - dnsServer: dnsServer, - wgInterface: wgInterface, + route: params.Route, + routeRefCounter: params.RouteRefCounter, + allowedIPsRefcounter: params.AllowedIPsRefCounter, + statusRecorder: params.StatusRecorder, + dnsServer: params.DnsServer, + wgInterface: params.WgInterface, + peerStore: params.PeerStore, + firewall: params.Firewall, + fakeIPManager: params.FakeIPManager, interceptedDomains: make(domainMap), - peerStore: peerStore, } } @@ -78,9 +83,13 @@ func (d *DnsInterceptor) RemoveRoute() error { var merr *multierror.Error for domain, prefixes := range d.interceptedDomains { for _, prefix := range prefixes { - if _, err := d.routeRefCounter.Decrement(prefix); err != nil { - merr = multierror.Append(merr, fmt.Errorf("remove dynamic route for IP %s: %v", prefix, err)) + // Routes should use fake IPs + routePrefix := d.transformRealToFakePrefix(prefix) + if _, err := d.routeRefCounter.Decrement(routePrefix); err != nil { + merr = multierror.Append(merr, fmt.Errorf("remove dynamic route for IP %s: %v", routePrefix, err)) } + + // AllowedIPs should use real IPs if d.currentPeerKey != "" { if _, err := d.allowedIPsRefcounter.Decrement(prefix); err != nil { merr = multierror.Append(merr, fmt.Errorf("remove allowed IP %s: %v", prefix, err)) @@ -88,8 +97,10 @@ func (d *DnsInterceptor) RemoveRoute() error { } } log.Debugf("removed dynamic route(s) for [%s]: %s", domain.SafeString(), strings.ReplaceAll(fmt.Sprintf("%s", prefixes), " ", ", ")) - } + + d.cleanupDNATMappings() + for _, domain := range d.route.Domains { d.statusRecorder.DeleteResolvedDomainsStates(domain) } @@ -102,6 +113,68 @@ func (d *DnsInterceptor) RemoveRoute() error { return nberrors.FormatErrorOrNil(merr) } +// transformRealToFakePrefix returns fake IP prefix for routes (if DNAT enabled) +func (d *DnsInterceptor) transformRealToFakePrefix(realPrefix netip.Prefix) netip.Prefix { + if _, hasDNAT := d.internalDnatFw(); !hasDNAT { + return realPrefix + } + + if fakeIP, ok := d.fakeIPManager.GetFakeIP(realPrefix.Addr()); ok { + return netip.PrefixFrom(fakeIP, realPrefix.Bits()) + } + + return realPrefix +} + +// addAllowedIPForPrefix handles the AllowedIPs logic for a single prefix (uses real IPs) +func (d *DnsInterceptor) addAllowedIPForPrefix(realPrefix netip.Prefix, peerKey string, domain domain.Domain) error { + // AllowedIPs always use real IPs + ref, err := d.allowedIPsRefcounter.Increment(realPrefix, peerKey) + if err != nil { + return fmt.Errorf("add allowed IP %s: %v", realPrefix, err) + } + + if ref.Count > 1 && ref.Out != peerKey { + log.Warnf("IP [%s] for domain [%s] is already routed by peer [%s]. HA routing disabled", + realPrefix.Addr(), + domain.SafeString(), + ref.Out, + ) + } + + return nil +} + +// addRouteAndAllowedIP handles both route and AllowedIPs addition for a prefix +func (d *DnsInterceptor) addRouteAndAllowedIP(realPrefix netip.Prefix, domain domain.Domain) error { + // Routes use fake IPs (so traffic to fake IPs gets routed to interface) + routePrefix := d.transformRealToFakePrefix(realPrefix) + if _, err := d.routeRefCounter.Increment(routePrefix, struct{}{}); err != nil { + return fmt.Errorf("add route for IP %s: %v", routePrefix, err) + } + + // Add to AllowedIPs if we have a current peer (uses real IPs) + if d.currentPeerKey == "" { + return nil + } + + return d.addAllowedIPForPrefix(realPrefix, d.currentPeerKey, domain) +} + +// removeAllowedIP handles AllowedIPs removal for a prefix (uses real IPs) +func (d *DnsInterceptor) removeAllowedIP(realPrefix netip.Prefix) error { + if d.currentPeerKey == "" { + return nil + } + + // AllowedIPs use real IPs + if _, err := d.allowedIPsRefcounter.Decrement(realPrefix); err != nil { + return fmt.Errorf("remove allowed IP %s: %v", realPrefix, err) + } + + return nil +} + func (d *DnsInterceptor) AddAllowedIPs(peerKey string) error { d.mu.Lock() defer d.mu.Unlock() @@ -109,14 +182,9 @@ func (d *DnsInterceptor) AddAllowedIPs(peerKey string) error { var merr *multierror.Error for domain, prefixes := range d.interceptedDomains { for _, prefix := range prefixes { - if ref, err := d.allowedIPsRefcounter.Increment(prefix, peerKey); err != nil { - merr = multierror.Append(merr, fmt.Errorf("add allowed IP %s: %v", prefix, err)) - } else if ref.Count > 1 && ref.Out != peerKey { - log.Warnf("IP [%s] for domain [%s] is already routed by peer [%s]. HA routing disabled", - prefix.Addr(), - domain.SafeString(), - ref.Out, - ) + // AllowedIPs use real IPs + if err := d.addAllowedIPForPrefix(prefix, peerKey, domain); err != nil { + merr = multierror.Append(merr, err) } } } @@ -132,6 +200,7 @@ func (d *DnsInterceptor) RemoveAllowedIPs() error { var merr *multierror.Error for _, prefixes := range d.interceptedDomains { for _, prefix := range prefixes { + // AllowedIPs use real IPs if _, err := d.allowedIPsRefcounter.Decrement(prefix); err != nil { merr = multierror.Append(merr, fmt.Errorf("remove allowed IP %s: %v", prefix, err)) } @@ -287,6 +356,8 @@ func (d *DnsInterceptor) writeMsg(w dns.ResponseWriter, r *dns.Msg) error { if err := d.updateDomainPrefixes(resolvedDomain, originalDomain, newPrefixes); err != nil { log.Errorf("failed to update domain prefixes: %v", err) } + + d.replaceIPsInDNSResponse(r, newPrefixes) } } @@ -297,6 +368,22 @@ func (d *DnsInterceptor) writeMsg(w dns.ResponseWriter, r *dns.Msg) error { return nil } +// logPrefixChanges handles the logging for prefix changes +func (d *DnsInterceptor) logPrefixChanges(resolvedDomain, originalDomain domain.Domain, toAdd, toRemove []netip.Prefix) { + if len(toAdd) > 0 { + log.Debugf("added dynamic route(s) for domain=%s (pattern: domain=%s): %s", + resolvedDomain.SafeString(), + originalDomain.SafeString(), + toAdd) + } + if len(toRemove) > 0 && !d.route.KeepRoute { + log.Debugf("removed dynamic route(s) for domain=%s (pattern: domain=%s): %s", + resolvedDomain.SafeString(), + originalDomain.SafeString(), + toRemove) + } +} + func (d *DnsInterceptor) updateDomainPrefixes(resolvedDomain, originalDomain domain.Domain, newPrefixes []netip.Prefix) error { d.mu.Lock() defer d.mu.Unlock() @@ -305,70 +392,163 @@ func (d *DnsInterceptor) updateDomainPrefixes(resolvedDomain, originalDomain dom toAdd, toRemove := determinePrefixChanges(oldPrefixes, newPrefixes) var merr *multierror.Error + var dnatMappings map[netip.Addr]netip.Addr + + // Handle DNAT mappings for new prefixes + if _, hasDNAT := d.internalDnatFw(); hasDNAT { + dnatMappings = make(map[netip.Addr]netip.Addr) + for _, prefix := range toAdd { + realIP := prefix.Addr() + if fakeIP, err := d.fakeIPManager.AllocateFakeIP(realIP); err == nil { + dnatMappings[fakeIP] = realIP + log.Tracef("allocated fake IP %s for real IP %s", fakeIP, realIP) + } else { + log.Errorf("Failed to allocate fake IP for %s: %v", realIP, err) + } + } + } // Add new prefixes for _, prefix := range toAdd { - if _, err := d.routeRefCounter.Increment(prefix, struct{}{}); err != nil { - merr = multierror.Append(merr, fmt.Errorf("add route for IP %s: %v", prefix, err)) - continue - } - - if d.currentPeerKey == "" { - continue - } - if ref, err := d.allowedIPsRefcounter.Increment(prefix, d.currentPeerKey); err != nil { - merr = multierror.Append(merr, fmt.Errorf("add allowed IP %s: %v", prefix, err)) - } else if ref.Count > 1 && ref.Out != d.currentPeerKey { - log.Warnf("IP [%s] for domain [%s] is already routed by peer [%s]. HA routing disabled", - prefix.Addr(), - resolvedDomain.SafeString(), - ref.Out, - ) + if err := d.addRouteAndAllowedIP(prefix, resolvedDomain); err != nil { + merr = multierror.Append(merr, err) } } + d.addDNATMappings(dnatMappings) + if !d.route.KeepRoute { // Remove old prefixes for _, prefix := range toRemove { - if _, err := d.routeRefCounter.Decrement(prefix); err != nil { - merr = multierror.Append(merr, fmt.Errorf("remove route for IP %s: %v", prefix, err)) + // Routes use fake IPs + routePrefix := d.transformRealToFakePrefix(prefix) + if _, err := d.routeRefCounter.Decrement(routePrefix); err != nil { + merr = multierror.Append(merr, fmt.Errorf("remove route for IP %s: %v", routePrefix, err)) } - if d.currentPeerKey != "" { - if _, err := d.allowedIPsRefcounter.Decrement(prefix); err != nil { - merr = multierror.Append(merr, fmt.Errorf("remove allowed IP %s: %v", prefix, err)) - } + // AllowedIPs use real IPs + if err := d.removeAllowedIP(prefix); err != nil { + merr = multierror.Append(merr, err) } } + + d.removeDNATMappings(toRemove) } - // Update domain prefixes using resolved domain as key + // Update domain prefixes using resolved domain as key - store real IPs if len(toAdd) > 0 || len(toRemove) > 0 { if d.route.KeepRoute { - // replace stored prefixes with old + added // nolint:gocritic newPrefixes = append(oldPrefixes, toAdd...) } d.interceptedDomains[resolvedDomain] = newPrefixes originalDomain = domain.Domain(strings.TrimSuffix(string(originalDomain), ".")) + + // Store real IPs for status (user-facing), not fake IPs d.statusRecorder.UpdateResolvedDomainsStates(originalDomain, resolvedDomain, newPrefixes, d.route.GetResourceID()) - if len(toAdd) > 0 { - log.Debugf("added dynamic route(s) for domain=%s (pattern: domain=%s): %s", - resolvedDomain.SafeString(), - originalDomain.SafeString(), - toAdd) - } - if len(toRemove) > 0 && !d.route.KeepRoute { - log.Debugf("removed dynamic route(s) for domain=%s (pattern: domain=%s): %s", - resolvedDomain.SafeString(), - originalDomain.SafeString(), - toRemove) - } + d.logPrefixChanges(resolvedDomain, originalDomain, toAdd, toRemove) } return nberrors.FormatErrorOrNil(merr) } +// removeDNATMappings removes DNAT mappings from the firewall for real IP prefixes +func (d *DnsInterceptor) removeDNATMappings(realPrefixes []netip.Prefix) { + if len(realPrefixes) == 0 { + return + } + + dnatFirewall, ok := d.internalDnatFw() + if !ok { + return + } + + for _, prefix := range realPrefixes { + realIP := prefix.Addr() + if fakeIP, exists := d.fakeIPManager.GetFakeIP(realIP); exists { + if err := dnatFirewall.RemoveInternalDNATMapping(fakeIP); err != nil { + log.Errorf("Failed to remove DNAT mapping for %s: %v", fakeIP, err) + } else { + log.Debugf("Removed DNAT mapping for: %s -> %s", fakeIP, realIP) + } + } + } +} + +// internalDnatFw checks if the firewall supports internal DNAT +func (d *DnsInterceptor) internalDnatFw() (internalDNATer, bool) { + if d.firewall == nil || runtime.GOOS != "android" { + return nil, false + } + fw, ok := d.firewall.(internalDNATer) + return fw, ok +} + +// addDNATMappings adds DNAT mappings to the firewall +func (d *DnsInterceptor) addDNATMappings(mappings map[netip.Addr]netip.Addr) { + if len(mappings) == 0 { + return + } + + dnatFirewall, ok := d.internalDnatFw() + if !ok { + return + } + + for fakeIP, realIP := range mappings { + if err := dnatFirewall.AddInternalDNATMapping(fakeIP, realIP); err != nil { + log.Errorf("Failed to add DNAT mapping %s -> %s: %v", fakeIP, realIP, err) + } else { + log.Debugf("Added DNAT mapping: %s -> %s", fakeIP, realIP) + } + } +} + +// cleanupDNATMappings removes all DNAT mappings for this interceptor +func (d *DnsInterceptor) cleanupDNATMappings() { + if _, ok := d.internalDnatFw(); !ok { + return + } + + for _, prefixes := range d.interceptedDomains { + d.removeDNATMappings(prefixes) + } +} + +// replaceIPsInDNSResponse replaces real IPs with fake IPs in the DNS response +func (d *DnsInterceptor) replaceIPsInDNSResponse(reply *dns.Msg, realPrefixes []netip.Prefix) { + if _, ok := d.internalDnatFw(); !ok { + return + } + + // Replace A and AAAA records with fake IPs + for _, answer := range reply.Answer { + switch rr := answer.(type) { + case *dns.A: + realIP, ok := netip.AddrFromSlice(rr.A) + if !ok { + continue + } + + if fakeIP, exists := d.fakeIPManager.GetFakeIP(realIP); exists { + rr.A = fakeIP.AsSlice() + log.Tracef("Replaced real IP %s with fake IP %s in DNS response", realIP, fakeIP) + } + + case *dns.AAAA: + realIP, ok := netip.AddrFromSlice(rr.AAAA) + if !ok { + continue + } + + if fakeIP, exists := d.fakeIPManager.GetFakeIP(realIP); exists { + rr.AAAA = fakeIP.AsSlice() + log.Tracef("Replaced real IP %s with fake IP %s in DNS response", realIP, fakeIP) + } + } + } +} + func determinePrefixChanges(oldPrefixes, newPrefixes []netip.Prefix) (toAdd, toRemove []netip.Prefix) { prefixSet := make(map[netip.Prefix]bool) for _, prefix := range oldPrefixes { diff --git a/client/internal/routemanager/dynamic/route.go b/client/internal/routemanager/dynamic/route.go index 47511d4af..5d561f0cf 100644 --- a/client/internal/routemanager/dynamic/route.go +++ b/client/internal/routemanager/dynamic/route.go @@ -14,6 +14,7 @@ import ( nberrors "github.com/netbirdio/netbird/client/errors" "github.com/netbirdio/netbird/client/internal/peer" + "github.com/netbirdio/netbird/client/internal/routemanager/common" "github.com/netbirdio/netbird/client/internal/routemanager/iface" "github.com/netbirdio/netbird/client/internal/routemanager/refcounter" "github.com/netbirdio/netbird/client/internal/routemanager/util" @@ -52,24 +53,16 @@ type Route struct { resolverAddr string } -func NewRoute( - rt *route.Route, - routeRefCounter *refcounter.RouteRefCounter, - allowedIPsRefCounter *refcounter.AllowedIPsRefCounter, - interval time.Duration, - statusRecorder *peer.Status, - wgInterface iface.WGIface, - resolverAddr string, -) *Route { +func NewRoute(params common.HandlerParams, resolverAddr string) *Route { return &Route{ - route: rt, - routeRefCounter: routeRefCounter, - allowedIPsRefcounter: allowedIPsRefCounter, - interval: interval, - dynamicDomains: domainMap{}, - statusRecorder: statusRecorder, - wgInterface: wgInterface, + route: params.Route, + routeRefCounter: params.RouteRefCounter, + allowedIPsRefcounter: params.AllowedIPsRefCounter, + interval: params.DnsRouterInterval, + statusRecorder: params.StatusRecorder, + wgInterface: params.WgInterface, resolverAddr: resolverAddr, + dynamicDomains: domainMap{}, } } diff --git a/client/internal/routemanager/fakeip/fakeip.go b/client/internal/routemanager/fakeip/fakeip.go new file mode 100644 index 000000000..1592045d2 --- /dev/null +++ b/client/internal/routemanager/fakeip/fakeip.go @@ -0,0 +1,93 @@ +package fakeip + +import ( + "fmt" + "net/netip" + "sync" +) + +// Manager manages allocation of fake IPs from the 240.0.0.0/8 block +type Manager struct { + mu sync.Mutex + nextIP netip.Addr // Next IP to allocate + allocated map[netip.Addr]netip.Addr // real IP -> fake IP + fakeToReal map[netip.Addr]netip.Addr // fake IP -> real IP + baseIP netip.Addr // First usable IP: 240.0.0.1 + maxIP netip.Addr // Last usable IP: 240.255.255.254 +} + +// NewManager creates a new fake IP manager using 240.0.0.0/8 block +func NewManager() *Manager { + baseIP := netip.AddrFrom4([4]byte{240, 0, 0, 1}) + maxIP := netip.AddrFrom4([4]byte{240, 255, 255, 254}) + + return &Manager{ + nextIP: baseIP, + allocated: make(map[netip.Addr]netip.Addr), + fakeToReal: make(map[netip.Addr]netip.Addr), + baseIP: baseIP, + maxIP: maxIP, + } +} + +// AllocateFakeIP allocates a fake IP for the given real IP +// Returns the fake IP, or existing fake IP if already allocated +func (m *Manager) AllocateFakeIP(realIP netip.Addr) (netip.Addr, error) { + if !realIP.Is4() { + return netip.Addr{}, fmt.Errorf("only IPv4 addresses supported") + } + + m.mu.Lock() + defer m.mu.Unlock() + + if fakeIP, exists := m.allocated[realIP]; exists { + return fakeIP, nil + } + + startIP := m.nextIP + for { + currentIP := m.nextIP + + // Advance to next IP, wrapping at boundary + if m.nextIP.Compare(m.maxIP) >= 0 { + m.nextIP = m.baseIP + } else { + m.nextIP = m.nextIP.Next() + } + + // Check if current IP is available + if _, inUse := m.fakeToReal[currentIP]; !inUse { + m.allocated[realIP] = currentIP + m.fakeToReal[currentIP] = realIP + return currentIP, nil + } + + // Prevent infinite loop if all IPs exhausted + if m.nextIP.Compare(startIP) == 0 { + return netip.Addr{}, fmt.Errorf("no more fake IPs available in 240.0.0.0/8 block") + } + } +} + +// GetFakeIP returns the fake IP for a real IP if it exists +func (m *Manager) GetFakeIP(realIP netip.Addr) (netip.Addr, bool) { + m.mu.Lock() + defer m.mu.Unlock() + + fakeIP, exists := m.allocated[realIP] + return fakeIP, exists +} + +// GetRealIP returns the real IP for a fake IP if it exists, otherwise false +func (m *Manager) GetRealIP(fakeIP netip.Addr) (netip.Addr, bool) { + m.mu.Lock() + defer m.mu.Unlock() + + realIP, exists := m.fakeToReal[fakeIP] + return realIP, exists +} + +// GetFakeIPBlock returns the fake IP block used by this manager +func (m *Manager) GetFakeIPBlock() netip.Prefix { + return netip.MustParsePrefix("240.0.0.0/8") +} diff --git a/client/internal/routemanager/fakeip/fakeip_test.go b/client/internal/routemanager/fakeip/fakeip_test.go new file mode 100644 index 000000000..ad3e4bd4e --- /dev/null +++ b/client/internal/routemanager/fakeip/fakeip_test.go @@ -0,0 +1,240 @@ +package fakeip + +import ( + "net/netip" + "sync" + "testing" +) + +func TestNewManager(t *testing.T) { + manager := NewManager() + + if manager.baseIP.String() != "240.0.0.1" { + t.Errorf("Expected base IP 240.0.0.1, got %s", manager.baseIP.String()) + } + + if manager.maxIP.String() != "240.255.255.254" { + t.Errorf("Expected max IP 240.255.255.254, got %s", manager.maxIP.String()) + } + + if manager.nextIP.Compare(manager.baseIP) != 0 { + t.Errorf("Expected nextIP to start at baseIP") + } +} + +func TestAllocateFakeIP(t *testing.T) { + manager := NewManager() + realIP := netip.MustParseAddr("8.8.8.8") + + fakeIP, err := manager.AllocateFakeIP(realIP) + if err != nil { + t.Fatalf("Failed to allocate fake IP: %v", err) + } + + if !fakeIP.Is4() { + t.Error("Fake IP should be IPv4") + } + + // Check it's in the correct range + if fakeIP.As4()[0] != 240 { + t.Errorf("Fake IP should be in 240.0.0.0/8 range, got %s", fakeIP.String()) + } + + // Should return same fake IP for same real IP + fakeIP2, err := manager.AllocateFakeIP(realIP) + if err != nil { + t.Fatalf("Failed to get existing fake IP: %v", err) + } + + if fakeIP.Compare(fakeIP2) != 0 { + t.Errorf("Expected same fake IP for same real IP, got %s and %s", fakeIP.String(), fakeIP2.String()) + } +} + +func TestAllocateFakeIPIPv6Rejection(t *testing.T) { + manager := NewManager() + realIPv6 := netip.MustParseAddr("2001:db8::1") + + _, err := manager.AllocateFakeIP(realIPv6) + if err == nil { + t.Error("Expected error for IPv6 address") + } +} + +func TestGetFakeIP(t *testing.T) { + manager := NewManager() + realIP := netip.MustParseAddr("1.1.1.1") + + // Should not exist initially + _, exists := manager.GetFakeIP(realIP) + if exists { + t.Error("Fake IP should not exist before allocation") + } + + // Allocate and check + expectedFakeIP, err := manager.AllocateFakeIP(realIP) + if err != nil { + t.Fatalf("Failed to allocate: %v", err) + } + + fakeIP, exists := manager.GetFakeIP(realIP) + if !exists { + t.Error("Fake IP should exist after allocation") + } + + if fakeIP.Compare(expectedFakeIP) != 0 { + t.Errorf("Expected %s, got %s", expectedFakeIP.String(), fakeIP.String()) + } +} + +func TestMultipleAllocations(t *testing.T) { + manager := NewManager() + + allocations := make(map[netip.Addr]netip.Addr) + + // Allocate multiple IPs + for i := 1; i <= 100; i++ { + realIP := netip.AddrFrom4([4]byte{10, 0, byte(i / 256), byte(i % 256)}) + fakeIP, err := manager.AllocateFakeIP(realIP) + if err != nil { + t.Fatalf("Failed to allocate fake IP for %s: %v", realIP.String(), err) + } + + // Check for duplicates + for _, existingFake := range allocations { + if fakeIP.Compare(existingFake) == 0 { + t.Errorf("Duplicate fake IP allocated: %s", fakeIP.String()) + } + } + + allocations[realIP] = fakeIP + } + + // Verify all allocations can be retrieved + for realIP, expectedFake := range allocations { + actualFake, exists := manager.GetFakeIP(realIP) + if !exists { + t.Errorf("Missing allocation for %s", realIP.String()) + } + if actualFake.Compare(expectedFake) != 0 { + t.Errorf("Mismatch for %s: expected %s, got %s", realIP.String(), expectedFake.String(), actualFake.String()) + } + } +} + +func TestGetFakeIPBlock(t *testing.T) { + manager := NewManager() + block := manager.GetFakeIPBlock() + + expected := "240.0.0.0/8" + if block.String() != expected { + t.Errorf("Expected %s, got %s", expected, block.String()) + } +} + +func TestConcurrentAccess(t *testing.T) { + manager := NewManager() + + const numGoroutines = 50 + const allocationsPerGoroutine = 10 + + var wg sync.WaitGroup + results := make(chan netip.Addr, numGoroutines*allocationsPerGoroutine) + + // Concurrent allocations + for i := 0; i < numGoroutines; i++ { + wg.Add(1) + go func(goroutineID int) { + defer wg.Done() + for j := 0; j < allocationsPerGoroutine; j++ { + realIP := netip.AddrFrom4([4]byte{192, 168, byte(goroutineID), byte(j)}) + fakeIP, err := manager.AllocateFakeIP(realIP) + if err != nil { + t.Errorf("Failed to allocate in goroutine %d: %v", goroutineID, err) + return + } + results <- fakeIP + } + }(i) + } + + wg.Wait() + close(results) + + // Check for duplicates + seen := make(map[netip.Addr]bool) + count := 0 + for fakeIP := range results { + if seen[fakeIP] { + t.Errorf("Duplicate fake IP in concurrent test: %s", fakeIP.String()) + } + seen[fakeIP] = true + count++ + } + + if count != numGoroutines*allocationsPerGoroutine { + t.Errorf("Expected %d allocations, got %d", numGoroutines*allocationsPerGoroutine, count) + } +} + +func TestIPExhaustion(t *testing.T) { + // Create a manager with limited range for testing + manager := &Manager{ + nextIP: netip.AddrFrom4([4]byte{240, 0, 0, 1}), + allocated: make(map[netip.Addr]netip.Addr), + fakeToReal: make(map[netip.Addr]netip.Addr), + baseIP: netip.AddrFrom4([4]byte{240, 0, 0, 1}), + maxIP: netip.AddrFrom4([4]byte{240, 0, 0, 3}), // Only 3 IPs available + } + + // Allocate all available IPs + realIPs := []netip.Addr{ + netip.MustParseAddr("1.0.0.1"), + netip.MustParseAddr("1.0.0.2"), + netip.MustParseAddr("1.0.0.3"), + } + + for _, realIP := range realIPs { + _, err := manager.AllocateFakeIP(realIP) + if err != nil { + t.Fatalf("Failed to allocate fake IP: %v", err) + } + } + + // Try to allocate one more - should fail + _, err := manager.AllocateFakeIP(netip.MustParseAddr("1.0.0.4")) + if err == nil { + t.Error("Expected exhaustion error") + } +} + +func TestWrapAround(t *testing.T) { + // Create manager starting near the end of range + manager := &Manager{ + nextIP: netip.AddrFrom4([4]byte{240, 0, 0, 254}), + allocated: make(map[netip.Addr]netip.Addr), + fakeToReal: make(map[netip.Addr]netip.Addr), + baseIP: netip.AddrFrom4([4]byte{240, 0, 0, 1}), + maxIP: netip.AddrFrom4([4]byte{240, 0, 0, 254}), + } + + // Allocate the last IP + fakeIP1, err := manager.AllocateFakeIP(netip.MustParseAddr("1.0.0.1")) + if err != nil { + t.Fatalf("Failed to allocate first IP: %v", err) + } + + if fakeIP1.String() != "240.0.0.254" { + t.Errorf("Expected 240.0.0.254, got %s", fakeIP1.String()) + } + + // Next allocation should wrap around to the beginning + fakeIP2, err := manager.AllocateFakeIP(netip.MustParseAddr("1.0.0.2")) + if err != nil { + t.Fatalf("Failed to allocate second IP: %v", err) + } + + if fakeIP2.String() != "240.0.0.1" { + t.Errorf("Expected 240.0.0.1 after wrap, got %s", fakeIP2.String()) + } +} diff --git a/client/internal/routemanager/manager.go b/client/internal/routemanager/manager.go index 919bf25e3..e0974ab2a 100644 --- a/client/internal/routemanager/manager.go +++ b/client/internal/routemanager/manager.go @@ -8,9 +8,11 @@ import ( "net/netip" "net/url" "runtime" + "slices" "sync" "time" + "github.com/google/uuid" "github.com/hashicorp/go-multierror" log "github.com/sirupsen/logrus" "golang.org/x/exp/maps" @@ -24,6 +26,8 @@ import ( "github.com/netbirdio/netbird/client/internal/peer" "github.com/netbirdio/netbird/client/internal/peerstore" "github.com/netbirdio/netbird/client/internal/routemanager/client" + "github.com/netbirdio/netbird/client/internal/routemanager/common" + "github.com/netbirdio/netbird/client/internal/routemanager/fakeip" "github.com/netbirdio/netbird/client/internal/routemanager/iface" "github.com/netbirdio/netbird/client/internal/routemanager/notifier" "github.com/netbirdio/netbird/client/internal/routemanager/refcounter" @@ -49,7 +53,7 @@ type Manager interface { GetClientRoutesWithNetID() map[route.NetID][]*route.Route SetRouteChangeListener(listener listener.NetworkChangeListener) InitialRouteRange() []string - EnableServerRouter(firewall firewall.Manager) error + SetFirewall(firewall.Manager) error Stop(stateManager *statemanager.Manager) } @@ -63,6 +67,7 @@ type ManagerConfig struct { InitialRoutes []*route.Route StateManager *statemanager.Manager DNSServer dns.Server + DNSFeatureFlag bool PeerStore *peerstore.Store DisableClientRoutes bool DisableServerRoutes bool @@ -89,11 +94,13 @@ type DefaultManager struct { // clientRoutes is the most recent list of clientRoutes received from the Management Service clientRoutes route.HAMap dnsServer dns.Server + firewall firewall.Manager peerStore *peerstore.Store useNewDNSRoute bool disableClientRoutes bool disableServerRoutes bool activeRoutes map[route.HAUniqueID]client.RouteHandler + fakeIPManager *fakeip.Manager } func NewManager(config ManagerConfig) *DefaultManager { @@ -129,11 +136,31 @@ func NewManager(config ManagerConfig) *DefaultManager { } if runtime.GOOS == "android" { - cr := dm.initialClientRoutes(config.InitialRoutes) - dm.notifier.SetInitialClientRoutes(cr) + dm.setupAndroidRoutes(config) } return dm } +func (m *DefaultManager) setupAndroidRoutes(config ManagerConfig) { + cr := m.initialClientRoutes(config.InitialRoutes) + + routesForComparison := slices.Clone(cr) + + if config.DNSFeatureFlag { + m.fakeIPManager = fakeip.NewManager() + + id := uuid.NewString() + fakeIPRoute := &route.Route{ + ID: route.ID(id), + Network: m.fakeIPManager.GetFakeIPBlock(), + NetID: route.NetID(id), + Peer: m.pubKey, + NetworkType: route.IPv4Network, + } + cr = append(cr, fakeIPRoute) + } + + m.notifier.SetInitialClientRoutes(cr, routesForComparison) +} func (m *DefaultManager) setupRefCounters(useNoop bool) { m.routeRefCounter = refcounter.New( @@ -222,16 +249,16 @@ func (m *DefaultManager) initSelector() *routeselector.RouteSelector { return routeselector.NewRouteSelector() } -func (m *DefaultManager) EnableServerRouter(firewall firewall.Manager) error { - if m.disableServerRoutes { +// SetFirewall sets the firewall manager for the DefaultManager +// Not thread-safe, should be called before starting the manager +func (m *DefaultManager) SetFirewall(firewall firewall.Manager) error { + m.firewall = firewall + + if m.disableServerRoutes || firewall == nil { log.Info("server routes are disabled") return nil } - if firewall == nil { - return errors.New("firewall manager is not set") - } - var err error m.serverRouter, err = server.NewRouter(m.ctx, m.wgInterface, firewall, m.statusRecorder) if err != nil { @@ -299,17 +326,20 @@ func (m *DefaultManager) updateSystemRoutes(newRoutes route.HAMap) error { } for id, route := range toAdd { - handler := client.HandlerFromRoute( - route, - m.routeRefCounter, - m.allowedIPsRefCounter, - m.dnsRouteInterval, - m.statusRecorder, - m.wgInterface, - m.dnsServer, - m.peerStore, - m.useNewDNSRoute, - ) + params := common.HandlerParams{ + Route: route, + RouteRefCounter: m.routeRefCounter, + AllowedIPsRefCounter: m.allowedIPsRefCounter, + DnsRouterInterval: m.dnsRouteInterval, + StatusRecorder: m.statusRecorder, + WgInterface: m.wgInterface, + DnsServer: m.dnsServer, + PeerStore: m.peerStore, + UseNewDNSRoute: m.useNewDNSRoute, + Firewall: m.firewall, + FakeIPManager: m.fakeIPManager, + } + handler := client.HandlerFromRoute(params) if err := handler.AddRoute(m.ctx); err != nil { merr = multierror.Append(merr, fmt.Errorf("add route %s: %w", handler.String(), err)) continue @@ -517,6 +547,7 @@ func (m *DefaultManager) initialClientRoutes(initialRoutes []*route.Route) []*ro for _, routes := range crMap { rs = append(rs, routes...) } + return rs } diff --git a/client/internal/routemanager/mock.go b/client/internal/routemanager/mock.go index 742294cdf..4e182f82c 100644 --- a/client/internal/routemanager/mock.go +++ b/client/internal/routemanager/mock.go @@ -87,7 +87,7 @@ func (m *MockManager) SetRouteChangeListener(listener listener.NetworkChangeList } -func (m *MockManager) EnableServerRouter(firewall firewall.Manager) error { +func (m *MockManager) SetFirewall(firewall.Manager) error { panic("implement me") } diff --git a/client/internal/routemanager/notifier/notifier.go b/client/internal/routemanager/notifier/notifier.go deleted file mode 100644 index 3cc7c3308..000000000 --- a/client/internal/routemanager/notifier/notifier.go +++ /dev/null @@ -1,124 +0,0 @@ -package notifier - -import ( - "net/netip" - "runtime" - "sort" - "strings" - "sync" - - "github.com/netbirdio/netbird/client/internal/listener" - "github.com/netbirdio/netbird/route" -) - -type Notifier struct { - initialRouteRanges []string - routeRanges []string - - listener listener.NetworkChangeListener - listenerMux sync.Mutex -} - -func NewNotifier() *Notifier { - return &Notifier{} -} - -func (n *Notifier) SetListener(listener listener.NetworkChangeListener) { - n.listenerMux.Lock() - defer n.listenerMux.Unlock() - n.listener = listener -} - -func (n *Notifier) SetInitialClientRoutes(clientRoutes []*route.Route) { - nets := make([]string, 0) - for _, r := range clientRoutes { - if r.IsDynamic() { - continue - } - nets = append(nets, r.Network.String()) - } - sort.Strings(nets) - n.initialRouteRanges = nets -} - -func (n *Notifier) OnNewRoutes(idMap route.HAMap) { - if runtime.GOOS != "android" { - return - } - - var newNets []string - for _, routes := range idMap { - for _, r := range routes { - if r.IsDynamic() { - continue - } - newNets = append(newNets, r.Network.String()) - } - } - - sort.Strings(newNets) - if !n.hasDiff(n.initialRouteRanges, newNets) { - return - } - - n.routeRanges = newNets - n.notify() -} - -// OnNewPrefixes is called from iOS only -func (n *Notifier) OnNewPrefixes(prefixes []netip.Prefix) { - newNets := make([]string, 0) - for _, prefix := range prefixes { - newNets = append(newNets, prefix.String()) - } - - sort.Strings(newNets) - if !n.hasDiff(n.routeRanges, newNets) { - return - } - - n.routeRanges = newNets - n.notify() -} - -func (n *Notifier) notify() { - n.listenerMux.Lock() - defer n.listenerMux.Unlock() - if n.listener == nil { - return - } - - go func(l listener.NetworkChangeListener) { - l.OnNetworkChanged(strings.Join(addIPv6RangeIfNeeded(n.routeRanges), ",")) - }(n.listener) -} - -func (n *Notifier) hasDiff(a []string, b []string) bool { - if len(a) != len(b) { - return true - } - for i, v := range a { - if v != b[i] { - return true - } - } - return false -} - -func (n *Notifier) GetInitialRouteRanges() []string { - return addIPv6RangeIfNeeded(n.initialRouteRanges) -} - -// addIPv6RangeIfNeeded returns the input ranges with the default IPv6 range when there is an IPv4 default route. -func addIPv6RangeIfNeeded(inputRanges []string) []string { - ranges := inputRanges - for _, r := range inputRanges { - // we are intentionally adding the ipv6 default range in case of ipv4 default range - // to ensure that all traffic is managed by the tunnel interface on android - if r == "0.0.0.0/0" { - ranges = append(ranges, "::/0") - break - } - } - return ranges -} diff --git a/client/internal/routemanager/notifier/notifier_android.go b/client/internal/routemanager/notifier/notifier_android.go new file mode 100644 index 000000000..dec0af87c --- /dev/null +++ b/client/internal/routemanager/notifier/notifier_android.go @@ -0,0 +1,127 @@ +//go:build android + +package notifier + +import ( + "net/netip" + "slices" + "sort" + "strings" + "sync" + + "github.com/netbirdio/netbird/client/internal/listener" + "github.com/netbirdio/netbird/route" +) + +type Notifier struct { + initialRoutes []*route.Route + currentRoutes []*route.Route + + listener listener.NetworkChangeListener + listenerMux sync.Mutex +} + +func NewNotifier() *Notifier { + return &Notifier{} +} + +func (n *Notifier) SetListener(listener listener.NetworkChangeListener) { + n.listenerMux.Lock() + defer n.listenerMux.Unlock() + n.listener = listener +} + +func (n *Notifier) SetInitialClientRoutes(initialRoutes []*route.Route, routesForComparison []*route.Route) { + // initialRoutes contains fake IP block for interface configuration + filteredInitial := make([]*route.Route, 0) + for _, r := range initialRoutes { + if r.IsDynamic() { + continue + } + filteredInitial = append(filteredInitial, r) + } + n.initialRoutes = filteredInitial + + // routesForComparison excludes fake IP block for comparison with new routes + filteredComparison := make([]*route.Route, 0) + for _, r := range routesForComparison { + if r.IsDynamic() { + continue + } + filteredComparison = append(filteredComparison, r) + } + n.currentRoutes = filteredComparison +} + +func (n *Notifier) OnNewRoutes(idMap route.HAMap) { + var newRoutes []*route.Route + for _, routes := range idMap { + for _, r := range routes { + if r.IsDynamic() { + continue + } + newRoutes = append(newRoutes, r) + } + } + + if !n.hasRouteDiff(n.currentRoutes, newRoutes) { + return + } + + n.currentRoutes = newRoutes + n.notify() +} + +func (n *Notifier) OnNewPrefixes([]netip.Prefix) { + // Not used on Android +} + +func (n *Notifier) notify() { + n.listenerMux.Lock() + defer n.listenerMux.Unlock() + if n.listener == nil { + return + } + + routeStrings := n.routesToStrings(n.currentRoutes) + sort.Strings(routeStrings) + go func(l listener.NetworkChangeListener) { + l.OnNetworkChanged(strings.Join(n.addIPv6RangeIfNeeded(routeStrings, n.currentRoutes), ",")) + }(n.listener) +} + +func (n *Notifier) routesToStrings(routes []*route.Route) []string { + nets := make([]string, 0, len(routes)) + for _, r := range routes { + nets = append(nets, r.NetString()) + } + return nets +} + +func (n *Notifier) hasRouteDiff(a []*route.Route, b []*route.Route) bool { + slices.SortFunc(a, func(x, y *route.Route) int { + return strings.Compare(x.NetString(), y.NetString()) + }) + slices.SortFunc(b, func(x, y *route.Route) int { + return strings.Compare(x.NetString(), y.NetString()) + }) + + return !slices.EqualFunc(a, b, func(x, y *route.Route) bool { + return x.NetString() == y.NetString() + }) +} + +func (n *Notifier) GetInitialRouteRanges() []string { + initialStrings := n.routesToStrings(n.initialRoutes) + sort.Strings(initialStrings) + return n.addIPv6RangeIfNeeded(initialStrings, n.initialRoutes) +} + +func (n *Notifier) addIPv6RangeIfNeeded(inputRanges []string, routes []*route.Route) []string { + for _, r := range routes { + if r.Network.Addr().Is4() && r.Network.Bits() == 0 { + return append(slices.Clone(inputRanges), "::/0") + } + } + return inputRanges +} diff --git a/client/internal/routemanager/notifier/notifier_ios.go b/client/internal/routemanager/notifier/notifier_ios.go new file mode 100644 index 000000000..bb125cfa4 --- /dev/null +++ b/client/internal/routemanager/notifier/notifier_ios.go @@ -0,0 +1,80 @@ +//go:build ios + +package notifier + +import ( + "net/netip" + "slices" + "sort" + "strings" + "sync" + + "github.com/netbirdio/netbird/client/internal/listener" + "github.com/netbirdio/netbird/route" +) + +type Notifier struct { + currentPrefixes []string + + listener listener.NetworkChangeListener + listenerMux sync.Mutex +} + +func NewNotifier() *Notifier { + return &Notifier{} +} + +func (n *Notifier) SetListener(listener listener.NetworkChangeListener) { + n.listenerMux.Lock() + defer n.listenerMux.Unlock() + n.listener = listener +} + +func (n *Notifier) SetInitialClientRoutes([]*route.Route, []*route.Route) { + // iOS doesn't care about initial routes +} + +func (n *Notifier) OnNewRoutes(route.HAMap) { + // Not used on iOS +} + +func (n *Notifier) OnNewPrefixes(prefixes []netip.Prefix) { + newNets := make([]string, 0) + for _, prefix := range prefixes { + newNets = append(newNets, prefix.String()) + } + + sort.Strings(newNets) + + if slices.Equal(n.currentPrefixes, newNets) { + return + } + + n.currentPrefixes = newNets + n.notify() +} + +func (n *Notifier) notify() { + n.listenerMux.Lock() + defer n.listenerMux.Unlock() + if n.listener == nil { + return + } + + go func(l listener.NetworkChangeListener) { + l.OnNetworkChanged(strings.Join(n.addIPv6RangeIfNeeded(n.currentPrefixes), ",")) + }(n.listener) +} + +func (n *Notifier) GetInitialRouteRanges() []string { + return nil +} + +func (n *Notifier) addIPv6RangeIfNeeded(inputRanges []string) []string { + for _, r := range inputRanges { + if r == "0.0.0.0/0" { + return append(slices.Clone(inputRanges), "::/0") + } + } + return inputRanges +} diff --git a/client/internal/routemanager/notifier/notifier_other.go b/client/internal/routemanager/notifier/notifier_other.go new file mode 100644 index 000000000..77045b839 --- /dev/null +++ b/client/internal/routemanager/notifier/notifier_other.go @@ -0,0 +1,36 @@ +//go:build !android && !ios + +package notifier + +import ( + "net/netip" + + "github.com/netbirdio/netbird/client/internal/listener" + "github.com/netbirdio/netbird/route" +) + +type Notifier struct{} + +func NewNotifier() *Notifier { + return &Notifier{} +} + +func (n *Notifier) SetListener(listener listener.NetworkChangeListener) { + // Not used on non-mobile platforms +} + +func (n *Notifier) SetInitialClientRoutes([]*route.Route, []*route.Route) { + // Not used on non-mobile platforms +} + +func (n *Notifier) OnNewRoutes(idMap route.HAMap) { + // Not used on non-mobile platforms +} + +func (n *Notifier) OnNewPrefixes(prefixes []netip.Prefix) { + // Not used on non-mobile platforms +} + +func (n *Notifier) GetInitialRouteRanges() []string { + return []string{} +} \ No newline at end of file diff --git a/client/internal/routemanager/static/route.go b/client/internal/routemanager/static/route.go index c8b9338e0..d480fdf00 100644 --- a/client/internal/routemanager/static/route.go +++ b/client/internal/routemanager/static/route.go @@ -6,6 +6,7 @@ import ( log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/client/internal/routemanager/common" "github.com/netbirdio/netbird/client/internal/routemanager/refcounter" "github.com/netbirdio/netbird/route" ) @@ -16,11 +17,11 @@ type Route struct { allowedIPsRefcounter *refcounter.AllowedIPsRefCounter } -func NewRoute(rt *route.Route, routeRefCounter *refcounter.RouteRefCounter, allowedIPsRefCounter *refcounter.AllowedIPsRefCounter) *Route { +func NewRoute(params common.HandlerParams) *Route { return &Route{ - route: rt, - routeRefCounter: routeRefCounter, - allowedIPsRefcounter: allowedIPsRefCounter, + route: params.Route, + routeRefCounter: params.RouteRefCounter, + allowedIPsRefcounter: params.AllowedIPsRefCounter, } }