diff --git a/go.mod b/go.mod index b971f6e24c..9361a0ee6d 100644 --- a/go.mod +++ b/go.mod @@ -95,7 +95,7 @@ require ( github.com/hashicorp/dbassert v0.0.0-20230405175854-2d88acd5134b github.com/hashicorp/go-kms-wrapping/extras/kms/v2 v2.0.0-20221122211539-47c893099f13 github.com/hashicorp/go-version v1.6.0 - github.com/hashicorp/nodeenrollment v0.2.4 + github.com/hashicorp/nodeenrollment v0.2.5 github.com/jackc/pgx/v5 v5.3.1 github.com/jimlambrt/gldap v0.1.7 github.com/kelseyhightower/envconfig v1.4.0 diff --git a/go.sum b/go.sum index 4b2f59cc61..b35a9d598a 100644 --- a/go.sum +++ b/go.sum @@ -745,8 +745,8 @@ github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4= github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= -github.com/hashicorp/nodeenrollment v0.2.4 h1:Rhdgtdd26Ng+r2+m6CDVWT8+wwQzN8y+6kApoc5Q91U= -github.com/hashicorp/nodeenrollment v0.2.4/go.mod h1:Do2FvaFHgnnMaqhdSTBX+TkQP0ep8VVQ0rOr+ZjUbJU= +github.com/hashicorp/nodeenrollment v0.2.5 h1:vmpYzF1jsUJSofBVNccqBfb8fXtSXOVgNcXRwrRSWpU= +github.com/hashicorp/nodeenrollment v0.2.5/go.mod h1:Do2FvaFHgnnMaqhdSTBX+TkQP0ep8VVQ0rOr+ZjUbJU= github.com/hashicorp/vault/api v1.9.1 h1:LtY/I16+5jVGU8rufyyAkwopgq/HpUnxFBg+QLOAV38= github.com/hashicorp/vault/api v1.9.1/go.mod h1:78kktNcQYbBGSrOjQfHjXN32OhhxXnbYl3zxpd2uPUs= github.com/hashicorp/vault/sdk v0.3.0 h1:kR3dpxNkhh/wr6ycaJYqp6AFT/i2xaftbfnwZduTKEY= diff --git a/internal/server/repository_workerauth.go b/internal/server/repository_workerauth.go index 21198c706e..513f94d074 100644 --- a/internal/server/repository_workerauth.go +++ b/internal/server/repository_workerauth.go @@ -245,9 +245,10 @@ func StoreNodeInformationTx(ctx context.Context, reader db.Reader, writer db.Wri } } - // If the incoming workerAuth matches what we have stored, then we can return as it's already stored + // If the incoming workerAuth matches what we have stored, return a duplicate record error + // This will cause nodeenrollment to lookup and return the already stored nodeInfo if nodeAuth.compare(nodeAuthLookup) { - return nil + return new(types.DuplicateRecordError) } if err := writer.Create(ctx, &nodeAuth); err != nil { diff --git a/internal/server/repository_workerauth_test.go b/internal/server/repository_workerauth_test.go index 9affc43111..933cdccae9 100644 --- a/internal/server/repository_workerauth_test.go +++ b/internal/server/repository_workerauth_test.go @@ -351,28 +351,6 @@ func TestStoreNodeInformationTx(t *testing.T) { } return nodeInfo } - testNodeInfoFn2 := func() *types.NodeInformation { - // This happens on the worker - storage, err := inmem.New(testCtx) - require.NoError(t, err) - nodeCreds, err := types.NewNodeCredentials(testCtx, storage) - require.NoError(t, err) - - nodePubKey, err := curve25519.X25519(nodeCreds.EncryptionPrivateKeyBytes, curve25519.Basepoint) - require.NoError(t, err) - // Add in node information to storage so we have a key to use - nodeInfo := &types.NodeInformation{ - Id: "fake-secondary-key-id", - CertificatePublicKeyPkix: nodeCreds.CertificatePublicKeyPkix, - CertificatePublicKeyType: nodeCreds.CertificatePrivateKeyType, - EncryptionPublicKeyBytes: nodePubKey, - EncryptionPublicKeyType: nodeCreds.EncryptionPrivateKeyType, - ServerEncryptionPrivateKeyBytes: []byte("whatever"), - RegistrationNonce: nodeCreds.RegistrationNonce, - State: testState, - } - return nodeInfo - } // For swapping out key ID for wrapping registration flow wrappingRegFlowStorage, err := inmem.New(testCtx) @@ -396,20 +374,15 @@ func TestStoreNodeInformationTx(t *testing.T) { } tests := []struct { - name string - reader db.Reader - writer db.Writer - scope string - kms *kms.Kms - node *types.NodeInformation - wantErr bool - wantErrIs errors.Code - wantErrContains string - storeTwice bool - secondStoreDifferentNode bool - wantSecondStoreErr bool - wantSecondStoreErrIs errors.Code - wantSecondStoreErrContains string + name string + reader db.Reader + writer db.Writer + scope string + kms *kms.Kms + node *types.NodeInformation + wantErr bool + wantErrIs errors.Code + wantErrContains string }{ { name: "missing-writer", @@ -522,28 +495,12 @@ func TestStoreNodeInformationTx(t *testing.T) { wantErrContains: "in wrapping registration flow but boundary version not provided", }, { - name: "success", - reader: rw, - writer: rw, - scope: scope.Global.String(), - kms: kmsCache, - node: testNodeInfoFn(), - storeTwice: true, - }, - { - // This test will fail because on the second store we change the incoming NodeInformation - // so that it does not match the already inserted record - name: "fail-store-twice-different-node-info", - reader: rw, - writer: rw, - scope: scope.Global.String(), - kms: kmsCache, - node: testNodeInfoFn2(), - storeTwice: true, - secondStoreDifferentNode: true, - wantSecondStoreErr: true, - wantSecondStoreErrIs: errors.NotUnique, - wantSecondStoreErrContains: "server.(WorkerAuthRepositoryStorage).StoreNodeInformationTx: db.Create: duplicate key value violates unique constraint \"worker_auth_authorized_pkey\": unique constraint violation: integrity violation: error #1002", + name: "success", + reader: rw, + writer: rw, + scope: scope.Global.String(), + kms: kmsCache, + node: testNodeInfoFn(), }, { name: "success-wrapflow", @@ -576,29 +533,159 @@ func TestStoreNodeInformationTx(t *testing.T) { return } require.NoError(err) - // Try to store the "same" node information twice - if tc.storeTwice { - node := tc.node - if tc.secondStoreDifferentNode { - storage, err := inmem.New(testCtx) - require.NoError(err) - nodeCreds, err := types.NewNodeCredentials(testCtx, storage) - require.NoError(err) - node.CertificatePublicKeyPkix = nodeCreds.CertificatePublicKeyPkix + }) + } +} + +func TestStoreNodeInformationTx_Twice(t *testing.T) { + t.Parallel() + testCtx := context.Background() + + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + + testWrapper := db.TestWrapper(t) + testKeyId, err := testWrapper.KeyId(testCtx) + require.NoError(t, err) + + kmsCache := kms.TestKms(t, conn, testWrapper) + // Ensures the global scope contains a valid root key + require.NoError(t, kmsCache.CreateKeys(context.Background(), scope.Global.String(), kms.WithRandomReader(rand.Reader))) + + testRootStorage, err := NewRepositoryStorage(testCtx, rw, rw, kmsCache) + require.NoError(t, err) + + _, err = rotation.RotateRootCertificates(testCtx, testRootStorage) + require.NoError(t, err) + + // Create struct to pass in with workerId that will be passed along to + // storage + testWorker := TestPkiWorker(t, conn, testWrapper) + + testState, err := AttachWorkerIdToState(testCtx, testWorker.PublicId) + require.NoError(t, err) + + testNodeInfoFn := func() *types.NodeInformation { + // This happens on the worker + storage, err := inmem.New(testCtx) + require.NoError(t, err) + nodeCreds, err := types.NewNodeCredentials(testCtx, storage) + require.NoError(t, err) + + nodePubKey, err := curve25519.X25519(nodeCreds.EncryptionPrivateKeyBytes, curve25519.Basepoint) + require.NoError(t, err) + // Add in node information to storage so we have a key to use + nodeInfo := &types.NodeInformation{ + Id: testKeyId, + CertificatePublicKeyPkix: nodeCreds.CertificatePublicKeyPkix, + CertificatePublicKeyType: nodeCreds.CertificatePrivateKeyType, + EncryptionPublicKeyBytes: nodePubKey, + EncryptionPublicKeyType: nodeCreds.EncryptionPrivateKeyType, + ServerEncryptionPrivateKeyBytes: []byte("whatever"), + RegistrationNonce: nodeCreds.RegistrationNonce, + State: testState, + } + return nodeInfo + } + testNodeInfoFn2 := func() *types.NodeInformation { + // This happens on the worker + storage, err := inmem.New(testCtx) + require.NoError(t, err) + nodeCreds, err := types.NewNodeCredentials(testCtx, storage) + require.NoError(t, err) + + nodePubKey, err := curve25519.X25519(nodeCreds.EncryptionPrivateKeyBytes, curve25519.Basepoint) + require.NoError(t, err) + // Add in node information to storage so we have a key to use + nodeInfo := &types.NodeInformation{ + Id: "fake-secondary-key-id", + CertificatePublicKeyPkix: nodeCreds.CertificatePublicKeyPkix, + CertificatePublicKeyType: nodeCreds.CertificatePrivateKeyType, + EncryptionPublicKeyBytes: nodePubKey, + EncryptionPublicKeyType: nodeCreds.EncryptionPrivateKeyType, + ServerEncryptionPrivateKeyBytes: []byte("whatever"), + RegistrationNonce: nodeCreds.RegistrationNonce, + State: testState, + } + return nodeInfo + } + + tests := []struct { + name string + reader db.Reader + writer db.Writer + scope string + kms *kms.Kms + node *types.NodeInformation + wantErr bool + wantErrIs errors.Code + wantErrContains string + secondStoreDifferentNode bool + wantSecondStoreErr bool + wantSecondStoreErrIs errors.Code + wantSecondStoreErrContains string + }{ + { + name: "duplicate record error", + reader: rw, + writer: rw, + scope: scope.Global.String(), + kms: kmsCache, + node: testNodeInfoFn(), + wantSecondStoreErr: true, + wantSecondStoreErrContains: "duplicate record found", + }, + { + // This test will fail because on the second store we change the incoming NodeInformation + // so that it does not match the already inserted record + name: "fail-store-twice-different-node-info", + reader: rw, + writer: rw, + scope: scope.Global.String(), + kms: kmsCache, + node: testNodeInfoFn2(), + secondStoreDifferentNode: true, + wantSecondStoreErr: true, + wantSecondStoreErrIs: errors.NotUnique, + wantSecondStoreErrContains: "server.(WorkerAuthRepositoryStorage).StoreNodeInformationTx: db.Create: duplicate key value violates unique constraint \"worker_auth_authorized_pkey\": unique constraint violation: integrity violation: error #1002", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + err := StoreNodeInformationTx(testCtx, tc.reader, tc.writer, tc.kms, tc.scope, tc.node) + if tc.wantErr { + require.Error(err) + if tc.wantErrIs != errors.Unknown { + assert.True(errors.Match(errors.T(tc.wantErrIs), err)) } - err = StoreNodeInformationTx(testCtx, tc.reader, tc.writer, tc.kms, tc.scope, node) - if tc.wantSecondStoreErr { - require.Error(err) - if tc.wantSecondStoreErrIs != errors.Unknown { - assert.True(errors.Match(errors.T(tc.wantSecondStoreErrIs), err)) - } - if tc.wantSecondStoreErrContains != "" { - assert.Contains(err.Error(), tc.wantSecondStoreErrContains) - } - return + if tc.wantErrContains != "" { + assert.Contains(err.Error(), tc.wantErrContains) } + return + } + require.NoError(err) + // Try to store the "same" node information twice + node := tc.node + if tc.secondStoreDifferentNode { + storage, err := inmem.New(testCtx) + require.NoError(err) + nodeCreds, err := types.NewNodeCredentials(testCtx, storage) require.NoError(err) + node.CertificatePublicKeyPkix = nodeCreds.CertificatePublicKeyPkix } + err = StoreNodeInformationTx(testCtx, tc.reader, tc.writer, tc.kms, tc.scope, node) + if tc.wantSecondStoreErr { + require.Error(err) + if tc.wantSecondStoreErrIs != errors.Unknown { + assert.True(errors.Match(errors.T(tc.wantSecondStoreErrIs), err)) + } + if tc.wantSecondStoreErrContains != "" { + assert.Contains(err.Error(), tc.wantSecondStoreErrContains) + } + return + } + require.NoError(err) }) } }