OpenStack Object Storage (swift)

Invalid X-Copy-From value and Account Quotas results in 500

Bug #1254626 reported by Daisuke Morita on 2013-11-25

This bug affects 1 person

Affects		Status	Importance	Assigned to	Milestone
	OpenStack Object Storage (swift)	Fix Released	Undecided	Fabien Boucher	OpenStack Object Storage (swift) 1.13.1 "icehouse"

Bug Description

When account-quotas is set in pipeline, a PUT request with invalid X-Copy-From value may result in 500 Internal Server Error.

When requesting with a header like "X-Copy-From: string_with_no_slash", this value is not verified in account_quotas.py [1] as is in proxy/controllers/obj.py [2].

[1] https://github.com/openstack/swift/blob/6342cb387a8538a8b59e4a4ba9e2574ac23ff45f/swift/common/middleware/account_quotas.py#L104
[2] https://github.com/openstack/swift/blob/f5648638ee6f939556ebfcb40dfdb8a590d3b5ae/swift/proxy/controllers/obj.py#L1000

Fabien Boucher (fabien-boucher) on 2013-11-25

Changed in swift:
assignee:	nobody → Fabien Boucher (fabien-boucher)

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2013-11-25: Fix proposed to swift (master)

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

Changed in swift:
status:	New → In Progress

Revision history for this message

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

Reviewed: https://review.openstack.org/58397
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=7188275ac24ecf26951ba5ab248934890becbc21
Submitter: Jenkins
Branch: master

commit 7188275ac24ecf26951ba5ab248934890becbc21
Author: Fabien Boucher <email address hidden>
Date: Mon Nov 25 23:26:09 2013 +0100

Fix account quota mw for handling a bad source path

    The copy source must be container/object.
    This patch avoids the server to return
    an internal server error when user provides
    a path without a container.

Change-Id: I8ea4e62b2a00c4b4a1666d30411a6c93a4f848a5
Fixes: bug #1254626

Changed in swift:
status:	In Progress → Fix Committed

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2014-03-18: Fix proposed to swift (feature/ec)

Fix proposed to branch: feature/ec
Review: https://review.openstack.org/81345

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2014-03-20: Fix merged to swift (feature/ec)

Download full text (8.4 KiB)

Reviewed: https://review.openstack.org/81345
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=78db7c23ea6afc7c7b50f327102cd8a9bb782066
Submitter: Jenkins
Branch: feature/ec

commit 0efac0cac29db96b7903a83d23551dd05d421cfb
Author: Clay Gerrard <email address hidden>
Date: Thu Mar 6 01:19:36 2014 -0800

make probetests work with conf.d configs

Change-Id: I451ff4629730a334ac1bd8fc6cd75de95314e153

commit 7463a46a6262eb95572f90d1bf93abc461576700
Author: Clay Gerrard <email address hidden>
Date: Tue Mar 11 14:18:55 2014 -0700

DRY out common.constraints a bit more

Change-Id: If8a71692b79e381036b34245e09697ac1009beca

commit 2abb829a5dd965b99bc8f18f13c3bfbe1ece27b2
Author: David Goetz <email address hidden>
Date: Wed Jan 15 14:49:31 2014 -0800

Make cors work better.

    CORS doesn't really work with swift right now. OPTIONS calls for the most part
    work but for so called "simple cross-site requests" (i.e. those that don't
    require a pre-flight OPTIONS request) Swift always returns the Origin it was
    given as the Access-Control-Allow-Origin in the response. This makes CORS
    "work" for these requests but if you actually wanted the javascript user agent
    to restrict anything for you it wouldn't be able to!

You can duplicate the issue with updated CORS test page:

http://docs.openstack.org/developer/swift/cors.html#test-cors-page

And a public container with an 'X-Container-Meta-Access-Control-Allow-Origin'
that does NOT match the webserver hosting the test-cors-page.

e.g.

with a public container that accepts cross-site requests from "example.com":

`swift post cors-container -m access-control-allow-origin:example.com -r .r:*`

You could point your browser at a copy of the test-cors-page on your
filesystem (the browser will will send 'Origin: null')

    Without a token the XMLHttpRequest will not request any custom headers (i.e.
    Access-Control-Request-Headers: x-auth-token) and the request will be made
    with-out a preflight OPTIONS request (which Swift would have denied anyway
    because the origin's don't match)

i.e. fill in "http://saio:8080/v1/AUTH_test/cors-container" for "URL" and
leave "Token" blank.

    You would expect that the browser would not complete the request because
    "Origin: null" does not match the configured "Access-Control-Allow-Origin:
    example.com" on the container metadata, and indeed with this patch - it won't!

Also:

    The way cors is set up does not play well with certain applications for swift.
    If you are running a CDN on top of swift and you have the
    Access-Control-Allow-Origin cors header set to * then you probably want the *
    to be cached on the the CDN, not the Origin that happened to result in an
    origin request.

Also:

    If you were unfortunate enough to allow cors headers to be saved directly
    onto objects then this allows them to supersede the headers coming from the
    container.

NOTE: There is a change is behavior with this patch. Because its cors, a
spec that was cre...

Reviewed:  https://review.openstack.org/81345
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=78db7c23ea6afc7c7b50f327102cd8a9bb782066
Submitter: Jenkins
Branch:    feature/ec

commit 0efac0cac29db96b7903a83d23551dd05d421cfb
Author: Clay Gerrard <clay.gerrard@gmail.com>
Date:   Thu Mar 6 01:19:36 2014 -0800

make probetests work with conf.d configs
    
    Change-Id: I451ff4629730a334ac1bd8fc6cd75de95314e153

commit 7463a46a6262eb95572f90d1bf93abc461576700
Author: Clay Gerrard <clay.gerrard@gmail.com>
Date:   Tue Mar 11 14:18:55 2014 -0700

DRY out common.constraints a bit more
    
    Change-Id: If8a71692b79e381036b34245e09697ac1009beca

commit 2abb829a5dd965b99bc8f18f13c3bfbe1ece27b2
Author: David Goetz <dpgoetz@gmail.com>
Date:   Wed Jan 15 14:49:31 2014 -0800

Make cors work better.
    
    CORS doesn't really work with swift right now. OPTIONS calls for the most part
    work but for so called "simple cross-site requests" (i.e. those that don't
    require a pre-flight OPTIONS request) Swift always returns the Origin it was
    given as the Access-Control-Allow-Origin in the response. This makes CORS
    "work" for these requests but if you actually wanted the javascript user agent
    to restrict anything for you it wouldn't be able to!
    
    You can duplicate the issue with updated CORS test page:
    
    http://docs.openstack.org/developer/swift/cors.html#test-cors-page
    
    And a public container with an 'X-Container-Meta-Access-Control-Allow-Origin'
    that does NOT match the webserver hosting the test-cors-page.
    
    e.g.
    
    with a public container that accepts cross-site requests from "example.com":
    
    `swift post cors-container -m access-control-allow-origin:example.com -r .r:*`
    
    You could point your browser at a copy of the test-cors-page on your
    filesystem (the browser will will send 'Origin: null')
    
    Without a token the XMLHttpRequest will not request any custom headers (i.e.
    Access-Control-Request-Headers: x-auth-token) and the request will be made
    with-out a preflight OPTIONS request (which Swift would have denied anyway
    because the origin's don't match)
    
    i.e. fill in "http://saio:8080/v1/AUTH_test/cors-container" for "URL" and
    leave "Token" blank.
    
    You would expect that the browser would not complete the request because
    "Origin: null" does not match the configured "Access-Control-Allow-Origin:
    example.com" on the container metadata, and indeed with this patch - it won't!
    
    Also:
    
    The way cors is set up does not play well with certain applications for swift.
    If you are running a CDN on top of swift and you have the
    Access-Control-Allow-Origin cors header set to * then you probably want the *
    to be cached on the the CDN, not the Origin that happened to result in an
    origin request.
    
    Also:
    
    If you were unfortunate enough to allow cors headers to be saved directly
    onto objects then this allows them to supersede the headers coming from the
    container.
    
    NOTE: There is a change is behavior with this patch. Because its cors, a
          spec that was created only to cause annoyance to all, I'll write out
          what's being changed and hopefully someone will speak up if it breaks
          there stuff.
    
    previous behavior: When a request was made with a Origin header set the
                       cors_validation decorator would always add that origin as
                       the Access-Control-Allow-Origin header in the response-
                       whether the passed origin was a match with the container's
                       X-Container-Meta-Access-Control-Allow-Origin or not, or even
                       if the container did not have CORS set up at all.
    new behavior: If strict_cors_mode is set to True in the proxy-server.conf
                  (which is the default) the cors_validation decorator will only
                  add the Access-Control-Allow-Origin header to the response when
                  the request's Origin matches the value set in
                  X-Container-Meta-Access-Control-Allow-Origin. NOTE- if the
                  container does not have CORS set up it won't just magically start
                  working. Furthremore, if the Origin doesn't match the
                  Access-Control-Allow-Origin - a successfully authorized request
                  (either by token or public ACL) won't be *denied* - it just
                  won't include the Access-Control-Allow-Origin header (it's up
                  to the security model in the browser to cancel the request
                  if the response doesn't include a matching Allow-Origin
                  header).  On the other hand, if you want to restrict requests
                  with CORS, you can actually do it now.
    
                  If you are worried about breaking current functionality you
                  must set:
    
                  strict_cors_mode = False
    
                  in the proxy-server.conf. This will continue with returning the
                  passed in Origin as the Access-Control-Allow-Origin in the
                  response.
    
    previous: If you had X-Container-Meta-Access-Control-Allow-Origin set to *
              and you passed in Origin: http://hey.com you'd get
              Access-Control-Allow-Origin: http://hey.com back. This was true for
              both OPTIONS and regular reqs.
    new: With X-Container-Meta-Access-Control-Allow-Origin set to * you get * back
         for both OPTIONS and regular reqs.
    
    previous: cors headers saved directly onto objects (by allowing them to be
              saved via the allowed_headers config in the object-server conf)
              would be overridden by whatever container cors you have set up.
    new: For regular (non-OPTIONS) calls the object headers will be kept. The
         container cors will only be applied to objects without the
         'Access-Control-Allow-Origin' and 'Access-Control-Expose-Headers' headers.
         This behavior doesn't make a whole lot of sense for OPTIONS calls so I
         left that as is. I don't think that allowing cors headers to be saved
         directly onto objects is a good idea and it should be discouraged.
    
    DocImpact
    
    Change-Id: I9b0219407e77c77a9bb1133cbcb179a4c681c4a8

commit e7fb089f59c6e17f29fc93bffdf16a878fd78b06
Author: Samuel Merritt <sam@swiftstack.com>
Date:   Tue Mar 11 11:25:04 2014 -0700

Fix ring-builder crash
    
    If you had a partition with 1 replica on a deleted device, 1 on a
    zero-weight device, and 1 on a normal device, then rebalancing would
    crash. This was because the ring builder was memoizing tiers_for_dev()
    for all *available* devices (i.e. weight > 0 and not deleted), but
    depended on it being present for all devices with partitions still on
    them.
    
    Since the builder moved the replica from the deleted device, it left
    the one on the zero-weight device alone, so you had an unavailable
    device with a partition replica still on it, triggering the crash.
    
    Now we go ahead and memoize tiers_for_dev() for *all* devices, not
    just available ones, thereby fixing the crash.
    
    Change-Id: Ie0b58b65e0353732cf785ab772e95e699f3a5b5d

commit fc499f3092c616deb6ec5dc089d5a05903538d5c
Author: madhuri <madhuri@madhuri-VirtualBox.(none)>
Date:   Thu Mar 6 12:31:38 2014 +0530

Added a test for empty metadata
    
    Here added a test for setting empty object metadata
    and checking its value in response headers.
    
    Change-Id: I460302661d150364a95bcd7f0ebbbc2a1e95507a

commit fa4d105e44180877dda9185bf5246b63634fcaf6
Author: anc <alistair.coles@hp.com>
Date:   Wed Mar 5 18:25:39 2014 +0000

Fix ineffective expect header test
    
    The test for expect header being sent
    with non-zero object PUTs doesn't seem
    to check its results, so passes whether
    the expect header is set or not.
    
    Change-Id: Ic74233c19e48aba43e63501467565e305ed399c3

commit 7188275ac24ecf26951ba5ab248934890becbc21
Author: Fabien Boucher <fabien.boucher@enovance.com>
Date:   Mon Nov 25 23:26:09 2013 +0100

Fix account quota mw for handling a bad source path
    
    The copy source must be container/object.
    This patch avoids the server to return
    an internal server error when user provides
    a path without a container.
    
    Change-Id: I8ea4e62b2a00c4b4a1666d30411a6c93a4f848a5
    Fixes: bug #1254626

Thierry Carrez (ttx) on 2014-04-04

Changed in swift:
milestone:	none → 1.13.1-rc1
status:	Fix Committed → Fix Released

Thierry Carrez (ttx) on 2014-04-17

Changed in swift:
milestone:	1.13.1-rc1 → 1.13.1

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.