diff --git a/ChangeLog.d/mbedtls_psa_ecp_generate_key-no_public_key.txt b/ChangeLog.d/mbedtls_psa_ecp_generate_key-no_public_key.txt new file mode 100644 index 0000000000..69c00e1a77 --- /dev/null +++ b/ChangeLog.d/mbedtls_psa_ecp_generate_key-no_public_key.txt @@ -0,0 +1,3 @@ +Changes + * Improve performance of PSA key generation with ECC keys: it no longer + computes the public key (which was immediately discarded). Fixes #9732. diff --git a/library/psa_crypto_ecp.c b/library/psa_crypto_ecp.c index 95baff6a0f..48b90ef57d 100644 --- a/library/psa_crypto_ecp.c +++ b/library/psa_crypto_ecp.c @@ -321,38 +321,36 @@ psa_status_t mbedtls_psa_ecp_generate_key( const psa_key_attributes_t *attributes, uint8_t *key_buffer, size_t key_buffer_size, size_t *key_buffer_length) { - psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - psa_ecc_family_t curve = PSA_KEY_TYPE_ECC_GET_FAMILY( attributes->type); mbedtls_ecp_group_id grp_id = mbedtls_ecc_group_from_psa(curve, attributes->bits); - - const mbedtls_ecp_curve_info *curve_info = - mbedtls_ecp_curve_info_from_grp_id(grp_id); - mbedtls_ecp_keypair ecp; - - if (grp_id == MBEDTLS_ECP_DP_NONE || curve_info == NULL) { + if (grp_id == MBEDTLS_ECP_DP_NONE) { return PSA_ERROR_NOT_SUPPORTED; } + mbedtls_ecp_keypair ecp; mbedtls_ecp_keypair_init(&ecp); - ret = mbedtls_ecp_gen_key(grp_id, &ecp, - mbedtls_psa_get_random, - MBEDTLS_PSA_RANDOM_STATE); + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + + ret = mbedtls_ecp_group_load(&ecp.grp, grp_id); if (ret != 0) { - mbedtls_ecp_keypair_free(&ecp); - return mbedtls_to_psa_error(ret); + goto exit; } - status = mbedtls_to_psa_error( - mbedtls_ecp_write_key_ext(&ecp, key_buffer_length, - key_buffer, key_buffer_size)); + ret = mbedtls_ecp_gen_privkey(&ecp.grp, &ecp.d, + mbedtls_psa_get_random, + MBEDTLS_PSA_RANDOM_STATE); + if (ret != 0) { + goto exit; + } + ret = mbedtls_ecp_write_key_ext(&ecp, key_buffer_length, + key_buffer, key_buffer_size); + +exit: mbedtls_ecp_keypair_free(&ecp); - - return status; + return mbedtls_to_psa_error(ret); } #endif /* MBEDTLS_PSA_BUILTIN_KEY_TYPE_ECC_KEY_PAIR_GENERATE */ diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 1f7e98c525..18c8bde8bd 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -428,6 +428,8 @@ class DriverVSReference_ecp_light_only(outcome_analysis.DriverVSReference): IGNORED_SUITES = [ # Modules replaced by drivers 'ecdsa', 'ecdh', 'ecjpake', + # Unit tests for the built-in implementation + 'psa_crypto_ecp', ] IGNORED_TESTS = { 'test_suite_config': [ @@ -468,6 +470,8 @@ class DriverVSReference_no_ecp_at_all(outcome_analysis.DriverVSReference): IGNORED_SUITES = [ # Modules replaced by drivers 'ecp', 'ecdsa', 'ecdh', 'ecjpake', + # Unit tests for the built-in implementation + 'psa_crypto_ecp', ] IGNORED_TESTS = { 'test_suite_config': [ @@ -508,6 +512,8 @@ class DriverVSReference_ecc_no_bignum(outcome_analysis.DriverVSReference): 'ecp', 'ecdsa', 'ecdh', 'ecjpake', 'bignum_core', 'bignum_random', 'bignum_mod', 'bignum_mod_raw', 'bignum.generated', 'bignum.misc', + # Unit tests for the built-in implementation + 'psa_crypto_ecp', ] IGNORED_TESTS = { 'test_suite_config': [ @@ -553,6 +559,8 @@ class DriverVSReference_ecc_ffdh_no_bignum(outcome_analysis.DriverVSReference): 'ecp', 'ecdsa', 'ecdh', 'ecjpake', 'dhm', 'bignum_core', 'bignum_random', 'bignum_mod', 'bignum_mod_raw', 'bignum.generated', 'bignum.misc', + # Unit tests for the built-in implementation + 'psa_crypto_ecp', ] IGNORED_TESTS = { 'ssl-opt': [ @@ -623,6 +631,8 @@ class DriverVSReference_tfm_config(outcome_analysis.DriverVSReference): 'ecp', 'ecdsa', 'ecdh', 'ecjpake', 'bignum_core', 'bignum_random', 'bignum_mod', 'bignum_mod_raw', 'bignum.generated', 'bignum.misc', + # Unit tests for the built-in implementation + 'psa_crypto_ecp', ] IGNORED_TESTS = { 'test_suite_config': [ diff --git a/tests/suites/test_suite_psa_crypto_ecp.data b/tests/suites/test_suite_psa_crypto_ecp.data new file mode 100644 index 0000000000..ffb7a7b41e --- /dev/null +++ b/tests/suites/test_suite_psa_crypto_ecp.data @@ -0,0 +1,82 @@ +ECC generate: unknown family (0) +generate_key:0:256:64:PSA_ERROR_NOT_SUPPORTED + +ECC generate: unknown family (0xff) +generate_key:0xff:256:64:PSA_ERROR_NOT_SUPPORTED + +ECC generate: SECP_R1 bad bit-size (0) +generate_key:PSA_ECC_FAMILY_SECP_R1:0:64:PSA_ERROR_NOT_SUPPORTED + +ECC generate: SECP_R1 bad bit-size (512) +generate_key:PSA_ECC_FAMILY_SECP_R1:512:64:PSA_ERROR_NOT_SUPPORTED + +ECC generate: SECP_R1 bad bit-size (528) +generate_key:PSA_ECC_FAMILY_SECP_R1:528:64:PSA_ERROR_NOT_SUPPORTED + +ECC generate: SECP_R1 256-bit not supported +depends_on:!MBEDTLS_PSA_BUILTIN_ECC_SECP_R1_256 +generate_key:PSA_ECC_FAMILY_SECP_R1:256:32:PSA_ERROR_NOT_SUPPORTED + +ECC generate: SECP_R1 384-bit not supported +depends_on:!MBEDTLS_PSA_BUILTIN_ECC_SECP_R1_384 +generate_key:PSA_ECC_FAMILY_SECP_R1:384:48:PSA_ERROR_NOT_SUPPORTED + +ECC generate: SECP_R1 521-bit not supported +depends_on:!MBEDTLS_PSA_BUILTIN_ECC_SECP_R1_521 +generate_key:PSA_ECC_FAMILY_SECP_R1:521:66:PSA_ERROR_NOT_SUPPORTED + +ECC generate: SECP_K1 256-bit not supported +depends_on:!MBEDTLS_PSA_BUILTIN_ECC_SECP_K1_256 +generate_key:PSA_ECC_FAMILY_SECP_K1:256:32:PSA_ERROR_NOT_SUPPORTED + +ECC generate: Curve25519 not supported +depends_on:!MBEDTLS_PSA_BUILTIN_ECC_MONTGOMERY_255 +generate_key:PSA_ECC_FAMILY_MONTGOMERY:255:32:PSA_ERROR_NOT_SUPPORTED + +ECC generate: Curve448 not supported +depends_on:!MBEDTLS_PSA_BUILTIN_ECC_MONTGOMERY_448 +generate_key:PSA_ECC_FAMILY_MONTGOMERY:448:56:PSA_ERROR_NOT_SUPPORTED + +ECC generate: SECP_R1 256-bit, size=31, too small +depends_on:MBEDTLS_PSA_BUILTIN_ECC_SECP_R1_256 +generate_key:PSA_ECC_FAMILY_SECP_R1:256:31:PSA_ERROR_BUFFER_TOO_SMALL + +ECC generate: SECP_R1 256-bit, size=32, ok +depends_on:MBEDTLS_PSA_BUILTIN_ECC_SECP_R1_256 +generate_key:PSA_ECC_FAMILY_SECP_R1:256:32:PSA_SUCCESS + +ECC generate: SECP_R1 256-bit, size=33, ok +depends_on:MBEDTLS_PSA_BUILTIN_ECC_SECP_R1_256 +generate_key:PSA_ECC_FAMILY_SECP_R1:256:33:PSA_SUCCESS + +ECC generate: SECP_R1 521-bit, size=65, too small +depends_on:MBEDTLS_PSA_BUILTIN_ECC_SECP_R1_521 +generate_key:PSA_ECC_FAMILY_SECP_R1:521:65:PSA_ERROR_BUFFER_TOO_SMALL + +ECC generate: SECP_R1 521-bit, size=66, ok +depends_on:MBEDTLS_PSA_BUILTIN_ECC_SECP_R1_521 +generate_key:PSA_ECC_FAMILY_SECP_R1:521:66:PSA_SUCCESS + +ECC generate: Curve25519, size=31, too small +depends_on:MBEDTLS_PSA_BUILTIN_ECC_MONTGOMERY_255 +generate_key:PSA_ECC_FAMILY_MONTGOMERY:255:31:PSA_ERROR_BUFFER_TOO_SMALL + +ECC generate: Curve25519, size=32, ok +depends_on:MBEDTLS_PSA_BUILTIN_ECC_MONTGOMERY_255 +generate_key:PSA_ECC_FAMILY_MONTGOMERY:255:32:PSA_SUCCESS + +ECC generate: Curve25519, size=33, ok +depends_on:MBEDTLS_PSA_BUILTIN_ECC_MONTGOMERY_255 +generate_key:PSA_ECC_FAMILY_MONTGOMERY:255:33:PSA_SUCCESS + +ECC generate: Curve448, size=55, too small +depends_on:MBEDTLS_PSA_BUILTIN_ECC_MONTGOMERY_448 +generate_key:PSA_ECC_FAMILY_MONTGOMERY:448:55:PSA_ERROR_BUFFER_TOO_SMALL + +ECC generate: Curve448, size=56, ok +depends_on:MBEDTLS_PSA_BUILTIN_ECC_MONTGOMERY_448 +generate_key:PSA_ECC_FAMILY_MONTGOMERY:448:56:PSA_SUCCESS + +ECC generate: Curve448, size=57, ok +depends_on:MBEDTLS_PSA_BUILTIN_ECC_MONTGOMERY_448 +generate_key:PSA_ECC_FAMILY_MONTGOMERY:448:57:PSA_SUCCESS diff --git a/tests/suites/test_suite_psa_crypto_ecp.function b/tests/suites/test_suite_psa_crypto_ecp.function new file mode 100644 index 0000000000..5be7a2ca6d --- /dev/null +++ b/tests/suites/test_suite_psa_crypto_ecp.function @@ -0,0 +1,165 @@ +/* BEGIN_HEADER */ +/* Unit tests for internal functions for built-in ECC mechanisms. */ +#include + +#include "psa_crypto_ecp.h" + +#if defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_ECC_KEY_PAIR_GENERATE) +/* + * Check if a buffer is all-0 bytes: + * return 1 if it is, + * 0 if it isn't. + * + * TODO: we use this in multiple test suites. Move it to tests/src. + */ +static int buffer_is_all_zero(const uint8_t *buf, size_t size) +{ + for (size_t i = 0; i < size; i++) { + if (buf[i] != 0) { + return 0; + } + } + return 1; +} + +typedef struct { + unsigned bit_bot; /* lowest non-forced bit */ + unsigned bit_top; /* highest non-forced bit */ +} ecc_private_key_stats_t; + +/* Do some sanity checks on an ECC private key. This is not intended to be + * a full validity check, just to catch some potential mistakes. */ +static int check_ecc_private_key(psa_ecc_family_t family, size_t bits, + const uint8_t *key, size_t key_length, + ecc_private_key_stats_t *stats) +{ + int ok = 0; + + /* Check the expected length (same calculation for all curves). */ + TEST_EQUAL(PSA_BITS_TO_BYTES(bits), key_length); + + /* All-bits zero is invalid and means no key material was copied to the + * output buffer, or a grave RNG pluming failure. */ + TEST_ASSERT(!buffer_is_all_zero(key, key_length)); + + /* Check the top byte of the value for non-byte-aligned curve sizes. + * This is a partial endianness check. */ + if (bits % 8 != 0) { + /* All supported non-byte-aligned curve sizes are for Weierstrass + * curves with a big-endian representation. */ + uint8_t top_byte = key[0]; + uint8_t mask = 0xff << (bits & 8); + TEST_EQUAL(top_byte & mask, 0); + } + + /* Check masked bits on Curve25519 and Curve448 scalars. + * See RFC 7748 \S4.1 (we expect the "decoded" form here). */ +#if defined(MBEDTLS_PSA_BUILTIN_ECC_MONTGOMERY_255) + if (family == PSA_ECC_FAMILY_MONTGOMERY && bits == 255) { + TEST_EQUAL(key[0] & 0xf8, key[0]); + TEST_EQUAL(key[31] & 0xc0, 0x40); + } +#endif /* MBEDTLS_PSA_BUILTIN_ECC_MONTGOMERY_255 */ +#if defined(MBEDTLS_PSA_BUILTIN_ECC_MONTGOMERY_448) + if (family == PSA_ECC_FAMILY_MONTGOMERY && bits == 448) { + TEST_EQUAL(key[0] & 0xfc, key[0]); + TEST_EQUAL(key[55] & 0x80, 0x80); + } +#endif /* MBEDTLS_PSA_BUILTIN_ECC_MONTGOMERY_448 */ + + /* Don't bother to check that the value is in the exact permitted range + * (1 to p-1 for Weierstrass curves, 2^{n-1} to p-1 for Montgomery curves). + * We would need to bring in bignum machinery, and on most curves + * the probability of a number being out of range is negligible. + */ + + /* Collect statistics on random-valued bits */ + /* Defaults for big-endian numbers */ + uint8_t bit_bot_mask = 0x01; + size_t bit_bot_index = key_length - 1; + uint8_t bit_top_mask = (bits % 8 == 0 ? 0x80 : 1 << (bits % 8 - 1)); + size_t bit_top_index = 0; + if (family == PSA_ECC_FAMILY_MONTGOMERY) { + bit_bot_index = 0; + bit_top_index = key_length - 1; + if (bits == 255) { + bit_bot_mask = 0x08; + bit_top_mask = 0x20; + } else { + bit_bot_mask = 0x04; + bit_top_mask = 0x40; + } + } + if (key[bit_bot_index] & bit_bot_mask) { + ++stats->bit_bot; + } + if (key[bit_top_index] & bit_top_mask) { + ++stats->bit_top; + } + + ok = 1; +exit: + return ok; +} +#endif + +/* END_HEADER */ + +/* BEGIN_DEPENDENCIES + * depends_on:MBEDTLS_PSA_CRYPTO_C:MBEDTLS_PSA_BUILTIN_KEY_TYPE_ECC_PUBLIC_KEY + * END_DEPENDENCIES + */ + +/* BEGIN_CASE depends_on:MBEDTLS_PSA_BUILTIN_KEY_TYPE_ECC_KEY_PAIR_GENERATE */ +void generate_key(int family_arg, int bits_arg, + int output_size_arg, + psa_status_t expected_status) +{ + psa_ecc_family_t family = family_arg; + size_t bits = bits_arg; + size_t output_size = output_size_arg; + + uint8_t *output = NULL; + size_t output_length = SIZE_MAX; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_set_key_type(&attributes, PSA_KEY_TYPE_ECC_KEY_PAIR(family)); + psa_set_key_bits(&attributes, bits); + ecc_private_key_stats_t stats = { 0, 0 }; + + PSA_INIT(); + TEST_CALLOC(output, output_size); + + /* In success cases, run multiple iterations so that we can make + * statistical observations. */ + unsigned iteration_count = expected_status == PSA_SUCCESS ? 256 : 1; + for (unsigned i = 0; i < iteration_count; i++) { + mbedtls_test_set_step(i); + TEST_EQUAL(mbedtls_psa_ecp_generate_key(&attributes, + output, output_size, + &output_length), + expected_status); + if (expected_status == PSA_SUCCESS) { + TEST_LE_U(output_length, output_size); + TEST_ASSERT(check_ecc_private_key(family, bits, + output, output_length, + &stats)); + } + } + + if (expected_status == PSA_SUCCESS) { + /* For selected bits, check that we saw the values 0 and 1 each + * at least some minimum number of times. The iteration count and + * the minimum are chosen so that a random failure is unlikely + * to more than cryptographic levels. */ + unsigned const min_times = 10; + TEST_LE_U(min_times, stats.bit_bot); + TEST_LE_U(stats.bit_bot, iteration_count - min_times); + TEST_LE_U(min_times, stats.bit_top); + TEST_LE_U(stats.bit_top, iteration_count - min_times); + } + +exit: + PSA_DONE(); + mbedtls_free(output); +} +/* END_CASE */