Glance

Comment 111 for bug 1546507

Revision history for this message

Brian Rosmaita (brian-rosmaita) wrote on 2017-01-26:

#111

Quick summary of where we are now:

(A) The situation:

Exploit can occur under *both* of the following conditions:
(1) show_multiple_locations=true (default is *false*)
==> deprecated in Newton, supposed to be removed in Ocata
(2) set_image_location policy allows user to set location (default is *admin* since Newton)

Even if (1) is removed in Ocata, a version of this problem will continue, plus that wouldn't help the stable branches anyway.

My understanding is that ceph and cinder are the most likely backends to be configured in this non-default way.

(B) stores and vulnerability assessment

The short story:
http, filesystem not vulnerable
cinder: vulnerable, requires patch, will be broken by requiring image_id in url
rbd, sheepdog, vmware, swift: vulnerable, requiring image_id in url is OK and will fix

We need to determine whether cinder could also require an image_id in the url for new images without breaking (i.e., is it that cinder *cannot* or *does not* do this now). Also, cinder can be configured in a kind of single-tenant mode (like swift), I'm not sure that the proposed patch prevents the exploit when in single-tenant mode.

One other thing about the fix ... it has to take into account all supported stores. Bringing that up because I was thinking that because of the cinder situation, we could maybe let the store being used say what counts as an OK url. But we cannot do that because it's possible for glance to have multiple stores (it can only use the default store for uploads, though). There was a bug Stuart fixed about the "swift+config://" scheme that reminded me of this:
https://bugs.launchpad.net/glance/+bug/1334196
So we have to worry about a case where a deployer uses ceph and then decides to add cinder as the default store--we still need to check a rbd:// url if a user adds it as a location. (The current Fei Long/Mike patch correctly does this; my point is just that as we're converging on a fix, we need to keep this in mind.)

Quick summary of where we are now:

(A) The situation:

Exploit can occur under *both* of the following conditions:
(1) show_multiple_locations=true  (default is *false*)
    ==> deprecated in Newton, supposed to be removed in Ocata
(2) set_image_location policy allows user to set location (default is *admin* since Newton)

Even if (1) is removed in Ocata, a version of this problem will continue, plus that wouldn't help the stable branches anyway.

My understanding is that ceph and cinder are the most likely backends to be configured in this non-default way.

(B) stores and vulnerability assessment

We need to determine whether cinder could also require an image_id in the url for new images without breaking (i.e., is it that cinder *cannot* or *does not* do this now).  Also, cinder can be configured in a kind of single-tenant mode (like swift), I'm not sure that the proposed patch prevents the exploit when in single-tenant mode.

One other thing about the fix ... it has to take into account all supported stores.  Bringing that up because I was thinking that because of the cinder situation, we could maybe let the store being used say what counts as an OK url.  But we cannot do that because it's possible for glance to have multiple stores (it can only use the default store for uploads, though).  There was a bug Stuart fixed about the "swift+config://" scheme that reminded me of this:
https://bugs.launchpad.net/glance/+bug/1334196
So we have to worry about a case where a deployer uses ceph and then decides to add cinder as the default store--we still need to check a rbd:// url if a user adds it as a location.  (The current Fei Long/Mike patch correctly does this; my point is just that as we're converging on a fix, we need to keep this in mind.)