From 24df33665854c59bcecc50623c86c05fb7341598 Mon Sep 17 00:00:00 2001 From: kpcraig <3031348+kpcraig@users.noreply.github.com> Date: Wed, 11 Oct 2023 17:06:58 +0000 Subject: [PATCH] backport of commit 30f19b383f4c5574d7a5175110f5d14154955cc1 --- builtin/logical/aws/path_static_roles.go | 14 ++++ builtin/logical/aws/path_static_roles_test.go | 65 +++++++++++++++++-- changelog/23528.txt | 3 + website/content/api-docs/secret/aws.mdx | 3 +- 4 files changed, 78 insertions(+), 7 deletions(-) create mode 100644 changelog/23528.txt diff --git a/builtin/logical/aws/path_static_roles.go b/builtin/logical/aws/path_static_roles.go index 52f1eaf3efba..f07eab54ab18 100644 --- a/builtin/logical/aws/path_static_roles.go +++ b/builtin/logical/aws/path_static_roles.go @@ -212,6 +212,20 @@ func (b *backend) pathStaticRolesWrite(ctx context.Context, req *logical.Request if err != nil { return nil, fmt.Errorf("failed to add item into the rotation queue for role %q: %w", config.Name, err) } + } else { + // creds already exist, so all we need to do is update the rotation + // what here stays the same and what changes? Can we change the name? + i, err := b.credRotationQueue.PopByKey(config.Name) + if err != nil { + return nil, fmt.Errorf("expected an item with name %q, but got an error: %w", config.Name, err) + } + i.Value = config + // update the next rotation to occur at now + the new rotation period + i.Priority = time.Now().Add(config.RotationPeriod).Unix() + err = b.credRotationQueue.Push(i) + if err != nil { + return nil, fmt.Errorf("failed to add updated item into the rotation queue for role %q: %w", config.Name, err) + } } return &logical.Response{ diff --git a/builtin/logical/aws/path_static_roles_test.go b/builtin/logical/aws/path_static_roles_test.go index f46981d6cee1..a56225b57580 100644 --- a/builtin/logical/aws/path_static_roles_test.go +++ b/builtin/logical/aws/path_static_roles_test.go @@ -124,12 +124,21 @@ func TestStaticRolesWrite(t *testing.T) { bgCTX := context.Background() cases := []struct { - name string - opts []awsutil.MockIAMOption + name string + // objects to return from mock IAM. + // You'll need a GetUserOutput (to validate the existence of the user being written, + // the keys the user has already been assigned, + // and the new key vault requests. + opts []awsutil.MockIAMOption // objects to return from the mock IAM + // the name, username if updating, and rotation_period of the user. This is the inbound request the cod would get. data map[string]interface{} expectedError bool findUser bool - isUpdate bool + // if data is sent the name "johnny", then we'll match an existing user with rotation period 24 hours. + isUpdate bool + newPriority int64 // update time of new item in queue, skip if isUpdate false. There is a wiggle room of 5 seconds + // so the deltas between the old and the new update time should be larger than that to ensure the difference + // can be detected. }{ { name: "happy path", @@ -168,7 +177,7 @@ func TestStaticRolesWrite(t *testing.T) { expectedError: true, }, { - name: "update existing user", + name: "update existing user, decreased rotation duration", opts: []awsutil.MockIAMOption{ awsutil.WithGetUserOutput(&iam.GetUserOutput{User: &iam.User{UserName: aws.String("john-doe"), UserId: aws.String("unique-id")}}), awsutil.WithListAccessKeysOutput(&iam.ListAccessKeysOutput{ @@ -187,8 +196,33 @@ func TestStaticRolesWrite(t *testing.T) { "name": "johnny", "rotation_period": "19m", }, - findUser: true, - isUpdate: true, + findUser: true, + isUpdate: true, + newPriority: time.Now().Add(19 * time.Minute).Unix(), + }, + { + name: "update existing user, increased rotation duration", + opts: []awsutil.MockIAMOption{ + awsutil.WithGetUserOutput(&iam.GetUserOutput{User: &iam.User{UserName: aws.String("john-doe"), UserId: aws.String("unique-id")}}), + awsutil.WithListAccessKeysOutput(&iam.ListAccessKeysOutput{ + AccessKeyMetadata: []*iam.AccessKeyMetadata{}, + IsTruncated: aws.Bool(false), + }), + awsutil.WithCreateAccessKeyOutput(&iam.CreateAccessKeyOutput{ + AccessKey: &iam.AccessKey{ + AccessKeyId: aws.String("abcdefghijklmnopqrstuvwxyz"), + SecretAccessKey: aws.String("zyxwvutsrqponmlkjihgfedcba"), + UserName: aws.String("john-doe"), + }, + }), + }, + data: map[string]interface{}{ + "name": "johnny", + "rotation_period": "40h", + }, + findUser: true, + isUpdate: true, + newPriority: time.Now().Add(40 * time.Hour).Unix(), }, } @@ -269,6 +303,11 @@ func TestStaticRolesWrite(t *testing.T) { expectedData = staticRole } + var actualItem *queue.Item + if c.isUpdate { + actualItem, _ = b.credRotationQueue.PopByKey(expectedData.Name) + } + if u, ok := fieldData.GetOk("username"); ok { expectedData.Username = u.(string) } @@ -289,6 +328,20 @@ func TestStaticRolesWrite(t *testing.T) { if en, an := expectedData.Name, actualData.Name; en != an { t.Fatalf("mismatched role name, expected %q, but got %q", en, an) } + + // one-off to avoid importing/casting + abs := func(x int64) int64 { + if x < 0 { + return -x + } + return x + } + + if c.isUpdate { + if ep, ap := c.newPriority, actualItem.Priority; abs(ep-ap) > 5 { // 5 second wiggle room for how long the test takes + t.Fatalf("mismatched updated priority, expected %d but got %d", ep, ap) + } + } }) } } diff --git a/changelog/23528.txt b/changelog/23528.txt new file mode 100644 index 000000000000..ad9ec4f4b7bd --- /dev/null +++ b/changelog/23528.txt @@ -0,0 +1,3 @@ +```release-note:bug +secrets/aws: update credential rotation deadline when static role rotation period is updated +``` diff --git a/website/content/api-docs/secret/aws.mdx b/website/content/api-docs/secret/aws.mdx index 54cfdbbfab33..9c3b4946a8e0 100644 --- a/website/content/api-docs/secret/aws.mdx +++ b/website/content/api-docs/secret/aws.mdx @@ -586,7 +586,7 @@ $ curl \ } ``` -## Create static role +## Create/Update static role This endpoint creates or updates static role definitions. A static role is a 1-to-1 mapping with an AWS IAM User, which will be adopted and managed by Vault, including rotating it according to the configured `rotation_period`. @@ -613,6 +613,7 @@ is specified as part of the URL. - `rotation_period` `(string/int: )` – Specifies the amount of time Vault should wait before rotating the password. The minimum is 1 minute. Can be specified in either `24h` or `86400` format (see [duration format strings](/vault/docs/concepts/duration-format)). +Updating the rotation period will 'reset' the next rotation to occur at `now` + `rotation_period`. ### Sample payload