From e3fb4ccabf02784bbf41010b837b520a4b43c115 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 19 Feb 2024 16:27:35 +0100 Subject: [PATCH 1/7] mbedtls_ecp_write_key_ext(): new function Same as mbedtls_ecp_write_key(), but doesn't require the caller to figure out the length of the output and possibly distinguish between Weierstrass and Montgomery curves. Signed-off-by: Gilles Peskine --- ChangeLog.d/ecp_write_key.txt | 4 ++ include/mbedtls/ecp.h | 20 ++++++ library/ecp.c | 32 +++++++++ tests/suites/test_suite_ecp.data | 103 +++++++++++++++++++++++++++ tests/suites/test_suite_ecp.function | 36 ++++++++++ 5 files changed, 195 insertions(+) create mode 100644 ChangeLog.d/ecp_write_key.txt diff --git a/ChangeLog.d/ecp_write_key.txt b/ChangeLog.d/ecp_write_key.txt new file mode 100644 index 000000000..19612396c --- /dev/null +++ b/ChangeLog.d/ecp_write_key.txt @@ -0,0 +1,4 @@ +Features + * The new function mbedtls_ecp_write_key_ext() is similar to + mbedtls_ecp_write_key(), but can be used without separately calculating + the output length. diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 0201963ab..b46d6c3dc 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -1370,6 +1370,26 @@ int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, int mbedtls_ecp_write_key(mbedtls_ecp_keypair *key, unsigned char *buf, size_t buflen); +/** + * \brief This function exports an elliptic curve private key. + * + * \param key The private key. + * \param olen On success, the length of the private key. + * This is always (`grp->nbits` + 7) / 8 bytes + * where `grp->nbits` is the private key size in bits. + * \param buf The output buffer for containing the binary representation + * of the key. + * \param buflen The total length of the buffer in bytes. + * #MBEDTLS_ECP_MAX_BYTES is always sufficient. + * + * \return \c 0 on success. + * \return #MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL if the \p key + * representation is larger than the available space in \p buf. + * \return Another negative error code on different kinds of failure. + */ +int mbedtls_ecp_write_key_ext(mbedtls_ecp_keypair *key, + size_t *olen, unsigned char *buf, size_t buflen); + /** * \brief This function exports an elliptic curve public key. * diff --git a/library/ecp.c b/library/ecp.c index 66b3dc1be..930102f97 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -3333,6 +3333,38 @@ cleanup: return ret; } +int mbedtls_ecp_write_key_ext(mbedtls_ecp_keypair *key, + size_t *olen, unsigned char *buf, size_t buflen) +{ + size_t len = (key->grp.nbits + 7) / 8; + if (len > buflen) { + /* For robustness, ensure *olen <= buflen even on error. */ + *olen = 0; + return MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL; + } + *olen = len; + + /* Private key not set */ + if (key->d.n == 0) { + return MBEDTLS_ERR_ECP_BAD_INPUT_DATA; + } + +#if defined(MBEDTLS_ECP_MONTGOMERY_ENABLED) + if (mbedtls_ecp_get_type(&key->grp) == MBEDTLS_ECP_TYPE_MONTGOMERY) { + return mbedtls_mpi_write_binary_le(&key->d, buf, len); + } +#endif + +#if defined(MBEDTLS_ECP_SHORT_WEIERSTRASS_ENABLED) + if (mbedtls_ecp_get_type(&key->grp) == MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS) { + return mbedtls_mpi_write_binary(&key->d, buf, len); + } +#endif + + /* Private key set but no recognized curve type? This shouldn't happen. */ + return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; +} + /* * Write a public key. */ diff --git a/tests/suites/test_suite_ecp.data b/tests/suites/test_suite_ecp.data index 1dd963a23..fd63657aa 100644 --- a/tests/suites/test_suite_ecp.data +++ b/tests/suites/test_suite_ecp.data @@ -888,6 +888,109 @@ ECP write key: Curve448, mostly-0 key, output_size=55 depends_on:MBEDTLS_ECP_DP_CURVE448_ENABLED ecp_write_key:MBEDTLS_ECP_DP_CURVE448:"0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000080":55:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL +ECP write key ext: secp256r1, nominal +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":32:0 + +ECP write key ext: secp256r1, output longer by 1 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":33:0 + +ECP write key ext: secp256r1, output short by 1 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":31:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + +ECP write key ext: secp256r1, output_size=0 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":0:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + +ECP write key ext: secp256r1, top byte = 0, output_size=32 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_SECP256R1:"00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff":32:0 + +ECP write key ext: secp256r1, top byte = 0, output_size=31 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_SECP256R1:"00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff":31:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + +ECP write key ext: secp256r1, top byte = 0, output_size=30 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_SECP256R1:"00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff":30:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + +ECP write key ext: secp256r1, mostly-0 key, output_size=32 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_SECP256R1:"0000000000000000000000000000000000000000000000000000000000000001":32:0 + +ECP write key ext: secp256r1, mostly-0 key, output_size=1 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_SECP256R1:"0000000000000000000000000000000000000000000000000000000000000001":1:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + +ECP write key ext: secp256r1, private key not set +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_SECP256R1:"":32:MBEDTLS_ERR_ECP_BAD_INPUT_DATA + +ECP write key ext: secp384r1, nominal +depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":48:0 + +ECP write key ext: secp384r1, output longer by 1 +depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":49:0 + +ECP write key ext: secp384r1, output short by 1 +depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":47:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + +ECP write key ext: Curve25519, nominal +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":32:0 + +ECP write key ext: Curve25519, output longer by 1 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":33:0 + +ECP write key ext: Curve25519, output short by 1 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":31:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + +ECP write key ext: Curve25519, output_size=0 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":0:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + +ECP write key ext: Curve25519, mostly-0 key, output_size=32 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_CURVE25519:"0000000000000000000000000000000000000000000000000000000000000040":32:0 + +ECP write key ext: Curve25519, mostly-0 key, output_size=31 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_CURVE25519:"0000000000000000000000000000000000000000000000000000000000000040":31:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + +ECP write key ext: Curve25519, private key not set +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_CURVE25519:"":32:MBEDTLS_ERR_ECP_BAD_INPUT_DATA + +ECP write key ext: Curve448, nominal +depends_on:MBEDTLS_ECP_DP_CURVE448_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_CURVE448:"3c262fddf9ec8e88495266fea19a34d28882acef045104d0d1aae121700a779c984c24f8cdd78fbff44943eba368f54b29259a4f1c600ad3":56:0 + +ECP write key ext: Curve448, output longer by 1 +depends_on:MBEDTLS_ECP_DP_CURVE448_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_CURVE448:"3c262fddf9ec8e88495266fea19a34d28882acef045104d0d1aae121700a779c984c24f8cdd78fbff44943eba368f54b29259a4f1c600ad3":57:0 + +ECP write key ext: Curve448, output short by 1 +depends_on:MBEDTLS_ECP_DP_CURVE448_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_CURVE448:"3c262fddf9ec8e88495266fea19a34d28882acef045104d0d1aae121700a779c984c24f8cdd78fbff44943eba368f54b29259a4f1c600ad3":55:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + +ECP write key ext: Curve448, mostly-0 key, output_size=56 +depends_on:MBEDTLS_ECP_DP_CURVE448_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_CURVE448:"0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000080":56:0 + +ECP write key ext: Curve448, mostly-0 key, output_size=55 +depends_on:MBEDTLS_ECP_DP_CURVE448_ENABLED +ecp_write_key_ext:MBEDTLS_ECP_DP_CURVE448:"0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000080":55:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + +ECP write key ext: group not set +ecp_write_key_ext:MBEDTLS_ECP_DP_NONE:"":32:MBEDTLS_ERR_ECP_BAD_INPUT_DATA + ECP mod p192 small (more than 192 bits, less limbs than 2 * 192 bits) depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED:MBEDTLS_ECP_NIST_OPTIM ecp_fast_mod:MBEDTLS_ECP_DP_SECP192R1:"0100000000000103010000000000010201000000000001010100000000000100" diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index 9cf0ce1b7..c0612755f 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -1296,6 +1296,42 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void ecp_write_key_ext(int grp_id, data_t *in_key, + int exported_size, int expected_ret) +{ + mbedtls_ecp_keypair key; + mbedtls_ecp_keypair_init(&key); + unsigned char *exported = NULL; + + if (in_key->len != 0) { + TEST_EQUAL(mbedtls_ecp_read_key(grp_id, &key, in_key->x, in_key->len), 0); + } else if (grp_id != MBEDTLS_ECP_DP_NONE) { + TEST_EQUAL(mbedtls_ecp_group_load(&key.grp, grp_id), 0); + } + + TEST_CALLOC(exported, exported_size); + size_t olen = 0xdeadbeef; + TEST_EQUAL(mbedtls_ecp_write_key_ext(&key, &olen, exported, exported_size), + expected_ret); + + if (expected_ret == 0) { + TEST_EQUAL(olen, (key.grp.nbits + 7) / 8); + TEST_LE_U(olen, MBEDTLS_ECP_MAX_BYTES); + TEST_MEMORY_COMPARE(in_key->x, in_key->len, + exported, olen); + } else { + /* Robustness check: even in the error case, insist that olen is less + * than the buffer size. */ + TEST_LE_U(olen, exported_size); + } + +exit: + mbedtls_ecp_keypair_free(&key); + mbedtls_free(exported); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_TEST_HOOKS:MBEDTLS_ECP_MONTGOMERY_ENABLED:MBEDTLS_ECP_LIGHT */ void genkey_mx_known_answer(int bits, data_t *seed, data_t *expected) { From acdc52e1543dfb1deb79de414758726538767dba Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 19 Feb 2024 16:42:54 +0100 Subject: [PATCH 2/7] mbedtls_ecp_write_key_ext(): recommend over the old function in documentation Signed-off-by: Gilles Peskine --- docs/psa-transition.md | 10 +++++----- include/mbedtls/ecp.h | 2 ++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/psa-transition.md b/docs/psa-transition.md index e89128cbd..94b57eba9 100644 --- a/docs/psa-transition.md +++ b/docs/psa-transition.md @@ -845,7 +845,6 @@ For an ECC private key (a future version of Mbed TLS [will provide a more direct ``` unsigned char buf[PSA_BITS_TO_BYTES(PSA_VENDOR_ECC_MAX_CURVE_BITS)]; -size_t length = PSA_BITS_TO_BYTES(mbedtls_pk_bitlen(&pk)); mbedtls_ecp_keypair *ec = mbedtls_pk_ec(&pk); psa_ecc_curve_t curve; { @@ -862,7 +861,8 @@ psa_ecc_curve_t curve; mbedtls_ecp_point_free(&Q); mbedtls_mpi_free(&d); } -mbedtls_ecp_write_key(ec, buf, length); +size_t length; +mbedtls_ecp_write_key_ext(ec, &length, buf, sizeof(buf)); psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; psa_set_key_type(&attributes, PSA_KEY_TYPE_ECC_KEY_PAIR(curve)); psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_... | ...); @@ -900,8 +900,8 @@ mbedtls_ecp_keypair_init(&ec); // Omitted: fill ec with key material // (the public key will not be used and does not need to be set) unsigned char buf[PSA_BITS_TO_BYTES(PSA_VENDOR_ECC_MAX_CURVE_BITS)]; -size_t length = PSA_BITS_TO_BYTES(mbedtls_pk_bitlen(&pk)); -mbedtls_ecp_write_key(&ec, buf, length); +size_t length; +mbedtls_ecp_write_key_ext(&ec, &length, buf, sizeof(buf)); psa_ecc_curve_t curve = ...; // need to determine the curve family manually psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; psa_set_key_attributes(&attributes, PSA_KEY_TYPE_ECC_KEY_PAIR(curve)); @@ -1300,7 +1300,7 @@ There is no PSA equivalent to the types `mbedtls_ecdsa_context` and `mbedtls_ecd The PSA API is a cryptography API, not an arithmetic API. As a consequence, there is no PSA equivalent for the ECC arithmetic functionality exposed by `ecp.h`: * Manipulation of point objects and input-output: the type `mbedtls_ecp_point` and functions operating on it (`mbedtls_ecp_point_xxx`, `mbedtls_ecp_copy`, `mbedtls_ecp_{set,is}_zero`, `mbedtls_ecp_tls_{read,write}_point`). Note that the PSA export format for public keys corresponds to the uncompressed point format (`MBEDTLS_ECP_PF_UNCOMPRESSED`), so [`psa_import_key`](https://mbed-tls.readthedocs.io/projects/api/en/development/api/group/group__import__export/#group__import__export_1ga0336ea76bf30587ab204a8296462327b), [`psa_export_key`](https://mbed-tls.readthedocs.io/projects/api/en/development/api/group/group__import__export/#group__import__export_1ga668e35be8d2852ad3feeef74ac6f75bf) and [`psa_export_public_key`](https://mbed-tls.readthedocs.io/projects/api/en/development/api/group/group__import__export/#group__import__export_1gaf22ae73312217aaede2ea02cdebb6062) are equivalent to `mbedtls_ecp_point_read_binary` and `mbedtls_ecp_point_write_binary` for uncompressed points. The PSA API does not currently support compressed points, but it is likely that such support will be added in the future. -* Manipulation of key pairs as such, with a bridge to bignum arithmetic (`mbedtls_ecp_keypair` type, `mbedtls_ecp_export`). However, the PSA export format for ECC private keys used by [`psa_import_key`](https://mbed-tls.readthedocs.io/projects/api/en/development/api/group/group__import__export/#group__import__export_1ga0336ea76bf30587ab204a8296462327b), [`psa_export_key`](https://mbed-tls.readthedocs.io/projects/api/en/development/api/group/group__import__export/#group__import__export_1ga668e35be8d2852ad3feeef74ac6f75bf) is the same as the format used by `mbedtls_ecp_read_key` and `mbedtls_ecp_write_key`. +* Manipulation of key pairs as such, with a bridge to bignum arithmetic (`mbedtls_ecp_keypair` type, `mbedtls_ecp_export`). However, the PSA export format for ECC private keys used by [`psa_import_key`](https://mbed-tls.readthedocs.io/projects/api/en/development/api/group/group__import__export/#group__import__export_1ga0336ea76bf30587ab204a8296462327b), [`psa_export_key`](https://mbed-tls.readthedocs.io/projects/api/en/development/api/group/group__import__export/#group__import__export_1ga668e35be8d2852ad3feeef74ac6f75bf) is the same as the format used by `mbedtls_ecp_read_key` and `mbedtls_ecp_write_key_ext`. * Elliptic curve arithmetic (`mbedtls_ecp_mul`, `mbedtls_ecp_muladd` and their restartable variants). ### Additional information about RSA diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index b46d6c3dc..58fc5e555 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -1338,6 +1338,8 @@ int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, * checking that the output buffer is large enough. * See the description of the \p buflen parameter for * how to calculate the nominal length. + * To avoid this difficulty, use mbedtls_ecp_write_key_ext() + * instead. * * \note If the private key was not set in \p key, * the output is unspecified. Future versions From 84b9f1b0391ec748ab0565b4cd89b0f4a667187b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 19 Feb 2024 16:44:29 +0100 Subject: [PATCH 3/7] mbedtls_ecp_write_key_ext(): migrate internally Stop using mbedtls_ecp_write_key() except to test it. Signed-off-by: Gilles Peskine --- library/pk.c | 3 +-- library/pkwrite.c | 2 +- library/psa_crypto_ecp.c | 21 +++-------------- library/ssl_tls12_server.c | 3 +-- tests/suites/test_suite_ecp.function | 35 +++++++++++++++++++--------- 5 files changed, 30 insertions(+), 34 deletions(-) diff --git a/library/pk.c b/library/pk.c index 1ded4872f..c647b459a 100644 --- a/library/pk.c +++ b/library/pk.c @@ -1401,8 +1401,7 @@ int mbedtls_pk_wrap_as_opaque(mbedtls_pk_context *pk, mbedtls_ecp_keypair *ec = mbedtls_pk_ec_rw(*pk); int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - d_len = PSA_BITS_TO_BYTES(ec->grp.nbits); - if ((ret = mbedtls_ecp_write_key(ec, d, d_len)) != 0) { + if ((ret = mbedtls_ecp_write_key_ext(ec, &d_len, d, sizeof(d))) != 0) { return ret; } diff --git a/library/pkwrite.c b/library/pkwrite.c index b9ddcf1d8..5e009c565 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -202,7 +202,7 @@ static int pk_write_ec_private(unsigned char **p, unsigned char *start, mbedtls_ecp_keypair *ec = mbedtls_pk_ec_rw(*pk); byte_length = (ec->grp.pbits + 7) / 8; - ret = mbedtls_ecp_write_key(ec, tmp, byte_length); + ret = mbedtls_ecp_write_key_ext(ec, &byte_length, tmp, sizeof(tmp)); if (ret != 0) { goto exit; } diff --git a/library/psa_crypto_ecp.c b/library/psa_crypto_ecp.c index 7edea8164..e373ad9f6 100644 --- a/library/psa_crypto_ecp.c +++ b/library/psa_crypto_ecp.c @@ -281,20 +281,8 @@ psa_status_t mbedtls_psa_ecp_export_key(psa_key_type_t type, return status; } else { - if (data_size < PSA_BITS_TO_BYTES(ecp->grp.nbits)) { - return PSA_ERROR_BUFFER_TOO_SMALL; - } - status = mbedtls_to_psa_error( - mbedtls_ecp_write_key(ecp, - data, - PSA_BITS_TO_BYTES(ecp->grp.nbits))); - if (status == PSA_SUCCESS) { - *data_length = PSA_BITS_TO_BYTES(ecp->grp.nbits); - } else { - memset(data, 0, data_size); - } - + mbedtls_ecp_write_key_ext(ecp, data_length, data, data_size)); return status; } } @@ -359,14 +347,11 @@ psa_status_t mbedtls_psa_ecp_generate_key( } status = mbedtls_to_psa_error( - mbedtls_ecp_write_key(&ecp, key_buffer, key_buffer_size)); + mbedtls_ecp_write_key_ext(&ecp, key_buffer_length, + key_buffer, key_buffer_size)); mbedtls_ecp_keypair_free(&ecp); - if (status == PSA_SUCCESS) { - *key_buffer_length = key_buffer_size; - } - return status; } #endif /* MBEDTLS_PSA_BUILTIN_KEY_TYPE_ECC_KEY_PAIR_GENERATE */ diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 53a9ce206..5bee18823 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -2703,8 +2703,7 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl) PSA_KEY_TYPE_ECC_KEY_PAIR(ssl->handshake->xxdh_psa_type)); psa_set_key_bits(&key_attributes, ssl->handshake->xxdh_psa_bits); - key_len = PSA_BITS_TO_BYTES(key->grp.pbits); - ret = mbedtls_ecp_write_key(key, buf, key_len); + ret = mbedtls_ecp_write_key_ext(key, &key_len, buf, sizeof(buf)); if (ret != 0) { mbedtls_platform_zeroize(buf, sizeof(buf)); break; diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index c0612755f..ef0781bb7 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -1204,27 +1204,40 @@ void mbedtls_ecp_read_key(int grp_id, data_t *in_key, int expected, int canonica TEST_EQUAL(mbedtls_mpi_cmp_int(&key.Q.Y, 2), 0); TEST_EQUAL(mbedtls_mpi_cmp_int(&key.Q.Z, 3), 0); - if (canonical) { + if (canonical && in_key->len == (key.grp.nbits + 7) / 8) { unsigned char buf[MBEDTLS_ECP_MAX_BYTES]; + size_t length = 0xdeadbeef; - ret = mbedtls_ecp_write_key(&key, buf, in_key->len); - TEST_ASSERT(ret == 0); + TEST_EQUAL(mbedtls_ecp_write_key_ext(&key, + &length, buf, in_key->len), 0); + TEST_MEMORY_COMPARE(in_key->x, in_key->len, + buf, length); + memset(buf, 0, sizeof(buf)); + TEST_EQUAL(mbedtls_ecp_write_key(&key, buf, in_key->len), 0); TEST_MEMORY_COMPARE(in_key->x, in_key->len, buf, in_key->len); } else { unsigned char export1[MBEDTLS_ECP_MAX_BYTES]; unsigned char export2[MBEDTLS_ECP_MAX_BYTES]; - ret = mbedtls_ecp_write_key(&key, export1, in_key->len); - TEST_ASSERT(ret == 0); - - ret = mbedtls_ecp_read_key(grp_id, &key2, export1, in_key->len); - TEST_ASSERT(ret == expected); - - ret = mbedtls_ecp_write_key(&key2, export2, in_key->len); - TEST_ASSERT(ret == 0); + size_t length1 = 0xdeadbeef; + TEST_EQUAL(mbedtls_ecp_write_key_ext(&key, &length1, + export1, sizeof(export1)), 0); + TEST_EQUAL(mbedtls_ecp_read_key(grp_id, &key2, export1, length1), + expected); + size_t length2 = 0xdeadbeef; + TEST_EQUAL(mbedtls_ecp_write_key_ext(&key2, &length2, + export2, sizeof(export2)), 0); + TEST_MEMORY_COMPARE(export1, length1, + export2, length2); + memset(export1, 0, sizeof(export1)); + memset(export2, 0, sizeof(export2)); + TEST_EQUAL(mbedtls_ecp_write_key(&key, export1, in_key->len), 0); + TEST_EQUAL(mbedtls_ecp_read_key(grp_id, &key2, export1, in_key->len), + expected); + TEST_EQUAL(mbedtls_ecp_write_key(&key2, export2, in_key->len), 0); TEST_MEMORY_COMPARE(export1, in_key->len, export2, in_key->len); } From c0f7a8680fe1b1a5346058fca7b6af38352dbe15 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 19 Feb 2024 16:50:39 +0100 Subject: [PATCH 4/7] mbedtls_ecp_write_key(): deprecate the old function Signed-off-by: Gilles Peskine --- ChangeLog.d/ecp_write_key.txt | 4 ++++ include/mbedtls/ecp.h | 11 ++++++++--- library/ecp.c | 2 ++ tests/suites/test_suite_ecp.function | 6 +++++- 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/ChangeLog.d/ecp_write_key.txt b/ChangeLog.d/ecp_write_key.txt index 19612396c..73354c863 100644 --- a/ChangeLog.d/ecp_write_key.txt +++ b/ChangeLog.d/ecp_write_key.txt @@ -2,3 +2,7 @@ Features * The new function mbedtls_ecp_write_key_ext() is similar to mbedtls_ecp_write_key(), but can be used without separately calculating the output length. + +New deprecations + * mbedtls_ecp_write_key() is deprecated in favor of + mbedtls_ecp_write_key_ext(). diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 58fc5e555..05778cdd1 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -24,6 +24,7 @@ #include "mbedtls/private_access.h" #include "mbedtls/build_info.h" +#include "mbedtls/platform_util.h" #include "mbedtls/bignum.h" @@ -1327,10 +1328,11 @@ int mbedtls_ecp_set_public_key(mbedtls_ecp_group_id grp_id, int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, const unsigned char *buf, size_t buflen); +#if !defined(MBEDTLS_DEPRECATED_REMOVED) /** * \brief This function exports an elliptic curve private key. * - * \note Note that although this function accepts an output + * \deprecated Note that although this function accepts an output * buffer that is smaller or larger than the key, most key * import interfaces require the output to have exactly * key's nominal length. It is generally simplest to @@ -1340,6 +1342,8 @@ int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, * how to calculate the nominal length. * To avoid this difficulty, use mbedtls_ecp_write_key_ext() * instead. + * mbedtls_ecp_write_key() is deprecated and will be + * removed in a future version of the library. * * \note If the private key was not set in \p key, * the output is unspecified. Future versions @@ -1369,8 +1373,9 @@ int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, * representation is larger than the available space in \p buf. * \return Another negative error code on different kinds of failure. */ -int mbedtls_ecp_write_key(mbedtls_ecp_keypair *key, - unsigned char *buf, size_t buflen); +int MBEDTLS_DEPRECATED mbedtls_ecp_write_key(mbedtls_ecp_keypair *key, + unsigned char *buf, size_t buflen); +#endif /* MBEDTLS_DEPRECATED_REMOVED */ /** * \brief This function exports an elliptic curve private key. diff --git a/library/ecp.c b/library/ecp.c index 930102f97..0dadaeaac 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -3302,6 +3302,7 @@ cleanup: /* * Write a private key. */ +#if !defined MBEDTLS_DEPRECATED_REMOVED int mbedtls_ecp_write_key(mbedtls_ecp_keypair *key, unsigned char *buf, size_t buflen) { @@ -3332,6 +3333,7 @@ cleanup: return ret; } +#endif /* MBEDTLS_DEPRECATED_REMOVED */ int mbedtls_ecp_write_key_ext(mbedtls_ecp_keypair *key, size_t *olen, unsigned char *buf, size_t buflen) diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index ef0781bb7..9b5c86f97 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -1213,10 +1213,12 @@ void mbedtls_ecp_read_key(int grp_id, data_t *in_key, int expected, int canonica TEST_MEMORY_COMPARE(in_key->x, in_key->len, buf, length); +#if defined(MBEDTLS_TEST_DEPRECATED) memset(buf, 0, sizeof(buf)); TEST_EQUAL(mbedtls_ecp_write_key(&key, buf, in_key->len), 0); TEST_MEMORY_COMPARE(in_key->x, in_key->len, buf, in_key->len); +#endif /* MBEDTLS_TEST_DEPRECATED */ } else { unsigned char export1[MBEDTLS_ECP_MAX_BYTES]; unsigned char export2[MBEDTLS_ECP_MAX_BYTES]; @@ -1232,6 +1234,7 @@ void mbedtls_ecp_read_key(int grp_id, data_t *in_key, int expected, int canonica TEST_MEMORY_COMPARE(export1, length1, export2, length2); +#if defined(MBEDTLS_TEST_DEPRECATED) memset(export1, 0, sizeof(export1)); memset(export2, 0, sizeof(export2)); TEST_EQUAL(mbedtls_ecp_write_key(&key, export1, in_key->len), 0); @@ -1240,6 +1243,7 @@ void mbedtls_ecp_read_key(int grp_id, data_t *in_key, int expected, int canonica TEST_EQUAL(mbedtls_ecp_write_key(&key2, export2, in_key->len), 0); TEST_MEMORY_COMPARE(export1, in_key->len, export2, in_key->len); +#endif /* MBEDTLS_TEST_DEPRECATED */ } } @@ -1249,7 +1253,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_TEST_DEPRECATED */ void ecp_write_key(int grp_id, data_t *in_key, int exported_size, int expected_ret) { From 04ae479b04eedc2a9cb7e720bd4ca51f87c91654 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 26 Feb 2024 09:15:08 +0100 Subject: [PATCH 5/7] mbedtls_ecp_write_key_ext: document error for no private key set Signed-off-by: Gilles Peskine --- include/mbedtls/ecp.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 05778cdd1..e2ae07444 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -1392,6 +1392,8 @@ int MBEDTLS_DEPRECATED mbedtls_ecp_write_key(mbedtls_ecp_keypair *key, * \return \c 0 on success. * \return #MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL if the \p key * representation is larger than the available space in \p buf. + * \return #MBEDTLS_ERR_ECP_BAD_INPUT_DATA if no private key is + * set in \p key. * \return Another negative error code on different kinds of failure. */ int mbedtls_ecp_write_key_ext(mbedtls_ecp_keypair *key, From b395e74edd3cce58b692eefe3b5127e096030075 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 28 Feb 2024 14:18:28 +0100 Subject: [PATCH 6/7] mbedtls_ecp_write_key_ext(): make key const Having a non-const `key` parameter was anotherf defect of mbedtls_ecp_write_key(). Take this opportunity to fix it. Signed-off-by: Gilles Peskine --- include/mbedtls/ecp.h | 2 +- library/ecp.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index e2ae07444..d8f73ae96 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -1396,7 +1396,7 @@ int MBEDTLS_DEPRECATED mbedtls_ecp_write_key(mbedtls_ecp_keypair *key, * set in \p key. * \return Another negative error code on different kinds of failure. */ -int mbedtls_ecp_write_key_ext(mbedtls_ecp_keypair *key, +int mbedtls_ecp_write_key_ext(const mbedtls_ecp_keypair *key, size_t *olen, unsigned char *buf, size_t buflen); /** diff --git a/library/ecp.c b/library/ecp.c index 0dadaeaac..427059bb5 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -3335,7 +3335,7 @@ cleanup: } #endif /* MBEDTLS_DEPRECATED_REMOVED */ -int mbedtls_ecp_write_key_ext(mbedtls_ecp_keypair *key, +int mbedtls_ecp_write_key_ext(const mbedtls_ecp_keypair *key, size_t *olen, unsigned char *buf, size_t buflen) { size_t len = (key->grp.nbits + 7) / 8; From 84a7bfbd33a2df49235231ff3b9c49c22038bf30 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 28 Feb 2024 14:21:32 +0100 Subject: [PATCH 7/7] mbedtls_ecp_write_key_ext(): Upgrade import_pair_into_psa as well It wasn't done with the others because that code was added in a concurrent branch. Signed-off-by: Gilles Peskine --- library/pk.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/library/pk.c b/library/pk.c index c647b459a..328bbe1ea 100644 --- a/library/pk.c +++ b/library/pk.c @@ -675,10 +675,7 @@ static int import_pair_into_psa(const mbedtls_pk_context *pk, #if defined(MBEDTLS_PK_USE_PSA_EC_DATA) psa_ecc_family_t from_family = pk->ec_family; #else /* MBEDTLS_PK_USE_PSA_EC_DATA */ - /* We're only reading the key, but mbedtls_ecp_write_key() - * is missing a const annotation on its key parameter, so - * we need the non-const accessor here. */ - mbedtls_ecp_keypair *ec = mbedtls_pk_ec_rw(*pk); + const mbedtls_ecp_keypair *ec = mbedtls_pk_ec_ro(*pk); size_t from_bits = 0; psa_ecc_family_t from_family = mbedtls_ecc_group_to_psa(ec->grp.id, &from_bits); @@ -704,12 +701,9 @@ static int import_pair_into_psa(const mbedtls_pk_context *pk, return MBEDTLS_ERR_PK_TYPE_MISMATCH; } unsigned char key_buffer[PSA_BITS_TO_BYTES(PSA_VENDOR_ECC_MAX_CURVE_BITS)]; - /* Make sure to pass the exact key length to - * mbedtls_ecp_write_key(), because it writes Montgomery keys - * at the start of the buffer but Weierstrass keys at the - * end of the buffer. */ - size_t key_length = PSA_BITS_TO_BYTES(ec->grp.nbits); - int ret = mbedtls_ecp_write_key(ec, key_buffer, key_length); + size_t key_length = 0; + int ret = mbedtls_ecp_write_key_ext(ec, &key_length, + key_buffer, sizeof(key_buffer)); if (ret < 0) { return ret; }