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

Bug #1284741 reported by Phil Day
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Won't Fix
Low
Unassigned
ec2-api
Confirmed
Low
Unassigned

Bug Description

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]

Tracy Jones (tjones-i)
tags: added: ec2
Sean Dague (sdague)
Changed in nova:
status: New → Confirmed
importance: Undecided → Low
tags: added: low
tags: added: low-hanging-fruit
removed: low
Changed in nova:
assignee: nobody → Chaitanya Challa (cvskchaitanya)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

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

Changed in nova:
status: Confirmed → In Progress
Changed in nova:
assignee: Chaitanya Challa (cvskchaitanya) → nobody
Changed in nova:
assignee: nobody → Chaitanya Challa (cchalla)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on nova (master)

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.

Changed in nova:
status: In Progress → New
assignee: Chaitanya Challa (cchalla) → nobody
Sean Dague (sdague)
Changed in nova:
status: New → Confirmed
Changed in nova:
assignee: nobody → Nobuteru Nishida (nobuteru-nishida)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

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

Changed in nova:
status: Confirmed → In Progress
Revision history for this message
Robert Collins (lifeless) wrote :

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.

Revision history for this message
Nobuteru Nishida (nobuteru-nishida) wrote :

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.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on nova (master)

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.

Changed in ec2-api:
importance: Undecided → Low
status: New → Confirmed
Changed in nova:
assignee: Nobuteru Nishida (nobuteru-nishida) → nobody
status: In Progress → Confirmed
Changed in nova:
status: Confirmed → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.