Don't allow Z coordinate being unset in ecp_add_mixed()
Previously, ecp_add_mixed(), commputing say P+Q, would allow for the
Q parameter to have an unset Z coordinate as a shortcut for Z == 1.
This was leveraged during computation and usage of the T-table
(storing low multiples of the to-be-multiplied point on the curve).
It is a potentially error-prone corner case, though, since an MPIs
with unset data pointer coordinate and limb size 0 is also a valid
representation of the number 0.
As a first step towards removing ECP points with unset Z coordinate,
the constant time T-array getter ecp_select_comb() has previously
been modified to return 'full' mbedtls_ecp_point structures,
including a 1-initialized Z-coordinate.
Similarly, this commit ...
- Modifies ecp_normalize_jac_many() to set the Z coordinates
of the points it operates on to 1 instead of freeing them.
- Frees the Z-coordinates of the T[]-array explicitly
once the computation and normalization of the T-table has finished.
As a minimal functional difference between old and new code,
the new code also frees the Z-coordinate of T[0]=P, which the
old code did not.
- Modifies ecp_add_mixed() to no longer allow unset Z coordinates.
Except for the post-precomputation storage form of the T[] array,
the code does therefore no longer use EC points with unset Z coordinate.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
diff --git a/library/ecp.c b/library/ecp.c
index 47ac5c0..f5ae8ee 100644
--- a/library/ecp.c
+++ b/library/ecp.c
@@ -1386,7 +1386,8 @@
*/
MBEDTLS_MPI_CHK( mbedtls_mpi_shrink( &T[i]->X, grp->P.n ) );
MBEDTLS_MPI_CHK( mbedtls_mpi_shrink( &T[i]->Y, grp->P.n ) );
- mbedtls_mpi_free( &T[i]->Z );
+
+ MPI_ECP_LSET( &T[i]->Z, 1 );
if( i == 0 )
break;
@@ -1529,8 +1530,6 @@
* due to the choice of precomputed points in the modified comb method.
* So branches for these cases do not leak secret information.
*
- * We accept Q->Z being unset (saving memory in tables) as meaning 1.
- *
* Cost: 1A := 8M + 3S
*/
static int ecp_add_mixed( const mbedtls_ecp_group *grp, mbedtls_ecp_point *R,
@@ -1558,19 +1557,22 @@
mbedtls_mpi * const Y = &R->Y;
mbedtls_mpi * const Z = &R->Z;
+ if( !MPI_ECP_VALID( &Q->Z ) )
+ return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA );
+
/*
* Trivial cases: P == 0 or Q == 0 (case 1)
*/
if( MPI_ECP_CMP_INT( &P->Z, 0 ) == 0 )
return( mbedtls_ecp_copy( R, Q ) );
- if( MPI_ECP_VALID( &Q->Z ) && MPI_ECP_CMP_INT( &Q->Z, 0 ) == 0 )
+ if( MPI_ECP_CMP_INT( &Q->Z, 0 ) == 0 )
return( mbedtls_ecp_copy( R, P ) );
/*
* Make sure Q coordinates are normalized
*/
- if( MPI_ECP_VALID( &Q->Z ) && MPI_ECP_CMP_INT( &Q->Z, 1 ) != 0 )
+ if( MPI_ECP_CMP_INT( &Q->Z, 1 ) != 0 )
return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA );
MPI_ECP_SQR( &tmp[0], &P->Z );
@@ -1918,6 +1920,14 @@
MBEDTLS_MPI_CHK( ecp_normalize_jac_many( grp, TT, j ) );
+ /* Free Z coordinate (=1 after normalization) to save RAM.
+ * This makes T[i] invalid as mbedtls_ecp_points, but this is OK
+ * since from this point onwards, they are only accessed indirectly
+ * via the getter function ecp_select_comb() which does set the
+ * target's Z coordinate to 1. */
+ for( i = 0; i < T_size; i++ )
+ mbedtls_mpi_free( &T[i].Z );
+
cleanup:
mbedtls_mpi_free( &tmp[0] );