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>

ChangeLog.d/mbedtls_ecc_group_of_psa.txt

@@ -0,0 +1,4 @@
+API changes
+   * The API glue function mbedtls_ecc_group_of_psa() now takes the curve size
+     in bits rather than bytes, with an additional flag to indicate if the
+     size may have been rounded up to a whole number of bytes.

include/psa/crypto_extra.h

@@ -637,16 +637,21 @@ static inline psa_ecc_family_t mbedtls_ecc_group_to_psa( mbedtls_ecp_group_id gr
  *
  * \param curve         A PSA elliptic curve identifier
  *                      (`PSA_ECC_FAMILY_xxx`).
- * \param byte_length   The byte-length of a private key on \p curve.
+ * \param bits          The bit-length of a private key on \p curve.
+ * \param bits_is_sloppy If true, \p bits may be the bit-length rounded up
+ *                      to the nearest multiple of 8. This allows the caller
+ *                      to infer the exact curve from the length of a key
+ *                      which is supplied as a byte string.
  *
  * \return              The corresponding Mbed TLS elliptic curve identifier
  *                      (`MBEDTLS_ECP_DP_xxx`).
  * \return              #MBEDTLS_ECP_DP_NONE if \c curve is not recognized.
- * \return              #MBEDTLS_ECP_DP_NONE if \p byte_length is not
+ * \return              #MBEDTLS_ECP_DP_NONE if \p bits is not
  *                      correct for \p curve.
  */
 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 );
 #endif /* MBEDTLS_ECP_C */
 
 /**@}*/

library/psa_crypto.c

@@ -413,71 +413,71 @@ static inline int psa_key_slot_is_external( const psa_key_slot_t *slot )
     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:
+                case 528:
-                    return( MBEDTLS_ECP_DP_NONE );
+                    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 @@ psa_status_t psa_sign_hash( mbedtls_svc_key_id_t key,
         {
             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 @@ psa_status_t psa_verify_hash( mbedtls_svc_key_id_t key,
         {
             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 @@ static psa_status_t psa_key_agreement_ecdh( const uint8_t *peer_key,
 
     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 @@ static psa_status_t psa_key_agreement_raw_internal( psa_algorithm_t alg,
             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 @@ static psa_status_t psa_generate_key_internal(
     {
         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;

library/psa_crypto_ecp.c

@@ -56,13 +56,15 @@
     defined(MBEDTLS_PSA_BUILTIN_ALG_ECDH) || \
     defined(MBEDTLS_PSA_BUILTIN_ALG_DETERMINISTIC_ECDSA)
 psa_status_t mbedtls_psa_ecp_load_representation(
-    psa_key_type_t type, const uint8_t *data, size_t data_length,
+    psa_key_type_t type, size_t curve_bits,
+    const uint8_t *data, size_t data_length,
     mbedtls_ecp_keypair **p_ecp )
 {
     mbedtls_ecp_group_id grp_id = MBEDTLS_ECP_DP_NONE;
     psa_status_t status;
     mbedtls_ecp_keypair *ecp = NULL;
-    size_t curve_size = data_length;
+    size_t curve_bytes = data_length;
+    int explicit_bits = ( curve_bits != 0 );
 
     if( PSA_KEY_TYPE_IS_PUBLIC_KEY( type ) &&
         PSA_KEY_TYPE_ECC_GET_FAMILY( type ) != PSA_ECC_FAMILY_MONTGOMERY )
@@ -75,7 +77,7 @@ psa_status_t mbedtls_psa_ecp_load_representation(
          */
         if( ( data_length & 1 ) == 0 )
             return( PSA_ERROR_INVALID_ARGUMENT );
-        curve_size = data_length / 2;
+        curve_bytes = data_length / 2;
 
         /* Montgomery public keys are represented in compressed format, meaning
          * their curve_size is equal to the amount of input. */
@@ -84,6 +86,20 @@ psa_status_t mbedtls_psa_ecp_load_representation(
          * format, meaning their curve_size is equal to the amount of input. */
     }
 
+    if( explicit_bits )
+    {
+        /* With an explicit bit-size, the data must have the matching length. */
+        if( curve_bytes != PSA_BITS_TO_BYTES( curve_bits ) )
+            return( PSA_ERROR_INVALID_ARGUMENT );
+    }
+    else
+    {
+        /* We need to infer the bit-size from the data. Since the only
+         * information we have is the length in bytes, the value of curve_bits
+         * at this stage is rounded up to the nearest multiple of 8. */
+        curve_bits = PSA_BYTES_TO_BITS( curve_bytes );
+    }
+
     /* Allocate and initialize a key representation. */
     ecp = mbedtls_calloc( 1, sizeof( mbedtls_ecp_keypair ) );
     if( ecp == NULL )
@@ -92,10 +108,16 @@ psa_status_t mbedtls_psa_ecp_load_representation(
 
     /* Load the group. */
     grp_id = mbedtls_ecc_group_of_psa( PSA_KEY_TYPE_ECC_GET_FAMILY( type ),
-                                       curve_size );
+                                       curve_bits, !explicit_bits );
     if( grp_id == MBEDTLS_ECP_DP_NONE )
     {
-        status = PSA_ERROR_INVALID_ARGUMENT;
+        /* We can't distinguish between a nonsensical family/size combination
+         * (which would warrant PSA_ERROR_INVALID_ARGUMENT) and a
+         * well-regarded curve that Mbed TLS just doesn't know about (which
+         * would warrant PSA_ERROR_NOT_SUPPORTED). For uniformity with how
+         * curves that Mbed TLS knows about but for which support is disabled
+         * at build time, return NOT_SUPPORTED. */
+        status = PSA_ERROR_NOT_SUPPORTED;
         goto exit;
     }
 
@@ -163,6 +185,7 @@ static psa_status_t ecp_import_key(
 
     /* Parse input */
     status = mbedtls_psa_ecp_load_representation( attributes->core.type,
+                                                  attributes->core.bits,
                                                   data,
                                                   data_length,
                                                   &ecp );
@@ -251,7 +274,8 @@ static psa_status_t ecp_export_public_key(
     mbedtls_ecp_keypair *ecp = NULL;
 
     status = mbedtls_psa_ecp_load_representation(
-                 attributes->core.type, key_buffer, key_buffer_size, &ecp );
+        attributes->core.type, attributes->core.bits,
+        key_buffer, key_buffer_size, &ecp );
     if( status != PSA_SUCCESS )
         return( status );
 

library/psa_crypto_ecp.h

@@ -35,6 +35,7 @@
  *                          when done.
  */
 psa_status_t mbedtls_psa_ecp_load_representation( psa_key_type_t type,
+                                                  size_t curve_bits,
                                                   const uint8_t *data,
                                                   size_t data_length,
                                                   mbedtls_ecp_keypair **p_ecp );

tests/src/drivers/key_management.c

@@ -76,7 +76,7 @@ psa_status_t test_transparent_generate_key(
         mbedtls_ecp_group_id grp_id =
             mbedtls_ecc_group_of_psa(
                 curve,
-                PSA_BITS_TO_BYTES( psa_get_key_bits( attributes ) ) );
+                psa_get_key_bits( attributes ), 0 );
         const mbedtls_ecp_curve_info *curve_info =
             mbedtls_ecp_curve_info_from_grp_id( grp_id );
         mbedtls_ecp_keypair ecp;

tests/suites/test_suite_psa_crypto.data

@@ -300,17 +300,41 @@ PSA import: reject raw data key of length 0 and declared size 8 bits
 # The spec allows either INVALID_ARGUMENT or NOT_SUPPORTED
 import_with_data:"":PSA_KEY_TYPE_RAW_DATA:8:PSA_ERROR_INVALID_ARGUMENT
 
-PSA import EC keypair: DER format
+PSA import EC keypair: explicit bit-size=255 for secp256r1
 depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
-import_with_data:"3077020101042049c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eeea00a06082a8648ce3d030107a144034200047772656f814b399279d5e1f1781fac6f099a3c5ca1b0e35351834b08b65e0b572590cdaf8f769361bcf34acfc11e5e074e8426bdde04be6e653945449617de45":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):0:PSA_ERROR_INVALID_ARGUMENT
+import_with_data:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):255:PSA_ERROR_NOT_SUPPORTED
 
-PSA import EC keypair: too short
+PSA import EC keypair: explicit bit-size=521 for secp521r1 (good)
-depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_DP_SECP521R1_ENABLED
-import_with_data:"0123456789abcdef0123456789abcdef":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):0:PSA_ERROR_INVALID_ARGUMENT
+import_with_data:"01b1b6ad07bb79e7320da59860ea28e055284f6058f279de666e06d435d2af7bda28d99fa47b7dd0963e16b0073078ee8b8a38d966a582f46d19ff95df3ad9685aae":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):521:PSA_SUCCESS
 
-PSA import EC keypair: public key
+PSA import EC keypair: explicit bit-size=528 for secp521r1 (bad)
+depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_DP_SECP521R1_ENABLED
+import_with_data:"01b1b6ad07bb79e7320da59860ea28e055284f6058f279de666e06d435d2af7bda28d99fa47b7dd0963e16b0073078ee8b8a38d966a582f46d19ff95df3ad9685aae":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):528:PSA_ERROR_NOT_SUPPORTED
+
+PSA import EC keypair: explicit bit-size, DER format
 depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
-import_with_data:"04dea5e45d0ea37fc566232a508f4ad20ea13d47e4bf5fa4d54a57a0ba012042087097496efc583fed8b24a5b9be9a51de063f5a00a8b698a16fd7f29b5485f320":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):0:PSA_ERROR_INVALID_ARGUMENT
+import_with_data:"3077020101042049c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eeea00a06082a8648ce3d030107a144034200047772656f814b399279d5e1f1781fac6f099a3c5ca1b0e35351834b08b65e0b572590cdaf8f769361bcf34acfc11e5e074e8426bdde04be6e653945449617de45":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):256:PSA_ERROR_INVALID_ARGUMENT
+
+PSA import EC keypair: explicit bit-size, too short
+depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+import_with_data:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13e":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):256:PSA_ERROR_INVALID_ARGUMENT
+
+PSA import EC keypair: explicit bit-size, too long (00 start)
+depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+import_with_data:"0049c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):256:PSA_ERROR_INVALID_ARGUMENT
+
+PSA import EC keypair: explicit bit-size, too long (00 end)
+depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+import_with_data:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee00":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):256:PSA_ERROR_INVALID_ARGUMENT
+
+PSA import EC keypair: explicit bit-size, public key
+depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+import_with_data:"04dea5e45d0ea37fc566232a508f4ad20ea13d47e4bf5fa4d54a57a0ba012042087097496efc583fed8b24a5b9be9a51de063f5a00a8b698a16fd7f29b5485f320":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):256:PSA_ERROR_INVALID_ARGUMENT
+
+PSA import EC keypair: implicit bit-size, not a valid length
+depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+import_with_data:"0123456789abcdef0123456789abcdef":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):0:PSA_ERROR_NOT_SUPPORTED
 
 PSA import EC keypair: secp256r1, all-bits-zero (bad)
 depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
@@ -332,10 +356,6 @@ PSA import EC public key: key pair
 depends_on:PSA_WANT_KEY_TYPE_ECC_PUBLIC_KEY:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
 import_with_data:"3078020101042100ab45435712649cb30bbddac49197eebf2740ffc7f874d9244c3460f54f322d3aa00a06082a8648ce3d030107a14403420004dea5e45d0ea37fc566232a508f4ad20ea13d47e4bf5fa4d54a57a0ba012042087097496efc583fed8b24a5b9be9a51de063f5a00a8b698a16fd7f29b5485f320":PSA_KEY_TYPE_ECC_PUBLIC_KEY(PSA_ECC_FAMILY_SECP_R1):0:PSA_ERROR_INVALID_ARGUMENT
 
-PSA import EC keypair: valid key but RSA
-depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_DP_BP512R1_ENABLED
-import_with_data:"3082013b020100024100ee2b131d6b1818a94ca8e91c42387eb15a7c271f57b89e7336b144d4535b16c83097ecdefbbb92d1b5313b5a37214d0e8f25922dca778b424b25295fc8a1a7070203010001024100978ac8eadb0dc6035347d6aba8671215ff21283385396f7897c04baf5e2a835f3b53ef80a82ed36ae687a925380b55a0c73eb85656e989dcf0ed7fb4887024e1022100fdad8e1c6853563f8b921d2d112462ae7d6b176082d2ba43e87e1a37fc1a8b33022100f0592cf4c55ba44307b18981bcdbda376c51e590ffa5345ba866f6962dca94dd02201995f1a967d44ff4a4cd1de837bc65bf97a2bf7eda730a9a62cea53254591105022027f96cf4b8ee68ff8d04062ec1ce7f18c0b74e4b3379b29f9bfea3fc8e592731022100cefa6d220496b43feb83194255d8fb930afcf46f36606e3aa0eb7a93ad88c10c":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_BRAINPOOL_P_R1):0:PSA_ERROR_INVALID_ARGUMENT
-
 PSA import AES: bits=0 ok
 depends_on:MBEDTLS_AES_C
 import_with_data:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_KEY_TYPE_AES:0:PSA_SUCCESS