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 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"{}"