From d0013b0498929d4808ef52fb2cd1906fab2df08b Mon Sep 17 00:00:00 2001 From: Christian Schwede Date: Fri, 28 Aug 2015 05:09:11 +0000 Subject: [PATCH] Validate acl and pass unauthenticated requests in staticweb middleware The staticweb middleware does not check the read acl of a container, despite saying so in the documentation. This makes it possible to anonymously list containers without any read acl set. 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. Change-Id: Ic9cbb855bf64d7dc1f09b568d6ff2ce6a3dae0ee --- swift/common/middleware/staticweb.py | 26 +++++++++++----- test/unit/common/middleware/test_staticweb.py | 43 ++++++++++++++++++++++----- 2 files changed, 53 insertions(+), 16 deletions(-) diff --git a/swift/common/middleware/staticweb.py b/swift/common/middleware/staticweb.py index d16b5ae..01e26d6 100644 --- a/swift/common/middleware/staticweb.py +++ b/swift/common/middleware/staticweb.py @@ -120,11 +120,12 @@ import cgi import json import time +from swift.common.middleware.acl import parse_acl, referrer_allowed from swift.common.utils import human_readable, split_path, config_true_value, \ - quote, register_swift_info + quote, register_swift_info, get_logger from swift.common.wsgi import make_pre_authed_env, WSGIContext from swift.common.http import is_success, is_redirection, HTTP_NOT_FOUND -from swift.common.swob import Response, HTTPMovedPermanently, HTTPNotFound +from swift.common.swob import Response, HTTPMovedPermanently, HTTPNotFound, Request from swift.proxy.controllers.base import get_container_info @@ -194,12 +195,15 @@ class _StaticWebContext(WSGIContext): self._dir_type = None container_info = get_container_info(env, self.app, swift_source='SW') if is_success(container_info['status']): - meta = container_info.get('meta', {}) - self._index = meta.get('web-index', '').strip() - self._error = meta.get('web-error', '').strip() - self._listings = meta.get('web-listings', '').strip() - self._listings_css = meta.get('web-listings-css', '').strip() - self._dir_type = meta.get('web-directory-type', '').strip() + req = Request(env.copy()) + referrers, groups = parse_acl(container_info.get('read_acl', '')) + if referrer_allowed(req.referer, referrers): + meta = container_info.get('meta', {}) + self._index = meta.get('web-index', '').strip() + self._error = meta.get('web-error', '').strip() + self._listings = meta.get('web-listings', '').strip() + self._listings_css = meta.get('web-listings-css', '').strip() + self._dir_type = meta.get('web-directory-type', '').strip() def _listing(self, env, start_response, prefix=None): """ @@ -463,6 +467,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): """ @@ -479,6 +484,11 @@ class StaticWeb(object): return self.app(env, start_response) if env['REQUEST_METHOD'] not in ('HEAD', 'GET'): return self.app(env, start_response) + if env.get('HTTP_X_AUTH_TOKEN') and not env.get('REMOTE_USER'): + self.logger.warning( + 'X-Auth-Token found but not yet authenticated. ' \ + 'Skipping staticweb middleware') + return self.app(env, start_response) if env.get('REMOTE_USER') and \ not config_true_value(env.get('HTTP_X_WEB_MODE', 'f')): return self.app(env, start_response) diff --git a/test/unit/common/middleware/test_staticweb.py b/test/unit/common/middleware/test_staticweb.py index c0836a3..64f77c6 100644 --- a/test/unit/common/middleware/test_staticweb.py +++ b/test/unit/common/middleware/test_staticweb.py @@ -54,16 +54,10 @@ meta_map = { 'web-error': 'error.html'}}, 'c13': {'meta': {'web-listings': 'f', 'web-listings-css': 'listing.css'}}, + 'c14': {'meta': {'web-listings': 't'}}, } -def mock_get_container_info(env, app, swift_source='SW'): - container = env['PATH_INFO'].rstrip('/').split('/')[3] - container_info = meta_map[container] - container_info.setdefault('status', 200) - container_info.setdefault('read_acl', '.r:*') - return container_info - class FakeApp(object): @@ -240,6 +234,8 @@ class FakeApp(object): elif env['PATH_INFO'] == '/v1/a/c12/200error.html': return Response(status='200 Ok', body='error file')(env, start_response) + elif env['PATH_INFO'] == '/v1/a/c14': + return Response(status='401 Unauthorized')(env, start_response) else: raise Exception('Unknown path %r' % env['PATH_INFO']) @@ -381,11 +377,20 @@ class FakeApp(object): class TestStaticWeb(unittest.TestCase): + def mock_get_container_info(self, env, app, swift_source='SW'): + container = env['PATH_INFO'].rstrip('/').split('/')[3] + container_info = meta_map[container] + container_info.setdefault('status', 200) + if self.mock_read_acl: + container_info.setdefault('read_acl', '.r:*') + return container_info + def setUp(self): self.app = FakeApp() self.test_staticweb = staticweb.filter_factory({})(self.app) self._orig_get_container_info = staticweb.get_container_info - staticweb.get_container_info = mock_get_container_info + staticweb.get_container_info = self.mock_get_container_info + self.mock_read_acl = True def tearDown(self): staticweb.get_container_info = self._orig_get_container_info @@ -701,6 +706,28 @@ class TestStaticWeb(unittest.TestCase): self.assertEquals(resp.body, '1') self.assertEquals(self.app.calls, 1) + def test_container14_token_unauthed(self): + # Request processed by staticweb middleware + resp = Request.blank('/v1/a/c14').get_response(self.test_staticweb) + self.assertEquals(resp.status_int, 301) + + # Simple pass-trough with a unauthenticated token + resp = Request.blank( + '/v1/a/c14', + environ={'HTTP_X_AUTH_TOKEN': 'token'}).get_response( + self.test_staticweb) + self.assertEquals(resp.status_int, 401) + + def test_container14_acl_check(self): + # Request processed by staticweb middleware + resp = Request.blank('/v1/a/c14').get_response(self.test_staticweb) + self.assertEquals(resp.status_int, 301) + + # Simple pass-trough with missing ACL + self.mock_read_acl = False + resp = Request.blank('/v1/a/c14').get_response(self.test_staticweb) + self.assertEquals(resp.status_int, 401) + if __name__ == '__main__': unittest.main() -- 1.9.1