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

add role assumption for s3 source #1477

Merged
merged 32 commits into from
Aug 18, 2023
Merged

add role assumption for s3 source #1477

merged 32 commits into from
Aug 18, 2023

Conversation

codevbus
Copy link
Contributor

Adds assumeRole for AWS/S3 Source

@codevbus codevbus requested review from a team as code owners July 11, 2023 16:06
@codevbus codevbus marked this pull request as draft July 11, 2023 16:06
@codevbus codevbus marked this pull request as ready for review July 22, 2023 05:38
Copy link
Collaborator

@mcastorina mcastorina left a comment

Choose a reason for hiding this comment

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

Looking good! Just one more piece to wire up and an ineffectual assignment to clean up.

main.go Outdated Show resolved Hide resolved
pkg/sources/s3/s3.go Outdated Show resolved Hide resolved
pkg/sources/s3/s3.go Outdated Show resolved Hide resolved
pkg/sources/s3/s3.go Outdated Show resolved Hide resolved
Mike Vanbuskirk added 2 commits July 26, 2023 15:08
@codevbus codevbus marked this pull request as draft July 28, 2023 03:48
Mike Vanbuskirk added 5 commits July 28, 2023 15:53
- Make sure s3 struct is populated with roles
- add separate new client instantiation for role-based access
- iterates through each role
@codevbus codevbus marked this pull request as ready for review July 28, 2023 20:11
@codevbus
Copy link
Contributor Author

codevbus commented Jul 28, 2023

@mcastorina @dustin-decker - This is ready for review. Some contextual notes and possible discussion points:

  • I ended up creating a separate function to handle generating a client for roles. It seemed much easier to break that functionality out vs. nesting a bunch of if/else and iteration inside an auth client
  • The S3_Unauthenticated type is actually what triggers if I run Trufflehog locally and have AWS credentials set in local files (~/.aws/credentials)
  • The S3_Unauthenticated case defaulted to explicit bucket inputs for generating a list of buckets to scan. I duplicated the functionality under the previous case to enumerate all of the buckets the IAM identity has the ability to make ListBuckets API call for.
  • Interestingly, if it defaults to the behavior of enumerating all buckets via ListBuckets, it will say scanning bucket and will not throw an error, even if the IAM policy does not grant the GetObject capability.
  • If you pass an explicit list of buckets and an explicit list of roles, it will attempted to scan each bucket with both IAM roles.

pkg/sources/s3/s3.go Show resolved Hide resolved
pkg/sources/s3/s3.go Outdated Show resolved Hide resolved
pkg/sources/s3/s3.go Outdated Show resolved Hide resolved
pkg/sources/s3/s3.go Outdated Show resolved Hide resolved
pkg/sources/s3/s3.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@mcastorina mcastorina left a comment

Choose a reason for hiding this comment

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

lgtm - nice work!

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@codevbus codevbus merged commit 64dd49f into main Aug 18, 2023
9 checks passed
@codevbus codevbus deleted the add-assume-role-aws branch August 18, 2023 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants