[OSSA 2015-016] all PUT tempurls leak existence via DLO manifest attack (CVE-2015-5223)
| Affects | Status | Importance | Assigned to | Milestone | |
|---|---|---|---|---|---|
| | OpenStack Object Storage (swift) |
Critical
|
Unassigned | ||
| | OpenStack Security Advisory |
Medium
|
Tristan Cacqueray | ||
Bug Description
If you get a PUT tempurl you can use DLO's to find objects in the container, or in the account.
If you are allowed to upload a DLO via PUT tempurl and the application that generated the tempurl believes it safe to generate a GET tempurl for the data they just authorized you to upload - they may accidentally authorize you to download any previously discovered data.
We should not allow uses to PUT DLO's via tempurl - it's currently insecure because of the existence leak attack; and can be difficult to reason about safely for application authors generating tempurls.
CVE References
| clayg (clay-gerrard) wrote : | #1 |
| Changed in ossa: | |
| status: | New → Confirmed |
| importance: | Undecided → Medium |
Can someone triage the swift bug ?
This looks like a type B* bug ( https:/
Unless someone object, we'll subscribe ossg-coresec next Monday.
| John Dickinson (notmyname) wrote : | #4 |
Tristan, this bug and https:/
| Changed in swift: | |
| status: | New → Confirmed |
| John Dickinson (notmyname) wrote : | #5 |
I'm not sure if this is a class B1 or class A bug yet. I definitely think this needs to be fixed on master. If we do end up blocking DLOs with tempurls (or at least creating DLOs with tempurls), then we need to decide if it's better to leave old versions vulnerable or change their behavior with a security update. My default response is to fix the bug and backport it.
Alright, so if it's a class A, then we could update the impact description of bug 1449212 to cover both bug within the same advisory.
Else, then let's get OSSG on board for this one to work on a security note.
| clayg (clay-gerrard) wrote : | #7 |
sorry, been busy - attached patch for review
Here's some bash that works on a swift-all-in-one to get you started:
#!/bin/bash
# resetswift
set -e
echo "password" > secret
swift upload private secret
swift post -H 'x-account-
# create a container for people to upload stuff too
swift post public
# attacker: "yes hello, can I have a safe place to upload some of my data?"
PUT_
curl -XPUT -H 'x-object-manifest: private/secret' "http://
# attacker: "oh yes, that data I just uploaded - can I download it please?"
PUT_
# attacker: only... it's not *MY* data - trolrolrololollolo
echo ""
echo "******
curl "http://
echo ""
echo "******
| Robert Clark (robert-clark) wrote : | #8 |
Looks like class A to me, not overly complicated to exploit given the right situation...
| John Dickinson (notmyname) wrote : | #9 |
Clay, the patch looks fine except that I can't make the functests fail with patched or unpatched server code. I'm always getting a 201 for the object PUTs.
| clayg (clay-gerrard) wrote : | #10 |
ah I think I forgot some else: self.fail('request did not error') sorta lines in there - nice catch! #willfix
| Kota Tsuyuzaki (tsuyuzaki-kota) wrote : | #11 |
@Clay
Here for you adding the self.fail to functional if manifest file upload succeeded.
Code looks good but just a question from me. Is this bug already defined as class A? If so, no problem to merge attached patch I guess.
However, if classified as B or so, I am wondering if we could take another option (or not? I'm not sure) that we can add 'x-object-manifest' to incoming_
| clayg (clay-gerrard) wrote : | #12 |
@Kota I want the explicit error because if someone is trying to make a dlo via tempurl they are either
a) trying to validate this security hole and they will know they are patched/hozed by the 400
b) trying to use this vulnerability as a make shift temporary-
I retested Kota's patch and I think the only thing that is missing is the Co-Author line ;)
| Kota Tsuyuzaki (tsuyuzaki-kota) wrote : | #13 |
@Clay That totally makes me sense and I agreed we should show the error explicitly. And thanks for adding me to the co-author line :)
| John Dickinson (notmyname) wrote : | #14 |
@Clay, the patch in #12 looks good.
Consider this my +2, which gives us 2 (from Kota's in #13).
We'll need to get backports here, and it looks like we don't have a CVE description yet. I'm not sure what the next step is there.
| description: | updated |
| John Dickinson (notmyname) wrote : | #15 |
attached is the exact same patch (including commit message, date, and author) except formatted with `git format-patch` so that `git am <file.patch` will apply it cleanly.
I am working on backport patches for juno and kilo, but I still have a few unit test errors there.
| John Dickinson (notmyname) wrote : | #16 |
backport for juno. This is based off of the proposed juno backport in https:/
| John Dickinson (notmyname) wrote : | #17 |
backport for kilo. This is based off of the proposed juno backport in https:/
| Changed in swift: | |
| importance: | Undecided → Critical |
@notmyname, would it makes sense to solve both bug 1453948 and bug 1449212 with a single OSSA/CVE ? If so, would that updated impact description good enough to cover both cases ?
Title: Information leak via Swift tempurls
Reporter: Richard Hawkins (Rackspace)
Products: Swift
Affects: versions through 2.2.0, and 2.2.1 versions through 2.3.0
Description:
Richard Hawkins from Rackspace reported a vulnerability in Swift tempurls. When in possession of a tempurl key for a Swift container, a malicious actor may retrieve objects within any other containers for the same Swift account (tenant). All Swift setup are affected.
| Alistair Coles (alistair-coles) wrote : | #19 |
I'm +2 for the patch in #15
The tests do not cover POST requests. The fix does so there is no vulnerability with a POST but I think it would be worth adding a test to prevent a regression of the kind illustrated in the attached diff (diff wrt patch from #15). I can propose that to master after this fix is released.
| Alistair Coles (alistair-coles) wrote : | #20 |
FWIW Here's a suggested diff wrt patch at #15 for extra test coverage of POSTs. But I repeat I am +2 for merging patch at #15 as it is and following up with the extra tests.
| John Dickinson (notmyname) wrote : | #21 |
Updated report description:
Title: Information leak via Swift tempurls
Reporter: Richard Hawkins and Swift core reviewers
Products: Swift
Affects: versions through 2.3.0
Description:
Richard Hawkins and Swift core reviewers reported a vulnerability in Swift tempurls. When in possession of a tempurl key authorized for PUT, a malicious actor may retrieve other objects in the same Swift account (tenant). All Swift setup are affected.
Thanks John.
Adding affiliation back to description:
Title: Information leak via Swift tempurls
Reporter: Richard Hawkins (Rackspace) and Swift core reviewers
Products: Swift
Affects: versions through 2.3.0
Description:
Richard Hawkins from Rackspace and Swift core reviewers reported a vulnerability in Swift tempurls. When in possession of a tempurl key authorized for PUT, a malicious actor may retrieve other objects in the same Swift account (tenant). All Swift setup are affected.
| Changed in ossa: | |
| status: | Incomplete → Triaged |
| assignee: | nobody → Tristan Cacqueray (tristan-cacqueray) |
As for the required patchs to fix the bug describe above:
master/liberty: bug 1453948 comment #15, bug 1449212 comment #85
stable/kilo: bug 1453948 comment #17, bug 1449212 comment #90
stable/juno: bug 1453948 comment #16
Seems like all the patch have now been pre-approved. Should we requests a CVE with description in comment #22 and set this disclosure date:
2015-08-25, 1500UTC
| Jeremy Stanley (fungi) wrote : | #24 |
The impact description in comment #22 looks fine, though I'd say "all Swift setups" rather than "all Swift setup" (minor grammar nit).
I'm good with the proposed Tuesday, August 25 disclosure, since that gives ample time for us to notify downstream stakeholders. As a reminder, this is intended to cover bugs 1453948 and 1449212.
| John Dickinson (notmyname) wrote : | #25 |
looks good
| Changed in ossa: | |
| status: | Triaged → In Progress |
| summary: |
- all PUT tempurls leak existence via DLO manifest attack + all PUT tempurls leak existence via DLO manifest attack (CVE-2015-5223) |
| Tristan Cacqueray (tristan-cacqueray) wrote : Re: all PUT tempurls leak existence via DLO manifest attack (CVE-2015-5223) | #26 |
Disclosure date (extended by one day):
2015-08-26, 1500UTC
| Changed in ossa: | |
| status: | In Progress → Fix Committed |
| information type: | Private Security → Public Security |
Fix proposed to branch: stable/kilo
Review: https:/
Related fix proposed to branch: master
Review: https:/
| summary: |
- all PUT tempurls leak existence via DLO manifest attack (CVE-2015-5223) + [OSSA 2015-016] all PUT tempurls leak existence via DLO manifest attack + (CVE-2015-5223) |
Reviewed: https:/
Committed: https:/
Submitter: Jenkins
Branch: stable/juno
commit 0694e1911d10a18
Author: Clay Gerrard <email address hidden>
Date: Thu Jul 23 22:36:21 2015 -0700
Disallow unsafe tempurl operations to point to unauthorized data
Do not allow PUT tempurls to create pointers to other data. Specifically
disallow the creation of DLO object manifests by returning an error if a
non-safe tempurl request includes an X-Object-Manifest header regardless of
the value of the header.
This prevents discoverability attacks which can use any PUT tempurl to probe
for private data by creating a DLO object manifest and then using the PUT
tempurl to head the object which would 404 if the prefix does not match any
object data or form a valid DLO HEAD response if it does.
This also prevents a tricky and potentially unexpected consequence of PUT
tempurls which would make it unsafe to allow a user to download objects
created by tempurl (even if they just created them) because the result of
reading the object created via tempurl may not be the data which was uploaded.
[CVE-2015-5223]
Co-Authored-By: Kota Tsuyuzaki <email address hidden>
Closes-Bug: 1453948
Change-Id: I91161dfb0f089c
| tags: | added: in-stable-juno |
Reviewed: https:/
Committed: https:/
Submitter: Jenkins
Branch: master
commit 10b2939b433a4a7
Author: Clay Gerrard <email address hidden>
Date: Thu Jul 23 22:36:21 2015 -0700
Disallow unsafe tempurl operations to point to unauthorized data
Do not allow PUT tempurls to create pointers to other data. Specifically
disallow the creation of DLO object manifests by returning an error if a
non-safe tempurl request includes an X-Object-Manifest header regardless of
the value of the header.
This prevents discoverability attacks which can use any PUT tempurl to probe
for private data by creating a DLO object manifest and then using the PUT
tempurl to head the object which would 404 if the prefix does not match any
object data or form a valid DLO HEAD response if it does.
This also prevents a tricky and potentially unexpected consequence of PUT
tempurls which would make it unsafe to allow a user to download objects
created by tempurl (even if they just created them) because the result of
reading the object created via tempurl may not be the data which was uploaded.
[CVE-2015-5223]
Co-Authored-By: Kota Tsuyuzaki <email address hidden>
Change-Id: I11e68830009d3f
Closes-Bug: 1453948
| Changed in swift: | |
| status: | Confirmed → Fix Committed |
Reviewed: https:/
Committed: https:/
Submitter: Jenkins
Branch: stable/kilo
commit 410778b86a49702
Author: Clay Gerrard <email address hidden>
Date: Thu Jul 23 22:36:21 2015 -0700
Disallow unsafe tempurl operations to point to unauthorized data
Do not allow PUT tempurls to create pointers to other data. Specifically
disallow the creation of DLO object manifests by returning an error if a
non-safe tempurl request includes an X-Object-Manifest header regardless of
the value of the header.
This prevents discoverability attacks which can use any PUT tempurl to probe
for private data by creating a DLO object manifest and then using the PUT
tempurl to head the object which would 404 if the prefix does not match any
object data or form a valid DLO HEAD response if it does.
This also prevents a tricky and potentially unexpected consequence of PUT
tempurls which would make it unsafe to allow a user to download objects
created by tempurl (even if they just created them) because the result of
reading the object created via tempurl may not be the data which was uploaded.
[CVE-2015-5223]
Co-Authored-By: Kota Tsuyuzaki <email address hidden>
Closes-Bug: 1453948
Change-Id: I91161dfb0f089c
| tags: | added: in-stable-kilo |
| Changed in swift: | |
| milestone: | none → 2.4.0 |
| status: | Fix Committed → Fix Released |
Fix proposed to branch: feature/crypto
Review: https:/
Reviewed: https:/
Committed: https:/
Submitter: Jenkins
Branch: master
commit 58a10a5fffec693
Author: Alistair Coles <email address hidden>
Date: Wed Aug 26 16:30:23 2015 +0100
Add test that a tempurl POST cannot set a DLO manifest header
Follow up to [1] to add tests for tempurl POSTs not being allowed
to set a DLO manifest header.
[1] I11e68830009d3f
Change-Id: I7c0ad5a936f71e
Related-Bug: 1453948
Reviewed: https:/
Committed: https:/
Submitter: Jenkins
Branch: feature/crypto
commit e02609c66a80484
Author: Samuel Merritt <email address hidden>
Date: Tue Sep 1 15:19:50 2015 -0700
Preserve traceback in swift-dispersio
Commit c690bcb fixed a bug in the dispersion report, but changed this
from a bare "raise" to "raise err", which loses the traceback. Not a
big deal, but worth putting back IMO.
Change-Id: Id5b72153a4b8df
commit d06d4ad0fd2dfe6
Author: Minwoo Bae <email address hidden>
Date: Tue Sep 1 15:08:44 2015 -0500
Included reference in swift.obj.diskfile to enumerate the string
used for data file paths.
Change-Id: Ie22caa678bc00d
commit 524c89b7eeff037
Author: John Dickinson <email address hidden>
Date: Fri Aug 21 13:39:41 2015 -0700
Updated CHANGELOG, AUTHORS, and .mailmap for 2.4.0 release.
Change-Id: Ic6301146b839c9
commit 05de1305a903ee4
Author: Clay Gerrard <email address hidden>
Date: Thu Aug 27 18:35:09 2015 -0700
Make ssync_sender send valid chunked requests
The connect method of ssync_sender tells the remote connection that it's
going to send a valid HTTP chunked request, but if the remote end needs
to respond with an error of any kind sender throws HTTP right out the
window, picks up his ball, and closes the socket down hard - much to the
surprise of the eventlet.wsgi server who up to this point had been
playing along quite nicely with this 'SSYNC' nonsense assuming that
everyone here is consenting mature adults.
If you're going to make a "Transfer-Encoding: chunked" request have the
good decency to finish the job with a proper '0\r\n\r\n'. [1]
N.B. It might be possible to handle an error status during the
initialize_
honestly it's not entirely clear to me when the server isn't going to
close the connection if the client is still expected to send the body
[2] - further if the error comes later during missing_check or updates
we'll for sure want to send the chunk transfer termination line before
we close down the socket and this way we cover both.
1. Really, eventlet.wsgi shouldn't be so blasted brittle about this [3]
2. https:/
3. https:/
Closes-Bug #1489587
Change-Id: Ic17c6c3075553f
commit 993ee4e37af1961
Author: nakagawamsa <email address hidden>
Date: Fri Aug 28 11:49:43 2015 +0900
Remove duplicate X-Backend-
There is duplicate 'X-Backend-
One key has fixed policy index value, and another ha...
| tags: | added: in-feature-crypto |
| Changed in ossa: | |
| status: | Fix Committed → Fix Released |
Related fix proposed to branch: feature/hummingbird
Review: https:/
Reviewed: https:/
Committed: https:/
Submitter: Jenkins
Branch: feature/hummingbird
commit cb683d391cb66d0
Author: OpenStack Proposal Bot <email address hidden>
Date: Sat Sep 5 06:17:51 2015 +0000
Imported Translations from Transifex
For more information about this automatic import see:
https:/
Change-Id: I2d92b8e34a665f
commit e4542455c8a07b7
Author: Emett Speer <email address hidden>
Date: Wed Sep 2 17:18:03 2015 -0700
[Labs] Update links to Cloud Admin Guide
Update links to the Cloud Admin Guide after the
RST conversion of that book altered URLs.
Change-Id: I899f8938498b74
commit 58fcc0752397830
Author: Christian Schwede <email address hidden>
Date: Wed Sep 2 10:52:34 2015 +0000
Test if container_sweep is executed on unmounted devices
This change ensures that container_sweep is not run if a device is not mounted
and mount_check is set to True.
Change-Id: I823083c8431d9e
commit e02609c66a80484
Author: Samuel Merritt <email address hidden>
Date: Tue Sep 1 15:19:50 2015 -0700
Preserve traceback in swift-dispersio
Commit c690bcb fixed a bug in the dispersion report, but changed this
from a bare "raise" to "raise err", which loses the traceback. Not a
big deal, but worth putting back IMO.
Change-Id: Id5b72153a4b8df
commit d06d4ad0fd2dfe6
Author: Minwoo Bae <email address hidden>
Date: Tue Sep 1 15:08:44 2015 -0500
Included reference in swift.obj.diskfile to enumerate the string
used for data file paths.
Change-Id: Ie22caa678bc00d
commit 615c7a204b9386e
Author: Brian Cline <email address hidden>
Date: Tue Sep 1 10:51:20 2015 -0500
Adds useful dispersion info from changelog
Change-Id: I1a45088fc32620
commit 3b8755098a1786c
Author: janonymous <email address hidden>
Date: Sun Aug 2 21:29:13 2015 +0530
Replace a / b with a // b to use integer division where needed
Change-Id: I72c81faa62786e
commit 524c89b7eeff037
Author: John Dickinson <email address hidden>
Date: Fri Aug 21 13:39:41 2015 -0700
Updated CHANGELOG, AUTHORS, and .mailmap for 2.4.0 release.
Change-Id: Ic6301146b839c9
commit 05de1305a903ee4
Author: Clay Gerrard <email address hidden>
Date: Thu Aug 27 18:35:09 2015 -0700
Make ssync_sender send valid chunked requests
The connect method of ssync_sender tells the remote connection that it's
going to send a valid HTTP chunked request, but if the remote end needs
to respond with an error of any kind sender th...
| tags: | added: in-feature-hummingbird |
Related fix proposed to branch: feature/crypto
Review: https:/
Change abandoned by John Dickinson (<email address hidden>) on branch: feature/crypto
Review: https:/
Reason: jrichli will do this
Reviewed: https:/
Committed: https:/
Submitter: Jenkins
Branch: feature/crypto
commit 9e95613d717489d
Author: Alistair Coles <email address hidden>
Date: Mon Sep 21 10:01:54 2015 +0100
Fix .coveragrc to prevent nose tests error
Since v4.0 the coverage package raises an error if unrecognized
options are found in .coveragrc [1]. Previously they were ignored. The
ignore-errors option therefore causes nosetests with coverage to error
because the option should be ignore_errors (underscore not hyphen).
[1] https:/
Change-Id: Ic488801b7cc432
commit 1fe8e4327b15f89
Author: Clay Gerrard <email address hidden>
Date: Fri Sep 18 13:54:52 2015 -0700
Fix recon tests on SAIO with multiple policies
Recon middleware used to only look on rings that exist on disk when it was
started, so if a test didn't create a ring in the temp swift_dir it can expect
the middleware to not report it.
However, after we started looking at policies to determine rings [1] - we need
to be more careful to patch policies to match up with the test requirements.
On development environments with only the legacy default polices the existing
recon tests were passing by accident - but not in my environment.
This change will patch policies for the TestCase so that tests will pass for
me. Individual test methods that have more specific policy requirements for
the test can continue to @patch_policies just for those tests but in general
the existing test_methods all seem to expect legacy policies - so we just make
the default for the TestCase legacy_only.
Change-Id: I778a0a59091ca8
commit a63f70c17d39237
Author: Minwoo Bae <email address hidden>
Date: Wed Sep 9 15:33:45 2015 -0500
Reconstructor logging to omit 404 warnings
Currently, the replicator does not log warning messages
for 404 responses. We would like the reconstructor to
do the same, as 404s are not considered unusual, and
are already handled by the object server.
Change-Id: Ia927bf30362548
Closes-Bug: #1491883
commit 530102ae07ea27b
Author: Bill Huber <email address hidden>
Date: Mon Sep 14 16:01:39 2015 -0500
Update EC Support on how to build an EC ring with replicas count
This doc is being updated to specify the replicas count parameter
to build an EC ring that enforces both data and parity placements
for each partition.
Change-Id: I770ad268e4017e
Closes-Bug: 1487203
commit 0d9142abd45f189
Author: Clay Gerrard <email address hidden>
Date: Mon Sep 14 17:17:29 2015 -0700
Fix typo in Deployment Guide and add some formatting
Change-Id: I58703162bf3e9f
commit 460a7e4b64d134d
I have a feeling this might be a significant enough behavior change (breaking a workflow which application developers might already have encoded into their software) that we wouldn't be able to safely backport it without having it as an optional mitigation that defaults to the original behavior. Then deployers can choose explicitly to disallow PUT DLO's via tempurl in their environments without forcing it on all deployments consuming stable branches.
If we do end up making it configurable and leave the original behavior as the default, then this is territory for documenting in a security note not an advisory.