Implement support for resumption with a HelloRetryRequest

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2341)

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index fd050f0d2ebfc12ec4ef7c8d0e15d9a0ef64f1d3..563eb576997217b70de387a95a842c694d52306f 100644 (file)
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -968,29 +968,103 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al)
 
     /*
      * If
+     *     we are a client
+     *     AND
      *     we have no key_share
      *     AND
      *     (we are not resuming
      *      OR the kex_mode doesn't allow non key_share resumes)
      * THEN
-     *     fail
+     *     fail;
      */
-    if (((s->server && s->s3->peer_tmp == NULL) || (!s->server && !sent))
+    if (!s->server
+            && !sent
             && (!s->hit
                 || (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE) == 0)) {
-        /* No suitable share */
-        if (s->server && s->hello_retry_request == 0 && sent) {
-            s->hello_retry_request = 1;
-            return 1;
-        }
-
         /* Nothing left we can do - just fail */
         *al = SSL_AD_HANDSHAKE_FAILURE;
         SSLerr(SSL_F_FINAL_KEY_SHARE, SSL_R_NO_SUITABLE_KEY_SHARE);
         return 0;
     }
+    /*
+     * If
+     *     we are a server
+     *     AND
+     *     we have no key_share
+     * THEN
+     *     If
+     *         we didn't already send a HelloRetryRequest
+     *         AND
+     *         the client sent a key_share extension
+     *         AND
+     *         (we are not resuming
+     *          OR the kex_mode allows key_share resumes)
+     *         AND
+     *         a shared group exists
+     *     THEN
+     *         send a HelloRetryRequest
+     *     ELSE If
+     *         we are not resuming
+     *         OR
+     *         the kex_mode doesn't allow non key_share resumes
+     *     THEN
+     *         fail;
+     */
+    if (s->server && s->s3->peer_tmp == NULL) {
+        /* No suitable share */
+        if (s->hello_retry_request == 0 && sent
+                && (!s->hit
+                    || (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE)
+                       != 0)) {
+            const unsigned char *pcurves, *pcurvestmp, *clntcurves;
+            size_t num_curves, clnt_num_curves, i;
+            unsigned int group_id;
+
+            /* Check a shared group exists */
+
+            /* Get the clients list of supported groups. */
+            if (!tls1_get_curvelist(s, 1, &clntcurves, &clnt_num_curves)) {
+                *al = SSL_AD_INTERNAL_ERROR;
+                SSLerr(SSL_F_FINAL_KEY_SHARE, ERR_R_INTERNAL_ERROR);
+                return 0;
+            }
+
+            /* Get our list of available groups */
+            if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves)) {
+                *al = SSL_AD_INTERNAL_ERROR;
+                SSLerr(SSL_F_FINAL_KEY_SHARE, ERR_R_INTERNAL_ERROR);
+                return 0;
+            }
+
+            /* Find the first group we allow that is also in client's list */
+            for (i = 0, pcurvestmp = pcurves; i < num_curves;
+                 i++, pcurvestmp += 2) {
+                group_id = pcurvestmp[0] << 8 | pcurvestmp[1];
+
+                if (check_in_list(s, group_id, clntcurves, clnt_num_curves, 1))
+                    break;
+            }
+
+            if (i < num_curves) {
+                /* A shared group exists so send a HelloRetryRequest */
+                s->s3->group_id = group_id;
+                s->hello_retry_request = 1;
+                return 1;
+            }
+        }
+        if (!s->hit
+                || (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE) == 0) {
+            /* Nothing left we can do - just fail */
+            *al = SSL_AD_HANDSHAKE_FAILURE;
+            SSLerr(SSL_F_FINAL_KEY_SHARE, SSL_R_NO_SUITABLE_KEY_SHARE);
+            return 0;
+        }
+    }
+
+    /* We have a key_share so don't send any more HelloRetryRequest messages */
+    if (s->server)
+        s->hello_retry_request = 0;
 
-    s->hello_retry_request = 0;
     /*
      * For a client side resumption with no key_share we need to generate
      * the handshake secret (otherwise this is done during key_share
@@ -1059,13 +1133,45 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
         goto err;
     }
 
+    if (EVP_DigestInit_ex(mctx, md, NULL) <= 0) {
+        SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
     /*
-     * Get a hash of the ClientHello up to the start of the binders.
-     * TODO(TLS1.3): This will need to be tweaked when we implement
-     * HelloRetryRequest to include the digest of the previous messages here.
+     * Get a hash of the ClientHello up to the start of the binders. If we are
+     * following a HelloRetryRequest then this includes the hash of the first
+     * ClientHello and the HelloRetryRequest itself.
      */
-    if (EVP_DigestInit_ex(mctx, md, NULL) <= 0
-            || EVP_DigestUpdate(mctx, msgstart, binderoffset) <= 0
+    if (s->hello_retry_request) {
+        size_t hdatalen;
+        void *hdata;
+
+        hdatalen = BIO_get_mem_data(s->s3->handshake_buffer, &hdata);
+        if (hdatalen <= 0) {
+            SSLerr(SSL_F_TLS_PSK_DO_BINDER, SSL_R_BAD_HANDSHAKE_LENGTH);
+            goto err;
+        }
+
+        /*
+         * For servers the handshake buffer data will include the second
+         * ClientHello - which we don't want - so we need to take that bit off.
+         */
+        if (s->server) {
+            if (hdatalen < s->init_num + SSL3_HM_HEADER_LENGTH) {
+                SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR);
+                goto err;
+            }
+            hdatalen -= s->init_num + SSL3_HM_HEADER_LENGTH;
+        }
+
+        if (EVP_DigestUpdate(mctx, hdata, hdatalen) <= 0) {
+            SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR);
+            goto err;
+        }
+    }
+
+    if (EVP_DigestUpdate(mctx, msgstart, binderoffset) <= 0
             || EVP_DigestFinal_ex(mctx, hash, NULL) <= 0) {
         SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR);
         goto err;

diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index f7b799030dab83f822f8bdebd85caf48de2e1d46..b555d68d8d6607426078bcdb20a73adeec90bef5 100644 (file)
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -456,37 +456,6 @@ int tls_parse_ctos_etm(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
     return 1;
 }
 
-/*
- * Checks a list of |groups| to determine if the |group_id| is in it. If it is
- * and |checkallow| is 1 then additionally check if the group is allowed to be
- * used. Returns 1 if the group is in the list (and allowed if |checkallow| is
- * 1) or 0 otherwise.
- */
-#ifndef OPENSSL_NO_TLS1_3
-static int check_in_list(SSL *s, unsigned int group_id,
-                         const unsigned char *groups, size_t num_groups,
-                         int checkallow)
-{
-    size_t i;
-
-    if (groups == NULL || num_groups == 0)
-        return 0;
-
-    for (i = 0; i < num_groups; i++, groups += 2) {
-        unsigned int share_id = (groups[0] << 8) | (groups[1]);
-
-        if (group_id == share_id
-                && (!checkallow
-                    || tls_curve_allowed(s, groups, SSL_SECOP_CURVE_CHECK))) {
-            break;
-        }
-    }
-
-    /* If i == num_groups then not in the list */
-    return i < num_groups;
-}
-#endif
-
 /*
  * Process a psk_kex_modes extension received in the ClientHello. |pkt| contains
  * the raw PACKET data for the extension. Returns 1 on success or 0 on failure.
@@ -1034,54 +1003,10 @@ int tls_construct_stoc_key_share(SSL *s, WPACKET *pkt, unsigned int context,
     if (ckey == NULL) {
         /* No key_share received from client */
         if (s->hello_retry_request) {
-            const unsigned char *pcurves, *pcurvestmp, *clntcurves;
-            size_t num_curves, clnt_num_curves, i;
-
-            /* Get the clients list of supported groups. */
-            if (!tls1_get_curvelist(s, 1, &clntcurves, &clnt_num_curves)) {
-                *al = SSL_AD_INTERNAL_ERROR;
-                SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE,
-                       ERR_R_INTERNAL_ERROR);
-                return 0;
-            }
-
-            /* Get our list of available groups */
-            if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves)) {
-                SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE,
-                       ERR_R_INTERNAL_ERROR);
-                return 0;
-            }
-
             if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_key_share)
-                    || !WPACKET_start_sub_packet_u16(pkt)) {
-                SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE,
-                       ERR_R_INTERNAL_ERROR);
-                return 0;
-            }
-
-            /* Find first group we allow that is also in client's list */
-            for (i = 0, pcurvestmp = pcurves; i < num_curves;
-                 i++, pcurvestmp += 2) {
-                unsigned int group_id = pcurvestmp[0] << 8 | pcurvestmp[1];
-
-                if (check_in_list(s, group_id, clntcurves, clnt_num_curves,
-                                  1)) {
-                    if (!WPACKET_put_bytes_u16(pkt, group_id)) {
-                        SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE,
-                               ERR_R_INTERNAL_ERROR);
-                        return 0;
-                    }
-                    break;
-                }
-            }
-            if (i == num_curves) {
-                /* No common groups */
-                *al = SSL_AD_HANDSHAKE_FAILURE;
-                SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE,
-                       SSL_R_NO_SHARED_GROUPS);
-                return 0;
-            }
-            if (!WPACKET_close(pkt)) {
+                    || !WPACKET_start_sub_packet_u16(pkt)
+                    || !WPACKET_put_bytes_u16(pkt, s->s3->group_id)
+                    || !WPACKET_close(pkt)) {
                 SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE,
                        ERR_R_INTERNAL_ERROR);
                 return 0;

diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index c3dd31a9adf3c72975aac1d32340ee69b14f6d9c..d7564e63cac18d65b37917881882afd651b07217 100644 (file)
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -1738,3 +1738,31 @@ int ssl_set_client_hello_version(SSL *s)
     s->client_version = ver_max;
     return 0;
 }
+
+/*
+ * Checks a list of |groups| to determine if the |group_id| is in it. If it is
+ * and |checkallow| is 1 then additionally check if the group is allowed to be
+ * used. Returns 1 if the group is in the list (and allowed if |checkallow| is
+ * 1) or 0 otherwise.
+ */
+int check_in_list(SSL *s, unsigned int group_id, const unsigned char *groups,
+                  size_t num_groups, int checkallow)
+{
+    size_t i;
+
+    if (groups == NULL || num_groups == 0)
+        return 0;
+
+    for (i = 0; i < num_groups; i++, groups += 2) {
+        unsigned int share_id = (groups[0] << 8) | (groups[1]);
+
+        if (group_id == share_id
+                && (!checkallow
+                    || tls_curve_allowed(s, groups, SSL_SECOP_CURVE_CHECK))) {
+            break;
+        }
+    }
+
+    /* If i == num_groups then not in the list */
+    return i < num_groups;
+}

diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index 429d5c703f163d5bac8cd08ba1896eae8fcc25d7..fa13a26126a7da877dcb0cf7f264cdb26303718d 100644 (file)
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -69,6 +69,9 @@ int statem_flush(SSL *s);
 
 typedef int (*confunc_f) (SSL *s, WPACKET *pkt);
 
+int check_in_list(SSL *s, unsigned int group_id, const unsigned char *groups,
+                  size_t num_groups, int checkallow);
+
 /*
  * TLS/DTLS client state machine functions
  */

diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 8aba6697f35343744952f4caca82d80c06c0e5a3..66743c37ad1224ed774e222bd1522abca245627c 100644 (file)
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1842,13 +1842,6 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)
             s->s3->tmp.new_cipher = s->session->cipher;
         }
 
-        if (!(s->verify_mode & SSL_VERIFY_PEER)) {
-            if (!ssl3_digest_cached_records(s, 0)) {
-                al = SSL_AD_INTERNAL_ERROR;
-                goto f_err;
-            }
-        }
-
         /*-
          * we now have the following setup.
          * client_random
@@ -1975,6 +1968,12 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
         goto err;
     }
 
+    if (!(s->verify_mode & SSL_VERIFY_PEER)
+            && !ssl3_digest_cached_records(s, 0)) {
+        al = SSL_AD_INTERNAL_ERROR;
+        goto err;
+    }
+
     return 1;
  err:
     ssl3_send_alert(s, SSL3_AL_FATAL, al);