Merge pull request #276 from dgreen-arm/dev/dgreen-arm/iotcrypt-616-fix-ecdsa-rng

Fix deterministic ECDSA RNG misuse

Author: Patater

include/mbedtls/config.h

@@ -441,6 +441,16 @@
  *            dependencies on them, and considering stronger message digests
  *            and ciphers instead.
  *
+ * \warning   If both MBEDTLS_ECDSA_SIGN_ALT and MBEDTLS_ECDSA_DETERMINISTIC are
+ *            enabled, then the deterministic ECDH signature functions pass the
+ *            the static HMAC-DRBG as RNG to mbedtls_ecdsa_sign(). Therefore
+ *            alternative implementations should use the RNG only for generating
+ *            the ephemeral key and nothing else. If this is not possible, then
+ *            MBEDTLS_ECDSA_DETERMINISTIC should be disabled and an alternative
+ *            implementation should be provided for mbedtls_ecdsa_sign_det_ext()
+ *            (and for mbedtls_ecdsa_sign_det() too if backward compatibility is
+ *            desirable).
+ *
  */
 //#define MBEDTLS_MD2_PROCESS_ALT
 //#define MBEDTLS_MD4_PROCESS_ALT

include/mbedtls/ecdsa.h

@@ -176,6 +176,12 @@ int mbedtls_ecdsa_sign( mbedtls_ecp_group *grp, mbedtls_mpi *r, mbedtls_mpi *s,
                 int (*f_rng)(void *, unsigned char *, size_t), void *p_rng );
 
 #if defined(MBEDTLS_ECDSA_DETERMINISTIC)
+#if ! defined(MBEDTLS_DEPRECATED_REMOVED)
+#if defined(MBEDTLS_DEPRECATED_WARNING)
+#define MBEDTLS_DEPRECATED    __attribute__((deprecated))
+#else
+#define MBEDTLS_DEPRECATED
+#endif
 /**
  * \brief           This function computes the ECDSA signature of a
  *                  previously-hashed message, deterministic version.
@@ -190,6 +196,19 @@ int mbedtls_ecdsa_sign( mbedtls_ecp_group *grp, mbedtls_mpi *r, mbedtls_mpi *s,
  *                  (SECG): SEC1 Elliptic Curve Cryptography</em>, section
  *                  4.1.3, step 5.
  *
+ * \warning         Since the output of the internal RNG is always the same for
+ *                  the same key and message, this limits the efficiency of
+ *                  blinding and leaks information through side channels. For
+ *                  secure behavior use mbedtls_ecdsa_sign_det_ext() instead.
+ *
+ *                  (Optimally the blinding is a random value that is different
+ *                  on every execution. In this case the blinding is still
+ *                  random from the attackers perspective, but is the same on
+ *                  each execution. This means that this blinding does not
+ *                  prevent attackers from recovering secrets by combining
+ *                  several measurement traces, but may prevent some attacks
+ *                  that exploit relationships between secret data.)
+ *
  * \see             ecp.h
  *
  * \param grp       The context for the elliptic curve to use.
@@ -214,7 +233,55 @@ int mbedtls_ecdsa_sign( mbedtls_ecp_group *grp, mbedtls_mpi *r, mbedtls_mpi *s,
 int mbedtls_ecdsa_sign_det( mbedtls_ecp_group *grp, mbedtls_mpi *r,
                             mbedtls_mpi *s, const mbedtls_mpi *d,
                             const unsigned char *buf, size_t blen,
-                            mbedtls_md_type_t md_alg );
+                            mbedtls_md_type_t md_alg ) MBEDTLS_DEPRECATED;
+#undef MBEDTLS_DEPRECATED
+#endif /* MBEDTLS_DEPRECATED_REMOVED */
+
+/**
+ * \brief           This function computes the ECDSA signature of a
+ *                  previously-hashed message, deterministic version.
+ *
+ *                  For more information, see <em>RFC-6979: Deterministic
+ *                  Usage of the Digital Signature Algorithm (DSA) and Elliptic
+ *                  Curve Digital Signature Algorithm (ECDSA)</em>.
+ *
+ * \note            If the bitlength of the message hash is larger than the
+ *                  bitlength of the group order, then the hash is truncated as
+ *                  defined in <em>Standards for Efficient Cryptography Group
+ *                  (SECG): SEC1 Elliptic Curve Cryptography</em>, section
+ *                  4.1.3, step 5.
+ *
+ * \see             ecp.h
+ *
+ * \param grp           The context for the elliptic curve to use.
+ *                      This must be initialized and have group parameters
+ *                      set, for example through mbedtls_ecp_group_load().
+ * \param r             The MPI context in which to store the first part
+ *                      the signature. This must be initialized.
+ * \param s             The MPI context in which to store the second part
+ *                      the signature. This must be initialized.
+ * \param d             The private signing key. This must be initialized
+ *                      and setup, for example through mbedtls_ecp_gen_privkey().
+ * \param buf           The hashed content to be signed. This must be a readable
+ *                      buffer of length \p blen Bytes. It may be \c NULL if
+ *                      \p blen is zero.
+ * \param blen          The length of \p buf in Bytes.
+ * \param md_alg        The hash algorithm used to hash the original data.
+ * \param f_rng_blind   The RNG function used for blinding. This must not be
+ *                      \c NULL.
+ * \param p_rng_blind   The RNG context to be passed to \p f_rng. This may be
+ *                      \c NULL if \p f_rng doesn't need a context parameter.
+ *
+ * \return          \c 0 on success.
+ * \return          An \c MBEDTLS_ERR_ECP_XXX or \c MBEDTLS_MPI_XXX
+ *                  error code on failure.
+ */
+int mbedtls_ecdsa_sign_det_ext( mbedtls_ecp_group *grp, mbedtls_mpi *r,
+                            mbedtls_mpi *s, const mbedtls_mpi *d,
+                            const unsigned char *buf, size_t blen,
+                            mbedtls_md_type_t md_alg,
+                            int (*f_rng_blind)(void *, unsigned char *, size_t),
+                            void *p_rng_blind );
 #endif /* MBEDTLS_ECDSA_DETERMINISTIC */
 
 /**
@@ -293,7 +360,8 @@ int mbedtls_ecdsa_verify( mbedtls_ecp_group *grp,
  *                  the signature written. Must not be \c NULL.
  * \param f_rng     The RNG function. This must not be \c NULL if
  *                  #MBEDTLS_ECDSA_DETERMINISTIC is unset. Otherwise,
- *                  it is unused and may be set to \c NULL.
+ *                  it is used only for blinding and may be set to \c NULL, but
+ *                  doing so is DEPRECATED.
  * \param p_rng     The RNG context to be passed to \p f_rng. This may be
  *                  \c NULL if \p f_rng is \c NULL or doesn't use a context.
  *

library/ecdsa.c

@@ -254,6 +254,8 @@ static int ecdsa_sign_restartable( mbedtls_ecp_group *grp,
                 mbedtls_mpi *r, mbedtls_mpi *s,
                 const mbedtls_mpi *d, const unsigned char *buf, size_t blen,
                 int (*f_rng)(void *, unsigned char *, size_t), void *p_rng,
+                int (*f_rng_blind)(void *, unsigned char *, size_t),
+                void *p_rng_blind,
                 mbedtls_ecdsa_restart_ctx *rs_ctx )
 {
     int ret, key_tries, sign_tries;
@@ -323,7 +325,9 @@ static int ecdsa_sign_restartable( mbedtls_ecp_group *grp,
 mul:
 #endif
             MBEDTLS_MPI_CHK( mbedtls_ecp_mul_restartable( grp, &R, pk, &grp->G,
-                                                  f_rng, p_rng, ECDSA_RS_ECP ) );
+                                                          f_rng_blind,
+                                                          p_rng_blind,
+                                                          ECDSA_RS_ECP ) );
             MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( pr, &R.X, &grp->N ) );
         }
         while( mbedtls_mpi_cmp_int( pr, 0 ) == 0 );
@@ -349,7 +353,8 @@ static int ecdsa_sign_restartable( mbedtls_ecp_group *grp,
          * Generate a random value to blind inv_mod in next step,
          * avoiding a potential timing leak.
          */
-        MBEDTLS_MPI_CHK( mbedtls_ecp_gen_privkey( grp, &t, f_rng, p_rng ) );
+        MBEDTLS_MPI_CHK( mbedtls_ecp_gen_privkey( grp, &t, f_rng_blind,
+                                                  p_rng_blind ) );
 
         /*
          * Step 6: compute s = (e + r * d) / k = t (e + rd) / (kt) mod n
@@ -406,8 +411,9 @@ int mbedtls_ecdsa_sign( mbedtls_ecp_group *grp, mbedtls_mpi *r, mbedtls_mpi *s,
     ECDSA_VALIDATE_RET( f_rng != NULL );
     ECDSA_VALIDATE_RET( buf   != NULL || blen == 0 );
 
+    /* Use the same RNG for both blinding and ephemeral key generation */
     return( ecdsa_sign_restartable( grp, r, s, d, buf, blen,
-                                    f_rng, p_rng, NULL ) );
+                                    f_rng, p_rng, f_rng, p_rng, NULL ) );
 }
 #endif /* !MBEDTLS_ECDSA_SIGN_ALT */
 
@@ -419,6 +425,8 @@ static int ecdsa_sign_det_restartable( mbedtls_ecp_group *grp,
                     mbedtls_mpi *r, mbedtls_mpi *s,
                     const mbedtls_mpi *d, const unsigned char *buf, size_t blen,
                     mbedtls_md_type_t md_alg,
+                    int (*f_rng_blind)(void *, unsigned char *, size_t),
+                    void *p_rng_blind,
                     mbedtls_ecdsa_restart_ctx *rs_ctx )
 {
     int ret;
@@ -465,8 +473,69 @@ static int ecdsa_sign_det_restartable( mbedtls_ecp_group *grp,
     ret = mbedtls_ecdsa_sign( grp, r, s, d, buf, blen,
                               mbedtls_hmac_drbg_random, p_rng );
 #else
-    ret = ecdsa_sign_restartable( grp, r, s, d, buf, blen,
-                      mbedtls_hmac_drbg_random, p_rng, rs_ctx );
+    if( f_rng_blind != NULL )
+        ret = ecdsa_sign_restartable( grp, r, s, d, buf, blen,
+                                      mbedtls_hmac_drbg_random, p_rng,
+                                      f_rng_blind, p_rng_blind, rs_ctx );
+    else
+    {
+        mbedtls_hmac_drbg_context *p_rng_blind_det;
+
+#if !defined(MBEDTLS_ECP_RESTARTABLE)
+        /*
+         * 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.
+         */
+        const char* blind_label = "BLINDING CONTEXT";
+        mbedtls_hmac_drbg_context rng_ctx_blind;
+
+        mbedtls_hmac_drbg_init( &rng_ctx_blind );
+        p_rng_blind_det = &rng_ctx_blind;
+        mbedtls_hmac_drbg_seed_buf( p_rng_blind_det, md_info,
+                                    data, 2 * grp_len );
+        ret = mbedtls_hmac_drbg_update_ret( p_rng_blind_det,
+                                            (const unsigned char*) blind_label,
+                                            strlen( blind_label ) );
+        if( ret != 0 )
+        {
+            mbedtls_hmac_drbg_free( &rng_ctx_blind );
+            goto cleanup;
+        }
+#else
+        /*
+         * In the case of restartable computations we would either need to store
+         * the second RNG in the restart context too or set it up at every
+         * restart. The first option would penalize the correct application of
+         * the function and the second would defeat the purpose of the
+         * restartable feature.
+         *
+         * Therefore in this case we reuse the original RNG. This comes with the
+         * price that the resulting signature might not be a valid deterministic
+         * ECDSA signature with a very low probability (same magnitude as
+         * successfully guessing the private key). However even then it is still
+         * a valid ECDSA signature.
+         */
+        p_rng_blind_det = p_rng;
+#endif /* MBEDTLS_ECP_RESTARTABLE */
+
+        /*
+         * 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_restartable( grp, r, s, d, buf, blen,
+                                      mbedtls_hmac_drbg_random, p_rng,
+                                      mbedtls_hmac_drbg_random, p_rng_blind_det,
+                                      rs_ctx );
+
+#if !defined(MBEDTLS_ECP_RESTARTABLE)
+        mbedtls_hmac_drbg_free( &rng_ctx_blind );
+#endif
+    }
 #endif /* MBEDTLS_ECDSA_SIGN_ALT */
 
 cleanup:
@@ -479,19 +548,43 @@ static int ecdsa_sign_det_restartable( mbedtls_ecp_group *grp,
 }
 
 /*
- * Deterministic signature wrapper
+ * Deterministic signature wrappers
  */
-int mbedtls_ecdsa_sign_det( mbedtls_ecp_group *grp, mbedtls_mpi *r, mbedtls_mpi *s,
-                    const mbedtls_mpi *d, const unsigned char *buf, size_t blen,
-                    mbedtls_md_type_t md_alg )
+
+#if !defined(MBEDTLS_DEPRECATED_REMOVED)
+int mbedtls_ecdsa_sign_det( mbedtls_ecp_group *grp, mbedtls_mpi *r,
+                            mbedtls_mpi *s, const mbedtls_mpi *d,
+                            const unsigned char *buf, size_t blen,
+                            mbedtls_md_type_t md_alg )
+{
+    ECDSA_VALIDATE_RET( grp   != NULL );
+    ECDSA_VALIDATE_RET( r     != NULL );
+    ECDSA_VALIDATE_RET( s     != NULL );
+    ECDSA_VALIDATE_RET( d     != NULL );
+    ECDSA_VALIDATE_RET( buf   != NULL || blen == 0 );
+
+    return( ecdsa_sign_det_restartable( grp, r, s, d, buf, blen, md_alg,
+                                        NULL, NULL, NULL ) );
+}
+#endif /* MBEDTLS_DEPRECATED_REMOVED */
+
+int mbedtls_ecdsa_sign_det_ext( mbedtls_ecp_group *grp, mbedtls_mpi *r,
+                                mbedtls_mpi *s, const mbedtls_mpi *d,
+                                const unsigned char *buf, size_t blen,
+                                mbedtls_md_type_t md_alg,
+                                int (*f_rng_blind)(void *, unsigned char *,
+                                                   size_t),
+                                void *p_rng_blind )
 {
     ECDSA_VALIDATE_RET( grp   != NULL );
     ECDSA_VALIDATE_RET( r     != NULL );
     ECDSA_VALIDATE_RET( s     != NULL );
     ECDSA_VALIDATE_RET( d     != NULL );
     ECDSA_VALIDATE_RET( buf   != NULL || blen == 0 );
+    ECDSA_VALIDATE_RET( f_rng_blind != NULL );
 
-    return( ecdsa_sign_det_restartable( grp, r, s, d, buf, blen, md_alg, NULL ) );
+    return( ecdsa_sign_det_restartable( grp, r, s, d, buf, blen, md_alg,
+                                        f_rng_blind, p_rng_blind, NULL ) );
 }
 #endif /* MBEDTLS_ECDSA_DETERMINISTIC */
 
@@ -670,20 +763,20 @@ int mbedtls_ecdsa_write_signature_restartable( mbedtls_ecdsa_context *ctx,
     mbedtls_mpi_init( &s );
 
 #if defined(MBEDTLS_ECDSA_DETERMINISTIC)
-    (void) f_rng;
-    (void) p_rng;
-
     MBEDTLS_MPI_CHK( ecdsa_sign_det_restartable( &ctx->grp, &r, &s, &ctx->d,
-                             hash, hlen, md_alg, rs_ctx ) );
+                                                 hash, hlen, md_alg, f_rng,
+                                                 p_rng, rs_ctx ) );
 #else
     (void) md_alg;
 
 #if defined(MBEDTLS_ECDSA_SIGN_ALT)
     MBEDTLS_MPI_CHK( mbedtls_ecdsa_sign( &ctx->grp, &r, &s, &ctx->d,
                          hash, hlen, f_rng, p_rng ) );
 #else
+    /* Use the same RNG for both blinding and ephemeral key generation */
     MBEDTLS_MPI_CHK( ecdsa_sign_restartable( &ctx->grp, &r, &s, &ctx->d,
-                         hash, hlen, f_rng, p_rng, rs_ctx ) );
+                                             hash, hlen, f_rng, p_rng, f_rng,
+                                             p_rng, rs_ctx ) );
 #endif /* MBEDTLS_ECDSA_SIGN_ALT */
 #endif /* MBEDTLS_ECDSA_DETERMINISTIC */
 

library/psa_crypto.c

@@ -3271,9 +3271,11 @@ static psa_status_t psa_ecdsa_sign( mbedtls_ecp_keypair *ecp,
         psa_algorithm_t hash_alg = PSA_ALG_SIGN_GET_HASH( alg );
         const mbedtls_md_info_t *md_info = mbedtls_md_info_from_psa( hash_alg );
         mbedtls_md_type_t md_alg = mbedtls_md_get_type( md_info );
-        MBEDTLS_MPI_CHK( mbedtls_ecdsa_sign_det( &ecp->grp, &r, &s, &ecp->d,
-                                                 hash, hash_length,
-                                                 md_alg ) );
+        MBEDTLS_MPI_CHK( mbedtls_ecdsa_sign_det_ext( &ecp->grp, &r, &s,
+                                                     &ecp->d, hash,
+                                                     hash_length, md_alg,
+                                                     mbedtls_ctr_drbg_random,
+                                                     &global_data.ctr_drbg ) );
     }
     else
 #endif /* MBEDTLS_ECDSA_DETERMINISTIC */

tests/suites/test_suite_ecdsa.function

@@ -55,25 +55,30 @@ void ecdsa_invalid_param( )
 
 #if defined(MBEDTLS_ECDSA_DETERMINISTIC)
     TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ECP_BAD_INPUT_DATA,
-                       mbedtls_ecdsa_sign_det( NULL, &m, &m, &m,
-                                                buf, sizeof( buf ),
-                                                valid_md ) );
-    TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ECP_BAD_INPUT_DATA,
-                       mbedtls_ecdsa_sign_det( &grp, NULL, &m, &m,
-                                                buf, sizeof( buf ),
-                                                valid_md ) );
-    TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ECP_BAD_INPUT_DATA,
-                       mbedtls_ecdsa_sign_det( &grp, &m, NULL, &m,
-                                                buf, sizeof( buf ),
-                                                valid_md ) );
-    TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ECP_BAD_INPUT_DATA,
-                       mbedtls_ecdsa_sign_det( &grp, &m, &m, NULL,
-                                                buf, sizeof( buf ),
-                                                valid_md ) );
-    TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ECP_BAD_INPUT_DATA,
-                       mbedtls_ecdsa_sign_det( &grp, &m, &m, &m,
-                                                NULL, sizeof( buf ),
-                                                valid_md ) );
+                       mbedtls_ecdsa_sign_det_ext( NULL, &m, &m, &m,
+                                                   buf, sizeof( buf ),
+                                                   valid_md,
+                                                   rnd_std_rand, NULL ) );
+    TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ECP_BAD_INPUT_DATA,
+                       mbedtls_ecdsa_sign_det_ext( &grp, NULL, &m, &m,
+                                                   buf, sizeof( buf ),
+                                                   valid_md,
+                                                   rnd_std_rand, NULL ) );
+    TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ECP_BAD_INPUT_DATA,
+                       mbedtls_ecdsa_sign_det_ext( &grp, &m, NULL, &m,
+                                                   buf, sizeof( buf ),
+                                                   valid_md,
+                                                   rnd_std_rand, NULL ) );
+    TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ECP_BAD_INPUT_DATA,
+                       mbedtls_ecdsa_sign_det_ext( &grp, &m, &m, NULL,
+                                                   buf, sizeof( buf ),
+                                                   valid_md,
+                                                   rnd_std_rand, NULL ) );
+    TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ECP_BAD_INPUT_DATA,
+                       mbedtls_ecdsa_sign_det_ext( &grp, &m, &m, &m,
+                                                   NULL, sizeof( buf ),
+                                                   valid_md,
+                                                   rnd_std_rand, NULL ) );
 #endif /* MBEDTLS_ECDSA_DETERMINISTIC */
 
     TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ECP_BAD_INPUT_DATA,
@@ -325,7 +330,10 @@ void ecdsa_det_test_vectors( int id, char * d_str, int md_alg, char * msg,
     TEST_ASSERT( mbedtls_md( md_info, (const unsigned char *) msg,
                  strlen( msg ), hash ) == 0 );
 
-    TEST_ASSERT( mbedtls_ecdsa_sign_det( &grp, &r, &s, &d, hash, hlen, md_alg ) == 0 );
+    TEST_ASSERT(
+                mbedtls_ecdsa_sign_det_ext( &grp, &r, &s, &d, hash, hlen,
+                                            md_alg, rnd_std_rand, NULL )
+                == 0 );
 
     TEST_ASSERT( mbedtls_mpi_cmp_mpi( &r, &r_check ) == 0 );
     TEST_ASSERT( mbedtls_mpi_cmp_mpi( &s, &s_check ) == 0 );