openssl / openssl

TLS/SSL and crypto library
https://www.openssl.org
Apache License 2.0
25.79k stars 10.13k forks source link

Initialize EC key using EC_PUB_X and EC_PUB_Y #16270

Open nikic opened 3 years ago

nikic commented 3 years ago

Currently, it's not possible to initialize an EC key using the PARAM_GROUP_NAME, PARAM_EC_PUB_X and PARAM_EC_PUB_Y parameters. EC_PUB_X/EC_PUB_Y only serve as getters. Instead PARAM_PUB_KEY needs to be used.

Going through PARAM_PUB_KEY is not particularly hard, but I feel like this breaks the abstraction of the param API a bit. Rather than specifying a group name and x&y, we need to drop down to lower-level APIs to construct an EC_GROUP from name, initialize an EC_POINT and then export it as an octet string.

Would it make sense to accept EC_PUB_X/EC_PUB_Y as an alternative construction method?

slontis commented 3 years ago

@romen may have some thoughts on this ...

romen commented 3 years ago

Yes!

There is no need to drop to a lower level API. It is possible to use PARAM_PUB_KEY passing one of the standardized formats in the SECG SEC1 standard (section 2.3.3). In particular if for some reason the user has separate x and y affine coordinates in uncompressed format, they can pass a buffer filled with 0x04 ¦¦ x ¦¦ y (assuming x and y are already formatted as padded octet strings as specified in the same document). Better still, when it is an option, the user can avoid extrapolating x and y altogether and just use the encoded point format when going from one format to another.

The reasons why I advocate for this is that the SECG standard precisely defines the expected format without room for ambiguity, it is generally portable across different libraries and frameworks, and supports compressed representations.

In contrast, adding our own x and y import format to the mix does not significantly improve usability, would add complexity to provider implementations that would have to handle more complex logic and more input formats, and does not help the OpenSSL project in keeping bloat in check.

Last time I checked this was a moderate inconvenience for people that have to deal with documents providing points as x and y coordinates (usually without even properly specifying how x and y are encoded). In particular, IIRC, FIPS documents with known answer tests and some JSON encodings. Arguably, the underdefined format is a defect of those documents, and should be fixed there rather than bloat the ecosystem in name of ill-perceived "usability".

AFAIK, other libraries generally offer the capability of extracting points in SECG format programmatically, and generally do default to the same format when saving public keys to files.

Last, but not least, there are security arguments against supporting our own custom x and y format in input rather than the standardized SECG format: in most cases input public points are under the control of a potential attacker. Supporting only the SECG format reduces the attack surface and the complexity, and makes it easier to harden provider implementations against attacks.

rwfranks commented 3 years ago

This is an excellent suggestion.

The vision of EVP, originally at least, was to provide an API framework which was largely independent of the crypto algorithm itself.

With 3.0.0, this has been achieved for DSA, RSA, ED25519, and ED448. These keys are constructed from (name,value) pairs composed from the same material that was used for the legacy APIs.

This proposal would bring ECDSA into line with these other algorithms and provide a smooth migration path from the legacy API to the EVP framework. External packing of otherwise perfectly good X and Y values into a buffer seems to me a step back from the lofty ideals of EVP, and an unnecessary obstacle for users keen to get rid of deprecated APIs.

rwfranks commented 3 years ago

May I nominate this as a useful non-breaking small feature to be merged into the master branch

abkarcher commented 2 years ago

I'd like to add my support for this functionality here. Combined with #17590, which will automatically change any oversized coordinates that are fed in, it means performing ACVP testing is pretty rough as it includes tests which have out of bounds values. I am trying to generate the octet string as @romen mentioned, hopefully this will bypass EC_POINTs odd functionality.

That being said, there are security implications. Ideally, there would be some straight forward way to verify a given qX/qY without having to worry about OpenSSL modifying it.

Thanks, Andrew

rwfranks commented 1 year ago

Please can this be attached to, with a firm commitment to deliver by, a specific milestone (preferably 3.2.0). The present drift and lack of proper planning will result in some very public failures when we crash into 4.0.0 and all the deprecated APIs disappear.