Skip to content

fix: Validate url domain for aws metadata urls#1079

Merged
sai-sunder-s merged 4 commits intogoogleapis:mainfrom
sai-sunder-s:awsurls
Nov 9, 2022
Merged

fix: Validate url domain for aws metadata urls#1079
sai-sunder-s merged 4 commits intogoogleapis:mainfrom
sai-sunder-s:awsurls

Conversation

@sai-sunder-s
Copy link
Copy Markdown
Contributor

Updating AWS credential source validation as per new updates in AIP. Make sure the host of url, region_url and imdsv2 session token url belong to AWS metadata server.

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> ☕️

If you write sample code, please follow the samples format.

@product-auto-label product-auto-label Bot added the size: m Pull request size is medium. label Nov 4, 2022
@sai-sunder-s sai-sunder-s marked this pull request as ready for review November 4, 2022 18:48
@sai-sunder-s sai-sunder-s requested review from a team and lsirac November 4, 2022 18:48
Copy link
Copy Markdown

@TimurSadykov TimurSadykov left a comment

Choose a reason for hiding this comment

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

LGTM with couple small comments

Comment thread oauth2_http/java/com/google/auth/oauth2/AwsCredentials.java
try {
URL url = new URL(urlString);
String host = url.getHost();
if (!host.equals("169.254.169.254") && !host.equals("[fd00:ec2::254]")) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Both constants are GCE-specific, better add as package constants (static final) to the ComputeEngineCredentials

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

They are AWS mds endpoints. 169.254.169.254 is same one used by GCE as well I guess. But I do not find any documentation about the ipv6 one for GCE

Copy link
Copy Markdown
Contributor

@lsirac lsirac left a comment

Choose a reason for hiding this comment

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

Thanks Sai

Comment thread oauth2_http/java/com/google/auth/oauth2/AwsCredentials.java Outdated
Comment thread oauth2_http/java/com/google/auth/oauth2/AwsCredentials.java Outdated
Comment thread oauth2_http/javatests/com/google/auth/oauth2/AwsCredentialsTest.java Outdated
Comment thread oauth2_http/javatests/com/google/auth/oauth2/AwsCredentialsTest.java Outdated
@sai-sunder-s sai-sunder-s merged commit 31fe461 into googleapis:main Nov 9, 2022
TimurSadykov pushed a commit that referenced this pull request Nov 11, 2022
* fix: Validate url domain for aws metadata urls

* fix external account tests

* static method and split tests

* rename param:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants