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

feature/use-aws-user-id-sso #665

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

omaotzu
Copy link

@omaotzu omaotzu commented May 22, 2024

Firstly to caveat this is my first time contributing to a project and also my first time writing anything in go so please bear with me.

My knowledge isn't great enough to foresee any untoward complications but we ran into several issues related to the randomised gtnd user id and thought I would give it a crack.

Any guidance or feedback would be greatly appreciated and understand that more work may be required on my part (or you can just outright dismiss this 😆 if it's stupid or my approach is wildly off)

Primarily the issues that we faced were the same as outlined within this issue: #614

What changed?

  • Optionally call stsClient.GetCallerIdentity when assuming to fetch the user id attach that via the role session name
  • Provide a config flag (uu) so this can optionally be used within the cli
  • Called Getenv to check for the env file flag to permanently within global environment variables

Why?

  • Firstly the random gtnd user id effectively made cloudtrail tracing by user physically impossible, each individual session had its own id and it was very difficult to attribute individual user actions to a specific user. This posed security concerns for us as we couldn;t use global deny all policies on these roles as there was no user specific attribute to add into the iam.
  • Some things within the aws console are specific to the user the created them. A few examples of this live within the lambda console itself. Private saved events for triggering are unique to the user and thus only exist for the lifespan on the unique gtnd user id. Some of our data team also wish to manually edit lambda function code in console within non production accounts to quickly iteration on solutions, but these are also unique to the specific user.

Thus finding a method of attributing a userId that was unique to the user rather to the session is imperative for us to continue using this package.

How did you test it?

  • Building the package locally with the proposed changes and adding in the config flag followed by running sts's get caller identity outputted the the random gtnd id without the flag and my specific user with the flag
  • Adding in GRANTED_USE_USER_ID=true within my zshrc defaulted this to always specific user when I assumed
  • Assuming into a variety of our accounts and checking cloudtrail in each of these environments allowed my to search specifically for my user id
  • Adding in an explicit deny all policy to a role I had actively assumed that was specifically attributed to my user id immediately blocked all permissions within aws

Potential risks

  • Error catching
  • Splitting of the called.UserId by : rather than a more thorough approach
  • Unsure and more feedback would be greatly appreciated!

Is patch release candidate?

Link to relevant docs PRs

…tionally call fire GetCallerIdentity and remove the principle id from the user id returned and add that into the roll sessioname
@omaotzu
Copy link
Author

omaotzu commented May 28, 2024

@shwethaumashanker Any chance of taking a look at this and getting some guidance / feedback if possible?

@chrnorm
Copy link
Contributor

chrnorm commented May 28, 2024

Hi @omaotzu, thanks so much for opening a contribution here. I certainly agree that the current random IDs make attribution difficult in CloudTrail and would like to improve this.

One concern I have is that the RoleSessionName field is user-controlled. If you rely on this for detection, it is possible for an attacker to manipulate this field and change the user ID that events are attributed to. Are you aware if there are any IAM conditions that we'd be able to recommend here to ensure users can only apply a RoleSessionName that matches claims from the IAM Identity Center SAML provider?

@omaotzu
Copy link
Author

omaotzu commented May 29, 2024

@chrnorm thanks for your swift response and useful feedback and now that you've mentioned you are correct.

Let me take it away and I'll get back to you as soon as I can!

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

2 participants