Add more generic functions to test/set an ECP point to zero

Add mbedtls_ecp_set_zero_ext() and mbedtls_ecp_is_zero_ext() functions
which takes the ECP group as first argument, so that the functions can
handle all curve types, and especially Edwards curves.

Deprecated the old mbedtls_ecp_set_zero() and mbedtls_ecp_is_zero()
functions.

Suggested during the initial EdDSA PR review:
Mbed-TLS#3245 (comment)

Signed-off-by: Aurelien Jarno <[email protected]>

ChangeLog.d/mbedtls_ecp_zero_point.txt

@@ -0,0 +1,8 @@
+API changes
+   * New functions mbedtls_ecp_set_zero_ext() and mbedtls_ecp_is_zero_ext()
+     working on all ECP curves.
+
+New deprecations
+   * Deprecate mbedtls_ecp_set_zero() and mbedtls_ecp_is_zero() in favor of the
+     mbedtls_ecp_set_zero_ext() and mbedtls_ecp_is_zero_ext() functions which
+     work on all ECP curves.

include/mbedtls/ecp.h

@@ -33,6 +33,7 @@
 
 #ifndef MBEDTLS_ECP_H
 #define MBEDTLS_ECP_H
+#include "mbedtls/platform_util.h"
 #include "mbedtls/private_access.h"
 
 #include "mbedtls/build_info.h"
@@ -677,27 +678,71 @@ int mbedtls_ecp_copy( mbedtls_ecp_point *P, const mbedtls_ecp_point *Q );
 int mbedtls_ecp_group_copy( mbedtls_ecp_group *dst,
                             const mbedtls_ecp_group *src );
 
+#if !defined(MBEDTLS_DEPRECATED_REMOVED)
 /**
  * \brief           This function sets a point to the point at infinity.
  *
+ * \note            This does not work for Edwards curves and will silently
+ *                  set the point to a wrong value in that case.
+ *
+ * \deprecated      This function is deprecated and has been replaced by
+ *                  \c mbedtls_ecp_set_zero_ext().
+ *
+ * \param pt        The point to set. This must be initialized.
+ *
+ * \return          \c 0 on success.
+ * \return          #MBEDTLS_ERR_MPI_ALLOC_FAILED on memory-allocation failure.
+ * \return          Another negative error code on other kinds of failure.
+ */
+int MBEDTLS_DEPRECATED mbedtls_ecp_set_zero( mbedtls_ecp_point *pt );
+#endif /* MBEDTLS_DEPRECATED_REMOVED */
+
+/**
+ * \brief           This function sets a point to the point at infinity.
+ *
+ * \param grp       The ECP group to use.
+ *                  This must be initialized and have group parameters
+ *                  set, for example through mbedtls_ecp_group_load().
  * \param pt        The point to set. This must be initialized.
  *
  * \return          \c 0 on success.
  * \return          #MBEDTLS_ERR_MPI_ALLOC_FAILED on memory-allocation failure.
  * \return          Another negative error code on other kinds of failure.
  */
-int mbedtls_ecp_set_zero( mbedtls_ecp_point *pt );
+int mbedtls_ecp_set_zero_ext( const mbedtls_ecp_group *grp, mbedtls_ecp_point *pt );
 
+#if !defined(MBEDTLS_DEPRECATED_REMOVED)
 /**
  * \brief           This function checks if a point is the point at infinity.
  *
+ * \note            This does not work for Edwards curves and will return a
+ *                  wrong result in that case.
+ *
+ * \deprecated      This function is deprecated and has been replaced by
+ *                  \c mbedtls_ecp_is_zero_ext().
+ *
+ * \param pt        The point to test. This must be initialized.
+ *
+ * \return          \c 1 if the point is zero.
+ * \return          \c 0 if the point is non-zero.
+ * \return          A negative error code on failure.
+ */
+int MBEDTLS_DEPRECATED mbedtls_ecp_is_zero( mbedtls_ecp_point *pt );
+#endif /* MBEDTLS_DEPRECATED_REMOVED */
+
+/**
+ * \brief           This function checks if a point is the point at infinity.
+ *
+ * \param grp       The ECP group to use.
+ *                  This must be initialized and have group parameters
+ *                  set, for example through mbedtls_ecp_group_load().
  * \param pt        The point to test. This must be initialized.
  *
  * \return          \c 1 if the point is zero.
  * \return          \c 0 if the point is non-zero.
  * \return          A negative error code on failure.
  */
-int mbedtls_ecp_is_zero( mbedtls_ecp_point *pt );
+int mbedtls_ecp_is_zero_ext( const mbedtls_ecp_group *grp, mbedtls_ecp_point *pt );
 
 /**
  * \brief           This function compares two points.

library/ecdh.c

@@ -124,7 +124,7 @@ static int ecdh_compute_shared_restartable( mbedtls_ecp_group *grp,
     MBEDTLS_MPI_CHK( mbedtls_ecp_mul_restartable( grp, &P, d, Q,
                                                   f_rng, p_rng, rs_ctx ) );
 
-    if( mbedtls_ecp_is_zero( &P ) )
+    if( mbedtls_ecp_is_zero_ext( grp, &P ) )
     {
         ret = MBEDTLS_ERR_ECP_BAD_INPUT_DATA;
         goto cleanup;

library/ecdsa.c

@@ -594,7 +594,7 @@ static int ecdsa_verify_restartable( mbedtls_ecp_group *grp,
     MBEDTLS_MPI_CHK( mbedtls_ecp_muladd_restartable( grp,
                      &R, pu1, &grp->G, pu2, Q, ECDSA_RS_ECP ) );
 
-    if( mbedtls_ecp_is_zero( &R ) )
+    if( mbedtls_ecp_is_zero_ext( grp, &R ) )
     {
         ret = MBEDTLS_ERR_ECP_VERIFY_FAILED;
         goto cleanup;

library/ecjpake.c

@@ -398,7 +398,7 @@ static int ecjpake_kkp_read( const mbedtls_md_info_t *md_info,
      * } ECJPAKEKeyKP;
      */
     MBEDTLS_MPI_CHK( mbedtls_ecp_tls_read_point( grp, X, p, end - *p ) );
-    if( mbedtls_ecp_is_zero( X ) )
+    if( mbedtls_ecp_is_zero_ext( grp, X ) )
     {
         ret = MBEDTLS_ERR_ECP_INVALID_KEY;
         goto cleanup;

library/ecp.c

@@ -683,7 +683,9 @@ int mbedtls_ecp_group_copy( mbedtls_ecp_group *dst, const mbedtls_ecp_group *src
 }
 
 /*
- * Set point to zero
+ * Set point to zero. This deprecated function does not work for Edwards curves
+ * and will silently set the point to a wrong value in that case. Use
+ * mbedtls_ecp_set_zero_ext() instead.
  */
 int mbedtls_ecp_set_zero( mbedtls_ecp_point *pt )
 {
@@ -699,7 +701,47 @@ int mbedtls_ecp_set_zero( mbedtls_ecp_point *pt )
 }
 
 /*
- * Tell if a point is zero
+ * Set point to zero.
+ */
+int mbedtls_ecp_set_zero_ext( const mbedtls_ecp_group *grp, mbedtls_ecp_point *pt )
+{
+    int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
+    ECP_VALIDATE_RET( pt != NULL );
+
+    switch( mbedtls_ecp_get_type( grp ) )
+    {
+#if defined(MBEDTLS_ECP_EDWARDS_ENABLED)
+        case MBEDTLS_ECP_TYPE_EDWARDS:
+            MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &pt->X , 0 ) );
+            MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &pt->Y , 1 ) );
+            MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &pt->Z , 1 ) );
+        break;
+#endif
+#if defined(MBEDTLS_ECP_MONTGOMERY_ENABLED)
+        case MBEDTLS_ECP_TYPE_MONTGOMERY:
+            MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &pt->X , 1 ) );
+            MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &pt->Y , 1 ) );
+            MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &pt->Z , 0 ) );
+        break;
+#endif
+#if defined(MBEDTLS_ECP_SHORT_WEIERSTRASS_ENABLED)
+        case MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS:
+            MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &pt->X , 1 ) );
+            MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &pt->Y , 1 ) );
+            MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &pt->Z , 0 ) );
+        break;
+#endif
+        default:
+            ret = MBEDTLS_ERR_ECP_BAD_INPUT_DATA;
+    }
+cleanup:
+    return( ret );
+}
+
+/*
+ * Tell if a point is zero. This deprecated function does not work for Edwards
+ * curves and will return a wrong result in that case. Use
+ * mbedtls_ecp_is_zero_ext() instead.
  */
 int mbedtls_ecp_is_zero( mbedtls_ecp_point *pt )
 {
@@ -708,6 +750,34 @@ int mbedtls_ecp_is_zero( mbedtls_ecp_point *pt )
     return( mbedtls_mpi_cmp_int( &pt->Z, 0 ) == 0 );
 }
 
+/*
+ * Tell if a point is zero.
+ */
+int mbedtls_ecp_is_zero_ext( const mbedtls_ecp_group *grp, mbedtls_ecp_point *pt )
+{
+    ECP_VALIDATE_RET( pt != NULL );
+
+    switch( mbedtls_ecp_get_type( grp ) )
+    {
+#if defined(MBEDTLS_ECP_EDWARDS_ENABLED)
+        case MBEDTLS_ECP_TYPE_EDWARDS:
+            return( mbedtls_mpi_cmp_int( &pt->X, 0 ) == 0 &&
+                    mbedtls_mpi_cmp_mpi( &pt->Y, &pt->Z ) == 0);
+        break;
+#endif
+#if defined(MBEDTLS_ECP_MONTGOMERY_ENABLED)
+        case MBEDTLS_ECP_TYPE_MONTGOMERY:
+            return( mbedtls_mpi_cmp_int( &pt->Z, 0 ) == 0 );
+#endif
+#if defined(MBEDTLS_ECP_SHORT_WEIERSTRASS_ENABLED)
+        case MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS:
+            return( mbedtls_mpi_cmp_int( &pt->Z, 0 ) == 0 );
+#endif
+        default:
+            return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA );
+    }
+}
+
 /*
  * Compare two points lazily
  */
@@ -897,7 +967,7 @@ int mbedtls_ecp_point_read_binary( const mbedtls_ecp_group *grp,
         if( buf[0] == 0x00 )
         {
             if( ilen == 1 )
-                return( mbedtls_ecp_set_zero( pt ) );
+                return( mbedtls_ecp_set_zero_ext( grp, pt ) );
             else
                 return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA );
         }
@@ -1678,7 +1748,7 @@ static int ecp_add_mixed( const mbedtls_ecp_group *grp, mbedtls_ecp_point *R,
         }
         else
         {
-            ret = mbedtls_ecp_set_zero( R );
+            ret = mbedtls_ecp_set_zero_ext( grp, R );
             goto cleanup;
         }
     }
@@ -3081,10 +3151,8 @@ static int ecp_mul_edxyz( mbedtls_ecp_group *grp, mbedtls_ecp_point *R,
     /* Read from P before writing to R, in case P == R */
     MBEDTLS_MPI_CHK( mbedtls_ecp_copy( &RP, P ) );
 
-    /* Set R to zero in projective coordinates */
-    MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &R->X, 0 ) );
-    MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &R->Y, 1 ) );
-    MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &R->Z, 1 ) );
+    /* Set R to zero */
+    MBEDTLS_MPI_CHK( mbedtls_ecp_set_zero_ext( grp, R ) );
 
     /* RP.X and RP.Y might be slightly larger than P, so reduce them */
     MOD_ADD( &RP.X );
@@ -3373,7 +3441,7 @@ static int mbedtls_ecp_mul_shortcuts( mbedtls_ecp_group *grp,
 
     if( mbedtls_mpi_cmp_int( m, 0 ) == 0 )
     {
-        MBEDTLS_MPI_CHK( mbedtls_ecp_set_zero( R ) );
+        MBEDTLS_MPI_CHK( mbedtls_ecp_set_zero_ext( grp, R ) );
     }
     else if( mbedtls_mpi_cmp_int( m, 1 ) == 0 )
     {

library/psa_crypto_ecp.c

@@ -215,7 +215,7 @@ psa_status_t mbedtls_psa_ecp_export_key( psa_key_type_t type,
     if( PSA_KEY_TYPE_IS_PUBLIC_KEY( type ) )
     {
         /* Check whether the public part is loaded */
-        if( mbedtls_ecp_is_zero( &ecp->Q ) )
+        if( mbedtls_ecp_is_zero_ext( &ecp->grp, &ecp->Q ) )
         {
             /* Calculate the public key */
             status = mbedtls_to_psa_error(
@@ -447,7 +447,7 @@ psa_status_t mbedtls_psa_ecdsa_verify_hash(
                                               curve_bytes ) );
 
     /* Check whether the public part is loaded. If not, load it. */
-    if( mbedtls_ecp_is_zero( &ecp->Q ) )
+    if( mbedtls_ecp_is_zero_ext( &ecp->grp, &ecp->Q ) )
     {
         MBEDTLS_MPI_CHK(
             mbedtls_ecp_mul( &ecp->grp, &ecp->Q, &ecp->d, &ecp->grp.G,

tests/suites/test_suite_ecdh.function

@@ -156,7 +156,7 @@ void ecdh_primitive_testvec( int id, data_t * rnd_buf_A, char * xA_str,
     TEST_ASSERT( mbedtls_ecdh_gen_public( &grp, &dA, &qA,
                                           mbedtls_test_rnd_buffer_rand,
                                           &rnd_info_A ) == 0 );
-    TEST_ASSERT( ! mbedtls_ecp_is_zero( &qA ) );
+    TEST_ASSERT( ! mbedtls_ecp_is_zero_ext( &grp, &qA ) );
     TEST_ASSERT( mbedtls_test_read_mpi( &check, 16, xA_str ) == 0 );
     TEST_ASSERT( mbedtls_mpi_cmp_mpi( &qA.X, &check ) == 0 );
     TEST_ASSERT( mbedtls_test_read_mpi( &check, 16, yA_str ) == 0 );
@@ -165,7 +165,7 @@ void ecdh_primitive_testvec( int id, data_t * rnd_buf_A, char * xA_str,
     TEST_ASSERT( mbedtls_ecdh_gen_public( &grp, &dB, &qB,
                                           mbedtls_test_rnd_buffer_rand,
                                           &rnd_info_B ) == 0 );
-    TEST_ASSERT( ! mbedtls_ecp_is_zero( &qB ) );
+    TEST_ASSERT( ! mbedtls_ecp_is_zero_ext( &grp, &qB ) );
     TEST_ASSERT( mbedtls_test_read_mpi( &check, 16, xB_str ) == 0 );
     TEST_ASSERT( mbedtls_mpi_cmp_mpi( &qB.X, &check ) == 0 );
     TEST_ASSERT( mbedtls_test_read_mpi( &check, 16, yB_str ) == 0 );

tests/suites/test_suite_ecp.function

@@ -712,19 +712,19 @@ void ecp_tls_write_read_point( int id )
     TEST_ASSERT( vbuf == buf + olen );
 
     memset( buf, 0x00, sizeof( buf ) ); vbuf = buf;
-    TEST_ASSERT( mbedtls_ecp_set_zero( &pt ) == 0 );
+    TEST_ASSERT( mbedtls_ecp_set_zero_ext( &grp, &pt ) == 0 );
     TEST_ASSERT( mbedtls_ecp_tls_write_point( &grp, &pt,
                     MBEDTLS_ECP_PF_COMPRESSED, &olen, buf, 256 ) == 0 );
     TEST_ASSERT( mbedtls_ecp_tls_read_point( &grp, &pt, &vbuf, olen ) == 0 );
-    TEST_ASSERT( mbedtls_ecp_is_zero( &pt ) );
+    TEST_ASSERT( mbedtls_ecp_is_zero_ext( &grp, &pt ) );
     TEST_ASSERT( vbuf == buf + olen );
 
     memset( buf, 0x00, sizeof( buf ) ); vbuf = buf;
-    TEST_ASSERT( mbedtls_ecp_set_zero( &pt ) == 0 );
+    TEST_ASSERT( mbedtls_ecp_set_zero_ext( &grp, &pt ) == 0 );
     TEST_ASSERT( mbedtls_ecp_tls_write_point( &grp, &pt,
                     MBEDTLS_ECP_PF_UNCOMPRESSED, &olen, buf, 256 ) == 0 );
     TEST_ASSERT( mbedtls_ecp_tls_read_point( &grp, &pt, &vbuf, olen ) == 0 );
-    TEST_ASSERT( mbedtls_ecp_is_zero( &pt ) );
+    TEST_ASSERT( mbedtls_ecp_is_zero_ext( &grp, &pt ) );
     TEST_ASSERT( vbuf == buf + olen );
 
 exit: