[OSSA 2013-002] glance image-download can display backend Swift password

Awesome ! glance-core, please +1 patches, i'll draft an impact statement for your reviewing pleasure.

I did not verify this functionally, but by looking at the code this vulnerability appears to affect Diablo (2011.3.1) and Cactus as well (not Bexar). Here is a proposed impact statement:

By creating an image in Glance by URL that references a mis-configured Swift endpoint, or if the Swift endpoint that a previously-ACTIVE image references for any reason becomes unusable, any user may gain the Glance operator's Swift credentials for that endpoint. Only setups that use the single-tenant Swift store are affected.

Impact statement looks good, s/gain/access maybe
My understanding is that the user must be authenticated, adding that to the mix as well, and adding headers, we get:

Title: Backend password leak in Glance error message
Reporter: Dan Prince (Red Hat)
Products: Glance
Affects: All versions

Dan Prince of Red Hat discovered an issue in Glance error reporting. By creating an image in Glance by URL that references a mis-configured Swift endpoint, or if the Swift endpoint that a previously-ACTIVE image references for any reason becomes unusable, an authenticated user may access the Glance operator's Swift credentials for that endpoint. Only setups that use the single-tenant Swift store are affected.

+1 to your impact statement, Thierry

OK, will push to downstream stakeholders as soon as two glance-core +1 the (relatively obvious) patches by commenting so here. Adding glance-core.

+1 on the grizzly, folsom & essex patches attached above.

Thanks for pointing that out Dan. I was combing through the swift code in glance/store and found a couple of other places where we display the uri in exceptions and logs. It may be good to sanitize them all(esp the exception). Will list them below.

139 LOG.debug(_("Invalid store uri %(uri)s: %(reason)s") % locals())

183 except IndexError:
184 reason = _("Badly formed Swift URI: %s") % uri
185 LOG.debug(reason)

375 except swiftclient.ClientException, e:
376 if e.http_status == httplib.CONFLICT:
377 raise exception.Duplicate(_("Swift already has an image at "
378 "location %s") %
379 location.get_uri())

Iccha: I've attached a second round patch patches which (this time) remove the URI/creds from log messages as well as exception messages. All of the LOG messages affected are debug level (which means most people probably don't see them in production). The downside of this approach is that things will be a bit more difficult to debug but I'm Okay with that in the name of security.

Remaining tasks before disclosure:
Glance-core: please review and approve on this bug the new set of proposed patches

Looks good to me +2 for all patches

Thanks Dan for making the changes

Thanks Dan for going the extra mile on this.

+1 approved from me.

Looks great, thanks guys!

rebasing the grizzly patch

Proposed public disclosure date/time:
*Tuesday January 29th, 1500UTC*

Okay. So in reviewing things for tomorrow I just realized we should make the same changes for the S3 backend as well. Should we do that in this ticket and postpone for a day or two?

Actually: Looks like the swift backend does a better job via the _sanitize function so the risk targeted in this ticket are avoided. Whew!

I assume you meant that the S3 backend does a better job?

Here is a patch for diablo (2011.3 on Ubuntu 11.10) that is untested. It adjusts glance/common/exception.py to make BadStoreUri() only take one argument ('reason') which is closer to what is happening in Essex. It changes all 'exception.BadStoreUri(uri, reason)' to use 'raise exception.BadStoreUri(reason)' instead so it touches files outside of glance/store/swift.py (which is where I independently noticed that S3 may also be affected). This patch deals only with this bug and did not try to pull back the other changes to glance/common/exception.py from later releases.

@Jamie: I did not run diablo with your patch but it looks sane to me. Note that we don't push patches to stable/diablo branch as part of the security process anymore since nobody looks after that branch anymore. Unless someone picks it up, looks like it will be a pain until 11.10 EOL...

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

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

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

Reviewed: https://review.openstack.org/20696
Committed: http://github.com/openstack/glance/commit/96a470be64adcef97f235ca96ed3c59ed954a4c1
Submitter: Jenkins
Branch: stable/folsom

commit 96a470be64adcef97f235ca96ed3c59ed954a4c1
Author: Dan Prince <email address hidden>
Date: Sat Jan 12 15:38:09 2013 -0500

    Remove Swift location/password from messages.

    Updates several exceptions and log messages in the Swift backend
    so that they don't include Swift location URI's which may contain
    passwords when used in Swift single tenant mode.

    Fixes LP Bug #1098962 (for Folsom).

    Change-Id: Ia97a95ce6ed5d98a76515eea8817e309bcf0889a

Fix proposed to branch: stable/diablo
Review: https://review.openstack.org/20705

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

commit e96273112b5b5da58d970796b7cfce04c5030a89
Author: Dan Prince <email address hidden>
Date: Sat Jan 12 15:29:56 2013 -0500

    Remove Swift location/password from messages.

    Updates several exceptions and log messages in the Swift backend
    so that they don't include Swift location URI's which may contain
    passwords when used in Swift single tenant mode.

    Fixes LP Bug #1098962.

    Change-Id: I458de58563343347d5c976fe1ad084cc7e3280eb

Reviewed: https://review.openstack.org/20697
Committed: http://github.com/openstack/glance/commit/37d4d96bf88c2bf3e7e9511b5e321cf4bed364b7
Submitter: Jenkins
Branch: stable/essex

commit 37d4d96bf88c2bf3e7e9511b5e321cf4bed364b7
Author: Dan Prince <email address hidden>
Date: Sat Jan 12 15:43:22 2013 -0500

    Remove Swift location/password from messages.

    Updates several exceptions and log messages in the Swift backend
    so that they don't include Swift location URI's which may contain
    passwords when used in Swift single tenant mode.

    Fixes LP Bug #1098962 (for Essex).

    Change-Id: I71c0e953cff959890d759ae07574e8b436636279

OSSA 2013-002