From d0d8cb36a4dd722c38a54e2fd380325074a9b617 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= <mpg2@elzevir.fr>
Date: Thu, 17 Sep 2015 14:16:30 +0200
Subject: [PATCH] Cache ClientHello extension

This extension is quite costly to generate, and we don't want to re-do it
again when the server performs a DTLS HelloVerify. So, cache the result the
first time and re-use if/when we build a new ClientHello.

Note: re-send due to timeouts are different, as the whole message is cached
already, so they don't need any special support.
---
 library/ssl_cli.c | 50 ++++++++++++++++++++++++++++++++++++++++++-----
 tests/ssl-opt.sh  | 12 ++++++++++++
 2 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/library/ssl_cli.c b/library/ssl_cli.c
index 3b7dde076..c0386b697 100644
--- a/library/ssl_cli.c
+++ b/library/ssl_cli.c
@@ -307,12 +307,47 @@ static void ssl_write_ecjpake_kkpp_ext( mbedtls_ssl_context *ssl,
     *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ECJPAKE_KKPP >> 8 ) & 0xFF );
     *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ECJPAKE_KKPP      ) & 0xFF );
 
-    if( ( ret = mbedtls_ecjpake_write_round_one( &ssl->handshake->ecjpake_ctx,
-                    p + 2, end - p - 2, &kkpp_len,
-                    ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 )
+    /*
+     * We may need to send ClientHello multiple times for Hello verification.
+     * We don't want to compute fresh values every time (both for performance
+     * and consistency reasons), so cache the extension content.
+     */
+    if( ssl->handshake->ecjpake_cache == NULL ||
+        ssl->handshake->ecjpake_cache_len == 0 )
     {
-        MBEDTLS_SSL_DEBUG_RET( 1 , "mbedtls_ecjpake_write_round_one", ret );
-        return;
+        MBEDTLS_SSL_DEBUG_MSG( 3, ( "generating new ecjpake parameters" ) );
+
+        if( ( ret = mbedtls_ecjpake_write_round_one( &ssl->handshake->ecjpake_ctx,
+                        p + 2, end - p - 2, &kkpp_len,
+                        ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 )
+        {
+            MBEDTLS_SSL_DEBUG_RET( 1 , "mbedtls_ecjpake_write_round_one", ret );
+            return;
+        }
+
+        ssl->handshake->ecjpake_cache = mbedtls_calloc( 1, kkpp_len );
+        if( ssl->handshake->ecjpake_cache == NULL )
+        {
+            MBEDTLS_SSL_DEBUG_MSG( 1, ( "allocation failed" ) );
+            return;
+        }
+
+        memcpy( ssl->handshake->ecjpake_cache, p + 2, kkpp_len );
+        ssl->handshake->ecjpake_cache_len = kkpp_len;
+    }
+    else
+    {
+        MBEDTLS_SSL_DEBUG_MSG( 3, ( "re-using cached ecjpake parameters" ) );
+
+        kkpp_len = ssl->handshake->ecjpake_cache_len;
+
+        if( (size_t)( end - p - 2 ) < kkpp_len )
+        {
+            MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
+            return;
+        }
+
+        memcpy( p + 2, ssl->handshake->ecjpake_cache, kkpp_len );
     }
 
     *p++ = (unsigned char)( ( kkpp_len >> 8 ) & 0xFF );
@@ -1090,6 +1125,11 @@ static int ssl_parse_ecjpake_kkpp( mbedtls_ssl_context *ssl,
         return( 0 );
     }
 
+    /* If we got here, we no longer need our cached extension */
+    mbedtls_free( ssl->handshake->ecjpake_cache );
+    ssl->handshake->ecjpake_cache = NULL;
+    ssl->handshake->ecjpake_cache_len = 0;
+
     if( ( ret = mbedtls_ecjpake_read_round_one( &ssl->handshake->ecjpake_ctx,
                                                 buf, len ) ) != 0 )
     {
diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
index 5c0e86638..198288d13 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -2535,6 +2535,7 @@ run_test    "ECJPAKE: working, TLS" \
             0 \
             -c "add ciphersuite: c0ff" \
             -c "adding ecjpake_kkpp extension" \
+            -C "re-using cached ecjpake parameters" \
             -s "found ecjpake kkpp extension" \
             -S "skip ecjpake kkpp extension" \
             -S "ciphersuite mismatch: ecjpake not configured" \
@@ -2548,6 +2549,7 @@ run_test    "ECJPAKE: password mismatch, TLS" \
             "$P_CLI debug_level=3 ecjpake_pw=bad \
              force_ciphersuite=TLS-ECJPAKE-WITH-AES-128-CCM-8" \
             1 \
+            -C "re-using cached ecjpake parameters" \
             -s "SSL - Verification of the message MAC failed"
 
 run_test    "ECJPAKE: working, DTLS" \
@@ -2555,6 +2557,15 @@ run_test    "ECJPAKE: working, DTLS" \
             "$P_CLI debug_level=3 dtls=1 ecjpake_pw=bla \
              force_ciphersuite=TLS-ECJPAKE-WITH-AES-128-CCM-8" \
             0 \
+            -c "re-using cached ecjpake parameters" \
+            -S "SSL - Verification of the message MAC failed"
+
+run_test    "ECJPAKE: working, DTLS, no cookie" \
+            "$P_SRV debug_level=3 dtls=1 ecjpake_pw=bla cookies=0" \
+            "$P_CLI debug_level=3 dtls=1 ecjpake_pw=bla \
+             force_ciphersuite=TLS-ECJPAKE-WITH-AES-128-CCM-8" \
+            0 \
+            -C "re-using cached ecjpake parameters" \
             -S "SSL - Verification of the message MAC failed"
 
 run_test    "ECJPAKE: password mismatch, DTLS" \
@@ -2562,6 +2573,7 @@ run_test    "ECJPAKE: password mismatch, DTLS" \
             "$P_CLI debug_level=3 dtls=1 ecjpake_pw=bad \
              force_ciphersuite=TLS-ECJPAKE-WITH-AES-128-CCM-8" \
             1 \
+            -c "re-using cached ecjpake parameters" \
             -s "SSL - Verification of the message MAC failed"
 
 # Tests for ciphersuites per version