s3api: Use constant-time string comparisons in check_signature

Disable rolling-upgrade job since the old tag doesn't constrain things
well enough to run on py2.

Change-Id: Ibe514a7ab22d475517b1efc50de676f47d741a4c
(cherry picked from commit 6142ce88cc71037ba0cd23113eb6082fa91346ac)

3 files changed, 15 insertions, 5 deletions

diff --git a/.zuul.yaml b/.zuul.yaml
index 6b9b77b11..e9bb228a5 100644
--- a/.zuul.yaml
+++ b/.zuul.yaml
@@ -703,6 +703,7 @@
               - ^doc/(requirements.txt|(saio|s3api|source)/.*)$
         - swift-multinode-rolling-upgrade:
             irrelevant-files: *functest-irrelevant-files
+            voting: false
         - tempest-integrated-object-storage:
             irrelevant-files: &tempest-irrelevant-files
               - ^(api-ref|doc|releasenotes)/.*$
@@ -746,8 +747,6 @@
             irrelevant-files: *unittest-irrelevant-files
         - openstack-tox-pep8:
             irrelevant-files: *pep8-irrelevant-files
-        - swift-multinode-rolling-upgrade:
-            irrelevant-files: *functest-irrelevant-files
         - tempest-integrated-object-storage:
             irrelevant-files: *tempest-irrelevant-files
         - tempest-ipv6-only:
diff --git a/swift/common/middleware/s3api/s3request.py b/swift/common/middleware/s3api/s3request.py
index 2d1f7f287..d105bfeef 100644
--- a/swift/common/middleware/s3api/s3request.py
+++ b/swift/common/middleware/s3api/s3request.py
@@ -26,7 +26,7 @@ from six.moves.urllib.parse import quote, unquote, parse_qsl
 import string
 
 from swift.common.utils import split_path, json, get_swift_info, \
-    close_if_possible, md5
+    close_if_possible, md5, streq_const_time
 from swift.common import swob
 from swift.common.http import HTTP_OK, HTTP_CREATED, HTTP_ACCEPTED, \
     HTTP_NO_CONTENT, HTTP_UNAUTHORIZED, HTTP_FORBIDDEN, HTTP_NOT_FOUND, \
@@ -160,7 +160,7 @@ class SigV4Mixin(object):
                 derived_secret, scope_piece.encode('utf8'), sha256).digest()
         valid_signature = hmac.new(
             derived_secret, self.string_to_sign, sha256).hexdigest()
-        return user_signature == valid_signature
+        return streq_const_time(user_signature, valid_signature)
 
     @property
     def _is_query_auth(self):
@@ -558,7 +558,7 @@ class S3Request(swob.Request):
             secret, self.string_to_sign, sha1).digest()).strip()
         if not six.PY2:
             valid_signature = valid_signature.decode('ascii')
-        return user_signature == valid_signature
+        return streq_const_time(user_signature, valid_signature)
 
     @property
     def timestamp(self):
diff --git a/test/unit/common/middleware/s3api/test_s3request.py b/test/unit/common/middleware/s3api/test_s3request.py
index 056bc32eb..34eca793e 100644
--- a/test/unit/common/middleware/s3api/test_s3request.py
+++ b/test/unit/common/middleware/s3api/test_s3request.py
@@ -801,6 +801,11 @@ class TestRequest(S3ApiTestCase):
         self.assertEqual(expected_sts, sigv2_req._string_to_sign())
         self.assertTrue(sigv2_req.check_signature(secret))
 
+        with patch('swift.common.middleware.s3api.s3request.streq_const_time',
+                   return_value=True) as mock_eq:
+            self.assertTrue(sigv2_req.check_signature(secret))
+        mock_eq.assert_called_once()
+
     def test_check_signature_sigv2(self):
         self._test_check_signature_sigv2(
             'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY')
@@ -841,6 +846,12 @@ class TestRequest(S3ApiTestCase):
         self.assertFalse(sigv4_req.check_signature(
             u'\u30c9\u30e9\u30b4\u30f3'))
 
+        with patch('swift.common.middleware.s3api.s3request.streq_const_time',
+                   return_value=False) as mock_eq:
+            self.assertFalse(sigv4_req.check_signature(
+                u'\u30c9\u30e9\u30b4\u30f3'))
+        mock_eq.assert_called_once()
+
     @patch.object(S3Request, '_validate_dates', lambda *a: None)
     def test_check_signature_sigv4_unsigned_payload(self):
         environ = {