From 7ab013a08a729dab0c43dfa860c8218b94ca83eb Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Mon, 8 Jan 2018 17:04:16 +0100
Subject: [PATCH] ssl_write_server_key_exchange refactor: move signature_len
 out

Move the writing of signature_len out of
ssl_prepare_server_key_exchange. This simplifies the control flow (one
less goto).
---
 library/ssl_srv.c | 50 +++++++++++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 23 deletions(-)

diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index 6c2059b62..daf87b96c 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -2826,7 +2826,8 @@ static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl )
 #endif /* MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) ||
           MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */
 
-static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl )
+static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl,
+                                            size_t *signature_len )
 {
     const mbedtls_ssl_ciphersuite_t *ciphersuite_info =
                             ssl->transform_negotiate->ciphersuite_info;
@@ -2839,6 +2840,7 @@ static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl )
 #endif /* MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED */
 #endif /* MBEDTLS_KEY_EXCHANGE__SOME_PFS__ENABLED */
     (void) ciphersuite_info; /* unused in some configurations */
+    (void) signature_len; /* unused in some configurations */
 
     MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write server key exchange" ) );
 
@@ -3018,7 +3020,6 @@ curve_matching_done:
     if( mbedtls_ssl_ciphersuite_uses_server_signature( ciphersuite_info ) )
     {
         size_t dig_signed_len = ssl->out_msg + ssl->out_msglen - dig_signed;
-        size_t signature_len = 0;
         unsigned int hashlen = 0;
         unsigned char hash[MBEDTLS_MD_MAX_SIZE];
         int ret;
@@ -3205,16 +3206,12 @@ curve_matching_done:
                     ssl->conf->p_async_connection_ctx,
                     ssl->handshake->p_async_operation_ctx,
                     ssl->out_msg + ssl->out_msglen + 2,
-                    &signature_len, sig_max_len );
+                    signature_len, sig_max_len );
                 if( ret != MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS )
                 {
                     ssl->handshake->p_async_operation_ctx = NULL;
-                    if( ret != 0 )
-                    {
-                        MBEDTLS_SSL_DEBUG_RET( 1, "f_async_resume", ret );
-                        return( ret );
-                    }
-                    goto have_signature;
+                    MBEDTLS_SSL_DEBUG_RET( 1, "f_async_resume", ret );
+                    return( ret );
                 }
                 /* FALLTHROUGH */
             case MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS:
@@ -3237,25 +3234,13 @@ curve_matching_done:
         if( ( ret = mbedtls_pk_sign( mbedtls_ssl_own_key( ssl ),
                                      md_alg, hash, hashlen,
                                      ssl->out_msg + ssl->out_msglen + 2,
-                                     &signature_len,
+                                     signature_len,
                                      ssl->conf->f_rng,
                                      ssl->conf->p_rng ) ) != 0 )
         {
             MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_pk_sign", ret );
             return( ret );
         }
-
-#if defined(MBEDTLS_SSL_ASYNC_PRIVATE_C)
-    have_signature:
-#endif /* MBEDTLS_SSL_ASYNC_PRIVATE_C */
-        ssl->out_msg[ssl->out_msglen++] = (unsigned char)( signature_len >> 8 );
-        ssl->out_msg[ssl->out_msglen++] = (unsigned char)( signature_len      );
-
-        MBEDTLS_SSL_DEBUG_BUF( 3, "my signature",
-                               ssl->out_msg + ssl->out_msglen,
-                               signature_len );
-
-        ssl->out_msglen += signature_len;
     }
 #endif /* MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED */
 
@@ -3265,6 +3250,7 @@ curve_matching_done:
 static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl )
 {
     int ret;
+    size_t signature_len = 0;
 
     /* Extract static ECDH parameters and abort if ServerKeyExchange
      * is not needed. */
@@ -3292,10 +3278,28 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl )
 #endif /* MBEDTLS_KEY_EXCHANGE__NON_PFS__ENABLED */
 
     /* ServerKeyExchange is needed. Prepare the message. */
-    ret = ssl_prepare_server_key_exchange( ssl );
+    ret = ssl_prepare_server_key_exchange( ssl, &signature_len );
     if( ret != 0 )
         return( ret );
 
+    /* If there is a signature, write its length.
+       ssl_prepare_server_key_exchange already wrote the signature
+       itself at its proper place in the output buffer. */
+#if defined(MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED)
+    if( signature_len != 0 )
+    {
+        ssl->out_msg[ssl->out_msglen++] = (unsigned char)( signature_len >> 8 );
+        ssl->out_msg[ssl->out_msglen++] = (unsigned char)( signature_len      );
+
+        MBEDTLS_SSL_DEBUG_BUF( 3, "my signature",
+                               ssl->out_msg + ssl->out_msglen,
+                               signature_len );
+
+        /* Skip over the already-written signature */
+        ssl->out_msglen += signature_len;
+    }
+#endif /* MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED */
+
     /* Add header and send. */
     ssl->out_msgtype = MBEDTLS_SSL_MSG_HANDSHAKE;
     ssl->out_msg[0]  = MBEDTLS_SSL_HS_SERVER_KEY_EXCHANGE;