Merge pull request #8860 from gilles-peskine-arm/ecp-write-doc-2.28

Backport 2.28: Document ECP write functions
Mbed-TLS · Feb 28, 2024 · cb086af · cb086af
2 parents 36e6bd6 + 9721b86
commit cb086af
Show file tree

Hide file tree

Showing 4 changed files with 216 additions and 12 deletions.
diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h
@@ -1265,6 +1265,8 @@ int mbedtls_ecp_gen_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key,
 /**
  * \brief           This function reads an elliptic curve private key.
  *
+ * \note            This function does not support Curve448 yet.
+ *
  * \param grp_id    The ECP group identifier.
  * \param key       The destination key.
  * \param buf       The buffer containing the binary representation of the
@@ -1286,17 +1288,43 @@ int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key,
 /**
  * \brief           This function exports an elliptic curve private key.
  *
+ * \note            Note that although this function accepts an output
+ *                  buffer that is smaller or larger than the key, most key
+ *                  import interfaces require the output to have exactly
+ *                  key's nominal length. It is generally simplest to
+ *                  pass the key's nominal length as \c buflen, after
+ *                  checking that the output buffer is large enough.
+ *                  See the description of the \p buflen parameter for
+ *                  how to calculate the nominal length.
+ *
+ * \note            If the private key was not set in \p key,
+ *                  the output is unspecified. Future versions
+ *                  may return an error in that case.
+ *
+ * \note            This function does not support Curve448 yet.
+ *
  * \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.)
+ *                  of the key.
+ *                  For Weierstrass curves, this is the big-endian
+ *                  representation, padded with null bytes at the beginning
+ *                  to reach \p buflen bytes.
+ *                  For Montgomery curves, this is the standard byte string
+ *                  representation (which is little-endian), padded with
+ *                  null bytes at the end to reach \p buflen bytes.
  * \param buflen    The total length of the buffer in bytes.
+ *                  The length of the output is
+ *                  (`grp->nbits` + 7) / 8 bytes
+ *                  where `grp->nbits` is the private key size in bits.
+ *                  For Weierstrass keys, if the output buffer is smaller,
+ *                  leading zeros are trimmed to fit if possible. For
+ *                  Montgomery keys, the output buffer must always be large
+ *                  enough for the nominal length.
  *
  * \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          #MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL or
+ *                  #MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL if the \p key
+ *                  representation is larger than the available space in \p buf.
  * \return          Another negative error code on different kinds of failure.
  */
 int mbedtls_ecp_write_key(mbedtls_ecp_keypair *key,

diff --git a/library/ecp.c b/library/ecp.c
@@ -927,7 +927,7 @@ int mbedtls_ecp_point_read_binary(const mbedtls_ecp_group *grp,
     size_t plen;
     ECP_VALIDATE_RET(grp != NULL);
     ECP_VALIDATE_RET(pt  != NULL);
-    ECP_VALIDATE_RET(buf != NULL);
+    ECP_VALIDATE_RET(ilen == 0 || buf != NULL);
 
     if (ilen < 1) {
         return MBEDTLS_ERR_ECP_BAD_INPUT_DATA;
@@ -996,7 +996,7 @@ int mbedtls_ecp_tls_read_point(const mbedtls_ecp_group *grp,
     ECP_VALIDATE_RET(grp != NULL);
     ECP_VALIDATE_RET(pt  != NULL);
     ECP_VALIDATE_RET(buf != NULL);
-    ECP_VALIDATE_RET(*buf != NULL);
+    ECP_VALIDATE_RET(buf_len == 0 || *buf != NULL);
 
     /*
      * We must have at least two bytes (1 for length, at least one for data)
@@ -1068,7 +1068,7 @@ int mbedtls_ecp_tls_read_group(mbedtls_ecp_group *grp,
     mbedtls_ecp_group_id grp_id;
     ECP_VALIDATE_RET(grp  != NULL);
     ECP_VALIDATE_RET(buf  != NULL);
-    ECP_VALIDATE_RET(*buf != NULL);
+    ECP_VALIDATE_RET(len == 0 || *buf != NULL);
 
     if ((ret = mbedtls_ecp_tls_read_group_id(&grp_id, buf, len)) != 0) {
         return ret;
@@ -1088,7 +1088,7 @@ int mbedtls_ecp_tls_read_group_id(mbedtls_ecp_group_id *grp,
     const mbedtls_ecp_curve_info *curve_info;
     ECP_VALIDATE_RET(grp  != NULL);
     ECP_VALIDATE_RET(buf  != NULL);
-    ECP_VALIDATE_RET(*buf != NULL);
+    ECP_VALIDATE_RET(len == 0 || *buf != NULL);
 
     /*
      * We expect at least three bytes (see below)
@@ -3358,10 +3358,10 @@ int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key,
 int mbedtls_ecp_write_key(mbedtls_ecp_keypair *key,
                           unsigned char *buf, size_t buflen)
 {
-    int ret = MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE;
+    int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
 
     ECP_VALIDATE_RET(key != NULL);
-    ECP_VALIDATE_RET(buf != NULL);
+    ECP_VALIDATE_RET(buflen == 0 || buf != NULL);
 
 #if defined(MBEDTLS_ECP_MONTGOMERY_ENABLED)
     if (mbedtls_ecp_get_type(&key->grp) == MBEDTLS_ECP_TYPE_MONTGOMERY) {

diff --git a/tests/suites/test_suite_ecp.data b/tests/suites/test_suite_ecp.data
@@ -550,6 +550,122 @@ 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 write key: secp256r1, nominal
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":32:0
+
+ECP write key: secp256r1, output longer by 1
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":33:0
+
+ECP write key: secp256r1, output longer by 32
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":64:0
+
+ECP write key: secp256r1, output longer by 33
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":65:0
+
+ECP write key: secp256r1, output short by 1
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":31:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL
+
+ECP write key: secp256r1, output_size=1
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":1:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL
+
+ECP write key: secp256r1, output_size=0
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":0:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL
+
+ECP write key: secp256r1, top byte = 0, output_size=32
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff":32:0
+
+ECP write key: secp256r1, top byte = 0, output_size=31 (fits)
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff":31:0
+
+ECP write key: secp256r1, top byte = 0, output_size=30 (too small)
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff":30:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL
+
+ECP write key: secp256r1, mostly-0 key, output_size=32
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"0000000000000000000000000000000000000000000000000000000000000001":32:0
+
+ECP write key: secp256r1, mostly-0 key, output_size=31 (fits)
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"0000000000000000000000000000000000000000000000000000000000000001":31:0
+
+ECP write key: secp256r1, mostly-0 key, output_size=1 (fits)
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"0000000000000000000000000000000000000000000000000000000000000001":1:0
+
+ECP write key: secp384r1, nominal
+depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":48:0
+
+ECP write key: secp384r1, output longer by 1
+depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":49:0
+
+ECP write key: secp384r1, output longer by 48
+depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":96:0
+
+ECP write key: secp384r1, output longer by 49
+depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":97:0
+
+ECP write key: secp384r1, output short by 1
+depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":47:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL
+
+ECP write key: secp384r1, output_size=1
+depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":1:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL
+
+ECP write key: secp384r1, output_size=0
+depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":0:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL
+
+ECP write key: Curve25519, nominal
+depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":32:0
+
+ECP write key: Curve25519, output longer by 1
+depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":33:0
+
+ECP write key: Curve25519, output longer by 32
+depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":64:0
+
+ECP write key: Curve25519, output longer by 33
+depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":65:0
+
+ECP write key: Curve25519, output short by 1
+depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":31:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL
+
+ECP write key: Curve25519, output_size=1
+depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":1:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL
+
+ECP write key: Curve25519, output_size=0
+depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":0:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL
+
+ECP write key: Curve25519, mostly-0 key, output_size=32
+depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"0000000000000000000000000000000000000000000000000000000000000040":32:0
+
+ECP write key: Curve25519, mostly-0 key, output_size=31
+depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED
+ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"0000000000000000000000000000000000000000000000000000000000000040":31:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL
+
 ECP mod p192 small (more than 192 bits, less limbs than 2 * 192 bits)
 depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED
 ecp_fast_mod:MBEDTLS_ECP_DP_SECP192R1:"0100000000000103010000000000010201000000000001010100000000000100"

diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function
@@ -1383,6 +1383,66 @@ exit:
 }
 /* END_CASE */
 
+/* BEGIN_CASE */
+void ecp_write_key(int grp_id, data_t *in_key,
+                   int exported_size, int expected_ret)
+{
+    mbedtls_ecp_keypair key;
+    mbedtls_ecp_keypair_init(&key);
+    unsigned char *exported = NULL;
+
+    TEST_EQUAL(mbedtls_ecp_read_key(grp_id, &key, in_key->x, in_key->len), 0);
+
+    TEST_CALLOC(exported, exported_size);
+    TEST_EQUAL(mbedtls_ecp_write_key(&key, exported, exported_size),
+               expected_ret);
+
+    if (expected_ret == 0) {
+        size_t length = (key.grp.nbits + 7) / 8;
+        const unsigned char *key_start = NULL;
+        const unsigned char *zeros_start = NULL;
+        switch (mbedtls_ecp_get_type(&key.grp)) {
+            case MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS:
+                if ((size_t) exported_size < length) {
+                    length = exported_size;
+                }
+                key_start = exported + exported_size - length;
+                zeros_start = exported;
+                break;
+            case MBEDTLS_ECP_TYPE_MONTGOMERY:
+                TEST_LE_U(length, exported_size);
+                key_start = exported;
+                zeros_start = exported + length;
+                break;
+            default:
+                TEST_FAIL("Unknown ECP curve type");
+                break;
+        }
+
+        if (length < in_key->len) {
+            /* Shorter output (only possible with Weierstrass keys) */
+            for (size_t i = 0; i < in_key->len - length; i++) {
+                mbedtls_test_set_step(i);
+                TEST_EQUAL(in_key->x[i], 0);
+            }
+            TEST_MEMORY_COMPARE(in_key->x + in_key->len - length, length,
+                                key_start, length);
+        } else {
+            TEST_MEMORY_COMPARE(in_key->x, in_key->len,
+                                key_start, length);
+            for (size_t i = 0; i < exported_size - length; i++) {
+                mbedtls_test_set_step(i);
+                TEST_EQUAL(zeros_start[i], 0);
+            }
+        }
+    }
+
+exit:
+    mbedtls_ecp_keypair_free(&key);
+    mbedtls_free(exported);
+}
+/* END_CASE */
+
 /* BEGIN_CASE depends_on:HAVE_FIX_NEGATIVE */
 void fix_negative(data_t *N_bin, int c, int bits)
 {