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.
diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h
index ba3be20..384286e 100644
--- a/include/mbedtls/ecp.h
+++ b/include/mbedtls/ecp.h
@@ -626,6 +626,9 @@
* \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 @@
* \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 @@
* \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 @@
* \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 f275605..c918c95 100644
--- a/library/ecp.c
+++ b/library/ecp.c
@@ -736,7 +736,7 @@
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 @@
}
#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 @@
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 @@
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 @@
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 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 87e863d..473193c 100644
--- a/tests/suites/test_suite_ecp.data
+++ b/tests/suites/test_suite_ecp.data
@@ -328,6 +328,10 @@
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"