OpenStack Object Storage (swift)

DLO GET/HEAD does not work if manifest value contains a '?'

Bug #1762997 reported by Christopher Bartz on 2018-04-11

This bug affects 1 person

Affects		Status	Importance	Assigned to	Milestone
	OpenStack Object Storage (swift)	Fix Released	Undecided	Unassigned

Bug Description

It is possible to create a DLO Manifest with a segments container containing a question mark inside. E.g.

curl -XPUT ... -H"x-object-manifest: a%3Fcontainer/prefix"

But if this DLO Object should be retrieved (GET or HEAD), a 404 (or incorrect 2xx) is returned.

The reason lies in the DLO Middleware: It unquotes the container [1], but does not quote it again when doing the container listing [2]. Because the path contains now an '?', the '?' is used as the path/query separator, and everything after the '?' is cutted. Thus the internal container listing request looks like this in the proxy log:

Apr 11 12:24:21 swift proxy-server: - - 11/Apr/2018/10/24/21 GET /v1/AUTH_account/a%3Fformat%3Djson%26prefix%3Dprefix HTTP/1.0 404

(be aware that the log line is quoted again)

This means the container listing is done on the container called "a" and not "a?container". If this container does not exist, a 404 is returned. If the container does exist, an incorrect 2xx response would be returned.

A similar problem arises, if the object prefix contains a '?' and a GET is performed. A 409 conflict is returned (if filesize > 0) and the traceback looks like:

Apr 11 13:54:20 swift proxy-server: - - 11/Apr/2018/11/54/20 GET /v1/AUTH_account/test_segments/serv%3F.json/1523441992.908/0%3Fmultipart-manifest%3Dget HTTP/1.0 499 - - 70 - tx5157e02bd5ab4e6fa345a-005acdf76c - 0.0202 DLO - 1523447660.865747929 1523447660.885992050 0
Apr 11 13:54:20 swift proxy-server: ERROR: An error occurred while retrieving segments: #012Traceback (most recent call last):#012 File "/usr/lib/python2.7/site-packages/swift/common/request_helpers.py", line 442, in _internal_iter#012 (self.name, seg_resp.status_int, seg_req.path))#012SegmentError: ERROR: While processing manifest /v1/AUTH_account/test/serv%3F.json, got 404 while retrieving /v1/AUTH_account/test_segments/serv (txn: tx5157e02bd5ab4e6fa345a-005acdf76c)

[1] https://github.com/openstack/swift/blob/master/swift/common/middleware/dlo.py#L233-L234
[2] https://github.com/openstack/swift/blob/master/swift/common/middleware/dlo.py#L147

See original description

Tags:

Christopher Bartz (bartz) on 2018-04-11

description:

updated

Christopher Bartz (bartz) on 2018-04-11

summary:	- dlo does not work when segments container contains a '?' + DLO GET/HEAD does not work if manifest value contains a '?'
description:	updated

Christopher Bartz (bartz) on 2018-04-12

description:

updated

Revision history for this message

Tim Burke (1-tim-z) wrote on 2018-04-12:

Well, I guess we've at least stopped returning 500 since https://bugs.launchpad.net/swift/+bug/1598093 ... but things are downright hilariously bad!

> If the container does exist, an incorrect 2xx response would be returned.

You ain't kidding! If 'a' doesn't include any objects starting with 'prefix' at least we just send an empty 200. But if there *are* objects, it returns as much of the JSON-formatted *full* container listing as the sum of sizes from the prefix listing allows!

Changed in swift:
status:	New → Confirmed

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2019-03-21: Fix merged to swift (master)

Reviewed: https://review.openstack.org/571905
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=fa678949ae310aa0499938fef788ec04409625d9
Submitter: Zuul
Branch: master

commit fa678949ae310aa0499938fef788ec04409625d9
Author: Tim Burke <email address hidden>
Date: Wed May 30 11:43:40 2018 -0700

Fix quoting for large objects

    Change-Id: I46bdb6da8f778a6c86e0f8e883b52fc31e9fd44e
    Partial-Bug: 1774238
    Closes-Bug: 1678022
    Closes-Bug: 1598093
    Closes-Bug: 1762997

Changed in swift:
status:	Confirmed → Fix Released

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2019-03-25: Fix included in openstack/swift 2.21.0

This issue was fixed in the openstack/swift 2.21.0 release.

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2019-03-27: Fix proposed to swift (feature/losf)

Fix proposed to branch: feature/losf
Review: https://review.openstack.org/648245

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2019-03-28: Fix merged to swift (feature/losf)

Download full text (11.9 KiB)

Reviewed: https://review.openstack.org/648245
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=6afc1130fd753306d64745c9bee7712182b273d3
Submitter: Zuul
Branch: feature/losf

commit 89e5927f7dd94fc28b3847944eb7dd227d516fa8
Author: Thiago da Silva <email address hidden>
Date: Tue Mar 26 10:46:02 2019 -0400

Fix mocking time

When running on Centos the side_effect was returning a MagicMock
object instead of the intended int.

Change-Id: I73713a9a96dc415073a637d85a40304021f76072

commit 50715acb1838fbde628e447e7b02545ce8469180
Author: OpenStack Release Bot <email address hidden>
Date: Mon Mar 25 17:07:54 2019 +0000

Update master for stable/stein

Add file to the reno documentation build to show release notes for
stable/stein.

    Use pbr instruction to increment the minor version number
    automatically so that master versions are higher than the versions on
    stable/stein.

Change-Id: I6109bff3227f87d914abf7bd1d76143aaf91419d
Sem-Ver: feature

commit 179fa7ccd4d6faeacc989715887b69f9422a17b2
Author: John Dickinson <email address hidden>
Date: Mon Mar 18 17:09:31 2019 -0700

authors/changelog update for 2.21.0 release

Change-Id: Iac51a69c71491e5a8db435aae396178a6c592c73

commit 64eec5fc93eb670e581cbb3a6dedb6a7aa501e99
Author: Tim Burke <email address hidden>
Date: Thu Mar 7 14:36:02 2019 -0800

Fix how we UTF-8-ify func tests

I noticed while poking at the DLO func tests that we don't actually use
non-ascii chars when we set up the test env.

By patching the create name function earlier (in SetUpClass) we can
ensure we get some more interesting characters in our object names.

Change-Id: I9480ddf74463310aeb11ad876b79527888d8c871

commit fe3a20f2e4b745bf7d81f9bda97082b593e8794a
Author: Tim Burke <email address hidden>
Date: Tue Mar 19 14:52:19 2019 -0700

Remove uncalled function

Change-Id: Ica67815f0ddf4b00bce1ffe183735490c7f7c0b5
Related-Change: I5629de9f2e9b2331ed3f455d253efc69d030df72

commit adc568c97f5b30d9d4628eaf448f81d736ad4e51
Author: John Dickinson <email address hidden>
Date: Fri Mar 15 15:18:36 2019 -0700

Fix bulk responses when using xml and Expect 100-continue

    When we fixed bulk response heartbeating in https://review.openstack.org/#/c/510715/,
    code review raised the issue of moving the xml header down to after the
    early-exit clauses. At the time, it didn't seem to break anything, so
    it was left in place. However, that insight was correct.

    The purpose of the earlier patch was to force eventlet to use chunked
    transfer encoding on the response in order to prevent eventlet from
    buffering the whole response, thus defeating the purpose of the
    heartbeat responses.

    Moving the first line of the body lower (ie after the early exit
    checks), allows other headers in a chunked transfer encoding response
    to be appropriately processed before sending the headers. Sending the
    xml declaration early causes it to get intermingled in the 100-continue
    protocol, thus breaking the chunked transfer encoding semantics.

Closes-Bug: #1819...

Reviewed:  https://review.openstack.org/648245
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=6afc1130fd753306d64745c9bee7712182b273d3
Submitter: Zuul
Branch:    feature/losf

commit 89e5927f7dd94fc28b3847944eb7dd227d516fa8
Author: Thiago da Silva <thiagodasilva@gmail.com>
Date:   Tue Mar 26 10:46:02 2019 -0400

Fix mocking time
    
    When running on Centos the side_effect was returning a MagicMock
    object instead of the intended int.
    
    Change-Id: I73713a9a96dc415073a637d85a40304021f76072

commit 50715acb1838fbde628e447e7b02545ce8469180
Author: OpenStack Release Bot <infra-root@openstack.org>
Date:   Mon Mar 25 17:07:54 2019 +0000

Update master for stable/stein
    
    Add file to the reno documentation build to show release notes for
    stable/stein.
    
    Use pbr instruction to increment the minor version number
    automatically so that master versions are higher than the versions on
    stable/stein.
    
    Change-Id: I6109bff3227f87d914abf7bd1d76143aaf91419d
    Sem-Ver: feature

commit 179fa7ccd4d6faeacc989715887b69f9422a17b2
Author: John Dickinson <me@not.mn>
Date:   Mon Mar 18 17:09:31 2019 -0700

authors/changelog update for 2.21.0 release
    
    Change-Id: Iac51a69c71491e5a8db435aae396178a6c592c73

commit 64eec5fc93eb670e581cbb3a6dedb6a7aa501e99
Author: Tim Burke <tim.burke@gmail.com>
Date:   Thu Mar 7 14:36:02 2019 -0800

Fix how we UTF-8-ify func tests
    
    I noticed while poking at the DLO func tests that we don't actually use
    non-ascii chars when we set up the test env.
    
    By patching the create name function earlier (in SetUpClass) we can
    ensure we get some more interesting characters in our object names.
    
    Change-Id: I9480ddf74463310aeb11ad876b79527888d8c871

commit fe3a20f2e4b745bf7d81f9bda97082b593e8794a
Author: Tim Burke <tim.burke@gmail.com>
Date:   Tue Mar 19 14:52:19 2019 -0700

Remove uncalled function
    
    Change-Id: Ica67815f0ddf4b00bce1ffe183735490c7f7c0b5
    Related-Change: I5629de9f2e9b2331ed3f455d253efc69d030df72

commit adc568c97f5b30d9d4628eaf448f81d736ad4e51
Author: John Dickinson <me@not.mn>
Date:   Fri Mar 15 15:18:36 2019 -0700

Fix bulk responses when using xml and Expect 100-continue
    
    When we fixed bulk response heartbeating in https://review.openstack.org/#/c/510715/,
    code review raised the issue of moving the xml header down to after the
    early-exit clauses. At the time, it didn't seem to break anything, so
    it was left in place. However, that insight was correct.
    
    The purpose of the earlier patch was to force eventlet to use chunked
    transfer encoding on the response in order to prevent eventlet from
    buffering the whole response, thus defeating the purpose of the
    heartbeat responses.
    
    Moving the first line of the body lower (ie after the early exit
    checks), allows other headers in a chunked transfer encoding response
    to be appropriately processed before sending the headers. Sending the
    xml declaration early causes it to get intermingled in the 100-continue
    protocol, thus breaking the chunked transfer encoding semantics.
    
    Closes-Bug: #1819252
    
    Change-Id: I072f4dab21cd7cdb81b9e41072eb504131411dc8

commit 585bf40cc0d8d88849dcf11d409e8c5a2a202a8d
Author: Clay Gerrard <clay.gerrard@gmail.com>
Date:   Mon Feb 18 20:05:46 2019 -0600

Simplify empty suffix handling
    
    We really only need to have one way to cleanup empty suffix dirs, and
    that's normally during suffix hashing which only happens when invalid
    suffixes get rehashed.
    
    When we iterate a suffix tree using yield hashes, we may discover an
    expired or otherwise reapable hashdir - when this happens we will now
    simply invalidate the suffix so that the next rehash can clean it up.
    
    This simplification removes an mis-behavior in the handling between the
    normal suffix rehashing cleanup and what was implemented in ssync.
    
    Change-Id: I5629de9f2e9b2331ed3f455d253efc69d030df72
    Related-Change-Id: I2849a757519a30684646f3a6f4467c21e9281707
    Closes-Bug: 1816501

commit e5eb673ccb5d3517107d28f6ce0672b066f53964
Author: Tim Burke <tim.burke@gmail.com>
Date:   Fri Mar 1 14:00:35 2019 -0800

Stop monkey-patching mimetools
    
    You could *try* doing something similar to what we were doing
    there over in email.message for py3, but you would end up
    breaking pkg_resources (and therefor entrypoints) in the
    process.
    
    Drive-by: have mem_diskfile implement more of the diskfile API.
    
    Change-Id: I1ece4b4500ce37408799ee634ed6d7832fb7b721

commit d6af42b6b6d54713f09c3e1e983435bf2c3fa07d
Author: Tim Burke <tim.burke@gmail.com>
Date:   Tue Feb 19 13:53:07 2019 -0800

Clean up how we walk through ranges in ECAppIter
    
    Besides being easier to reason about, this also lets us
    run more unit tests under py37 which complains about a
    a generator raising StopIteration
    
    Change-Id: Ia6b945afef51bcc8ed20a7069fc60d5b8f9c9c0b

commit c9773bfd2664f7090f590d288d9010d13851ea92
Author: Tim Burke <tim.burke@gmail.com>
Date:   Wed Mar 13 16:20:00 2019 -0700

Add non-voting py37 unit test job
    
    Change-Id: I83f8f59023eabc97386481c18ed8bbf8fab64fa8

commit 95da1d97b11b43d04d20b98838ddc0c4f20cb6be
Author: Tim Burke <tim.burke@gmail.com>
Date:   Wed Mar 13 16:29:09 2019 -0700

Fix py35 unit test job
    
    Looks like some base templates got moved from xenial to bionic, which
    doesn't have py35. Explicitly say that this job needs xenial.
    
    Change-Id: I44df8736d0c33fc2c58c9be6b5b8023932f14a83

commit 53b56b65512fabc97890464c91faafdd0e3dbdaf
Author: John Dickinson <me@not.mn>
Date:   Wed Mar 13 11:41:00 2019 -0700

crediting contributors to the un-landed hummingbird branch
    
    Change-Id: I51708cb2f0deca61b147589e062b520ac7a1807e

commit fa678949ae310aa0499938fef788ec04409625d9
Author: Tim Burke <tim.burke@gmail.com>
Date:   Wed May 30 11:43:40 2018 -0700

Fix quoting for large objects
    
    Change-Id: I46bdb6da8f778a6c86e0f8e883b52fc31e9fd44e
    Partial-Bug: 1774238
    Closes-Bug: 1678022
    Closes-Bug: 1598093
    Closes-Bug: 1762997

commit a30a477755f669a11aef5ce492f287627565d978
Author: Kota Tsuyuzaki <tsuyuzaki.kota@lab.ntt.co.jp>
Date:   Wed Feb 27 12:52:06 2019 +0900

Stop overwriting reserved term
    
    `dir` is a reserved instruction term in python, so this patch
    avoiding to assing a value to it.
    
    Change-Id: If780c4ffb72808b834e25a396665f17bd8383870

commit 74664af7ed761a729fbb9130e86ccff4070f0dcb
Author: Michele Valsecchi <mvalsecc@redhat.com>
Date:   Tue Mar 12 13:56:27 2019 +0900

Fix a typo
    
    Replace 'o' with 'to'.
    
    Change-Id: I0a9b1547016b2662002c050e8388591d7d91ef97

commit 13e7f3641e3bffbcf89733ebb50d3ca6847105c6
Author: zhufl <zhu.fanglei@zte.com.cn>
Date:   Mon Mar 11 14:28:20 2019 +0800

Do not use self in classmethod
    
    cls should be used in classmethd, instead of self.
    
    Change-Id: I149b18935ce909ef978f2b7147b109e11c22f923

commit e1a12dc3dd04bc63d6b5b31d4ffd6a96bf8af918
Author: Clay Gerrard <clay.gerrard@gmail.com>
Date:   Wed Mar 6 16:37:59 2019 -0800

Refactor write_affinity DELETE handling
    
    There's some code duplication we can drop, and some tests scenarios we
    can expand on.  I don't believe there's any behavior change here.
    
    Change-Id: I2271d1cb757c989c4b0bfe228cd26c8620a151db

commit d748851766309b7def5947025457de820219f9ec
Author: Tim Burke <tim.burke@gmail.com>
Date:   Tue Mar 5 14:50:22 2019 -0800

s3token: Add note about config change when upgrading from swift3
    
    Change-Id: I2610cbdc9b7bc2b4d614eaedb4f3369d7a424ab3

commit d185b607bbdda8b47b0bb090f045a6b4ad8ed8b9
Author: Tim Burke <tim.burke@gmail.com>
Date:   Mon Mar 4 17:37:09 2019 -0800

docs: clean up SAIO formatting
    
    Drive-by: use six.moves in s3api; fix "unexpected indent" warning when building
    docs on py3
    
    Change-Id: I2a354e2624c763a68fcea7a6404e9c2fde30d631

commit 4ac81ebbd73784e0e1faf7c3e983b38ea4a66754
Author: Tim Burke <tim.burke@gmail.com>
Date:   Fri Mar 1 13:04:58 2019 -0800

py3: fix copying unicode names
    
    Turns out, unquote()ing WSGI strings is a great way to mangle them.
    
    Change-Id: I42a08d84aa22a1a7ee7ccab97aaec55d845264f9

commit 5d4303edbf601c5ff692a378c11ed5da9aa407c9
Author: Tim Burke <tim.burke@gmail.com>
Date:   Thu Feb 21 14:34:48 2019 -0800

manage-shard-ranges: nicer message if we can't get_info()
    
    Tracebacks are ugly.
    
    Change-Id: I09b907608127e4c633b554be2926245b35402dbf

commit 61e6ac0ebddc630390dfbe1292cd392c57f0ca07
Author: Pete Zaitcev <zaitcev@kotori.zaitcev.us>
Date:   Tue Feb 26 23:06:52 2019 -0600

py3: port formpost middleware
    
    Change-Id: I8f3d4d5f6976ef5b63facd9b5723aac894066b74

commit baf18edc00851f6749a40794587ca14a52135bf3
Author: Tim Burke <tim.burke@gmail.com>
Date:   Thu Oct 18 10:35:31 2018 -0700

Clean up account-reaper a bit
    
    - Drop the (partial) logging translation
    - Save our log concatenations until the end
    - Stop encoding object names; direct_client is happy to take Unicode
    - Remove a couple loop breaks that were only used by tests
    
    Change-Id: I4a4f301a7a6cb0f217ca0bf8712ee0291bbc14a3
    Partial-Bug: #1674543

commit 9b3ca9423eb8cf9420a3e98f60cd56dd281b4208
Author: Simeon Gourlin <simeon.gourlin@infomaniak.com>
Date:   Tue Jan 29 09:13:16 2019 +0100

Fix decryption for broken objects
    
    Try to get decryption object key from stored metadata (key_id path from
    X-Object-Sysmeta-Crypto-Body-Meta) because sometime object.path is wrong
    during encryption process.
    This patch doesn't solve the underlying issue, but is needed to decrypt
    already wrongly stored objects.
    
    Change-Id: I1a6bcdebdb46ef03c342428aeed73ae76db29922
    Co-Author: Thomas Goirand <thomas@goirand.fr>
    Partial-Bug: #1813725

commit 3a8f5dbf9c49fdf1cf2d0b7ba35b82f25f88e634
Author: Tim Burke <tim.burke@gmail.com>
Date:   Tue Dec 11 15:29:35 2018 -0800

Verify client input for v4 signatures
    
    Previously, we would use the X-Amz-Content-SHA256 value when calculating
    signatures, but wouldn't actually check the content that was sent. This
    would allow a malicious third party that managed to capture the headers
    for an object upload to overwrite that with arbitrary content provided
    they could do so within the 5-minute clock-skew window.
    
    Now, we wrap the wsgi.input that's sent on to the proxy-server app to
    hash content as it's read and raise an error if there's a mismatch. Note
    that clients using presigned-urls to upload have no defense against a
    similar replay attack.
    
    Notwithstanding the above security consideration, this *also* provides
    better assurances that the client's payload was received correctly. Note
    that this *does not* attempt to send an etag in footers, however, so the
    proxy-to-object-server connection is not guarded against bit-flips.
    
    In the future, Swift will hopefully grow a way to perform SHA256
    verification on the object-server. This would offer two main benefits:
    
      - End-to-end message integrity checking.
      - Move CPU load of calculating the hash from the proxy (which is
        somewhat CPU-bound) to the object-server (which tends to have CPU to
        spare).
    
    Change-Id: I61eb12455c37376be4d739eee55a5f439216f0e9
    Closes-Bug: 1765834

commit 37693a4e1523fc61d653e231e57d33b37464c2b5
Author: Tim Burke <tim.burke@gmail.com>
Date:   Thu Dec 27 22:34:05 2018 +0000

Run ceph-s3-tests job less
    
    We don't need it for unit-test-only changes or most doc changes.
    
    Change-Id: I803e0dc6861786db44cbcf5943032424ba319d54

commit a563ba26fa3d9dfb23b368ed79940c19e3a9135c
Author: HCLTech-SSW <hcl_ss_oss@hcl.com>
Date:   Mon May 14 23:23:57 2018 -0700

Implemented the fix to handle the HTTP request methods other than GET.
    
    Change-Id: I8db01a5a59f72c562aa8039b459a965283b1b3ad
    Closes-Bug: #1695855

tags:

added: in-feature-losf

Report a bug

This report contains Public information

Everyone can see this information.

You are

Subscribing...

Edit bug mail

Other bug subscribers

Remote bug watches

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