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

fix: Validate url domain for aws metadata urls #1079

Merged
merged 4 commits into from Nov 9, 2022

Conversation

sai-sunder-s
Copy link
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 a review from a team as a code owner November 4, 2022 18:48
Copy link
Member

@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

try {
URL url = new URL(urlString);
String host = url.getHost();
if (!host.equals("169.254.169.254") && !host.equals("[fd00:ec2::254]")) {
Copy link
Member

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
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
Collaborator

@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

@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.

None yet

3 participants