From 1d7019dfd9511d1a32f0a35bfa36c50b044e92f7 Mon Sep 17 00:00:00 2001
From: julioloayzam <julio@loayzameneses.com>
Date: Fri, 2 Aug 2024 15:27:32 +0200
Subject: [PATCH] fix ECDSA uncompressed pubkeys

As described in #5, crypto-condor passes (hex) strings to the ECDSA
implementation when using PubKeyEncoding.UNCOMPRESSED instead of bytes,
which is not the intended behaviour. This also causes a crash when
saving the results with debug data, as the __str__ method of SigData
tries to convert the key to a hex string with bytes' hex() method.

To solve this, the hex string is converted to bytes, to match the
expected type. A case is added to test_ECDSA.py::test_verify to confirm
that the correct type is provided to the implementation.

Additionally, test_verify is now aware of whether NIST or Wycheproof
vectors should have been used for testing depending on the elliptic
curve and hash function.

Closes #5
---
 crypto_condor/primitives/ECDSA.py |  2 +-
 tests/primitives/test_ECDSA.py    | 64 +++++++++++++++++++++++--------
 2 files changed, 50 insertions(+), 16 deletions(-)

diff --git a/crypto_condor/primitives/ECDSA.py b/crypto_condor/primitives/ECDSA.py
index 5a6003a..9901d64 100644
--- a/crypto_condor/primitives/ECDSA.py
+++ b/crypto_condor/primitives/ECDSA.py
@@ -650,7 +650,7 @@ def _test_verify_wycheproof(
             case PubKeyEncoding.PEM:
                 key = group["keyPem"].encode()
             case PubKeyEncoding.UNCOMPRESSED:
-                key = group["key"]["uncompressed"]
+                key = bytes.fromhex(group["key"]["uncompressed"])
         for test in group["tests"]:
             tid = test["tcId"]
             test_type = TestType(test["result"])
diff --git a/tests/primitives/test_ECDSA.py b/tests/primitives/test_ECDSA.py
index 9cf725f..f59fb59 100644
--- a/tests/primitives/test_ECDSA.py
+++ b/tests/primitives/test_ECDSA.py
@@ -3,6 +3,7 @@
 from pathlib import Path
 
 import pytest
+from cryptography.exceptions import InvalidSignature
 from cryptography.hazmat.primitives import serialization
 from cryptography.hazmat.primitives.asymmetric import ec, utils
 
@@ -77,17 +78,24 @@ def test_verify_file(curve: ECDSA.Curve, hash_function: ECDSA.Hash, tmp_path: Pa
 
 
 @pytest.mark.parametrize(
-    "curve_name, hash_name",
+    "curve_name, hash_name, nist, wycheproof",
     [
-        ("secp192r1", "sha256"),
-        ("secp224r1", "sha256"),
-        ("secp256r1", "sha256"),
-        ("secp256k1", "sha256"),
-        ("brainpoolP256r1", "sha256"),
+        ("secp192r1", "sha256", True, True),
+        ("secp224r1", "sha256", True, True),
+        ("secp256r1", "sha256", True, True),
+        ("secp256k1", "sha256", False, True),
+        ("brainpoolP256r1", "sha256", False, True),
     ],
 )
-def test_verify(curve_name: str, hash_name: str):
-    """Tests :meth:`crypto_condor.primitives.ECDSA.test_verify`."""
+def test_verify(curve_name: str, hash_name: str, nist: bool, wycheproof: bool):
+    """Tests :meth:`crypto_condor.primitives.ECDSA.test_verify`.
+
+    Args:
+        curve_name: Name of the curve to test.
+        hash_name: Name of the hash function to test.
+        nist: Whether results from NIST test vectors are expected.
+        wycheproof: Whether results from Wycheproof test vectors are expected.
+    """
     curve = ECDSA.Curve.from_name(curve_name)
     hash_function = ECDSA.Hash.from_name(hash_name)
 
@@ -95,13 +103,39 @@ def verify(public_key: bytes, message: bytes, signature: bytes) -> bool:
         """Lower-order function to wrap :class:`ECDSA`'s `verify`."""
         return ECDSA._verify(public_key, hash_function, message, signature)
 
-    group = ECDSA.test_verify(verify, curve, hash_function, ECDSA.PubKeyEncoding.DER)
-    if group.get("nist", None) is not None:
-        console.print_results(group["nist"])
-        assert group["nist"].check()
-    if group.get("wycheproof", None) is not None:
-        console.print_results(group["wycheproof"])
-        assert group["wycheproof"].check()
+    def _verify_uncompressed(
+        public_key: bytes, message: bytes, signature: bytes
+    ) -> bool:
+        pk = ec.EllipticCurvePublicKey.from_encoded_point(
+            curve.get_curve_instance(), public_key
+        )
+        try:
+            pk.verify(signature, message, ec.ECDSA(hash_function.get_hash_instance()))
+            return True
+        except InvalidSignature:
+            return False
+
+    results = ECDSA.test_verify(verify, curve, hash_function, ECDSA.PubKeyEncoding.DER)
+    if nist:
+        assert results.get("nist", None) is not None
+        console.print_results(results["nist"])
+        assert results["nist"].check()
+    if wycheproof:
+        assert results.get("wycheproof", None) is not None
+        console.print_results(results["wycheproof"])
+        assert results["wycheproof"].check()
+
+    results = ECDSA.test_verify(
+        _verify_uncompressed, curve, hash_function, ECDSA.PubKeyEncoding.UNCOMPRESSED
+    )
+    if nist:
+        assert results.get("nist", None) is not None
+        console.print_results(results["nist"])
+        assert results["nist"].check()
+    if wycheproof:
+        assert results.get("wycheproof", None) is not None
+        console.print_results(results["wycheproof"])
+        assert results["wycheproof"].check()
 
 
 @pytest.mark.parametrize(