[3.0] Honor OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT as set and default to UNCOMPRESSED #19901
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…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. Expand documentation about `OSSL_PKEY_PARAM_PUB_KEY` and mention the change in behavior for our providers. Fixes openssl#16595 (cherry picked from commit 926db47)
…fault to UNCOMPRESSED
(cherry picked from commit d656efb)
romen
added
approval: review pending
github-actions
bot
added
the
severity: fips change
|
Notice that once this PR is merged, the changes in a9c2d1e will be applied to Presumably, Do we consider that a problem? |
|
I do not think this is a problem for 3.1 beta1. It would be better resolved before the 3.1.0 final though. |
|
IMO 3.0.8 should be done before 3.1.0 final. |
t8m
approved these changes
Dec 14, 2022
t8m
removed
the
approval: otc review pending
hlandau
approved these changes
Dec 14, 2022
hlandau
added
approval: done
openssl-machine
added
approval: ready to merge
|
This pull request is ready to merge |
This was referenced Dec 21, 2022
|
Merged to 3.0 branch. Thank you. @romen Could you please create a fixup PR against master branch to correct the documentation and resync the CHANGES.md? |
openssl-machine
pushed a commit
that referenced
this pull request
Dec 22, 2022
…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 #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 #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. Expand documentation about `OSSL_PKEY_PARAM_PUB_KEY` and mention the change in behavior for our providers. Fixes #16595 (cherry picked from commit 926db47) Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #19901)
romen
added a commit
to romen/openssl
that referenced
this pull request
Jan 6, 2023
…_CONVERSION_FORMAT openssl#19901 backported the "Honor OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT as set and default to UNCOMPRESSED" changeset to 3.0. This commit updates: - the HISTORY notes of the relevant documentation to mark the change happened since 3.0.8.
Closed
2 tasks
openssl-machine
pushed a commit
that referenced
this pull request
Jan 31, 2023
…_CONVERSION_FORMAT #19901 backported the "Honor OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT as set and default to UNCOMPRESSED" changeset to 3.0. This commit updates: - the HISTORY notes of the relevant documentation to mark the change happened since 3.0.8. - the `CHANGES.md file` to sync up with the tip of the `openssl-3.0` branch Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #20003) (cherry picked from commit f66c127)
openssl-machine
pushed a commit
that referenced
this pull request
Jan 31, 2023
…_CONVERSION_FORMAT #19901 backported the "Honor OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT as set and default to UNCOMPRESSED" changeset to 3.0. This commit updates: - the HISTORY notes of the relevant documentation to mark the change happened since 3.0.8. - the `CHANGES.md file` to sync up with the tip of the `openssl-3.0` branch Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #20003)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a backport of #19681 to
openssl-3.0as per openssl/technical-policies#60.CHANGES.mdrequired manual intervention, as expected.Fixes #16595
Checklist