From c22817fdb5088d7095643dbcdd2fae8022e0189b Mon Sep 17 00:00:00 2001 From: irenarindos Date: Fri, 14 Jun 2024 08:45:34 -0400 Subject: [PATCH] worker auth: use new previous pkix field --- internal/daemon/worker/auth_rotation.go | 7 ++----- internal/daemon/worker/auth_rotation_test.go | 12 ------------ internal/server/repository_workerauth.go | 16 ++++++++++------ internal/server/repository_workerauth_test.go | 4 ++-- 4 files changed, 14 insertions(+), 25 deletions(-) diff --git a/internal/daemon/worker/auth_rotation.go b/internal/daemon/worker/auth_rotation.go index b1e9758424..dca47f3ba4 100644 --- a/internal/daemon/worker/auth_rotation.go +++ b/internal/daemon/worker/auth_rotation.go @@ -235,11 +235,8 @@ func rotateWorkerAuth(ctx context.Context, w *Worker, currentNodeCreds *types.No if err != nil { return 0, berrors.Wrap(ctx, err, op) } - - err = newNodeCreds.SetPreviousEncryptionKey(currentNodeCreds) - if err != nil { - return 0, berrors.Wrap(ctx, err, op) - } + + newNodeCreds.PreviousCertificatePublicKeyPkix = currentNodeCreds.CertificatePublicKeyPkix // Get a signed request from the new credentials fetchReq, err := newNodeCreds.CreateFetchNodeCredentialsRequest(ctx, randReaderOpt) diff --git a/internal/daemon/worker/auth_rotation_test.go b/internal/daemon/worker/auth_rotation_test.go index 2cf8e8d247..e6dc4f0f21 100644 --- a/internal/daemon/worker/auth_rotation_test.go +++ b/internal/daemon/worker/auth_rotation_test.go @@ -106,9 +106,6 @@ func TestRotationTicking(t *testing.T) { require.NoError(err) currKey := currNodeCreds.CertificatePublicKeyPkix - priorKeyId, err := nodeenrollment.KeyIdFromPkix(currKey) - require.NoError(err) - // Now we wait and check that we see new values in the DB and different // creds on the worker after each rotation period @@ -150,11 +147,6 @@ func TestRotationTicking(t *testing.T) { require.NoError(err) assert.Equal(currKeyId, w.Worker().WorkerAuthCurrentKeyId.Load()) - // Check that we've got the correct prior encryption key - previousKeyId, _, err := currNodeCreds.PreviousX25519EncryptionKey() - require.NoError(err) - assert.Equal(priorKeyId, previousKeyId) - // Get workerAuthSet for this worker id and compare keys workerAuthSet, err := workerAuthRepo.FindWorkerAuthByWorkerId(c.Context(), newWorker.PublicId) require.NoError(err) @@ -162,10 +154,6 @@ func TestRotationTicking(t *testing.T) { assert.NotNil(workerAuthSet.Previous) assert.NotNil(workerAuthSet.Current) assert.Equal(workerAuthSet.Current.WorkerKeyIdentifier, currKeyId) - assert.Equal(workerAuthSet.Previous.WorkerKeyIdentifier, previousKeyId) - - // Save priorKeyId - priorKeyId = currKeyId // Stop and start the client connections to ensure the new credentials // are valid; if not, we won't establish a new connection and rotation diff --git a/internal/server/repository_workerauth.go b/internal/server/repository_workerauth.go index 30ce8fbfc3..7a289e738b 100644 --- a/internal/server/repository_workerauth.go +++ b/internal/server/repository_workerauth.go @@ -249,9 +249,13 @@ func StoreNodeInformationTx(ctx context.Context, reader db.Reader, writer db.Wri // It's possible a connection dropped during a rotate credentials response, so the control plane's stored // previous and current WorkerAuth records may not match what the worker has stored. // Check what the worker indicates is its previous key and fix what we have stored before inserting the new record. - if node.PreviousEncryptionKey != nil { + if node.PreviousCertificatePublicKeyPkix != nil { + previousKeyId, err := nodeenrollment.KeyIdFromPkix(node.PreviousCertificatePublicKeyPkix) + if err != nil { + return errors.Wrap(ctx, err, op) + } query := getWorkerAuthStateByKeyIdQuery - rows, err := reader.Query(ctx, query, []any{sql.Named("worker_key_identifier", node.PreviousEncryptionKey.KeyId)}) + rows, err := reader.Query(ctx, query, []any{sql.Named("worker_key_identifier", previousKeyId)}) if err != nil { return errors.Wrap(ctx, err, op) } @@ -271,12 +275,12 @@ func StoreNodeInformationTx(ctx context.Context, reader db.Reader, writer db.Wri // ensure proper state rotation on store if state == previousWorkerAuthState { query := deleteWorkerAuthByKeyId - _, err := writer.Exec(ctx, query, []any{sql.Named("worker_key_identifier", node.PreviousEncryptionKey.KeyId)}) + _, err := writer.Exec(ctx, query, []any{sql.Named("worker_key_identifier", previousKeyId)}) if err != nil { return errors.Wrap(ctx, err, op) } query = updateWorkerAuthStateByKeyId - _, err = writer.Exec(ctx, query, []any{sql.Named("worker_key_identifier", node.PreviousEncryptionKey.KeyId)}) + _, err = writer.Exec(ctx, query, []any{sql.Named("worker_key_identifier", previousKeyId)}) if err != nil { return errors.Wrap(ctx, err, op) } @@ -773,7 +777,7 @@ func (r *WorkerAuthRepositoryStorage) LoadByNodeId(ctx context.Context, msg node var err error switch t := msg.(type) { - case *types.NodeInformations: + case *types.NodeInformationSet: err = r.loadNodeInfosByNodeId(ctx, t) default: return errors.New(ctx, errors.InvalidParameter, op, fmt.Sprintf("message type %T not supported for LoadByNodeId", t)) @@ -790,7 +794,7 @@ func (r *WorkerAuthRepositoryStorage) LoadByNodeId(ctx context.Context, msg node return nil } -func (r *WorkerAuthRepositoryStorage) loadNodeInfosByNodeId(ctx context.Context, nodeInfos *types.NodeInformations) error { +func (r *WorkerAuthRepositoryStorage) loadNodeInfosByNodeId(ctx context.Context, nodeInfos *types.NodeInformationSet) error { const op = "server.(WorkerAuthRepositoryStorage).loadNodeInfosByNodeId" if nodeInfos == nil { return errors.New(ctx, errors.InvalidParameter, op, "missing NodeInformations") diff --git a/internal/server/repository_workerauth_test.go b/internal/server/repository_workerauth_test.go index 8b9197cb1d..ac1baeafa8 100644 --- a/internal/server/repository_workerauth_test.go +++ b/internal/server/repository_workerauth_test.go @@ -804,7 +804,7 @@ func TestSplitBrain(t *testing.T) { newCreds, err := types.NewNodeCredentials(ctx, workerStorage, nodeenrollment.WithSkipStorage(true)) require.NoError(err) - require.NoError(newCreds.SetPreviousEncryptionKey(initCreds)) + newCreds.PreviousCertificatePublicKeyPkix = initCreds.CertificatePublicKeyPkix fetchReq, err = newCreds.CreateFetchNodeCredentialsRequest(ctx) require.NoError(err) @@ -831,7 +831,7 @@ func TestSplitBrain(t *testing.T) { require.NoError(err) require.NotEqual(t, newNewCreds.CertificatePublicKeyPkix, newCreds.CertificatePublicKeyPkix) - require.NoError(newNewCreds.SetPreviousEncryptionKey(initCreds)) + newNewCreds.PreviousCertificatePublicKeyPkix = initCreds.CertificatePublicKeyPkix fetchReq, err = newNewCreds.CreateFetchNodeCredentialsRequest(ctx) require.NoError(err)