Skip to content

Commit

Permalink
Merge pull request #4896 from hashicorp/backport/irindos-nonamefix/un…
Browse files Browse the repository at this point in the history
…iformly-fond-spider

This pull request was automerged via backport-assistant
  • Loading branch information
hc-github-team-secure-boundary committed Jun 14, 2024
2 parents 72b0967 + a8b7e92 commit e2f22f7
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 27 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ require (
github.com/hashicorp/go-kms-wrapping/extras/kms/v2 v2.0.0-20231219183231-6bac757bb482
github.com/hashicorp/go-rate v0.0.0-20231204194614-cc8d401f70ab
github.com/hashicorp/go-version v1.6.0
github.com/hashicorp/nodeenrollment v0.2.12
github.com/hashicorp/nodeenrollment v0.2.13
github.com/jackc/pgx/v5 v5.5.5
github.com/jimlambrt/gldap v0.1.10
github.com/kelseyhightower/envconfig v1.4.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,8 @@ github.com/hashicorp/hcl v1.0.1-vault-5 h1:kI3hhbbyzr4dldA8UdTb7ZlVVlI2DACdCfz31
github.com/hashicorp/hcl v1.0.1-vault-5/go.mod h1:XYhtn6ijBSAj6n4YqAaf7RBPS4I06AItNorpy+MoQNM=
github.com/hashicorp/mql v0.1.3 h1:SZdOsocDPovwp3Q5AzoH6s000BD5zcr+hV8xAobOvuo=
github.com/hashicorp/mql v0.1.3/go.mod h1:CrbXH2f2ndS1X35x0E8aHdNYc3POYrEWpx/1Q+pq+iw=
github.com/hashicorp/nodeenrollment v0.2.12 h1:x5kaSvsXHZ2Y8j9CsRURh4V2/GZtdOFLu/HPeV4zGz8=
github.com/hashicorp/nodeenrollment v0.2.12/go.mod h1:3TcYV0L7N4EmeGHIQWr/JFAAsV+yHJaX9IQjeff/w5Q=
github.com/hashicorp/nodeenrollment v0.2.13 h1:TkVH6giSXgq4ojwWzbK1Q9JQgvGVilWtdmAdw7Lfeug=
github.com/hashicorp/nodeenrollment v0.2.13/go.mod h1:3TcYV0L7N4EmeGHIQWr/JFAAsV+yHJaX9IQjeff/w5Q=
github.com/hashicorp/vault/api v1.12.0 h1:meCpJSesvzQyao8FCOgk2fGdoADAnbDu2WPJN1lDLJ4=
github.com/hashicorp/vault/api v1.12.0/go.mod h1:si+lJCYO7oGkIoNPAN8j3azBLTn9SjMGS+jFaHd1Cck=
github.com/hashicorp/vault/sdk v0.11.0 h1:KP/tBUywaVcvOebAfMPNCCiXKeCNEbm3JauYmrZd7RI=
Expand Down
5 changes: 1 addition & 4 deletions internal/daemon/worker/auth_rotation.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,7 @@ func rotateWorkerAuth(ctx context.Context, w *Worker, currentNodeCreds *types.No
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)
Expand Down
12 changes: 0 additions & 12 deletions internal/daemon/worker/auth_rotation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -150,22 +147,13 @@ 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)
assert.NotNil(workerAuthSet)
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
Expand Down
16 changes: 10 additions & 6 deletions internal/server/repository_workerauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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))
Expand All @@ -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")
Expand Down
4 changes: 2 additions & 2 deletions internal/server/repository_workerauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand Down

0 comments on commit e2f22f7

Please sign in to comment.