Non-ASCII query params can cause 503s

Bug #1843816 reported by Tim Burke
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Undecided
Unassigned

Bug Description

Seen in the stable checks recently:

proxy ERROR: ERROR with Container server 127.0.0.1:50284/sdb1 re: Trying to GET /v1/a/5_vc_non_ascii%C2%A3:
Traceback (most recent call last):
  File ".../swift/proxy/controllers/base.py", line 1121, in _make_node_request
    query_string=self.req_query_string)
  File ".../swift/common/bufferedhttp.py", line 217, in http_connect
    ipaddr, port, method, path, headers, query_string, ssl)
  File ".../swift/common/bufferedhttp.py", line 245, in http_connect_raw
    conn.putrequest(method, path, skip_host=(headers and 'Host' in headers))
  File ".../swift/common/bufferedhttp.py", line 169, in putrequest
    skip_accept_encoding)
  File "/usr/lib/python2.7/httplib.py", line 960, in putrequest
    % (url, match.group()))
InvalidURL: URL can't contain control characters. '/sdb1/3/a/5_vc_non_ascii%C2%A3?prefix=00f5_o_non_ascii\xc2\xa3/' (found at least '\xc2')

(See https://174f6a355be5fe262f0c-fe779e5654b21fdff79727b204dfb7d6.ssl.cf5.rackcdn.com/periodic-stable/opendev.org/openstack/swift/stable/ocata/openstack-tox-py27/8efb03a/job-output.txt for example)

Apparently recent versions of py27 (those after https://github.com/python/cpython/commit/bb8071a; see https://bugs.python.org/issue30458) have begun raising InvalidURL if you try to include non-ASCII characters in the request path. This was observed in the periodic checks of both stable/ocata and stable/pike. In particular, we would spin up some in-process servers in test.unit.proxy.test_server.TestSocketObjectVersions and do a container listing with a prefix param that included raw (unquoted) UTF-8. This query string would pass unmolested through the proxy, tripping the InvalidURL error when bufferedhttp called putrequest.

More recent versions of Swift would not exhibit this particular failure, as the listing_formats middleware would force a decoding/re-encoding of the query string for account and container requests. However, object requests with errant query strings would likely be able to trip the same error.

Swift on py3 should not exhibit this behavior, as we so thoroughly re-write the request line to avoid hitting https://bugs.python.org/issue33973.

Impact on a cluster should be pretty low; the error's tripped before we make any backend requests. My main concern would be that this could be used to try to probe what version of python the proxy is running.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (stable/pike)

Fix proposed to branch: stable/pike
Review: https://review.opendev.org/681879

Revision history for this message
Tim Burke (1-tim-z) wrote :

Ha! Affects func tests, too -- master has https://github.com/openstack/swift/commit/c0ae48b which should avoid it, but I'd expect all stable branches through stein to be affected.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (stable/stein)

Fix proposed to branch: stable/stein
Review: https://review.opendev.org/682112

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (stable/pike)

Fix proposed to branch: stable/pike
Review: https://review.opendev.org/683284

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on swift (stable/pike)

Change abandoned by Tim Burke (<email address hidden>) on branch: stable/pike
Review: https://review.opendev.org/681879
Reason: Have to land two fixes at once; dropping in favor of https://review.opendev.org/#/c/683284/

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (stable/rocky)

Fix proposed to branch: stable/rocky
Review: https://review.opendev.org/683754

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (stable/queens)

Fix proposed to branch: stable/queens
Review: https://review.opendev.org/683756

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (stable/ocata)

Fix proposed to branch: stable/ocata
Review: https://review.opendev.org/683757

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

Reviewed: https://review.opendev.org/681875
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=49f62f6ab7fd1b833e9b5bfbcaafa4b45b592d34
Submitter: Zuul
Branch: master

commit 49f62f6ab7fd1b833e9b5bfbcaafa4b45b592d34
Author: Tim Burke <email address hidden>
Date: Thu Sep 12 10:59:08 2019 -0700

    bufferedhttp: ensure query params are properly quoted

    Recent versions of py27 [1] have begun raising InvalidURL if you try to
    include non-ASCII characters in the request path. This was observed
    recently in the periodic checks of stable/ocata and stable/pike. In
    particular, we would spin up some in-process servers in
    test.unit.proxy.test_server.TestSocketObjectVersions and do a container
    listing with a prefix param that included raw (unquoted) UTF-8. This
    query string would pass unmolested through the proxy, tripping the
    InvalidURL error when bufferedhttp called putrequest.

    More recent versions of Swift would not exhibit this particular failure,
    as the listing_formats middleware would force a decoding/re-encoding of
    the query string for account and container requests. However, object
    requests with errant query strings would likely be able to trip the same
    error.

    Swift on py3 should not exhibit this behavior, as we so
    thoroughly re-write the request line to avoid hitting
    https://bugs.python.org/issue33973.

    Now, always parse and re-encode the query string in bufferedhttp. This
    prevents any errors on object requests and cleans up any callers that
    might use bufferedhttp directly.

    [1] Anything after https://github.com/python/cpython/commit/bb8071a;
        see https://bugs.python.org/issue30458

    Closes-Bug: 1843816
    Change-Id: I73f84b96f164e6fc5d3cb890355871c26ed271a6
    Related-Change: Id3ce37aa0402e2d8dd5784ce329d7cb4fbaf700d
    Related-Change: Ie648f5c04d4415f3b620fb196fa567ce7575d522

Changed in swift:
status: New → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (stable/stein)

Reviewed: https://review.opendev.org/682112
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=9cc6d4138946034516fdf579ac084cb954ea6b06
Submitter: Zuul
Branch: stable/stein

commit 9cc6d4138946034516fdf579ac084cb954ea6b06
Author: Tim Burke <email address hidden>
Date: Thu Sep 12 10:59:08 2019 -0700

    bufferedhttp: ensure query params are properly quoted

    Recent versions of py27 [1] have begun raising InvalidURL if you try to
    include non-ASCII characters in the request path. This was observed
    recently in the periodic checks of stable/ocata and stable/pike. In
    particular, we would spin up some in-process servers in
    test.unit.proxy.test_server.TestSocketObjectVersions and do a container
    listing with a prefix param that included raw (unquoted) UTF-8. This
    query string would pass unmolested through the proxy, tripping the
    InvalidURL error when bufferedhttp called putrequest.

    More recent versions of Swift would not exhibit this particular failure,
    as the listing_formats middleware would force a decoding/re-encoding of
    the query string for account and container requests. However, object
    requests with errant query strings would likely be able to trip the same
    error.

    Swift on py3 should not exhibit this behavior, as we so
    thoroughly re-write the request line to avoid hitting
    https://bugs.python.org/issue33973.

    Now, always parse and re-encode the query string in bufferedhttp. This
    prevents any errors on object requests and cleans up any callers that
    might use bufferedhttp directly.

    [1] Anything after https://github.com/python/cpython/commit/bb8071a;
        see https://bugs.python.org/issue30458

    Closes-Bug: 1843816
    Change-Id: I73f84b96f164e6fc5d3cb890355871c26ed271a6
    Related-Change: Id3ce37aa0402e2d8dd5784ce329d7cb4fbaf700d
    Related-Change: Ie648f5c04d4415f3b620fb196fa567ce7575d522
    (cherry picked from commit 49f62f6ab7fd1b833e9b5bfbcaafa4b45b592d34)

tags: added: in-stable-stein
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (stable/rocky)

Reviewed: https://review.opendev.org/683754
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=b164767dcb00ea804a26066f40c7ffd78857702e
Submitter: Zuul
Branch: stable/rocky

commit b164767dcb00ea804a26066f40c7ffd78857702e
Author: Tim Burke <email address hidden>
Date: Thu Sep 12 10:59:08 2019 -0700

    bufferedhttp: ensure query params are properly quoted

    Recent versions of py27 [1] have begun raising InvalidURL if you try to
    include non-ASCII characters in the request path. This was observed
    recently in the periodic checks of stable/ocata and stable/pike. In
    particular, we would spin up some in-process servers in
    test.unit.proxy.test_server.TestSocketObjectVersions and do a container
    listing with a prefix param that included raw (unquoted) UTF-8. This
    query string would pass unmolested through the proxy, tripping the
    InvalidURL error when bufferedhttp called putrequest.

    More recent versions of Swift would not exhibit this particular failure,
    as the listing_formats middleware would force a decoding/re-encoding of
    the query string for account and container requests. However, object
    requests with errant query strings would likely be able to trip the same
    error.

    Swift on py3 should not exhibit this behavior, as we so
    thoroughly re-write the request line to avoid hitting
    https://bugs.python.org/issue33973.

    Now, always parse and re-encode the query string in bufferedhttp. This
    prevents any errors on object requests and cleans up any callers that
    might use bufferedhttp directly.

    [1] Anything after https://github.com/python/cpython/commit/bb8071a;
        see https://bugs.python.org/issue30458

    Closes-Bug: 1843816
    Change-Id: I73f84b96f164e6fc5d3cb890355871c26ed271a6
    Related-Change: Id3ce37aa0402e2d8dd5784ce329d7cb4fbaf700d
    Related-Change: Ie648f5c04d4415f3b620fb196fa567ce7575d522
    (cherry picked from commit 49f62f6ab7fd1b833e9b5bfbcaafa4b45b592d34)

tags: added: in-stable-rocky
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (stable/queens)

Reviewed: https://review.opendev.org/683756
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=a317d33dbcda3681a967ce1ed983020ff3512311
Submitter: Zuul
Branch: stable/queens

commit a317d33dbcda3681a967ce1ed983020ff3512311
Author: Tim Burke <email address hidden>
Date: Thu Sep 12 10:59:08 2019 -0700

    bufferedhttp: ensure query params are properly quoted

    Recent versions of py27 [1] have begun raising InvalidURL if you try to
    include non-ASCII characters in the request path. This was observed
    recently in the periodic checks of stable/ocata and stable/pike. In
    particular, we would spin up some in-process servers in
    test.unit.proxy.test_server.TestSocketObjectVersions and do a container
    listing with a prefix param that included raw (unquoted) UTF-8. This
    query string would pass unmolested through the proxy, tripping the
    InvalidURL error when bufferedhttp called putrequest.

    More recent versions of Swift would not exhibit this particular failure,
    as the listing_formats middleware would force a decoding/re-encoding of
    the query string for account and container requests. However, object
    requests with errant query strings would likely be able to trip the same
    error.

    Swift on py3 should not exhibit this behavior, as we so
    thoroughly re-write the request line to avoid hitting
    https://bugs.python.org/issue33973.

    Now, always parse and re-encode the query string in bufferedhttp. This
    prevents any errors on object requests and cleans up any callers that
    might use bufferedhttp directly.

    [1] Anything after https://github.com/python/cpython/commit/bb8071a;
        see https://bugs.python.org/issue30458

    Closes-Bug: 1843816
    Change-Id: I73f84b96f164e6fc5d3cb890355871c26ed271a6
    Related-Change: Id3ce37aa0402e2d8dd5784ce329d7cb4fbaf700d
    Related-Change: Ie648f5c04d4415f3b620fb196fa567ce7575d522
    (cherry picked from commit 49f62f6ab7fd1b833e9b5bfbcaafa4b45b592d34)

tags: added: in-stable-queens
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (stable/ocata)

Reviewed: https://review.opendev.org/683757
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=53a6386791e3237e2a95db3a1a86a046b11e0664
Submitter: Zuul
Branch: stable/ocata

commit 53a6386791e3237e2a95db3a1a86a046b11e0664
Author: Tim Burke <email address hidden>
Date: Tue Mar 12 13:36:21 2019 -0700

    Fix stable gate

    This is a combination of 2 commits.

    ---

    py2/3: Stop using stdlib's putrequest(); it only does ASCII

    Note that this only affects the functest client.

    See also: https://bugs.python.org/issue36274 and
    https://bugs.python.org/issue38216

    This was previously done just for py3 compatibility, but following
    https://github.com/python/cpython/commit/bb8071a our stable gates are
    all broken -- apparently, they're running a 2.7 pre-release?

    (cherry picked from commit c0ae48ba9aafb0b91869ea3bae8da07a32088777)
    (cherry picked from commit 2b4d58952cae8b174fb60529d5284c1d328e9287)

    ---

    bufferedhttp: ensure query params are properly quoted

    Recent versions of py27 [1] have begun raising InvalidURL if you try to
    include non-ASCII characters in the request path. This was observed
    recently in the periodic checks of stable/ocata and stable/pike. In
    particular, we would spin up some in-process servers in
    test.unit.proxy.test_server.TestSocketObjectVersions and do a container
    listing with a prefix param that included raw (unquoted) UTF-8. This
    query string would pass unmolested through the proxy, tripping the
    InvalidURL error when bufferedhttp called putrequest.

    More recent versions of Swift would not exhibit this particular failure,
    as the listing_formats middleware would force a decoding/re-encoding of
    the query string for account and container requests. However, object
    requests with errant query strings would likely be able to trip the same
    error.

    Swift on py3 should not exhibit this behavior, as we so
    thoroughly re-write the request line to avoid hitting
    https://bugs.python.org/issue33973.

    Now, always parse and re-encode the query string in bufferedhttp. This
    prevents any errors on object requests and cleans up any callers that
    might use bufferedhttp directly.

    [1] Anything after https://github.com/python/cpython/commit/bb8071a;
        see https://bugs.python.org/issue30458

    Closes-Bug: 1843816
    Related-Change: Id3ce37aa0402e2d8dd5784ce329d7cb4fbaf700d
    Related-Change: Ie648f5c04d4415f3b620fb196fa567ce7575d522
    (cherry picked from commit 49f62f6ab7fd1b833e9b5bfbcaafa4b45b592d34)
    (cherry picked from commit 9cc6d4138946034516fdf579ac084cb954ea6b06)

    ---

    Change-Id: I4eafc5f057df8a3c15560ace255d05602db56ef6

tags: added: in-stable-ocata
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (stable/pike)

Reviewed: https://review.opendev.org/683284
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=1dd0e1ca4b1a18743f2d20cbf958114293400178
Submitter: Zuul
Branch: stable/pike

commit 1dd0e1ca4b1a18743f2d20cbf958114293400178
Author: Tim Burke <email address hidden>
Date: Tue Mar 12 13:36:21 2019 -0700

    Fix stable gate

    This is a combination of 2 commits.

    ---

    py2/3: Stop using stdlib's putrequest(); it only does ASCII

    Note that this only affects the functest client.

    See also: https://bugs.python.org/issue36274 and
    https://bugs.python.org/issue38216

    This was previously done just for py3 compatibility, but following
    https://github.com/python/cpython/commit/bb8071a our stable gates are
    all broken -- apparently, they're running a 2.7 pre-release?

    (cherry picked from commit c0ae48ba9aafb0b91869ea3bae8da07a32088777)
    (cherry picked from commit 2b4d58952cae8b174fb60529d5284c1d328e9287)

    ---

    bufferedhttp: ensure query params are properly quoted

    Recent versions of py27 [1] have begun raising InvalidURL if you try to
    include non-ASCII characters in the request path. This was observed
    recently in the periodic checks of stable/ocata and stable/pike. In
    particular, we would spin up some in-process servers in
    test.unit.proxy.test_server.TestSocketObjectVersions and do a container
    listing with a prefix param that included raw (unquoted) UTF-8. This
    query string would pass unmolested through the proxy, tripping the
    InvalidURL error when bufferedhttp called putrequest.

    More recent versions of Swift would not exhibit this particular failure,
    as the listing_formats middleware would force a decoding/re-encoding of
    the query string for account and container requests. However, object
    requests with errant query strings would likely be able to trip the same
    error.

    Swift on py3 should not exhibit this behavior, as we so
    thoroughly re-write the request line to avoid hitting
    https://bugs.python.org/issue33973.

    Now, always parse and re-encode the query string in bufferedhttp. This
    prevents any errors on object requests and cleans up any callers that
    might use bufferedhttp directly.

    [1] Anything after https://github.com/python/cpython/commit/bb8071a;
        see https://bugs.python.org/issue30458

    Depends-On: https://review.opendev.org/684769
    Closes-Bug: 1843816
    Related-Change: Id3ce37aa0402e2d8dd5784ce329d7cb4fbaf700d
    Related-Change: Ie648f5c04d4415f3b620fb196fa567ce7575d522
    (cherry picked from commit 49f62f6ab7fd1b833e9b5bfbcaafa4b45b592d34)
    (cherry picked from commit 9cc6d4138946034516fdf579ac084cb954ea6b06)

    ---

    Change-Id: I4eafc5f057df8a3c15560ace255d05602db56ef6

tags: added: in-stable-pike
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/swift 2.23.0

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/losf)

Fix proposed to branch: feature/losf
Review: https://review.opendev.org/686864

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (feature/losf)
Download full text (26.4 KiB)

Reviewed: https://review.opendev.org/686864
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=bfa8e9feb51f2b10adfec3a741661a76fcf73216
Submitter: Zuul
Branch: feature/losf

commit cb76e00e90aea834c8f3dd8a6ca5131acd43663b
Author: OpenStack Proposal Bot <email address hidden>
Date: Fri Oct 4 07:05:07 2019 +0000

    Imported Translations from Zanata

    For more information about this automatic import see:
    https://docs.openstack.org/i18n/latest/reviewing-translation-import.html

    Change-Id: I40ce1d36f1c207a0d3e99a3a84a162b21b3c57cf

commit 527a57ffcdefc03a5080b07d63f0ded319e08dfe
Author: OpenStack Release Bot <email address hidden>
Date: Thu Oct 3 16:35:36 2019 +0000

    Update master for stable/train

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

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

    Change-Id: Ia93e0b690f47c6231423a25dfd6a108a60378a21
    Sem-Ver: feature

commit 8a4becb12fbe3d4988ddee73536673d6f55682dd
Author: Tim Burke <email address hidden>
Date: Fri Sep 27 15:18:59 2019 -0700

    Authors/changelog for 2.23.0

    Also, make some CHANGELOG formatting more consistent.

    Change-Id: I380ee50e075a8676590e755f24a3fd7a7a331029

commit bf9346d88de2aeb06da3b2cde62ffa6200936367
Author: Tim Burke <email address hidden>
Date: Thu Aug 15 14:33:06 2019 -0700

    Fix some request-smuggling vectors on py3

    A Python 3 bug causes us to abort header parsing in some cases. We
    mostly worked around that in the related change, but that was *after*
    eventlet used the parsed headers to determine things like message
    framing. As a result, a client sending a malformed request (for example,
    sending both Content-Length *and* Transfer-Encoding: chunked headers)
    might have that request parsed properly and authorized by a proxy-server
    running Python 2, but the proxy-to-backend request could get misparsed
    if the backend is running Python 3. As a result, the single client
    request could be interpretted as multiple requests by an object server,
    only the first of which was properly authorized at the proxy.

    Now, after we find and parse additional headers that weren't parsed by
    Python, fix up eventlet's wsgi.input to reflect the message framing we
    expect given the complete set of headers. As an added precaution, if the
    client included Transfer-Encoding: chunked *and* a Content-Length,
    ensure that the Content-Length is not forwarded to the backend.

    Change-Id: I70c125df70b2a703de44662adc66f740cc79c7a9
    Related-Change: I0f03c211f35a9a49e047a5718a9907b515ca88d7
    Closes-Bug: 1840507

commit 0217b12b6d7d6f3727a54db65614ff1ef52d6286
Author: Matthew Oliver <email address hidden>
Date: Wed Sep 4 14:30:33 2019 +1000

    PDF Documentation Build tox target

    This patch adds a `pdf-docs` tox target that will build
    PDF versions of our docs. As per the Train community goal:

      https://governance.openstack.org/tc/goals/selected/train/pdf-doc-...

tags: added: in-feature-losf
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/swift 2.19.2

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/swift 2.21.1

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/swift ocata-eol

This issue was fixed in the openstack/swift ocata-eol release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/swift pike-eol

This issue was fixed in the openstack/swift pike-eol release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/swift queens-eol

This issue was fixed in the openstack/swift queens-eol release.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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