From 7b1ec1cfb76dfd71519d4a1482be0355817b06fc Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.org>
Date: Tue, 9 May 2017 08:52:04 +0100
Subject: [PATCH] Fix HRR bug

If an HRR gets sent without a key_share (e.g. cookie only) then the code
fails when it should not.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3414)
---
 ssl/statem/extensions_clnt.c | 42 +++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 898992d99b..3f7fce0da0 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -530,14 +530,26 @@ int tls_construct_ctos_psk_kex_modes(SSL *s, WPACKET *pkt, unsigned int context,
 #ifndef OPENSSL_NO_TLS1_3
 static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id)
 {
-    unsigned char *encoded_point;
-    EVP_PKEY *key_share_key;
+    unsigned char *encoded_point = NULL;
+    EVP_PKEY *key_share_key = NULL;
     size_t encodedlen;
 
-    key_share_key = ssl_generate_pkey_curve(curve_id);
-    if (key_share_key == NULL) {
-        SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_EVP_LIB);
-        return 0;
+    if (s->s3->tmp.pkey != NULL) {
+        assert(s->hello_retry_request);
+        if (!s->hello_retry_request) {
+            SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_INTERNAL_ERROR);
+            return 0;
+        }
+        /*
+         * Could happen if we got an HRR that wasn't requesting a new key_share
+         */
+        key_share_key = s->s3->tmp.pkey;
+    } else {
+        key_share_key = ssl_generate_pkey_curve(curve_id);
+        if (key_share_key == NULL) {
+            SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_EVP_LIB);
+            return 0;
+        }
     }
 
     /* Encode the public key. */
@@ -545,17 +557,14 @@ static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id)
                                                 &encoded_point);
     if (encodedlen == 0) {
         SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_EC_LIB);
-        EVP_PKEY_free(key_share_key);
-        return 0;
+        goto err;
     }
 
     /* Create KeyShareEntry */
     if (!WPACKET_put_bytes_u16(pkt, curve_id)
             || !WPACKET_sub_memcpy_u16(pkt, encoded_point, encodedlen)) {
         SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_INTERNAL_ERROR);
-        EVP_PKEY_free(key_share_key);
-        OPENSSL_free(encoded_point);
-        return 0;
+        goto err;
     }
 
     /*
@@ -568,6 +577,11 @@ static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id)
     OPENSSL_free(encoded_point);
 
     return 1;
+ err:
+    if (s->s3->tmp.pkey == NULL)
+        EVP_PKEY_free(key_share_key);
+    OPENSSL_free(encoded_point);
+    return 0;
 }
 #endif
 
@@ -594,12 +608,6 @@ int tls_construct_ctos_key_share(SSL *s, WPACKET *pkt, unsigned int context,
         return 0;
     }
 
-    if (s->s3->tmp.pkey != NULL) {
-        /* Shouldn't happen! */
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR);
-        return 0;
-    }
-
     /*
      * TODO(TLS1.3): Make the number of key_shares sent configurable. For
      * now, just send one