apple / swift-crypto

Open-source implementation of a substantial portion of the API of Apple CryptoKit suitable for use on Linux platforms.
https://apple.github.io/swift-crypto
Apache License 2.0
1.47k stars 165 forks source link

Add five new Wycheproof ECDSA test vectors using IEEE P1363 format #167

Closed Sajjon closed 1 year ago

Sajjon commented 1 year ago

This PR is branched from source branch of https://github.com/apple/swift-crypto/pull/166, so please do review and merge that first.

Add five new Wychyeproof ECDSA test vectors using IEEE P1363 format

Checklist

If you've made changes to gyb files

Motivation:

We ought to use as many and as up to date test vectors as possible. Wycheproofs v0 were created 4 years ago and were due some upgrade.

Modifications:

  1. Removed 4 unused test vectors: ecdh_test, ecdh_webcrypto_test, ecdsa_test, ecdsa_webcrypto_test
  2. Added 5 new test vectors: ecdsa_secp256r1_sha256_p1363_test, ecdsa_secp256r1_sha512_p1363_test, ecdsa_secp384r1_sha384_p1363_test, ecdsa_secp384r1_sha512_p1363_test, ecdsa_secp521r1_sha512_p1363_test.
  3. Update ECDSASignatureTests renaming testWycheProofEdDSA -> testWycheproofEdDSA, testWycheProofP256 -> testWycheproofP256DER and analgously for P384 and P521.
  4. Update ECDSASignatureTests with three new unit test functions testWycheproofP256P1363 testWycheproofP384P1363 testWycheproofP521P1363

Result:

(252 + 322 + 270 + 308 + 308 =) 1460 new tests are being run for ECDSA.

Lukasa commented 1 year ago

Hrm, this will need to be cleaned up to merge nicely on the patch we just landed. I'm inclined, in general, to try not to add new test files here: I think keeping them merged together is reasonable.

Sajjon commented 1 year ago

@Lukasa ok I will rework the PR a bit and try first not adding any init test swift files

What about my removal of the 4 unused test vectors, are you OK with that?

Sajjon commented 1 year ago

@Lukasa I've reverted the unit test file changes, and updated the description in the PR to reflect the modifications => ready for re-review.

Lukasa commented 1 year ago

@swift-server-bot test this please