Update ServerHello to new draft-22 format

The new ServerHello format is essentially now the same as the old TLSv1.2
one, but it must additionally include supported_versions. The version
field is fixed at TLSv1.2, and the version negotiation happens solely via
supported_versions.
Reviewed-by: NBen Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4701)

crypto/err/openssl.txt

@@ -1292,6 +1292,8 @@ SSL_F_TLS_CONSTRUCT_STOC_SESSION_TICKET:460:tls_construct_stoc_session_ticket
 SSL_F_TLS_CONSTRUCT_STOC_STATUS_REQUEST:461:tls_construct_stoc_status_request
 SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS:544:\
 	tls_construct_stoc_supported_groups
+SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_VERSIONS:608:\
+	tls_construct_stoc_supported_versions
 SSL_F_TLS_CONSTRUCT_STOC_USE_SRTP:462:tls_construct_stoc_use_srtp
 SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO:521:\
 	tls_early_post_process_client_hello
@@ -1332,6 +1334,7 @@ SSL_F_TLS_PARSE_STOC_SCT:564:tls_parse_stoc_sct
 SSL_F_TLS_PARSE_STOC_SERVER_NAME:583:tls_parse_stoc_server_name
 SSL_F_TLS_PARSE_STOC_SESSION_TICKET:584:tls_parse_stoc_session_ticket
 SSL_F_TLS_PARSE_STOC_STATUS_REQUEST:585:tls_parse_stoc_status_request
+SSL_F_TLS_PARSE_STOC_SUPPORTED_VERSIONS:609:tls_parse_stoc_supported_versions
 SSL_F_TLS_PARSE_STOC_USE_SRTP:446:tls_parse_stoc_use_srtp
 SSL_F_TLS_POST_PROCESS_CLIENT_HELLO:378:tls_post_process_client_hello
 SSL_F_TLS_POST_PROCESS_CLIENT_KEY_EXCHANGE:384:\

include/openssl/sslerr.h

@@ -340,6 +340,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_TLS_CONSTRUCT_STOC_SESSION_TICKET          460
 # define SSL_F_TLS_CONSTRUCT_STOC_STATUS_REQUEST          461
 # define SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS        544
+# define SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_VERSIONS      608
 # define SSL_F_TLS_CONSTRUCT_STOC_USE_SRTP                462
 # define SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO        521
 # define SSL_F_TLS_FINISH_HANDSHAKE                       597
@@ -379,6 +380,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_TLS_PARSE_STOC_SERVER_NAME                 583
 # define SSL_F_TLS_PARSE_STOC_SESSION_TICKET              584
 # define SSL_F_TLS_PARSE_STOC_STATUS_REQUEST              585
+# define SSL_F_TLS_PARSE_STOC_SUPPORTED_VERSIONS          609
 # define SSL_F_TLS_PARSE_STOC_USE_SRTP                    446
 # define SSL_F_TLS_POST_PROCESS_CLIENT_HELLO              378
 # define SSL_F_TLS_POST_PROCESS_CLIENT_KEY_EXCHANGE       384

ssl/ssl_err.c

@@ -524,6 +524,8 @@ static const ERR_STRING_DATA SSL_str_functs[] = {
      "tls_construct_stoc_status_request"},
     {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS, 0),
      "tls_construct_stoc_supported_groups"},
+    {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_VERSIONS, 0),
+     "tls_construct_stoc_supported_versions"},
     {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_CONSTRUCT_STOC_USE_SRTP, 0),
      "tls_construct_stoc_use_srtp"},
     {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, 0),
@@ -593,6 +595,8 @@ static const ERR_STRING_DATA SSL_str_functs[] = {
      "tls_parse_stoc_session_ticket"},
     {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_PARSE_STOC_STATUS_REQUEST, 0),
      "tls_parse_stoc_status_request"},
+    {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_PARSE_STOC_SUPPORTED_VERSIONS, 0),
+     "tls_parse_stoc_supported_versions"},
     {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_PARSE_STOC_USE_SRTP, 0),
      "tls_parse_stoc_use_srtp"},
     {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_POST_PROCESS_CLIENT_HELLO, 0),

ssl/ssl_locl.h

@@ -2266,7 +2266,8 @@ __owur int ssl_check_version_downgrade(SSL *s);
 __owur int ssl_set_version_bound(int method_version, int version, int *bound);
 __owur int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello,
                                      DOWNGRADE *dgrd);
-__owur int ssl_choose_client_version(SSL *s, int version, int checkdgrd);
+__owur int ssl_choose_client_version(SSL *s, int version,
+                                     RAW_EXTENSION *extensions);
 int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version);
 
 __owur long tls1_default_timeout(void);

ssl/statem/extensions.c

@@ -9,6 +9,7 @@
 
 #include <string.h>
 #include "internal/nelem.h"
+#include "internal/cryptlib.h"
 #include "../ssl_locl.h"
 #include "statem_locl.h"
 
@@ -261,11 +262,13 @@ static const EXTENSION_DEFINITION ext_defs[] = {
     },
     {
         TLSEXT_TYPE_supported_versions,
-        SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS_IMPLEMENTATION_ONLY
-        | SSL_EXT_TLS1_3_ONLY,
+        SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO
+        | SSL_EXT_TLS1_3_SERVER_HELLO | SSL_EXT_TLS_IMPLEMENTATION_ONLY,
         NULL,
         /* Processed inline as part of version selection */
-        NULL, NULL, NULL, tls_construct_ctos_supported_versions, NULL
+        NULL, tls_parse_stoc_supported_versions,
+        tls_construct_stoc_supported_versions,
+        tls_construct_ctos_supported_versions, NULL
     },
     {
         TLSEXT_TYPE_psk_kex_modes,
@@ -357,6 +360,44 @@ static int validate_context(SSL *s, unsigned int extctx, unsigned int thisctx)
     return 1;
 }
 
+int tls_validate_all_contexts(SSL *s, unsigned int thisctx, RAW_EXTENSION *exts)
+{
+    size_t i, num_exts, builtin_num = OSSL_NELEM(ext_defs), offset;
+    RAW_EXTENSION *thisext;
+    unsigned int context;
+    ENDPOINT role = ENDPOINT_BOTH;
+
+    if ((thisctx & SSL_EXT_CLIENT_HELLO) != 0)
+        role = ENDPOINT_SERVER;
+    else if ((thisctx & SSL_EXT_TLS1_2_SERVER_HELLO) != 0)
+        role = ENDPOINT_CLIENT;
+
+    /* Calculate the number of extensions in the extensions list */
+    num_exts = builtin_num + s->cert->custext.meths_count;
+
+    for (thisext = exts, i = 0; i < num_exts; i++, thisext++) {
+        if (!thisext->present)
+            continue;
+
+        if (i < builtin_num) {
+            context = ext_defs[i].context;
+        } else {
+            custom_ext_method *meth = NULL;
+
+            meth = custom_ext_find(&s->cert->custext, role, thisext->type,
+                                   &offset);
+            if (!ossl_assert(meth != NULL))
+                return 0;
+            context = meth->context;
+        }
+
+        if (!validate_context(s, context, thisctx))
+            return 0;
+    }
+
+    return 1;
+}
+
 /*
  * Verify whether we are allowed to use the extension |type| in the current
  * |context|. Returns 1 to indicate the extension is allowed or unknown or 0 to

ssl/statem/extensions_clnt.c

@@ -507,6 +507,20 @@ EXT_RETURN tls_construct_ctos_supported_versions(SSL *s, WPACKET *pkt,
 {
     int currv, min_version, max_version, reason;
 
+    reason = ssl_get_min_max_version(s, &min_version, &max_version);
+    if (reason != 0) {
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                 SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_VERSIONS, reason);
+        return EXT_RETURN_FAIL;
+    }
+
+    /*
+     * Don't include this if we can't negotiate TLSv1.3. We can do a straight
+     * comparison here because we will never be called in DTLS.
+     */
+    if (max_version < TLS1_3_VERSION)
+        return EXT_RETURN_NOT_SENT;
+
     if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_versions)
             || !WPACKET_start_sub_packet_u16(pkt)
             || !WPACKET_start_sub_packet_u8(pkt)) {
@@ -516,13 +530,6 @@ EXT_RETURN tls_construct_ctos_supported_versions(SSL *s, WPACKET *pkt,
         return EXT_RETURN_FAIL;
     }
 
-    reason = ssl_get_min_max_version(s, &min_version, &max_version);
-    if (reason != 0) {
-        SSLfatal(s, SSL_AD_INTERNAL_ERROR,
-                 SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_VERSIONS, reason);
-        return EXT_RETURN_FAIL;
-    }
-
     /*
      * TODO(TLS1.3): There is some discussion on the TLS list as to whether
      * we should include versions <TLS1.2. For the moment we do. To be
@@ -1633,6 +1640,29 @@ int tls_parse_stoc_ems(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
     return 1;
 }
 
+int tls_parse_stoc_supported_versions(SSL *s, PACKET *pkt, unsigned int context,
+                                      X509 *x, size_t chainidx)
+{
+    unsigned int version;
+
+    if (!PACKET_get_net_2(pkt, &version)
+            || PACKET_remaining(pkt) != 0) {
+        SSLfatal(s, SSL_AD_DECODE_ERROR,
+                 SSL_F_TLS_PARSE_STOC_SUPPORTED_VERSIONS,
+                 SSL_R_LENGTH_MISMATCH);
+        return 0;
+    }
+
+    /* TODO(TLS1.3): Remove this before release */
+    if (version == TLS1_3_VERSION_DRAFT)
+        version = TLS1_3_VERSION;
+
+    /* We just set it here. We validate it in ssl_choose_client_version */
+    s->version = version;
+
+    return 1;
+}
+
 int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
                              size_t chainidx)
 {

ssl/statem/extensions_srvr.c

@@ -1213,6 +1213,27 @@ EXT_RETURN tls_construct_stoc_ems(SSL *s, WPACKET *pkt, unsigned int context,
     return EXT_RETURN_SENT;
 }
 
+EXT_RETURN tls_construct_stoc_supported_versions(SSL *s, WPACKET *pkt,
+                                                 unsigned int context, X509 *x,
+                                                 size_t chainidx)
+{
+    if (!SSL_IS_TLS13(s))
+        return EXT_RETURN_NOT_SENT;
+
+    if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_versions)
+            || !WPACKET_start_sub_packet_u16(pkt)
+                /* TODO(TLS1.3): Update to remove the TLSv1.3 draft indicator */
+            || !WPACKET_put_bytes_u16(pkt, TLS1_3_VERSION_DRAFT)
+            || !WPACKET_close(pkt)) {
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                 SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_VERSIONS,
+                 ERR_R_INTERNAL_ERROR);
+        return EXT_RETURN_FAIL;
+    }
+
+    return EXT_RETURN_SENT;
+}
+
 EXT_RETURN tls_construct_stoc_key_share(SSL *s, WPACKET *pkt,
                                         unsigned int context, X509 *x,
                                         size_t chainidx)

ssl/statem/statem_clnt.c

@@ -1332,60 +1332,30 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
         goto err;
     }
 
-    /*
-     * We do this immediately so we know what format the ServerHello is in.
-     * Must be done after reading the random data so we can check for the
-     * TLSv1.3 downgrade sentinels
-     */
-    if (!ssl_choose_client_version(s, sversion, 1)) {
-        /* SSLfatal() already called */
+    /* Get the session-id. */
+    if (!PACKET_get_length_prefixed_1(pkt, &session_id)) {
+        SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
+                 SSL_R_LENGTH_MISMATCH);
         goto err;
     }
-
-    /*
-     * In TLSv1.3 a ServerHello message signals a key change so the end of the
-     * message must be on a record boundary.
-     */
-    if (SSL_IS_TLS13(s) && RECORD_LAYER_processed_read_pending(&s->rlayer)) {
-        SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_TLS_PROCESS_SERVER_HELLO,
-                 SSL_R_NOT_ON_RECORD_BOUNDARY);
+    session_id_len = PACKET_remaining(&session_id);
+    if (session_id_len > sizeof(s->session->session_id)
+        || session_id_len > SSL3_SESSION_ID_SIZE) {
+        SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PROCESS_SERVER_HELLO,
+                 SSL_R_SSL3_SESSION_ID_TOO_LONG);
         goto err;
     }
 
-    /* Get the session-id. */
-    if (!SSL_IS_TLS13(s)) {
-        if (!PACKET_get_length_prefixed_1(pkt, &session_id)) {
-            SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
-                     SSL_R_LENGTH_MISMATCH);
-            goto err;
-        }
-        session_id_len = PACKET_remaining(&session_id);
-        if (session_id_len > sizeof(s->session->session_id)
-            || session_id_len > SSL3_SESSION_ID_SIZE) {
-            SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
-                     SSL_F_TLS_PROCESS_SERVER_HELLO,
-                     SSL_R_SSL3_SESSION_ID_TOO_LONG);
-            goto err;
-        }
-    } else {
-        PACKET_null_init(&session_id);
-        session_id_len = 0;
-    }
-
     if (!PACKET_get_bytes(pkt, &cipherchars, TLS_CIPHER_LEN)) {
         SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
                  SSL_R_LENGTH_MISMATCH);
         goto err;
     }
 
-    if (!SSL_IS_TLS13(s)) {
-        if (!PACKET_get_1(pkt, &compression)) {
-            SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
-                     SSL_R_LENGTH_MISMATCH);
-            goto err;
-        }
-    } else {
-        compression = 0;
+    if (!PACKET_get_1(pkt, &compression)) {
+        SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
+                 SSL_R_LENGTH_MISMATCH);
+        goto err;
     }
 
     /* TLS extensions */
@@ -1398,10 +1368,44 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
         goto err;
     }
 
+    if (!tls_collect_extensions(s, &extpkt,
+                                SSL_EXT_TLS1_2_SERVER_HELLO
+                                | SSL_EXT_TLS1_3_SERVER_HELLO,
+                                &extensions, NULL, 1)) {
+        /* SSLfatal() already called */
+        goto err;
+    }
+
+    if (!ssl_choose_client_version(s, sversion, extensions)) {
+        /* SSLfatal() already called */
+        goto err;
+    }
+
+    /*
+     * Now we have chosen the version we need to check again that the extensions
+     * are appropriate for this version.
+     */
     context = SSL_IS_TLS13(s) ? SSL_EXT_TLS1_3_SERVER_HELLO
                               : SSL_EXT_TLS1_2_SERVER_HELLO;
-    if (!tls_collect_extensions(s, &extpkt, context, &extensions, NULL, 1)) {
-        /* SSLfatal() already called */
+    if (!tls_validate_all_contexts(s, context, extensions)) {
+        SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PROCESS_SERVER_HELLO,
+                 SSL_R_BAD_EXTENSION);
+        goto err;
+    }
+
+    /*
+     * In TLSv1.3 a ServerHello message signals a key change so the end of the
+     * message must be on a record boundary.
+     */
+    if (SSL_IS_TLS13(s) && RECORD_LAYER_processed_read_pending(&s->rlayer)) {
+        SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_TLS_PROCESS_SERVER_HELLO,
+                 SSL_R_NOT_ON_RECORD_BOUNDARY);
+        goto err;
+    }
+
+    if (SSL_IS_TLS13(s) && compression != 0) {
+        SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PROCESS_SERVER_HELLO,
+               SSL_R_INVALID_COMPRESSION_ALGORITHM);
         goto err;
     }
 
@@ -1411,7 +1415,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
         /* This will set s->hit if we are resuming */
         if (!tls_parse_extension(s, TLSEXT_IDX_psk,
                                  SSL_EXT_TLS1_3_SERVER_HELLO,
-                                 extensions, NULL, 0l)) {
+                                 extensions, NULL, 0)) {
             /* SSLfatal() already called */
             goto err;
         }

ssl/statem/statem_lib.c

@@ -1739,21 +1739,31 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd)
  *
  * @s: client SSL handle.
  * @version: The proposed version from the server's HELLO.
- * @checkdgrd: Whether to check the downgrade sentinels in the server_random
+ * @extensions: The extensions received
  *
  * Returns 1 on success or 0 on error.
  */
-int ssl_choose_client_version(SSL *s, int version, int checkdgrd)
+int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions)
 {
     const version_info *vent;
     const version_info *table;
     int highver = 0;
+    int origv;
 
-    /* TODO(TLS1.3): Remove this before release */
-    if (version == TLS1_3_VERSION_DRAFT)
-        version = TLS1_3_VERSION;
+    origv = s->version;
+    s->version = version;
 
-    if (s->hello_retry_request && version != TLS1_3_VERSION) {
+    /* This will overwrite s->version if the extension is present */
+    if (!tls_parse_extension(s, TLSEXT_IDX_supported_versions,
+                             SSL_EXT_TLS1_2_SERVER_HELLO
+                             | SSL_EXT_TLS1_3_SERVER_HELLO, extensions,
+                             NULL, 0)) {
+        s->version = origv;
+        return 0;
+    }
+
+    if (s->hello_retry_request && s->version != TLS1_3_VERSION) {
+        s->version = origv;
         SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_F_SSL_CHOOSE_CLIENT_VERSION,
                  SSL_R_WRONG_SSL_VERSION);
         return 0;
@@ -1761,7 +1771,8 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd)
 
     switch (s->method->version) {
     default:
-        if (version != s->version) {
+        if (s->version != s->method->version) {
+            s->version = origv;
             SSLfatal(s, SSL_AD_PROTOCOL_VERSION,
                      SSL_F_SSL_CHOOSE_CLIENT_VERSION,
                      SSL_R_WRONG_SSL_VERSION);
@@ -1790,13 +1801,14 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd)
         if (vent->cmeth == NULL)
             continue;
 
-        if (highver != 0 && version != vent->version)
+        if (highver != 0 && s->version != vent->version)
             continue;
 
         method = vent->cmeth();
         err = ssl_method_error(s, method);
         if (err != 0) {
-            if (version == vent->version) {
+            if (s->version == vent->version) {
+                s->version = origv;
                 SSLfatal(s, SSL_AD_PROTOCOL_VERSION,
                          SSL_F_SSL_CHOOSE_CLIENT_VERSION, err);
                 return 0;
@@ -1807,43 +1819,43 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd)
         if (highver == 0)
             highver = vent->version;
 
-        if (version != vent->version)
+        if (s->version != vent->version)
             continue;
 
 #ifndef OPENSSL_NO_TLS13DOWNGRADE
         /* Check for downgrades */
-        if (checkdgrd) {
-            if (version == TLS1_2_VERSION && highver > version) {
-                if (memcmp(tls12downgrade,
-                           s->s3->server_random + SSL3_RANDOM_SIZE
-                                                - sizeof(tls12downgrade),
-                           sizeof(tls12downgrade)) == 0) {
-                    SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
-                             SSL_F_SSL_CHOOSE_CLIENT_VERSION,
-                             SSL_R_INAPPROPRIATE_FALLBACK);
-                    return 0;
-                }
-            } else if (!SSL_IS_DTLS(s)
-                       && version < TLS1_2_VERSION
-                       && highver > version) {
-                if (memcmp(tls11downgrade,
-                           s->s3->server_random + SSL3_RANDOM_SIZE
-                                                - sizeof(tls11downgrade),
-                           sizeof(tls11downgrade)) == 0) {
-                    SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
-                             SSL_F_SSL_CHOOSE_CLIENT_VERSION,
-                             SSL_R_INAPPROPRIATE_FALLBACK);
-                    return 0;
-                }
+        if (s->version == TLS1_2_VERSION && highver > s->version) {
+            if (memcmp(tls12downgrade,
+                       s->s3->server_random + SSL3_RANDOM_SIZE
+                                            - sizeof(tls12downgrade),
+                       sizeof(tls12downgrade)) == 0) {
+                s->version = origv;
+                SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
+                         SSL_F_SSL_CHOOSE_CLIENT_VERSION,
+                         SSL_R_INAPPROPRIATE_FALLBACK);
+                return 0;
+            }
+        } else if (!SSL_IS_DTLS(s)
+                   && s->version < TLS1_2_VERSION
+                   && highver > s->version) {
+            if (memcmp(tls11downgrade,
+                       s->s3->server_random + SSL3_RANDOM_SIZE
+                                            - sizeof(tls11downgrade),
+                       sizeof(tls11downgrade)) == 0) {
+                s->version = origv;
+                SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
+                         SSL_F_SSL_CHOOSE_CLIENT_VERSION,
+                         SSL_R_INAPPROPRIATE_FALLBACK);
+                return 0;
             }
         }
 #endif
 
         s->method = method;
-        s->version = version;
         return 1;
     }
 
+    s->version = origv;
     SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_F_SSL_CHOOSE_CLIENT_VERSION,
              SSL_R_UNSUPPORTED_PROTOCOL);
     return 0;

ssl/statem/statem_locl.h

@@ -161,6 +161,8 @@ typedef enum ext_return_en {
     EXT_RETURN_NOT_SENT
 } EXT_RETURN;
 
+__owur int tls_validate_all_contexts(SSL *s, unsigned int thisctx,
+                                     RAW_EXTENSION *exts);
 __owur int extension_is_relevant(SSL *s, unsigned int extctx,
                                  unsigned int thisctx);
 __owur int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
@@ -271,6 +273,9 @@ EXT_RETURN tls_construct_stoc_etm(SSL *s, WPACKET *pkt, unsigned int context,
                                   X509 *x, size_t chainidx);
 EXT_RETURN tls_construct_stoc_ems(SSL *s, WPACKET *pkt, unsigned int context,
                                   X509 *x, size_t chainidx);
+EXT_RETURN tls_construct_stoc_supported_versions(SSL *s, WPACKET *pkt,
+                                                 unsigned int context, X509 *x,
+                                                 size_t chainidx);
 EXT_RETURN tls_construct_stoc_key_share(SSL *s, WPACKET *pkt,
                                         unsigned int context, X509 *x,
                                         size_t chainidx);
@@ -388,6 +393,8 @@ int tls_parse_stoc_etm(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
                        size_t chainidx);
 int tls_parse_stoc_ems(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
                        size_t chainidx);
+int tls_parse_stoc_supported_versions(SSL *s, PACKET *pkt, unsigned int context,
+                                      X509 *x, size_t chainidx);
 int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
                              size_t chainidx);
 int tls_parse_stoc_cookie(SSL *s, PACKET *pkt, unsigned int context, X509 *x,

ssl/statem/statem_srvr.c

@@ -2193,8 +2193,7 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
     size_t sl, len;
     int version;
 
-    /* TODO(TLS1.3): Remove the DRAFT conditional before release */
-    version = SSL_IS_TLS13(s) ? TLS1_3_VERSION_DRAFT : s->version;
+    version = SSL_IS_TLS13(s) ? TLS1_2_VERSION : s->version;
     if (!WPACKET_put_bytes_u16(pkt, version)
                /*
                 * Random stuff. Filling of the server_random takes place in
@@ -2234,21 +2233,23 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
         return 0;
     }
 
+    /* Never send a session_id back in the ServerHello */
+    if (SSL_IS_TLS13(s))
+        sl = 0;
+
     /* set up the compression method */
 #ifdef OPENSSL_NO_COMP
     compm = 0;
 #else
-    if (s->s3->tmp.new_compression == NULL)
+    if (SSL_IS_TLS13(s) || s->s3->tmp.new_compression == NULL)
         compm = 0;
     else
         compm = s->s3->tmp.new_compression->id;
 #endif
 
-    if ((!SSL_IS_TLS13(s)
-                && !WPACKET_sub_memcpy_u8(pkt, s->session->session_id, sl))
+    if (!WPACKET_sub_memcpy_u8(pkt, s->session->session_id, sl)
             || !s->method->put_cipher_by_char(s->s3->tmp.new_cipher, pkt, &len)
-            || (!SSL_IS_TLS13(s)
-                && !WPACKET_put_bytes_u8(pkt, compm))
+            || !WPACKET_put_bytes_u8(pkt, compm)
             || !tls_construct_extensions(s, pkt,
                                          SSL_IS_TLS13(s)
                                             ? SSL_EXT_TLS1_3_SERVER_HELLO

ssl/t1_trce.c

@@ -823,6 +823,17 @@ static int ssl_print_extension(BIO *bio, int indent, int server,
         break;
 
     case TLSEXT_TYPE_supported_versions:
+        if (server) {
+            int version;
+
+            if (extlen != 2)
+                return 0;
+            version = (ext[0] << 8) | ext[1];
+            BIO_indent(bio, indent + 4, 80);
+            BIO_printf(bio, "%s (%d)\n",
+                       ssl_trace_str(version, ssl_version_tbl), version);
+            break;
+        }
         if (extlen < 1)
             return 0;
         xlen = ext[0];

test/asynciotest.c

@@ -146,7 +146,7 @@ static int async_write(BIO *bio, const char *in, int inl)
                 return -1;
 
             while (PACKET_remaining(&pkt) > 0) {
-                PACKET payload, wholebody;
+                PACKET payload, wholebody, sessionid, extensions;
                 unsigned int contenttype, versionhi, versionlo, data;
                 unsigned int msgtype = 0, negversion = 0;
 
@@ -164,11 +164,43 @@ static int async_write(BIO *bio, const char *in, int inl)
                         && !PACKET_get_1(&wholebody, &msgtype))
                     return -1;
 
-                if (msgtype == SSL3_MT_SERVER_HELLO
-                        && (!PACKET_forward(&wholebody,
+                if (msgtype == SSL3_MT_SERVER_HELLO) {
+                    if (!PACKET_forward(&wholebody,
                                             SSL3_HM_HEADER_LENGTH - 1)
-                            || !PACKET_get_net_2(&wholebody, &negversion)))
-                    return -1;
+                            || !PACKET_get_net_2(&wholebody, &negversion)
+                               /* Skip random (32 bytes) */
+                            || !PACKET_forward(&wholebody, 32)
+                               /* Skip session id */
+                            || !PACKET_get_length_prefixed_1(&wholebody,
+                                                             &sessionid)
+                               /*
+                                * Skip ciphersuite (2 bytes) and compression
+                                * method (1 byte)
+                                */
+                            || !PACKET_forward(&wholebody, 2 + 1)
+                            || !PACKET_get_length_prefixed_2(&wholebody,
+                                                             &extensions))
+                        return -1;
+
+                    /*
+                     * Find the negotiated version in supported_versions
+                     * extension, if present.
+                     */
+                    while (PACKET_remaining(&extensions)) {
+                        unsigned int type;
+                        PACKET extbody;
+
+                        if (!PACKET_get_net_2(&extensions, &type)
+                                || !PACKET_get_length_prefixed_2(&extensions,
+                                &extbody))
+                            return -1;
+
+                        if (type == TLSEXT_TYPE_supported_versions
+                                && (!PACKET_get_net_2(&extbody, &negversion)
+                                    || PACKET_remaining(&extbody) != 0))
+                            return -1;
+                    }
+                }
 
                 while (PACKET_get_1(&payload, &data)) {
                     /* Create a new one byte long record for each byte in the

test/recipes/70-test_tls13kexmodes.t

@@ -122,6 +122,8 @@ $ENV{CTLOG_FILE} = srctop_file("test", "ct", "log_list.conf");
     [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_PSK,
         checkhandshake::PSK_CLI_EXTENSION],
 
+    [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_SUPPORTED_VERSIONS,
+        checkhandshake::DEFAULT_EXTENSIONS],
     [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_KEY_SHARE,
         checkhandshake::KEY_SHARE_SRV_EXTENSION],
     [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_PSK,

test/recipes/70-test_tls13messages.t

@@ -122,6 +122,8 @@ $ENV{CTLOG_FILE} = srctop_file("test", "ct", "log_list.conf");
     [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_PSK,
         checkhandshake::PSK_CLI_EXTENSION],
 
+    [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_SUPPORTED_VERSIONS,
+        checkhandshake::DEFAULT_EXTENSIONS],
     [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_KEY_SHARE,
         checkhandshake::DEFAULT_EXTENSIONS],
     [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_PSK,

util/perl/TLSProxy/ServerHello.pm

@@ -46,29 +46,21 @@ sub parse
     my $ptr = 2;
     my ($server_version) = unpack('n', $self->data);
 
-    # TODO(TLS1.3): Replace this reference to draft version before release
-    if ($server_version == TLSProxy::Record::VERS_TLS_1_3_DRAFT) {
-        $server_version = TLSProxy::Record::VERS_TLS_1_3;
-        TLSProxy::Proxy->is_tls13(1);
-    }
-
     my $random = substr($self->data, $ptr, 32);
     $ptr += 32;
     my $session_id_len = 0;
     my $session = "";
-    if (!TLSProxy::Proxy->is_tls13()) {
-        $session_id_len = unpack('C', substr($self->data, $ptr));
-        $ptr++;
-        $session = substr($self->data, $ptr, $session_id_len);
-        $ptr += $session_id_len;
-    }
+    $session_id_len = unpack('C', substr($self->data, $ptr));
+    $ptr++;
+    $session = substr($self->data, $ptr, $session_id_len);
+    $ptr += $session_id_len;
+
     my $ciphersuite = unpack('n', substr($self->data, $ptr));
     $ptr += 2;
     my $comp_meth = 0;
-    if (!TLSProxy::Proxy->is_tls13()) {
-        $comp_meth = unpack('C', substr($self->data, $ptr));
-        $ptr++;
-    }
+    $comp_meth = unpack('C', substr($self->data, $ptr));
+    $ptr++;
+
     my $extensions_len = unpack('n', substr($self->data, $ptr));
     if (!defined $extensions_len) {
         $extensions_len = 0;
@@ -96,6 +88,15 @@ sub parse
         my $extdata = substr($extension_data, 4, $size);
         $extension_data = substr($extension_data, 4 + $size);
         $extensions{$type} = $extdata;
+        if ($type == TLSProxy::Message::EXT_SUPPORTED_VERSIONS) {
+            $server_version = unpack('n', $extdata);
+        }
+    }
+
+    # TODO(TLS1.3): Replace this reference to draft version before release
+    if ($server_version == TLSProxy::Record::VERS_TLS_1_3_DRAFT) {
+        $server_version = TLSProxy::Record::VERS_TLS_1_3;
+        TLSProxy::Proxy->is_tls13(1);
     }
 
     $self->server_version($server_version);
@@ -138,14 +139,10 @@ sub set_message_contents
 
     $data = pack('n', $self->server_version);
     $data .= $self->random;
-    if (!TLSProxy::Proxy->is_tls13()) {
-        $data .= pack('C', $self->session_id_len);
-        $data .= $self->session;
-    }
+    $data .= pack('C', $self->session_id_len);
+    $data .= $self->session;
     $data .= pack('n', $self->ciphersuite);
-    if (!TLSProxy::Proxy->is_tls13()) {
-        $data .= pack('C', $self->comp_meth);
-    }
+    $data .= pack('C', $self->comp_meth);
 
     foreach my $key (keys %{$self->extension_data}) {
         my $extdata = ${$self->extension_data}{$key};