From 28eb06df16d2205cfc6f49015e6786110b34c8ce Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 26 Feb 2019 10:53:34 +0000 Subject: [PATCH] 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"