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.

Closes-Bug: 1489749
Change-Id: Icf159d7e567ac5481e710c5910db686bdcba6336
Depends-On: Ie24bb995023c377e49796910ad80a256b00daa03

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 += ' </body>\n</html>\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)

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.assertEqual(resp.body, '1')
         self.assertEqual(self.app.calls, 1)
 
+    def test_no_auth_middleware(self):
+        resp = Request.blank('/v1/a/c3').get_response(self.test_staticweb)
+        self.assertEqual(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.assertEqual(resp.status_int, 200)
 
 if __name__ == '__main__':
     unittest.main()