aws: Implementation of IAM Roles anywhere support in aws request signing (https://docs.aws.amazon.com/rolesanywhere/latest/userguide/introduction.html) by nbaws · Pull Request #37193 · envoyproxy/envoy

nbaws · 2024-11-17T23:22:28Z

Commit Message: aws: Implementation of IAM Roles anywhere support in aws request signing
Additional Description: Creates an additional x509 credential type and adds custom signing support for IAM Roles Anywhere
Risk Level: Low
Testing: Unit
Docs Changes: doc update and sample config included
Release Notes: changelog update
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: Nigel Brittain <[email protected]>

repokitteh-read-only · 2024-11-17T23:22:42Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @mattklein123
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #37193 was opened by nbaws.

see: more, trace.

Signed-off-by: Nigel Brittain <[email protected]> update initialisation Signed-off-by: Nigel Brittain <[email protected]> make tests compile Signed-off-by: Nigel Brittain <[email protected]> refactor Signed-off-by: Nigel Brittain <[email protected]> builds ok Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> add test cases Signed-off-by: Nigel Brittain <[email protected]> update test Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> cleanup Signed-off-by: Nigel Brittain <[email protected]> cleanup Signed-off-by: Nigel Brittain <[email protected]> update test cases Signed-off-by: Nigel Brittain <[email protected]> signing test cases Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> documentation Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> content type Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> revert some configs Signed-off-by: Nigel Brittain <[email protected]> revert some changes Signed-off-by: Nigel Brittain <[email protected]> revert some changes Signed-off-by: Nigel Brittain <[email protected]> revert additional change Signed-off-by: Nigel Brittain <[email protected]> partial refactor Signed-off-by: Nigel Brittain <[email protected]> refactor Signed-off-by: Nigel Brittain <[email protected]> refactor and format Signed-off-by: Nigel Brittain <[email protected]> fix mocks Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> documentation Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> webidentity Signed-off-by: Nigel Brittain <[email protected]> still one test case broken Signed-off-by: Nigel Brittain <[email protected]> fixing test cases Signed-off-by: Nigel Brittain <[email protected]> working expiration Signed-off-by: Nigel Brittain <[email protected]> more test cases Signed-off-by: Nigel Brittain <[email protected]> update test cases Signed-off-by: Nigel Brittain <[email protected]> asan Signed-off-by: Nigel Brittain <[email protected]> ssl init Signed-off-by: Nigel Brittain <[email protected]> leaks Signed-off-by: Nigel Brittain <[email protected]> coverage Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> additional test cases Signed-off-by: Nigel Brittain <[email protected]> additional test cases Signed-off-by: Nigel Brittain <[email protected]> data source modifications Signed-off-by: Nigel Brittain <[email protected]> more data source mods Signed-off-by: Nigel Brittain <[email protected]> test case fix Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> cert signing algorithm Signed-off-by: Nigel Brittain <[email protected]> update headers Signed-off-by: Nigel Brittain <[email protected]> update headers Signed-off-by: Nigel Brittain <[email protected]> update Signed-off-by: Nigel Brittain <[email protected]> test case Signed-off-by: Nigel Brittain <[email protected]> test case Signed-off-by: Nigel Brittain <[email protected]> update test case Signed-off-by: Nigel Brittain <[email protected]> no test leak Signed-off-by: Nigel Brittain <[email protected]> updated test case Signed-off-by: Nigel Brittain <[email protected]> update tests Signed-off-by: Nigel Brittain <[email protected]> test update Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> format Signed-off-by: Nigel Brittain <[email protected]> update test case Signed-off-by: Nigel Brittain <[email protected]> update test case Signed-off-by: Nigel Brittain <[email protected]> update test cases Signed-off-by: Nigel Brittain <[email protected]> update asn1 time Signed-off-by: Nigel Brittain <[email protected]> fix test cases Signed-off-by: Nigel Brittain <[email protected]>

nbaws · 2024-11-28T11:22:36Z

@suniltheta if you have bandwidth

Signed-off-by: Nigel Brittain <[email protected]>

suniltheta

Couldn't go through all the files. Would it be possible to break the PR into multiple smaller chunks?

There are several functions, for example functions inside source/extensions/common/aws/signer_base_impl.cc or pemToAlgorithmSerialExpiration which are tricky to review since I don't have context on how it should be written. If you are able to break down the PR into individual pieces you can link in code comments from the docs what that piece of function is trying to solve based on the guidance given in https://docs.aws.amazon.com/rolesanywhere/latest/userguide/authentication-sign-process.html or other docs page.

I see we have two new high level CredentailsProvider class IAMRolesAnywhereX509CredentialsProvider & IAMRolesAnywhereCredentialsProvider. Would it be possible to add docs comments to explain what each of these classes are doing? If you happen to have a short doc, will help with the getting the context.

suniltheta · 2024-11-30T16:28:40Z

api/envoy/extensions/common/aws/v3/credential_provider.proto

+
+  string role_session_name = 7;
+
+  google.protobuf.Duration session_duration = 8 [(validate.rules).duration = {


does this take a default value if not specified?

If not specified there will be no duration sent, and the service will choose the duration. From https://docs.aws.amazon.com/rolesanywhere/latest/userguide/authentication-create-session.html:

durationSeconds
The duration, in seconds, of the role session. The value is optional and can range from 900 seconds (15 minutes) up to 43200 seconds (12 hours).

If durationSeconds is provided in the CreateSession (IAM Roles Anywhere credentialing API) request, the duration of the resulting session will be min(durationSeconds, profileDurationSeconds), where profileDurationSeconds is the durationSeconds value on the profile used in the request. Otherwise, if durationSeconds isn't provided in the request, profileDurationSeconds is used. Note that all of this just determines the value for durationSeconds that IAM Roles Anywhere sends in the AssumeRole request to STS (which the service will do internally). So if the maxSessionDuration on the role doesn't allow for the duration obtained through the described process, you'll receive a 403.

See: https://docs.aws.amazon.com/rolesanywhere/latest/userguide/authentication-create-session.html#credentials-object

so the proto constraint should enforce the sizing in config, however it's entirely optional for the user to provide here. we should have 403 covered in test cases, but i will double check what is logged so it is possible to troubleshoot it

source/extensions/common/aws/credentials_provider.h

source/extensions/common/aws/credentials_provider_impl.cc

nbaws · 2024-12-01T01:23:42Z

Couldn't go through all the files. Would it be possible to break the PR into multiple smaller chunks?

There are several functions, for example functions inside source/extensions/common/aws/signer_base_impl.cc or pemToAlgorithmSerialExpiration which are tricky to review since I don't have context on how it should be written. If you are able to break down the PR into individual pieces you can link in code comments from the docs what that piece of function is trying to solve based on the guidance given in https://docs.aws.amazon.com/rolesanywhere/latest/userguide/authentication-sign-process.html or other docs page.

I see we have two new high level CredentailsProvider class IAMRolesAnywhereX509CredentialsProvider & IAMRolesAnywhereCredentialsProvider. Would it be possible to add docs comments to explain what each of these classes are doing? If you happen to have a short doc, will help with the getting the context.

TY for your look through. I will add additional comments, and also carve off this code into its own source file so it is easier to understand. I will also add design documentation as per your suggestion.

Signed-off-by: Nigel Brittain <[email protected]>

nbaws · 2024-12-04T02:29:51Z

@suniltheta I've refactored so this has less impact on existing source files. Fundamental code is the same and I've added a small design doc here: #37440
Inline documentation can also be found in source/extensions/common/aws/iam_roles_anywhere_credentials_provider_impl.h

Please let me know if you'd like any more detail here.

13ajay

I haven't gone through the entire PR yet, but I have gone through some of it. I'll add comments on the rest when I get around to it.

13ajay · 2024-12-09T01:28:42Z

api/envoy/extensions/common/aws/v3/credential_provider.proto

+
+  string role_session_name = 7;
+
+  google.protobuf.Duration session_duration = 8 [(validate.rules).duration = {


If durationSeconds is provided in the CreateSession (IAM Roles Anywhere credentialing API) request, the duration of the resulting session will be min(durationSeconds, profileDurationSeconds), where profileDurationSeconds is the durationSeconds value on the profile used in the request. Otherwise, if durationSeconds isn't provided in the request, profileDurationSeconds is used. Note that all of this just determines the value for durationSeconds that IAM Roles Anywhere sends in the AssumeRole request to STS (which the service will do internally). So if the maxSessionDuration on the role doesn't allow for the duration obtained through the described process, you'll receive a 403.

See: https://docs.aws.amazon.com/rolesanywhere/latest/userguide/authentication-create-session.html#credentials-object

docs/root/configuration/http/http_filters/_include/aws_credentials.rst

source/extensions/common/aws/credentials_provider.h

source/extensions/common/aws/utility.cc

source/extensions/common/aws/credentials_provider_impl.cc

source/extensions/common/aws/credentials_provider_impl.h

source/extensions/common/aws/iam_roles_anywhere_credentials_provider_impl.cc

Signed-off-by: Nigel Brittain <[email protected]>

nbaws · 2024-12-10T03:24:10Z

/retest

Signed-off-by: Nigel Brittain <[email protected]>

github-actions · 2025-01-10T20:01:35Z

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

github-actions · 2025-01-18T00:03:27Z

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

Commit Message: aws: Implementation of IAM Roles anywhere support in aws request signing Additional Description: Part 1 of patch to implement IAM Roles anywhere capability. Adds the x509 credential type and the x509 signing base library. Design brief: #37440 Previous PR: #37193 (https://docs.aws.amazon.com/rolesanywhere/latest/userguide/introduction.html) Risk Level: Negligible - code currently unlinked Testing: Unit Docs Changes: N/A Release Notes: N/A Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Nigel Brittain <[email protected]>

Commit Message: aws: Implementation of IAM Roles anywhere support in aws request signing Additional Description: Part 1 of patch to implement IAM Roles anywhere capability. Adds the x509 credential type and the x509 signing base library. Design brief: envoyproxy#37440 Previous PR: envoyproxy#37193 (https://docs.aws.amazon.com/rolesanywhere/latest/userguide/introduction.html) Risk Level: Negligible - code currently unlinked Testing: Unit Docs Changes: N/A Release Notes: N/A Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Nigel Brittain <[email protected]>

nbaws added 16 commits November 3, 2024 10:06

first commit

a3f6bf6

Signed-off-by: Nigel Brittain <[email protected]>

proto

ba08314

Signed-off-by: Nigel Brittain <[email protected]>

cred handling and cluster

94a7bc5

Signed-off-by: Nigel Brittain <[email protected]>

certificate credential provider

4681752

Signed-off-by: Nigel Brittain <[email protected]>

update cert

efcf1a3

Signed-off-by: Nigel Brittain <[email protected]>

cleanup

3f2c8ea

Signed-off-by: Nigel Brittain <[email protected]>

update cert reading

a19897a

Signed-off-by: Nigel Brittain <[email protected]>

almost completed

341e851

Signed-off-by: Nigel Brittain <[email protected]>

cert algorithm

87d5207

Signed-off-by: Nigel Brittain <[email protected]>

update proto

541fa18

Signed-off-by: Nigel Brittain <[email protected]>

update signer

29e715f

Signed-off-by: Nigel Brittain <[email protected]>

fixed

d682456

Signed-off-by: Nigel Brittain <[email protected]>

prep for json merge

9033826

Signed-off-by: Nigel Brittain <[email protected]>

Merge branch 'main' into roles_anywhere

3c18f70

Signed-off-by: Nigel Brittain <[email protected]>

add explicit credential provider flag

32eab8b

Signed-off-by: Nigel Brittain <[email protected]>

fixed lowercase

cad0399

Signed-off-by: Nigel Brittain <[email protected]>

nbaws requested a review from mattklein123 as a code owner November 17, 2024 23:22

nbaws marked this pull request as draft November 17, 2024 23:22

repokitteh-read-only bot added the api label Nov 17, 2024

repokitteh-read-only bot assigned mattklein123 Nov 17, 2024

nbaws force-pushed the roles_anywhere branch from 3884e8e to e752b1d Compare November 28, 2024 11:21

nbaws marked this pull request as ready for review November 28, 2024 11:22

nbaws and others added 3 commits November 28, 2024 22:23

Merge branch 'envoyproxy:main' into roles_anywhere

4f63c48

changelog and expiration test

69b7099

Signed-off-by: Nigel Brittain <[email protected]>

format

dedc901

Signed-off-by: Nigel Brittain <[email protected]>

suniltheta reviewed Nov 30, 2024

View reviewed changes

mattklein123 assigned suniltheta Dec 2, 2024

nbaws added 3 commits December 4, 2024 01:42

refactor

76b4c0d

Signed-off-by: Nigel Brittain <[email protected]>

cleanup

9608191

Signed-off-by: Nigel Brittain <[email protected]>

punctuation

ae8fe68

Signed-off-by: Nigel Brittain <[email protected]>

repokitteh-read-only bot removed the waiting label Dec 4, 2024

nbaws added 4 commits December 4, 2024 02:09

minor cleanup

eebeed9

Signed-off-by: Nigel Brittain <[email protected]>

doc comment

81ec91a

Signed-off-by: Nigel Brittain <[email protected]>

doc comment

bb821f5

Signed-off-by: Nigel Brittain <[email protected]>

doc comment

0c74569

Signed-off-by: Nigel Brittain <[email protected]>

mattklein123 added the waiting label Dec 6, 2024

13ajay reviewed Dec 9, 2024

View reviewed changes

address some feedback

3e271a7

Signed-off-by: Nigel Brittain <[email protected]>

repokitteh-read-only bot removed the waiting label Dec 10, 2024

nbaws and others added 5 commits December 10, 2024 14:33

Merge branch 'main' into roles_anywhere

e1098ea

Signed-off-by: Nigel Brittain <[email protected]>

merge

6732a31

Signed-off-by: Nigel Brittain <[email protected]>

update after merge

622a432

Signed-off-by: Nigel Brittain <[email protected]>

update testing

a7fbfe5

Signed-off-by: Nigel Brittain <[email protected]>

update test cases

83e2ed2

Signed-off-by: Nigel Brittain <[email protected]>

mattklein123 added the waiting label Dec 11, 2024

github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Jan 10, 2025

github-actions bot closed this Jan 18, 2025

nbaws mentioned this pull request Mar 8, 2025

aws: Implementation of IAM Roles anywhere support in aws request signing #38683

Closed

nbaws mentioned this pull request Mar 18, 2025

aws: iam roles anywhere patch part 1 #38786

Merged

nbaws mentioned this pull request Apr 5, 2025

aws: Implementation of IAM Roles anywhere support in aws request signing - part 2 #39025

Merged


		string role_session_name = 7;

		google.protobuf.Duration session_duration = 8 [(validate.rules).duration = {

Conversation

nbaws commented Nov 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

repokitteh-read-only bot commented Nov 17, 2024

Uh oh!

nbaws commented Nov 28, 2024

Uh oh!

suniltheta left a comment

Choose a reason for hiding this comment

Uh oh!

suniltheta Nov 30, 2024

Choose a reason for hiding this comment

Uh oh!

nbaws Dec 4, 2024

Choose a reason for hiding this comment

Uh oh!

13ajay Dec 9, 2024

Choose a reason for hiding this comment

Uh oh!

nbaws Dec 10, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nbaws commented Dec 1, 2024

Uh oh!

nbaws commented Dec 4, 2024

Uh oh!

13ajay left a comment

Choose a reason for hiding this comment

Uh oh!

13ajay Dec 9, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nbaws commented Dec 10, 2024

Uh oh!

github-actions bot commented Jan 10, 2025

Uh oh!

github-actions bot commented Jan 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nbaws commented Nov 17, 2024 •

edited

Loading