ECC import: more useful choice of INVALID_ARGUMENT vs NOT_SUPPORTED
Attempting to create an ECC key with a curve specification that is not
valid can plausibly fail with PSA_ERROR_INVALID_ARGUMENT ("this is not
a curve specification at all") or PSA_ERROR_NOT_SUPPORTED ("this may
be a curve specification, but not one I support"). The choice of error
is somewhat subjective.
Before this commit, due to happenstance in the implementation, an
attempt to use a curve that is declared in the PSA API but not
implemented in Mbed TLS returned PSA_ERROR_INVALID_ARGUMENT, whereas
an attempt to use a curve that Mbed TLS supports but for which support
was disabled at compile-time returned PSA_ERROR_NOT_SUPPORTED. This
inconsistency made it difficult to write negative tests that could
work whether the curve is implemented via Mbed TLS code or via a
driver.
After this commit, any attempt to use parameters that are not
recognized fails with NOT_SUPPORTED, whether a curve with the
specified size might plausibly exist or not, because "might plausibly
exist" is not something Mbed TLS can determine.
To keep returning INVALID_ARGUMENT when importing an ECC key with an
explicit "bits" attribute that is inconsistent with the size of the
key material, this commit changes the way mbedtls_ecc_group_of_psa()
works: it now works on a size in bits rather than bytes, with an extra
flag indicating whether the bit-size must be exact or not.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 88e25e1..c00875b 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -413,71 +413,71 @@
defined(MBEDTLS_PSA_ACCEL_KEY_TYPE_ECC_KEY_PAIR) || \
defined(MBEDTLS_PSA_ACCEL_KEY_TYPE_ECC_PUBLIC_KEY)
mbedtls_ecp_group_id mbedtls_ecc_group_of_psa( psa_ecc_family_t curve,
- size_t byte_length )
+ size_t bits,
+ int bits_is_sloppy )
{
switch( curve )
{
case PSA_ECC_FAMILY_SECP_R1:
- switch( byte_length )
+ switch( bits )
{
- case PSA_BITS_TO_BYTES( 192 ):
+ case 192:
return( MBEDTLS_ECP_DP_SECP192R1 );
- case PSA_BITS_TO_BYTES( 224 ):
+ case 224:
return( MBEDTLS_ECP_DP_SECP224R1 );
- case PSA_BITS_TO_BYTES( 256 ):
+ case 256:
return( MBEDTLS_ECP_DP_SECP256R1 );
- case PSA_BITS_TO_BYTES( 384 ):
+ case 384:
return( MBEDTLS_ECP_DP_SECP384R1 );
- case PSA_BITS_TO_BYTES( 521 ):
+ case 521:
return( MBEDTLS_ECP_DP_SECP521R1 );
- default:
- return( MBEDTLS_ECP_DP_NONE );
+ case 528:
+ if( bits_is_sloppy )
+ return( MBEDTLS_ECP_DP_SECP521R1 );
+ break;
}
break;
case PSA_ECC_FAMILY_BRAINPOOL_P_R1:
- switch( byte_length )
+ switch( bits )
{
- case PSA_BITS_TO_BYTES( 256 ):
+ case 256:
return( MBEDTLS_ECP_DP_BP256R1 );
- case PSA_BITS_TO_BYTES( 384 ):
+ case 384:
return( MBEDTLS_ECP_DP_BP384R1 );
- case PSA_BITS_TO_BYTES( 512 ):
+ case 512:
return( MBEDTLS_ECP_DP_BP512R1 );
- default:
- return( MBEDTLS_ECP_DP_NONE );
}
break;
case PSA_ECC_FAMILY_MONTGOMERY:
- switch( byte_length )
+ switch( bits )
{
- case PSA_BITS_TO_BYTES( 255 ):
+ case 255:
return( MBEDTLS_ECP_DP_CURVE25519 );
- case PSA_BITS_TO_BYTES( 448 ):
+ case 256:
+ if( bits_is_sloppy )
+ return( MBEDTLS_ECP_DP_CURVE25519 );
+ break;
+ case 448:
return( MBEDTLS_ECP_DP_CURVE448 );
- default:
- return( MBEDTLS_ECP_DP_NONE );
}
break;
case PSA_ECC_FAMILY_SECP_K1:
- switch( byte_length )
+ switch( bits )
{
- case PSA_BITS_TO_BYTES( 192 ):
+ case 192:
return( MBEDTLS_ECP_DP_SECP192K1 );
- case PSA_BITS_TO_BYTES( 224 ):
+ case 224:
return( MBEDTLS_ECP_DP_SECP224K1 );
- case PSA_BITS_TO_BYTES( 256 ):
+ case 256:
return( MBEDTLS_ECP_DP_SECP256K1 );
- default:
- return( MBEDTLS_ECP_DP_NONE );
}
break;
-
- default:
- return( MBEDTLS_ECP_DP_NONE );
}
+
+ return( MBEDTLS_ECP_DP_NONE );
}
#endif /* defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_ECC_KEY_PAIR) ||
* defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_ECC_PUBLIC_KEY) ||
@@ -3472,6 +3472,7 @@
{
mbedtls_ecp_keypair *ecp = NULL;
status = mbedtls_psa_ecp_load_representation( slot->attr.type,
+ slot->attr.bits,
slot->key.data,
slot->key.bytes,
&ecp );
@@ -3575,6 +3576,7 @@
{
mbedtls_ecp_keypair *ecp = NULL;
status = mbedtls_psa_ecp_load_representation( slot->attr.type,
+ slot->attr.bits,
slot->key.data,
slot->key.bytes,
&ecp );
@@ -5647,6 +5649,7 @@
status = mbedtls_psa_ecp_load_representation(
PSA_KEY_TYPE_ECC_PUBLIC_KEY(curve),
+ bits,
peer_key,
peer_key_length,
&their_key );
@@ -5703,6 +5706,7 @@
mbedtls_ecp_keypair *ecp = NULL;
psa_status_t status = mbedtls_psa_ecp_load_representation(
private_key->attr.type,
+ private_key->attr.bits,
private_key->key.data,
private_key->key.bytes,
&ecp );
@@ -6115,7 +6119,7 @@
{
psa_ecc_family_t curve = PSA_KEY_TYPE_ECC_GET_FAMILY( type );
mbedtls_ecp_group_id grp_id =
- mbedtls_ecc_group_of_psa( curve, PSA_BITS_TO_BYTES( bits ) );
+ mbedtls_ecc_group_of_psa( curve, bits, 0 );
const mbedtls_ecp_curve_info *curve_info =
mbedtls_ecp_curve_info_from_grp_id( grp_id );
mbedtls_ecp_keypair ecp;