otp icon indicating copy to clipboard operation
otp copied to clipboard

extract EC group from private key for explicit curves

Open RoadRunnr opened this issue 8 months ago • 4 comments

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

RoadRunnr avatar Apr 15 '25 08:04 RoadRunnr

CT Test Results

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

Results for commit 8df6f90e.

:recycle: 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

github-actions[bot] avatar Apr 15 '25 08:04 github-actions[bot]

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?

sverker avatar Apr 29 '25 12:04 sverker

@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.

RoadRunnr avatar Apr 30 '25 06:04 RoadRunnr

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.

dgud avatar Apr 30 '25 06:04 dgud