Merge pull request #3425 from stevew817/montgomery-keys-clarification

Fix endianness handling of Curve25519 in PSA Crypto core

ChangeLog.d/psa_curve25519_key_support.txt

@@ -0,0 +1,9 @@
+Features
+   * The new function mbedtls_ecp_write_key() exports private ECC keys back to
+     a byte buffer. It is the inverse of the existing mbedtls_ecp_read_key().
+
+Bugfix
+   * Fix the endianness of Curve25519 keys imported/exported through the PSA
+     APIs. psa_import_key and psa_export_key will now correctly expect/output
+     Montgomery keys in little-endian as defined by RFC7748. Contributed by
+     Steven Cooreman in #3425.

include/mbedtls/ecp.h

@@ -1149,6 +1149,26 @@ int mbedtls_ecp_gen_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key,
  */
 int mbedtls_ecp_read_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key,
                           const unsigned char *buf, size_t buflen );
+
+/**
+ * \brief           This function exports an elliptic curve private key.
+ *
+ * \param key       The private key.
+ * \param buf       The output buffer for containing the binary representation
+ *                  of the key. (Big endian integer for Weierstrass curves, byte
+ *                  string for Montgomery curves.)
+ * \param buflen    The total length of the buffer in bytes.
+ *
+ * \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_FEATURE_UNAVAILABLE if the operation for
+ *                  the group is not implemented.
+ * \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 );
+
 /**
  * \brief           This function checks that the keypair objects
  *                  \p pub and \p prv have the same group and the

include/psa/crypto.h

@@ -738,8 +738,9 @@ psa_status_t psa_import_key(const psa_key_attributes_t *attributes,
  *   `PSA_ECC_FAMILY_CURVEXXX`), and in big-endian order for Weierstrass
  *   curves (curve types `PSA_ECC_FAMILY_SECTXXX`, `PSA_ECC_FAMILY_SECPXXX`
  *   and `PSA_ECC_FAMILY_BRAINPOOL_PXXX`).
- *   This is the content of the `privateKey` field of the `ECPrivateKey`
+ *   For Weierstrass curves, this is the content of the `privateKey` field of
- *   format defined by RFC 5915.
+ *   the `ECPrivateKey` format defined by RFC 5915.  For Montgomery curves,
+ *   the format is defined by RFC 7748, and output is masked according to §5.
  * - For Diffie-Hellman key exchange key pairs (key types for which
  *   #PSA_KEY_TYPE_IS_DH_KEY_PAIR is true), the
  *   format is the representation of the private key `x` as a big-endian byte

library/ecp.c

@@ -3224,6 +3224,45 @@ cleanup:
     return( ret );
 }
 
+/*
+ * Write a private key.
+ */
+int mbedtls_ecp_write_key( mbedtls_ecp_keypair *key,
+                           unsigned char *buf, size_t buflen )
+{
+    int ret = MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE;
+
+    ECP_VALIDATE_RET( key != NULL );
+    ECP_VALIDATE_RET( buf != NULL );
+
+#if defined(ECP_MONTGOMERY)
+    if( mbedtls_ecp_get_type( &key->grp ) == MBEDTLS_ECP_TYPE_MONTGOMERY )
+    {
+        if( key->grp.id == MBEDTLS_ECP_DP_CURVE25519 )
+        {
+            if( buflen < ECP_CURVE25519_KEY_SIZE )
+                return MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL;
+
+            MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary_le( &key->d, buf, buflen ) );
+        }
+        else
+            ret = MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE;
+    }
+
+#endif
+#if defined(ECP_SHORTWEIERSTRASS)
+    if( mbedtls_ecp_get_type( &key->grp ) == MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS )
+    {
+        MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( &key->d, buf, buflen ) );
+    }
+
+#endif
+cleanup:
+
+    return( ret );
+}
+
+
 /*
  * Check a public-private key pair
  */

library/pkwrite.c

@@ -164,7 +164,7 @@ static int pk_write_ec_private( unsigned char **p, unsigned char *start,
     size_t byte_length = ( ec->grp.pbits + 7 ) / 8;
     unsigned char tmp[MBEDTLS_ECP_MAX_BYTES];
 
-    ret = mbedtls_mpi_write_binary( &ec->d, tmp, byte_length );
+    ret = mbedtls_ecp_write_key( ec, tmp, byte_length );
     if( ret != 0 )
         goto exit;
     ret = mbedtls_asn1_write_octet_string( p, start, tmp, byte_length );

library/psa_crypto.c

@@ -667,16 +667,12 @@ static psa_status_t psa_import_ec_private_key( psa_ecc_family_t curve,
     if( status != PSA_SUCCESS )
         goto exit;
 
-    /* Load the secret value. */
+    /* Load and validate the secret key */
     status = mbedtls_to_psa_error(
-        mbedtls_mpi_read_binary( &ecp->d, data, data_length ) );
+        mbedtls_ecp_read_key( ecp->grp.id, ecp, data, data_length ) );
-    if( status != PSA_SUCCESS )
-        goto exit;
-    /* Validate the private key. */
-    status = mbedtls_to_psa_error(
-        mbedtls_ecp_check_privkey( &ecp->grp, &ecp->d ) );
     if( status != PSA_SUCCESS )
         goto exit;
+
     /* Calculate the public key from the private key. */
     status = mbedtls_to_psa_error(
         mbedtls_ecp_mul( &ecp->grp, &ecp->Q, &ecp->d, &ecp->grp.G,
@@ -1327,7 +1323,8 @@ static psa_status_t psa_internal_export_key( const psa_key_slot_t *slot,
         if( bytes > data_size )
             return( PSA_ERROR_BUFFER_TOO_SMALL );
         status = mbedtls_to_psa_error(
-            mbedtls_mpi_write_binary( &slot->data.ecp->d, data, bytes ) );
+            mbedtls_ecp_write_key( slot->data.ecp,
+                                   data, bytes ) );
         if( status != PSA_SUCCESS )
             return( status );
         memset( data + bytes, 0, data_size - bytes );

tests/suites/test_suite_ecp.data

@@ -278,65 +278,69 @@ mbedtls_ecp_gen_key:MBEDTLS_ECP_DP_SECP192R1
 
 ECP read key #1 (short weierstrass, too small)
 depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED
-mbedtls_ecp_read_key:MBEDTLS_ECP_DP_SECP192R1:"00":MBEDTLS_ERR_ECP_INVALID_KEY
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_SECP192R1:"00":MBEDTLS_ERR_ECP_INVALID_KEY:0
 
 ECP read key #2 (short weierstrass, smallest)
 depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED
-mbedtls_ecp_read_key:MBEDTLS_ECP_DP_SECP192R1:"01":0
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_SECP192R1:"01":0:1
 
 ECP read key #3 (short weierstrass, biggest)
 depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED
-mbedtls_ecp_read_key:MBEDTLS_ECP_DP_SECP192R1:"FFFFFFFFFFFFFFFFFFFFFFFF99DEF836146BC9B1B4D22830":0
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_SECP192R1:"FFFFFFFFFFFFFFFFFFFFFFFF99DEF836146BC9B1B4D22830":0:1
 
 ECP read key #4 (short weierstrass, too big)
 depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED
-mbedtls_ecp_read_key:MBEDTLS_ECP_DP_SECP192R1:"FFFFFFFFFFFFFFFFFFFFFFFF99DEF836146BC9B1B4D22831":MBEDTLS_ERR_ECP_INVALID_KEY
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_SECP192R1:"FFFFFFFFFFFFFFFFFFFFFFFF99DEF836146BC9B1B4D22831":MBEDTLS_ERR_ECP_INVALID_KEY:0
 
 ECP read key #5 (Curve25519, most significant bit set)
 depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
-mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"000000000000000000000000000000000000000000000000000000000000000C":0
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"00000000000000000000000000000000000000000000000000000000000000C0":0:0
 
 ECP read key #6 (Curve25519, second most significant bit unset)
 depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
-mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"0FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF3":0
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"F0FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF3F":0:0
 
 ECP read key #7 (Curve25519, msb OK)
 depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
-mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"0000000000000000000000000000000000000000000000000000000000000004":0
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"0000000000000000000000000000000000000000000000000000000000000040":0:1
 
 ECP read key #8 (Curve25519, bit 0 set)
 depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
-mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"1000000000000000000000000000000000000000000000000000000000000000":0
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"0100000000000000000000000000000000000000000000000000000000000040":0:0
 
 ECP read key #9 (Curve25519, bit 1 set)
 depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
-mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"2000000000000000000000000000000000000000000000000000000000000004":0
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"0200000000000000000000000000000000000000000000000000000000000040":0:0
 
 ECP read key #10 (Curve25519, bit 2 set)
 depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
-mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"4000000000000000000000000000000000000000000000000000000000000004":0
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"0400000000000000000000000000000000000000000000000000000000000040":0:0
 
 ECP read key #11 (Curve25519, OK)
 depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
-mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"8FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7":0
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"F8FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7F":0:1
 
 ECP read key #12 (Curve25519, too long)
 depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
-mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"00000000000000000000000000000000000000000000000000000000000000000C":MBEDTLS_ERR_ECP_INVALID_KEY
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"00000000000000000000000000000000000000000000000000000000000000000C":MBEDTLS_ERR_ECP_INVALID_KEY:0
 
 ECP read key #13 (Curve25519, not long enough)
 depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
-mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"0FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF3":MBEDTLS_ERR_ECP_INVALID_KEY
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"F0FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF3F":MBEDTLS_ERR_ECP_INVALID_KEY:0
 
 ECP read key #14 (Curve448, not supported)
-mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE448:"FCFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF":MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE448:"FCFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF":MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE:0
 
 ECP read key #15 (Curve25519, not supported)
 depends_on:!MBEDTLS_ECP_DP_CURVE25519_ENABLED
-mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"8FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7":MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"F8FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7F":MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE:0
 
 ECP read key #15 (invalid curve)
-mbedtls_ecp_read_key:INT_MAX:"8FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7":MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE
+mbedtls_ecp_read_key:INT_MAX:"F8FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7F":MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE:0
+
+ECP read key #16 (Curve25519 RFC, OK)
+depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
+mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"70076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c6a":0:1
 
 ECP mod p192 small (more than 192 bits, less limbs than 2 * 192 bits)
 depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED

tests/suites/test_suite_ecp.function

@@ -1100,12 +1100,14 @@ exit:
 /* END_CASE */
 
 /* BEGIN_CASE */
-void mbedtls_ecp_read_key( int grp_id, data_t* in_key, int expected )
+void mbedtls_ecp_read_key( int grp_id, data_t* in_key, int expected, int canonical )
 {
     int ret = 0;
     mbedtls_ecp_keypair key;
+    mbedtls_ecp_keypair key2;
 
     mbedtls_ecp_keypair_init( &key );
+    mbedtls_ecp_keypair_init( &key2 );
 
     ret = mbedtls_ecp_read_key( grp_id, &key, in_key->x, in_key->len );
     TEST_ASSERT( ret == expected );
@@ -1114,10 +1116,39 @@ void mbedtls_ecp_read_key( int grp_id, data_t* in_key, int expected )
     {
         ret = mbedtls_ecp_check_privkey( &key.grp, &key.d );
         TEST_ASSERT( ret == 0 );
+
+        if( canonical )
+        {
+            unsigned char buf[MBEDTLS_ECP_MAX_BYTES];
+
+            ret = mbedtls_ecp_write_key( &key, buf, in_key->len );
+            TEST_ASSERT( ret == 0 );
+
+            ASSERT_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 );
+
+            ASSERT_COMPARE( export1, in_key->len,
+                            export2, in_key->len );
+        }
     }
 
 exit:
     mbedtls_ecp_keypair_free( &key );
+    mbedtls_ecp_keypair_free( &key2 );
 }
 /* END_CASE */
 

tests/suites/test_suite_psa_crypto.data

@@ -220,6 +220,22 @@ PSA import/export-public EC brainpool512r1: good
 depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_PK_WRITE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_BP512R1_ENABLED
 import_export_public_key:"372c9778f69f726cbca3f4a268f16b4d617d10280d79a6a029cd51879fe1012934dfe5395455337df6906dc7d6d2eea4dbb2065c0228f73b3ed716480e7d71d2":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_BRAINPOOL_P_R1):PSA_ALG_ECDSA_ANY:0:PSA_SUCCESS:"0438b7ec92b61c5c6c7fbc28a4ec759d48fcd4e2e374defd5c4968a54dbef7510e517886fbfc38ea39aa529359d70a7156c35d3cbac7ce776bdb251dd64bce71234424ee7049eed072f0dbc4d79996e175d557e263763ae97095c081e73e7db2e38adc3d4c9a0487b1ede876dc1fca61c902e9a1d8722b8612928f18a24845591a"
 
+PSA import/export EC curve25519 key pair: good (already properly masked)
+depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_PK_WRITE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_CURVE25519_ENABLED
+import_export:"70076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c6a":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_MONTGOMERY):PSA_KEY_USAGE_EXPORT:PSA_ALG_ECDH:255:0:PSA_SUCCESS:1
+
+PSA import/export EC curve25519 key pair: unmasked input (check export-import-export yields properly masked output)
+depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_PK_WRITE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_CURVE25519_ENABLED
+import_export:"77076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c2a":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_MONTGOMERY):PSA_KEY_USAGE_EXPORT:PSA_ALG_ECDH:255:0:PSA_SUCCESS:0
+
+PSA import/export-public EC curve25519: accept unmasked input
+depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_PK_WRITE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_CURVE25519_ENABLED
+import_export_public_key:"77076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c2a":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_MONTGOMERY):PSA_ALG_ECDH:0:PSA_SUCCESS:"8520f0098930a754748b7ddcb43ef75a0dbf3a0d26381af4eba4a98eaa9b4e6a"
+
+PSA import/export-public EC curve25519: accept masked input
+depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_PK_WRITE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_CURVE25519_ENABLED
+import_export_public_key:"70076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c6a":PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_MONTGOMERY):PSA_ALG_ECDH:0:PSA_SUCCESS:"8520f0098930a754748b7ddcb43ef75a0dbf3a0d26381af4eba4a98eaa9b4e6a"
+
 PSA import/export-public: cannot export-public a symmetric key
 depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_PK_WRITE_C:MBEDTLS_RSA_C
 import_export_public_key:"2b7e151628aed2a6abf7158809cf4f3c":PSA_KEY_TYPE_AES:PSA_ALG_CBC_NO_PADDING:0:PSA_ERROR_INVALID_ARGUMENT:""