Repository: opensearch-project/opensearch-py (python)

Issue:
[BUG] AWSV4SignerAuth doesn't freeze credentials before signing, leading to a race condition
### What is the bug?
AWSV4SignerAuth passes the raw credentials object to SigV4Auth, instead of checking for a `get_frozen_credentials` method and calling that. This leads to the access key for a request differing from the secret key used to sign it.

### How can one reproduce the bug?

1. Setup your AWS SDK with a refreshable credential mechanism.
2. Sign requests like it's going out of business
3. When you cross the refresh threshold, you'll get a request indicating it was signed with a different key than it actually was.

### What is the expected behavior?
All requests get signed with matching access keys & secrets.

### Do you have any additional context?
See this Python docstring in botocore for a more thorough explanation of the problem.

https://github.com/boto/botocore/blob/c29640a5e7a5f77cc4c6bb27e30a5387bc12832a/botocore/credentials.py#L399-L406


Pull request description:
Fix race condition in AWS request signers
Closes #423

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5ba180f0..478b7f92 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 ### Deprecated
 ### Removed
 ### Fixed
+- Fixed race condition in AWSV4SignerAuth & AWSV4SignerAsyncAuth when using refreshable credentials ([#470](https://github.com/opensearch-project/opensearch-py/pull/470))
 ### Security
 ### Dependencies
 - Bumps `sphinx` from <7.1 to <7.2
@@ -115,4 +116,4 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 [2.1.0]: https://github.com/opensearch-project/opensearch-py/compare/v2.0.1...v2.1.0
 [2.1.1]: https://github.com/opensearch-project/opensearch-py/compare/v2.1.0...v2.1.1
 [2.2.0]: https://github.com/opensearch-project/opensearch-py/compare/v2.1.1...v2.2.0
-[2.3.0]: https://github.com/opensearch-project/opensearch-py/compare/v2.2.0...v2.3.0
\ No newline at end of file
+[2.3.0]: https://github.com/opensearch-project/opensearch-py/compare/v2.2.0...v2.3.0
diff --git a/opensearchpy/helpers/asyncsigner.py b/opensearchpy/helpers/asyncsigner.py
index 08a81748..e21007d3 100644
--- a/opensearchpy/helpers/asyncsigner.py
+++ b/opensearchpy/helpers/asyncsigner.py
@@ -50,7 +50,21 @@ class AWSV4SignerAsyncAuth:
             data=body,
         )
 
-        sig_v4_auth = SigV4Auth(self.credentials, self.service, self.region)
+        # credentials objects expose access_key, secret_key and token attributes
+        # via @property annotations that call _refresh() on every access,
+        # creating a race condition if the credentials expire before secret_key
+        # is called but after access_key- the end result is the access_key doesn't
+        # correspond to the secret_key used to sign the request. To avoid this,
+        # get_frozen_credentials() which returns non-refreshing credentials is
+        # called if it exists.
+        credentials = (
+            self.credentials.get_frozen_credentials()
+            if hasattr(self.credentials, "get_frozen_credentials")
+            and callable(self.credentials.get_frozen_credentials)
+            else self.credentials
+        )
+
+        sig_v4_auth = SigV4Auth(credentials, self.service, self.region)
         sig_v4_auth.add_auth(aws_request)
         aws_request.headers["X-Amz-Content-SHA256"] = sig_v4_auth.payload(aws_request)
 
diff --git a/opensearchpy/helpers/signer.py b/opensearchpy/helpers/signer.py
index 9c330b7b..176f6ac9 100644
--- a/opensearchpy/helpers/signer.py
+++ b/opensearchpy/helpers/signer.py
@@ -83,7 +83,21 @@ class AWSV4SignerAuth(requests.auth.AuthBase):
             data=prepared_request.body,
         )
 
-        sig_v4_auth = SigV4Auth(self.credentials, self.service, self.region)
+        # credentials objects expose access_key, secret_key and token attributes
+        # via @property annotations that call _refresh() on every access,
+        # creating a race condition if the credentials expire before secret_key
+        # is called but after access_key- the end result is the access_key doesn't
+        # correspond to the secret_key used to sign the request. To avoid this,
+        # get_frozen_credentials() which returns non-refreshing credentials is
+        # called if it exists.
+        credentials = (
+            self.credentials.get_frozen_credentials()
+            if hasattr(self.credentials, "get_frozen_credentials")
+            and callable(self.credentials.get_frozen_credentials)
+            else self.credentials
+        )
+
+        sig_v4_auth = SigV4Auth(credentials, self.service, self.region)
         sig_v4_auth.add_auth(aws_request)
 
         # copy the headers from AWS request object into the prepared_request


Test patch:
diff --git a/test_opensearchpy/test_async/test_signer.py b/test_opensearchpy/test_async/test_signer.py
index 2c642db4..5b92c89d 100644
--- a/test_opensearchpy/test_async/test_signer.py
+++ b/test_opensearchpy/test_async/test_signer.py
@@ -26,11 +26,11 @@ class TestAsyncSigner:
         dummy_session.access_key = access_key
         dummy_session.secret_key = secret_key
         dummy_session.token = token
+
+        del dummy_session.get_frozen_credentials
+
         return dummy_session
 
-    @pytest.mark.skipif(
-        sys.version_info < (3, 6), reason="AWSV4SignerAsyncAuth requires python3.6+"
-    )
     async def test_aws_signer_async_as_http_auth(self):
         region = "us-west-2"
 
@@ -85,3 +85,34 @@ class TestAsyncSigner:
         assert "X-Amz-Date" in headers
         assert "X-Amz-Security-Token" in headers
         assert "X-Amz-Content-SHA256" in headers
+
+
+class TestAsyncSignerWithFrozenCredentials(TestAsyncSigner):
+    def mock_session(self, disable_get_frozen=True):
+        access_key = uuid.uuid4().hex
+        secret_key = uuid.uuid4().hex
+        token = uuid.uuid4().hex
+        dummy_session = Mock()
+        dummy_session.access_key = access_key
+        dummy_session.secret_key = secret_key
+        dummy_session.token = token
+        dummy_session.get_frozen_credentials = Mock(return_value=dummy_session)
+
+        return dummy_session
+
+    @pytest.mark.skipif(
+        sys.version_info < (3, 6), reason="AWSV4SignerAsyncAuth requires python3.6+"
+    )
+    async def test_aws_signer_async_frozen_credentials_as_http_auth(self):
+        region = "us-west-2"
+
+        from opensearchpy.helpers.asyncsigner import AWSV4SignerAsyncAuth
+
+        mock_session = self.mock_session()
+
+        auth = AWSV4SignerAsyncAuth(mock_session, region)
+        headers = auth("GET", "http://localhost", {}, {})
+        assert "Authorization" in headers
+        assert "X-Amz-Date" in headers
+        assert "X-Amz-Security-Token" in headers
+        assert len(mock_session.get_frozen_credentials.mock_calls) == 1
diff --git a/test_opensearchpy/test_connection.py b/test_opensearchpy/test_connection.py
index c2480946..5ec6e09d 100644
--- a/test_opensearchpy/test_connection.py
+++ b/test_opensearchpy/test_connection.py
@@ -402,6 +402,8 @@ class TestUrllib3Connection(TestCase):
         dummy_session.access_key = access_key
         dummy_session.secret_key = secret_key
         dummy_session.token = token
+        del dummy_session.get_frozen_credentials
+
         return dummy_session
 
     def test_uses_https_if_verify_certs_is_off(self):
@@ -502,6 +504,43 @@ class TestUrllib3Connection(TestCase):
         assert str(e.value) == "Wasn't modified!"
 
 
+class TestSignerWithFrozenCredentials(TestUrllib3Connection):
+    def mock_session(self):
+        access_key = uuid.uuid4().hex
+        secret_key = uuid.uuid4().hex
+        token = uuid.uuid4().hex
+        dummy_session = Mock()
+        dummy_session.access_key = access_key
+        dummy_session.secret_key = secret_key
+        dummy_session.token = token
+        dummy_session.get_frozen_credentials = Mock(return_value=dummy_session)
+
+        return dummy_session
+
+    @pytest.mark.skipif(
+        sys.version_info < (3, 6), reason="AWSV4SignerAuth requires python3.6+"
+    )
+    def test_aws_signer_frozen_credentials_as_http_auth(self):
+        region = "us-west-2"
+
+        import requests
+
+        from opensearchpy.helpers.signer import AWSV4SignerAuth
+
+        mock_session = self.mock_session()
+
+        auth = AWSV4SignerAuth(mock_session, region)
+        con = RequestsHttpConnection(http_auth=auth)
+        prepared_request = requests.Request("GET", "http://localhost").prepare()
+        auth(prepared_request)
+        self.assertEqual(auth, con.session.auth)
+        self.assertIn("Authorization", prepared_request.headers)
+        self.assertIn("X-Amz-Date", prepared_request.headers)
+        self.assertIn("X-Amz-Security-Token", prepared_request.headers)
+        self.assertIn("X-Amz-Content-SHA256", prepared_request.headers)
+        mock_session.get_frozen_credentials.assert_called_once()
+
+
 class TestRequestsConnection(TestCase):
     def _get_mock_connection(
         self, connection_params={}, status_code=200, response_body=b"{}"