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>
This commit is contained in:
Hanno Becker 2022-01-09 05:46:18 +00:00
parent c27a0e0093
commit ee95f6c4c9

View file

@ -1386,7 +1386,8 @@ static int ecp_normalize_jac_many( const mbedtls_ecp_group *grp,
*/
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 @@ cleanup:
* 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 @@ static int ecp_add_mixed( const mbedtls_ecp_group *grp, mbedtls_ecp_point *R,
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 @@ norm_add:
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] );