commit 675439620d3029c313e0a804e0ae1e24d6ef376a
author Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com> Fri Apr 21 13:19:43 2017 +0200
committer Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com> Wed Aug 09 11:44:53 2017 +0200
tree 6260b3269a5f292fe8c2508e25d296f2c8514e31
parent b90883dc1dc42891354c0f40a908e298d8d0af32

Improve sign/key_tries handling

(Unrelated to restartable work, just noticed while staring at the code.)

Checking at the end is inefficient as we might give up when we just generated
a valid signature or key.

library/ecdsa.c

diff --git a/library/ecdsa.c b/library/ecdsa.c
index 4c0b614..14854ff 100644
--- a/library/ecdsa.c
+++ b/library/ecdsa.c
@@ -252,6 +252,12 @@
     sign_tries = 0;
     do
     {
+        if( sign_tries++ > 10 )
+        {
+            ret = MBEDTLS_ERR_ECP_RANDOM_FAILED;
+            goto cleanup;
+        }
+
         /*
          * Steps 1-3: generate a suitable ephemeral keypair
          * and set r = xR mod n
@@ -259,14 +265,14 @@
         key_tries = 0;
         do
         {
-            MBEDTLS_MPI_CHK( mbedtls_ecp_gen_keypair( grp, &k, &R, f_rng, p_rng ) );
-            MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( r, &R.X, &grp->N ) );
-
             if( key_tries++ > 10 )
             {
                 ret = MBEDTLS_ERR_ECP_RANDOM_FAILED;
                 goto cleanup;
             }
+
+            MBEDTLS_MPI_CHK( mbedtls_ecp_gen_keypair( grp, &k, &R, f_rng, p_rng ) );
+            MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( r, &R.X, &grp->N ) );
         }
         while( mbedtls_mpi_cmp_int( r, 0 ) == 0 );
 
@@ -303,12 +309,6 @@
         MBEDTLS_MPI_CHK( mbedtls_mpi_inv_mod( s, &k, &grp->N ) );
         MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( s, s, &e ) );
         MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( s, s, &grp->N ) );
-
-        if( sign_tries++ > 10 )
-        {
-            ret = MBEDTLS_ERR_ECP_RANDOM_FAILED;
-            goto cleanup;
-        }
     }
     while( mbedtls_mpi_cmp_int( s, 0 ) == 0 );
 

library/ecp.c

diff --git a/library/ecp.c b/library/ecp.c
index 0f1cbfc..9b586ee 100644
--- a/library/ecp.c
+++ b/library/ecp.c
@@ -2531,24 +2531,23 @@
          * - keep the leftmost nbits bits of the generated octet string;
          * - try until result is in the desired range.
          * This also avoids any biais, which is especially important for ECDSA.
+         *
+         * Each try has at worst a probability 1/2 of failing (the msb has
+         * a probability 1/2 of being 0, and then the result will be < N),
+         * so after 30 tries failure probability is a most 2**(-30).
+         *
+         * For most curves, 1 try is enough with overwhelming probability,
+         * since N starts with a lot of 1s in binary, but some curves
+         * such as secp224k1 are actually very close to the worst case.
          */
         do
         {
+            if( ++count > 30 )
+                return( MBEDTLS_ERR_ECP_RANDOM_FAILED );
+
             MBEDTLS_MPI_CHK( f_rng( p_rng, rnd, n_size ) );
             MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( d, rnd, n_size ) );
             MBEDTLS_MPI_CHK( mbedtls_mpi_shift_r( d, 8 * n_size - grp->nbits ) );
-
-            /*
-             * Each try has at worst a probability 1/2 of failing (the msb has
-             * a probability 1/2 of being 0, and then the result will be < N),
-             * so after 30 tries failure probability is a most 2**(-30).
-             *
-             * For most curves, 1 try is enough with overwhelming probability,
-             * since N starts with a lot of 1s in binary, but some curves
-             * such as secp224k1 are actually very close to the worst case.
-             */
-            if( ++count > 30 )
-                return( MBEDTLS_ERR_ECP_RANDOM_FAILED );
         }
         while( mbedtls_mpi_cmp_int( d, 1 ) < 0 ||
                mbedtls_mpi_cmp_mpi( d, &grp->N ) >= 0 );