Simplify version checks in the TLSv1.3 client

Ensure that the server announced TLSv1.3 (and nothing higher) in the
supported_versions extension.  In that case, the legacy_version must
be TLSv1.2 according to RFC 8446, 4.1.3 and 4.2.1.

This commit also removes some unreachable code which is a remnant of
very early TLSv1.3 code from before the legacy fallback was introduced.
Simplify a few checks and adjust some comments nearby.

ok jsing

diff --git a/lib/libssl/tls13_client.c b/lib/libssl/tls13_client.c
index 1f51748..38eca61 100644 (file)
--- a/lib/libssl/tls13_client.c
+++ b/lib/libssl/tls13_client.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls13_client.c,v 1.70 2021/01/06 20:15:35 tb Exp $ */
+/* $OpenBSD: tls13_client.c,v 1.71 2021/02/22 16:13:31 tb Exp $ */
 /*
  * Copyright (c) 2018, 2019 Joel Sing <jsing@openbsd.org>
  *
@@ -271,25 +271,14 @@ tls13_server_hello_process(struct tls13_ctx *ctx, CBS *cbs)
        }
 
        /*
-        * See if a supported versions extension was returned. If it was then
-        * the legacy version must be set to 0x0303 (RFC 8446 section 4.1.3).
-        * Otherwise, fallback to the legacy version, ensuring that it is both
-        * within range and not TLS 1.3 or greater (which must use the
-        * supported version extension.
+        * The supported versions extension indicated 0x0304 or greater.
+        * Ensure that it was 0x0304 and that legacy version is set to 0x0303
+        * (RFC 8446 section 4.2.1).
         */
-       if (ctx->hs->server_version != 0) {
-               if (legacy_version != TLS1_2_VERSION) {
-                       ctx->alert = TLS13_ALERT_PROTOCOL_VERSION;
-                       goto err;
-               }
-       } else {
-               if (legacy_version < ctx->hs->min_version ||
-                   legacy_version > ctx->hs->max_version ||
-                   legacy_version > TLS1_2_VERSION) {
-                       ctx->alert = TLS13_ALERT_PROTOCOL_VERSION;
-                       goto err;
-               }
-               ctx->hs->server_version = legacy_version;
+       if (ctx->hs->server_version != TLS1_3_VERSION ||
+           legacy_version != TLS1_2_VERSION) {
+               ctx->alert = TLS13_ALERT_PROTOCOL_VERSION;
+               goto err;
        }
 
        /* The session_id must match. */
@@ -301,15 +290,14 @@ tls13_server_hello_process(struct tls13_ctx *ctx, CBS *cbs)
 
        /*
         * Ensure that the cipher suite is one that we offered in the client
-        * hello and that it matches the TLS version selected.
+        * hello and that it is a TLSv1.3 cipher suite.
         */
        cipher = ssl3_get_cipher_by_value(cipher_suite);
        if (cipher == NULL || !ssl_cipher_in_list(SSL_get_ciphers(s), cipher)) {
                ctx->alert = TLS13_ALERT_ILLEGAL_PARAMETER;
                goto err;
        }
-       if (ctx->hs->server_version == TLS1_3_VERSION &&
-           cipher->algorithm_ssl != SSL_TLSV1_3) {
+       if (cipher->algorithm_ssl != SSL_TLSV1_3) {
                ctx->alert = TLS13_ALERT_ILLEGAL_PARAMETER;
                goto err;
        }