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

Update ec2_metadata to use IMDSV2 (Continued from #1457) #1520

Merged
merged 13 commits into from
Jan 25, 2021

Conversation

sawanoboly
Copy link
Contributor

@sawanoboly sawanoboly commented Sep 30, 2020

Reflected the comments on the pull request in #1457.

I have confirmed that this works with an EC2 instance with IMDSv2 only enabled.

Here's a quote from the original PR by @wilkosz

Agenda

Update all service calls to EC2 metadata to use IMDSV2 spec:
https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instancedata-data-retrieval.html

EC2 metadata service IMDSV1 introduced quite a few vulnerabilities: https://hackerone.com/reports/508459

Fix

Add v2 token generation for each NET::HTTP::Get request.
Closes issue #1411

Description

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have run the pre-merge tests locally and they pass.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commits have been signed-off for the Developer Certificate of Origin.

@sawanoboly sawanoboly requested review from a team as code owners September 30, 2020 05:16
@kchristensen
Copy link

I'd love to see this get merged, it's the one remaining thing we have making IMDSv1 requests still.

@TheKPLD
Copy link

TheKPLD commented Nov 20, 2020

I'd love to see this get merged, it's the one remaining thing we have making IMDSv1 requests still.

Yes please, +1

@tas50
Copy link
Contributor

tas50 commented Dec 9, 2020

@sawanoboly Any chance you can take a look at the test failures. It may also help to rebase this on master.

@sawanoboly
Copy link
Contributor Author

At the time of 15bfc27 the test was successful, taking over a PR that did not pass. I'll try to sort out the differences from the current master, as the changes are difficult to understand.

Signed-off-by: sawanoboly <sawanoboriyu@higanworks.com>
@sawanoboly
Copy link
Contributor Author

@tas50 Thanks for notify, All checks have passed now.

@jaymzh
Copy link
Collaborator

jaymzh commented Jan 25, 2021

@tas50 - this seems sane?

@tas50 tas50 merged commit c33e81b into chef:master Jan 25, 2021
@tas50
Copy link
Contributor

tas50 commented Jan 25, 2021

We'll need to validate this against and existing aws setup once it hits chef 17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants