Extended master secret fixes and checks.
Add new flag TLS1_FLAGS_RECEIVED_EXTMS which is set when the peer sends
the extended master secret extension.
Server now sends extms if and only if the client sent extms.
Check consistency of extms extension when resuming sessions following (where
practical) RFC7627.
Reviewed-by: Matt Caswell <matt@openssl.org>
diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h
index ef93c08..2a2974c 100644
--- a/include/openssl/ssl3.h
+++ b/include/openssl/ssl3.h
@@ -369,6 +369,8 @@
/* Set if we encrypt then mac instead of usual mac then encrypt */
# define TLS1_FLAGS_ENCRYPT_THEN_MAC 0x0100
+/* Set if extended master secret extension received from peer */
+# define TLS1_FLAGS_RECEIVED_EXTMS 0x0200
# define SSL3_MT_HELLO_REQUEST 0
# define SSL3_MT_CLIENT_HELLO 1
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 34091d3..1e1b265 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -2021,8 +2021,9 @@
__owur int dtls1_process_heartbeat(SSL *s, unsigned char *p, unsigned int length);
# endif
-__owur int tls1_process_ticket(SSL *s, const PACKET *ext,
- const PACKET *session_id, SSL_SESSION **ret);
+__owur int tls_check_serverhello_tlsext_early(SSL *s, const PACKET *ext,
+ const PACKET *session_id,
+ SSL_SESSION **ret);
__owur int tls12_get_sigandhash(unsigned char *p, const EVP_PKEY *pk,
const EVP_MD *md);
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 5265b15..aff615e 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -497,6 +497,10 @@
ss->ssl_version = s->version;
ss->verify_result = X509_V_OK;
+ /* If client supports extended master secret set it in session */
+ if (s->s3->flags & TLS1_FLAGS_RECEIVED_EXTMS)
+ ss->flags |= SSL_SESS_FLAG_EXTMS;
+
return (1);
}
@@ -533,8 +537,8 @@
if (len == 0)
try_session_cache = 0;
- /* sets s->tlsext_ticket_expected */
- r = tls1_process_ticket(s, ext, session_id, &ret);
+ /* sets s->tlsext_ticket_expected and extended master secret flag */
+ r = tls_check_serverhello_tlsext_early(s, ext, session_id, &ret);
switch (r) {
case -1: /* Error during processing */
fatal = 1;
@@ -670,6 +674,20 @@
goto err;
}
+ /* Check extended master secret extension consistency */
+ if (ret->flags & SSL_SESS_FLAG_EXTMS) {
+ /* If old session includes extms, but new does not: abort handshake */
+ if (!(s->s3->flags & TLS1_FLAGS_RECEIVED_EXTMS)) {
+ SSLerr(SSL_F_SSL_GET_PREV_SESSION, SSL_R_INCONSISTENT_EXTMS);
+ ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
+ fatal = 1;
+ goto err;
+ }
+ } else if (s->s3->flags & TLS1_FLAGS_RECEIVED_EXTMS) {
+ /* If new session includes extms, but old does not: do not resume */
+ goto err;
+ }
+
s->session_ctx->stats.sess_hit++;
SSL_SESSION_free(s->session);
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index a6f2502..83015e8 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1699,7 +1699,7 @@
}
}
#endif
- if (!s->hit && s->session->flags & SSL_SESS_FLAG_EXTMS) {
+ if (s->s3->flags & TLS1_FLAGS_RECEIVED_EXTMS) {
s2n(TLSEXT_TYPE_extended_master_secret, ret);
s2n(0, ret);
}
@@ -2269,10 +2269,11 @@
else if (type == TLSEXT_TYPE_encrypt_then_mac)
s->s3->flags |= TLS1_FLAGS_ENCRYPT_THEN_MAC;
#endif
- else if (type == TLSEXT_TYPE_extended_master_secret) {
- if (!s->hit)
- s->session->flags |= SSL_SESS_FLAG_EXTMS;
- }
+ /*
+ * Note: extended master secret extension handled in
+ * tls_check_serverhello_tlsext_early()
+ */
+
/*
* If this ClientHello extension was unhandled and this is a
* nonresumed connection, check whether the extension is a custom
@@ -2366,6 +2367,8 @@
s->s3->flags &= ~TLS1_FLAGS_ENCRYPT_THEN_MAC;
#endif
+ s->s3->flags &= ~TLS1_FLAGS_RECEIVED_EXTMS;
+
if (!PACKET_get_net_2(pkt, &length))
goto ri_check;
@@ -2554,6 +2557,7 @@
}
#endif
else if (type == TLSEXT_TYPE_extended_master_secret) {
+ s->s3->flags |= TLS1_FLAGS_RECEIVED_EXTMS;
if (!s->hit)
s->session->flags |= SSL_SESS_FLAG_EXTMS;
}
@@ -2603,6 +2607,19 @@
return 0;
}
+ if (s->hit) {
+ /*
+ * Check extended master secret extension is consistent with
+ * original session.
+ */
+ if (!(s->s3->flags & TLS1_FLAGS_RECEIVED_EXTMS) !=
+ !(s->session->flags & SSL_SESS_FLAG_EXTMS)) {
+ *al = SSL_AD_HANDSHAKE_FAILURE;
+ SSLerr(SSL_F_SSL_SCAN_SERVERHELLO_TLSEXT, SSL_R_INCONSISTENT_EXTMS);
+ return 0;
+ }
+ }
+
return 1;
}
@@ -2892,8 +2909,11 @@
/*-
* Since the server cache lookup is done early on in the processing of the
- * ClientHello, and other operations depend on the result, we need to handle
- * any TLS session ticket extension at the same time.
+ * ClientHello and other operations depend on the result some extensions
+ * need to be handled at the same time.
+ *
+ * Two extensions are currently handled, session ticket and extended master
+ * secret.
*
* session_id: ClientHello session ID.
* ext: ClientHello extensions (including length prefix)
@@ -2920,23 +2940,29 @@
* a session ticket or we couldn't use the one it gave us, or if
* s->ctx->tlsext_ticket_key_cb asked to renew the client's ticket.
* Otherwise, s->tlsext_ticket_expected is set to 0.
+ *
+ * For extended master secret flag is set if the extension is present.
+ *
*/
-int tls1_process_ticket(SSL *s, const PACKET *ext, const PACKET *session_id,
- SSL_SESSION **ret)
+int tls_check_serverhello_tlsext_early(SSL *s, const PACKET *ext,
+ const PACKET *session_id,
+ SSL_SESSION **ret)
{
unsigned int i;
PACKET local_ext = *ext;
int retv = -1;
+ int have_ticket = 0;
+ int use_ticket = tls_use_ticket(s);
+
*ret = NULL;
s->tlsext_ticket_expected = 0;
+ s->s3->flags &= ~TLS1_FLAGS_RECEIVED_EXTMS;
/*
* If tickets disabled behave as if no ticket present to permit stateful
* resumption.
*/
- if (!tls_use_ticket(s))
- return 0;
if ((s->version <= SSL3_VERSION))
return 0;
@@ -2957,10 +2983,17 @@
retv = 0;
goto end;
}
- if (type == TLSEXT_TYPE_session_ticket) {
+ if (type == TLSEXT_TYPE_session_ticket && use_ticket) {
int r;
unsigned char *etick;
+ /* Duplicate extension */
+ if (have_ticket != 0) {
+ retv = -1;
+ goto end;
+ }
+ have_ticket = 1;
+
if (size == 0) {
/*
* The client will accept a ticket but doesn't currently have
@@ -2968,7 +3001,7 @@
*/
s->tlsext_ticket_expected = 1;
retv = 1;
- goto end;
+ continue;
}
if (s->tls_session_secret_cb) {
/*
@@ -2978,7 +3011,7 @@
* calculate the master secret later.
*/
retv = 2;
- goto end;
+ continue;
}
if (!PACKET_get_bytes(&local_ext, &etick, size)) {
/* Shouldn't ever happen */
@@ -3003,15 +3036,18 @@
retv = -1;
break;
}
- goto end;
+ continue;
} else {
+ if (type == TLSEXT_TYPE_extended_master_secret)
+ s->s3->flags |= TLS1_FLAGS_RECEIVED_EXTMS;
if (!PACKET_forward(&local_ext, size)) {
retv = -1;
goto end;
}
}
}
- retv = 0;
+ if (have_ticket == 0)
+ retv = 0;
end:
return retv;
}
