From 8320432440d7dc8a0d9da16a60e005ef8650d112 Mon Sep 17 00:00:00 2001 From: JulioLoayzaM Date: Thu, 22 Aug 2024 09:45:20 +0200 Subject: [PATCH] fix(ECDH): wrong test attribute in Wycheproof test For context, testing an ECDH implementation with Wycheproof vectors led to an AttributeError on line 420 that used test.secret instead of test.private. Fixing this typo uncovered two additional bugs in test_exchange_wycheproof regarding the correct interpretation of the test result. This showed a problem with the test that should cover that function. The example wrapper used P-192, but Wycheproof does not provide vectors for that curve. And a bug in run_wrapper_python meant that Wycheproof vectors were disabled anyway. As such, the wrapper example now uses P-256 and the bug is fixed, so the function is properly covered and verifies that the fixes applied are working. Co-authored-by: Baptistin BOILOT Issue: #11 --- crypto_condor/primitives/ECDH.py | 11 +++++++---- .../wrappers/ECDH/Python-examples/1/ECDH_wrapper.py | 8 ++++---- tests/cli/test_run.py | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/crypto_condor/primitives/ECDH.py b/crypto_condor/primitives/ECDH.py index 03811b5..10fa356 100644 --- a/crypto_condor/primitives/ECDH.py +++ b/crypto_condor/primitives/ECDH.py @@ -417,7 +417,7 @@ def test_exchange_wycheproof(ecdh: ECDH, curve: Curve) -> Results | None: for test in group.tests: test_type = TestType(test.result) info = TestInfo.new(test.id, test_type, test.flags, test.comment) - secret = int.from_bytes(test.secret, "big") + secret = int.from_bytes(test.private, "big") data = EcdhWycheproofData(secret, test.public, test.shared) try: shared = ecdh.exchange_wycheproof(secret, test.public) @@ -425,7 +425,10 @@ def test_exchange_wycheproof(ecdh: ECDH, curve: Curve) -> Results | None: logger.info("Method not implemented, test skipped") return None except Exception as error: - info.fail(f"Exchange failed: {str(error)}", data) + if test_type == TestType.VALID: + info.fail(f"Exchange failed: {str(error)}", data) + else: + info.ok(data) logger.debug("Exception raised by exchange_wycheproof", exc_info=True) results.add(info) continue @@ -433,7 +436,7 @@ def test_exchange_wycheproof(ecdh: ECDH, curve: Curve) -> Results | None: res = shared == test.shared data.result = shared match (test_type, res): - case (TestType.VALID, True, TestType.INVALID, False): + case (TestType.VALID, True) | (TestType.INVALID, False): info.ok(data) case (TestType.VALID, False): info.fail("Implementation returned wrong shared secret", data) @@ -569,7 +572,7 @@ def run_wrapper_python( logger.debug("Reloading ECDH wrapper module %s", wrapper.stem) ecdh_wrapper = importlib.reload(ecdh_wrapper) ecdh = ecdh_wrapper.CC_ECDH() - return test_exchange(ecdh, curve) + return test_exchange(ecdh, curve, compliance=compliance, resilience=resilience) def run_wrapper( diff --git a/crypto_condor/resources/wrappers/ECDH/Python-examples/1/ECDH_wrapper.py b/crypto_condor/resources/wrappers/ECDH/Python-examples/1/ECDH_wrapper.py index c565a61..84fd5e2 100644 --- a/crypto_condor/resources/wrappers/ECDH/Python-examples/1/ECDH_wrapper.py +++ b/crypto_condor/resources/wrappers/ECDH/Python-examples/1/ECDH_wrapper.py @@ -1,4 +1,4 @@ -"""Wrapper example 1: PyCryptodome ECDH over P-192. +"""Wrapper example 1: PyCryptodome ECDH over P-256. Usage: This example implements both methods of the CC_ECDH class to test the PyCryptodome @@ -19,9 +19,9 @@ def exchange_nist( ) -> bytes: """ECDH exchange with NIST vectors.""" # We can directly create the public key from the coordinates. - pub = ECC.construct(curve="P-192", point_x=pub_x, point_y=pub_y) + pub = ECC.construct(curve="P-256", point_x=pub_x, point_y=pub_y) # Same goes for the private key and the private value. - priv = ECC.construct(curve="P-192", d=secret) + priv = ECC.construct(curve="P-256", d=secret) # We run the key agreement with a KDF (key derivation function) that does # nothing, as crypto-condor expects the "raw" shared secret. shared = DH.key_agreement(static_priv=priv, static_pub=pub, kdf=lambda x: x) @@ -33,7 +33,7 @@ def exchange_wycheproof(self, secret: int, pub_key: bytes) -> bytes: # included in the key itself. pub = ECC.import_key(pub_key) # No difference for the private key. - priv = ECC.construct(curve="P-192", d=secret) + priv = ECC.construct(curve="P-256", d=secret) # Also no change for the exchange. shared = DH.key_agreement(static_priv=priv, static_pub=pub, kdf=lambda x: x) return shared diff --git a/tests/cli/test_run.py b/tests/cli/test_run.py index 3265245..5b077c1 100644 --- a/tests/cli/test_run.py +++ b/tests/cli/test_run.py @@ -492,7 +492,7 @@ class TestECDH: @pytest.mark.parametrize( ("lang", "example", "curve"), - [("Python", "1", "P-192"), ("Python", "2", "P-192")], + [("Python", "1", "P-256"), ("Python", "2", "P-192")], ) def test_examples(self, lang: str, example: str, curve: str, tmp_path: Path): """Tests ECDH examples."""