From a02b05c9b2f77315a646b76d2560bc42a25614a4 Mon Sep 17 00:00:00 2001 From: Christian Schwede Date: Sat, 29 Aug 2015 16:03:07 +0000 Subject: [PATCH] Do not use pre-authenticated requests in staticweb staticweb middleware uses make_pre_authed_env, this makes it possible to anonymously list containers without any read acl set if the metadata "web-listings: true" is set on a container. Using make_env enforces correct read_acl validation; however it is now also required to add ".rlistings" to the read acl. Also, if the staticweb middleware is put in the proxy pipeline before an authentication middleware, it broke authenticated GET and HEAD requests. This has some side effects in clients, because a html response is sent that might be parsed wrongly by the client. In case of python-swiftclient this was shown as an empty container without any ACL or web-listings:true meta set. This might lead to information leaks, because a user trusts the output from python-swiftclient and assumes an empty, private container even if the container contains public readable data. staticweb now checks if "swift.authorize" is included in the environ and skips itself if not. Change-Id: Icf159d7e567ac5481e710c5910db686bdcba6336 --- swift/common/middleware/staticweb.py | 17 ++++++++++++----- test/unit/common/middleware/test_staticweb.py | 22 +++++++++++++++++++++- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/swift/common/middleware/staticweb.py b/swift/common/middleware/staticweb.py index d16b5ae..ebcf78d 100644 --- a/swift/common/middleware/staticweb.py +++ b/swift/common/middleware/staticweb.py @@ -92,6 +92,7 @@ Example usage of this middleware via ``swift``: Turn on listings:: + swift post -r '.r:*,.rlistings' container swift post -m 'web-listings: true' container Now you should see object listings for paths and pseudo paths that have no @@ -121,8 +122,8 @@ import json import time from swift.common.utils import human_readable, split_path, config_true_value, \ - quote, register_swift_info -from swift.common.wsgi import make_pre_authed_env, WSGIContext + quote, register_swift_info, get_logger +from swift.common.wsgi import make_env, WSGIContext from swift.common.http import is_success, is_redirection, HTTP_NOT_FOUND from swift.common.swob import Response, HTTPMovedPermanently, HTTPNotFound from swift.proxy.controllers.base import get_container_info @@ -167,7 +168,7 @@ class _StaticWebContext(WSGIContext): save_response_status = self._response_status save_response_headers = self._response_headers save_response_exc_info = self._response_exc_info - resp = self._app_call(make_pre_authed_env( + resp = self._app_call(make_env( env, 'GET', '/%s/%s/%s/%s%s' % ( self.version, self.account, self.container, self._get_status_int(), self._error), @@ -236,7 +237,7 @@ class _StaticWebContext(WSGIContext): body += ' \n\n' resp = HTTPNotFound(body=body)(env, self._start_response) return self._error_response(resp, env, start_response) - tmp_env = make_pre_authed_env( + tmp_env = make_env( env, 'GET', '/%s/%s/%s' % ( self.version, self.account, self.container), self.agent, swift_source='SW') @@ -429,7 +430,7 @@ class _StaticWebContext(WSGIContext): return resp if status_int == HTTP_NOT_FOUND: if env['PATH_INFO'][-1] != '/': - tmp_env = make_pre_authed_env( + tmp_env = make_env( env, 'GET', '/%s/%s/%s' % ( self.version, self.account, self.container), self.agent, swift_source='SW') @@ -463,6 +464,7 @@ class StaticWeb(object): self.app = app #: The filter configuration dict. self.conf = conf + self.logger = get_logger(conf, log_route='staticweb') def __call__(self, env, start_response): """ @@ -472,6 +474,11 @@ class StaticWeb(object): :param start_response: The WSGI start_response hook. """ env['staticweb.start_time'] = time.time() + if 'swift.authorize' not in env: + self.logger.warning( + 'No authentication middleware authorized request yet. ' \ + 'Skipping staticweb') + return self.app(env, start_response) try: (version, account, container, obj) = \ split_path(env['PATH_INFO'], 2, 4, True) diff --git a/test/unit/common/middleware/test_staticweb.py b/test/unit/common/middleware/test_staticweb.py index c0836a3..81d026c 100644 --- a/test/unit/common/middleware/test_staticweb.py +++ b/test/unit/common/middleware/test_staticweb.py @@ -379,11 +379,22 @@ class FakeApp(object): body=body)(env, start_response) +class FakeAuthFilter(object): + + def __init__(self, app): + self.app = app + + def __call__(self, env, start_response): + env['swift.authorize'] = None + return self.app(env, start_response) + + class TestStaticWeb(unittest.TestCase): def setUp(self): self.app = FakeApp() - self.test_staticweb = staticweb.filter_factory({})(self.app) + self.test_staticweb = FakeAuthFilter( + staticweb.filter_factory({})(self.app)) self._orig_get_container_info = staticweb.get_container_info staticweb.get_container_info = mock_get_container_info @@ -701,6 +712,15 @@ class TestStaticWeb(unittest.TestCase): self.assertEquals(resp.body, '1') self.assertEquals(self.app.calls, 1) + def test_no_auth_middleware(self): + resp = Request.blank('/v1/a/c3').get_response(self.test_staticweb) + self.assertEquals(resp.status_int, 301) + # Test without an authentication middleware before staticweb + # This is no longer handled by staticweb middleware, thus not returning + # a 301 redirect + self.test_staticweb = staticweb.filter_factory({})(self.app) + resp = Request.blank('/v1/a/c3').get_response(self.test_staticweb) + self.assertEquals(resp.status_int, 200) if __name__ == '__main__': unittest.main() -- 1.9.1