[OSSA 2013-007] v1 api returns location as header for cached images

Or to additionally handle the case where the response header's x-image-meta-location filed
has been populated:

$ git diff glance/api/v1/images.py
 diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py
 index 7068820..3e8741c 100644
 --- a/glance/api/v1/images.py
 +++ b/glance/api/v1/images.py
 @@ -947,6 +947,10 @@ class ImageSerializer(wsgi.JSONResponseSerializer):
          self.notifier = notifier.Notifier()

      def _inject_location_header(self, response, image_meta):
 + if 'location' in image_meta:
 + del image_meta['location']
 + if 'x-image-meta-location' in response.headers:
 + del response.headers['x-image-meta-location']
          location = self._get_image_location(image_meta)
          response.headers['Location'] = location.encode('utf-8')

Let me know if there's something I need to do here (eg push something up to review.openstack.org).

Nope, at this point we need glance-core to approve your patch on this private bug.
Does it affect other versions ? Folsom/Essex ?

This looks a bit simpler to fix, but would my attached patch fix what you're seeing?

Hmm, for security patches, simpler is better. Other glance-core opinions on patch ?

Mark, I tested and your patch does solve the specific case raised in this bug.

I do see value in taking out the location/x-image-meta-location' in the serialize code (my patch)
since all requests pass through there, but maybe we can revisit that at another stage.

I've looked at stable/folsom, stable/essex and from code analysis I think they are affected. stable/diablo is before the cache rework and as far as I can tell should be ok. I'm interested to know if others concur...

Stuart:

I do believe my patch covers all the cases we need. However, I agree it makes sense to put in a stronger protection to ensure we are not returning 'x-image-meta-location'. It seems like maybe the right place to do this is in glance/common/utils.py image_meta_to_http_headers. But making that change involves a lot of test changes as well.

Maybe we can use the simpler fix for stable branches, and a change to glance/common/utils.py for grizzly+

Thoughts?

> Maybe we can use the simpler fix for stable branches, and a change to glance/common/utils.py for grizzly+

Yes, sounds good.

That works for me. Please propose patches for each affected series.

Not sure if I count anymore, but +2 to Stuart's 3 latest patches

On Thu, Mar 7, 2013 at 9:33 AM, Stuart McLaren
<email address hidden> wrote:
> ** Patch added: "stable/folsom patch -- not unit/functional tested"
> https://bugs.launchpad.net/glance/+bug/1135541/+attachment/3562409/+files/stable-folsom.patch
>
> --
> You received this bug notification because you are a member of Glance
> Core, which is subscribed to the bug report.
> https://bugs.launchpad.net/bugs/1135541
>
> Title:
> v1 api returns location as header for cached images
>
> Status in OpenStack Image Registry and Delivery Service (Glance):
> Triaged
>
> Bug description:
>
> When an image which is not in cache is downloaded:
>
> rm /opt/stack/data/glance/cache/2e877e45-5c13-42e8-995a-ffb28f888df4 ;
> glance --os-image-api-version 1 --os-username glance --os-password
> XXX --os-tenant-name service --os-auth-url http://localhost:5000/v2.0
> image-download 2e877e45-5c13-42e8-995a-ffb28f888df4
>
> The headers don't contain the backend 'location' field:
>
> T 10.6.51.191:9292 -> 10.6.51.191:52150 [AP]
> HTTP/1.1 200 OK.
> Content-Type: application/octet-stream.
> X-Image-Meta-Id: 2e877e45-5c13-42e8-995a-ffb28f888df4.
> X-Image-Meta-Deleted: False.
> X-Image-Meta-Container_format: bare.
> X-Image-Meta-Checksum: ccad32d005750c1005a80079b6fe8a9f.
> X-Image-Meta-Protected: False.
> X-Image-Meta-Min_disk: 0.
> X-Image-Meta-Created_at: 2013-02-27T10:37:24.
> X-Image-Meta-Size: 332.
> X-Image-Meta-Status: active.
> X-Image-Meta-Is_public: False.
> X-Image-Meta-Min_ram: 0.
> X-Image-Meta-Owner: 1bc9a6b4c0064336bc877f813244ffa1.
> X-Image-Meta-Updated_at: 2013-02-27T10:37:25.
> X-Image-Meta-Disk_format: qcow2.
> X-Image-Meta-Name: Image1.
> Location: http://10.6.51.191:9292/v1/images/2e877e45-5c13-42e8-995a-ffb28f888df4.
> Etag: ccad32d005750c1005a80079b6fe8a9f.
> X-Openstack-Request-Id: req-60ab20e6-4c3b-43a6-8203-c20d113fd7a4.
> Date: Thu, 28 Feb 2013 10:23:06 GMT.
> Transfer-Encoding: chunked.
> .
> 14c.
>
>
> But if the image is cached:
>
> glance --os-image-api-version 1 --os-username glance --os-password
> XXX --os-tenant-name service --os-auth-url http://localhost:5000/v2.0
> image-download 2e877e45-5c13-42e8-995a-ffb28f888df4
>
> the backend location field (with credentials) is returned as a header:
>
> T 10.6.51.191:9292 -> 10.6.51.191:51954 [AP]
> HTTP/1.1 200 OK.
> Content-Length: 332.
> Content-Type: application/octet-stream.
> X-Image-Meta-Id: 2e877e45-5c13-42e8-995a-ffb28f888df4.
> X-Image-Meta-Deleted: False.
> X-Image-Meta-Container_format: bare.
> X-Image-Meta-Location: swift+http://service%3Aglance:<redacted>@10.6.51.191:5000/v2.0/glance/2e877e45-5c13-42e8-995a-f fb28f888df4.
> X-Image-Meta-Protected: False.
> X-Image-Meta-Min_disk: 0.
> X-Image-Meta-Created_at: 2013-02-27T10:37:24.
> X-Image-Meta-Size: 332.
> X-Image-Meta-Status: active.
> X-Image-Meta-Is_public: False.
> X-Image-Meta-Min_ram: 0.
> X-Image-Meta-Owner: 1bc9a6b4c0064336bc877f813244ffa1.
> X-Image-Meta-Updated_at: 2013-02-27T10:37:25.
> X-Imag...

@Mark: you always count. Could you get bcwaldon (or another glance-core) to +2 them as well ?

Proposed impact description. Please doublecheck in particular that only v1 is affected and only the Swift backend is affected. Is there a way to disable v1 completely to work around the issue ?

---------------------
Title: Backend credentials leak in Glance v1 API
Reporter: Stuart McLaren (HP)
Products: Glance
Affects: All versions

Description:
Stuart McLaren from HP reported a vulnerability in the information potentially returned to the user in Glance v1 API. If an authenticated user requests, through the v1 API, an image that is already cached, the headers returned may disclose the Glance operator's Swift credentials for that endpoint. Only setups accepting the Glance v1 API and using the single-tenant Swift store are affected.
----------------------

Thierry,

Thanks for drafting. One nit -- any backend that includes credentials in the location field, eg swift/s3 are affected, backend stores which don't include credentials in the location field, eg 'file' are not.

and ... yes, you're correct, its just v1.

New rev:

---------------------
Title: Backend credentials leak in Glance v1 API
Reporter: Stuart McLaren (HP)
Products: Glance
Affects: All versions

Description:
Stuart McLaren from HP reported a vulnerability in the information potentially returned to the user in Glance v1 API. If an authenticated user requests, through the v1 API, an image that is already cached, the headers returned may disclose the Glance operator's backend credentials for that endpoint. Only setups accepting the Glance v1 API and using either the single-tenant Swift store or S3 store are affected.
----------------------

Thierry,

Thanks. That looks good to me.

+1 to the description, I'll make sure Waldon reviews the patches today

On Fri, Mar 8, 2013 at 9:11 AM, Stuart McLaren
<email address hidden> wrote:
> Thierry,
>
> Thanks. That looks good to me.
>
> --
> You received this bug notification because you are a member of Glance
> Core, which is subscribed to the bug report.
> https://bugs.launchpad.net/bugs/1135541
>
> Title:
> v1 api returns location as header for cached images
>
> Status in OpenStack Image Registry and Delivery Service (Glance):
> Triaged
>
> Bug description:
>
> When an image which is not in cache is downloaded:
>
> rm /opt/stack/data/glance/cache/2e877e45-5c13-42e8-995a-ffb28f888df4 ;
> glance --os-image-api-version 1 --os-username glance --os-password
> XXX --os-tenant-name service --os-auth-url http://localhost:5000/v2.0
> image-download 2e877e45-5c13-42e8-995a-ffb28f888df4
>
> The headers don't contain the backend 'location' field:
>
> T 10.6.51.191:9292 -> 10.6.51.191:52150 [AP]
> HTTP/1.1 200 OK.
> Content-Type: application/octet-stream.
> X-Image-Meta-Id: 2e877e45-5c13-42e8-995a-ffb28f888df4.
> X-Image-Meta-Deleted: False.
> X-Image-Meta-Container_format: bare.
> X-Image-Meta-Checksum: ccad32d005750c1005a80079b6fe8a9f.
> X-Image-Meta-Protected: False.
> X-Image-Meta-Min_disk: 0.
> X-Image-Meta-Created_at: 2013-02-27T10:37:24.
> X-Image-Meta-Size: 332.
> X-Image-Meta-Status: active.
> X-Image-Meta-Is_public: False.
> X-Image-Meta-Min_ram: 0.
> X-Image-Meta-Owner: 1bc9a6b4c0064336bc877f813244ffa1.
> X-Image-Meta-Updated_at: 2013-02-27T10:37:25.
> X-Image-Meta-Disk_format: qcow2.
> X-Image-Meta-Name: Image1.
> Location: http://10.6.51.191:9292/v1/images/2e877e45-5c13-42e8-995a-ffb28f888df4.
> Etag: ccad32d005750c1005a80079b6fe8a9f.
> X-Openstack-Request-Id: req-60ab20e6-4c3b-43a6-8203-c20d113fd7a4.
> Date: Thu, 28 Feb 2013 10:23:06 GMT.
> Transfer-Encoding: chunked.
> .
> 14c.
>
>
> But if the image is cached:
>
> glance --os-image-api-version 1 --os-username glance --os-password
> XXX --os-tenant-name service --os-auth-url http://localhost:5000/v2.0
> image-download 2e877e45-5c13-42e8-995a-ffb28f888df4
>
> the backend location field (with credentials) is returned as a header:
>
> T 10.6.51.191:9292 -> 10.6.51.191:51954 [AP]
> HTTP/1.1 200 OK.
> Content-Length: 332.
> Content-Type: application/octet-stream.
> X-Image-Meta-Id: 2e877e45-5c13-42e8-995a-ffb28f888df4.
> X-Image-Meta-Deleted: False.
> X-Image-Meta-Container_format: bare.
> X-Image-Meta-Location: swift+http://service%3Aglance:<redacted>@10.6.51.191:5000/v2.0/glance/2e877e45-5c13-42e8-995a-f fb28f888df4.
> X-Image-Meta-Protected: False.
> X-Image-Meta-Min_disk: 0.
> X-Image-Meta-Created_at: 2013-02-27T10:37:24.
> X-Image-Meta-Size: 332.
> X-Image-Meta-Status: active.
> X-Image-Meta-Is_public: False.
> X-Image-Meta-Min_ram: 0.
> X-Image-Meta-Owner: 1bc9a6b4c0064336bc877f813244ffa1.
> X-Image-Meta-Updated_at: 2013-02-27T10:37:25.
> X-Image-Meta-Checksum: ccad32d005750c1005a80079b6fe8a9f.
> X-Image-Meta-Disk_format: qcow2.
> X-Image-Meta-Name: Imag...

+1 from me

Sent to downstream stakeholders

Proposed public disclosure date/time:
Thursday, March 14, 1500UTC

Adding Stable and Essex maintainers, in preparation for CVE release.

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

Fix proposed to branch: stable/folsom
Review: https://review.openstack.org/24438

Fix proposed to branch: stable/essex
Review: https://review.openstack.org/24439

OSSA-2013-007

Reviewed: https://review.openstack.org/24437
Committed: http://github.com/openstack/glance/commit/e75764eee34915f8bc5b664ac18e47a556c9d3dd
Submitter: Jenkins
Branch: master

commit e75764eee34915f8bc5b664ac18e47a556c9d3dd
Author: Stuart McLaren <email address hidden>
Date: Thu Mar 14 14:27:35 2013 +0000

    Do not return location in headers

    In some cases credentials were being leaked when downloading a cached
    v1 image.

    Fixes bug 1135541, CVE-2013-1840

    Change-Id: Ie4077470d31249c5bd654985fcef577f4f5cfc59

Reviewed: https://review.openstack.org/24438
Committed: http://github.com/openstack/glance/commit/dd849a9be540bedd4fd904cc0b86ccd9c3e34af2
Submitter: Jenkins
Branch: stable/folsom

commit dd849a9be540bedd4fd904cc0b86ccd9c3e34af2
Author: Stuart McLaren <email address hidden>
Date: Thu Mar 14 13:43:36 2013 +0000

    Do not return location in headers

    In some cases credentials were being leaked when downloading a cached
    v1 image.

    Fixes bug 1135541, CVE-2013-1840

    Change-Id: I3ec0a8f484fe1bdc32c3c56fce810fcef347a7f6

Reviewed: https://review.openstack.org/24439
Committed: http://github.com/openstack/glance/commit/74b067df9726f9cf3e6e17e248719794a6ee0745
Submitter: Jenkins
Branch: stable/essex

commit 74b067df9726f9cf3e6e17e248719794a6ee0745
Author: Stuart McLaren <email address hidden>
Date: Thu Mar 14 14:22:00 2013 +0000

    Do not return location in headers

    In some cases credentials were being leaked when downloading a cached
    v1 image.

    Fixes bug 1135541, CVE-2013-1840

    Change-Id: Ib16ad40d3ea1c642384053be54ce029c386d7ea6