*LO subrequests don't pass on the referer or req.acl on

Bug #1526575 reported by Matthew Oliver
24
This bug affects 4 people
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Medium
Matthew Oliver

Bug Description

When a *LO object is requested, the subrequests sent to get the segments doesn't contain the req.acl or HTTP_REFERER details. This is a problem when you put a READ ACL on a container containing an *LO object, when read a 403 will be raised unless you make the segment container public readable.

If we pass the referer on to the subrequests we can use the same ACL's on the segment and *LO container.

  $ curl -i -H "X-Auth-Token: $TOKEN" $STORAGE_URL/c -H "X-Container-Read: .r:*.exapmle.com,.rlistings" -XPOST

  $ curl -i -H "X-Auth-Token: $TOKEN" $STORAGE_URL/segs -H "X-Container-Read: .r:*.exapmle.com,.rlistings" -XPOST
  HTTP/1.1 204 No Content
  Content-Length: 0
  Content-Type: text/html; charset=UTF-8
  X-Trans-Id: tx353b8ed48c2a4b2294074-005670a588
  Date: Tue, 15 Dec 2015 23:43:04 GMT

  $ curl -i -H "X-Auth-Token: $USER_TOKEN" $STORAGE_URL/c/dlo -e "http://blah.example.com"
  HTTP/1.1 403 Forbidden
  Content-Length: 73
  Content-Type: text/html; charset=UTF-8
  X-Trans-Id: tx519503979f4b4fd594f70-005670a58c
  Date: Tue, 15 Dec 2015 23:43:08 GMT

  <html><h1>Forbidden</h1><p>Access was denied to this resource.</p></html>

This is happening on the current master.

CVE References

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

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

Changed in swift:
status: New → In Progress
Revision history for this message
Kota Tsuyuzaki (tsuyuzaki-kota) wrote :

I got a 401 Unauthorized for dlo with this scenario but this seems true that the GET dlo blocked even though both segment container and dlo manifest are readable since their container acls.

Revision history for this message
Kota Tsuyuzaki (tsuyuzaki-kota) wrote :

Ah, same account but another user may get 403 for this...hmm...

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

Reviewed: https://review.openstack.org/258280
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=87f7e907ee412f5847f1f9ffca7a566fb148c6b1
Submitter: Jenkins
Branch: master

commit 87f7e907ee412f5847f1f9ffca7a566fb148c6b1
Author: Matthew Oliver <email address hidden>
Date: Wed Dec 16 17:19:24 2015 +1100

    Pass HTTP_REFERER down to subrequests

    Currently a HTTP_REFERER (Referer) header isn't passed down to
    subrequests. This means *LO subrequests to segment containers
    return a 403 on a *LO GET when accessed by requests using referer
    ACLs.
    Currently the only way around referer access to *LO's is to make the
    segments container world readable.

    This change makes sure the referer header is passed into subrequests
    allowing a segments container to only need to be locked down with
    the same referer as the *LO container.

    This is a 1 line change to code, but also adds a unit and 2 functional
    functional tests (one for DLO and one for SLO).

    Change-Id: I1fa5328979302d9c8133aa739787c8dae6084f54
    Closes-Bug: #1526575

Changed in swift:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/hummingbird)

Fix proposed to branch: feature/hummingbird
Review: https://review.openstack.org/264517

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

Reviewed: https://review.openstack.org/264517
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=93ddaffaebb620bc712d52d0267194562dce02be
Submitter: Jenkins
Branch: feature/hummingbird

commit f53cf1043d078451c4b9957027bf3af378aa0166
Author: Ondřej Nový <email address hidden>
Date: Tue Jan 5 20:20:15 2016 +0100

    Fixed few misspellings in comments

    Change-Id: I8479c85cb8821c48b5da197cac37c80e5c1c7f05

commit 79222e327f9df6335b58e17a6c8dd0dc44b86c17
Author: ChangBo Guo(gcb) <email address hidden>
Date: Sat Dec 26 13:13:37 2015 +0800

    Fix AttributeError for LogAdapter

    LogAdapter object has no attribute 'warn' but has attribute
    'warning'.

    Closes-Bug: #1529321
    Change-Id: I0e0bd0a3dbc4bb5c1f0b343a8809e53491a1da5f

commit 684c4c04592278a280032002b5313b171ee7a4c0
Author: janonymous <email address hidden>
Date: Sun Aug 2 22:47:42 2015 +0530

    Python 3 deprecated the logger.warn method in favor of warning

    DeprecationWarning: The 'warn' method is deprecated, use 'warning'
    instead

    Change-Id: I35df44374c4521b1f06be7a96c0b873e8c3674d8

commit d0a026fcb8e8a9f5475699cc56e1998bdc4cd5ca
Author: Hisashi Osanai <email address hidden>
Date: Wed Dec 16 18:50:37 2015 +0900

    Fix duplication for headers in Access-Control-Expose-Headers

    There are following problems with Access-Control-Expose-Headers.

    * If headers in X-Container-Meta-Access-Control-Expose-Headers are
      configured, the headers are kept with case-sensitive string.
      Then a CORS request comes, the headers are merged into
      Access-Control-Expose-Headers as case-sensitive string even if
      there is a same header which is not case-sensitive string.

    * Access-Control-Expose-Headers is handled by a list.
      If X-Container/Object-Meta-XXX is configured in container/object
      and X-Container-Meta-Access-Control-Expose-Headers, same header
      is listed in Access-Control-Expose-Headers.

    This patch provides a fix for the problems.

    Change-Id: Ifc1c14eb3833ec6a851631cfc23008648463bd81

commit 0bcd7fd50ec0763dcb366dbf43a9696ca3806f15
Author: Bill Huber <email address hidden>
Date: Fri Nov 20 12:09:26 2015 -0600

    Update Erasure Coding Overview doc to remove Beta version

    The major functionality of EC has been released for Liberty and
    the beta version of the code has been removed since it is now
    in production.

    Change-Id: If60712045fb1af803093d6753fcd60434e637772

commit 84ba24a75640be4212e0f984c284faf4c894e7c6
Author: Alistair Coles <email address hidden>
Date: Fri Dec 18 11:24:34 2015 +0000

    Fix rst errors so that html docs are complete

    rst table format errors don't break the gate job
    but do cause sections of the documents to go missing
    from the html output.

    Change-Id: Ic8c9953c93d03dcdafd8f47b271d276c7b356dc3

commit 87f7e907ee412f5847f1f9ffca7a566fb148c6b1
Author: Matthew Oliver <email address hidden>
Date: Wed Dec 16 17:19:24 2015 +1100

    Pass HTTP_REFERER down to subrequests

    Currently a HTTP_REFERER (Referer) header isn't passed down to
    subreq...

tags: added: in-feature-hummingbird
Revision history for this message
Doug Hellmann (doug-hellmann) wrote : Fix included in openstack/swift 2.6.0

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

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

Fix proposed to branch: feature/crypto
Review: https://review.openstack.org/272201

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

Reviewed: https://review.openstack.org/272201
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=f9b7fd3074b5b0e5d6ea879d4144f7bfeec5d46b
Submitter: Jenkins
Branch: feature/crypto

commit e13a03c379273ee10e678818078b9c40a96a7dc9
Author: Tim Burke <email address hidden>
Date: Wed Jan 20 16:06:26 2016 -0800

    Stop overriding builtin range

    Change-Id: I315f8b554bb9e96659b455f4158f074961bd6498

commit 0a404def7d54d1ef1c85c11a378052260c4fda4c
Author: John Dickinson <email address hidden>
Date: Wed Jan 20 15:19:35 2016 -0800

    remove unneeded duplicate dict keys

    Change-Id: I926d7aaa9df093418aaae54fe26e8f7bc8210645

commit 221f94fdd39fd2dcd9a2e5565adceab615d55913
Author: John Dickinson <email address hidden>
Date: Tue Jan 19 14:50:24 2016 -0800

    authors and changelog updates for 2.6.0

    Change-Id: Idd0ff9e70abc0773be183c37cd6125fe852da7c0

commit 58359269b0e971e52f0eb7f97221566ca2148014
Author: Samuel Merritt <email address hidden>
Date: Tue Dec 8 16:36:05 2015 -0800

    Fix memory/socket leak in proxy on truncated SLO/DLO GET

    When a client disconnected while consuming an SLO or DLO GET response,
    the proxy would leak a socket. This could be observed via strace as a
    socket that had shutdown() called on it, but was never closed. It
    could also be observed by counting entries in /proc/<pid>/fd, where
    <pid> is the pid of a proxy server worker process.

    This is due to a memory leak in SegmentedIterable. A SegmentedIterable
    has an 'app_iter' attribute, which is a generator. That generator
    references 'self' (the SegmentedIterable object). This creates a
    cyclic reference: the generator refers to the SegmentedIterable, and
    the SegmentedIterable refers to the generator.

    Python can normally handle cyclic garbage; reference counting won't
    reclaim it, but the garbage collector will. However, objects with
    finalizers will stop the garbage collector from collecting them* and
    the cycle of which they are part.

    For most objects, "has finalizer" is synonymous with "has a __del__
    method". However, a generator has a finalizer once it's started
    running and before it finishes: basically, while it has stack frames
    associated with it**.

    When a client disconnects mid-stream, we get a memory leak. We have
    our SegmentedIterable object (call it "si"), and its associated
    generator. si.app_iter is the generator, and the generator closes over
    si, so we have a cycle; and the generator has started but not yet
    finished, so the generator needs finalization; hence, the garbage
    collector won't ever clean it up.

    The socket leak comes in because the generator *also* refers to the
    request's WSGI environment, which contains wsgi.input, which
    ultimately refers to a _socket object from the standard
    library. Python's _socket objects only close their underlying file
    descriptor when their reference counts fall to 0***.

    This commit makes SegmentedIterable.close() call
    self.app_iter.close(), thereby unwinding its generator's stack and
    making it eligible for garbage collection.

    * in Python < 3...

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

Fix proposed to branch: feature/crypto
Review: https://review.openstack.org/277950

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

Change abandoned by Alistair Coles (<email address hidden>) on branch: feature/crypto
Review: https://review.openstack.org/277950

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

Duplicates of this bug

Other bug subscribers

Remote bug watches

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