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.

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

Add EC2 IMDS v2 support #4857

Merged
merged 3 commits into from
Feb 14, 2020
Merged

Add EC2 IMDS v2 support #4857

merged 3 commits into from
Feb 14, 2020

Conversation

prognant
Copy link
Contributor

@prognant prognant commented Feb 7, 2020 •
edited
Loading

What does this PR do?

Add instance metadata service v2 (IMDS v2) support.
AWS official documentation : https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/configuring-instance-metadata-service.html
The agent will still rely on IMDS v1 first. It will do IMDS v2 request only if unauthenticated (IMDS v1) requests fail, thus the actual agent behaviour will only change if IMDS v1 is explicitly disabled, ensuring compatibility with existing deployments.

Motivation

Properly support EC2 instances with IMDSv1 disabled.

Additional Notes

Tests captured on an EC2 live instance with an agent development build :
Request without token on an IMDSv2 only instance :

GET /latest/meta-data/instance-id HTTP/1.1
Host: 169.254.169.254
User-Agent: Go-http-client/1.1
Accept-Encoding: gzip

HTTP/1.0 401 Unauthorized
Content-Length: 343
Content-Type: text/html
Date: Fri, 07 Feb 2020 13:31:39 GMT
Connection: close
Server: EC2ws

<?xml version="1.0" encoding="iso-8859-1"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
	"http:https://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http:https://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
 <head>
  <title>401 - Unauthorized</title>
 </head>
 <body>
  <h1>401 - Unauthorized</h1>
 </body>
</html>

Token request (PUT):

PUT /latest/api/token HTTP/1.1
Host: 169.254.169.254
User-Agent: Go-http-client/1.1
Content-Length: 0
X-Aws-Ec2-Metadata-Token-Ttl-Seconds: 60
Accept-Encoding: gzip

HTTP/1.0 200 OK
Content-Length: 56
Content-Type: text/plain
Date: Fri, 07 Feb 2020 13:31:39 GMT
X-Aws-Ec2-Metadata-Token-Ttl-Seconds: 60
Connection: close
Server: EC2ws

AQAAAFKw7LwD9Nn5LmMKwWMp9-xj2CI5EuASEmX6F3_g9F3iWvlFTQ==

Request to metadata with token:

GET /latest/meta-data/instance-id HTTP/1.1
Host: 169.254.169.254
User-Agent: Go-http-client/1.1
X-Aws-Ec2-Metadata-Token: AQAAAFKw7LwD9Nn5LmMKwWMp9-xj2CI5EuASEmX6F3_g9F3iWvlFTQ==
Accept-Encoding: gzip

HTTP/1.0 200 OK
Accept-Ranges: bytes
Content-Length: 19
Content-Type: text/plain
Date: Fri, 07 Feb 2020 13:31:39 GMT
Last-Modified: Fri, 07 Feb 2020 13:30:01 GMT
X-Aws-Ec2-Metadata-Token-Ttl-Seconds: 60
Connection: close
Server: EC2ws

i-0c583456e1956d03e

@prognant prognant added the [deprecated] team/agent-core Deprecated. Use metrics-logs / shared-components labels instead.. label Feb 7, 2020
@prognant prognant added this to the 7.18.0 milestone Feb 7, 2020
@prognant prognant requested a review from a team as a code owner February 7, 2020 16:02
@prognant
Add EC2 IMDS v2 support
798e0f5 
The agent will still rely on IMDS v1 first. It will do IMDS v2
request only if unauthenticated (IMDS v1) requests fail.
ogaca-dd
ogaca-dd reviewed Feb 13, 2020
View reviewed changes
Copy link
Contributor

@ogaca-dd ogaca-dd left a comment

Choose a reason for hiding this comment

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

LGTM just minor comments.

pkg/util/ec2/ec2.go Outdated Show resolved Hide resolved
pkg/util/ec2/ec2.go Outdated Show resolved Hide resolved
pkg/util/ec2/ec2.go Outdated Show resolved Hide resolved
pkg/util/ec2/ec2.go Outdated Show resolved Hide resolved
pkg/util/ec2/ec2.go Outdated Show resolved Hide resolved
ogaca-dd
ogaca-dd approved these changes Feb 14, 2020
View reviewed changes
Copy link
Contributor

@ogaca-dd ogaca-dd left a comment

Choose a reason for hiding this comment

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

👍

@prognant prognant merged commit 4ebaf03 into master Feb 14, 2020
@prognant prognant deleted the prognant/ec-imds-v2-support branch February 14, 2020 15:12
@ishug86
Copy link

ishug86 commented Feb 14, 2020 •
edited
Loading

@prognant @ogaca-dd I was just curious about this change. Looks like you first try to fetch the info from v1 and then defaults to v2.
Should it not be reversed?

@prognant
Copy link
Contributor Author

prognant commented Feb 17, 2020 •
edited
Loading

@ishug86 It could be reversed, but the idea was to keep the same behaviour than before the change (i.e. doing IMDSv1) where possible and support deployment with IMDSv2 only/IMDSv1 disabled. Not changing the default behaviour let some margin to ensure the IMDSv2 implementation is trouble-free.

If we think forward we could plan to do IMDSv2 only because :

  • IMDSv2 is enabled everywhere (but optional, i.e. IMDSv1 still works) by default
  • IMDSv2 can be mandatory by AWS configuration
  • IMDSv1 alone is not possible

So strictly speaking IMDSv1 support is not required anymore and is likely to be dropped at some point .

@ishug86
Copy link

ishug86 commented Mar 5, 2020

Thanks @prognant for the explanation, and sorry for responding so late.
As far as I know, IMDS V2 will always be enabled on all instances as its on AWS backend API (on the hypervisor).
However, With this implementation, the problem could be when someone disable v1 on their machine, it will lead to some call failures for v1 and also not recommended from security perspective as it may confuse folks if they want to disable IMDS V1 on the instance running datadog agent.

@prognant
Copy link
Contributor Author

Thanks for your feedback @ishug86. We understand that IMDSv2 will always be there, we agree that it makes sense to favor IMDSv2 regarding security. Thus we will work on allowing the agent to query IMDSv2 by default in a future release

@ishug86
Copy link

ishug86 commented Mar 12, 2020

Thanks @prognant , I will look forward and track the new release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogaca-dd ogaca-dd ogaca-dd approved these changes

Assignees
No one assigned
Labels
[deprecated] team/agent-core Deprecated. Use metrics-logs / shared-components labels instead..
Projects
None yet
Milestone
7.18.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@prognant @ishug86 @ogaca-dd