s3: Make s3 support configurable

Amazon S3 compatibility:
Due to security concerns raised, this change makes S3 support tunable
using a config option and is turned off by default.

Change-Id: I077f78946983f5d6b3b725dd6aa3ed178dc5604e
Signed-off-by: Prashanth Pai <ppai@redhat.com>

README.md

@@ -80,6 +80,8 @@ swauth when `auth_type` in swauth is configured to be *Plaintext* (default).
     [pipeline:main]
     pipeline = catch_errors cache swift3 swauth proxy-server
 
-It can be used with `auth_type` set to Sha1/Sha512 too but with certain caveats.
+It can be used with `auth_type` set to Sha1/Sha512 too but with certain caveats
+and security concern. Hence, s3 support is disabled by default and you have to
+explicitly enable it in your configuration.
 Refer to swift3 compatibility [section](https://swauth.readthedocs.io/en/latest/#swift3-middleware-compatibility)
 in documentation for further details

doc/source/index.rst

@@ -124,12 +124,21 @@ Web Admin Install
 
 Swift3 Middleware Compatibility
 -------------------------------
-`Swift3 middleware <https://github.com/openstack/swift3>`_ can be used with
-swauth when `auth_type` in swauth is configured to be *Plaintext* (default)::
+
+
+`Swift3 middleware <https://github.com/openstack/swift3>`_ support has to be
+explicitly turned on in conf file using `s3_support` config option. It can
+easily be used with swauth when `auth_type` in swauth is configured to be
+*Plaintext* (default)::
 
     [pipeline:main]
     pipeline = catch_errors cache swift3 swauth proxy-server
 
+    [filter:swauth]
+    use = egg:swauth#swauth
+    super_admin_key = swauthkey
+    s3_support = on
+
 The AWS S3 client uses password in plaintext to
 `compute HMAC signature <https://docs.aws.amazon.com/AmazonS3/latest/dev/RESTAuthentication.html>`_
 When `auth_type` in swauth is configured to be *Sha1* or *Sha512*, swauth
@@ -139,7 +148,22 @@ in signature mismatch although the user credentials are correct.
 When `auth_type` is **not** *Plaintext*, the only way for S3 clients to
 authenticate is by giving SHA1/SHA512 of password as input to it's HMAC
 function. In this case, the S3 clients will have to know `auth_type` and
-`salt` beforehand.
+`auth_type_salt` beforehand. Here is a sample configuration::
+
+    [pipeline:main]
+    pipeline = catch_errors cache swift3 swauth proxy-server
+
+    [filter:swauth]
+    use = egg:swauth#swauth
+    super_admin_key = swauthkey
+    s3_support = on
+    auth_type = Sha512
+    auth_type_salt = mysalt
+
+**Security Concern**: Swauth stores user information (username, password hash,
+salt etc) as objects in the Swift cluster. If these backend objects which
+contain password hashes gets stolen, the intruder will be able to authenticate
+using the hash directly when S3 API is used.
 
 
 Contents

swauth/middleware.py

@@ -168,6 +168,16 @@ def __init__(self, app, conf):
         # If auth_type_salt is not set in conf file, a random salt will be
         # generated for each new password to be encoded.
         self.auth_encoder.salt = conf.get('auth_type_salt', None)
+
+        # Due to security concerns, S3 support is disabled by default.
+        self.s3_support = conf.get('s3_support', 'off').lower() in TRUE_VALUES
+        if self.s3_support and self.auth_type != 'Plaintext' \
+                and not self.auth_encoder.salt:
+            msg = _('S3 support requires salt to be manually set in conf '
+                    'file using auth_type_salt config option.')
+            self.logger.warning(msg)
+            self.s3_support = False
+
         self.allow_overrides = \
             conf.get('allow_overrides', 't').lower() in TRUE_VALUES
         self.agent = '%(orig)s Swauth'
@@ -232,6 +242,9 @@ def __call__(self, env, start_response):
             elif env.get('PATH_INFO', '').startswith(self.auth_prefix):
                 return self.handle(env, start_response)
         s3 = env.get('HTTP_AUTHORIZATION')
+        if s3 and not self.s3_support:
+            msg = 'S3 support is disabled in swauth.'
+            return HTTPBadRequest(body=msg)(env, start_response)
         token = env.get('HTTP_X_AUTH_TOKEN', env.get('HTTP_X_STORAGE_TOKEN'))
         if token and len(token) > swauth.authtypes.MAX_TOKEN_LENGTH:
             return HTTPBadRequest(body='Token exceeds maximum length.')(env,
@@ -310,6 +323,9 @@ def get_groups(self, env, token):
                     groups = None
 
         if env.get('HTTP_AUTHORIZATION'):
+            if not self.s3_support:
+                self.logger.warning('S3 support is disabled in swauth.')
+                return None
             if self.swauth_remote:
                 # TODO(gholt): Support S3-style authorization with
                 # swauth_remote mode

test/unit/test_middleware.py

@@ -4033,13 +4033,50 @@ def test_token_too_long(self):
         self.assertEqual(resp.status_int, 400)
         self.assertEqual(resp.body, 'Token exceeds maximum length.')
 
-    def test_crazy_authorization(self):
+    def test_s3_enabled_when_conditions_are_met(self):
+        # auth_type_salt needs to be set
+        for atype in ('Sha1', 'Sha512'):
+            test_auth = \
+                auth.filter_factory({
+                    'super_admin_key': 'supertest',
+                    's3_support': 'on',
+                    'auth_type_salt': 'blah',
+                    'auth_type': atype})(FakeApp())
+            self.assertTrue(test_auth.s3_support)
+        # auth_type_salt need not be set for Plaintext
+        test_auth = \
+            auth.filter_factory({
+                'super_admin_key': 'supertest',
+                's3_support': 'on',
+                'auth_type': 'Plaintext'})(FakeApp())
+        self.assertTrue(test_auth.s3_support)
+
+    def test_s3_disabled_when_conditions_not_met(self):
+        # Conf says that it wants s3 support but other conditions are not met
+        # In that case s3 support should be disabled.
+        for atype in ('Sha1', 'Sha512'):
+            # auth_type_salt is not set
+            test_auth = \
+                auth.filter_factory({
+                    'super_admin_key': 'supertest',
+                    's3_support': 'on',
+                    'auth_type': atype})(FakeApp())
+            self.assertFalse(test_auth.s3_support)
+
+    def test_s3_authorization_default_off(self):
+        self.assertFalse(self.test_auth.s3_support)
         req = self._make_request('/v1/AUTH_account', headers={
-            'authorization': 'somebody elses header value'})
+            'authorization': 's3_header'})
         resp = req.get_response(self.test_auth)
-        self.assertEqual(resp.status_int, 401)
-        self.assertEqual(resp.environ['swift.authorize'],
-                         self.test_auth.denied_response)
+        self.assertEqual(resp.status_int, 400)  # HTTPBadRequest
+        self.assertTrue(resp.environ.get('swift.authorize') is None)
+
+    def test_s3_turned_off_get_groups(self):
+        env = \
+            {'HTTP_AUTHORIZATION': 's3 header'}
+        token = 'whatever'
+        self.test_auth.logger = mock.Mock()
+        self.assertEqual(self.test_auth.get_groups(env, token), None)
 
     def test_default_storage_policy(self):
         ath = auth.filter_factory({})(FakeApp())
@@ -4050,6 +4087,7 @@ def test_default_storage_policy(self):
         self.assertEqual(ath.default_storage_policy, 'ssd')
 
     def test_s3_creds_unicode(self):
+        self.test_auth.s3_support = True
         self.test_auth.app = FakeApp(iter([
             ('200 Ok', {},
              json.dumps({"auth": unicode("plaintext:key)"),
@@ -4064,6 +4102,7 @@ def test_s3_creds_unicode(self):
         self.assertEqual(self.test_auth.get_groups(env, token), None)
 
     def test_s3_only_hash_passed_to_hmac(self):
+        self.test_auth.s3_support = True
         key = 'dadada'
         salt = 'zuck'
         key_hash = hashlib.sha1('%s%s' % (salt, key)).hexdigest()