Forbid EMS from changing during renegotation.
Changing parameters on renegotiation makes all our APIs confusing. This
one has no reason to change, so lock it down. In particular, our
preference to forbid Token Binding + renego may be overridden at the
IETF, even though it's insane. Loosening it will be a bit less of a
headache if EMS can't change.
https://www.ietf.org/mail-archive/web/unbearable/current/msg00690.html
claims that this is already in the specification and enforced by NSS. I
can't find anything to this effect in the specification. It just says
the client MUST disable renegotiation when EMS is missing, which is
wishful thinking. At a glance, NSS doesn't seem to check, though I could
be misunderstanding the code.
Nonetheless, locking this down is a good idea anyway. Accurate or not,
take the email as an implicit endorsement of this from Mozilla.
Change-Id: I236b05991d28bed199763dcf2f47bbfb9d0322d7
Reviewed-on: https://boringssl-review.googlesource.com/10721
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 0b08400..e550796 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -958,10 +958,6 @@
*
* https://tools.ietf.org/html/rfc7627 */
-static void ext_ems_init(SSL *ssl) {
- ssl->s3->tmp.extended_master_secret = 0;
-}
-
static int ext_ems_add_clienthello(SSL *ssl, CBB *out) {
uint16_t min_version, max_version;
if (!ssl_get_version_range(ssl, &min_version, &max_version)) {
@@ -983,6 +979,17 @@
static int ext_ems_parse_serverhello(SSL *ssl, uint8_t *out_alert,
CBS *contents) {
+ /* 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;
+ return 0;
+ }
+
+ return 1;
+ }
+
if (contents == NULL) {
return 1;
}
@@ -2381,7 +2388,7 @@
},
{
TLSEXT_TYPE_extended_master_secret,
- ext_ems_init,
+ NULL,
ext_ems_add_clienthello,
ext_ems_parse_serverhello,
ext_ems_parse_clienthello,