From a778a94b7d8e241920775ea44f43a07b8f319732 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 13 Feb 2019 10:28:28 +0000 Subject: [PATCH 01/17] Add little endian import to Bignum The function `mbedtls_mpi_read_binary()` expects big endian byte order, but we need to be able to read from little endian in some caseses. (For example when handling keys corresponding to Montgomery curves.) Used `echo xx | tac -rs .. | tr [a-z] [A-Z]` to transform the test data to little endian and `echo "ibase=16;xx" | bc` to convert to decimal. --- include/mbedtls/bignum.h | 20 ++++++++++++++++++-- library/bignum.c | 28 ++++++++++++++++++++++++++++ tests/suites/test_suite_mpi.data | 3 +++ tests/suites/test_suite_mpi.function | 19 +++++++++++++++++++ 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/bignum.h b/include/mbedtls/bignum.h index a54c18e37..0727917c5 100644 --- a/include/mbedtls/bignum.h +++ b/include/mbedtls/bignum.h @@ -490,8 +490,24 @@ int mbedtls_mpi_read_binary( mbedtls_mpi *X, const unsigned char *buf, size_t buflen ); /** - * \brief Export an MPI into unsigned big endian binary data - * of fixed size. + * \brief Import X from unsigned binary data, little endian + * + * \param X The destination MPI. This must point to an initialized MPI. + * \param buf The input buffer. This must be a readable buffer of length + * \p buflen Bytes. + * \param buflen The length of the input buffer \p p in Bytes. + * + * \return \c 0 if successful. + * \return #MBEDTLS_ERR_MPI_ALLOC_FAILED if memory allocation failed. + * \return Another negative error code on different kinds of failure. + */ +int mbedtls_mpi_read_binary_le( mbedtls_mpi *X, + const unsigned char *buf, size_t buflen ); + +/** + * \brief Export X into unsigned binary data, big endian. + * Always fills the whole buffer, which will start with zeros + * if the number is smaller. * * \param X The source MPI. This must point to an initialized MPI. * \param buf The output buffer. This must be a writable buffer of length diff --git a/library/bignum.c b/library/bignum.c index 87015af0c..d28d3beb7 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -813,6 +813,34 @@ static void mpi_bigendian_to_host( mbedtls_mpi_uint * const p, size_t limbs ) } } +/* + * Import X from unsigned binary data, little endian + */ +int mbedtls_mpi_read_binary_le( mbedtls_mpi *X, + const unsigned char *buf, size_t buflen ) +{ + int ret; + size_t i; + size_t const limbs = CHARS_TO_LIMBS( buflen ); + + /* Ensure that target MPI has exactly the necessary number of limbs */ + if( X->n != limbs ) + { + mbedtls_mpi_free( X ); + mbedtls_mpi_init( X ); + MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, limbs ) ); + } + + MBEDTLS_MPI_CHK( mbedtls_mpi_lset( X, 0 ) ); + + for( i = 0; i < buflen; i++ ) + X->p[i / ciL] |= ((mbedtls_mpi_uint) buf[i]) << ((i % ciL) << 3); + +cleanup: + + return( ret ); +} + /* * Import X from unsigned binary data, big endian */ diff --git a/tests/suites/test_suite_mpi.data b/tests/suites/test_suite_mpi.data index 8b5f97d38..7ad7ef663 100644 --- a/tests/suites/test_suite_mpi.data +++ b/tests/suites/test_suite_mpi.data @@ -58,6 +58,9 @@ mpi_read_write_string:16:"-1":16:"":3:0:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL Base test mbedtls_mpi_read_binary #1 mbedtls_mpi_read_binary:"0941379d00fed1491fe15df284dfde4a142f68aa8d412023195cee66883e6290ffe703f4ea5963bf212713cee46b107c09182b5edcd955adac418bf4918e2889af48e1099d513830cec85c26ac1e158b52620e33ba8692f893efbb2f958b4424":10:"56125680981752282334141896320372489490613963693556392520816017892111350604111697682705498319512049040516698827829292076808006940873974979584527073481012636016353913462376755556720019831187364993587901952757307830896531678727717924" +Base test mbedtls_mpi_read_binary_le #1 +mbedtls_mpi_read_binary_le:"0941379d00fed1491fe15df284dfde4a142f68aa8d412023195cee66883e6290ffe703f4ea5963bf212713cee46b107c09182b5edcd955adac418bf4918e2889af48e1099d513830cec85c26ac1e158b52620e33ba8692f893efbb2f958b4424":10:"219946662473865722255717126709915431768051735954189829340600976826409773245337023925691629251672268961177825243440202069039100741562168093042339401187848509859789949044607421190014088260008793380554914226244485299326152319899746569" + Base test mbedtls_mpi_write_binary #1 mbedtls_mpi_write_binary:10:"56125680981752282334141896320372489490613963693556392520816017892111350604111697682705498319512049040516698827829292076808006940873974979584527073481012636016353913462376755556720019831187364993587901952757307830896531678727717924":"0941379d00fed1491fe15df284dfde4a142f68aa8d412023195cee66883e6290ffe703f4ea5963bf212713cee46b107c09182b5edcd955adac418bf4918e2889af48e1099d513830cec85c26ac1e158b52620e33ba8692f893efbb2f958b4424":200:0 diff --git a/tests/suites/test_suite_mpi.function b/tests/suites/test_suite_mpi.function index d1fa5a46c..0ec45f655 100644 --- a/tests/suites/test_suite_mpi.function +++ b/tests/suites/test_suite_mpi.function @@ -328,6 +328,25 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void mbedtls_mpi_read_binary_le( data_t * buf, int radix_A, char * input_A ) +{ + mbedtls_mpi X; + unsigned char str[1000]; + size_t len; + + mbedtls_mpi_init( &X ); + + + TEST_ASSERT( mbedtls_mpi_read_binary_le( &X, buf->x, buf->len ) == 0 ); + TEST_ASSERT( mbedtls_mpi_write_string( &X, radix_A, (char *) str, sizeof( str ), &len ) == 0 ); + TEST_ASSERT( strcmp( (char *) str, input_A ) == 0 ); + +exit: + mbedtls_mpi_free( &X ); +} +/* END_CASE */ + /* BEGIN_CASE */ void mbedtls_mpi_write_binary( int radix_X, char * input_X, data_t * input_A, int output_size, From 59b813c7bebc64c1b0780fb6ddb2fb668b1b3b52 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 13 Feb 2019 10:44:06 +0000 Subject: [PATCH 02/17] Add Montgomery points to ecp_point_read_binary The library is able to perform computations and cryptographic schemes on curves with x coordinate ladder representation. Here we add the capability to import such points. --- library/ecp.c | 51 ++++++++++++++++++++-------- tests/suites/test_suite_ecp.data | 12 +++++++ tests/suites/test_suite_ecp.function | 19 +++++++++-- 3 files changed, 65 insertions(+), 17 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index ecea5910e..c509c820c 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -788,7 +788,7 @@ cleanup: } /* - * Import a point from unsigned binary data (SEC1 2.3.4) + * Import a point from unsigned binary data (SEC1 2.3.4 and RFC7748) */ int mbedtls_ecp_point_read_binary( const mbedtls_ecp_group *grp, mbedtls_ecp_point *pt, @@ -803,24 +803,45 @@ int mbedtls_ecp_point_read_binary( const mbedtls_ecp_group *grp, if( ilen < 1 ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); - if( buf[0] == 0x00 ) - { - if( ilen == 1 ) - return( mbedtls_ecp_set_zero( pt ) ); - else - return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); - } - plen = mbedtls_mpi_size( &grp->P ); - if( buf[0] != 0x04 ) - return( MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE ); +#if defined(ECP_MONTGOMERY) + if( ecp_get_type( grp ) == ECP_TYPE_MONTGOMERY ) + { + if( plen != ilen ) + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); - if( ilen != 2 * plen + 1 ) - return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary_le( &pt->X, buf, plen ) ); + mbedtls_mpi_free( &pt->Y ); + + if( grp->id == MBEDTLS_ECP_DP_CURVE25519 ) + /* Set most significant bit to 0 */ + MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &pt->X, plen * 8 - 1, 0 ) ); + } +#endif +#if defined(ECP_SHORTWEIERSTRASS) + if( ecp_get_type( grp ) != ECP_TYPE_MONTGOMERY ) + { + if( buf[0] == 0x00 ) + { + if( ilen == 1 ) + return( mbedtls_ecp_set_zero( pt ) ); + else + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + } + + if( buf[0] != 0x04 ) + return( MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE ); + + if( ilen != 2 * plen + 1 ) + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + + MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &pt->X, buf + 1, plen ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &pt->Y, + buf + 1 + plen, plen ) ); + } +#endif - MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &pt->X, buf + 1, plen ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &pt->Y, buf + 1 + plen, plen ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &pt->Z, 1 ) ); cleanup: diff --git a/tests/suites/test_suite_ecp.data b/tests/suites/test_suite_ecp.data index 30d5ec6f1..77fa724de 100644 --- a/tests/suites/test_suite_ecp.data +++ b/tests/suites/test_suite_ecp.data @@ -112,6 +112,18 @@ ECP read binary #6 (non-zero, OK) depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED ecp_read_binary:MBEDTLS_ECP_DP_SECP192R1:"0448d8082a3a1e3112bc03a8ef2f6d40d0a77a6f8e00cc99336ceed4d7cba482e288669ee1b6415626d6f34d28501e060c":"48d8082a3a1e3112bc03a8ef2f6d40d0a77a6f8e00cc9933":"6ceed4d7cba482e288669ee1b6415626d6f34d28501e060c":"01":0 +ECP read binary #7 (Montgomery, OK) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_read_binary:MBEDTLS_ECP_DP_CURVE25519:"8520f0098930a754748b7ddcb43ef75a0dbf3a0d26381af4eba4a98eaa9b4e6a":"6a4e9baa8ea9a4ebf41a38260d3abf0d5af73eb4dc7d8b7454a7308909f02085":"0":"1":0 + +ECP read binary #8 (Montgomery, masked first bit) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_read_binary:MBEDTLS_ECP_DP_CURVE25519:"8520f0098930a754748b7ddcb43ef75a0dbf3a0d26381af4eba4a98eaa9b4efa":"7a4e9baa8ea9a4ebf41a38260d3abf0d5af73eb4dc7d8b7454a7308909f02085":"0":"1":0 + +ECP read binary #9 (Montgomery, invalid length) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_read_binary:MBEDTLS_ECP_DP_CURVE25519:"20f0098930a754748b7ddcb43ef75a0dbf3a0d26381af4eba4a98eaa9b4e6a":"6a4e9baa8ea9a4ebf41a38260d3abf0d5af73eb4dc7d8b7454a7308909f020":"0":"1":MBEDTLS_ERR_ECP_BAD_INPUT_DATA + ECP tls read point #1 (zero, invalid length byte) depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED mbedtls_ecp_tls_read_point:MBEDTLS_ECP_DP_SECP192R1:"0200":"01":"01":"00":MBEDTLS_ERR_ECP_BAD_INPUT_DATA diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index 606ddd22a..58361db12 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -760,8 +760,23 @@ void ecp_read_binary( int id, data_t * buf, char * x, char * y, char * z, if( ret == 0 ) { TEST_ASSERT( mbedtls_mpi_cmp_mpi( &P.X, &X ) == 0 ); - TEST_ASSERT( mbedtls_mpi_cmp_mpi( &P.Y, &Y ) == 0 ); - TEST_ASSERT( mbedtls_mpi_cmp_mpi( &P.Z, &Z ) == 0 ); + /* + * At the time of writing, the following condition is equivalent with + * if( ecp_get_type( grp ) == ECP_TYPE_MONTGOMERY ) + * but has the advantage of not using internal symbols. + */ + if( grp.G.Y.p == NULL ) + { + TEST_ASSERT( mbedtls_mpi_cmp_int( &Y, 0 ) == 0 ); + TEST_ASSERT( P.Y.p == NULL ); + TEST_ASSERT( mbedtls_mpi_cmp_int( &Z, 1 ) == 0 ); + TEST_ASSERT( mbedtls_mpi_cmp_int( &P.Z, 1 ) == 0 ); + } + else + { + TEST_ASSERT( mbedtls_mpi_cmp_mpi( &P.Y, &Y ) == 0 ); + TEST_ASSERT( mbedtls_mpi_cmp_mpi( &P.Z, &Z ) == 0 ); + } } exit: From 171a7efd02c7eb79a69bcdaae945a4f16545d63b Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Fri, 15 Feb 2019 16:17:45 +0000 Subject: [PATCH 03/17] Add mbedtls_ecp_read_key The private keys used in ECDH differ in the case of Weierstrass and Montgomery curves. They have different constraints, the former is based on big endian, the latter little endian byte order. The fundamental approach is different too: - Weierstrass keys have to be in the right interval, otherwise they are rejected. - Any byte array of the right size is a valid Montgomery key and it needs to be masked before interpreting it as a number. Historically it was sufficient to use mbedtls_mpi_read_binary() to read private keys, but as a preparation to improve support for Montgomery curves we add mbedtls_ecp_read_key() to enable uniform treatment of EC keys. For the masking the `mbedtls_mpi_set_bit()` function is used. This is suboptimal but seems to provide the best trade-off at this time. Alternatives considered: - Making a copy of the input buffer (less efficient) - removing the `const` constraint from the input buffer (breaks the api and makes it less user friendly) - applying the mask directly to the limbs (violates the api between the modules and creates and unwanted dependency) --- include/mbedtls/ecp.h | 16 ++++++++ library/bignum.c | 10 +++++ library/ecp.c | 57 +++++++++++++++++++++++++++ tests/suites/test_suite_ecdh.function | 7 ++-- tests/suites/test_suite_ecp.data | 52 ++++++++++++++++++++++++ tests/suites/test_suite_ecp.function | 22 +++++++++++ 6 files changed, 160 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 065a4cc0b..7dee3e37e 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -1093,6 +1093,22 @@ int mbedtls_ecp_gen_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ); +/** + * \brief This function reads an ECP key. + * + * \param grp_id The ECP group identifier. + * \param key The destination key. + * \param buf The the buffer containing the binary representation of the + * key. (Big endian integer for Weierstrass curves, byte + * string for Montgomery curves.) + * \param buflen The length of the buffer in bytes. + * + * \return \c 0 on success. + * \return An \c MBEDTLS_ERR_ECP_XXX or \c MBEDTLS_MPI_XXX error code + * on failure. + */ +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 checks that the keypair objects * \p pub and \p prv have the same group and the diff --git a/library/bignum.c b/library/bignum.c index d28d3beb7..eb3264082 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -838,6 +838,11 @@ int mbedtls_mpi_read_binary_le( mbedtls_mpi *X, cleanup: + /* + * This function is also used to import keys. However, wiping the buffers + * upon failure is not necessary because failure only can happen before any + * input is copied. + */ return( ret ); } @@ -875,6 +880,11 @@ int mbedtls_mpi_read_binary( mbedtls_mpi *X, const unsigned char *buf, size_t bu cleanup: + /* + * This function is also used to import keys. However, wiping the buffers + * upon failure is not necessary because failure only can happen before any + * input is copied. + */ return( ret ); } diff --git a/library/ecp.c b/library/ecp.c index c509c820c..49f35afd5 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2830,6 +2830,63 @@ int mbedtls_ecp_gen_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, return( mbedtls_ecp_gen_keypair( &key->grp, &key->d, &key->Q, f_rng, p_rng ) ); } +#define ECP_CURVE255_KEY_SIZE 32 +/* + * Read a private key. + */ +int mbedtls_ecp_read_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, + const unsigned char *buf, size_t buflen ) +{ + int ret = 0; + + if( ( ret = mbedtls_ecp_group_load( &key->grp, grp_id ) ) != 0 ) + return( ret ); + +#if defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) + /* + * If it is Curve25519 curve then mask the key as mandated by RFC7748 + */ + if( grp_id == MBEDTLS_ECP_DP_CURVE25519 ) + { + if( buflen != ECP_CURVE255_KEY_SIZE ) + return MBEDTLS_ERR_ECP_INVALID_KEY; + + MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary_le( &key->d, buf, buflen ) ); + + /* Set the three least significant bits to 0 */ + MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &key->d, 0, 0 ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &key->d, 1, 0 ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &key->d, 2, 0 ) ); + + /* Set the most significant bit to 0 */ + MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &key->d, + ECP_CURVE255_KEY_SIZE * 8 - 1, + 0 ) ); + + /* Set the second most significant bit to 1 */ + MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &key->d, + ECP_CURVE255_KEY_SIZE * 8 - 2, + 1 ) ); + } + +#endif +#if defined(ECP_SHORTWEIERSTRASS) + if( ecp_get_type( &key->grp ) != ECP_TYPE_MONTGOMERY ) + { + MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &key->d, buf, buflen ) ); + + MBEDTLS_MPI_CHK( mbedtls_ecp_check_privkey( &key->grp, &key->d ) ); + } + +#endif +cleanup: + + if( ret != 0 ) + mbedtls_mpi_free( &key->d ); + + return( ret ); +} + /* * Check a public-private key pair */ diff --git a/tests/suites/test_suite_ecdh.function b/tests/suites/test_suite_ecdh.function index 1a33d8175..d6bed7f4c 100644 --- a/tests/suites/test_suite_ecdh.function +++ b/tests/suites/test_suite_ecdh.function @@ -22,10 +22,9 @@ static int load_private_key( int grp_id, data_t *private_key, rnd_pseudo_info *rnd_info ) { int ok = 0; - TEST_ASSERT( mbedtls_ecp_group_load( &ecp->grp, grp_id ) == 0 ); - TEST_ASSERT( mbedtls_mpi_read_binary( &ecp->d, - private_key->x, - private_key->len ) == 0 ); + TEST_ASSERT( mbedtls_ecp_read_key( grp_id, ecp, + private_key->x, + private_key->len ) == 0 ); TEST_ASSERT( mbedtls_ecp_check_privkey( &ecp->grp, &ecp->d ) == 0 ); /* Calculate the public key from the private key. */ TEST_ASSERT( mbedtls_ecp_mul( &ecp->grp, &ecp->Q, &ecp->d, diff --git a/tests/suites/test_suite_ecp.data b/tests/suites/test_suite_ecp.data index 77fa724de..1933d3fc2 100644 --- a/tests/suites/test_suite_ecp.data +++ b/tests/suites/test_suite_ecp.data @@ -256,6 +256,58 @@ ECP gen keypair wrapper depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED 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 + +ECP read key #2 (short weierstrass, smallest) +depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED +mbedtls_ecp_read_key:MBEDTLS_ECP_DP_SECP192R1:"01":0 + +ECP read key #3 (short weierstrass, biggest) +depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED +mbedtls_ecp_read_key:MBEDTLS_ECP_DP_SECP192R1:"FFFFFFFFFFFFFFFFFFFFFFFF99DEF836146BC9B1B4D22830":0 + +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 + +ECP read key #5 (montgomery, too big) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"000000000000000000000000000000000000000000000000000000000000000C":0 + +ECP read key #6 (montgomery, not big enough) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"0FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF3":0 + +ECP read key #7 (montgomery, msb OK) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"0000000000000000000000000000000000000000000000000000000000000004":0 + +ECP read key #8 (montgomery, bit 0 set) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"1000000000000000000000000000000000000000000000000000000000000000":0 + +ECP read key #9 (montgomery, bit 1 set) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"2000000000000000000000000000000000000000000000000000000000000004":0 + +ECP read key #10 (montgomery, bit 2 set) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"4000000000000000000000000000000000000000000000000000000000000004":0 + +ECP read key #11 (montgomery, OK) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"8FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7":0 + +ECP read key #12 (montgomery, too long) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"00000000000000000000000000000000000000000000000000000000000000000C":MBEDTLS_ERR_ECP_INVALID_KEY + +ECP read key #13 (montgomery, not long enough) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"0FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF3":MBEDTLS_ERR_ECP_INVALID_KEY + 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 index 58361db12..f1f96a1f2 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -1021,6 +1021,28 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void mbedtls_ecp_read_key( int grp_id, data_t* in_key, int expected ) +{ + int ret = 0; + mbedtls_ecp_keypair key; + + mbedtls_ecp_keypair_init( &key ); + + ret = mbedtls_ecp_read_key( grp_id, &key, in_key->x, in_key->len ); + TEST_ASSERT( ret == expected ); + + if( expected == 0 ) + { + ret = mbedtls_ecp_check_privkey( &key.grp, &key.d ); + TEST_ASSERT( ret == 0 ); + } + +exit: + mbedtls_ecp_keypair_free( &key ); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_SELF_TEST */ void ecp_selftest( ) { From e344d0f6fc8fab4097e9340a5bd3dddcfa2475ab Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 19 Feb 2019 16:17:40 +0000 Subject: [PATCH 04/17] Add little endian export to Bignum The function `mbedtls_mpi_write_binary()` writes big endian byte order, but we need to be able to write little endian in some caseses. (For example when handling keys corresponding to Montgomery curves.) Used `echo xx | tac -rs ..` to transform the test data to little endian. --- include/mbedtls/bignum.h | 18 +++++++++++++ library/bignum.c | 39 ++++++++++++++++++++++++++++ tests/suites/test_suite_mpi.data | 9 +++++++ tests/suites/test_suite_mpi.function | 31 ++++++++++++++++++++++ 4 files changed, 97 insertions(+) diff --git a/include/mbedtls/bignum.h b/include/mbedtls/bignum.h index 0727917c5..c4d768658 100644 --- a/include/mbedtls/bignum.h +++ b/include/mbedtls/bignum.h @@ -522,6 +522,24 @@ int mbedtls_mpi_read_binary_le( mbedtls_mpi *X, int mbedtls_mpi_write_binary( const mbedtls_mpi *X, unsigned char *buf, size_t buflen ); +/** + * \brief Export X into unsigned binary data, little endian. + * Always fills the whole buffer, which will end with zeros + * if the number is smaller. + * + * \param X The source MPI. This must point to an initialized MPI. + * \param buf The output buffer. This must be a writable buffer of length + * \p buflen Bytes. + * \param buflen The size of the output buffer \p buf in Bytes. + * + * \return \c 0 if successful. + * \return #MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL if \p buf isn't + * large enough to hold the value of \p X. + * \return Another negative error code on different kinds of failure. + */ +int mbedtls_mpi_write_binary_le( const mbedtls_mpi *X, + unsigned char *buf, size_t buflen ); + /** * \brief Perform a left-shift on an MPI: X <<= count * diff --git a/library/bignum.c b/library/bignum.c index eb3264082..f0882db4b 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -888,6 +888,45 @@ cleanup: return( ret ); } +/* + * Export X into unsigned binary data, little endian + */ +int mbedtls_mpi_write_binary_le( const mbedtls_mpi *X, + unsigned char *buf, size_t buflen ) +{ + size_t stored_bytes = X->n * ciL; + size_t bytes_to_copy; + size_t i; + + if( stored_bytes < buflen ) + { + bytes_to_copy = stored_bytes; + } + else + { + bytes_to_copy = buflen; + + /* The output buffer is smaller than the allocated size of X. + * However X may fit if its leading bytes are zero. */ + for( i = bytes_to_copy; i < stored_bytes; i++ ) + { + if( GET_BYTE( X, i ) != 0 ) + return( MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL ); + } + } + + for( i = 0; i < bytes_to_copy; i++ ) + buf[i] = GET_BYTE( X, i ); + + if( stored_bytes < buflen ) + { + /* Write trailing 0 bytes */ + memset( buf + stored_bytes, 0, buflen - stored_bytes ); + } + + return( 0 ); +} + /* * Export X into unsigned binary data, big endian */ diff --git a/tests/suites/test_suite_mpi.data b/tests/suites/test_suite_mpi.data index 7ad7ef663..f2be14816 100644 --- a/tests/suites/test_suite_mpi.data +++ b/tests/suites/test_suite_mpi.data @@ -70,6 +70,15 @@ mbedtls_mpi_write_binary:16:"123123123123123123123123123":"012312312312312312312 Test mbedtls_mpi_write_binary #2 (Buffer too small) mbedtls_mpi_write_binary:16:"123123123123123123123123123":"23123123123123123123123123":13:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL +Base test mbedtls_mpi_write_binary_le #1 +mbedtls_mpi_write_binary_le:10:"56125680981752282334141896320372489490613963693556392520816017892111350604111697682705498319512049040516698827829292076808006940873974979584527073481012636016353913462376755556720019831187364993587901952757307830896531678727717924":"24448b952fbbef93f89286ba330e62528b151eac265cc8ce3038519d09e148af89288e91f48b41acad55d9dc5e2b18097c106be4ce132721bf6359eaf403e7ff90623e8866ee5c192320418daa682f144adedf84f25de11f49d1fe009d374109":200:0 + +Test mbedtls_mpi_write_binary_le #1 (Buffer just fits) +mbedtls_mpi_write_binary_le:16:"123123123123123123123123123":"2331122331122331122331122301":14:0 + +Test mbedtls_mpi_write_binary_le #2 (Buffer too small) +mbedtls_mpi_write_binary_le:16:"123123123123123123123123123":"23311223311223311223311223":13:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL + Base test mbedtls_mpi_read_file #1 mbedtls_mpi_read_file:10:"data_files/mpi_10":"01f55332c3a48b910f9942f6c914e58bef37a47ee45cb164a5b6b8d1006bf59a059c21449939ebebfdf517d2e1dbac88010d7b1f141e997bd6801ddaec9d05910f4f2de2b2c4d714e2c14a72fc7f17aa428d59c531627f09":0 diff --git a/tests/suites/test_suite_mpi.function b/tests/suites/test_suite_mpi.function index 0ec45f655..5358379f0 100644 --- a/tests/suites/test_suite_mpi.function +++ b/tests/suites/test_suite_mpi.function @@ -378,6 +378,37 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void mbedtls_mpi_write_binary_le( int radix_X, char * input_X, + data_t * input_A, int output_size, + int result ) +{ + mbedtls_mpi X; + unsigned char buf[1000]; + size_t buflen; + + memset( buf, 0x00, 1000 ); + + mbedtls_mpi_init( &X ); + + TEST_ASSERT( mbedtls_mpi_read_string( &X, radix_X, input_X ) == 0 ); + + buflen = mbedtls_mpi_size( &X ); + if( buflen > (size_t) output_size ) + buflen = (size_t) output_size; + + TEST_ASSERT( mbedtls_mpi_write_binary_le( &X, buf, buflen ) == result ); + if( result == 0) + { + + TEST_ASSERT( hexcmp( buf, input_A->x, buflen, input_A->len ) == 0 ); + } + +exit: + mbedtls_mpi_free( &X ); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_FS_IO */ void mbedtls_mpi_read_file( int radix_X, char * input_file, data_t * input_A, int result ) From ab0f71a22ae8b73e32b909f52f923b5114b5540a Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 20 Feb 2019 10:48:49 +0000 Subject: [PATCH 05/17] ECDH: Add test vectors for Curve25519 The test vectors added are published in RFC 7748. --- library/ecdh.c | 4 ++++ tests/suites/test_suite_ecdh.data | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/library/ecdh.c b/library/ecdh.c index c5726877d..30c5f9f7f 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -637,6 +637,10 @@ static int ecdh_calc_secret_internal( mbedtls_ecdh_context_mbed *ctx, return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); *olen = ctx->grp.pbits / 8 + ( ( ctx->grp.pbits % 8 ) != 0 ); + + if( ctx->grp.id == MBEDTLS_ECP_DP_CURVE25519 ) + return mbedtls_mpi_write_binary_le( &ctx->z, buf, *olen ); + return mbedtls_mpi_write_binary( &ctx->z, buf, *olen ); } diff --git a/tests/suites/test_suite_ecdh.data b/tests/suites/test_suite_ecdh.data index af25359d3..fb4a232fc 100644 --- a/tests/suites/test_suite_ecdh.data +++ b/tests/suites/test_suite_ecdh.data @@ -88,6 +88,18 @@ ECDH calc_secret: theirs first, SECP256R1 (RFC 5903) depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED ecdh_exchange_calc_secret:MBEDTLS_ECP_DP_SECP256R1:"c6ef9c5d78ae012a011164acb397ce2088685d8f06bf9be0b283ab46476bee53":"04dad0b65394221cf9b051e1feca5787d098dfe637fc90b9ef945d0c37725811805271a0461cdb8252d61f1c456fa3e59ab1f45b33accf5f58389e0577b8990bb3":1:"d6840f6b42f6edafd13116e0e12565202fef8e9ece7dce03812464d04b9442de" +ecdh calc_secret: ours first (Alice), curve25519 (rfc 7748) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecdh_exchange_calc_secret:MBEDTLS_ECP_DP_CURVE25519:"77076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c2a":"de9edb7d7b7dc1b4d35b61c2ece435373f8343c85b78674dadfc7e146f882b4f":0:"4a5d9d5ba4ce2de1728e3bf480350f25e07e21c947d19e3376f09b3c1e161742" + +ecdh calc_secret: theirs first (Alice), curve25519 (rfc 7748) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecdh_exchange_calc_secret:MBEDTLS_ECP_DP_CURVE25519:"77076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c2a":"de9edb7d7b7dc1b4d35b61c2ece435373f8343c85b78674dadfc7e146f882b4f":1:"4a5d9d5ba4ce2de1728e3bf480350f25e07e21c947d19e3376f09b3c1e161742" + +ecdh calc_secret: ours first (Bob), curve25519 (rfc 7748) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecdh_exchange_calc_secret:MBEDTLS_ECP_DP_CURVE25519:"5dab087e624a8a4b79e17f8b83800ee66f3bb1292618b6fd1c2f8b27ff88e0eb":"8520f0098930a754748b7ddcb43ef75a0dbf3a0d26381af4eba4a98eaa9b4e6a":0:"4a5d9d5ba4ce2de1728e3bf480350f25e07e21c947d19e3376f09b3c1e161742" + ECDH get_params with mismatched groups: our BP256R1, their SECP256R1 depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_BP256R1_ENABLED ecdh_exchange_get_params_fail:MBEDTLS_ECP_DP_BP256R1:"1234567812345678123456781234567812345678123456781234567812345678":MBEDTLS_ECP_DP_SECP256R1:"04dad0b65394221cf9b051e1feca5787d098dfe637fc90b9ef945d0c37725811805271a0461cdb8252d61f1c456fa3e59ab1f45b33accf5f58389e0577b8990bb3":0:MBEDTLS_ERR_ECP_BAD_INPUT_DATA From 7caf8e452fe3ae80e19a46767af8a17db5c7466e Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 20 Feb 2019 12:00:22 +0000 Subject: [PATCH 06/17] Add Montgomery points to ecp_point_write_binary The library is able to perform computations and cryptographic schemes on curves with x coordinate ladder representation. Here we add the capability to export such points. --- library/ecp.c | 71 +++++++++++++++++++------------- tests/suites/test_suite_ecp.data | 8 ++++ 2 files changed, 51 insertions(+), 28 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 49f35afd5..66cf58ee7 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -729,7 +729,7 @@ cleanup: } /* - * Export a point into unsigned binary data (SEC1 2.3.3) + * Export a point into unsigned binary data (SEC1 2.3.3 and RFC7748) */ int mbedtls_ecp_point_write_binary( const mbedtls_ecp_group *grp, const mbedtls_ecp_point *P, @@ -745,43 +745,58 @@ int mbedtls_ecp_point_write_binary( const mbedtls_ecp_group *grp, ECP_VALIDATE_RET( format == MBEDTLS_ECP_PF_UNCOMPRESSED || format == MBEDTLS_ECP_PF_COMPRESSED ); - /* - * Common case: P == 0 - */ - if( mbedtls_mpi_cmp_int( &P->Z, 0 ) == 0 ) - { - if( buflen < 1 ) - return( MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL ); - - buf[0] = 0x00; - *olen = 1; - - return( 0 ); - } - plen = mbedtls_mpi_size( &grp->P ); - if( format == MBEDTLS_ECP_PF_UNCOMPRESSED ) +#if defined(ECP_MONTGOMERY) + if( ecp_get_type( grp ) == ECP_TYPE_MONTGOMERY ) { - *olen = 2 * plen + 1; - + *olen = plen; if( buflen < *olen ) - return( MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL ); + return( MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL ); - buf[0] = 0x04; - MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( &P->X, buf + 1, plen ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( &P->Y, buf + 1 + plen, plen ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary_le( &P->X, buf, plen ) ); } - else if( format == MBEDTLS_ECP_PF_COMPRESSED ) +#endif +#if defined(ECP_SHORTWEIERSTRASS) + if( ecp_get_type( grp ) != ECP_TYPE_MONTGOMERY ) { - *olen = plen + 1; + /* + * Common case: P == 0 + */ + if( mbedtls_mpi_cmp_int( &P->Z, 0 ) == 0 ) + { + if( buflen < 1 ) + return( MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL ); - if( buflen < *olen ) - return( MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL ); + buf[0] = 0x00; + *olen = 1; - buf[0] = 0x02 + mbedtls_mpi_get_bit( &P->Y, 0 ); - MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( &P->X, buf + 1, plen ) ); + return( 0 ); + } + + if( format == MBEDTLS_ECP_PF_UNCOMPRESSED ) + { + *olen = 2 * plen + 1; + + if( buflen < *olen ) + return( MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL ); + + buf[0] = 0x04; + MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( &P->X, buf + 1, plen ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( &P->Y, buf + 1 + plen, plen ) ); + } + else if( format == MBEDTLS_ECP_PF_COMPRESSED ) + { + *olen = plen + 1; + + if( buflen < *olen ) + return( MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL ); + + buf[0] = 0x02 + mbedtls_mpi_get_bit( &P->Y, 0 ); + MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( &P->X, buf + 1, plen ) ); + } } +#endif cleanup: return( ret ); diff --git a/tests/suites/test_suite_ecp.data b/tests/suites/test_suite_ecp.data index 1933d3fc2..ffa526db8 100644 --- a/tests/suites/test_suite_ecp.data +++ b/tests/suites/test_suite_ecp.data @@ -88,6 +88,14 @@ ECP write binary #9 (odd, compressed, buffer just fits) depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED ecp_write_binary:MBEDTLS_ECP_DP_SECP192R1:"48d8082a3a1e3112bc03a8ef2f6d40d0a77a6f8e00cc9933":"93112b28345b7d1d7799611e49bea9d8290cb2d7afe1f9f3":"01":MBEDTLS_ECP_PF_COMPRESSED:"0348d8082a3a1e3112bc03a8ef2f6d40d0a77a6f8e00cc9933":25:0 +ECP write binary #10 (Montgomery, buffer just fits) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_binary:MBEDTLS_ECP_DP_CURVE25519:"11223344556677889900aabbccddeeff11223344556677889900aabbccddeeff":"0":"1":MBEDTLS_ECP_PF_COMPRESSED:"ffeeddccbbaa00998877665544332211ffeeddccbbaa00998877665544332211":32:0 + +ECP write binary #11 (Montgomery, buffer too small) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_binary:MBEDTLS_ECP_DP_CURVE25519:"11223344556677889900aabbccddeeff11223344556677889900aabbccddeeff":"0":"1":MBEDTLS_ECP_PF_COMPRESSED:"ffeeddccbbaa00998877665544332211ffeeddccbbaa00998877665544332211":31:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + ECP read binary #1 (zero, invalid ilen) depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED ecp_read_binary:MBEDTLS_ECP_DP_SECP192R1:"0000":"01":"01":"00":MBEDTLS_ERR_ECP_BAD_INPUT_DATA From ffbd7e8ff328e41ccea11ac48c096d4feac4f90c Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 25 Feb 2019 11:35:20 +0000 Subject: [PATCH 07/17] Improve mbedtls_ecp_point_read_binary tests Renamed the tests because they are explicitly testing Curve25519 and nothing else. Improved test coverage, test documentation and extended in-code documentation with a specific reference to the standard as well. --- library/ecp.c | 2 +- tests/suites/test_suite_ecp.data | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 66cf58ee7..8d0f49ae3 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -830,7 +830,7 @@ int mbedtls_ecp_point_read_binary( const mbedtls_ecp_group *grp, mbedtls_mpi_free( &pt->Y ); if( grp->id == MBEDTLS_ECP_DP_CURVE25519 ) - /* Set most significant bit to 0 */ + /* Set most significant bit to 0 as prescribed in RFC7748 §5 */ MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &pt->X, plen * 8 - 1, 0 ) ); } #endif diff --git a/tests/suites/test_suite_ecp.data b/tests/suites/test_suite_ecp.data index ffa526db8..87e863d5c 100644 --- a/tests/suites/test_suite_ecp.data +++ b/tests/suites/test_suite_ecp.data @@ -120,18 +120,30 @@ ECP read binary #6 (non-zero, OK) depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED ecp_read_binary:MBEDTLS_ECP_DP_SECP192R1:"0448d8082a3a1e3112bc03a8ef2f6d40d0a77a6f8e00cc99336ceed4d7cba482e288669ee1b6415626d6f34d28501e060c":"48d8082a3a1e3112bc03a8ef2f6d40d0a77a6f8e00cc9933":"6ceed4d7cba482e288669ee1b6415626d6f34d28501e060c":"01":0 -ECP read binary #7 (Montgomery, OK) +ECP read binary #7 (Curve25519, OK) depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED ecp_read_binary:MBEDTLS_ECP_DP_CURVE25519:"8520f0098930a754748b7ddcb43ef75a0dbf3a0d26381af4eba4a98eaa9b4e6a":"6a4e9baa8ea9a4ebf41a38260d3abf0d5af73eb4dc7d8b7454a7308909f02085":"0":"1":0 -ECP read binary #8 (Montgomery, masked first bit) +ECP read binary #8 (Curve25519, masked first bit) depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED ecp_read_binary:MBEDTLS_ECP_DP_CURVE25519:"8520f0098930a754748b7ddcb43ef75a0dbf3a0d26381af4eba4a98eaa9b4efa":"7a4e9baa8ea9a4ebf41a38260d3abf0d5af73eb4dc7d8b7454a7308909f02085":"0":"1":0 -ECP read binary #9 (Montgomery, invalid length) +ECP read binary #9 (Curve25519, too short) depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED ecp_read_binary:MBEDTLS_ECP_DP_CURVE25519:"20f0098930a754748b7ddcb43ef75a0dbf3a0d26381af4eba4a98eaa9b4e6a":"6a4e9baa8ea9a4ebf41a38260d3abf0d5af73eb4dc7d8b7454a7308909f020":"0":"1":MBEDTLS_ERR_ECP_BAD_INPUT_DATA +ECP read binary #10 (Curve25519, non-canonical) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_read_binary:MBEDTLS_ECP_DP_CURVE25519:"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f":"7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff":"0":"1":0 + +ECP read binary #11 (Curve25519, masked non-canonical) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_read_binary:MBEDTLS_ECP_DP_CURVE25519:"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff":"7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff":"0":"1":0 + +ECP read binary #12 (Curve25519, too long) +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_read_binary:MBEDTLS_ECP_DP_CURVE25519:"8520f0098930a754748b7ddcb43ef75a0dbf3a0d26381af4eba4a98eaa9b4e6a00":"6a4e9baa8ea9a4ebf41a38260d3abf0d5af73eb4dc7d8b7454a7308909f02085":"0":"1":MBEDTLS_ERR_ECP_BAD_INPUT_DATA + ECP tls read point #1 (zero, invalid length byte) depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED mbedtls_ecp_tls_read_point:MBEDTLS_ECP_DP_SECP192R1:"0200":"01":"01":"00":MBEDTLS_ERR_ECP_BAD_INPUT_DATA From e5670f26632aa7563a6a7eb3daad79993f515968 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 25 Feb 2019 16:11:58 +0000 Subject: [PATCH 08/17] Remove unnecessary cast from ECP test --- tests/suites/test_suite_mpi.function | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_mpi.function b/tests/suites/test_suite_mpi.function index 5358379f0..67894e654 100644 --- a/tests/suites/test_suite_mpi.function +++ b/tests/suites/test_suite_mpi.function @@ -313,14 +313,14 @@ exit: void mbedtls_mpi_read_binary( data_t * buf, int radix_A, char * input_A ) { mbedtls_mpi X; - unsigned char str[1000]; + char str[1000]; size_t len; mbedtls_mpi_init( &X ); TEST_ASSERT( mbedtls_mpi_read_binary( &X, buf->x, buf->len ) == 0 ); - TEST_ASSERT( mbedtls_mpi_write_string( &X, radix_A, (char *) str, sizeof( str ), &len ) == 0 ); + TEST_ASSERT( mbedtls_mpi_write_string( &X, radix_A, str, sizeof( str ), &len ) == 0 ); TEST_ASSERT( strcmp( (char *) str, input_A ) == 0 ); exit: @@ -332,14 +332,14 @@ exit: void mbedtls_mpi_read_binary_le( data_t * buf, int radix_A, char * input_A ) { mbedtls_mpi X; - unsigned char str[1000]; + char str[1000]; size_t len; mbedtls_mpi_init( &X ); TEST_ASSERT( mbedtls_mpi_read_binary_le( &X, buf->x, buf->len ) == 0 ); - TEST_ASSERT( mbedtls_mpi_write_string( &X, radix_A, (char *) str, sizeof( str ), &len ) == 0 ); + TEST_ASSERT( mbedtls_mpi_write_string( &X, radix_A, str, sizeof( str ), &len ) == 0 ); TEST_ASSERT( strcmp( (char *) str, input_A ) == 0 ); exit: From 7780096f3b927b11c13e08891b5debd46c3cae84 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 25 Feb 2019 16:32:08 +0000 Subject: [PATCH 09/17] Fix typo in ECP module --- library/ecp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 8d0f49ae3..f27560566 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2845,7 +2845,7 @@ int mbedtls_ecp_gen_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, return( mbedtls_ecp_gen_keypair( &key->grp, &key->d, &key->Q, f_rng, p_rng ) ); } -#define ECP_CURVE255_KEY_SIZE 32 +#define ECP_CURVE25519_KEY_SIZE 32 /* * Read a private key. */ @@ -2863,7 +2863,7 @@ int mbedtls_ecp_read_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, */ if( grp_id == MBEDTLS_ECP_DP_CURVE25519 ) { - if( buflen != ECP_CURVE255_KEY_SIZE ) + if( buflen != ECP_CURVE25519_KEY_SIZE ) return MBEDTLS_ERR_ECP_INVALID_KEY; MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary_le( &key->d, buf, buflen ) ); @@ -2875,12 +2875,12 @@ int mbedtls_ecp_read_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, /* Set the most significant bit to 0 */ MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &key->d, - ECP_CURVE255_KEY_SIZE * 8 - 1, + ECP_CURVE25519_KEY_SIZE * 8 - 1, 0 ) ); /* Set the second most significant bit to 1 */ MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &key->d, - ECP_CURVE255_KEY_SIZE * 8 - 2, + ECP_CURVE25519_KEY_SIZE * 8 - 2, 1 ) ); } From b65853c6b638f2fb1069b4ae74ab9bf8253bfd0f Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 25 Feb 2019 16:33:28 +0000 Subject: [PATCH 10/17] Improve documentation of mbedtls_ecp_read_key --- include/mbedtls/ecp.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 7dee3e37e..ba3be207a 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -1094,7 +1094,7 @@ int mbedtls_ecp_gen_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, void *p_rng ); /** - * \brief This function reads an ECP key. + * \brief This function reads an elliptic curve private key. * * \param grp_id The ECP group identifier. * \param key The destination key. @@ -1104,8 +1104,10 @@ int mbedtls_ecp_gen_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, * \param buflen The length of the buffer in bytes. * * \return \c 0 on success. - * \return An \c MBEDTLS_ERR_ECP_XXX or \c MBEDTLS_MPI_XXX error code - * on failure. + * \return #MBEDTLS_ERR_ECP_INVALID_KEY error if the key is + * invalid. + * \return #MBEDTLS_ERR_MPI_ALLOC_FAILED if memory allocation failed. + * \return Another negative error code on different kinds of failure. */ int mbedtls_ecp_read_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, const unsigned char *buf, size_t buflen ); From 28eb06df16d2205cfc6f49015e6786110b34c8ce Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 26 Feb 2019 10:53:34 +0000 Subject: [PATCH 11/17] ECP: Catch unsupported import/export mbedtls_ecp_read_key() module returned without an error even when importing keys corresponding to the requested group was not implemented. We change this and return an error when the requested group is not supported and make the remaining import/export functions more robust. --- include/mbedtls/ecp.h | 18 ++++++--- library/ecp.c | 67 +++++++++++++++++++------------- tests/suites/test_suite_ecp.data | 4 ++ 3 files changed, 57 insertions(+), 32 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index ba3be207a..384286edb 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -626,6 +626,9 @@ int mbedtls_ecp_point_read_string( mbedtls_ecp_point *P, int radix, * \param P The point to export. This must be initialized. * \param format The point format. This must be either * #MBEDTLS_ECP_PF_COMPRESSED or #MBEDTLS_ECP_PF_UNCOMPRESSED. + * (For groups without these formats, this parameter is + * ignored. But it still has to be either of the above + * values.) * \param olen The address at which to store the length of * the output in Bytes. This must not be \c NULL. * \param buf The output buffer. This must be a writable buffer @@ -635,11 +638,14 @@ int mbedtls_ecp_point_read_string( mbedtls_ecp_point *P, int radix, * \return \c 0 on success. * \return #MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL if the output buffer * is too small to hold the point. + * \return #MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE if the point format + * or the export for the given group is not implemented. * \return Another negative error code on other kinds of failure. */ -int mbedtls_ecp_point_write_binary( const mbedtls_ecp_group *grp, const mbedtls_ecp_point *P, - int format, size_t *olen, - unsigned char *buf, size_t buflen ); +int mbedtls_ecp_point_write_binary( const mbedtls_ecp_group *grp, + const mbedtls_ecp_point *P, + int format, size_t *olen, + unsigned char *buf, size_t buflen ); /** * \brief This function imports a point from unsigned binary data. @@ -660,8 +666,8 @@ int mbedtls_ecp_point_write_binary( const mbedtls_ecp_group *grp, const mbedtls_ * \return \c 0 on success. * \return #MBEDTLS_ERR_ECP_BAD_INPUT_DATA if the input is invalid. * \return #MBEDTLS_ERR_MPI_ALLOC_FAILED on memory-allocation failure. - * \return #MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE if the point format - * is not implemented. + * \return #MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE if the import for the + * given group is not implemented. */ int mbedtls_ecp_point_read_binary( const mbedtls_ecp_group *grp, mbedtls_ecp_point *P, @@ -1107,6 +1113,8 @@ int mbedtls_ecp_gen_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, * \return #MBEDTLS_ERR_ECP_INVALID_KEY error if the key is * invalid. * \return #MBEDTLS_ERR_MPI_ALLOC_FAILED if memory allocation failed. + * \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_read_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, diff --git a/library/ecp.c b/library/ecp.c index f27560566..c918c95dc 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -736,7 +736,7 @@ int mbedtls_ecp_point_write_binary( const mbedtls_ecp_group *grp, int format, size_t *olen, unsigned char *buf, size_t buflen ) { - int ret = 0; + int ret = MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE; size_t plen; ECP_VALIDATE_RET( grp != NULL ); ECP_VALIDATE_RET( P != NULL ); @@ -758,7 +758,7 @@ int mbedtls_ecp_point_write_binary( const mbedtls_ecp_group *grp, } #endif #if defined(ECP_SHORTWEIERSTRASS) - if( ecp_get_type( grp ) != ECP_TYPE_MONTGOMERY ) + if( ecp_get_type( grp ) == ECP_TYPE_SHORT_WEIERSTRASS ) { /* * Common case: P == 0 @@ -809,7 +809,7 @@ int mbedtls_ecp_point_read_binary( const mbedtls_ecp_group *grp, mbedtls_ecp_point *pt, const unsigned char *buf, size_t ilen ) { - int ret; + int ret = MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE; size_t plen; ECP_VALIDATE_RET( grp != NULL ); ECP_VALIDATE_RET( pt != NULL ); @@ -832,10 +832,12 @@ int mbedtls_ecp_point_read_binary( const mbedtls_ecp_group *grp, if( grp->id == MBEDTLS_ECP_DP_CURVE25519 ) /* Set most significant bit to 0 as prescribed in RFC7748 §5 */ MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &pt->X, plen * 8 - 1, 0 ) ); + + MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &pt->Z, 1 ) ); } #endif #if defined(ECP_SHORTWEIERSTRASS) - if( ecp_get_type( grp ) != ECP_TYPE_MONTGOMERY ) + if( ecp_get_type( grp ) == ECP_TYPE_SHORT_WEIERSTRASS ) { if( buf[0] == 0x00 ) { @@ -854,11 +856,10 @@ int mbedtls_ecp_point_read_binary( const mbedtls_ecp_group *grp, MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &pt->X, buf + 1, plen ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &pt->Y, buf + 1 + plen, plen ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &pt->Z, 1 ) ); } #endif - MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &pt->Z, 1 ) ); - cleanup: return( ret ); } @@ -2854,39 +2855,51 @@ int mbedtls_ecp_read_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, { int ret = 0; + ECP_VALIDATE_RET( key != NULL ); + ECP_VALIDATE_RET( buf != NULL ); + if( ( ret = mbedtls_ecp_group_load( &key->grp, grp_id ) ) != 0 ) return( ret ); -#if defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) - /* - * If it is Curve25519 curve then mask the key as mandated by RFC7748 - */ - if( grp_id == MBEDTLS_ECP_DP_CURVE25519 ) + ret = MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE; + +#if defined(ECP_MONTGOMERY) + if( ecp_get_type( &key->grp ) == ECP_TYPE_MONTGOMERY ) { - if( buflen != ECP_CURVE25519_KEY_SIZE ) - return MBEDTLS_ERR_ECP_INVALID_KEY; + /* + * If it is Curve25519 curve then mask the key as mandated by RFC7748 + */ + if( grp_id == MBEDTLS_ECP_DP_CURVE25519 ) + { + if( buflen != ECP_CURVE25519_KEY_SIZE ) + return MBEDTLS_ERR_ECP_INVALID_KEY; - MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary_le( &key->d, buf, buflen ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary_le( &key->d, buf, buflen ) ); - /* Set the three least significant bits to 0 */ - MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &key->d, 0, 0 ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &key->d, 1, 0 ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &key->d, 2, 0 ) ); + /* Set the three least significant bits to 0 */ + MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &key->d, 0, 0 ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &key->d, 1, 0 ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &key->d, 2, 0 ) ); - /* Set the most significant bit to 0 */ - MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &key->d, - ECP_CURVE25519_KEY_SIZE * 8 - 1, - 0 ) ); + /* Set the most significant bit to 0 */ + MBEDTLS_MPI_CHK( + mbedtls_mpi_set_bit( &key->d, + ECP_CURVE25519_KEY_SIZE * 8 - 1, 0 ) + ); - /* Set the second most significant bit to 1 */ - MBEDTLS_MPI_CHK( mbedtls_mpi_set_bit( &key->d, - ECP_CURVE25519_KEY_SIZE * 8 - 2, - 1 ) ); + /* Set the second most significant bit to 1 */ + MBEDTLS_MPI_CHK( + mbedtls_mpi_set_bit( &key->d, + ECP_CURVE25519_KEY_SIZE * 8 - 2, 1 ) + ); + } + else + ret = MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE; } #endif #if defined(ECP_SHORTWEIERSTRASS) - if( ecp_get_type( &key->grp ) != ECP_TYPE_MONTGOMERY ) + if( ecp_get_type( &key->grp ) == ECP_TYPE_SHORT_WEIERSTRASS ) { MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &key->d, buf, buflen ) ); diff --git a/tests/suites/test_suite_ecp.data b/tests/suites/test_suite_ecp.data index 87e863d5c..473193cfe 100644 --- a/tests/suites/test_suite_ecp.data +++ b/tests/suites/test_suite_ecp.data @@ -328,6 +328,10 @@ ECP read key #13 (montgomery, not long enough) depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"0FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF3":MBEDTLS_ERR_ECP_INVALID_KEY +ECP read key #14 (Curve448, not supported) +depends_on:MBEDTLS_ECP_DP_CURVE448_ENABLED +mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE448:"FCFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF":MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE + 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" From 4ffdbe0979c4d9718479e569058289b35fc30e68 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 26 Feb 2019 12:03:02 +0000 Subject: [PATCH 12/17] Add more tests for ecp_read_key --- tests/suites/test_suite_ecp.data | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_ecp.data b/tests/suites/test_suite_ecp.data index 473193cfe..54fff6d8c 100644 --- a/tests/suites/test_suite_ecp.data +++ b/tests/suites/test_suite_ecp.data @@ -329,9 +329,15 @@ depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"0FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF3":MBEDTLS_ERR_ECP_INVALID_KEY ECP read key #14 (Curve448, not supported) -depends_on:MBEDTLS_ECP_DP_CURVE448_ENABLED mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE448:"FCFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF":MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE +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 + +ECP read key #15 (invalid curve) +mbedtls_ecp_read_key:INT_MAX:"8FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7":MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE + 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" From df9295b7ecb0184f59ca3a848c2b261920d67205 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 26 Feb 2019 12:36:52 +0000 Subject: [PATCH 13/17] Make ecp_get_type public The ecp_get_type function comes handy in higher level modules and tests as well. It is not inline anymore, to enable alternative implementations to implement it for themselves. --- include/mbedtls/ecp.h | 15 +++++++++ library/ecp.c | 48 +++++++++++----------------- tests/suites/test_suite_ecp.function | 7 +--- 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 384286edb..0b2504e1d 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -99,6 +99,16 @@ typedef enum */ #define MBEDTLS_ECP_DP_MAX 12 +/* + * Curve types + */ +typedef enum +{ + MBEDTLS_ECP_TYPE_NONE = 0, + MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS, /* y^2 = x^3 + a x + b */ + MBEDTLS_ECP_TYPE_MONTGOMERY, /* y^2 = x^3 + a x^2 + x */ +} mbedtls_ecp_curve_type; + /** * Curve information, for use by other modules. */ @@ -417,6 +427,11 @@ void mbedtls_ecp_set_max_ops( unsigned max_ops ); int mbedtls_ecp_restart_is_enabled( void ); #endif /* MBEDTLS_ECP_RESTARTABLE */ +/* + * Get the type of a curve + */ +mbedtls_ecp_curve_type mbedtls_ecp_get_type( const mbedtls_ecp_group *grp ); + /** * \brief This function retrieves the information defined in * mbedtls_ecp_curve_info() for all supported curves in order diff --git a/library/ecp.c b/library/ecp.c index c918c95dc..69c432b76 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -363,16 +363,6 @@ int mbedtls_ecp_check_budget( const mbedtls_ecp_group *grp, #define ECP_MONTGOMERY #endif -/* - * Curve types: internal for now, might be exposed later - */ -typedef enum -{ - ECP_TYPE_NONE = 0, - ECP_TYPE_SHORT_WEIERSTRASS, /* y^2 = x^3 + a x + b */ - ECP_TYPE_MONTGOMERY, /* y^2 = x^3 + a x^2 + x */ -} ecp_curve_type; - /* * List of supported curves: * - internal ID @@ -522,15 +512,15 @@ const mbedtls_ecp_curve_info *mbedtls_ecp_curve_info_from_name( const char *name /* * Get the type of a curve */ -static inline ecp_curve_type ecp_get_type( const mbedtls_ecp_group *grp ) +mbedtls_ecp_curve_type mbedtls_ecp_get_type( const mbedtls_ecp_group *grp ) { if( grp->G.X.p == NULL ) - return( ECP_TYPE_NONE ); + return( MBEDTLS_ECP_TYPE_NONE ); if( grp->G.Y.p == NULL ) - return( ECP_TYPE_MONTGOMERY ); + return( MBEDTLS_ECP_TYPE_MONTGOMERY ); else - return( ECP_TYPE_SHORT_WEIERSTRASS ); + return( MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS ); } /* @@ -748,7 +738,7 @@ int mbedtls_ecp_point_write_binary( const mbedtls_ecp_group *grp, plen = mbedtls_mpi_size( &grp->P ); #if defined(ECP_MONTGOMERY) - if( ecp_get_type( grp ) == ECP_TYPE_MONTGOMERY ) + if( mbedtls_ecp_get_type( grp ) == MBEDTLS_ECP_TYPE_MONTGOMERY ) { *olen = plen; if( buflen < *olen ) @@ -758,7 +748,7 @@ int mbedtls_ecp_point_write_binary( const mbedtls_ecp_group *grp, } #endif #if defined(ECP_SHORTWEIERSTRASS) - if( ecp_get_type( grp ) == ECP_TYPE_SHORT_WEIERSTRASS ) + if( mbedtls_ecp_get_type( grp ) == MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS ) { /* * Common case: P == 0 @@ -821,7 +811,7 @@ int mbedtls_ecp_point_read_binary( const mbedtls_ecp_group *grp, plen = mbedtls_mpi_size( &grp->P ); #if defined(ECP_MONTGOMERY) - if( ecp_get_type( grp ) == ECP_TYPE_MONTGOMERY ) + if( mbedtls_ecp_get_type( grp ) == MBEDTLS_ECP_TYPE_MONTGOMERY ) { if( plen != ilen ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); @@ -837,7 +827,7 @@ int mbedtls_ecp_point_read_binary( const mbedtls_ecp_group *grp, } #endif #if defined(ECP_SHORTWEIERSTRASS) - if( ecp_get_type( grp ) == ECP_TYPE_SHORT_WEIERSTRASS ) + if( mbedtls_ecp_get_type( grp ) == MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS ) { if( buf[0] == 0x00 ) { @@ -2394,11 +2384,11 @@ int mbedtls_ecp_mul_restartable( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, ret = MBEDTLS_ERR_ECP_BAD_INPUT_DATA; #if defined(ECP_MONTGOMERY) - if( ecp_get_type( grp ) == ECP_TYPE_MONTGOMERY ) + if( mbedtls_ecp_get_type( grp ) == MBEDTLS_ECP_TYPE_MONTGOMERY ) MBEDTLS_MPI_CHK( ecp_mul_mxz( grp, R, m, P, f_rng, p_rng ) ); #endif #if defined(ECP_SHORTWEIERSTRASS) - if( ecp_get_type( grp ) == ECP_TYPE_SHORT_WEIERSTRASS ) + if( mbedtls_ecp_get_type( grp ) == MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS ) MBEDTLS_MPI_CHK( ecp_mul_comb( grp, R, m, P, f_rng, p_rng, rs_ctx ) ); #endif @@ -2537,7 +2527,7 @@ int mbedtls_ecp_muladd_restartable( ECP_VALIDATE_RET( n != NULL ); ECP_VALIDATE_RET( Q != NULL ); - if( ecp_get_type( grp ) != ECP_TYPE_SHORT_WEIERSTRASS ) + if( mbedtls_ecp_get_type( grp ) != MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS ) return( MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE ); mbedtls_ecp_point_init( &mP ); @@ -2657,11 +2647,11 @@ int mbedtls_ecp_check_pubkey( const mbedtls_ecp_group *grp, return( MBEDTLS_ERR_ECP_INVALID_KEY ); #if defined(ECP_MONTGOMERY) - if( ecp_get_type( grp ) == ECP_TYPE_MONTGOMERY ) + if( mbedtls_ecp_get_type( grp ) == MBEDTLS_ECP_TYPE_MONTGOMERY ) return( ecp_check_pubkey_mx( grp, pt ) ); #endif #if defined(ECP_SHORTWEIERSTRASS) - if( ecp_get_type( grp ) == ECP_TYPE_SHORT_WEIERSTRASS ) + if( mbedtls_ecp_get_type( grp ) == MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS ) return( ecp_check_pubkey_sw( grp, pt ) ); #endif return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); @@ -2677,7 +2667,7 @@ int mbedtls_ecp_check_privkey( const mbedtls_ecp_group *grp, ECP_VALIDATE_RET( d != NULL ); #if defined(ECP_MONTGOMERY) - if( ecp_get_type( grp ) == ECP_TYPE_MONTGOMERY ) + if( mbedtls_ecp_get_type( grp ) == MBEDTLS_ECP_TYPE_MONTGOMERY ) { /* see RFC 7748 sec. 5 para. 5 */ if( mbedtls_mpi_get_bit( d, 0 ) != 0 || @@ -2693,7 +2683,7 @@ int mbedtls_ecp_check_privkey( const mbedtls_ecp_group *grp, } #endif /* ECP_MONTGOMERY */ #if defined(ECP_SHORTWEIERSTRASS) - if( ecp_get_type( grp ) == ECP_TYPE_SHORT_WEIERSTRASS ) + if( mbedtls_ecp_get_type( grp ) == MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS ) { /* see SEC1 3.2 */ if( mbedtls_mpi_cmp_int( d, 1 ) < 0 || @@ -2725,7 +2715,7 @@ int mbedtls_ecp_gen_privkey( const mbedtls_ecp_group *grp, n_size = ( grp->nbits + 7 ) / 8; #if defined(ECP_MONTGOMERY) - if( ecp_get_type( grp ) == ECP_TYPE_MONTGOMERY ) + if( mbedtls_ecp_get_type( grp ) == MBEDTLS_ECP_TYPE_MONTGOMERY ) { /* [M225] page 5 */ size_t b; @@ -2753,7 +2743,7 @@ int mbedtls_ecp_gen_privkey( const mbedtls_ecp_group *grp, #endif /* ECP_MONTGOMERY */ #if defined(ECP_SHORTWEIERSTRASS) - if( ecp_get_type( grp ) == ECP_TYPE_SHORT_WEIERSTRASS ) + if( mbedtls_ecp_get_type( grp ) == MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS ) { /* SEC1 3.2.1: Generate d such that 1 <= n < N */ int count = 0; @@ -2864,7 +2854,7 @@ int mbedtls_ecp_read_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, ret = MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE; #if defined(ECP_MONTGOMERY) - if( ecp_get_type( &key->grp ) == ECP_TYPE_MONTGOMERY ) + if( mbedtls_ecp_get_type( &key->grp ) == MBEDTLS_ECP_TYPE_MONTGOMERY ) { /* * If it is Curve25519 curve then mask the key as mandated by RFC7748 @@ -2899,7 +2889,7 @@ int mbedtls_ecp_read_key( mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, #endif #if defined(ECP_SHORTWEIERSTRASS) - if( ecp_get_type( &key->grp ) == ECP_TYPE_SHORT_WEIERSTRASS ) + if( mbedtls_ecp_get_type( &key->grp ) == MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS ) { MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &key->d, buf, buflen ) ); diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index f1f96a1f2..ba271b1a5 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -760,12 +760,7 @@ void ecp_read_binary( int id, data_t * buf, char * x, char * y, char * z, if( ret == 0 ) { TEST_ASSERT( mbedtls_mpi_cmp_mpi( &P.X, &X ) == 0 ); - /* - * At the time of writing, the following condition is equivalent with - * if( ecp_get_type( grp ) == ECP_TYPE_MONTGOMERY ) - * but has the advantage of not using internal symbols. - */ - if( grp.G.Y.p == NULL ) + if( mbedtls_ecp_get_type( &grp ) == MBEDTLS_ECP_TYPE_MONTGOMERY ) { TEST_ASSERT( mbedtls_mpi_cmp_int( &Y, 0 ) == 0 ); TEST_ASSERT( P.Y.p == NULL ); From bf42408528f5db2ba1d94e10dd34a784a52c5b2d Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 26 Feb 2019 13:53:55 +0000 Subject: [PATCH 14/17] Improve ECP test names --- tests/suites/test_suite_ecp.data | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/suites/test_suite_ecp.data b/tests/suites/test_suite_ecp.data index 54fff6d8c..73d5315db 100644 --- a/tests/suites/test_suite_ecp.data +++ b/tests/suites/test_suite_ecp.data @@ -292,39 +292,39 @@ 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 -ECP read key #5 (montgomery, too big) +ECP read key #5 (Curve25519, too big) depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"000000000000000000000000000000000000000000000000000000000000000C":0 -ECP read key #6 (montgomery, not big enough) +ECP read key #6 (Curve25519, not big enough) depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"0FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF3":0 -ECP read key #7 (montgomery, msb OK) +ECP read key #7 (Curve25519, msb OK) depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"0000000000000000000000000000000000000000000000000000000000000004":0 -ECP read key #8 (montgomery, bit 0 set) +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 -ECP read key #9 (montgomery, bit 1 set) +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 -ECP read key #10 (montgomery, bit 2 set) +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 -ECP read key #11 (montgomery, OK) +ECP read key #11 (Curve25519, OK) depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"8FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7":0 -ECP read key #12 (montgomery, too long) +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 -ECP read key #13 (montgomery, not long enough) +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 From 52ff8e938719eaeb33d6bf552afab2ea67e3e1ce Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 26 Feb 2019 13:56:04 +0000 Subject: [PATCH 15/17] Fix ECDH secret export for Mongomery curves We only switched to little endian for Curve25519, but all Montgomery curves require little endian byte order. --- library/ecdh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ecdh.c b/library/ecdh.c index 30c5f9f7f..eecae9131 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -638,7 +638,7 @@ static int ecdh_calc_secret_internal( mbedtls_ecdh_context_mbed *ctx, *olen = ctx->grp.pbits / 8 + ( ( ctx->grp.pbits % 8 ) != 0 ); - if( ctx->grp.id == MBEDTLS_ECP_DP_CURVE25519 ) + if( mbedtls_ecp_get_type( &ctx->grp ) == MBEDTLS_ECP_TYPE_MONTGOMERY ) return mbedtls_mpi_write_binary_le( &ctx->z, buf, *olen ); return mbedtls_mpi_write_binary( &ctx->z, buf, *olen ); From f607813f53532d2e6c8a3770887d5ffe4e302c72 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 26 Feb 2019 17:02:37 +0000 Subject: [PATCH 16/17] ECP: remove extra whitespaces --- library/ecp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ecp.c b/library/ecp.c index 69c432b76..c3c5ce540 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -742,7 +742,7 @@ int mbedtls_ecp_point_write_binary( const mbedtls_ecp_group *grp, { *olen = plen; if( buflen < *olen ) - return( MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL ); + return( MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL ); MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary_le( &P->X, buf, plen ) ); } From 54ba3eb7de94270efe26efa39f08d1b18d1bdf71 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 27 Feb 2019 14:47:10 +0000 Subject: [PATCH 17/17] ECP: Clarify test descriptions --- tests/suites/test_suite_ecp.data | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_ecp.data b/tests/suites/test_suite_ecp.data index 73d5315db..86533665c 100644 --- a/tests/suites/test_suite_ecp.data +++ b/tests/suites/test_suite_ecp.data @@ -292,11 +292,11 @@ 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 -ECP read key #5 (Curve25519, too big) +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 -ECP read key #6 (Curve25519, not big enough) +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