Skip to content

CredentialsEndpointProvider.java: Adds the sending of a User-Agent other than default User-Agent to the metadata service #471

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 7, 2018
Merged

CredentialsEndpointProvider.java: Adds the sending of a User-Agent other than default User-Agent to the metadata service #471

merged 1 commit into from
May 7, 2018

Conversation

willbengtson
Copy link

@willbengtson willbengtson commented Apr 24, 2018 •
edited
Loading

Adds the sending of a User-Agent other than default User-Agent in Java when making requests to metadata service

Similar to AWS Golang SDK and Ruby SDK, send a User-Agent other than the default User-Agent when making requests to the EC2 metadata for credentials.
aws/aws-sdk-java#1562 for aws-sdk-java addresses this also. PR #1445 for botocore implements this too.

This will enable protection of AWS credentials from Server Side Request Forgery (SSRF) vectors by being able to use a metadata proxy to block requests that do not have the right User-Agent set. User-Agent is not controllable by an attacker via SSRF.

Description

Set default headers in the CredentialsEndpointProvider interface

Motivation and Context

This will enable protection of AWS credentials from Server Side Request Forgery (SSRF) vectors by being able to use a metadata proxy to block requests that do not have the right User-Agent set. User-Agent is not controllable by an attacker via SSRF.

Testing

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed
  • A short description of the change has been added to the CHANGELOG

License

  • I confirm that this pull request can be released under the Apache 2 license

zoewangg
zoewangg requested changes May 4, 2018
View reviewed changes
Copy link
Contributor

@zoewangg zoewangg left a comment

Choose a reason for hiding this comment

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

Looks like HttpCredentialsUtilsTest failed, could you fix that?

core/src/main/java/software/amazon/awssdk/core/internal/CredentialsEndpointProvider.java Outdated
@@ -56,7 +57,12 @@ default CredentialsEndpointRetryPolicy retryPolicy() {
* Allows passing additional headers to the request
*/
default Map<String, String> headers() {
return Collections.emptyMap();
Map<String, String> requestHeaders = new HashMap<String, String>();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we use diamond operator here?

Copy link
Author

Choose a reason for hiding this comment

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

sure thing

@willbengtson
Copy link
Author

willbengtson commented May 4, 2018 •
edited
Loading

@zoewangg the tests fail because of NullPointerException. I'm trying to use import software.amazon.awssdk.core.util.VersionInfo; but for some reason it's shows Null when running the tests. I'm not super familiar with what to modify to get the test to pick the file up correctly. Am I seeing that correctly?

@zoewangg
Copy link
Contributor

zoewangg commented May 4, 2018

VersionInfo is generated from the build process. I think the tests failed because we are mocking the connectToEndpoint method with empty headers here.

Mockito.when(mockConnection.connectToEndpoint(endpoint, emptyHeaders)).thenThrow(new IOException());

https://github.com/aws/aws-sdk-java-v2/blob/master/core/src/test/java/software/amazon/awssdk/core/internal/HttpCredentialsUtilsTest.java#L156

So we just need to update the empty headers with the default headers.

@zoewangg
Copy link
Contributor

zoewangg commented May 4, 2018

Once the build succeeds, could you add a new-change entry? you can do that by running the scripts/new-change script and following the instructions.

Can you confirm that this PR is submitted under Apache 2.0 license by checking the I confirm that this pull request can be released under the Apache 2 license box under license section in the description?

@willbengtson
Copy link
Author

Will do, working on these changes

@willbengtson
Copy link
Author

@zoewangg updated

@zoewangg
Copy link
Contributor

zoewangg commented May 7, 2018 •
edited
Loading

@willbengtson Thanks for making the changes! LGTM.

Looks like the branch is out-of-date with master. Could you do a merge from master and then squash all commits to a single commit? I'll merge it afterwards.

CredentialsEndpointProvider.java: Adds the sending of a User-Agent ot…
313f7c9 
…her than default User-Agent in Java when making requests to metadata service

Similar to AWS Golang SDK and Ruby SDK, send a User-Agent other than the default User-Agent when making requests to the EC2 metadata for credentials.
aws/aws-sdk-java#1562 for aws-sdk-java addresses this also.  PR aws#1445 for botocore implements this too.

This will enable protection of AWS credentials from Server Side Request Forgery (SSRF) vectors by being able to use a metadata proxy to block requests
that do not have the right User-Agent set.  User-Agent is not controllable by an attacker via SSRF.
@willbengtson willbengtson force-pushed the add-useragent-to-metadata-calls branch from f89c21f to 313f7c9 Compare May 7, 2018 17:27
@willbengtson
Copy link
Author

@zoewangg Done

@zoewangg zoewangg merged commit dd0e577 into aws:master May 7, 2018
aws-sdk-java-automation added a commit that referenced this pull request Apr 4, 2019
@aws-sdk-java-automation
Merge pull request #471 from aws/staging/a671180a-b536-44af-99a2-6ce5…
d808d7d 
…7083fb6a

Pull request: release <- staging/a671180a-b536-44af-99a2-6ce57083fb6a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zoewangg zoewangg zoewangg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@willbengtson @zoewangg