commit f04c2e987871f852d0ec391005d9202157999523
author David Benjamin <davidben@google.com> Tue Dec 06 13:35:25 2016 -0500
committer Adam Langley <agl@google.com> Thu Dec 08 16:39:52 2016 +0000
tree 3661c096b6be8fdddab4147a30c76fbae432aa2d
parent a2bda9fb95a9e9945d6ee12847c9b6022e8cd8af

Move client_version into SSL_HANDSHAKE.

There is no need to retain it beyond this point.

Change-Id: Ib5722ab30fc013380198b1582d1240f0fe0aa770
Reviewed-on: https://boringssl-review.googlesource.com/12620
Reviewed-by: Adam Langley <agl@google.com>

ssl/handshake_client.c

diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index 09b16eb..4a58af9 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -642,7 +642,7 @@
 
   /* For SSLv3, the SCSV is added. Otherwise the renegotiation extension is
    * added. */
-  if (ssl->client_version == SSL3_VERSION &&
+  if (max_version == SSL3_VERSION &&
       !ssl->s3->initial_handshake_complete) {
     if (!CBB_add_u16(&child, SSL3_CK_SCSV & 0xffff)) {
       return 0;
@@ -675,7 +675,7 @@
                     !ssl->s3->initial_handshake_complete;
 
   CBB child;
-  if (!CBB_add_u16(&body, ssl->client_version) ||
+  if (!CBB_add_u16(&body, hs->client_version) ||
       !CBB_add_bytes(&body, ssl->s3->client_random, SSL3_RANDOM_SIZE) ||
       !CBB_add_u8_length_prefixed(&body, &child) ||
       (has_session &&
@@ -739,18 +739,18 @@
     return -1;
   }
 
+  uint16_t max_wire_version = ssl->method->version_to_wire(max_version);
   assert(ssl->state == SSL3_ST_CW_CLNT_HELLO_A);
   if (!ssl->s3->have_version) {
-    ssl->version = ssl->method->version_to_wire(max_version);
-    /* Only set |ssl->client_version| on the initial handshake. Renegotiations,
-     * although locked to a version, reuse the value. When using the plain RSA
-     * key exchange, the ClientHello version is checked in the premaster secret.
-     * Some servers fail when this value changes. */
-    ssl->client_version = ssl->version;
+    ssl->version = max_wire_version;
+  }
 
-    if (max_version >= TLS1_3_VERSION) {
-      ssl->client_version = ssl->method->version_to_wire(TLS1_2_VERSION);
-    }
+  /* Always advertise the ClientHello version from the original maximum version,
+   * even on renegotiation. The static RSA key exchange uses this field, and
+   * some servers fail when it changes across handshakes. */
+  hs->client_version = max_wire_version;
+  if (max_version >= TLS1_3_VERSION) {
+    hs->client_version = ssl->method->version_to_wire(TLS1_2_VERSION);
   }
 
   /* If the configured session has expired or was created at a disabled
@@ -1603,8 +1603,8 @@
 
     EVP_PKEY_free(pkey);
 
-    pms[0] = ssl->client_version >> 8;
-    pms[1] = ssl->client_version & 0xff;
+    pms[0] = hs->client_version >> 8;
+    pms[1] = hs->client_version & 0xff;
     if (!RAND_bytes(&pms[2], SSL_MAX_MASTER_KEY_LENGTH - 2)) {
       goto err;
     }