From 2c604cb9af4a879ea43fd7fd84883a5e97ab0fe0 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 24 Feb 2017 09:30:54 +0000 Subject: [PATCH] Validate the ticket age for resumed sessions If the ticket age calcualtions do not check out then we must not accept early data (it could be a replay). Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/2737) --- ssl/ssl_locl.h | 10 ++++++++++ ssl/statem/extensions_srvr.c | 36 ++++++++++++++++++++++++++++++++---- ssl/statem/statem_srvr.c | 12 ++++++++---- 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 2151d631e8..52515cb589 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -638,6 +638,14 @@ typedef enum { */ # define EARLY_DATA_CIPHERTEXT_OVERHEAD ((6 * (EVP_GCM_TLS_TAG_LEN + 1)) + 2) +/* + * The allowance we have between the client's calculated ticket age and our own. + * We allow for 10 seconds (units are in ms). If a ticket is presented and the + * client's age calculation is different by more than this than our own then we + * do not allow that ticket for early_data. + */ +# define TICKET_AGE_ALLOWANCE (10 * 1000) + #define MAX_COMPRESSIONS_SIZE 255 struct ssl_comp_st { @@ -1197,6 +1205,8 @@ struct ssl_st { /* Are we expecting to receive early data? */ int early_data; + /* Is the session suitable for early data? */ + int early_data_ok; } ext; /* Parsed form of the ClientHello, kept around across early_cb calls. */ diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 57f38738d8..99ce6ce80d 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -171,7 +171,8 @@ int tls_parse_ctos_early_data(SSL *s, PACKET *pkt, unsigned int context, } if (s->max_early_data == 0 || !s->hit || s->session->ext.tick_identity != 0 - || s->early_data_state != SSL_EARLY_DATA_ACCEPTING) { + || s->early_data_state != SSL_EARLY_DATA_ACCEPTING + || !s->ext.early_data_ok) { s->ext.early_data = SSL_EARLY_DATA_REJECTED; } else { s->ext.early_data = SSL_EARLY_DATA_ACCEPTED; @@ -693,6 +694,7 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x, SSL_SESSION *sess = NULL; unsigned int id, i; const EVP_MD *md = NULL; + uint32_t ticket_age, now, agesec, agems; /* * If we have no PSK kex mode that we recognise then we can't resume so @@ -709,16 +711,16 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x, for (id = 0; PACKET_remaining(&identities) != 0; id++) { PACKET identity; - unsigned long ticket_age; + unsigned long ticket_agel; int ret; if (!PACKET_get_length_prefixed_2(&identities, &identity) - || !PACKET_get_net_4(&identities, &ticket_age)) { + || !PACKET_get_net_4(&identities, &ticket_agel)) { *al = SSL_AD_DECODE_ERROR; return 0; } - /* TODO(TLS1.3): Should we validate the ticket age? */ + ticket_age = (uint32_t)ticket_agel; ret = tls_decrypt_ticket(s, PACKET_data(&identity), PACKET_remaining(&identity), NULL, 0, &sess); @@ -777,6 +779,32 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x, } sess->ext.tick_identity = id; + + now = (uint32_t)time(NULL); + agesec = now - (uint32_t)sess->time; + agems = agesec * (uint32_t)1000; + ticket_age -= sess->ext.tick_age_add; + + + /* + * For simplicity we do our age calculations in seconds. If the client does + * it in ms then it could appear that their ticket age is longer than ours + * (our ticket age calculation should always be slightly longer than the + * client's due to the network latency). Therefore we add 1000ms to our age + * calculation to adjust for rounding errors. + */ + if (sess->timeout >= agesec + && agems / (uint32_t)1000 == agesec + && ticket_age <= agems + 1000 + && ticket_age + TICKET_AGE_ALLOWANCE >= agems + 1000) { + /* + * Ticket age is within tolerance and not expired. We allow it for early + * data + */ + s->ext.early_data_ok = 1; + } + + SSL_SESSION_free(s->session); s->session = sess; return 1; diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index e6a84dfa7b..56db987ee4 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -3400,6 +3400,7 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt) if (RAND_bytes(age_add_u.age_add_c, sizeof(age_add_u)) <= 0) goto err; s->session->ext.tick_age_add = age_add_u.age_add; + s->session->time = (long)time(NULL); } /* get session encoding length */ @@ -3494,11 +3495,14 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt) } /* - * Ticket lifetime hint (advisory only): We leave this unspecified - * for resumed session (for simplicity), and guess that tickets for - * new sessions will live as long as their sessions. + * Ticket lifetime hint: For TLSv1.2 this is advisory only and we leave this + * unspecified for resumed session (for simplicity). + * In TLSv1.3 we reset the "time" field above, and always specify the + * timeout. */ - if (!WPACKET_put_bytes_u32(pkt, s->hit ? 0 : s->session->timeout) + if (!WPACKET_put_bytes_u32(pkt, + (s->hit && !SSL_IS_TLS13(s)) + ? 0 : s->session->timeout) || (SSL_IS_TLS13(s) && !WPACKET_put_bytes_u32(pkt, age_add_u.age_add)) /* Now the actual ticket data */