1 files changed, 73 insertions, 37 deletions

diff --git a/src/lib/libssl/tls13_client.c b/src/lib/libssl/tls13_client.c
index 0da08f62c3..dffabf1753 100644
--- a/src/lib/libssl/tls13_client.c
+++ b/src/lib/libssl/tls13_client.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls13_client.c,v 1.50 2020/04/21 16:55:17 jsing Exp $ */
+/* $OpenBSD: tls13_client.c,v 1.51 2020/04/22 17:05:07 jsing Exp $ */
 /*
  * Copyright (c) 2018, 2019 Joel Sing <jsing@openbsd.org>
  *
@@ -281,6 +281,24 @@ tls13_server_hello_is_legacy(CBS *cbs)
 }
 
 static int
+tls13_server_hello_is_retry(CBS *cbs)
+{
+        CBS server_hello, server_random;
+        uint16_t legacy_version;
+
+        CBS_dup(cbs, &server_hello);
+
+        if (!CBS_get_u16(&server_hello, &legacy_version))
+                return 0;
+        if (!CBS_get_bytes(&server_hello, &server_random, SSL3_RANDOM_SIZE))
+                return 0;
+
+        /* See if this is a HelloRetryRequest. */
+        return CBS_mem_equal(&server_random, tls13_hello_retry_request_hash,
+            sizeof(tls13_hello_retry_request_hash));
+}
+
+static int
 tls13_server_hello_process(struct tls13_ctx *ctx, CBS *cbs)
 {
         CBS server_random, session_id;
@@ -331,7 +349,8 @@ tls13_server_hello_process(struct tls13_ctx *ctx, CBS *cbs)
         /* From here on in we know we are doing TLSv1.3. */
         tls13_record_layer_allow_legacy_alerts(ctx->rl, 0);
 
-        /* See if this is a Hello Retry Request. */
+        /* See if this is a HelloRetryRequest. */
+        /* XXX - see if we can avoid doing this twice. */
         if (CBS_mem_equal(&server_random, tls13_hello_retry_request_hash,
             sizeof(tls13_hello_retry_request_hash))) {
                 tlsext_msg_type = SSL_TLSEXT_MSG_HRR;
@@ -515,40 +534,75 @@ tls13_client_engage_record_protection(struct tls13_ctx *ctx)
 }
 
 int
+tls13_server_hello_retry_request_recv(struct tls13_ctx *ctx, CBS *cbs)
+{
+        /*
+         * The state machine has no way of knowing if we're going to receive a
+         * HelloRetryRequest or a ServerHello. As such, we have to handle
+         * this case here and hand off to the appropriate function.
+         */
+        if (!tls13_server_hello_is_retry(cbs)) {
+                ctx->handshake_stage.hs_type |= WITHOUT_HRR;
+                return tls13_server_hello_recv(ctx, cbs);
+        }
+
+        if (!tls13_server_hello_process(ctx, cbs))
+                return 0;
+
+        /*
+         * This may have been a TLSv1.2 or earlier ServerHello that just happened
+         * to have matching server random...
+         */
+        if (ctx->hs->use_legacy)
+                return tls13_use_legacy_client(ctx);
+
+        if (!ctx->hs->hrr)
+                return 0;
+
+        if (!tls13_client_synthetic_handshake_message(ctx))
+                return 0;
+        if (!tls13_handshake_msg_record(ctx))
+                return 0;
+
+        ctx->hs->hrr = 0;
+
+        return 1;
+}
+
+int
 tls13_server_hello_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
         SSL *s = ctx->ssl;
 
         /*
-         * We may have received a legacy (pre-TLSv1.3) server hello,
-         * a TLSv1.3 server hello or a TLSv1.3 hello retry request.
+         * We may have received a legacy (pre-TLSv1.3) ServerHello or a TLSv1.3
+         * ServerHello. HelloRetryRequests have already been handled.
          */
         if (!tls13_server_hello_process(ctx, cbs))
                 return 0;
 
-        tls1_transcript_unfreeze(s);
-
-        if (ctx->hs->hrr) {
-                if (!tls13_client_synthetic_handshake_message(ctx))
+        if (ctx->handshake_stage.hs_type & WITHOUT_HRR) {
+                tls1_transcript_unfreeze(s);
+                if (!tls13_handshake_msg_record(ctx))
                         return 0;
         }
 
-        if (!tls13_handshake_msg_record(ctx))
-                return 0;
-
-        if (ctx->hs->use_legacy)
+        if (ctx->hs->use_legacy) {
+                if (!(ctx->handshake_stage.hs_type & WITHOUT_HRR))
+                        return 0;
                 return tls13_use_legacy_client(ctx);
+        }
 
-        if (!ctx->hs->hrr) {
-                if (!tls13_client_engage_record_protection(ctx))
-                        return 0;
+        if (ctx->hs->hrr) {
+                /* The server has sent two HelloRetryRequests. */
+                ctx->alert = SSL_AD_ILLEGAL_PARAMETER;
+                return 0;
         }
 
-        ctx->handshake_stage.hs_type |= NEGOTIATED;
-        if (ctx->hs->hrr)
-                ctx->handshake_stage.hs_type |= WITH_HRR;
+        if (!tls13_client_engage_record_protection(ctx))
+                return 0;
 
-        ctx->hs->hrr = 0;
+        ctx->handshake_stage.hs_type |= NEGOTIATED;
 
         return 1;
 }
@@ -581,24 +635,6 @@ tls13_client_hello_retry_send(struct tls13_ctx *ctx, CBB *cbb)
 }
 
 int
-tls13_server_hello_retry_recv(struct tls13_ctx *ctx, CBS *cbs)
-{
-        if (!tls13_server_hello_process(ctx, cbs))
-                return 0;
-
-        if (ctx->hs->use_legacy)
-                return 0; /* XXX alert */
-
-        if (ctx->hs->hrr)
-                return 0; /* XXX alert */
-
-        if (!tls13_client_engage_record_protection(ctx))
-                return 0;
-
-        return 1;
-}
-
-int
 tls13_server_encrypted_extensions_recv(struct tls13_ctx *ctx, CBS *cbs)
 {
         int alert_desc;