From f9bed74d1bba6a512becd057c3139c54c176c226 Mon Sep 17 00:00:00 2001 From: Clay Gerrard <clay.gerrard@gmail.com> Date: Wed, 29 Oct 2014 15:59:45 -0700 Subject: [PATCH] Return 403 on unauthorized upload when over account quota If you try an unauthorized upload into a container that is over quota you get a 403 instead of a 413, but if you try to unauthorized upload when an *account* is over quota you can see the 413 even though the upload would have been rejected by the authorize callback. By wrapping the authorize callback associated with the incoming request we can make sure to only return our 413 when the request would have been authorized otherwise. Drive by doc fixes thanks to acoles: * State that container_quotas should be after auth middleware in the class doc string. * Add note to proxy-server.conf.sample that account_quotas should be after auth middleware. The equivalent statements are already in place for each quota middleware. Doc-Impact Closes-Bug: #1387415 Change-Id: I2a88b3ec79d35bfdd73ea6ad64e376b7c7af4ea6 --- etc/proxy-server.conf-sample | 9 +-- swift/common/middleware/account_quotas.py | 16 ++++- swift/common/middleware/container_quotas.py | 15 +++- .../common/middleware/test_account_quotas.py | 69 ++++++++++++++++++- 4 files changed, 100 insertions(+), 9 deletions(-) diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample index e17f0a7703..51995e392b 100644 --- a/etc/proxy-server.conf-sample +++ b/etc/proxy-server.conf-sample @@ -532,10 +532,6 @@ use = egg:swift#bulk # delete_container_retry_count = 0 -# Note: Put after auth in the pipeline. -[filter:container-quotas] -use = egg:swift#container_quotas - # Note: Put after auth and staticweb in the pipeline. [filter:slo] use = egg:swift#slo @@ -568,6 +564,11 @@ use = egg:swift#dlo # Time limit on GET requests (seconds) # max_get_time = 86400 +# Note: Put after auth in the pipeline. +[filter:container-quotas] +use = egg:swift#container_quotas + +# Note: Put after auth in the pipeline. [filter:account-quotas] use = egg:swift#account_quotas diff --git a/swift/common/middleware/account_quotas.py b/swift/common/middleware/account_quotas.py index a3c7dc96cd..fcb55b5573 100644 --- a/swift/common/middleware/account_quotas.py +++ b/swift/common/middleware/account_quotas.py @@ -53,7 +53,8 @@ account size has been updated. """ from swift.common.constraints import check_copy_from_header -from swift.common.swob import HTTPForbidden, Response, HTTPBadRequest, wsgify +from swift.common.swob import HTTPForbidden, HTTPBadRequest, \ + HTTPRequestEntityTooLarge, wsgify from swift.common.utils import register_swift_info from swift.proxy.controllers.base import get_account_info, get_object_info @@ -136,7 +137,18 @@ class AccountQuotaMiddleware(object): new_size = int(account_info['bytes']) + content_length if quota < new_size: - return Response(status=413, body='Upload exceeds quota.') + resp = HTTPRequestEntityTooLarge(body='Upload exceeds quota.') + if 'swift.authorize' in request.environ: + orig_authorize = request.environ['swift.authorize'] + + def reject_authorize(*args, **kwargs): + aresp = orig_authorize(*args, **kwargs) + if aresp: + return aresp + return resp + request.environ['swift.authorize'] = reject_authorize + else: + return resp return self.app diff --git a/swift/common/middleware/container_quotas.py b/swift/common/middleware/container_quotas.py index 3edaf1fb2a..4aef895d2f 100644 --- a/swift/common/middleware/container_quotas.py +++ b/swift/common/middleware/container_quotas.py @@ -40,10 +40,21 @@ set: | X-Container-Meta-Quota-Count | Maximum object count of the | | | container. | +---------------------------------------------+-------------------------------+ + +The ``container_quotas`` middleware should be added to the pipeline in your +``/etc/swift/proxy-server.conf`` file just after any auth middleware. +For example:: + + [pipeline:main] + pipeline = catch_errors cache tempauth container_quotas proxy-server + + [filter:container_quotas] + use = egg:swift#container_quotas """ from swift.common.constraints import check_copy_from_header from swift.common.http import is_success -from swift.common.swob import Response, HTTPBadRequest, wsgify +from swift.common.swob import HTTPRequestEntityTooLarge, HTTPBadRequest, \ + wsgify from swift.common.utils import register_swift_info from swift.proxy.controllers.base import get_container_info, get_object_info @@ -60,7 +71,7 @@ class ContainerQuotaMiddleware(object): aresp = req.environ['swift.authorize'](req) if aresp: return aresp - return Response(status=413, body='Upload exceeds quota.') + return HTTPRequestEntityTooLarge(body='Upload exceeds quota.') @wsgify def __call__(self, req): diff --git a/test/unit/common/middleware/test_account_quotas.py b/test/unit/common/middleware/test_account_quotas.py index 75bd1d70eb..e8c2563c5a 100644 --- a/test/unit/common/middleware/test_account_quotas.py +++ b/test/unit/common/middleware/test_account_quotas.py @@ -13,7 +13,7 @@ import unittest -from swift.common.swob import Request +from swift.common.swob import Request, wsgify, HTTPForbidden from swift.common.middleware import account_quotas @@ -51,6 +51,10 @@ class FakeApp(object): self.headers = headers def __call__(self, env, start_response): + if 'swift.authorize' in env: + aresp = env['swift.authorize'](Request(env)) + if aresp: + return aresp(env, start_response) if env['REQUEST_METHOD'] == "HEAD" and \ env['PATH_INFO'] == '/v1/a/c2/o2': env_key = get_object_env_key('a', 'c2', 'o2') @@ -67,6 +71,21 @@ class FakeApp(object): return [] +class FakeAuthFilter(object): + + def __init__(self, app): + self.app = app + + @wsgify + def __call__(self, req): + def authorize(req): + if req.headers['x-auth-token'] == 'secret': + return + return HTTPForbidden(request=req) + req.environ['swift.authorize'] = authorize + return req.get_response(self.app) + + class TestAccountQuota(unittest.TestCase): def test_unauthorized(self): @@ -142,6 +161,54 @@ class TestAccountQuota(unittest.TestCase): self.assertEquals(res.status_int, 413) self.assertEquals(res.body, 'Upload exceeds quota.') + def test_exceed_quota_not_authorized(self): + headers = [('x-account-bytes-used', '1000'), + ('x-account-meta-quota-bytes', '0')] + app = FakeAuthFilter( + account_quotas.AccountQuotaMiddleware(FakeApp(headers))) + cache = FakeCache(None) + req = Request.blank('/v1/a/c/o', method='PUT', + headers={'x-auth-token': 'bad-secret'}, + environ={'swift.cache': cache}) + res = req.get_response(app) + self.assertEquals(res.status_int, 403) + + def test_exceed_quota_authorized(self): + headers = [('x-account-bytes-used', '1000'), + ('x-account-meta-quota-bytes', '0')] + app = FakeAuthFilter( + account_quotas.AccountQuotaMiddleware(FakeApp(headers))) + cache = FakeCache(None) + req = Request.blank('/v1/a/c/o', method='PUT', + headers={'x-auth-token': 'secret'}, + environ={'swift.cache': cache}) + res = req.get_response(app) + self.assertEquals(res.status_int, 413) + + def test_under_quota_not_authorized(self): + headers = [('x-account-bytes-used', '0'), + ('x-account-meta-quota-bytes', '1000')] + app = FakeAuthFilter( + account_quotas.AccountQuotaMiddleware(FakeApp(headers))) + cache = FakeCache(None) + req = Request.blank('/v1/a/c/o', method='PUT', + headers={'x-auth-token': 'bad-secret'}, + environ={'swift.cache': cache}) + res = req.get_response(app) + self.assertEquals(res.status_int, 403) + + def test_under_quota_authorized(self): + headers = [('x-account-bytes-used', '0'), + ('x-account-meta-quota-bytes', '1000')] + app = FakeAuthFilter( + account_quotas.AccountQuotaMiddleware(FakeApp(headers))) + cache = FakeCache(None) + req = Request.blank('/v1/a/c/o', method='PUT', + headers={'x-auth-token': 'secret'}, + environ={'swift.cache': cache}) + res = req.get_response(app) + self.assertEquals(res.status_int, 200) + def test_over_quota_container_create_still_works(self): headers = [('x-account-bytes-used', '1001'), ('x-account-meta-quota-bytes', '1000')]