Add more ECDH test vectors

[mirceanis]

I'm trying to use this lib to do some P-256 key agreement and I seem to have stumbled upon a problem. The shared secret being generated is 33 bytes long for P256. This tells me that I'm getting the compressed form of the point. I looked at some test vectors I found in other places and saw that they are expecting 32 bytes (only the X coord). The fix for this would be easy.

BUT

Then I started looking at the test vectors being used here and at first glance everything seems to coincide with other public vectors, but upon running the tests I noticed that NONE of the relevant vectors are actually included in the tests for ECDH!

The "problem" seems to be this line where they are excluded based on encoding, but all the vectors are using the same encoding, so all get excluded.

[paulmillr]

1. There is NO standard for ECDH. 33-byte ECDH is acceptable. 32-byte ECDH is acceptable. Hashed ECDH is acceptable. Unhashed ECDH is acceptable. NIST uses one way. Bitcoin uses another way. Someone hashes result with sha256. Someone hashes with sha3.
2. We do 33-byte because it's not possible to fully restore point from 32-byte hex. 33-byte hex has y-parity as first byte, meaning it's possible to restore full public key. If you need 32-byte, do result.subarray(1)
3. We don't support ASN encoding. We barely support DER, just for encoding and decoding of signatures. The test vectors use ASN encoding for points, and since we don't support it, there is no way to test for them. If you would like to contribute ASN parsing for the tests, we can merge it.
4. We extensively test point operations and do fuzzing, everything has been correct so far.

[mirceanis]

I understand all your points and I'm not questioning the correctness of the raw implementation. I raised the issue because it may cause issues for other folks that need to work with other stacks that use the 32 byte output (as I had to). Since the result of that usually gets hashed before being used as a key it's going to be a pain to debug when things go wrong.

Still, having the test vectors there and not actually using them creates a false sense of security, especially for the "invalid" cases, which pass for the wrong reasons now. Sadly I don't have the bandwidth to attempt any ASN/DER parsing implementation. Would you accept a devDependency that does that parsing for the test suite?

[paulmillr]

Still, having the test vectors there and not actually using them creates a false sense of security, especially for the "invalid" cases, which pass for the wrong reasons now

I agree. For this reason I will do add a quick and dirty parsing right now. Simple tests will pass, other will get filtered out.

[paulmillr]

Done: a4abd8a. I see 1249 correctly executed test vectors.

[paulmillr]

Now 3515 tests with c525356. I think that's good enough.

[mirceanis]

amazing, thanks for looking into this!