Repository: boto/botocore
Issue: Clients sigv4 signing thread safety

I was working on porting the AWS CLI `s3` commands over to clients, and I ran into occasional signing errors when performing large multi-threaded multi-part uploads that required sigv4 signing. The mismatch was because the timestamps was inconsistent in the signature. This is typically exhibited when there is a concurrency bug in our signing process. I believe it is because we do not lock the signing process for clients, which is needed because the timestamp state is shared across threads. I would like it though if we removed that timestamp shared state instead to avoid running into potential concurrency issues in the future.


Changed files:
tests/unit/auth/test_sigv4.py, botocore/awsrequest.py, botocore/auth.py, tests/unit/auth/test_signers.py

diff --git a/botocore/auth.py b/botocore/auth.py
index 6ab27ba5e..2e243d2cc 100644
--- a/botocore/auth.py
+++ b/botocore/auth.py
@@ -42,6 +42,7 @@ EMPTY_SHA256_HASH = (
 # gave the best result (in terms of performance).
 PAYLOAD_BUFFER = 1024 * 1024
 ISO8601 = '%Y-%m-%dT%H:%M:%SZ'
+SIGV4_TIMESTAMP = '%Y%m%dT%H%M%SZ'
 
 
 class BaseSigner(object):
@@ -149,8 +150,6 @@ class SigV4Auth(BaseSigner):
         # We initialize these value here so the unit tests can have
         # valid values.  But these will get overriden in ``add_auth``
         # later for real requests.
-        self._now = datetime.datetime.utcnow()
-        self.timestamp = self._now.strftime('%Y%m%dT%H%M%SZ')
         self._region_name = region_name
         self._service_name = service_name
 
@@ -265,17 +264,17 @@ class SigV4Auth(BaseSigner):
         normalized_path = quote(normalize_url_path(path), safe='/~')
         return normalized_path
 
-    def scope(self, args):
+    def scope(self, request):
         scope = [self.credentials.access_key]
-        scope.append(self.timestamp[0:8])
+        scope.append(request.context['timestamp'][0:8])
         scope.append(self._region_name)
         scope.append(self._service_name)
         scope.append('aws4_request')
         return '/'.join(scope)
 
-    def credential_scope(self, args):
+    def credential_scope(self, request):
         scope = []
-        scope.append(self.timestamp[0:8])
+        scope.append(request.context['timestamp'][0:8])
         scope.append(self._region_name)
         scope.append(self._service_name)
         scope.append('aws4_request')
@@ -288,15 +287,15 @@ class SigV4Auth(BaseSigner):
         were included in the StringToSign.
         """
         sts = ['AWS4-HMAC-SHA256']
-        sts.append(self.timestamp)
+        sts.append(request.context['timestamp'])
         sts.append(self.credential_scope(request))
         sts.append(sha256(canonical_request.encode('utf-8')).hexdigest())
         return '\\n'.join(sts)
 
-    def signature(self, string_to_sign):
+    def signature(self, string_to_sign, request):
         key = self.credentials.secret_key
         k_date = self._sign(('AWS4' + key).encode('utf-8'),
-                            self.timestamp[0:8])
+                            request.context['timestamp'][0:8])
         k_region = self._sign(k_date, self._region_name)
         k_service = self._sign(k_region, self._service_name)
         k_signing = self._sign(k_service, 'aws4_request')
@@ -305,9 +304,8 @@ class SigV4Auth(BaseSigner):
     def add_auth(self, request):
         if self.credentials is None:
             raise NoCredentialsError
-        # Create a new timestamp for each signing event
-        self._now = datetime.datetime.utcnow()
-        self.timestamp = self._now.strftime('%Y%m%dT%H%M%SZ')
+        datetime_now = datetime.datetime.utcnow()
+        request.context['timestamp'] = datetime_now.strftime(SIGV4_TIMESTAMP)
         # This could be a retry.  Make sure the previous
         # authorization header is removed first.
         self._modify_request_before_signing(request)
@@ -316,7 +314,7 @@ class SigV4Auth(BaseSigner):
         logger.debug('CanonicalRequest:\\n%s', canonical_request)
         string_to_sign = self.string_to_sign(request, canonical_request)
         logger.debug('StringToSign:\\n%s', string_to_sign)
-        signature = self.signature(string_to_sign)
+        signature = self.signature(string_to_sign, request)
         logger.debug('Signature:\\n%s', signature)
 
         self._inject_signature_to_request(request, signature)
@@ -344,14 +342,16 @@ class SigV4Auth(BaseSigner):
         # header.  Otherwise we use the X-Amz-Date header.
         if 'Date' in request.headers:
             del request.headers['Date']
+            datetime_timestamp = datetime.datetime.strptime(
+                request.context['timestamp'], SIGV4_TIMESTAMP)
             request.headers['Date'] = formatdate(
-                int(calendar.timegm(self._now.timetuple())))
+                int(calendar.timegm(datetime_timestamp.timetuple())))
             if 'X-Amz-Date' in request.headers:
                 del request.headers['X-Amz-Date']
         else:
             if 'X-Amz-Date' in request.headers:
                 del request.headers['X-Amz-Date']
-            request.headers['X-Amz-Date'] = self.timestamp
+            request.headers['X-Amz-Date'] = request.context['timestamp']
 
 
 class S3SigV4Auth(SigV4Auth):
@@ -387,7 +387,7 @@ class SigV4QueryAuth(SigV4Auth):
         auth_params = {
             'X-Amz-Algorithm': 'AWS4-HMAC-SHA256',
             'X-Amz-Credential': self.scope(request),
-            'X-Amz-Date': self.timestamp,
+            'X-Amz-Date': request.context['timestamp'],
             'X-Amz-Expires': self._expires,
             'X-Amz-SignedHeaders': 'host',
         }
diff --git a/botocore/awsrequest.py b/botocore/awsrequest.py
index c7e001784..00764c7cd 100644
--- a/botocore/awsrequest.py
+++ b/botocore/awsrequest.py
@@ -263,6 +263,14 @@ class AWSRequest(models.RequestEncodingMixin, models.Request):
             for key, value in self.headers.items():
                 headers[key] = value
         self.headers = headers
+        # This is a dictionary to hold information that is used when
+        # processing the request. What is inside of ``context`` is open-ended.
+        # For example, it may have a timestamp key that is used for holding
+        # what the timestamp is when signing the request. Note that none
+        # of the information that is inside of ``context`` is directly
+        # sent over the wire; the information is only used to assist in
+        # creating what is sent over the wire.
+        self.context = {}
 
     def prepare(self):
         """Constructs a :class:`AWSPreparedRequest <AWSPreparedRequest>`."""
diff --git a/tests/unit/auth/test_signers.py b/tests/unit/auth/test_signers.py
index 230ca8095..e2cbc4178 100644
--- a/tests/unit/auth/test_signers.py
+++ b/tests/unit/auth/test_signers.py
@@ -328,6 +328,45 @@ class TestSigV4(unittest.TestCase):
                     "%22directors%5E10%22%5D%7D")
         self.assertEqual(actual, expected)
 
+    def test_thread_safe_timestamp(self):
+        request = AWSRequest()
+        request.url = (
+            'https://search-testdomain1-j67dwxlet67gf7ghwfmik2c67i.us-west-2.'
+            'cloudsearch.amazonaws.com/'
+            '2013-01-01/search?format=sdk&pretty=true&'
+            'q.options=%7B%22defaultOperator%22%3A%20%22and%22%2C%20%22'
+            'fields%22%3A%5B%22directors%5E10%22%5D%7D&q=George%20Lucas'
+        )
+        request.method = 'GET'
+        auth = botocore.auth.SigV4Auth(
+            self.credentials, 'cloudsearchdomain', 'us-west-2')
+        with mock.patch.object(
+                botocore.auth.datetime, 'datetime',
+                mock.Mock(wraps=datetime.datetime)) as mock_datetime:
+            original_utcnow = datetime.datetime(2014, 1, 1, 0, 0)
+ 
+            mock_datetime.utcnow.return_value = original_utcnow
+            # Go through the add_auth process once. This will attach
+            # a timestamp to the request at the beginning of auth.
+            auth.add_auth(request)
+            self.assertEqual(request.context['timestamp'], '20140101T000000Z')
+            # Ensure the date is in the Authorization header
+            self.assertIn('20140101', request.headers['Authorization'])
+            # Now suppose the utc time becomes the next day all of a sudden
+            mock_datetime.utcnow.return_value = datetime.datetime(
+                2014, 1, 2, 0, 0)
+            # Smaller methods like the canonical request and string_to_sign
+            # should  have the timestamp attached to the request in their
+            # body and not what the time is now mocked as. This is to ensure
+            # there is no mismatching in timestamps when signing.
+            cr = auth.canonical_request(request)
+            self.assertIn('x-amz-date:20140101T000000Z', cr)
+            self.assertNotIn('x-amz-date:20140102T000000Z', cr)
+
+            sts = auth.string_to_sign(request, cr)
+            self.assertIn('20140101T000000Z', sts)
+            self.assertNotIn('20140102T000000Z', sts)
+
 
 class TestSigV4Resign(unittest.TestCase):
 
@@ -341,13 +380,6 @@ class TestSigV4Resign(unittest.TestCase):
         self.request = AWSRequest()
         self.request.method = 'PUT'
         self.request.url = 'https://ec2.amazonaws.com/'
-        self.datetime_patch = mock.patch('botocore.auth.datetime')
-        self.datetime_mock = self.datetime_patch.start()
-        self.now = datetime.datetime.utcnow()
-        self.datetime_mock.datetime.utcnow.return_value = self.now
-
-    def tearDown(self):
-        self.datetime_patch.stop()
 
     def test_resign_request_with_date(self):
         self.request.headers['Date'] = 'Thu, 17 Nov 2005 18:49:58 GMT'
diff --git a/tests/unit/auth/test_sigv4.py b/tests/unit/auth/test_sigv4.py
index 89662ba2f..d048539da 100644
--- a/tests/unit/auth/test_sigv4.py
+++ b/tests/unit/auth/test_sigv4.py
@@ -122,6 +122,8 @@ def create_request_from_raw_request(raw_request):
     if raw.error_code is not None:
         raise Exception(raw.error_message)
     request.method = raw.command
+    datetime_now = datetime.datetime(2011, 9, 9, 23, 36)
+    request.context['timestamp'] = datetime_now.strftime('%Y%m%dT%H%M%SZ')
     for key, val in raw.headers.items():
         request.headers[key] = val
     request.data = raw.rfile.read().decode('utf-8')