Skip to content

extract EC group from private key for explicit curves #9729

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RoadRunnr
Copy link
Contributor

Going though the curve NID or name only works for standardized curves. However, explicit curves can use parameters that that deviate from the named curves. The public key generation would in that case fail to extract the group parameters from the key.

Instead extract all parameters explicitly and build the group from that.

fixes #9723

Going though the curve NID or name only works for standardized
curves. However, explicit curves can use parameters that that deviate
from the named curves. The public key generation would in that case
fail to extract the group parameters from the key.

Instead extract all parameters explicitly and build the group from
that.

fixes erlang#9723
Copy link
Contributor

github-actions bot commented Apr 15, 2025

CT Test Results

  2 files   14 suites   4m 44s ⏱️
190 tests 174 ✅  16 💤 0 ❌
473 runs  331 ✅ 142 💤 0 ❌

Results for commit 8df6f90.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@u3s u3s added the team:VM Assigned to OTP team VM label Apr 15, 2025
@sverker sverker added fix testing currently being tested, tag is used by OTP internal CI labels Apr 16, 2025
@sverker
Copy link
Contributor

sverker commented Apr 29, 2025

The new test case ecdh_generate_explicit_curve fails on Fedora Linux.

They seem to have their own patched OpenSSL. When drilling down I found this in OpenSSL source file ec_lib.c:

 if (named_group == group) {
        if (EC_GROUP_check_named_curve(group, 0, NULL) == NID_undef) {
            ERR_raise(ERR_LIB_EC, EC_R_UNKNOWN_GROUP);
            goto err;
        }
#if 0
        /*
         * If we did not find a named group then the encoding should be explicit
         * if it was specified
         */
        ptmp = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_EC_ENCODING);
        if (ptmp != NULL
            && !ossl_ec_encoding_param2id(ptmp, &encoding_flag)) {
            ERR_raise(ERR_LIB_EC, EC_R_INVALID_ENCODING);
            goto err;
        }
        if (encoding_flag == OPENSSL_EC_NAMED_CURVE) {
            ERR_raise(ERR_LIB_EC, EC_R_INVALID_ENCODING);
            goto err;
        }
        EC_GROUP_set_asn1_flag(group, OPENSSL_EC_EXPLICIT_CURVE);
#endif

The error on NID_undef and the following removal of code with #if 0 is not part of the original OpenSSL version of the code.

This looks to me like they have disabled the use of unnamed curves.
@RoadRunnr Do you know anything about this? Should we just skip the test case if OS is Fedora?

@RoadRunnr
Copy link
Contributor Author

RoadRunnr commented Apr 30, 2025

@sverker it seems that RedHat decided that their OpenSSL version always operates in FIPS mode and that mode forbids explicit curves with parameters that don't match well known, named curves.

The patch that breaks it points to https://bugzilla.redhat.com/show_bug.cgi?id=2066412 for an explanation. That URL is hidden behind logins and I seems normal RH or Fedora accounts can not access it. So I can't know why this is broken.

Maybe you have some corporate RH account that has access?

I found a reference to the Bug in here https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/9/html/9.1_release_notes/bug_fixes#bug-fix_security, it says:

Previously, the checks of explicit curve parameters safety were incomplete. As a consequence, arbitrary elliptic curves with sufficiently large p values worked in RHEL. With this update, the checks now verify that the explicit curve parameters match one of the well-known supported curves. As a result, the option to specify arbitrary curves through the use of explicit curve parameters has been removed from OpenSSL. Parameter files, private keys, public keys, and certificates that specify arbitrary explicit curves no longer work in OpenSSL. Using explicit curve parameters to specify one of the well known and supported curves such as P-224, P-256, P-384, P-521, and secp256k1 remains supported in non-FIPS mode.

In any case, this leaves two options for OTP:

  1. add a short note to the documentation that only well known (named) curves are supported
  2. add a note about explicit curves not working in RedHat, Fedora and derived distributions.

As much as I hate it, option 1. is probably the simplest for OTP as it is highly unlikely that anyone else has a use case for explicit curves.

@dgud
Copy link
Contributor

dgud commented Apr 30, 2025

When we looked at adding newer ASN.1 specs in public_key we noticed that they are removed from the (newer, but still old) ASN.1 specs.
I.E. non named ec curves have been removed from the specs a long time ago.

@sverker sverker removed the testing currently being tested, tag is used by OTP internal CI label Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix team:VM Assigned to OTP team VM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

crypto:generate_key/3 on explicit curves is broken
4 participants