diff --git a/management/internals/shared/grpc/certposture.go b/management/internals/shared/grpc/certposture.go index a0c2fa675..e97f84349 100644 --- a/management/internals/shared/grpc/certposture.go +++ b/management/internals/shared/grpc/certposture.go @@ -14,18 +14,37 @@ import ( const certChallengeKeyDomain = "netbird-cert-challenge-key" -// certChallenger derives the nonce secret from the management WireGuard key so every -// instance sharing that key issues and verifies the same nonces without extra state. -func certChallenger(serverKey wgtypes.Key) *certposture.Challenger { +// newCertChallenger derives the nonce secret from the data store encryption key, which +// is generated once and written back to the configuration, so the same secret survives +// a restart and is shared by every instance reading that configuration. A nonce carries +// no state of its own, so one instance can only verify what another issued if both +// derive the same secret. +// +// The server's WireGuard key cannot be used for this: it is generated afresh in every +// process, so it would invalidate every outstanding nonce on restart and make each +// instance reject the others'. A peer meeting that rejects its whole proof set and +// loses the policies the certificate check gates until it signs again. +// +// Without an encryption key the secret falls back to the WireGuard key, which is still +// unpredictable but no longer persisted. It must stay unpredictable above all else: a +// peer that could guess it would mint the nonces of future windows, sign them while its +// key is present, and keep passing long after the key is gone. +func newCertChallenger(encryptionKey string, serverKey wgtypes.Key) *certposture.Challenger { + secret := []byte(encryptionKey) + if len(secret) == 0 { + log.Warnf("no data store encryption key, deriving certificate challenges from the ephemeral server key: peers will be rejected once per restart and across instances") + secret = serverKey[:] + } + h := sha256.New() h.Write([]byte(certChallengeKeyDomain)) - h.Write(serverKey[:]) + h.Write(secret) return certposture.NewChallenger(h.Sum(nil)) } // stampCertificateChallenges fills the per-peer nonce into every certificate challenge // right before the response is encrypted for that peer. -func stampCertificateChallenges(checks []*proto.Checks, peerKey, serverKey wgtypes.Key) { +func stampCertificateChallenges(checks []*proto.Checks, challenger *certposture.Challenger, peerKey wgtypes.Key) { var nonce []byte for _, check := range checks { challenge := check.GetCertificateChallenge() @@ -33,7 +52,7 @@ func stampCertificateChallenges(checks []*proto.Checks, peerKey, serverKey wgtyp continue } if nonce == nil { - nonce = certChallenger(serverKey).Nonce(peerKey[:], time.Now()) + nonce = challenger.Nonce(peerKey[:], time.Now()) } challenge.Nonce = nonce } @@ -46,17 +65,11 @@ func (s *Server) verifiedCertificates(ctx context.Context, peerKey wgtypes.Key, if len(proofs) == 0 { return nil } - serverKey, err := s.secretsManager.GetWGKey() - if err != nil { - log.WithContext(ctx).Warnf("skipping certificate proofs of peer %s: %v", peerKey, err) - return nil - } - challenger := certChallenger(serverKey) now := time.Now() chains := make([]string, 0, len(proofs)) for _, p := range proofs { - chain, err := challenger.Verify(certposture.Proof{ + chain, err := s.challenger.Verify(certposture.Proof{ Nonce: p.GetNonce(), Chain: p.GetChain(), SigAlg: p.GetSigAlg(), diff --git a/management/internals/shared/grpc/certposture_test.go b/management/internals/shared/grpc/certposture_test.go index 5239264ba..d8d5d365f 100644 --- a/management/internals/shared/grpc/certposture_test.go +++ b/management/internals/shared/grpc/certposture_test.go @@ -16,7 +16,8 @@ import ( ) func TestCertificateChallenge_StampAndVerifyRoundTrip(t *testing.T) { - serverKey := generateKey(t) + const encryptionKey = "data-store-encryption-key" + challenger := newCertChallenger(encryptionKey, generateKey(t)) peerKey := generateKey(t).PublicKey() ca := certtest.NewCA(t, "corp-root") ctx := context.Background() @@ -29,7 +30,7 @@ func TestCertificateChallenge_StampAndVerifyRoundTrip(t *testing.T) { require.Equal(t, []string{ca.PEM}, checks[0].GetCertificateChallenge().GetCaCertificates()) require.Empty(t, checks[0].GetCertificateChallenge().GetNonce()) - stampCertificateChallenges(checks, peerKey, serverKey) + stampCertificateChallenges(checks, challenger, peerKey) nonce := checks[0].GetCertificateChallenge().GetNonce() require.NotEmpty(t, nonce) @@ -39,7 +40,7 @@ func TestCertificateChallenge_StampAndVerifyRoundTrip(t *testing.T) { require.NoError(t, err) proofs := []*proto.CertificateProof{{Nonce: nonce, Chain: [][]byte{leaf.Raw}, SigAlg: sigAlg, Signature: sig}} - s := &Server{secretsManager: &TimeBasedAuthSecretsManager{wgKey: serverKey}} + s := &Server{challenger: challenger} chains := s.verifiedCertificates(ctx, peerKey, proofs) require.Len(t, chains, 1) @@ -48,10 +49,17 @@ func TestCertificateChallenge_StampAndVerifyRoundTrip(t *testing.T) { t.Run("proof replayed by another peer is rejected", func(t *testing.T) { assert.Nil(t, s.verifiedCertificates(ctx, generateKey(t).PublicKey(), proofs)) }) - t.Run("nonce from another management key is rejected", func(t *testing.T) { - other := &Server{secretsManager: &TimeBasedAuthSecretsManager{wgKey: generateKey(t)}} + t.Run("nonce from another deployment's secret is rejected", func(t *testing.T) { + other := &Server{challenger: newCertChallenger("a-different-encryption-key", generateKey(t))} assert.Nil(t, other.verifiedCertificates(ctx, peerKey, proofs)) }) + t.Run("a second instance with the same encryption key accepts it", func(t *testing.T) { + // The point of deriving from the stored key: another instance, with its own + // freshly generated WireGuard key, verifies what this one issued. This is also + // what a restart looks like, and it used to reject. + peer := &Server{challenger: newCertChallenger(encryptionKey, generateKey(t))} + assert.Len(t, peer.verifiedCertificates(ctx, peerKey, proofs), 1) + }) t.Run("one invalid proof rejects the whole set", func(t *testing.T) { bad := &proto.CertificateProof{Nonce: nonce, Chain: [][]byte{leaf.Raw}, SigAlg: sigAlg, Signature: []byte("junk")} assert.Nil(t, s.verifiedCertificates(ctx, peerKey, append(proofs, bad))) @@ -63,7 +71,7 @@ func TestCertificateChallenge_StampAndVerifyRoundTrip(t *testing.T) { func TestStampCertificateChallenges_SkipsFileOnlyChecks(t *testing.T) { checks := []*proto.Checks{{Files: []string{"/bin/agent"}}} - stampCertificateChallenges(checks, generateKey(t).PublicKey(), generateKey(t)) + stampCertificateChallenges(checks, newCertChallenger("key", generateKey(t)), generateKey(t).PublicKey()) assert.Nil(t, checks[0].GetCertificateChallenge()) } diff --git a/management/internals/shared/grpc/peer_update_handler.go b/management/internals/shared/grpc/peer_update_handler.go index d3ff9b6af..d80a7f203 100644 --- a/management/internals/shared/grpc/peer_update_handler.go +++ b/management/internals/shared/grpc/peer_update_handler.go @@ -7,6 +7,7 @@ import ( "github.com/netbirdio/netbird/encryption" "github.com/netbirdio/netbird/management/internals/controllers/network_map" "github.com/netbirdio/netbird/management/server/telemetry" + "github.com/netbirdio/netbird/shared/management/certposture" "github.com/netbirdio/netbird/shared/management/proto" log "github.com/sirupsen/logrus" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" @@ -18,12 +19,14 @@ func PeerUpdateHandlerFactory( peerKey wgtypes.Key, updates chan *network_map.UpdateMessage, secretsManager SecretsManager, + challenger *certposture.Challenger, srv proto.ManagementService_SyncServer, cleanupfunc func()) *PeerUpdateHandler { return &PeerUpdateHandler{ peerKey: peerKey, updates: updates, secretsManager: secretsManager, + challenger: challenger, srv: srv, encrypter: encryption.DefaultEncrypter{}, debouncer: NewUpdateDebouncer(1000 * time.Millisecond), @@ -40,6 +43,7 @@ type PeerUpdateHandler struct { updates chan *network_map.UpdateMessage appMetrics telemetry.AppMetrics secretsManager SecretsManager + challenger *certposture.Challenger srv syncSender encrypter encryption.Encrypter debouncer Debouncer @@ -117,7 +121,7 @@ func (pu *PeerUpdateHandler) SendUpdate(ctx context.Context, update *network_map return status.Errorf(codes.Internal, "failed processing update message") } - stampCertificateChallenges(update.Update.GetChecks(), pu.peerKey, key) + stampCertificateChallenges(update.Update.GetChecks(), pu.challenger, pu.peerKey) encryptedResp, err := pu.encrypter.EncryptMessage(pu.peerKey, key, update.Update) if err != nil { pu.cleanupFunc() diff --git a/management/internals/shared/grpc/server.go b/management/internals/shared/grpc/server.go index 82a765481..aec1e107d 100644 --- a/management/internals/shared/grpc/server.go +++ b/management/internals/shared/grpc/server.go @@ -45,6 +45,7 @@ import ( "github.com/netbirdio/netbird/management/server/settings" "github.com/netbirdio/netbird/management/server/telemetry" "github.com/netbirdio/netbird/management/server/types" + "github.com/netbirdio/netbird/shared/management/certposture" "github.com/netbirdio/netbird/shared/management/networkmap/nmdata" "github.com/netbirdio/netbird/shared/management/proto" internalStatus "github.com/netbirdio/netbird/shared/management/status" @@ -70,6 +71,7 @@ type Server struct { peerLocks sync.Map authManager auth.Manager sessionStore *auth.SessionStore + challenger *certposture.Challenger logBlockedPeers bool blockPeersWithSameConfig bool @@ -131,7 +133,13 @@ func NewServer( } } + serverKey, err := secretsManager.GetWGKey() + if err != nil { + return nil, fmt.Errorf("get server WireGuard key: %w", err) + } + return &Server{ + challenger: newCertChallenger(config.DataStoreEncryptionKey, serverKey), jobManager: jobManager, accountManager: accountManager, settingsManager: settingsManager, @@ -338,7 +346,7 @@ func (s *Server) Sync(req *proto.EncryptedMessage, srv proto.ManagementService_S s.syncSem.Add(-1) - return PeerUpdateHandlerFactory(peerKey, updates, s.secretsManager, srv, func() { s.cancelPeerRoutines(ctx, accountID, peer, syncStart) }). + return PeerUpdateHandlerFactory(peerKey, updates, s.secretsManager, s.challenger, srv, func() { s.cancelPeerRoutines(ctx, accountID, peer, syncStart) }). WithMetrics(s.appMetrics).HandleUpdates(ctx) } @@ -721,7 +729,7 @@ func (s *Server) Login(ctx context.Context, req *proto.EncryptedMessage) (*proto return nil, status.Errorf(codes.Internal, "failed logging in peer") } - stampCertificateChallenges(loginResp.Checks, peerKey, key) + stampCertificateChallenges(loginResp.Checks, s.challenger, peerKey) encryptedResp, err := encryption.EncryptMessage(peerKey, key, loginResp) if err != nil { log.WithContext(ctx).Warnf("failed encrypting peer %s message", peer.ID) @@ -973,7 +981,7 @@ func (s *Server) sendInitialSync(ctx context.Context, peerKey wgtypes.Key, peer return status.Errorf(codes.Internal, "failed getting server key") } - stampCertificateChallenges(plainResp.Checks, peerKey, key) + stampCertificateChallenges(plainResp.Checks, s.challenger, peerKey) encryptedResp, err := encryption.EncryptMessage(peerKey, key, plainResp) if err != nil { return status.Errorf(codes.Internal, "error handling request")