search

quarkslab / crypto-condor

crypto-condor is a Python library for compliance testing of implementations of cryptographic primitives
https://quarkslab.github.io/crypto-condor/latest/index.html
Apache License 2.0
16 stars 3 forks source link

ECDSA verify test crashes when saving the results #5

Closed bboilot-ledger closed 3 months ago

bboilot-ledger commented 3 months ago

Hi :wave: I am having an issue with crypto-condor when using the verify test of the ECDSA primitive.

To reproduce:

Unexpected behavior:

The following stack trace is obtained:

Save the results to a file? [y/n] (n): y
Save debug data? (Increases file size considerably) [y/n] (n): y
Filename to save? (.txt extension added automatically) (cc_2024-07-30_17:51:17.txt): 
Writing debug data 1/1 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━   0% -:--:--
Traceback (most recent call last):
    Console().process_results(res)
  File "/home/.../venv/lib/python3.10/site-packages/crypto_condor/primitives/common.py", line 820, in process_results
    printer.print(str(data))
  File "/home/.../venv/lib/python3.10/site-packages/crypto_condor/primitives/ECDSA.py", line 197, in __str__
    s += f"key = {self.key.hex()}\n"
AttributeError: 'str' object has no attribute 'hex'

Also, the public key is sometimes given as bytes and sometime given as hex string to the test harness.

Possible cause:

During the run and depending on the curve/have combination, the public key may be given as bytes or as an hex string to the harness. The function that provide the parameters to the harness is also the one that create the resulting data used by Console().process_results(). They may be an issue related to the way the test vectors are processed. Given the tool documentation and code typing, the harness should only receive bytes, not hex string.

If I use one of these combination, I end up with bytes as intended:

However, if I use one of these combinations, I get an hex string:

I think that it may be related to the test vectors source because the pk actually seems to be given as bytes for NIST but as hex string for Wycheproof.

JulioLoayzaM commented 3 months ago

Hi,

Thanks for the detailed report! That's definitely a bug, especially if we're passing (hex) strings to the implementation instead of bytes. As noted it probably comes from the difference between how NIST and Wycheproof vectors are stored. Should be an easy fix. 🤞

JulioLoayzaM commented 3 months ago

Hi again,

As noted, the bug did come from our usage of Wycheproof test vectors: as they come in JSON files, we import them directly using the json module. This means that converting from string to bytes has to be done by the test function, and was the step missing for uncompressed public keys. I'll merge the fix from 1d7019d to main, which should close this issue, and create a new release.

Thanks again for the detailed report!

JulioLoayzaM commented 3 months ago

Last update, version 2024.08.03 is out with the fix applied. :)