From e4fc7f1506ec1b9d2d2b7439a941aaa16190e996 Mon Sep 17 00:00:00 2001 From: Artem Lifshits <55093318+artem-lifshits@users.noreply.github.com> Date: Mon, 13 Feb 2023 14:25:06 +0300 Subject: [PATCH] Clouds: autorotation implementation via periodicfunc (#128) Clouds: autorotation implementation via periodicfunc PR implements autorotation for clouds root passwords via periodicfunc which happens every 1h. All root passwords for all clouds will be rotated, if root_password_ttl was not set on creation - a default 60 days rotation duration will be used. Acceptance tests vault-plugin-secrets-openstack % make functional Running acceptance tests... === RUN TestPlugin === RUN TestPlugin/TestCloudLifecycle === RUN TestPlugin/TestCloudLifecycle/WriteCloud === RUN TestPlugin/TestCloudLifecycle/ReadCloud === RUN TestPlugin/TestCloudLifecycle/ListClouds === RUN TestPlugin/TestCloudLifecycle/ListClouds/method-LIST === PAUSE TestPlugin/TestCloudLifecycle/ListClouds/method-LIST === RUN TestPlugin/TestCloudLifecycle/ListClouds/method-GET === PAUSE TestPlugin/TestCloudLifecycle/ListClouds/method-GET === CONT TestPlugin/TestCloudLifecycle/ListClouds/method-LIST === CONT TestPlugin/TestCloudLifecycle/ListClouds/method-GET === RUN TestPlugin/TestCloudLifecycle/DeleteCloud === RUN TestPlugin/TestCredsLifecycle === RUN TestPlugin/TestCredsLifecycle/user_password === RUN TestPlugin/TestCredsLifecycle/user_domain_id_token === RUN TestPlugin/TestCredsLifecycle/root_token === RUN TestPlugin/TestCredsLifecycle/user_token === RUN TestPlugin/TestInfo === RUN TestPlugin/TestRoleLifecycle === RUN TestPlugin/TestRoleLifecycle/WriteRole === RUN TestPlugin/TestRoleLifecycle/ReadRole === RUN TestPlugin/TestRoleLifecycle/ListRoles === RUN TestPlugin/TestRoleLifecycle/ListRoles/method-LIST === PAUSE TestPlugin/TestRoleLifecycle/ListRoles/method-LIST === RUN TestPlugin/TestRoleLifecycle/ListRoles/method-GET === PAUSE TestPlugin/TestRoleLifecycle/ListRoles/method-GET === CONT TestPlugin/TestRoleLifecycle/ListRoles/method-LIST === CONT TestPlugin/TestRoleLifecycle/ListRoles/method-GET === RUN TestPlugin/TestRoleLifecycle/DeleteRole === RUN TestPlugin/TestRootRotate rotate_test.go:65: Cloud with name default1 was created rotate_test.go:68: Cloud with name xe9o was created plugin_test.go:337: Cloud with name xe9o has been removed plugin_test.go:337: Cloud with name default1 has been removed === RUN TestPlugin/TestStaticCredsLifecycle === RUN TestPlugin/TestStaticCredsLifecycle/user_password === RUN TestPlugin/TestStaticCredsLifecycle/user_token_project_id === RUN TestPlugin/TestStaticCredsLifecycle/user_token_project_name === RUN TestPlugin/TestStaticCredsLifecycle/user_domain_id_token === RUN TestPlugin/TestStaticRoleLifecycle === RUN TestPlugin/TestStaticRoleLifecycle/WriteRole === RUN TestPlugin/TestStaticRoleLifecycle/ReadRole === RUN TestPlugin/TestStaticRoleLifecycle/ListRoles === RUN TestPlugin/TestStaticRoleLifecycle/ListRoles/method-LIST === PAUSE TestPlugin/TestStaticRoleLifecycle/ListRoles/method-LIST === RUN TestPlugin/TestStaticRoleLifecycle/ListRoles/method-GET === PAUSE TestPlugin/TestStaticRoleLifecycle/ListRoles/method-GET === CONT TestPlugin/TestStaticRoleLifecycle/ListRoles/method-LIST === CONT TestPlugin/TestStaticRoleLifecycle/ListRoles/method-GET === RUN TestPlugin/TestStaticRoleLifecycle/DeleteRole --- PASS: TestPlugin (31.87s) --- PASS: TestPlugin/TestCloudLifecycle (0.05s) --- PASS: TestPlugin/TestCloudLifecycle/WriteCloud (0.04s) --- PASS: TestPlugin/TestCloudLifecycle/ReadCloud (0.00s) --- PASS: TestPlugin/TestCloudLifecycle/ListClouds (0.00s) --- PASS: TestPlugin/TestCloudLifecycle/ListClouds/method-LIST (0.00s) --- PASS: TestPlugin/TestCloudLifecycle/ListClouds/method-GET (0.00s) --- PASS: TestPlugin/TestCloudLifecycle/DeleteCloud (0.00s) --- PASS: TestPlugin/TestCredsLifecycle (7.94s) --- PASS: TestPlugin/TestCredsLifecycle/user_password (1.89s) --- PASS: TestPlugin/TestCredsLifecycle/user_domain_id_token (1.98s) --- PASS: TestPlugin/TestCredsLifecycle/root_token (0.82s) --- PASS: TestPlugin/TestCredsLifecycle/user_token (2.35s) --- PASS: TestPlugin/TestInfo (0.00s) --- PASS: TestPlugin/TestRoleLifecycle (0.59s) --- PASS: TestPlugin/TestRoleLifecycle/WriteRole (0.58s) --- PASS: TestPlugin/TestRoleLifecycle/ReadRole (0.00s) --- PASS: TestPlugin/TestRoleLifecycle/ListRoles (0.00s) --- PASS: TestPlugin/TestRoleLifecycle/ListRoles/method-GET (0.00s) --- PASS: TestPlugin/TestRoleLifecycle/ListRoles/method-LIST (0.00s) --- PASS: TestPlugin/TestRoleLifecycle/DeleteRole (0.00s) --- PASS: TestPlugin/TestRootRotate (4.56s) --- PASS: TestPlugin/TestStaticCredsLifecycle (15.54s) --- PASS: TestPlugin/TestStaticCredsLifecycle/user_password (3.26s) --- PASS: TestPlugin/TestStaticCredsLifecycle/user_token_project_id (3.70s) --- PASS: TestPlugin/TestStaticCredsLifecycle/user_token_project_name (3.76s) --- PASS: TestPlugin/TestStaticCredsLifecycle/user_domain_id_token (3.77s) --- PASS: TestPlugin/TestStaticRoleLifecycle (2.72s) --- PASS: TestPlugin/TestStaticRoleLifecycle/WriteRole (1.01s) --- PASS: TestPlugin/TestStaticRoleLifecycle/ReadRole (0.00s) --- PASS: TestPlugin/TestStaticRoleLifecycle/ListRoles (0.00s) --- PASS: TestPlugin/TestStaticRoleLifecycle/ListRoles/method-LIST (0.00s) --- PASS: TestPlugin/TestStaticRoleLifecycle/ListRoles/method-GET (0.00s) --- PASS: TestPlugin/TestStaticRoleLifecycle/DeleteRole (0.00s) PASS ok github.com/opentelekomcloud/vault-plugin-secrets-openstack/acceptance 32.365s Reviewed-by: Anton Sidelnikov Reviewed-by: Aloento --- doc/source/api.md | 2 +- openstack/backend.go | 89 ++++++++++++++++++++++++++++++++++-- openstack/backend_test.go | 32 +++++++++++++ openstack/path_cloud.go | 2 +- openstack/path_cloud_test.go | 2 +- 5 files changed, 121 insertions(+), 6 deletions(-) diff --git a/doc/source/api.md b/doc/source/api.md index 5153851..a2e360d 100644 --- a/doc/source/api.md +++ b/doc/source/api.md @@ -22,7 +22,7 @@ will overwrite them. * `password` `(string: )` - OpenStack password of the root user. -* `root_password_ttl` `(string: )` - Password rotation period. Default period is six month. +* `root_password_ttl` `(string: )` - Password rotation period. Default period is 2 month. * `username_template` `(string: "vault{{random 8 | lowercase}}")` - Template used for usernames of temporary users. For details on templating syntax please refer to diff --git a/openstack/backend.go b/openstack/backend.go index 46bba4b..356fb48 100644 --- a/openstack/backend.go +++ b/openstack/backend.go @@ -3,7 +3,10 @@ package openstack import ( "context" "fmt" + "github.com/gophercloud/gophercloud/openstack/identity/v3/users" + "github.com/hashicorp/go-multierror" "github.com/opentelekomcloud/vault-plugin-secrets-openstack/openstack/common" + "net/http" "sync" "time" @@ -32,8 +35,8 @@ type sharedCloud struct { type backend struct { *framework.Backend - - clouds map[string]*sharedCloud + clouds map[string]*sharedCloud + checkAutoRotateAfter time.Time } func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) { @@ -62,7 +65,8 @@ func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, secretToken(b), secretUser(b), }, - BackendType: logical.TypeLogical, + BackendType: logical.TypeLogical, + PeriodicFunc: b.periodicFunc, } if err := b.Setup(ctx, conf); err != nil { @@ -147,3 +151,82 @@ func (c *sharedCloud) initClient(ctx context.Context, s logical.Storage) error { return nil } + +func (b *backend) periodicFunc(ctx context.Context, req *logical.Request) error { + // Check for autorotation once an hour to avoid unnecessarily iterating + // over all keys too frequently. + if time.Now().Before(b.checkAutoRotateAfter) { + return nil + } + b.Logger().Debug("periodic func", "rotate-root", "rotation cycle in progress") + b.checkAutoRotateAfter = time.Now().Add(1 * time.Hour) + + return b.autoRotateKeys(ctx, req) +} + +func (b *backend) autoRotateKeys(ctx context.Context, req *logical.Request) error { + keys, err := req.Storage.List(ctx, "clouds/") + if err != nil { + return err + } + + // Collect errors in a multierror to ensure a single failure doesn't prevent + // all keys from being rotated. + var errs *multierror.Error + + for _, key := range keys { + cloudEntry := b.getSharedCloud(key) + if cloudEntry == nil { + continue + } + + err = b.rotateIfRequired(ctx, req, cloudEntry) + if err != nil { + errs = multierror.Append(errs, err) + } + } + b.Logger().Debug("periodic func", "rotate-root", "rotation cycle complete") + return errs.ErrorOrNil() +} + +func (b *backend) rotateIfRequired(ctx context.Context, req *logical.Request, sCloud *sharedCloud) error { + cloudConfig, err := sCloud.getCloudConfig(ctx, req.Storage) + if err != nil { + return err + } + if time.Now().After(cloudConfig.RootPasswordExpirationDate) { + client, err := sCloud.getClient(ctx, req.Storage) + if err != nil { + return logical.CodedError(http.StatusConflict, common.LogHttpError(err).Error()) + } + newPassword, err := sCloud.passwords.Generate(ctx) + if err != nil { + return err + } + + // make sure we don't use this cloud until the password is changed + sCloud.lock.Lock() + defer sCloud.lock.Unlock() + + user, err := tokens.Get(client, client.Token()).ExtractUser() + if err != nil { + return logical.CodedError(http.StatusConflict, common.LogHttpError(err).Error()) + } + err = users.ChangePassword(client, user.ID, users.ChangePasswordOpts{ + Password: newPassword, + OriginalPassword: cloudConfig.Password, + }).ExtractErr() + if err != nil { + errorMessage := fmt.Sprintf("error changing root password: %s", common.LogHttpError(err).Error()) + return logical.CodedError(http.StatusConflict, errorMessage) + } + cloudConfig.Password = newPassword + cloudConfig.RootPasswordExpirationDate = time.Now().Add(cloudConfig.RootPasswordTTL) + + if err := cloudConfig.save(ctx, req.Storage); err != nil { + return err + } + b.Logger().Debug("password rotated", "cloud", cloudConfig.Name) + } + return nil +} diff --git a/openstack/backend_test.go b/openstack/backend_test.go index 63db846..2da9df3 100644 --- a/openstack/backend_test.go +++ b/openstack/backend_test.go @@ -3,6 +3,7 @@ package openstack import ( "context" "fmt" + "github.com/hashicorp/vault/sdk/helper/logging" "net/http" "sync" "testing" @@ -13,6 +14,7 @@ import ( th "github.com/gophercloud/gophercloud/testhelper" thClient "github.com/gophercloud/gophercloud/testhelper/client" "github.com/hashicorp/go-hclog" + log "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" "github.com/stretchr/testify/assert" @@ -25,6 +27,8 @@ const ( failVerbPut failVerbList failVerbDelete + defaultLeaseTTLHr = 1 * time.Hour + maxLeaseTTLHr = 12 * time.Hour ) func testBackend(t *testing.T, fvs ...failVerb) (*backend, logical.Storage) { @@ -165,3 +169,31 @@ func TestSharedCloud_client(t *testing.T) { assert.NoError(t, err) }) } + +func TestPeriodicFuncNilConfig(t *testing.T) { + th.SetupHTTP() + defer th.TeardownHTTP() + + b, _ := testBackend(t) + + config := &logical.BackendConfig{ + Logger: logging.NewVaultLogger(log.Trace), + System: &logical.StaticSystemView{ + DefaultLeaseTTLVal: defaultLeaseTTLHr, + MaxLeaseTTLVal: maxLeaseTTLHr, + }, + StorageView: &logical.InmemStorage{}, + } + err := b.Setup(context.Background(), config) + if err != nil { + t.Fatalf("unable to create backend: %v", err) + } + + err = b.periodicFunc(context.Background(), &logical.Request{ + Storage: config.StorageView, + }) + + if err != nil { + t.Fatalf("periodicFunc error not nil: %v", err) + } +} diff --git a/openstack/path_cloud.go b/openstack/path_cloud.go index 2a15d88..211a8a0 100644 --- a/openstack/path_cloud.go +++ b/openstack/path_cloud.go @@ -21,7 +21,7 @@ Configure the root credentials for an OpenStack cloud using the above parameters pathCloudListHelpDesc = `List existing OpenStack clouds by name.` DefaultUsernameTemplate = "vault{{random 8 | lowercase}}" - defaultRootPasswordTTL = 4380 * time.Hour + defaultRootPasswordTTL = 1440 * time.Hour ) func storageCloudKey(name string) string { diff --git a/openstack/path_cloud_test.go b/openstack/path_cloud_test.go index 16851c3..600a278 100644 --- a/openstack/path_cloud_test.go +++ b/openstack/path_cloud_test.go @@ -221,7 +221,7 @@ func TestConfig(t *testing.T) { "username": "test-username-1", "user_domain_name": "testUserDomainName", "username_template": "user-{{ .RoleName }}-{{ random 4 }}", - "root_password_ttl": 15768000, + "root_password_ttl": 5184000, "password_policy": "", }, },