assumeRole chain support #224

waahm7 · 2024-01-10T22:22:51Z

Description of changes:
Adds support for STS assumeRole chaining.
Rules:

When chaining assume role profiles, SDKs must terminate the chain as soon as they hit a profile with static credentials even if the profile also contains a source_profile.
A profile is allowed to use itself as a source profile for static credentials.

Examples:

ProfileA contains static creds
- Return credentials from the profile
ProfileA contains static creds as well as source profile set to ProfileA
- Use static creds in ProfileA to call the sts assumeRole and returns the credentials from the sts assumeRole call.
ProfileA contains source_profile set to ProfileB. ProfileB contains static creds.
- Use static creds in ProfileB to call the sts assumeRole and return the credentials from the sts assumeRole call.
ProfileA contains source_profile set to ProfileB. ProfileB contains source_profile set to ProfileC. ProfileC contains static creds.
- Use static creds in ProfileC to call the sts assumeRole with role_arn from profileB.
- Use the response creds to call sts assumeRole again with role_arn from profileA.
- Return creds from the second STS call.
ProfileA contains source_profile set to ProfileB. ProfileB contains source_profile set to ProfileC and static creds. ProfileC contains static creds.
- Use static creds in ProfileB to call the sts assumeRole with role_arn from profileA.
- Return creds from the STS call.
ProfileA contains source_profile set to ProfileB. ProfileB contains source_profile set to ProfileA with/without static creds.
- Error out with profile contains a cycle
ProfileA contains source_profile set to ProfileB. ProfileB contains source_profile set to ProfileC and also contains partial static credentials. Profile C contains credentials.
- Error out with partial credentials error.
ProfileA contains source_profile set to ProfileB and static creds. ProfileB contains static creds.
- Use static creds in ProfileB to call the sts assumeRole and return the credentials from the sts assumeRole call.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

codecov-commenter · 2024-01-10T23:28:46Z

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (3ce2463) 79.57% compared to head (cb467b5) 79.62%.

Files	Patch %	Lines
source/credentials_provider_profile.c	89.18%	4 Missing ⚠️
source/credentials_provider_sts.c	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #224      +/-   ##
==========================================
+ Coverage   79.57%   79.62%   +0.05%     
==========================================
  Files          33       33              
  Lines        5798     5832      +34     
==========================================
+ Hits         4614     4644      +30     
- Misses       1184     1188       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

DmitriyMusatkin · 2024-01-17T21:55:16Z

source/credentials_provider_profile.c

+                allocator,
+                3,
+                aws_hash_byte_cursor_ptr,
+                (aws_hash_callback_eq_fn *)aws_byte_cursor_eq,


any function type casting is ub. it will probably work in most cases, but its still better to wrap

DmitriyMusatkin · 2024-01-17T21:57:16Z

source/credentials_provider_profile.c

@@ -428,11 +455,35 @@ struct aws_credentials_provider *aws_credentials_provider_new_profile(
        goto on_finished;
    }
    const struct aws_profile_property *role_arn_property = aws_profile_get_property(profile, s_role_arn_name);
+    bool profile_contains_access_key = aws_profile_get_property(profile, s_access_key_id_profile_var);
+    bool profile_contains_secret_access_key = aws_profile_get_property(profile, s_secret_access_key_profile_var);
+    bool profile_contains_credentials = profile_contains_access_key || profile_contains_secret_access_key;


should this be end? cred should not be valid if it only contains one of the 2

It's an Or condition so that we don't treat the profile as an STS profile, even if partial creds are present at https://github.com/awslabs/aws-c-auth/pull/224/files#diff-f7cca12a8538a30712ad4b18e39bb61a240457b820982e916f6f27e76628c64fR477. Later, we fail the creation as invalid if either of them is missing at https://github.com/awslabs/aws-c-auth/pull/224/files#diff-f7cca12a8538a30712ad4b18e39bb61a240457b820982e916f6f27e76628c64fR481. This handles cases where the config file is as follows:

[profile userA] role_arn = arn source_profile = userB [profile userB] role_arn = arn aws_access_key_id = accessJey source_profile = userC [profile userC] aws_access_key_id = acess_key secret_access_key_id = secret

We should fail the request at profileB due to partial credentials instead of using creds from profileC. I copied this behavior from the AWS CLI.

source/credentials_provider_profile.c

Co-authored-by: Michael Graeb <[email protected]>

waahm7 added 5 commits January 10, 2024 13:43

Initial assumeRole chain

67c213b

temp recursion detection support

2eae450

WIP use static cred if available instead of going through the chain

d93a72b

refactor

08de613

fix declaration

17a41d0

waahm7 added 17 commits January 10, 2024 15:50

Add cycle detection test

877f0b1

Add tests for more cases

c1912cc

cleanup includes

4df49d2

Fix chain with static creds case

6922cf9

remove const and rename

71b94f7

Update comment

69715ca

rename error code

b04d960

update comment

f66992b

Use profile provider even if one of the creds are available

4136812

Cleanup contains creds logic

9cb8a8f

fix typo

f5acf59

update comment

1093132

Re-add raise error

b9dcd63

Update tests

84d4891

provider error logs

91a53ed

update profile

a98b6c5

Reset the test

dcf1ec0

waahm7 changed the title ~~WIP | assumeRole chain support~~ assumeRole chain support Jan 17, 2024

waahm7 marked this pull request as ready for review January 17, 2024 00:39

DmitriyMusatkin reviewed Jan 17, 2024

View reviewed changes

waahm7 added 3 commits January 17, 2024 15:19

add comment and fix ignore case matching

e1a91ac

Use c string instead of byte cursor

9437736

fix typos

cb467b5

graebm approved these changes Jan 17, 2024

View reviewed changes

source/credentials_provider_profile.c Outdated Show resolved Hide resolved

source/credentials_provider_profile.c Outdated Show resolved Hide resolved

waahm7 and others added 5 commits January 17, 2024 16:02

fix comment

3eee62c

Update source/credentials_provider_profile.c

3443882

Co-authored-by: Michael Graeb <[email protected]>

lint fix

e344ad8

undo unintentional changes

21c3f20

undo

e178b9b

DmitriyMusatkin approved these changes Jan 18, 2024

View reviewed changes

waahm7 merged commit 7911fa2 into main Jan 18, 2024
29 checks passed

waahm7 deleted the assumeRoleChain branch January 18, 2024 17:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

assumeRole chain support #224

assumeRole chain support #224

waahm7 commented Jan 10, 2024 •

edited

Loading

codecov-commenter commented Jan 10, 2024 •

edited

Loading

DmitriyMusatkin Jan 17, 2024

DmitriyMusatkin Jan 17, 2024

waahm7 Jan 17, 2024

assumeRole chain support #224

assumeRole chain support #224

Conversation

waahm7 commented Jan 10, 2024 • edited Loading

codecov-commenter commented Jan 10, 2024 • edited Loading

Codecov Report

DmitriyMusatkin Jan 17, 2024

Choose a reason for hiding this comment

DmitriyMusatkin Jan 17, 2024

Choose a reason for hiding this comment

waahm7 Jan 17, 2024

Choose a reason for hiding this comment

waahm7 commented Jan 10, 2024 •

edited

Loading

codecov-commenter commented Jan 10, 2024 •

edited

Loading