diff --git a/CHANGELOG.md b/CHANGELOG.md index 94afe9a94c..0843205589 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/internal/kms/const.go b/internal/kms/const.go index 1ffba56f96..54020a20d6 100644 --- a/internal/kms/const.go +++ b/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 diff --git a/internal/kms/kms.go b/internal/kms/kms.go index 9f33748431..dafa286637 100644 --- a/internal/kms/kms.go +++ b/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 } diff --git a/internal/kms/kms_ext_test.go b/internal/kms/kms_ext_test.go index 8b0be3c813..7c8ece8bee 100644 --- a/internal/kms/kms_ext_test.go +++ b/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) }) } } diff --git a/internal/kms/options.go b/internal/kms/options.go index b599ba78a1..4473d2b4b7 100644 --- a/internal/kms/options.go +++ b/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 + } +} diff --git a/internal/servers/controller/controller.go b/internal/servers/controller/controller.go index 3e4bce496d..35144a5258 100644 --- a/internal/servers/controller/controller.go +++ b/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) } diff --git a/internal/servers/controller/controller_test.go b/internal/servers/controller/controller_test.go index 1866b1a951..20c732fdaa 100644 --- a/internal/servers/controller/controller_test.go +++ b/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() }) }