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

Ensure that metadata API is actually listening at given endpoint #543

Closed
radeksimko opened this issue Feb 7, 2016 · 15 comments
Closed

Ensure that metadata API is actually listening at given endpoint #543

radeksimko opened this issue Feb 7, 2016 · 15 comments
Assignees
Labels
feature-request A feature should be added or improved. service-api This issue is due to a problem in a service API, not the SDK implementation.

Comments

@radeksimko
Copy link
Contributor

We've been using custom EC2-env detection in Terraform for a while, I think mostly because of the inconveniently long timeout in non-EC2 environments. It is now possible to set the timeout though, so I'm trying to clean up the auth logic and leverage most of the SDK logic where possible.

It seems SDK doesn't really check what is listening at http://169.254.169.254:80 (or any other given endpoint) and just blindly believes that it's always metadata API (or metadata API-compatible endpoint). We had a simple check in place which I intend to remove as I'm making SDK responsible for these things.

func responseContainsEC2Header(resp *http.Response) bool {
    return resp.Header["Server"] != nil && strings.Contains(resp.Header["Server"][0], "EC2")
}

Would there be interest in adding such checks into SDK?

btw. It's not intended to be security check as it's trivial to bypass such check (and we want to keep it that way to allow mocking the webserver for tests), it is rather to avoid strange issues where something completely irrelevant happens to be listening on that IP/port.

@jasdel
Copy link
Contributor

jasdel commented Feb 9, 2016

Hi @radeksimko thanks for contacting us. I'm taking a look at this feature. I'm curious to know more about the background on this change. Where there situations where apps were mistakenly setting themselves up on that host:port? It doesn't look like any of the other AWS SDKs implement this feature.

I ask because adding this might prevent users from running local proxy's caches in front of ec2metadata service.

@jasdel jasdel added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Feb 9, 2016
@radeksimko
Copy link
Contributor Author

I think this was more of a check for non-EC2 environments. We can (almost 100%?) safely assume that this IP address will always point to the metadata API on running EC2 instances unless I (as the VPC/subnet architect) decide to choose this CIDR or happen to peer with VPC which uses that CIDR or use Direct Connect to connect to a network which happens to use this CIDR. Maybe even then I'd expect the AWS API to warn me and/or still try to route the traffic to the metadata API.

However there's no way we can make any assumptions about networks of users which are connecting to AWS using other methods (env variables, shared creds file, hard-coded creds). For any reason those auth methods may fail and it will eventually fall through to EC2Role provider. 169.254.0.0/16 is APIPA range which means that pretty much any machine may eventually assign such address itself according to RFC3927 if it lost connection to the gateway.

In our context (Terraform), many ops/devs runs the tool directly from their laptops and they change the network very often, which also increases the chance for being part of 169.254.0.0/16 CIDR.

I ask because adding this might prevent users from running local proxy's caches in front of ec2metadata service.

I may be naive, but shouldn't such proxy be proxying HTTP headers too?

I may be also misinterpreting @catsby's intentions (it was originally his idea) - want to add any comments or further explanations here, Clint?

@catsby
Copy link

catsby commented Feb 10, 2016

You seem spot on, @radeksimko . The background issue/pr for Terraform is here:

Basically, if we're running on AWS, we want to try and use the EC2RoleProvider.
We're currently unaware of any reliable means to answer the question "are we running on AWS?", so we check this metadata url. The server header is more of a safety net in the event that something is listening on the same ip:port but not actually the metadata service. The odds of that are small, but small odds have bitten me in the past 😄

Our Nomad project does a similar check (minus the header check)

@radeksimko
Copy link
Contributor Author

@jasdel If we come up with a PR implementing this check, is there any chance of getting such PR merged or is your concern about proxy caches not forwarding headers too strong?

@jasdel jasdel removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Feb 24, 2016
@jasdel
Copy link
Contributor

jasdel commented Feb 24, 2016

Thanks a lot for the extra information @radeksimko and @catsby. I like the idea of being able to filter if the metadata connection is to the intended service, but I'm concerned that the Server: EC2ws header doesn't seem to be an officially supported or documented feature. Adding a dependency on the header into the SDK could create a reliance on a feature which is not official documented or supported. If this header were to be removed, or changed in the future it could break existing users of the SDK.

In the general case the SDK relying on this feature could introduce unexpected outcomes if the header was removed or changed in an unexpected way. Though this is a feature users could explicitly opt into by adding the check to the EC2 Metadata client's ValidateResponse request handler.

With that said the following code could be added to a code base to enable this validation.

sess := session.New()

metadataSvc := ec2metadata.New(sess)
metadataSvc.Handlers.ValidateResponse.PushBack(func(r *request.Request) {
    if r.HTTPResponse.Header.Get("Server") != "EC2ws" {
        r.Error = awserr.New("ValidationException",
            "EC2 metadata service does not appear to be a valid EC2 metadata service",
            nil)
    }
})

// Use metadataSvc client throughout your application

but shouldn't such proxy be proxying HTTP headers too?

You're right, proxies really aren't that significant of an issue. Though hypothetically if a user had custom caching service between ec2 metadata and the SDK that didn't forward the headers, and the SDK introduced this change it would be considered a breaking change.

@jasdel jasdel added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Feb 24, 2016
@radeksimko
Copy link
Contributor Author

Though this is a feature users could explicitly opt into by adding the check to the EC2 Metadata client's ValidateResponse request handler.

That looks a lot cleaner than the current solution we have in place. 👍

If Server: EC2ws is officially undocumented, is there any documented feature we could use to verify connection to AWS metadata API then?

@jasdel jasdel removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Feb 24, 2016
@jasdel
Copy link
Contributor

jasdel commented Feb 24, 2016

I'll reach out to the EC2 service team to see if there is any other prefered method for verifying the API endpoint.

In addition I suggest also posting to the EC2 Forums you might be able to get a quicker answer reaching out to them directly.

@jasdel jasdel added the service-api This issue is due to a problem in a service API, not the SDK implementation. label Feb 24, 2016
@radeksimko
Copy link
Contributor Author

In addition I suggest also posting to the EC2 Forums you might be able to get a quicker answer reaching out to them directly.

https://forums.aws.amazon.com/thread.jspa?threadID=226140

@jasdel
Copy link
Contributor

jasdel commented Feb 26, 2016

Hi @radeksimko I was reached out to EC2 and they pointed me to the Instance Identity Documents endpoint. This process does require an extra logic to first verify the EC2 Metadata host with the the identity docs.

Alternatively to be absolutely sure the EC2 metadata endpoint is a valid ec2 metadata host, you can verify the instance's signature as mentioned in the above doc. This is quite a bit more of an involved process though.

@radeksimko
Copy link
Contributor Author

@jasdel Understood, thanks for reaching out to EC2 team and pointing me to the right direction.

Would you prefer to have this logic in the SDK or do you expect the consumer (projects using the SDK) to implement this? I'm happy to send the PR in the first case.

@jasdel
Copy link
Contributor

jasdel commented Mar 1, 2016

@radeksimko we'd be more than glad to take a look at a PR adding a convenience method to verify the validity of the metadata endpoint. Were you thinking this functionality would be automatic or an opt in operation that users would explicitly call, returning if the metadata endpoint appears to be an EC2 Metadata service?

@radeksimko
Copy link
Contributor Author

Were you thinking this functionality would be automatic or an opt in operation that users would explicitly call, returning if the metadata endpoint appears to be an EC2 Metadata service?

That is a good question, I was originally thinking of "opt-out model", but opt-in where the consumer/user would just call one extra method (isEndpointValid() bool or something equally easy) is ok with me too.

It is still much cleaner than anything we have or discussed so far (manually calling IP address or metadataSvc.Handlers.ValidateResponse with custom header-checking logic).
I may get to this in a few days, so any suggestions regarding the interface are welcomed.

@radeksimko
Copy link
Contributor Author

See #590

The usage is not as simple as I originally thought it would be, but I didn't want to confuse people with naming conventions - i.e. making isEndpointValid() responsible for checking existence & parse-ability of the instance identity document would be IMO misleading.

@jasdel
Copy link
Contributor

jasdel commented Mar 15, 2016

Per our discussion on #590 the simplest way that is table to detect a host is the ec2 instance is to use the EC2Metdata#Available method. This method will detect if the ec2metadata endpoint is valid and the endpoint returns an instance id.

@jasdel
Copy link
Contributor

jasdel commented Mar 16, 2016

I've pulled in the PR for adding ident doc support. Since we have the available method and it looks to be good for this use case I'm going to go ahead and close this issue. Thanks for brining the issue up, and creating the associated PR.

@jasdel jasdel closed this as completed Mar 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. service-api This issue is due to a problem in a service API, not the SDK implementation.
Projects
None yet
Development

No branches or pull requests

3 participants