OpenStack Object Storage (swift)

A user is able to remove his/her own Account Quota

Bug #1204110 reported by Joe Hakim Rahme on 2013-07-23

This bug affects 1 person

Affects		Status	Importance	Assigned to	Milestone
	OpenStack Object Storage (swift)	Fix Released	Medium	Jon Snitow Solera	OpenStack Object Storage (swift) 1.9.1

Bug Description

On a standard devstack account, I want to put an account quota on the user demo (tenant_id = c5ed34278aea420abf23896e805f05fc)

$admin_token and $demo_token are retrieved in advance.

# I use the ResellerAdmin account to set a Bytes Quota on the account
$ curl -i -X POST -H "X-Auth-Token: $admin_token" -H "X-Account-Meta-Quota-Bytes: 20" http://192.168.33.10:8080/v1/AUTH_c5ed34278aea420abf23896e805f05fc
HTTP/1.1 204 No Content
Content-Length: 0
Content-Type: text/html; charset=UTF-8
X-Trans-Id: txdfb060b872614d7598553-0051ee84ef
Date: Tue, 23 Jul 2013 13:28:15 GMT

# I check that the quota has been set correctly
$ source demorc # get the demo credentials
$ swift stat
   Account: AUTH_c5ed34278aea420abf23896e805f05fc
Containers: 0
   Objects: 0
     Bytes: 0
Meta Quota-Bytes: 20
X-Timestamp: 1374586095.80445
X-Trans-Id: tx95f304c793a44d3197811-0051ee84f7
Content-Type: text/plain; charset=utf-8
Accept-Ranges: bytes

# Using the a token of the demo (non-priviledged) account, I can remove the quota metadata on the demo account
$ curl -i -X POST -H "X-Auth-Token: $demo_token" -H "X-Remove-Account-Meta-Quota-Bytes: 20" http://192.168.33.10:8080/v1/AUTH_c5ed34278aea420abf23896e805f05fc
HTTP/1.1 204 No Content
Content-Length: 0
Content-Type: text/html; charset=UTF-8
X-Trans-Id: tx08ba4bea795b4f0a93596-0051ee853a
Date: Tue, 23 Jul 2013 13:29:30 GMT

# I check, the quota has been removed.
$ swift stat
   Account: AUTH_c5ed34278aea420abf23896e805f05fc
Containers: 0
   Objects: 0
     Bytes: 0
Accept-Ranges: bytes
X-Timestamp: 1374586095.80445
X-Trans-Id: txf66ff7b4a61840a4a30d6-0051ee8547
Content-Type: text/plain; charset=utf-8

Revision history for this message

Fabien Boucher (fabien-boucher) wrote on 2013-07-23:

Hi Joe,

It's seem there are two way to remove a metadata in swift by using X-Remove-* stuff or by using X-Account-Meta- stuff with an empty value. The reason there two way seems to be due to cURL.
Have a look here :
http://docs.openstack.org/api/openstack-object-storage/1.0/content/delete-account-metadata.html

Please can you try to remove that meta with the common way (without X-Remove) and validate you cannot remove the meta as
regular user.

Beside of that, after looking at the middleware code, it seems the middleware does not handle X-Remove but it should.

Samuel Merritt (torgomatic) on 2013-07-23

Changed in swift:
status:	New → Confirmed

Samuel Merritt (torgomatic) on 2013-07-23

Changed in swift:
assignee:	nobody → Samuel Merritt (torgomatic)

Samuel Merritt (torgomatic) on 2013-07-23

Changed in swift:
importance:	Undecided → Medium

Revision history for this message

Joe Hakim Rahme (rahmu) wrote on 2013-07-24:

Hi Fabien,

When I try to remove the metadata the "common" way, here's what happens:

$ curl -i -X POST -H "X-Auth-Token: $demo_token" -H "X-Account-Meta-Quota-Bytes: " http://192.168.33.10:8080/v1/AUTH_64b8fce216c749d8b167d1641ea46e1c
HTTP/1.1 204 No Content
Content-Length: 0
Content-Type: text/html; charset=UTF-8
X-Trans-Id: txc8d9ddfb9ee44ac7af367-0051ef978a
Date: Wed, 24 Jul 2013 08:59:54 GMT

$ swift stat
   Account: AUTH_64b8fce216c749d8b167d1641ea46e1c
Containers: 0
   Objects: 0
     Bytes: 0
Meta Quota-Bytes: 40
X-Timestamp: 1374656119.71895
X-Trans-Id: tx96272785067b4929acd88-0051ef9790
Content-Type: text/plain; charset=utf-8
Accept-Ranges: bytes

As you can see, it doesn't remove the metadata. However, it returns a 204 success code. Shouldn't return an error code like 403?

Revision history for this message

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

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

Changed in swift:
assignee:	Samuel Merritt (torgomatic) → Jon Snitow (otherjon)
status:	Confirmed → In Progress

Revision history for this message

Jon Snitow Solera (otherjon) wrote on 2013-07-24:

Hi Joe,

cURL can't send empty headers without hackery. (See http://curl.haxx.se/mail/lib-2010-08/0171.html for more info.) If you run the above command with the "-v" flag, you'll see that it isn't actually sending what you want it to.

If you really want to test this with cURL, try something like the following:

curl -v -i -X POST -H "$(printf "X-Account-Meta-Quota-Bytes:\r\nX-Auth-Token: ")$demo_token" http://192.168.33.10:8080/v1/AUTH_64b8fce216c749d8b167d1641ea46e1c

Revision history for this message

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

Reviewed: https://review.openstack.org/38563
Committed: http://github.com/openstack/swift/commit/eb0629fc8210cc8bb4275d70408dddab67f667f7
Submitter: Jenkins
Branch: master

commit eb0629fc8210cc8bb4275d70408dddab67f667f7
Author: Jon Snitow <email address hidden>
Date: Wed Jul 24 15:58:55 2013 -0700

Make sure users can't remove their account quotas

Protect X-Remove-Account-Meta-Quota-Bytes same as X-Account-Meta-Quota-Bytes

Fixes bug 1204110

Change-Id: Ibac5b555f50b1fe41b2999c0d5776d90f9c9f3d1

Changed in swift:
status:	In Progress → Fix Committed

Revision history for this message

Joe Hakim Rahme (rahmu) wrote on 2013-07-25:

Hey Jon,

Thanks for the tip about cURL, I did not know this.
Also, thanks for fixing this bug fast :)

Revision history for this message

Jon Snitow Solera (otherjon) wrote on 2013-07-25:

You're very welcome, Joe. Good luck!

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2013-08-01: Fix proposed to swift (feature/ec)

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

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2013-08-01: Fix merged to swift (feature/ec)

Download full text (12.7 KiB)

Reviewed: https://review.openstack.org/39740
Committed: http://github.com/openstack/swift/commit/7a404cdea400e12d1ee8660c9f9477a87e419602
Submitter: Jenkins
Branch: feature/ec

commit 3035a93ed2c467fb248a77389274033ff540590c
Author: David Goetz <email address hidden>
Date: Tue Jul 30 11:52:23 2013 -0700

Tell swift to figure out content type

    Be able to tell swift to figure out the content-type even if it is
    sent because old client code / curl has trouble sending blank
    content-type headers.

Change-Id: Ie65ddf8993a19ea74e0b85a2ae56da84a617c19d

commit 47e34cf15eb652bd54d53b0cfb5238d04e4416d1
Author: TheSriram <email address hidden>
Date: Tue Jul 30 14:57:48 2013 -0400

fix(gitignore) : ignore *.egg and *.egg-info

Change-Id: I9496dab9e8f779a7ae045bb73b2cf8e77bda0d30

commit 22e7cbceed4730f93ee601a9955215e208e64295
Author: Alex Gaynor <email address hidden>
Date: Mon Jul 29 22:41:29 2013 -0700

When iterating over a range of a file, always close it

This is needed on Pythons without reference counting garbage collectors (e.g.
PyPy).

Change-Id: I1d06eb8fe08ee6eeb45caa47b653d6af0bb18267

commit 1e3ad44784bed7cbd8a8f367d633cc1d7c1d4172
Author: Peter Portante <email address hidden>
Date: Mon Jul 29 15:49:37 2013 -0400

Merge object base module into diskfile.

    All of the module methods of the (now defunct) base module we really
    concerned with the on-disk layout which is what the DiskFile module is
    really about.

Change-Id: I96e022c5f96e31537ced74139185851a2751701c
Signed-off-by: Peter Portante <email address hidden>

commit eb99e8f84cf78c20e03f4a4fd92e243a22171b23
Author: Donagh McCabe <email address hidden>
Date: Tue Jul 23 15:10:09 2013 +0100

Obscure the X-Auth-Token in proxy log

The X-Auth-Token is sensitive data. If revealed to an unauthozied person,
they can now make requests against an account until the token expires.

    This implementation maintains current behavior (i.e, the token
    is logged). Implementers can choose to set reveal_sensitive_prefix
    to (e.g.) 12 so only first 12 characters of the token are logged.
    Or, set to 0 to replace the token with "...".

DocImpact

Part of bug #1004114

Change-Id: Iecefa843d8f9ef59b9dcf0860e7a4d0e186a6cb5

commit f9e73a0fe6da7da94955c40ffb50ddc6526c1466
Author: Pete Zaitcev <email address hidden>
Date: Mon Jul 29 19:12:10 2013 -0600

Eliminate can_delete_db

    The method can_delete_db() appears to be not only unused, but has
    always been so. I verified this going back to Austin release. It is
    very strange that we never noticed it until now, but here it is.

Change-Id: I4445c4b2c4721f880c9dbb1eac055c0601ae6372

commit be688c31562a5789fba678a0675eff1040308202
Author: Alex Gaynor <email address hidden>
Date: Fri Jul 19 20:07:27 2013 -0700

Encode unicode from JSON before using it as a string.

    Right now this code fails when used with a JSON
    decoder that always produces unicode. This isn't
    usually the case with CPython, where simplejson
    is used most of the time, however w...

Reviewed:  https://review.openstack.org/39740
Committed: http://github.com/openstack/swift/commit/7a404cdea400e12d1ee8660c9f9477a87e419602
Submitter: Jenkins
Branch:    feature/ec

commit 3035a93ed2c467fb248a77389274033ff540590c
Author: David Goetz <dpgoetz@gmail.com>
Date:   Tue Jul 30 11:52:23 2013 -0700

Tell swift to figure out content type
    
    Be able to tell swift to figure out the content-type even if it is
    sent because old client code / curl has trouble sending blank
    content-type headers.
    
    Change-Id: Ie65ddf8993a19ea74e0b85a2ae56da84a617c19d

commit 47e34cf15eb652bd54d53b0cfb5238d04e4416d1
Author: TheSriram <sriram@klusterkloud.com>
Date:   Tue Jul 30 14:57:48 2013 -0400

fix(gitignore) : ignore *.egg and *.egg-info
    
    Change-Id: I9496dab9e8f779a7ae045bb73b2cf8e77bda0d30

commit 22e7cbceed4730f93ee601a9955215e208e64295
Author: Alex Gaynor <alex.gaynor@gmail.com>
Date:   Mon Jul 29 22:41:29 2013 -0700

When iterating over a range of a file, always close it
    
    This is needed on Pythons without reference counting garbage collectors (e.g.
    PyPy).
    
    Change-Id: I1d06eb8fe08ee6eeb45caa47b653d6af0bb18267

commit 1e3ad44784bed7cbd8a8f367d633cc1d7c1d4172
Author: Peter Portante <peter.portante@redhat.com>
Date:   Mon Jul 29 15:49:37 2013 -0400

Merge object base module into diskfile.
    
    All of the module methods of the (now defunct) base module we really
    concerned with the on-disk layout which is what the DiskFile module is
    really about.
    
    Change-Id: I96e022c5f96e31537ced74139185851a2751701c
    Signed-off-by: Peter Portante <peter.portante@redhat.com>

commit eb99e8f84cf78c20e03f4a4fd92e243a22171b23
Author: Donagh McCabe <donagh.mccabe@hp.com>
Date:   Tue Jul 23 15:10:09 2013 +0100

Obscure the X-Auth-Token in proxy log
    
    The X-Auth-Token is sensitive data. If revealed to an unauthozied person,
    they can now make requests against an account until the token expires.
    
    This implementation maintains current behavior (i.e, the token
    is logged). Implementers can choose to set reveal_sensitive_prefix
    to (e.g.) 12 so only first 12 characters of the token are logged.
    Or, set to 0 to replace the token with "...".
    
    DocImpact
    
    Part of bug #1004114
    
    Change-Id: Iecefa843d8f9ef59b9dcf0860e7a4d0e186a6cb5

commit f9e73a0fe6da7da94955c40ffb50ddc6526c1466
Author: Pete Zaitcev <zaitcev@kotori.zaitcev.us>
Date:   Mon Jul 29 19:12:10 2013 -0600

Eliminate can_delete_db
    
    The method can_delete_db() appears to be not only unused, but has
    always been so. I verified this going back to Austin release. It is
    very strange that we never noticed it until now, but here it is.
    
    Change-Id: I4445c4b2c4721f880c9dbb1eac055c0601ae6372

commit be688c31562a5789fba678a0675eff1040308202
Author: Alex Gaynor <alex.gaynor@gmail.com>
Date:   Fri Jul 19 20:07:27 2013 -0700

Encode unicode from JSON before using it as a string.
    
    Right now this code fails when used with a JSON
    decoder that always produces unicode. This isn't
    usually the case with CPython, where simplejson
    is used most of the time, however with the stdlib
    JSON library (as used on PyPy), this code fails.
    
    Change-Id: Ib2343243f40194d5b2784551a807c7f58970a6e9

commit 0f3b0410e3e8aa99971087ca5d8141e04cf6d39d
Author: Alex Gaynor <alex.gaynor@gmail.com>
Date:   Sat Jul 27 21:10:37 2013 -0700

Removed unnecessary monkeypatching of __builtin__
    
    Replaced it with explicitly importing the gettext function, which is
    significantly more readable.
    
    Change-Id: Ia0a7edcf685fb6e4052a8290367b233169529ab8

commit 750684f7543488a25bf68406bc7c5f6d9bc7fc7c
Author: David Goetz <dpgoetz@gmail.com>
Date:   Fri Jul 26 10:00:33 2013 -0700

add utf-8 charset to multipart-manifest=get resp
    
    Change-Id: Ic06e8b07a4db4ccde633f3b56a49f198cdd467cb

commit db2a4787b447b2907e8de7f604a500c31a2d1330
Author: Pete Zaitcev <zaitcev@kotori.zaitcev.us>
Date:   Thu Jul 25 18:18:46 2013 -0600

Unify _commit_puts for accounts and containers
    
    The _commit_puts method is exactly the same in AccountBroker and
    ContainerBroker, except for what it puts on the list pending records.
    It seems to be easy to factor out without victimizing the code
    clarity unduly.
    
    We also factor common checking for LockTimeout: not so much for
    any code savings, but to underscore that all these little segments
    really are the same, so a reader does not need to prove uniformity
    every time.
    
    Change-Id: Idf9b553a3cf28de3c8803aaa47cf9a2be6817f00

commit 934cd9d9a5e82a1ca5fc02085fcea0380d08f107
Author: Peter Portante <peter.portante@redhat.com>
Date:   Thu Jul 25 16:48:38 2013 -0400

Ignore coverage HTML directory and MANIFEST.
    
    Change-Id: I5286cd1b066da11eef98dbfea9252366ca3ffd9c
    Signed-off-by: Peter Portante <peter.portante@redhat.com>

commit 741c3166d10d15a5975499f60151be580b51cf2a
Author: Kun Huang <gareth@unitedstack.com>
Date:   Thu Jul 25 19:53:29 2013 +0800

fix name 'recon_container' to 'rcache'
    
    self.recon_container is called before assigned. Accounding the context
    codes, this is used to store somehing like self.rcache. So I guess the
    name of 'recon_container' is a mistake, and change it to 'rcache'
    (we could look at other places using dump_recon_cache)
    
    Also add unit tests
    
    fixes bug #1201958
    
    Change-Id: I3a6e3d22ba1dbffc4309bc22ff37873b4a3f09b3

commit 0508842470ab27e5635d858bf0f9c8c14e1b300e
Author: Kota Tsuyuzaki <tsuyuzaki.kota@lab.ntt.co.jp>
Date:   Tue Jul 23 19:28:08 2013 -0700

Fix incorrect status handling at staticweb
    
    Staticweb middleware always searchs html
    for error response when response status code
    is not redirect.
    It causes swift to return error response
    even if the request succeeded.
    (e.g. When finding index.html with 200,
          staticweb returns 200error.html
          if it exist)
    
    This patch modifies the constraint on response
    status and fix bug 1204319.
    
    Fixes bug #1204319.
    
    Change-Id: Ib83c303917da7fb94999f2d4d35063b450d0e992

commit df7fc9658b350cf38d8ecf4e49043fbe44e1d2d3
Author: Samuel Merritt <sam@swiftstack.com>
Date:   Wed Jul 24 12:55:25 2013 -0700

Catch swob responses that are raised.
    
    This lets us get rid of some really repetitive exception conversion
    code, like everybody that called common.utils.get_param() had to catch
    a UnicodeDecodeError and turn that into returning HTTPBadRequest. Now
    get_param() just raises HTTPBadRequest directly, and the __call__
    methods in the account/container/object servers catch and return
    it. All that "except UnicodeDecodeError" stuff goes away.
    
    Refactored the path splitting and device validation in the object
    server too.
    
    There are other things that can benefit from this as well, but this
    patch is big enough.
    
    Change-Id: I2be96ef757d04bfd6af180cd9c92393c841db21f

commit eb0629fc8210cc8bb4275d70408dddab67f667f7
Author: Jon Snitow <otherjon@swiftstack.com>
Date:   Wed Jul 24 15:58:55 2013 -0700

Make sure users can't remove their account quotas
    
    Protect X-Remove-Account-Meta-Quota-Bytes same as X-Account-Meta-Quota-Bytes
    
    Fixes bug 1204110
    
    Change-Id: Ibac5b555f50b1fe41b2999c0d5776d90f9c9f3d1

commit ff5a6d01115609866fc406fafeb7b4a49e507b7d
Author: Alex Gaynor <alex.gaynor@gmail.com>
Date:   Tue Jul 23 16:41:45 2013 -0700

Corrected many style violations in the tests.
    
    I focussed primarily on F-category violations, they are all but all fixed with
    this patch.
    
    Change-Id: I343f6945c97984ed1093bc347b6def6994297041

commit b7187cecee634e3fdc95777b7f7eb9e75126f685
Author: Donagh McCabe <donagh.mccabe@hp.com>
Date:   Mon Jul 22 18:15:51 2013 +0100

Return name of header larger than MAX_HEADER_SIZE
    
    Change-Id: I3130d8f8f0beebc8f92600f76b72cf64a3f12894

commit c7a79991f82a7e472c1767e5e56a96fde485d9ae
Author: Kun Huang <academicgareth@gmail.com>
Date:   Mon Jul 15 15:43:56 2013 +0800

log return value before arguments when rsync error
    
    The arguments of rsync command seems too long. If some errors occur, it
    is hard to debug. Moving return value of rsync command first could help
    people at least know reason of rsync error by manuals.
    
    fixes bug #903406
    
    Change-Id: I1cac7bbca74824a6d47e6ceb9f654d4046fcbb9e

commit d2dd3e54889266adb054add41c874a318c55ad19
Author: Marcelo Martins <marcelo.martins@rackspace.com>
Date:   Fri Jun 28 14:51:15 2013 -0500

Configuration options for error regex and log file in the config now
    
    Making it possible for one to overwrite the default set of regexes
    used to search for device block errors in the log file. Also making
    the log file naming pattern configurable by setting them in the
    drive-audit.conf file.
    
    Updating "Detecting Failed Drives" section on the admin guide as well.
    
    Change-Id: I7bd3acffed196da3e09db4c9dcbb48a20bdd1cf0

commit 1804b13ca3579b54475e5d06792e5d7e923efe2a
Author: Fabien Boucher <fabien.boucher@enovance.com>
Date:   Mon Jul 22 16:43:10 2013 +0200

Tempurl md use of split_path in _get_account
    
    Code refactoring in _get_account method by
    using split_path from common.utils.
    
    Change-Id: I7a04c7135b18f28673dd551a25bad679c89e2c31

commit 86cbcecb5991af7acaaa5371eec1277ae3be2ef4
Author: Kun Huang <gareth@unitedstack.com>
Date:   Sat Jul 20 08:40:42 2013 +0800

remove assert syntax
    
    assert is not a good tool for checking common mistakes, but could be
    used test conditions that `should never happen` (there're many talks
    about it)
    
    Change-Id: Iccf9710008293465bb9bb59e65f0ed77d7910688

commit 1f43ee050b94d368182d1d6e0340416ea1436e76
Author: Clay Gerrard <clay.gerrard@gmail.com>
Date:   Fri Jul 19 01:39:42 2013 -0700

Add option to make probetests more brittle
    
    Currently probetests take advantage of a number of assumptions about the SUT.
    Unfortunately after some time with a working SAIO, configuration drift may
    result in a system that is no longer compatible with these assumptions.  To
    help weary developers more quickly identify the changes they've made since
    they last ran probetests successfully, some handy validators have been added
    to test.probe.common
    
    Additionally a new option 'validate_rsync' in test.conf, when enabled, will
    run a series of up front validations during the setup of each probetest by
    inspecting the ring, the mounted devices, and the rsync exports ("modules") in
    order to ensure that when probetests fail the do so early and with specific
    complaints.
    
    To preserve existing failures, the option is disabled by default.
    
    Change-Id: I2be11c7e67ccd0bc0589c360c170049b6288c152

commit de3acec4bff777f20571bd6a545a79dc64007fab
Author: Clay Gerrard <clay.gerrard@gmail.com>
Date:   Thu Jul 11 17:00:57 2013 -0700

Set default wsgi workers to cpu_count
    
    Change the default value of wsgi workers from 1 to auto.  The new default
    value for workers in the proxy, container, account & object wsgi servers will
    spawn as many workers per process as you have cpu cores.
    
    This will not be ideal for some configurations, but it's much more likely to
    produce a successful out of the box deployment.
    
    Inspect the number of cpu_cores using python's multiprocessing when available.
    Multiprocessing was added in python 2.6, but I know I've compiled python
    without it before on accident.  The cpu_count method seems to be pretty system
    agnostic, but it says it can raise NotImplementedError or sometimes return 0.
    
    Add a new utility method 'config_auto_int_value' to pull an integer out of the
    config which has a dynamic default.
    
     * drive by s/container/proxy/ in proxy-server.conf.5
     * fix misplaced max_clients in *-server.conf-sample
     * update doc/development_saio to force workers = 1
    
    DocImpact
    
    Change-Id: Ifa563d22952c902ab8cbe1d339ba385413c54e95

commit 89ccd95996194bcfc5fb41c6102131457aa6fb9d
Author: Chmouel Boudjnah <chmouel@enovance.com>
Date:   Wed Jul 17 15:46:03 2013 +0200

Add bulk middleware to proxy-server.conf-sample
    
    - Fixes bug 1201844.
    
    Change-Id: I8eed54d0a17a0c6b746ed616634fc9adb89e5f37

commit 5449155fb0225fbd2747188c31318ce1b1de067e
Author: Thomas Leaman <thomas.leaman@hp.com>
Date:   Fri Jun 28 08:51:24 2013 +0000

Allow floating point value for dispersion_coverage
    
    For systems with very large numbers of partitions, 1% dispersion
    coverage may simply be too much/take too long. This fix allows <1
    values to be used for dispersion_coverage.
    
    DocImpact
    
    Change-Id: I5ed35b69754d55a410e66e658b3854de57c7666b

Thierry Carrez (ttx) on 2013-08-07

Changed in swift:
milestone:	none → 1.9.1
status:	Fix Committed → Fix Released

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.