add parsing of ECC group from ECC parameters

[falko-strenzke]

Tasks:

• integrate new test vectors into new test in the test_ecdh.cpp

[randombit]

Change looks fine, needs some test cases though

[randombit]

This is not an appropriate file for this test - test_pubkey.cpp is test infrastructure for public key algorithms

For example it would be appropriate in this file to add PK_Private_Key_Decoding_Test base class and then deriving it somewhere for your actual test.

Considering the test is already hard coded to ECDH keys might as well be in test_ecdh.cpp I suppose.

[randombit]

Actually we already have exactly such a generic test - PK_Key_Decoding_Test which doesn't even need to be derived from it just reads from src/tests/data/pubkey/key_encoding.vec

The only issue is that right now this test only handles valid keys and not invalid. This should be easy to extend by adding a [Valid] header for the current test data plus [Invalid] which inverts the success/fail logic.

[falko-strenzke]

Actually we already have exactly such a generic test - PK_Key_Decoding_Test which doesn't even need to be derived from it just reads from src/tests/data/pubkey/key_encoding.vec

The existing test is different. It decodes a complete private key info structure, that is not working for the intended test. I will thus create a new one in the test_ecdh.cpp.

[falko-strenzke]

@randombit
I think I addressed all your review comments now. Could you check it again?

[falko-strenzke]

@randombit
I am further wondering about the way that

EC_PrivateKey::EC_PrivateKey(const AlgorithmIdentifier& alg_id,
                             std::span<const uint8_t> key_bits,
                             bool with_modular_inverse)

handles the alg_id: it never looks at the OID, only checks for the presence of parameters. A caller that calls the function with a specific algorithm identifier might expect the function to check that alg_id actually denotes a valid ECC key. Should we add a check for alg_id indicating either an ECDSA or an ECDH key?

I also think that to make the interface clearer, passing alg_id should be optional. Getting the group from the encoded key might be what the caller intends.

[randombit]

Should we add a check for alg_id indicating either an ECDSA or an ECDH key?

Not helpful since this is generic across algorithms this could also be SM2, ECKCDSA, GOST, ...