From 7b774483bfa994f03115dec929a188b6eb8130c0 Mon Sep 17 00:00:00 2001
From: Janos Follath <janos.follath@arm.com>
Date: Mon, 7 Jan 2019 17:27:56 +0000
Subject: [PATCH] Correct deterministic ECDSA behavior

We were still reusing the internal HMAC-DRBG of the deterministic ECDSA
for blinding. This meant that with cryptographically low likelyhood the
result was not the same signature as the one the deterministic ECDSA
algorithm has to produce (however it is still a valid ECDSA signature).

To correct this we seed a second HMAC-DRBG with the same seed to restore
correct behavior. We also apply a label to avoid reusing the bits of the
ephemeral key for a different purpose and reduce the chance that they
leak.

This workaround can't be implemented in the restartable case without
penalising the case where external RNG is available or completely
defeating the purpose of the restartable feature, therefore in this case
the small chance of incorrect behavior remains.
---
 library/ecdsa.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/library/ecdsa.c b/library/ecdsa.c
index b3e516179..2059599a1 100644
--- a/library/ecdsa.c
+++ b/library/ecdsa.c
@@ -195,12 +195,16 @@ static int ecdsa_sign_det_internal( mbedtls_ecp_group *grp, mbedtls_mpi *r,
     size_t grp_len = ( grp->nbits + 7 ) / 8;
     const mbedtls_md_info_t *md_info;
     mbedtls_mpi h;
+    /* Variables for deterministic blinding fallback */
+    const char* blind_label = "BLINDING CONTEXT";
+    mbedtls_hmac_drbg_context rng_ctx_blind;
 
     if( ( md_info = mbedtls_md_info_from_type( md_alg ) ) == NULL )
         return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA );
 
     mbedtls_mpi_init( &h );
     mbedtls_hmac_drbg_init( &rng_ctx );
+    mbedtls_hmac_drbg_init( &rng_ctx_blind );
 
     /* Use private key and message hash (reduced) to initialize HMAC_DRBG */
     MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( d, data, grp_len ) );
@@ -213,18 +217,38 @@ static int ecdsa_sign_det_internal( mbedtls_ecp_group *grp, mbedtls_mpi *r,
                                    mbedtls_hmac_drbg_random, &rng_ctx,
                                    f_rng_blind, p_rng_blind );
     else
+    {
         /*
-         * Use the same RNG for both blinding and ephemeral key generation.
-         * Since the RNG output is always the same for the same key and message,
-         * this limits the efficiency of blinding and leaks information through
-         * side channels.
+         * To avoid reusing rng_ctx and risking incorrect behavior we seed a
+         * second HMAC-DRBG with the same seed. We also apply a label to avoid
+         * reusing the bits of the ephemeral key for blinding and eliminate the
+         * risk that they leak this way.
+         */
+
+        mbedtls_hmac_drbg_seed_buf( &rng_ctx_blind, md_info,
+                                    data, 2 * grp_len );
+        ret = mbedtls_hmac_drbg_update_ret( &rng_ctx_blind,
+                                            (const unsigned char*) blind_label,
+                                            strlen( blind_label ) );
+        if( ret != 0 )
+            goto cleanup;
+
+        /*
+         * Since the output of the RNGs is always the same for the same key and
+         * message, this limits the efficiency of blinding and leaks information
+         * through side channels. After mbedtls_ecdsa_sign_det() is removed NULL
+         * won't be a valid value for f_rng_blind anymore. Therefore it should
+         * be checked by the caller and this branch and check can be removed.
          */
         ret = ecdsa_sign_internal( grp, r, s, d, buf, blen,
                                    mbedtls_hmac_drbg_random, &rng_ctx,
-                                   mbedtls_hmac_drbg_random, &rng_ctx );
+                                   mbedtls_hmac_drbg_random, &rng_ctx_blind );
+
+    }
 
 cleanup:
     mbedtls_hmac_drbg_free( &rng_ctx );
+    mbedtls_hmac_drbg_free( &rng_ctx_blind );
     mbedtls_mpi_free( &h );
 
     return( ret );