EC2 metadata service doesn't account for request forwarding when using neutron metadata-proxy

[Phil Day]

When an EC2 metadata request is received via the neutron metadata proxy Nova assumes that the X-Forwarded-For item in teh header is the address of the instance:

https://github.com/openstack/nova/blob/master/nova/api/metadata/handler.py#L149

In fact depending on the network path this could be a comma separated list of of addresses, only the first element of which is the address of the instance.

The correct handling should be something like:

remote_address = req.headers.get('X-Forwarded-For').split(',')[0]

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/133400

[OpenStack Infra (hudson-openstack)]

Change abandoned by Sean Dague (<email address hidden>) on branch: master
Review: https://review.openstack.org/133400
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/156460

[Robert Collins (lifeless)]

I've commented on the review for this; but proxies might turn up like so:

VM -> proxy A -> ns-agent -> metadata-agent -> proxy B -> Nova

Or without the ns-agent

VM -> proxy C -> metadata-agent -> proxy D -> Nova

We currently fail open when:
 - proxy A is used (the wrong instance data will be returned).

We fail closed when:
 - proxy B/C/D are used (too many elements, crash in the code)

Fixing the case for B and D, which I presume this is about will cause C to fail open and possibly allow obtaining metadata for arbitrary instances (including root password). So I believe we need to ensure that we don't open this up inappropriately, and fix the existing security hole [which is mitigated by the fact that you have to be running a proxy A to be attacked, whereas for C it potentially allowing any instance to be queried would be substantially worse.

[Nobuteru Nishida (nobuteru-nishida)]

Thank you for comment.

Does ns-agent means quantum-ns-metadata-proxy?

I took your review as follows.
Is there anything wrong ?

- arbitary instance -> proxy A (VM in a neutron-namespace)
REMOTE_ADDR: arbitary instance

- proxy A -> namespace-metadata-proxy
REMOTE_ADDR: proxy A(changed)
X-FORWARDED-FOR: REMOTE_ADDR

- namespace-metadata-proxy -> metadata-agent
X-FORWARDED-FOR : proxy A

- metadata-agent -> nova-metadata-api
X-FORWARDED-FOR: proxy A

And return proxy A's metadata to arbitrary instance inappropriately.
Additionally, My patch increases security concern in the case of proxy C used
and proxy C connects to metadata-agent directly without metadata-proxy
because metadata-agent trust X-FORWARDED-FOR and pass it on as-is to nova-metadata-api.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Joe Gordon (<email address hidden>) on branch: master
Review: https://review.openstack.org/156460
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.