fix (kms): properly reconcile DEKs for existing scopes (#1976)

CHANGELOG.md

@@ -11,6 +11,8 @@ Canonical reference for changes, improvements, and bugfixes for Boundary.
 * worker: create new error to prevent `event.newError: missing error: invalid parameter` and handle session cancel 
   with no TOFU token ([Issue](https://github.com/hashicorp/boundary/issues/1902),
   [PR](https://github.com/hashicorp/boundary/pull/1929))
+* controller: Reconcile DEKs with existing scopes ([Issue](https://github.com/hashicorp/boundary/issues/1856),
+  [PR](https://github.com/hashicorp/boundary/pull/1976))
   
 ## 0.7.6 (2022/03/15)
 

internal/kms/const.go

@@ -4,6 +4,11 @@ package kms
 // is used to select which DEK to return
 type KeyPurpose uint
 
+// ****************************************************************************
+// IMPORTANT: if you're adding a new KeyPurpose, you should consider whether or
+// not existing scopes need this new type of key.  If they do, then you may want
+// to add the new key into kms.ReconcileKeys(...)
+// ****************************************************************************
 const (
 	// KeyPurposeUnknown is the default, and indicates that a correct purpose
 	// wasn't specified

internal/kms/kms.go

@@ -287,8 +287,10 @@ func (k *Kms) loadRoot(ctx context.Context, scopeId string, opt ...Option) (*mul
 	return pooled, rootKeyId, nil
 }
 
-// ReconcileKeys will reconcile the keys in the kms against known possible issues.
-func (k *Kms) ReconcileKeys(ctx context.Context, randomReader io.Reader) error {
+// ReconcileKeys will reconcile the keys in the kms against known possible
+// issues.  This function reconciles the global scope unless the
+// WithScopeIds(...) option is provided
+func (k *Kms) ReconcileKeys(ctx context.Context, randomReader io.Reader, opt ...Option) error {
 	const op = "kms.ReconcileKeys"
 	if isNil(randomReader) {
 		return errors.New(ctx, errors.InvalidParameter, op, "missing rand reader")
@@ -315,6 +317,46 @@ func (k *Kms) ReconcileKeys(ctx context.Context, randomReader io.Reader) error {
 			errors.Wrap(ctx, err, op)
 		}
 	}
+
+	opts := getOpts(opt...)
+	for _, id := range opts.withScopeIds {
+		var scopeRootWrapper *multi.PooledWrapper
+
+		for _, purpose := range []KeyPurpose{
+			// just add additional purposes as needed going forward to reconcile
+			// new keys as they are added.
+			//
+			// NOTE: don't add an audit key here since it can only be created in
+			// the global scope.
+			KeyPurposeOidc,
+		} {
+			if _, err := k.GetWrapper(ctx, id, purpose); err != nil {
+				switch {
+				case errors.Match(errors.T(errors.KeyNotFound), err):
+					if scopeRootWrapper == nil {
+						if scopeRootWrapper, _, err = k.loadRoot(ctx, id); err != nil {
+							return errors.Wrap(ctx, err, op)
+						}
+					}
+					key, err := generateKey(ctx, randomReader)
+					if err != nil {
+						return errors.Wrap(ctx, err, op, errors.WithMsg(fmt.Sprintf("error generating random bytes for %q key in scope %q", purpose.String(), id)))
+					}
+					switch purpose {
+					case KeyPurposeOidc:
+						if _, _, err := k.repo.CreateOidcKey(ctx, scopeRootWrapper, key); err != nil {
+							return errors.Wrap(ctx, err, op, errors.WithMsg(fmt.Sprintf("error creating %q key in scope %s", purpose.String(), id)))
+						}
+					default:
+						return errors.New(ctx, errors.Internal, op, fmt.Sprintf("error creating %q key in scope %s since it's not a supported KeyPurpose", purpose.String(), id))
+					}
+				default:
+					errors.Wrap(ctx, err, op)
+				}
+			}
+		}
+	}
+
 	return nil
 }
 

internal/kms/kms_ext_test.go

@@ -143,11 +143,16 @@ func TestKms_ReconcileKeys(t *testing.T) {
 	conn, _ := db.TestSetup(t, "postgres")
 	rw := db.New(conn)
 	wrapper := db.TestWrapper(t)
+	org, _ := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper))
+	org2, _ := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper))
+
 	tests := []struct {
 		name            string
 		kms             *kms.Kms
+		scopeIds        []string
 		reader          io.Reader
-		setup           func()
+		setup           func(*kms.Kms)
+		wantPurpose     []kms.KeyPurpose
 		wantErr         bool
 		wantErrMatch    *errors.Template
 		wantErrContains string
@@ -177,10 +182,36 @@ func TestKms_ReconcileKeys(t *testing.T) {
 			name:   "reconcile-audit-key",
 			kms:    kms.TestKms(t, conn, wrapper),
 			reader: rand.Reader,
-			setup: func() {
+			setup: func(k *kms.Kms) {
 				db.TestDeleteWhere(t, conn, func() interface{} { i := kms.AllocAuditKey(); return &i }(), "1=1")
+				// make sure the kms is in the proper state for the unit test
+				// before proceeding.
+				_, err := k.GetWrapper(testCtx, scope.Global.String(), kms.KeyPurposeAudit)
+				require.Error(t, err)
 			},
-			wantErr: false,
+			wantPurpose: []kms.KeyPurpose{kms.KeyPurposeAudit},
+			wantErr:     false,
+		},
+		{
+			name:     "reconcile-oidc-key-multiple-scopes",
+			kms:      kms.TestKms(t, conn, wrapper),
+			scopeIds: []string{org.PublicId, org2.PublicId},
+			reader:   rand.Reader,
+			setup: func(k *kms.Kms) {
+				// create initial keys for the test scope ids...
+				for _, id := range []string{org.PublicId, org2.PublicId} {
+					_, err := kms.CreateKeysTx(context.Background(), rw, rw, wrapper, rand.Reader, id)
+					require.NoError(t, err)
+				}
+				db.TestDeleteWhere(t, conn, func() interface{} { i := kms.AllocOidcKey(); return &i }(), "1=1")
+				// make sure the kms is in the proper state for the unit test
+				// before proceeding.
+				for _, id := range []string{org.PublicId, org2.PublicId} {
+					_, err := k.GetWrapper(testCtx, id, kms.KeyPurposeAudit)
+					require.Error(t, err)
+				}
+			},
+			wantPurpose: []kms.KeyPurpose{kms.KeyPurposeOidc},
 		},
 	}
 	for _, tt := range tests {
@@ -188,14 +219,15 @@ func TestKms_ReconcileKeys(t *testing.T) {
 			assert, require := assert.New(t), require.New(t)
 			// start with no keys...
 			db.TestDeleteWhere(t, conn, func() interface{} { i := kms.AllocRootKey(); return &i }(), "1=1")
+
+			// create initial keys for the global scope...
 			_, err := kms.CreateKeysTx(context.Background(), rw, rw, wrapper, rand.Reader, scope.Global.String())
 			require.NoError(err)
 
 			if tt.setup != nil {
-				tt.setup()
+				tt.setup(tt.kms)
 			}
-
-			err = tt.kms.ReconcileKeys(testCtx, tt.reader)
+			err = tt.kms.ReconcileKeys(testCtx, tt.reader, kms.WithScopeIds(tt.scopeIds...))
 			if tt.wantErr {
 				assert.Error(err)
 				if tt.wantErrMatch != nil {
@@ -207,6 +239,16 @@ func TestKms_ReconcileKeys(t *testing.T) {
 				return
 			}
 			assert.NoError(err)
+			if len(tt.scopeIds) > 0 {
+				for _, id := range tt.scopeIds {
+					for _, p := range tt.wantPurpose {
+						_, err := tt.kms.GetWrapper(testCtx, id, p)
+						require.NoError(err)
+					}
+				}
+			}
+			_, err = tt.kms.GetWrapper(testCtx, scope.Global.String(), kms.KeyPurposeAudit)
+			require.NoError(err)
 		})
 	}
 }

internal/kms/options.go

@@ -26,6 +26,7 @@ type options struct {
 	withRepository        *Repository
 	withOrderByVersion    db.OrderBy
 	withKeyId             string
+	withScopeIds          []string
 }
 
 func getDefaultOptions() options {
@@ -87,3 +88,10 @@ func WithKeyId(keyId string) Option {
 		o.withKeyId = keyId
 	}
 }
+
+// WithScopeIds allows the specifying of optional scope ids.
+func WithScopeIds(scopeId ...string) Option {
+	return func(o *options) {
+		o.withScopeIds = scopeId
+	}
+}

internal/servers/controller/controller.go

@@ -220,7 +220,21 @@ func New(ctx context.Context, conf *Config) (*Controller, error) {
 	); err != nil {
 		return nil, fmt.Errorf("error adding config keys to kms: %w", err)
 	}
-	if err := c.kms.ReconcileKeys(ctx, c.conf.SecureRandomReader); err != nil {
+
+	// we need to get all the scopes so we can reconcile the DEKs for each scope.
+	iamRepo, err := iam.NewRepository(dbase, dbase, c.kms, iam.WithRandomReader(c.conf.SecureRandomReader))
+	if err != nil {
+		return nil, fmt.Errorf("unable to initialize iam repository: %w", err)
+	}
+	allScopes, err := iamRepo.ListScopesRecursively(ctx, scope.Global.String())
+	if err != nil {
+		return nil, fmt.Errorf("error listing all scopes for reconciling keys: %w", err)
+	}
+	reconcileScopeIds := make([]string, 0, len(allScopes))
+	for _, s := range allScopes {
+		reconcileScopeIds = append(reconcileScopeIds, s.PublicId)
+	}
+	if err := c.kms.ReconcileKeys(ctx, c.conf.SecureRandomReader, kms.WithScopeIds(reconcileScopeIds...)); err != nil {
 		return nil, fmt.Errorf("error reconciling kms keys: %w", err)
 	}
 

internal/servers/controller/controller_test.go

@@ -6,14 +6,17 @@ import (
 
 	"github.com/hashicorp/boundary/internal/cmd/base"
 	"github.com/hashicorp/boundary/internal/db"
+	"github.com/hashicorp/boundary/internal/iam"
 	"github.com/hashicorp/boundary/internal/kms"
+	"github.com/hashicorp/boundary/internal/types/scope"
 	"github.com/hashicorp/go-secure-stdlib/listenerutil"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestController_New(t *testing.T) {
 	t.Run("ReconcileKeys", func(t *testing.T) {
-		require := require.New(t)
+		assert, require := assert.New(t), require.New(t)
 		testCtx := context.Background()
 		ctx, cancel := context.WithCancel(context.Background())
 		tc := &TestController{
@@ -22,16 +25,53 @@ func TestController_New(t *testing.T) {
 			cancel: cancel,
 			opts:   nil,
 		}
+
+		// TestControllerConfig(...) will create initial scopes
 		conf := TestControllerConfig(t, ctx, tc, nil)
+		org, _ := iam.TestScopes(t, iam.TestRepo(t, conf.Server.Database, conf.RootKms))
 
 		// this tests a scenario where there is an audit DEK
-		c, err := New(testCtx, conf)
+		_, err := New(testCtx, conf)
 		require.NoError(err)
 
+		verifyFn := func() {
+			// verify audit DEKs
+			kmsCache := kms.TestKms(t, conf.Server.Database, conf.RootKms)
+			w, err := kmsCache.GetWrapper(testCtx, scope.Global.String(), kms.KeyPurposeAudit)
+			require.NoError(err)
+			assert.NotNil(w)
+			w, err = kmsCache.GetWrapper(testCtx, scope.Global.String(), kms.KeyPurposeOidc)
+			require.NoError(err)
+			assert.NotNil(w)
+			w, err = kmsCache.GetWrapper(testCtx, org.PublicId, kms.KeyPurposeOidc)
+			require.NoError(err)
+			assert.NotNil(w)
+		}
+
+		verifyFn()
+
 		// this tests a scenario where there is NOT an audit DEK
-		db.TestDeleteWhere(t, c.conf.Server.Database, func() interface{} { i := kms.AllocAuditKey(); return &i }(), "1=1")
+		db.TestDeleteWhere(t, conf.Server.Database, func() interface{} { i := kms.AllocAuditKey(); return &i }(), "1=1")
+		db.TestDeleteWhere(t, conf.Server.Database, func() interface{} { i := kms.AllocOidcKey(); return &i }(), "1=1")
+
+		// re-init an empty cache and assert that the DEKs are not there.
+		kmsCache := kms.TestKms(t, conf.Server.Database, conf.RootKms)
+		w, err := kmsCache.GetWrapper(testCtx, scope.Global.String(), kms.KeyPurposeAudit)
+		require.Error(err)
+		assert.Nil(w)
+		w, err = kmsCache.GetWrapper(testCtx, scope.Global.String(), kms.KeyPurposeOidc)
+		require.Error(err)
+		assert.Nil(w)
+		w, err = kmsCache.GetWrapper(testCtx, org.PublicId, kms.KeyPurposeOidc)
+		require.Error(err)
+		assert.Nil(w)
+
+		// New(...) will reconcile the keys
 		_, err = New(testCtx, conf)
 		require.NoError(err)
+
+		// verify audit DEKs
+		verifyFn()
 	})
 }