Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[master] Honor OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT as set and default to UNCOMPRESSED #19781

Closed

Conversation

romen
Copy link
Member

@romen romen commented Nov 28, 2022

This ports #19681 to master.

They differ exclusively for the CHANGES.md (same entry text, different position): in master we describe directly the differences from 3.0 to 3.2, skipping 3.1 altogether.

Fixes #16595

Checklist
  • documentation is added or updated
  • tests are added or updated

…o UNCOMPRESSED

Originally the code to im/export the EC pubkey was meant to be consumed
only by the im/export functions when crossing the provider boundary.
Having our providers exporting to a COMPRESSED format octet string made
sense to avoid memory waste, as it wasn't exposed outside the provider
API, and providers had all tools available to convert across the three
formats.

Later on, with openssl#13139 deprecating the `EC_KEY_*` functions, more state
was added among the params imported/exported on an EC provider-native
key (including `OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT`, although it
did not affect the format used to export `OSSL_PKEY_PARAM_PUB_KEY`).

Finally, in openssl#14800, `EVP_PKEY_todata()` was introduced and prominently
exposed directly to users outside the provider API, and the choice of
COMPRESSED over UNCOMPRESSED as the default became less sensible in
light of usability, given the latter is more often needed by
applications and protocols.

This commit fixes it, by using `EC_KEY_get_conv_form()` to get the
point format from the internal state (an `EC_KEY` under the hood) of the
provider-side object, and using it on
`EVP_PKEY_export()`/`EVP_PKEY_todata()` to format
`OSSL_PKEY_PARAM_PUB_KEY`.
The default for an `EC_KEY` was already UNCOMPRESSED, and it is altered
if the user sets `OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT` via
`EVP_PKEY_fromdata()`, `EVP_PKEY_set_params()`, or one of the
more specialized methods.

For symmetry, this commit also alters `ec_pkey_export_to()` in
`crypto/ec/ec_ameth.c`, part of the `EVP_PKEY_ASN1_METHOD` for legacy EC
keys: it exclusively used COMPRESSED format, and now it honors the
conversion format specified in the EC_KEY object being exported to a
provider when this function is called.

Fixes openssl#16595
…fault to UNCOMPRESSED

Expand documentation about `OSSL_PKEY_PARAM_PUB_KEY` and mention the
3.1 change in behavior for our providers.
@romen romen added branch: master Merge to master branch triaged: bug The issue/pr is/fixes a bug tests: present The PR has suitable tests present labels Nov 28, 2022
@romen romen requested review from hlandau and t8m November 28, 2022 15:09
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Nov 28, 2022
@t8m
Copy link
Member

t8m commented Nov 28, 2022

Once 3.1 is released we will update CHANGES.md in master to include the 3.1 changes entries. So this one will be removed from the 3.2 section. But that does not matter for now.

@t8m t8m added approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member labels Nov 28, 2022
@t8m t8m removed the approval: otc review pending This pull request needs review by an OTC member label Nov 28, 2022
@t8m
Copy link
Member

t8m commented Nov 29, 2022

The 3.1 PR cherry-picked cleanly to master so I am closing this.

@t8m t8m closed this Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: review pending This pull request needs review by a committer branch: master Merge to master branch severity: fips change The pull request changes FIPS provider sources tests: present The PR has suitable tests present triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EVP_PKEY_todata exporting compressed EC public key
2 participants