Improve TLSv1.3 state machine for HelloRetryRequest handling.

The state machine currently handles the HelloRetryRequest case by using
WITH_HRR - in other words, we're explicitly indicating when we transition
to the alternate path. The problem here is that we do not know if we're
going to receive a ServerHello or a HelloRetryRequest until we process
the message. This means that the ServerHello processing code has to handle
both types of messages.

The state machine and associated processing code becomes cleaner if we flip
this around so that we assume we are going to receive a HelloRetryRequest
and upon discovering that it is not, trigger WITHOUT_HRR and hand off to
the ServerHello processing function. In particular, this makes the logic
much more straight forward on the server side, when adding support for HRR.

With feedback from tb@

ok tb@

1 files changed, 18 insertions, 16 deletions

diff --git a/src/lib/libssl/tls13_handshake.c b/src/lib/libssl/tls13_handshake.c
index 51585d31ba..86046144de 100644
--- a/src/lib/libssl/tls13_handshake.c
+++ b/src/lib/libssl/tls13_handshake.c
@@ -1,4 +1,4 @@
-/*      $OpenBSD: tls13_handshake.c,v 1.52 2020/03/10 17:15:02 jsing Exp $      */
+/*      $OpenBSD: tls13_handshake.c,v 1.53 2020/04/22 17:05:07 jsing Exp $      */
 /*
  * Copyright (c) 2018-2019 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2019 Joel Sing <jsing@openbsd.org>
@@ -96,11 +96,11 @@ struct tls13_handshake_action state_machine[] = {
                 .sent = tls13_server_hello_sent,
                 .recv = tls13_server_hello_recv,
         },
-        [SERVER_HELLO_RETRY] = {
+        [SERVER_HELLO_RETRY_REQUEST] = {
                 .handshake_type = TLS13_MT_SERVER_HELLO,
                 .sender = TLS13_HS_SERVER,
-                .send = tls13_server_hello_retry_send,
-                .recv = tls13_server_hello_retry_recv,
+                .send = tls13_server_hello_retry_request_send,
+                .recv = tls13_server_hello_retry_request_recv,
         },
         [SERVER_ENCRYPTED_EXTENSIONS] = {
                 .handshake_type = TLS13_MT_ENCRYPTED_EXTENSIONS,
@@ -145,10 +145,14 @@ struct tls13_handshake_action state_machine[] = {
 enum tls13_message_type handshakes[][TLS13_NUM_MESSAGE_TYPES] = {
         [INITIAL] = {
                 CLIENT_HELLO,
+                SERVER_HELLO_RETRY_REQUEST,
+                CLIENT_HELLO_RETRY,
                 SERVER_HELLO,
         },
         [NEGOTIATED] = {
                 CLIENT_HELLO,
+                SERVER_HELLO_RETRY_REQUEST,
+                CLIENT_HELLO_RETRY,
                 SERVER_HELLO,
                 SERVER_ENCRYPTED_EXTENSIONS,
                 SERVER_CERTIFICATE_REQUEST,
@@ -159,11 +163,9 @@ enum tls13_message_type handshakes[][TLS13_NUM_MESSAGE_TYPES] = {
                 CLIENT_FINISHED,
                 APPLICATION_DATA,
         },
-        [NEGOTIATED | WITH_HRR] = {
+        [NEGOTIATED | WITHOUT_HRR] = {
                 CLIENT_HELLO,
                 SERVER_HELLO,
-                CLIENT_HELLO_RETRY,
-                SERVER_HELLO_RETRY,
                 SERVER_ENCRYPTED_EXTENSIONS,
                 SERVER_CERTIFICATE_REQUEST,
                 SERVER_CERTIFICATE,
@@ -175,6 +177,8 @@ enum tls13_message_type handshakes[][TLS13_NUM_MESSAGE_TYPES] = {
         },
         [NEGOTIATED | WITHOUT_CR] = {
                 CLIENT_HELLO,
+                SERVER_HELLO_RETRY_REQUEST,
+                CLIENT_HELLO_RETRY,
                 SERVER_HELLO,
                 SERVER_ENCRYPTED_EXTENSIONS,
                 SERVER_CERTIFICATE,
@@ -183,11 +187,9 @@ enum tls13_message_type handshakes[][TLS13_NUM_MESSAGE_TYPES] = {
                 CLIENT_FINISHED,
                 APPLICATION_DATA,
         },
-        [NEGOTIATED | WITH_HRR | WITHOUT_CR] = {
+        [NEGOTIATED | WITHOUT_HRR | WITHOUT_CR] = {
                 CLIENT_HELLO,
                 SERVER_HELLO,
-                CLIENT_HELLO_RETRY,
-                SERVER_HELLO_RETRY,
                 SERVER_ENCRYPTED_EXTENSIONS,
                 SERVER_CERTIFICATE,
                 SERVER_CERTIFICATE_VERIFY,
@@ -197,17 +199,17 @@ enum tls13_message_type handshakes[][TLS13_NUM_MESSAGE_TYPES] = {
         },
         [NEGOTIATED | WITH_PSK] = {
                 CLIENT_HELLO,
+                SERVER_HELLO_RETRY_REQUEST,
+                CLIENT_HELLO_RETRY,
                 SERVER_HELLO,
                 SERVER_ENCRYPTED_EXTENSIONS,
                 SERVER_FINISHED,
                 CLIENT_FINISHED,
                 APPLICATION_DATA,
         },
-        [NEGOTIATED | WITH_HRR | WITH_PSK] = {
+        [NEGOTIATED | WITHOUT_HRR | WITH_PSK] = {
                 CLIENT_HELLO,
                 SERVER_HELLO,
-                CLIENT_HELLO_RETRY,
-                SERVER_HELLO_RETRY,
                 SERVER_ENCRYPTED_EXTENSIONS,
                 SERVER_FINISHED,
                 CLIENT_FINISHED,
@@ -215,6 +217,8 @@ enum tls13_message_type handshakes[][TLS13_NUM_MESSAGE_TYPES] = {
         },
         [NEGOTIATED | WITH_CCV] = {
                 CLIENT_HELLO,
+                SERVER_HELLO_RETRY_REQUEST,
+                CLIENT_HELLO_RETRY,
                 SERVER_HELLO,
                 SERVER_ENCRYPTED_EXTENSIONS,
                 SERVER_CERTIFICATE_REQUEST,
@@ -226,11 +230,9 @@ enum tls13_message_type handshakes[][TLS13_NUM_MESSAGE_TYPES] = {
                 CLIENT_FINISHED,
                 APPLICATION_DATA,
         },
-        [NEGOTIATED | WITH_HRR | WITH_CCV] = {
+        [NEGOTIATED | WITHOUT_HRR | WITH_CCV] = {
                 CLIENT_HELLO,
                 SERVER_HELLO,
-                CLIENT_HELLO_RETRY,
-                SERVER_HELLO_RETRY,
                 SERVER_ENCRYPTED_EXTENSIONS,
                 SERVER_CERTIFICATE_REQUEST,
                 SERVER_CERTIFICATE,