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

Some EC_Group usage cleanups #4038

Merged
merged 5 commits into from
May 24, 2024
Merged

Some EC_Group usage cleanups #4038

merged 5 commits into from
May 24, 2024

Conversation

randombit
Copy link
Owner

This hides all symbols of CurveGFp and CurveGFp_Repr (which were already marked as BOTAN_UNSTABLE_API and intentionally undocumented) and makes them accessible only to a few specific friends.

For EC_Point deprecate a number of functions only needed for internal use. Also add a couple of functions to simplify a future bridge between our current APIs and pcurves.

In EC_Group likewise begin some cleanup and deprecation.

@randombit randombit requested a review from reneme May 2, 2024 12:45
@randombit randombit force-pushed the jack/ec-group-cleanups branch 3 times, most recently from 6ce128f to abe0e42 Compare May 2, 2024 21:59
@coveralls
Copy link

coveralls commented May 2, 2024

Coverage Status

coverage: 91.83% (-0.007%) from 91.837%
when pulling fe2ff50 on jack/ec-group-cleanups
into 4f6b4ba on master.

@randombit randombit force-pushed the jack/ec-group-cleanups branch 4 times, most recently from a9e67b0 to 44d543e Compare May 5, 2024 19:24
@randombit randombit force-pushed the jack/ec-group-cleanups branch 5 times, most recently from f87e076 to c648a60 Compare May 19, 2024 02:29
@randombit randombit added this to the Botan 3.5.0 milestone May 22, 2024
Copy link
Collaborator

@reneme reneme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me in general, modulo a few nits and suggestions.

src/lib/pubkey/ec_group/ec_group.h Outdated Show resolved Hide resolved
src/lib/pubkey/ec_group/ec_group.h Outdated Show resolved Hide resolved
Comment on lines 127 to +134
/**
* Create an EC domain from PEM encoding (as from PEM_encode), or
* from an OID name (eg "secp256r1", or "1.2.840.10045.3.1.7")
* @param pem_or_oid PEM-encoded data, or an OID

*
* @warning Support for PEM in this function is deprecated. Use
* EC_Group_from_PEM
* EC_Group::from_PEM or EC_Group::from_OID or EC_Group::from_name
*/
explicit EC_Group(std::string_view pem_or_oid);
BOTAN_DEPRECATED("Use EC_Group::from_{name,OID,PEM}") explicit EC_Group(std::string_view pem_or_oid);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Full ack on splitting this initialization functionality into several factory methods. Less magic!

src/lib/pubkey/ec_group/ec_group.h Outdated Show resolved Hide resolved
src/lib/pubkey/ec_group/ec_group.cpp Outdated Show resolved Hide resolved
src/lib/pubkey/ec_group/ec_point.h Show resolved Hide resolved
src/lib/pubkey/ec_group/curve_gfp.h Outdated Show resolved Hide resolved
src/lib/pubkey/ecc_key/ecc_key.cpp Outdated Show resolved Hide resolved
src/lib/pubkey/eckcdsa/eckcdsa.cpp Outdated Show resolved Hide resolved
src/lib/pubkey/eckcdsa/eckcdsa.cpp Outdated Show resolved Hide resolved
randombit and others added 4 commits May 24, 2024 14:02
This hides all symbols of CurveGFp and CurveGFp_Repr (which were
already marked as BOTAN_UNSTABLE_API and intentionally undocumented)
and makes them accessible only to a few specific friends.

For EC_Point deprecate a number of functions only needed for internal
use. Also add a couple of functions to simplify a future bridge
between our current APIs and pcurves.

In EC_Group likewise begin some cleanup and deprecation.
Unlike the existing (deprecated) constructor, this constructor
requires an OID, does not accept a cofactor, limits the field to at
most 521 bits, and performs basic validation of the arguments.
@randombit randombit force-pushed the jack/ec-group-cleanups branch from 8039917 to af9b85f Compare May 24, 2024 18:02
@randombit randombit force-pushed the jack/ec-group-cleanups branch from af9b85f to fe2ff50 Compare May 24, 2024 18:10
@randombit randombit merged commit 9bca3f7 into master May 24, 2024
43 checks passed
@randombit randombit deleted the jack/ec-group-cleanups branch May 24, 2024 19:52
randombit added a commit that referenced this pull request Jun 15, 2024
Interface changes/deprecations were introduced in #4056 and #4038,
bump the module version to make it easier for users to handle this
if they want to.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants