From 23e416261c5dd1e7cb43db5c8c6766af56509d3e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?=
 <manuel.pegourie-gonnard@arm.com>
Date: Thu, 18 May 2017 12:35:37 +0200
Subject: [PATCH] ECDH: not restartable unless explicitly enabled

This is mainly for the benefit of SSL modules, which only supports restart in
a limited number of cases. In the other cases (ECDHE_PSK) it would currently
return ERR_ECP_IN_PROGRESS and the user would thus call ssl_handshake() again,
but the SSL code wouldn't handle state properly and things would go wrong in
possibly unexpected ways.  This is undesirable, so it should be possible for
the SSL module to choose if ECDHE should behave the old or the new way.

Not that it also brings ECDHE more in line with the other modules which
already have that choice available (by passing a NULL or valid restart
context).
---
 include/mbedtls/ecdh.h                | 17 ++++++++++++++
 library/ecdh.c                        | 19 +++++++++++++---
 library/ssl_cli.c                     |  3 +++
 tests/suites/test_suite_ecdh.data     | 32 ++++++++++++++++++++-------
 tests/suites/test_suite_ecdh.function |  9 +++++++-
 5 files changed, 68 insertions(+), 12 deletions(-)

diff --git a/include/mbedtls/ecdh.h b/include/mbedtls/ecdh.h
index 2e344a8c95..6f3fe137c9 100644
--- a/include/mbedtls/ecdh.h
+++ b/include/mbedtls/ecdh.h
@@ -53,6 +53,7 @@ typedef struct
     mbedtls_ecp_point Vf;       /*!<  un-blinding value (for later)         */
     mbedtls_mpi _d;             /*!<  previous d (for later)                */
 #if defined(MBEDTLS_ECP_RESTARTABLE)
+    int restart_enabled;        /*!<  enable restartalbe EC computations?   */
     mbedtls_ecp_restart_ctx rs; /*!<  restart context for EC computations   */
 #endif
 }
@@ -220,6 +221,22 @@ int mbedtls_ecdh_calc_secret( mbedtls_ecdh_context *ctx, size_t *olen,
                       int (*f_rng)(void *, unsigned char *, size_t),
                       void *p_rng );
 
+#if defined(MBEDTLS_ECP_RESTARTABLE)
+/**
+ * \brief           Enable restartable EC computations for this context.
+ *                  (Default: disabled.)
+ *
+ * \sa              \c mbedtls_ecp_set_max_ops()
+ *
+ * \note            It is not possible to safely disable restartable
+ *                  computations once enabled, except by free-ing the context,
+ *                  which cancels possible in-progress operations.
+ *
+ * \param ctx       ECDH context
+ */
+void mbedtls_ecdh_enable_restart( mbedtls_ecdh_context *ctx );
+#endif /* MBEDTLS_ECP_RESTARTABLE */
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/library/ecdh.c b/library/ecdh.c
index b2859c4b6e..cae3b290f4 100644
--- a/library/ecdh.c
+++ b/library/ecdh.c
@@ -155,6 +155,16 @@ void mbedtls_ecdh_free( mbedtls_ecdh_context *ctx )
 #endif
 }
 
+#if defined(MBEDTLS_ECP_RESTARTABLE)
+/*
+ * Enable restartable operations for context
+ */
+void mbedtls_ecdh_enable_restart( mbedtls_ecdh_context *ctx )
+{
+    ctx->restart_enabled = 1;
+}
+#endif
+
 /*
  * Setup and write the ServerKeyExhange parameters (RFC 4492)
  *      struct {
@@ -175,7 +185,8 @@ int mbedtls_ecdh_make_params( mbedtls_ecdh_context *ctx, size_t *olen,
         return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA );
 
 #if defined(MBEDTLS_ECP_RESTARTABLE)
-    rs_ctx = &ctx->rs;
+    if( ctx->restart_enabled )
+        rs_ctx = &ctx->rs;
 #endif
 
     if( ( ret = ecdh_gen_public_restartable( &ctx->grp, &ctx->d, &ctx->Q,
@@ -260,7 +271,8 @@ int mbedtls_ecdh_make_public( mbedtls_ecdh_context *ctx, size_t *olen,
         return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA );
 
 #if defined(MBEDTLS_ECP_RESTARTABLE)
-    rs_ctx = &ctx->rs;
+    if( ctx->restart_enabled )
+        rs_ctx = &ctx->rs;
 #endif
 
     if( ( ret = ecdh_gen_public_restartable( &ctx->grp, &ctx->d, &ctx->Q,
@@ -307,7 +319,8 @@ int mbedtls_ecdh_calc_secret( mbedtls_ecdh_context *ctx, size_t *olen,
         return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA );
 
 #if defined(MBEDTLS_ECP_RESTARTABLE)
-    rs_ctx = &ctx->rs;
+    if( ctx->restart_enabled )
+        rs_ctx = &ctx->rs;
 #endif
 
     if( ( ret = ecdh_compute_shared_restartable( &ctx->grp,
diff --git a/library/ssl_cli.c b/library/ssl_cli.c
index faaedb7f3c..cbd46475cd 100644
--- a/library/ssl_cli.c
+++ b/library/ssl_cli.c
@@ -2902,6 +2902,9 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl )
         i = 4;
 
 #if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
+        if( ssl->handshake->ec_restart_enabled)
+            mbedtls_ecdh_enable_restart( &ssl->handshake->ecdh_ctx );
+
         if( ssl->handshake->ecrs_state == ssl_ecrs_ecdh_public_done )
             goto ecdh_calc_secret;
 #endif
diff --git a/tests/suites/test_suite_ecdh.data b/tests/suites/test_suite_ecdh.data
index 991d11388d..da30633ac8 100644
--- a/tests/suites/test_suite_ecdh.data
+++ b/tests/suites/test_suite_ecdh.data
@@ -38,18 +38,34 @@ ECDH exchange #2
 depends_on:MBEDTLS_ECP_DP_SECP521R1_ENABLED
 ecdh_exchange:MBEDTLS_ECP_DP_SECP521R1
 
-ECDH restartable rfc 5903 p256 restart disabled
+ECDH restartable rfc 5903 p256 restart enabled max_ops=0
 depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
-ecdh_restart:MBEDTLS_ECP_DP_SECP256R1:"C88F01F510D9AC3F70A292DAA2316DE544E9AAB8AFE84049C62A9C57862D1433":"C6EF9C5D78AE012A011164ACB397CE2088685D8F06BF9BE0B283AB46476BEE53":"D6840F6B42F6EDAFD13116E0E12565202FEF8E9ECE7DCE03812464D04B9442DE":0:0:0
+ecdh_restart:MBEDTLS_ECP_DP_SECP256R1:"C88F01F510D9AC3F70A292DAA2316DE544E9AAB8AFE84049C62A9C57862D1433":"C6EF9C5D78AE012A011164ACB397CE2088685D8F06BF9BE0B283AB46476BEE53":"D6840F6B42F6EDAFD13116E0E12565202FEF8E9ECE7DCE03812464D04B9442DE":1:0:0:0
 
-ECDH restartable rfc 5903 p256 restart max_ops=1
+ECDH restartable rfc 5903 p256 restart enabled max_ops=1
 depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
-ecdh_restart:MBEDTLS_ECP_DP_SECP256R1:"C88F01F510D9AC3F70A292DAA2316DE544E9AAB8AFE84049C62A9C57862D1433":"C6EF9C5D78AE012A011164ACB397CE2088685D8F06BF9BE0B283AB46476BEE53":"D6840F6B42F6EDAFD13116E0E12565202FEF8E9ECE7DCE03812464D04B9442DE":1:1:10000
+ecdh_restart:MBEDTLS_ECP_DP_SECP256R1:"C88F01F510D9AC3F70A292DAA2316DE544E9AAB8AFE84049C62A9C57862D1433":"C6EF9C5D78AE012A011164ACB397CE2088685D8F06BF9BE0B283AB46476BEE53":"D6840F6B42F6EDAFD13116E0E12565202FEF8E9ECE7DCE03812464D04B9442DE":1:1:1:10000
 
-ECDH restartable rfc 5903 p256 restart max_ops=10000
+ECDH restartable rfc 5903 p256 restart enabled max_ops=10000
 depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
-ecdh_restart:MBEDTLS_ECP_DP_SECP256R1:"C88F01F510D9AC3F70A292DAA2316DE544E9AAB8AFE84049C62A9C57862D1433":"C6EF9C5D78AE012A011164ACB397CE2088685D8F06BF9BE0B283AB46476BEE53":"D6840F6B42F6EDAFD13116E0E12565202FEF8E9ECE7DCE03812464D04B9442DE":10000:0:0
+ecdh_restart:MBEDTLS_ECP_DP_SECP256R1:"C88F01F510D9AC3F70A292DAA2316DE544E9AAB8AFE84049C62A9C57862D1433":"C6EF9C5D78AE012A011164ACB397CE2088685D8F06BF9BE0B283AB46476BEE53":"D6840F6B42F6EDAFD13116E0E12565202FEF8E9ECE7DCE03812464D04B9442DE":1:10000:0:0
 
-ECDH restartable rfc 5903 p256 restart max_ops=250
+ECDH restartable rfc 5903 p256 restart enabled max_ops=250
 depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
-ecdh_restart:MBEDTLS_ECP_DP_SECP256R1:"C88F01F510D9AC3F70A292DAA2316DE544E9AAB8AFE84049C62A9C57862D1433":"C6EF9C5D78AE012A011164ACB397CE2088685D8F06BF9BE0B283AB46476BEE53":"D6840F6B42F6EDAFD13116E0E12565202FEF8E9ECE7DCE03812464D04B9442DE":250:2:32
+ecdh_restart:MBEDTLS_ECP_DP_SECP256R1:"C88F01F510D9AC3F70A292DAA2316DE544E9AAB8AFE84049C62A9C57862D1433":"C6EF9C5D78AE012A011164ACB397CE2088685D8F06BF9BE0B283AB46476BEE53":"D6840F6B42F6EDAFD13116E0E12565202FEF8E9ECE7DCE03812464D04B9442DE":1:250:2:32
+
+ECDH restartable rfc 5903 p256 restart disabled max_ops=0
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecdh_restart:MBEDTLS_ECP_DP_SECP256R1:"C88F01F510D9AC3F70A292DAA2316DE544E9AAB8AFE84049C62A9C57862D1433":"C6EF9C5D78AE012A011164ACB397CE2088685D8F06BF9BE0B283AB46476BEE53":"D6840F6B42F6EDAFD13116E0E12565202FEF8E9ECE7DCE03812464D04B9442DE":0:0:0:0
+
+ECDH restartable rfc 5903 p256 restart disabled max_ops=1
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecdh_restart:MBEDTLS_ECP_DP_SECP256R1:"C88F01F510D9AC3F70A292DAA2316DE544E9AAB8AFE84049C62A9C57862D1433":"C6EF9C5D78AE012A011164ACB397CE2088685D8F06BF9BE0B283AB46476BEE53":"D6840F6B42F6EDAFD13116E0E12565202FEF8E9ECE7DCE03812464D04B9442DE":0:1:0:0
+
+ECDH restartable rfc 5903 p256 restart disabled max_ops=10000
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecdh_restart:MBEDTLS_ECP_DP_SECP256R1:"C88F01F510D9AC3F70A292DAA2316DE544E9AAB8AFE84049C62A9C57862D1433":"C6EF9C5D78AE012A011164ACB397CE2088685D8F06BF9BE0B283AB46476BEE53":"D6840F6B42F6EDAFD13116E0E12565202FEF8E9ECE7DCE03812464D04B9442DE":0:10000:0:0
+
+ECDH restartable rfc 5903 p256 restart disabled max_ops=250
+depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+ecdh_restart:MBEDTLS_ECP_DP_SECP256R1:"C88F01F510D9AC3F70A292DAA2316DE544E9AAB8AFE84049C62A9C57862D1433":"C6EF9C5D78AE012A011164ACB397CE2088685D8F06BF9BE0B283AB46476BEE53":"D6840F6B42F6EDAFD13116E0E12565202FEF8E9ECE7DCE03812464D04B9442DE":0:250:0:0
diff --git a/tests/suites/test_suite_ecdh.function b/tests/suites/test_suite_ecdh.function
index 911464ad08..05e61e4ebb 100644
--- a/tests/suites/test_suite_ecdh.function
+++ b/tests/suites/test_suite_ecdh.function
@@ -161,7 +161,7 @@ exit:
 
 /* BEGIN_CASE depends_on:MBEDTLS_ECP_RESTARTABLE */
 void ecdh_restart( int id, char *dA_str, char *dB_str, char *z_str,
-                   int max_ops, int min_restart, int max_restart )
+                   int enable, int max_ops, int min_restart, int max_restart )
 {
     int ret;
     mbedtls_ecdh_context srv, cli;
@@ -192,8 +192,15 @@ void ecdh_restart( int id, char *dA_str, char *dB_str, char *z_str,
      * as in ecdh_primitive_test_vec */
     TEST_ASSERT( srv.grp.nbits % 8 == 0 );
 
+    /* set up restart parameters */
     mbedtls_ecp_set_max_ops( max_ops );
 
+    if( enable)
+    {
+        mbedtls_ecdh_enable_restart( &srv );
+        mbedtls_ecdh_enable_restart( &cli );
+    }
+
     /* server writes its paramaters */
     memset( buf, 0x00, sizeof( buf ) );
     len = 0;