Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(aws): add ability to provide a role session name when generating STS credentials #11345

Merged
merged 5 commits into from May 17, 2021

Conversation

RXC001
Copy link
Contributor

@RXC001 RXC001 commented Apr 12, 2021

We have a need for an IAM session ARN to register an AWS CodeDeploy on-premises instance, more info here; the problem comes with renewing the temporary credentials. In Vault, the IAM session ARN associated with newly generated credentials (using this endpoint) is dynamically built with the current time and a random int. Because of this, renewing temporary credentials using the same IAM session ARN would be impossible, as the ARN would be changed when Vault generates new credentials.

The proposed solution comes in two parts. The first is to add a new parameter called role_session_name when generating new credentials. With this, users can pass in a role session name and expect an ARN with a static role session name every time new STS credentials are read from Vault. If role_session_name isn't provided, then the previous behavior of dynamically creating a session name is followed.

The second part of this solution is to return the IAM session ARN generated when Vault assumed the role. In the return payload, the IAM session ARN will be in the arn field. This provides transparency on which IAM user/role the returned credentials are associated with.

@vercel vercel bot temporarily deployed to Preview – vault-storybook April 12, 2021 21:20 Inactive
@vercel vercel bot temporarily deployed to Preview – vault April 12, 2021 21:22 Inactive
Copy link
Contributor

@tomhjp tomhjp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @RXC001, thanks for the contribution! I have a few minor suggestions, but a couple of questions too. Forgive me if I'm missing some context as CodeDeploy is new to me.

  • You mention you can't renew the generated temporary credentials, but I'm unsure why you can't renew them using the Vault lease ID: https://www.vaultproject.io/api-docs/system/leases#renew-lease, the equivalent of vault lease renew aws/creds/my-role/Dmlmy... using the CLI. With the features in this PR, how will you use the ARN to renew the credentials; will it be through Vault or a different mechanism?
  • Assuming you can't use Vault's built-in lease renewal, why do you need the generated ARN to be stable in addition to having the ARN returned in the response? As above, I don't have context on how you plan to handle renewals, but it seems to me like either feature would be enough for you to know the ARN. I feel like I'm probably missing a separate requirement here.

website/content/api-docs/secret/aws.mdx Outdated Show resolved Hide resolved
builtin/logical/aws/secret_access_keys.go Outdated Show resolved Hide resolved
roleSessionName, roleSessionNameWarning = genUsername(displayName, roleName, "iam_user")
} else {
roleSessionName = normalizeDisplayName(roleSessionName)
if len(roleSessionName) > 64 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know the code this is based on doesn't, but please could you add a comment with the source of this number? Is https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_iam-quotas.html#reference_iam-quotas-entity-length the right link?

@ibspoof
Copy link

ibspoof commented May 14, 2021

I work with @RXC001 and was involved in the feature and code.

@tomhjp Will have to check on lease renew method. We are using Vault Agent and haven't tried that method before.

Assuming you can't use Vault's built-in lease renewal, why do you need the generated ARN to be stable in addition to having the ARN returned in the response? As above, I don't have context on how you plan to handle renewals, but it seems to me like either feature would be enough for you to know the ARN. I feel like I'm probably missing a separate requirement here.

CodeDeploy registers an on premise instance based on the session arn (role arn + session name) and as long as the full session arn remains the same across multiple STS provided access key, secret key, session token we can refresh the aws credentials without re-registering the instance. This is why we require a session arn that has a consistent session name associated.

We also have a need outside of code deploy to be able to set a session name on our end when creating new STS creds to track the usage for the specific client in AWS CloudTrail as we have multiple VMs using the same AppRole and therefore currently the same session name with timestamp added. Right now we have to correlate Vault Audit logs and and timestamps to getting STS creds to find one when X VM requested from vault and then match to cloudtrail. From a security standpoint this is both painful and not a process we can automate. With having a settable session name that vault passes through to AWS through VaultAgent we can easily tag the instance requesting the sts creds and match the usage in CloudTrail with a quickly identifying session name. We can also trigger alerts in AWS for non expected session name or a session coming from an unexpected IP with ease.

The reason we added the arn to the response is:

  1. In the current setup vault truncates the session arn with a warning, but doesn't return what the session arn is requiring an additional call to AWS APIs to retrieve the session name vault generated
  2. Allows for validation by the client the session arn/name in the assumed role matches the value expected from vault without having to call AWS API to get the arn
  3. The key details are already present in the response of the key, secret, and session token adding the arn isn't leaking any information

Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>
@tomhjp
Copy link
Contributor

tomhjp commented May 17, 2021

Thanks for that extra context @ibspoof, that makes sense and I can see how this all fits together now. The lease renewal can be considered a separate issue as far as I'm concerned. I'll try and get this merged, although it looks like we have some unrelated failures in CI 😞

EDIT: Also just for posterity, I have pulled this down and tested it, and it all works as expected.

changelog/11345.txt Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview – vault-storybook May 17, 2021 18:02 Inactive
@vercel vercel bot temporarily deployed to Preview – vault May 17, 2021 18:02 Inactive
@kalafut kalafut merged commit 4c8a818 into hashicorp:master May 17, 2021
tvoran pushed a commit that referenced this pull request May 17, 2021
… STS credentials (#11345)

* feat(aws): add ability to provide a sessionName to sts credentials

Co-authored-by: Brad Vernon <bvernon@nvidia.com>
Co-authored-by: Jim Kalafut <jim@kalafut.net>
Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>
tvoran added a commit that referenced this pull request May 17, 2021
… STS credentials (#11345) (#11636)

* feat(aws): add ability to provide a sessionName to sts credentials

Co-authored-by: Brad Vernon <bvernon@nvidia.com>
Co-authored-by: Jim Kalafut <jim@kalafut.net>
Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>
Co-authored-by: Ricardo Cardenas <rcardenas@nvidia.com>
@kalafut kalafut added this to the 1.7.2 milestone May 17, 2021
@ibspoof
Copy link

ibspoof commented May 18, 2021

@tomhjp Thanks!

AndreyZamyslov pushed a commit to yandex-cloud/vault that referenced this pull request Jun 10, 2021
… STS credentials (hashicorp#11345)

* feat(aws): add ability to provide a sessionName to sts credentials

Co-authored-by: Brad Vernon <bvernon@nvidia.com>
Co-authored-by: Jim Kalafut <jim@kalafut.net>
Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>
jartek pushed a commit to jartek/vault that referenced this pull request Sep 11, 2021
… STS credentials (hashicorp#11345)

* feat(aws): add ability to provide a sessionName to sts credentials

Co-authored-by: Brad Vernon <bvernon@nvidia.com>
Co-authored-by: Jim Kalafut <jim@kalafut.net>
Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants