From 1b4ff1b1b44801e96f98c346b1258b407365d5fa Mon Sep 17 00:00:00 2001 From: Kyle Schochenmaier Date: Thu, 13 Apr 2023 15:02:39 -0500 Subject: [PATCH] Revert changes to STS leases but keep the ttl field (#20034) * revert STS lease changes, now create a lease for STS credentials but keep the ttl Co-authored-by: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> --- builtin/logical/aws/secret_access_keys.go | 64 ++++++++++++++++------- changelog/20034.txt | 3 ++ 2 files changed, 48 insertions(+), 19 deletions(-) create mode 100644 changelog/20034.txt diff --git a/builtin/logical/aws/secret_access_keys.go b/builtin/logical/aws/secret_access_keys.go index a4c57d278..2f1ac442b 100644 --- a/builtin/logical/aws/secret_access_keys.go +++ b/builtin/logical/aws/secret_access_keys.go @@ -158,15 +158,28 @@ func (b *backend) getFederationToken(ctx context.Context, s logical.Storage, return logical.ErrorResponse("Error generating STS keys: %s", err), awsutil.CheckAWSError(err) } - // STS credentials cannot be revoked so do not create a lease - return &logical.Response{ - Data: map[string]interface{}{ - "access_key": *tokenResp.Credentials.AccessKeyId, - "secret_key": *tokenResp.Credentials.SecretAccessKey, - "security_token": *tokenResp.Credentials.SessionToken, - "ttl": uint64(tokenResp.Credentials.Expiration.Sub(time.Now()).Seconds()), - }, - }, nil + // While STS credentials cannot be revoked/renewed, we will still create a lease since users are + // relying on a non-zero `lease_duration` in order to manage their lease lifecycles manually. + // + ttl := tokenResp.Credentials.Expiration.Sub(time.Now()) + resp := b.Secret(secretAccessKeyType).Response(map[string]interface{}{ + "access_key": *tokenResp.Credentials.AccessKeyId, + "secret_key": *tokenResp.Credentials.SecretAccessKey, + "security_token": *tokenResp.Credentials.SessionToken, + "ttl": uint64(ttl.Seconds()), + }, map[string]interface{}{ + "username": username, + "policy": policy, + "is_sts": true, + }) + + // Set the secret TTL to appropriately match the expiration of the token + resp.Secret.TTL = ttl + + // STS are purposefully short-lived and aren't renewable + resp.Secret.Renewable = false + + return resp, nil } func (b *backend) assumeRole(ctx context.Context, s logical.Storage, @@ -233,16 +246,29 @@ func (b *backend) assumeRole(ctx context.Context, s logical.Storage, return logical.ErrorResponse("Error assuming role: %s", err), awsutil.CheckAWSError(err) } - // STS credentials cannot be revoked so do not create a lease - return &logical.Response{ - Data: map[string]interface{}{ - "access_key": *tokenResp.Credentials.AccessKeyId, - "secret_key": *tokenResp.Credentials.SecretAccessKey, - "security_token": *tokenResp.Credentials.SessionToken, - "arn": *tokenResp.AssumedRoleUser.Arn, - "ttl": uint64(tokenResp.Credentials.Expiration.Sub(time.Now()).Seconds()), - }, - }, nil + // While STS credentials cannot be revoked/renewed, we will still create a lease since users are + // relying on a non-zero `lease_duration` in order to manage their lease lifecycles manually. + // + ttl := tokenResp.Credentials.Expiration.Sub(time.Now()) + resp := b.Secret(secretAccessKeyType).Response(map[string]interface{}{ + "access_key": *tokenResp.Credentials.AccessKeyId, + "secret_key": *tokenResp.Credentials.SecretAccessKey, + "security_token": *tokenResp.Credentials.SessionToken, + "arn": *tokenResp.AssumedRoleUser.Arn, + "ttl": uint64(ttl.Seconds()), + }, map[string]interface{}{ + "username": roleSessionName, + "policy": roleArn, + "is_sts": true, + }) + + // Set the secret TTL to appropriately match the expiration of the token + resp.Secret.TTL = ttl + + // STS are purposefully short-lived and aren't renewable + resp.Secret.Renewable = false + + return resp, nil } func readConfig(ctx context.Context, storage logical.Storage) (rootConfig, error) { diff --git a/changelog/20034.txt b/changelog/20034.txt new file mode 100644 index 000000000..c1050795b --- /dev/null +++ b/changelog/20034.txt @@ -0,0 +1,3 @@ +```release-note: bug +secrets/aws: Revert changes that removed the lease on STS credentials, while leaving the new ttl field in place. +```