Mbed-TLS / mbedtls

An open source, portable, easy to use, readable and flexible TLS library, and reference implementation of the PSA Cryptography API. Releases are on a varying cadence, typically around 3 - 6 months between releases.
https://www.trustedfirmware.org/projects/mbed-tls/
Other
5.53k stars 2.6k forks source link

mbedtls_ecp_curve_info_from_grp_id returns null for ids MBEDTLS_ECP_DP_CURVE448, MBEDTLS_ECP_DP_CURVE25519 #3445

Open mu578 opened 4 years ago

mu578 commented 4 years ago

Description


Bug

OS
Any

mbed TLS build:
Version: 2.16.6

Expected behavior

Not NULL;

Actual behavior

NULL

Steps to reproduce


const int curve_id[] =
{
      MBEDTLS_ECP_DP_SECP192R1  /*!< Domain parameters for the 192-bit curve defined by FIPS 186-4 and SEC1. */
    , MBEDTLS_ECP_DP_SECP224R1  /*!< Domain parameters for the 224-bit curve defined by FIPS 186-4 and SEC1. */
    , MBEDTLS_ECP_DP_SECP256R1  /*!< Domain parameters for the 256-bit curve defined by FIPS 186-4 and SEC1. */
    , MBEDTLS_ECP_DP_SECP384R1  /*!< Domain parameters for the 384-bit curve defined by FIPS 186-4 and SEC1. */
    , MBEDTLS_ECP_DP_SECP521R1  /*!< Domain parameters for the 521-bit curve defined by FIPS 186-4 and SEC1. */
    , MBEDTLS_ECP_DP_BP256R1    /*!< Domain parameters for 256-bit Brainpool curve. */
    , MBEDTLS_ECP_DP_BP384R1    /*!< Domain parameters for 384-bit Brainpool curve. */
    , MBEDTLS_ECP_DP_BP512R1    /*!< Domain parameters for 512-bit Brainpool curve. */
    , MBEDTLS_ECP_DP_CURVE25519 /*!< Domain parameters for Curve25519. */
    , MBEDTLS_ECP_DP_SECP192K1  /*!< Domain parameters for 192-bit "Koblitz" curve. */
    , MBEDTLS_ECP_DP_SECP224K1  /*!< Domain parameters for 224-bit "Koblitz" curve. */
    , MBEDTLS_ECP_DP_SECP256K1  /*!< Domain parameters for 256-bit "Koblitz" curve. */
    , MBEDTLS_ECP_DP_CURVE448   /*!< Domain parameters for Curve448. */
};

for (size_t k = 0; k < sizeof(curve_id) / sizeof(int); k++) {
    const mbedtls_ecp_curve_info * curve_info = mbedtls_ecp_curve_info_from_grp_id(curve_id[k]);
    if (NULL == curve_info) {
        fprintf(stderr, "+ curve_info is null for enabled curve_id: %d\n", curve_id[k]);
    }
}
gilles-peskine-arm commented 4 years ago

The missing ones are Curve25519 and Curve448. There's nothing wrong with secp192k1, is there?

Montgomery curves are omitted from mbedtls_ecp_curve_list() and mbedtls_ecp_grp_id_list(). The reason for that is that the library only has partial support for them, and it was designed with all or nothing in mind. Adding Montgomery curves would cause software that relies on this list to assume that ECDH and ECDSA is possible with those curves, but it isn't. We changed the list functions to list Montgomery curves some time after 2.16, but that change was too disruptive for long-time support branches.

However there's no reason why mbedtls_ecp_curve_info_from_grp_id and mbedtls_ecp_curve_info_from_name should be restricted to curves in the “fully supported list”. It's a bug because these functions use mbedtls_ecp_curve_list() internally. Instead these functions should use a longer list.

mbedtls_ecp_curve_info_from_tls_id also calls mbedtls_ecp_curve_list(), but the library doesn't support Montgomery curves in TLS yet, so that's ok.

mu578 commented 4 years ago

@gilles-peskine-arm, ok sorry my bad yes MBEDTLS_ECP_DP_CURVE25519 [indeces 8, 12]; thank you. I was just running a blind conformity test to pass any type of curves within an algorithm. Wanted to print-out the details attached within a given id; got some bad-access; I should have called mbedtls_ecp_curve_list or mbedtls_ecp_grp_id_list anyways rather than using a copy of the enum; we'll ignore MBEDTLS_ECP_DP_CURVE25519, MBEDTLS_ECP_DP_CURVE448; we have proxy functions so it helps.