From 062e19acd717187fc759fe72dc3f2344b78894a3 Mon Sep 17 00:00:00 2001 From: texastony <5892063+texastony@users.noreply.github.com> Date: Wed, 22 Apr 2026 12:55:23 -0700 Subject: [PATCH] fix: use issubclass instead of isinstance in _set_signature_type Replace isinstance(signing_algorithm_info, type(ec.EllipticCurve)) with issubclass(signing_algorithm_info, ec.EllipticCurve) wrapped in try/except TypeError. The old check used isinstance against ABCMeta which was fragile and semantically incorrect for checking class hierarchy. Update tests to use real EC curve classes (ec.SECP256R1, ec.SECP384R1) instead of MagicMock with spec=ec.EllipticCurve, which masked the bug. Based on PR #794. --- .../internal/crypto/authentication.py | 5 +++- .../unit/test_crypto_authentication_signer.py | 29 ++++++++++--------- .../test_crypto_authentication_verifier.py | 17 ++++++----- .../test_crypto_prehashing_authenticator.py | 12 ++++++-- 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/src/aws_encryption_sdk/internal/crypto/authentication.py b/src/aws_encryption_sdk/internal/crypto/authentication.py index f59903b2a..f769d4a39 100644 --- a/src/aws_encryption_sdk/internal/crypto/authentication.py +++ b/src/aws_encryption_sdk/internal/crypto/authentication.py @@ -36,7 +36,10 @@ def __init__(self, algorithm, key): def _set_signature_type(self): """Ensures that the algorithm signature type is a known type and sets a reference value.""" - if not isinstance(self.algorithm.signing_algorithm_info, type(ec.EllipticCurve)): + try: + if not issubclass(self.algorithm.signing_algorithm_info, ec.EllipticCurve): + raise NotSupportedError("Unsupported signing algorithm info") + except TypeError: raise NotSupportedError("Unsupported signing algorithm info") return ec.EllipticCurve diff --git a/test/unit/test_crypto_authentication_signer.py b/test/unit/test_crypto_authentication_signer.py index 4d2623c62..c3520ba82 100644 --- a/test/unit/test_crypto_authentication_signer.py +++ b/test/unit/test_crypto_authentication_signer.py @@ -3,6 +3,7 @@ """Unit test suite for ``aws_encryption_sdk.internal.crypto.authentication.Signer``.""" import cryptography.hazmat.primitives.serialization import pytest +from cryptography.hazmat.primitives.asymmetric import ec from mock import MagicMock, patch, sentinel from pytest_mock import mocker # noqa pylint: disable=unused-import @@ -73,8 +74,8 @@ def test_GIVEN_no_encoding_WHEN_signer_from_key_bytes_THEN_load_der_private_key( patch_build_hasher, patch_ec ): - mock_algorithm_info = MagicMock(return_value=sentinel.algorithm_info, spec=patch_ec.EllipticCurve) - _algorithm = MagicMock(signing_algorithm_info=mock_algorithm_info) + patch_ec.EllipticCurve = ec.EllipticCurve + _algorithm = MagicMock(signing_algorithm_info=ec.SECP256R1) # Make a new patched serialization module for this test. # The default patch introduces serialization as `serialization.Encoding.DER` @@ -106,8 +107,8 @@ def test_GIVEN_PEM_encoding_WHEN_signer_from_key_bytes_THEN_load_pem_private_key patch_build_hasher, patch_ec ): - mock_algorithm_info = MagicMock(return_value=sentinel.algorithm_info, spec=patch_ec.EllipticCurve) - _algorithm = MagicMock(signing_algorithm_info=mock_algorithm_info) + patch_ec.EllipticCurve = ec.EllipticCurve + _algorithm = MagicMock(signing_algorithm_info=ec.SECP256R1) # When: from_key_bytes signer = Signer.from_key_bytes( @@ -132,8 +133,8 @@ def test_GIVEN_unrecognized_encoding_WHEN_signer_from_key_bytes_THEN_raise_Value patch_build_hasher, patch_ec ): - mock_algorithm_info = MagicMock(return_value=sentinel.algorithm_info, spec=patch_ec.EllipticCurve) - _algorithm = MagicMock(signing_algorithm_info=mock_algorithm_info) + patch_ec.EllipticCurve = ec.EllipticCurve + _algorithm = MagicMock(signing_algorithm_info=ec.SECP256R1) # Then: Raises ValueError with pytest.raises(ValueError): @@ -147,8 +148,8 @@ def test_GIVEN_unrecognized_encoding_WHEN_signer_from_key_bytes_THEN_raise_Value def test_signer_key_bytes(patch_default_backend, patch_serialization, patch_build_hasher, patch_ec): - mock_algorithm_info = MagicMock(return_value=sentinel.algorithm_info, spec=patch_ec.EllipticCurve) - algorithm = MagicMock(signing_algorithm_info=mock_algorithm_info) + patch_ec.EllipticCurve = ec.EllipticCurve + algorithm = MagicMock(signing_algorithm_info=ec.SECP256R1) private_key = MagicMock() signer = Signer(algorithm, key=private_key) @@ -174,8 +175,8 @@ def test_signer_encoded_public_key( patch_base64.b64encode.return_value = sentinel.encoded_point private_key = MagicMock() - mock_algorithm_info = MagicMock(return_value=sentinel.algorithm_info, spec=patch_ec.EllipticCurve) - algorithm = MagicMock(signing_algorithm_info=mock_algorithm_info) + patch_ec.EllipticCurve = ec.EllipticCurve + algorithm = MagicMock(signing_algorithm_info=ec.SECP256R1) signer = Signer(algorithm, key=private_key) test_key = signer.encoded_public_key() @@ -186,8 +187,8 @@ def test_signer_encoded_public_key( def test_signer_update(patch_default_backend, patch_serialization, patch_build_hasher, patch_ec): - mock_algorithm_info = MagicMock(return_value=sentinel.algorithm_info, spec=patch_ec.EllipticCurve) - algorithm = MagicMock(signing_algorithm_info=mock_algorithm_info) + patch_ec.EllipticCurve = ec.EllipticCurve + algorithm = MagicMock(signing_algorithm_info=ec.SECP256R1) signer = Signer(algorithm, key=MagicMock()) signer.update(sentinel.data) patch_build_hasher.return_value.update.assert_called_once_with(sentinel.data) @@ -196,8 +197,8 @@ def test_signer_update(patch_default_backend, patch_serialization, patch_build_h def test_signer_finalize( patch_default_backend, patch_serialization, patch_build_hasher, patch_ecc_static_length_signature, patch_ec ): - mock_algorithm_info = MagicMock(return_value=sentinel.algorithm_info, spec=patch_ec.EllipticCurve) - algorithm = MagicMock(signing_algorithm_info=mock_algorithm_info) + patch_ec.EllipticCurve = ec.EllipticCurve + algorithm = MagicMock(signing_algorithm_info=ec.SECP256R1) private_key = MagicMock() signer = Signer(algorithm, key=private_key) diff --git a/test/unit/test_crypto_authentication_verifier.py b/test/unit/test_crypto_authentication_verifier.py index 7562fa762..4b29aa3ab 100644 --- a/test/unit/test_crypto_authentication_verifier.py +++ b/test/unit/test_crypto_authentication_verifier.py @@ -2,6 +2,7 @@ # SPDX-License-Identifier: Apache-2.0 """Unit test suite for ``aws_encryption_sdk.internal.crypto.authentication.Verifier``.""" import pytest +from cryptography.hazmat.primitives.asymmetric import ec from mock import MagicMock, sentinel from pytest_mock import mocker # noqa pylint: disable=unused-import @@ -85,23 +86,23 @@ def test_verifier_from_encoded_point( mock_point_instance.public_key.return_value = sentinel.public_key patch_ecc_public_numbers_from_compressed_point.return_value = mock_point_instance patch_base64.b64decode.return_value = sentinel.compressed_point - mock_algorithm_info = MagicMock(return_value=sentinel.algorithm_info, spec=patch_ec.EllipticCurve) - mock_algorithm = MagicMock(signing_algorithm_info=mock_algorithm_info) + mock_algorithm = MagicMock(signing_algorithm_info=ec.SECP256R1) + patch_ec.EllipticCurve = ec.EllipticCurve verifier = Verifier.from_encoded_point(algorithm=mock_algorithm, encoded_point=sentinel.encoded_point) patch_base64.b64decode.assert_called_once_with(sentinel.encoded_point) - mock_algorithm.signing_algorithm_info.assert_called_once_with() - patch_ecc_public_numbers_from_compressed_point.assert_called_once_with( - curve=mock_algorithm.signing_algorithm_info.return_value, compressed_point=sentinel.compressed_point - ) + patch_ecc_public_numbers_from_compressed_point.assert_called_once() + call_kwargs = patch_ecc_public_numbers_from_compressed_point.call_args + assert isinstance(call_kwargs[1]["curve"], ec.SECP256R1) + assert call_kwargs[1]["compressed_point"] is sentinel.compressed_point mock_point_instance.public_key.assert_called_once_with(patch_default_backend.return_value) assert isinstance(verifier, Verifier) def test_verifier_update(patch_default_backend, patch_serialization, patch_build_hasher, patch_ec): - mock_algorithm_info = MagicMock(return_value=sentinel.algorithm_info, spec=patch_ec.EllipticCurve) - mock_algorithm = MagicMock(signing_algorithm_info=mock_algorithm_info) + patch_ec.EllipticCurve = ec.EllipticCurve + mock_algorithm = MagicMock(signing_algorithm_info=ec.SECP256R1) verifier = Verifier(algorithm=mock_algorithm, key=MagicMock()) verifier.update(sentinel.data) verifier._hasher.update.assert_called_once_with(sentinel.data) diff --git a/test/unit/test_crypto_prehashing_authenticator.py b/test/unit/test_crypto_prehashing_authenticator.py index 631391f53..79deb158d 100644 --- a/test/unit/test_crypto_prehashing_authenticator.py +++ b/test/unit/test_crypto_prehashing_authenticator.py @@ -2,6 +2,7 @@ # SPDX-License-Identifier: Apache-2.0 """Unit test suite for ``aws_encryption_sdk.internal.crypto._PrehashingAuthenticater``.""" import pytest +from cryptography.hazmat.primitives.asymmetric import ec from mock import MagicMock, sentinel from pytest_mock import mocker # noqa pylint: disable=unused-import @@ -56,13 +57,20 @@ def test_init(patch_set_signature_type, patch_build_hasher): def test_set_signature_type_elliptic_curve( patch_build_hasher, patch_cryptography_ec ): - mock_algorithm_info = MagicMock(return_value=sentinel.algorithm_info, spec=patch_cryptography_ec.EllipticCurve) - mock_algorithm = MagicMock(signing_algorithm_info=mock_algorithm_info) + patch_cryptography_ec.EllipticCurve = ec.EllipticCurve + mock_algorithm = MagicMock(signing_algorithm_info=ec.SECP256R1) test = _PrehashingAuthenticator(algorithm=mock_algorithm, key=sentinel.key) assert test._signature_type is patch_cryptography_ec.EllipticCurve +def test_set_signature_type_elliptic_curve_known_value(patch_build_hasher): + mock_algorithm = MagicMock(signing_algorithm_info=ec.SECP384R1) + test = _PrehashingAuthenticator(algorithm=mock_algorithm, key=sentinel.key) + + assert test._signature_type is ec.EllipticCurve + + def test_set_signature_type_unknown( patch_build_hasher, patch_cryptography_ec ):