From 8276986c3ea14f51a9dc96370e7851b1c44d2288 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Mar 2021 12:06:16 +0100 Subject: [PATCH 1/5] Curve448 is not yet supported via the PSA API Filed as https://github.com/ARMmbed/mbedtls/issues/4249. In the meantime, disable the feature. Signed-off-by: Gilles Peskine --- include/mbedtls/config_psa.h | 3 ++- include/psa/crypto_config.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/config_psa.h b/include/mbedtls/config_psa.h index ea822803b991..c46ed56a5099 100644 --- a/include/mbedtls/config_psa.h +++ b/include/mbedtls/config_psa.h @@ -642,7 +642,8 @@ extern "C" { #define PSA_WANT_ECC_MONTGOMERY_255 #endif -#if defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) +/* Curve448 is not yet supported via the PSA API (https://github.com/ARMmbed/mbedtls/issues/4249) */ +#if 0 && defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) #define MBEDTLS_PSA_BUILTIN_ECC_MONTGOMERY_448 1 #define PSA_WANT_ECC_MONTGOMERY_448 #endif diff --git a/include/psa/crypto_config.h b/include/psa/crypto_config.h index 97395d89492b..bad1e34f2f0d 100644 --- a/include/psa/crypto_config.h +++ b/include/psa/crypto_config.h @@ -84,7 +84,8 @@ #define PSA_WANT_ECC_BRAINPOOL_P_R1_384 1 #define PSA_WANT_ECC_BRAINPOOL_P_R1_512 1 #define PSA_WANT_ECC_MONTGOMERY_255 1 -#define PSA_WANT_ECC_MONTGOMERY_448 1 +/* Curve448 is not yet supported via the PSA API (https://github.com/ARMmbed/mbedtls/issues/4249) */ +//#define PSA_WANT_ECC_MONTGOMERY_448 1 #define PSA_WANT_ECC_SECP_K1_192 1 #define PSA_WANT_ECC_SECP_K1_224 1 #define PSA_WANT_ECC_SECP_K1_256 1 From 398413024def98cb82771db57af2a4a89075e5fe Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Mar 2021 12:06:45 +0100 Subject: [PATCH 2/5] SECP224K1 is not yet supported via the PSA API Filed as https://github.com/ARMmbed/mbedtls/issues/3541. In the meantime, disable the feature. Signed-off-by: Gilles Peskine --- include/mbedtls/config_psa.h | 3 ++- include/psa/crypto_config.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/config_psa.h b/include/mbedtls/config_psa.h index c46ed56a5099..39a500163435 100644 --- a/include/mbedtls/config_psa.h +++ b/include/mbedtls/config_psa.h @@ -678,7 +678,8 @@ extern "C" { #define PSA_WANT_ECC_SECP_K1_192 #endif -#if defined(MBEDTLS_ECP_DP_SECP224K1_ENABLED) +/* SECP224K1 is buggy via the PSA API (https://github.com/ARMmbed/mbedtls/issues/3541) */ +#if 0 && defined(MBEDTLS_ECP_DP_SECP224K1_ENABLED) #define MBEDTLS_PSA_BUILTIN_ECC_SECP_K1_224 1 #define PSA_WANT_ECC_SECP_K1_224 #endif diff --git a/include/psa/crypto_config.h b/include/psa/crypto_config.h index bad1e34f2f0d..afbaa66e54e3 100644 --- a/include/psa/crypto_config.h +++ b/include/psa/crypto_config.h @@ -87,7 +87,8 @@ /* Curve448 is not yet supported via the PSA API (https://github.com/ARMmbed/mbedtls/issues/4249) */ //#define PSA_WANT_ECC_MONTGOMERY_448 1 #define PSA_WANT_ECC_SECP_K1_192 1 -#define PSA_WANT_ECC_SECP_K1_224 1 +/* SECP224K1 is buggy via the PSA API (https://github.com/ARMmbed/mbedtls/issues/3541) */ +//#define PSA_WANT_ECC_SECP_K1_224 1 #define PSA_WANT_ECC_SECP_K1_256 1 #define PSA_WANT_ECC_SECP_R1_192 1 #define PSA_WANT_ECC_SECP_R1_224 1 From a1684f42d33343435e8229db578ed245c79b08fd Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Mar 2021 13:12:34 +0100 Subject: [PATCH 3/5] PSA: Reject curves that are not enabled in the PSA configuration If an elliptic curve was enabled in the Mbed TLS classic API (#define MBEDTLS_ECP_DP_xxx), but not enabled in the PSA configuration (#define PSA_WANT_ECC_xxx), it would still work if you tried to use it through PSA. This is generally benign, but could be a security issue if you want to disable a curve in PSA for some security reason (such as a known bug in its implementation, which may not matter in the classic API if Mbed TLS is running in a secure enclave and is only reachable from untrusted callers through the PSA API). More urgently, this broke test_suite_psa_crypto_not_supported.generated. So if a curve is not enabled in the PSA configuration, ensure that it's treated as unsupported through the PSA software implementation. Signed-off-by: Gilles Peskine --- library/psa_crypto.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 8c61cb9683e7..a4ca1d0bfa3f 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -423,58 +423,84 @@ mbedtls_ecp_group_id mbedtls_ecc_group_of_psa( psa_ecc_family_t curve, case PSA_ECC_FAMILY_SECP_R1: switch( bits ) { +#if defined(PSA_WANT_ECC_SECP_R1_192) case 192: return( MBEDTLS_ECP_DP_SECP192R1 ); +#endif +#if defined(PSA_WANT_ECC_SECP_R1_224) case 224: return( MBEDTLS_ECP_DP_SECP224R1 ); +#endif +#if defined(PSA_WANT_ECC_SECP_R1_256) case 256: return( MBEDTLS_ECP_DP_SECP256R1 ); +#endif +#if defined(PSA_WANT_ECC_SECP_R1_384) case 384: return( MBEDTLS_ECP_DP_SECP384R1 ); +#endif +#if defined(PSA_WANT_ECC_SECP_R1_521) case 521: return( MBEDTLS_ECP_DP_SECP521R1 ); case 528: if( bits_is_sloppy ) return( MBEDTLS_ECP_DP_SECP521R1 ); break; +#endif } break; case PSA_ECC_FAMILY_BRAINPOOL_P_R1: switch( bits ) { +#if defined(PSA_WANT_ECC_BRAINPOOL_P_R1_256) case 256: return( MBEDTLS_ECP_DP_BP256R1 ); +#endif +#if defined(PSA_WANT_ECC_BRAINPOOL_P_R1_384) case 384: return( MBEDTLS_ECP_DP_BP384R1 ); +#endif +#if defined(PSA_WANT_ECC_BRAINPOOL_P_R1_512) case 512: return( MBEDTLS_ECP_DP_BP512R1 ); +#endif } break; case PSA_ECC_FAMILY_MONTGOMERY: switch( bits ) { +#if defined(PSA_WANT_ECC_MONTGOMERY_255) case 255: return( MBEDTLS_ECP_DP_CURVE25519 ); case 256: if( bits_is_sloppy ) return( MBEDTLS_ECP_DP_CURVE25519 ); break; +#endif +#if defined(PSA_WANT_ECC_MONTGOMERY_448) case 448: return( MBEDTLS_ECP_DP_CURVE448 ); +#endif } break; case PSA_ECC_FAMILY_SECP_K1: switch( bits ) { +#if defined(PSA_WANT_ECC_SECP_K1_192) case 192: return( MBEDTLS_ECP_DP_SECP192K1 ); +#endif +#if defined(PSA_WANT_ECC_SECP_K1_224) case 224: return( MBEDTLS_ECP_DP_SECP224K1 ); +#endif +#if defined(PSA_WANT_ECC_SECP_K1_256) case 256: return( MBEDTLS_ECP_DP_SECP256K1 ); +#endif } break; } From defdc3bc53bf5199a226fd0d69b24c4d4cc3dcb4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Mar 2021 13:59:58 +0100 Subject: [PATCH 4/5] SECP224K1 is not yet supported via the PSA API Filed as https://github.com/ARMmbed/mbedtls/issues/3541. In the meantime, disable the ssl-opt.sh test case that uses it. Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index abd4936605d1..6c54900cec9a 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1306,8 +1306,13 @@ requires_config_enabled MBEDTLS_ECP_DP_BP256R1_ENABLED run_test_psa_force_curve "brainpoolP256r1" requires_config_enabled MBEDTLS_ECP_DP_SECP224R1_ENABLED run_test_psa_force_curve "secp224r1" -requires_config_enabled MBEDTLS_ECP_DP_SECP224K1_ENABLED -run_test_psa_force_curve "secp224k1" +## SECP224K1 is buggy via the PSA API +## (https://github.com/ARMmbed/mbedtls/issues/3541), +## so it is disabled in PSA even when it's enabled in Mbed TLS. +## The proper dependency would be on PSA_WANT_ECC_SECP_K1_224 but +## dependencies on PSA symbols in ssl-opt.sh are not implemented yet. +#requires_config_enabled MBEDTLS_ECP_DP_SECP224K1_ENABLED +#run_test_psa_force_curve "secp224k1" requires_config_enabled MBEDTLS_ECP_DP_SECP192R1_ENABLED run_test_psa_force_curve "secp192r1" requires_config_enabled MBEDTLS_ECP_DP_SECP192K1_ENABLED From 71f45ba0e84b31ee5978ecb783ed4a6685d5dec5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Mar 2021 14:17:55 +0100 Subject: [PATCH 5/5] Fix unused parameter warning in some configurations Signed-off-by: Gilles Peskine --- library/psa_crypto.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index a4ca1d0bfa3f..5c560c29b10a 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -505,6 +505,7 @@ mbedtls_ecp_group_id mbedtls_ecc_group_of_psa( psa_ecc_family_t curve, break; } + (void) bits_is_sloppy; return( MBEDTLS_ECP_DP_NONE ); } #endif /* defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_ECC_KEY_PAIR) ||