RustCrypto / elliptic-curves

Collection of pure Rust elliptic curve implementations: NIST P-224, P-256, P-384, P-521, secp256k1, SM2
649 stars 178 forks source link

Additional wycheproof test cases for secp256k1 #1060

Open XuJiandong opened 1 month ago

XuJiandong commented 1 month ago

From the tool wycheproof2blb the test case for secp256k1 only includes ecdsa_secp256k1_sha256_test.json.

But in original wycheproof folder, there are many others:

❯ find . | grep secp256k1 | grep ecdsa
./testvectors_v1/ecdsa_secp256k1_sha3_512_test.json
./testvectors_v1/ecdsa_secp256k1_sha512_test.json
./testvectors_v1/ecdsa_secp256k1_shake256_p1363_test.json
./testvectors_v1/ecdsa_secp256k1_shake128_test.json
./testvectors_v1/ecdsa_secp256k1_sha256_bitcoin_test.json
./testvectors_v1/ecdsa_secp256k1_sha256_p1363_test.json
./testvectors_v1/ecdsa_secp256k1_shake256_test.json
./testvectors_v1/ecdsa_secp256k1_sha512_p1363_test.json
./testvectors_v1/ecdsa_secp256k1_sha3_256_test.json
./testvectors_v1/ecdsa_secp256k1_shake128_p1363_test.json
./testvectors_v1/ecdsa_secp256k1_sha256_test.json
./testvectors/ecdsa_secp256k1_sha3_512_test.json
./testvectors/ecdsa_secp256k1_sha512_test.json
./testvectors/ecdsa_secp256k1_sha256_p1363_test.json
./testvectors/ecdsa_secp256k1_sha512_p1363_test.json
./testvectors/ecdsa_secp256k1_sha3_256_test.json
./testvectors/ecdsa_secp256k1_sha256_test.json

Is it necessary to include all the test cases? Or can we prioritize and include only the most critical ones?

tarcieri commented 1 month ago

Those all look like ECDSA with unusual alternative hash functions.

If you would like to work towards including those, that's okay, but it seems like a lot of work and not something I'm particularly interested in working on. I also believe it would also transcend what is possible with our existing macros for writing Wycheproof tests.

XuJiandong commented 1 month ago

It seems the folder with "v1" is the new one. So we can simply focus on all test cases in testvectors_v1. After filtering out unusual hash function test cases, we have the following list:

Since the last one is already included, we can add the two extra cases to secp256k1. Is it worth doing? I can submit a PR for it.

P.S. What is "our existing macros"? Could you give a link to it?

tarcieri commented 1 month ago

Sure, that sounds good. The new_wycheproof_test! macro is defined here: https://github.com/RustCrypto/signatures/blob/ff9a205/ecdsa/src/dev.rs#L147

tarcieri commented 1 day ago

@XuJiandong is this complete then?