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
17 stars 3 forks source link

Fix ECDSA test_sign that would fail verifying the signature when using pre_hashed option #4

Closed bboilot-ledger closed 3 months ago

bboilot-ledger commented 3 months ago

Hi :wave:

When using the pre_hashed option on ECDSA.test_sign, the message replaced by its hash. But following the call to the sign harness, the results are then given to crypto-condor _verify function. However, the message now contains the hash of the message which will make the call to _verify to return that the signature is invalid in every case. This PR fixes this issue by keeping the complete message even when the pre_hashed option is enabled.

I've also added the missing return value in the documentation for the ECDSA sign protocol.

Let me know if you want me to fix it differently or add anything to this.

JulioLoayzaM commented 3 months ago

Hi!

Thanks for the PR :) I think my intention was to add a prehashed option to _sign like for _verify but forgot to do so, and there isn't a test for signing with pre-hashing. :P

The fix looks good to me, I'd just prefer to keep the hashed-or-not message instead of raw_message in SigData if you can. That way the user gets the actual input given to their function in the debug information of the report.

And I'll add a unit test for this case after merging.

bboilot-ledger commented 3 months ago

It makes sense! I've made the modifications according to your feedback :smiley:

JulioLoayzaM commented 3 months ago

Thanks!