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>
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;
}
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 4bac0d5..c114074 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -544,8 +544,9 @@
return 0;
}
-static int negotiate_version(SSL *ssl, uint8_t *out_alert,
+static int negotiate_version(SSL_HANDSHAKE *hs, uint8_t *out_alert,
const SSL_CLIENT_HELLO *client_hello) {
+ SSL *const ssl = hs->ssl;
uint16_t min_version, max_version;
if (!ssl_get_version_range(ssl, &min_version, &max_version)) {
*out_alert = SSL_AD_PROTOCOL_VERSION;
@@ -635,7 +636,7 @@
return 0;
}
- ssl->client_version = client_hello->version;
+ hs->client_version = client_hello->version;
ssl->version = ssl->method->version_to_wire(version);
ssl->s3->enc_method = ssl3_get_enc_method(version);
assert(ssl->s3->enc_method != NULL);
@@ -866,7 +867,7 @@
/* Negotiate the protocol version if we have not done so yet. */
if (!ssl->s3->have_version) {
- if (!negotiate_version(ssl, &al, &client_hello)) {
+ if (!negotiate_version(hs, &al, &client_hello)) {
goto f_err;
}
@@ -1708,9 +1709,9 @@
/* The premaster secret must begin with |client_version|. This too must be
* checked in constant time (http://eprint.iacr.org/2003/052/). */
good &= constant_time_eq_8(decrypt_buf[padding_len],
- (unsigned)(ssl->client_version >> 8));
+ (unsigned)(hs->client_version >> 8));
good &= constant_time_eq_8(decrypt_buf[padding_len + 1],
- (unsigned)(ssl->client_version & 0xff));
+ (unsigned)(hs->client_version & 0xff));
/* Select, in constant time, either the decrypted premaster or the random
* premaster based on |good|. */
diff --git a/ssl/internal.h b/ssl/internal.h
index 19f6bc1..995adcf 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1020,6 +1020,9 @@
/* ticket_expected is one if a TLS 1.2 NewSessionTicket message is to be sent
* or received. */
unsigned ticket_expected:1;
+
+ /* client_version is the value sent or received in the ClientHello version. */
+ uint16_t client_version;
} /* SSL_HANDSHAKE */;
SSL_HANDSHAKE *ssl_handshake_new(SSL *ssl);
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index d504240..9eb8cd2 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2910,8 +2910,6 @@
ssl->d1->mtu = mtu;
}
- ssl->client_version = ssl->version;
-
return 1;
}
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index fa1c067..474f50a 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2733,8 +2733,8 @@
int ssl_add_clienthello_tlsext(SSL_HANDSHAKE *hs, CBB *out, size_t header_len) {
SSL *const ssl = hs->ssl;
- /* don't add extensions for SSLv3 unless doing secure renegotiation */
- if (ssl->client_version == SSL3_VERSION &&
+ /* Don't add extensions for SSLv3 unless doing secure renegotiation. */
+ if (hs->client_version == SSL3_VERSION &&
!ssl->s3->send_connection_binding) {
return 1;
}
