From 171a7efd02c7eb79a69bcdaae945a4f16545d63b Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Fri, 15 Feb 2019 16:17:45 +0000 Subject: [PATCH] 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( ) {