From: Matt Caswell <matt@openssl.org>
Date: Mon, 31 Oct 2016 17:05:20 +0000 (+0000)
Subject: Address some review feedback comments for supported_versions
X-Git-Tag: OpenSSL_1_1_1-pre1~3126
X-Git-Url: https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff_plain;h=16bce0e08b16b28a1953795bde3f913957b08ef2

Address some review feedback comments for supported_versions

Reviewed-by: Rich Salz <rsalz@openssl.org>
---

diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index b8bca0e4dc..004383c413 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -781,11 +781,12 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt)
      * the negotiated version.
      *
      * For TLS 1.3 we always set the ClientHello version to 1.2 and rely on the
-     * supported_versions extension for the reall supported versions.
+     * supported_versions extension for the real supported versions.
      */
     if (!WPACKET_put_bytes_u16(pkt,
-                (!SSL_IS_DTLS(s) && s->client_version >= TLS1_3_VERSION)
-                ? TLS1_2_VERSION : s->client_version)
+                               (!SSL_IS_DTLS(s)
+                                   && s->client_version >= TLS1_3_VERSION)
+                               ? TLS1_2_VERSION : s->client_version)
             || !WPACKET_memcpy(pkt, s->s3->client_random, SSL3_RANDOM_SIZE)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);
         return 0;
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index a7f2a0f5d7..2aadc77a77 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -1036,8 +1036,7 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello)
         const SSL_METHOD *best_method = NULL;
         PACKET versionslist;
 
-        if (!PACKET_get_length_prefixed_1(&suppversions->data, &versionslist)
-                || PACKET_remaining(&suppversions->data) != 0) {
+        if (!PACKET_as_length_prefixed_1(&suppversions->data, &versionslist)) {
             /* Trailing or invalid data? */
             return SSL_R_LENGTH_MISMATCH;
         }
@@ -1052,7 +1051,8 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello)
                 continue;
             for (vent = table;
                  vent->version != 0 && vent->version != (int)candidate_vers;
-                 ++vent);
+                 ++vent)
+                ;
             if (vent->version != 0) {
                 const SSL_METHOD *method;
 
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 2122726213..de941b7f0d 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1371,8 +1371,6 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
         return 0;
     }
 
-
-    /* TODO(TLS1.3): Should we add this extension for versions < TLS1.3? */
     if (!SSL_IS_DTLS(s) && s->version >= TLS1_3_VERSION) {
         int min_version, max_version, reason, currv;
         if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_versions)