fix: use issubclass instead of isinstance in _set_signature_type (#800)

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.

Author: texastony

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
 

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)

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)

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
 ):