Move tmp.extended_master_secret to SSL_HANDSHAKE.
The two non-trivial changes are:
1. The public API now queries it out of the session. There is a long
comment over the old field explaining why the state was separate, but
this predates EMS being forbidden from changing across resumption. It
is not possible for established_session and the socket to disagree on
EMS.
2. Since SSL_HANDSHAKE gets reset on each handshake, the check that EMS
does not change on renego looks different. I've reworked that function a
bit, but it should have the same effect.
Change-Id: If72e5291f79681381cf4d8ceab267f76618b7c3d
Reviewed-on: https://boringssl-review.googlesource.com/13910
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index e6b8c09..c4f5e8e 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -998,8 +998,7 @@
}
if (ssl->session != NULL &&
- ssl->s3->tmp.extended_master_secret !=
- ssl->session->extended_master_secret) {
+ hs->extended_master_secret != ssl->session->extended_master_secret) {
al = SSL_AD_HANDSHAKE_FAILURE;
if (ssl->session->extended_master_secret) {
OPENSSL_PUT_ERROR(SSL, SSL_R_RESUMED_EMS_SESSION_WITHOUT_EMS_EXTENSION);
@@ -1649,7 +1648,7 @@
if (hs->new_session->master_key_length == 0) {
goto err;
}
- hs->new_session->extended_master_secret = ssl->s3->tmp.extended_master_secret;
+ hs->new_session->extended_master_secret = hs->extended_master_secret;
OPENSSL_cleanse(pms, pms_len);
OPENSSL_free(pms);
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index fdf78e4..51338e2 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -933,8 +933,7 @@
}
if (session != NULL) {
- if (session->extended_master_secret &&
- !ssl->s3->tmp.extended_master_secret) {
+ if (session->extended_master_secret && !hs->extended_master_secret) {
/* A ClientHello without EMS that attempts to resume a session with EMS
* is fatal to the connection. */
al = SSL_AD_HANDSHAKE_FAILURE;
@@ -945,8 +944,7 @@
if (!ssl_session_is_resumable(hs, session) ||
/* If the client offers the EMS extension, but the previous session
* didn't use it, then negotiate a new session. */
- ssl->s3->tmp.extended_master_secret !=
- session->extended_master_secret) {
+ hs->extended_master_secret != session->extended_master_secret) {
SSL_SESSION_free(session);
session = NULL;
}
@@ -1743,7 +1741,7 @@
if (hs->new_session->master_key_length == 0) {
goto err;
}
- hs->new_session->extended_master_secret = ssl->s3->tmp.extended_master_secret;
+ hs->new_session->extended_master_secret = hs->extended_master_secret;
OPENSSL_cleanse(premaster_secret, premaster_secret_len);
OPENSSL_free(premaster_secret);
diff --git a/ssl/internal.h b/ssl/internal.h
index 040b6fd..b2c9fcd 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1107,6 +1107,10 @@
/* v2_clienthello is one if we received a V2ClientHello. */
unsigned v2_clienthello:1;
+ /* extended_master_secret is one if the extended master secret extension is
+ * negotiated in this handshake. */
+ unsigned extended_master_secret:1;
+
/* client_version is the value sent or received in the ClientHello version. */
uint16_t client_version;
} /* SSL_HANDSHAKE */;
@@ -1619,14 +1623,6 @@
uint8_t new_mac_secret_len;
uint8_t new_key_len;
uint8_t new_fixed_iv_len;
-
- /* extended_master_secret indicates whether the extended master secret
- * computation is used in this handshake. Note that this is different from
- * whether it was used for the current session. If this is a resumption
- * handshake then EMS might be negotiated in the client and server hello
- * messages, but it doesn't matter if the session that's being resumed
- * didn't use it to create the master secret initially. */
- char extended_master_secret;
} tmp;
/* established_session is the session established by the connection. This
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 517ddbb..d0151bb 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1224,11 +1224,26 @@
int SSL_get_verify_mode(const SSL *ssl) { return ssl->verify_mode; }
int SSL_get_extms_support(const SSL *ssl) {
+ /* TLS 1.3 does not require extended master secret and always reports as
+ * supporting it. */
if (!ssl->s3->have_version) {
return 0;
}
- return ssl3_protocol_version(ssl) >= TLS1_3_VERSION ||
- ssl->s3->tmp.extended_master_secret == 1;
+ if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION) {
+ return 1;
+ }
+
+ /* If the initial handshake completed, query the established session. */
+ if (ssl->s3->established_session != NULL) {
+ return ssl->s3->established_session->extended_master_secret;
+ }
+
+ /* Otherwise, query the in-progress handshake. */
+ if (ssl->s3->hs != NULL) {
+ return ssl->s3->hs->extended_master_secret;
+ }
+ assert(0);
+ return 0;
}
int SSL_CTX_get_read_ahead(const SSL_CTX *ctx) { return 0; }
diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c
index 909d6df..9f11e05 100644
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -473,7 +473,7 @@
const uint8_t *premaster,
size_t premaster_len) {
const SSL *ssl = hs->ssl;
- if (ssl->s3->tmp.extended_master_secret) {
+ if (hs->extended_master_secret) {
uint8_t digests[EVP_MAX_MD_SIZE];
size_t digests_len;
if (!SSL_TRANSCRIPT_get_hash(&hs->transcript, digests, &digests_len) ||
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 7a9f83a..d6ef1ff 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -870,38 +870,32 @@
static int ext_ems_parse_serverhello(SSL_HANDSHAKE *hs, uint8_t *out_alert,
CBS *contents) {
SSL *const ssl = hs->ssl;
- /* Whether EMS is negotiated may not change on renegotation. */
- if (ssl->s3->initial_handshake_complete) {
- if ((contents != NULL) != ssl->s3->tmp.extended_master_secret) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_RENEGOTIATION_EMS_MISMATCH);
- *out_alert = SSL_AD_ILLEGAL_PARAMETER;
+
+ if (contents != NULL) {
+ if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION ||
+ ssl->version == SSL3_VERSION ||
+ CBS_len(contents) != 0) {
return 0;
}
- return 1;
+ hs->extended_master_secret = 1;
}
- if (contents == NULL) {
- return 1;
- }
-
- if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION ||
- ssl->version == SSL3_VERSION) {
+ /* Whether EMS is negotiated may not change on renegotiation. */
+ if (ssl->s3->established_session != NULL &&
+ hs->extended_master_secret !=
+ ssl->s3->established_session->extended_master_secret) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_RENEGOTIATION_EMS_MISMATCH);
+ *out_alert = SSL_AD_ILLEGAL_PARAMETER;
return 0;
}
- if (CBS_len(contents) != 0) {
- return 0;
- }
-
- ssl->s3->tmp.extended_master_secret = 1;
return 1;
}
static int ext_ems_parse_clienthello(SSL_HANDSHAKE *hs, uint8_t *out_alert,
CBS *contents) {
- SSL *const ssl = hs->ssl;
- uint16_t version = ssl3_protocol_version(ssl);
+ uint16_t version = ssl3_protocol_version(hs->ssl);
if (version >= TLS1_3_VERSION ||
version == SSL3_VERSION) {
return 1;
@@ -915,12 +909,12 @@
return 0;
}
- ssl->s3->tmp.extended_master_secret = 1;
+ hs->extended_master_secret = 1;
return 1;
}
static int ext_ems_add_serverhello(SSL_HANDSHAKE *hs, CBB *out) {
- if (!hs->ssl->s3->tmp.extended_master_secret) {
+ if (!hs->extended_master_secret) {
return 1;
}
