From: Matt Caswell Date: Tue, 5 Jun 2018 11:23:28 +0000 (+0100) Subject: Don't store the ticket nonce in the session X-Git-Tag: OpenSSL_1_1_1-pre8~64 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6cf2dbd9faffbed52a6bede924fe0a93345b8bfa;p=openssl Don't store the ticket nonce in the session We generate the secrets based on the nonce immediately so there is no need to keep the nonce. Reviewed-by: Andy Polyakov (Merged from https://github.com/openssl/openssl/pull/6415) --- diff --git a/ssl/ssl_asn1.c b/ssl/ssl_asn1.c index 8c2afbe6af..9af4b84d36 100644 --- a/ssl/ssl_asn1.c +++ b/ssl/ssl_asn1.c @@ -41,7 +41,6 @@ typedef struct { uint64_t flags; uint32_t max_early_data; ASN1_OCTET_STRING *alpn_selected; - ASN1_OCTET_STRING *tick_nonce; uint32_t tlsext_max_fragment_len_mode; ASN1_OCTET_STRING *ticket_appdata; } SSL_SESSION_ASN1; @@ -73,9 +72,8 @@ ASN1_SEQUENCE(SSL_SESSION_ASN1) = { ASN1_EXP_OPT_EMBED(SSL_SESSION_ASN1, tlsext_tick_age_add, ZUINT32, 14), ASN1_EXP_OPT_EMBED(SSL_SESSION_ASN1, max_early_data, ZUINT32, 15), ASN1_EXP_OPT(SSL_SESSION_ASN1, alpn_selected, ASN1_OCTET_STRING, 16), - ASN1_EXP_OPT(SSL_SESSION_ASN1, tick_nonce, ASN1_OCTET_STRING, 17), - ASN1_EXP_OPT_EMBED(SSL_SESSION_ASN1, tlsext_max_fragment_len_mode, ZUINT32, 18), - ASN1_EXP_OPT(SSL_SESSION_ASN1, ticket_appdata, ASN1_OCTET_STRING, 19) + ASN1_EXP_OPT_EMBED(SSL_SESSION_ASN1, tlsext_max_fragment_len_mode, ZUINT32, 17), + ASN1_EXP_OPT(SSL_SESSION_ASN1, ticket_appdata, ASN1_OCTET_STRING, 18) } static_ASN1_SEQUENCE_END(SSL_SESSION_ASN1) IMPLEMENT_STATIC_ASN1_ENCODE_FUNCTIONS(SSL_SESSION_ASN1) @@ -124,7 +122,6 @@ int i2d_SSL_SESSION(SSL_SESSION *in, unsigned char **pp) ASN1_OCTET_STRING psk_identity, psk_identity_hint; #endif ASN1_OCTET_STRING alpn_selected; - ASN1_OCTET_STRING tick_nonce; ASN1_OCTET_STRING ticket_appdata; long l; @@ -195,12 +192,6 @@ int i2d_SSL_SESSION(SSL_SESSION *in, unsigned char **pp) ssl_session_oinit(&as.alpn_selected, &alpn_selected, in->ext.alpn_selected, in->ext.alpn_selected_len); - if (in->ext.tick_nonce == NULL) - as.tick_nonce = NULL; - else - ssl_session_oinit(&as.tick_nonce, &tick_nonce, - in->ext.tick_nonce, in->ext.tick_nonce_len); - as.tlsext_max_fragment_len_mode = in->ext.max_fragment_len_mode; if (in->ticket_appdata == NULL) @@ -374,15 +365,6 @@ SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **a, const unsigned char **pp, ret->ext.alpn_selected_len = 0; } - if (as->tick_nonce != NULL) { - ret->ext.tick_nonce = as->tick_nonce->data; - ret->ext.tick_nonce_len = as->tick_nonce->length; - as->tick_nonce->data = NULL; - } else { - ret->ext.tick_nonce = NULL; - ret->ext.tick_nonce_len = 0; - } - ret->ext.max_fragment_len_mode = as->tlsext_max_fragment_len_mode; if (as->ticket_appdata != NULL) { diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 23608561ac..86c250b695 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -569,8 +569,6 @@ struct ssl_session_st { /* Session lifetime hint in seconds */ unsigned long tick_lifetime_hint; uint32_t tick_age_add; - unsigned char *tick_nonce; - size_t tick_nonce_len; int tick_identity; /* Max number of bytes that can be sent as early data */ uint32_t max_early_data; diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index 52ec670787..525edb3289 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -133,7 +133,6 @@ SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket) #endif dest->peer_chain = NULL; dest->peer = NULL; - dest->ext.tick_nonce = NULL; dest->ticket_appdata = NULL; memset(&dest->ex_data, 0, sizeof(dest->ex_data)); @@ -230,13 +229,6 @@ SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket) } } - if (src->ext.tick_nonce != NULL) { - dest->ext.tick_nonce = OPENSSL_memdup(src->ext.tick_nonce, - src->ext.tick_nonce_len); - if (dest->ext.tick_nonce == NULL) - goto err; - } - #ifndef OPENSSL_NO_SRP if (src->srp_username) { dest->srp_username = OPENSSL_strdup(src->srp_username); @@ -824,7 +816,6 @@ void SSL_SESSION_free(SSL_SESSION *ss) OPENSSL_free(ss->srp_username); #endif OPENSSL_free(ss->ext.alpn_selected); - OPENSSL_free(ss->ext.tick_nonce); OPENSSL_free(ss->ticket_appdata); CRYPTO_THREAD_lock_free(ss->lock); OPENSSL_clear_free(ss, sizeof(*ss)); diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 99445a6564..754fedb1af 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -2559,12 +2559,12 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt) RAW_EXTENSION *exts = NULL; PACKET nonce; + PACKET_null_init(&nonce); + if (!PACKET_get_net_4(pkt, &ticket_lifetime_hint) || (SSL_IS_TLS13(s) && (!PACKET_get_net_4(pkt, &age_add) - || !PACKET_get_length_prefixed_1(pkt, &nonce) - || !PACKET_memdup(&nonce, &s->session->ext.tick_nonce, - &s->session->ext.tick_nonce_len))) + || !PACKET_get_length_prefixed_1(pkt, &nonce))) || !PACKET_get_net_2(pkt, &ticklen) || (!SSL_IS_TLS13(s) && PACKET_remaining(pkt) != ticklen) || (SSL_IS_TLS13(s) @@ -2692,8 +2692,8 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt) if (!tls13_hkdf_expand(s, md, s->resumption_master_secret, nonce_label, sizeof(nonce_label) - 1, - s->session->ext.tick_nonce, - s->session->ext.tick_nonce_len, + PACKET_data(&nonce), + PACKET_remaining(&nonce), s->session->master_key, hashlen)) { /* SSLfatal() already called */ diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index c690cf0191..c2976b7a32 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -3753,6 +3753,7 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt) unsigned char iv[EVP_MAX_IV_LENGTH]; unsigned char key_name[TLSEXT_KEYNAME_LENGTH]; int iv_len; + unsigned char tick_nonce[TICKET_NONCE_SIZE]; size_t macoffset, macendoffset; union { unsigned char age_add_c[sizeof(uint32_t)]; @@ -3762,7 +3763,7 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt) if (SSL_IS_TLS13(s)) { size_t i, hashlen; uint64_t nonce; - const char nonce_label[] = "resumption"; + static const unsigned char nonce_label[] = "resumption"; const EVP_MD *md = ssl_handshake_md(s); void (*cb) (const SSL *ssl, int type, int val) = NULL; int hashleni = EVP_MD_size(md); @@ -3781,7 +3782,6 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt) else if (s->ctx->info_callback != NULL) cb = s->ctx->info_callback; - if (cb != NULL) { /* * We don't start and stop the handshake in between each ticket when @@ -3823,26 +3823,17 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt) } s->session->ext.tick_age_add = age_add_u.age_add; - OPENSSL_free(s->session->ext.tick_nonce); - s->session->ext.tick_nonce = OPENSSL_zalloc(TICKET_NONCE_SIZE); - if (s->session->ext.tick_nonce == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, - SSL_F_TLS_CONSTRUCT_NEW_SESSION_TICKET, - ERR_R_MALLOC_FAILURE); - goto err; - } nonce = s->next_ticket_nonce; - for (i = TICKET_NONCE_SIZE; nonce > 0 && i > 0; i--) { - s->session->ext.tick_nonce[i - 1] = nonce & 0xff; + for (i = TICKET_NONCE_SIZE; i > 0; i--) { + tick_nonce[i - 1] = (unsigned char)(nonce & 0xff); nonce >>= 8; } - s->session->ext.tick_nonce_len = TICKET_NONCE_SIZE; if (!tls13_hkdf_expand(s, md, s->resumption_master_secret, - (const unsigned char *)nonce_label, + nonce_label, sizeof(nonce_label) - 1, - s->session->ext.tick_nonce, - s->session->ext.tick_nonce_len, + tick_nonce, + TICKET_NONCE_SIZE, s->session->master_key, hashlen)) { /* SSLfatal() already called */ @@ -3992,8 +3983,8 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt) ? 0 : s->session->timeout) || (SSL_IS_TLS13(s) && (!WPACKET_put_bytes_u32(pkt, age_add_u.age_add) - || !WPACKET_sub_memcpy_u8(pkt, s->session->ext.tick_nonce, - s->session->ext.tick_nonce_len))) + || !WPACKET_sub_memcpy_u8(pkt, tick_nonce, + TICKET_NONCE_SIZE))) /* Now the actual ticket data */ || !WPACKET_start_sub_packet_u16(pkt) || !WPACKET_get_total_written(pkt, &macoffset) diff --git a/test/session.pem b/test/session.pem index 8b01ffcb29..ea0b0bcec2 100644 --- a/test/session.pem +++ b/test/session.pem @@ -1,7 +1,7 @@ -----BEGIN SSL SESSION PARAMETERS----- -MIIFRAIBAQICAwQEAhMCBCDom190ggLdEV9HNhMrbc8/MLs9NS3nqoWFoIJLgQqS -tgQwzskkzvykWInToBTKeUhVYe4BidOBYHdHZ65Z2ETBf63lz1dMKRraxwl6K07f -BUyBoQYCBFlct3qiBAICHCCjggPrMIID5zCCAs+gAwIBAgIJALnu1NlVpZ6zMA0G +MIIFSgIBAQICAwQEAhMCBCAUv8MKab5ruWM6I8xtEH++u+bb2B1OznYnDrRcpLll +6AQwzwJoGXOQ3uCa7bCy07owBiH4Bf13MiDtwaHSnNTEyfLEZBy3SgCE06wa5TJk +Fx8aoQYCBFsWdRqiBAICHCCjggPrMIID5zCCAs+gAwIBAgIJALnu1NlVpZ6zMA0G CSqGSIb3DQEBBQUAMHAxCzAJBgNVBAYTAlVLMRYwFAYDVQQKDA1PcGVuU1NMIEdy b3VwMSIwIAYDVQQLDBlGT1IgVEVTVElORyBQVVJQT1NFUyBPTkxZMSUwIwYDVQQD DBxPcGVuU1NMIFRlc3QgSW50ZXJtZWRpYXRlIENBMB4XDTExMTIwODE0MDE0OFoX @@ -22,10 +22,10 @@ Wz9qoeoFZax+QBpIZYjROU3TS3fpyLsrnlr0CDQ5R7kCCDGa8dkXxemmpZZLbUCp W2Uoy8sAA4JjN9OtsZY7dvUXFgJ7vVNTRnI01ghknbtD+2SxSQd3CWF6QhcRMAzZ J1z1cbbwGDDzfvGFPzJ+Sq+zEPdsxoVLLSetCiBc+40ZcDS5dV98h9XD7JMTQfxz A7mNGv73JoZJA6nFgj+ADSlJsY/tJBv+z1iQRueoh9Qeee+ZbRifPouCB8FDx+Al -tvHTANdAq0t/K3o+pplMVKQCBAClAwIBFakEAgIcIKqBwwSBwFNYKC1r6z0zp+wI -V+A8n63Wh4X/0HtKa7dJCGhvLxjI+BL9QK8JB2Qrs3OR32VjVyVWD9K0atHwhyTR -wwFJfBEfgv9reCtOiQg2oHadD3iCbHjhhGCvbj+zCChMGSEE8NtqkBpwGATtwgN7 -qoLShh+JyHwhfXWKhKlEibr8W0ipe6R3VUW9+wsW8nTGs4FmvQSIkLI1WCr226LN -wkRIx5+3Q3mZB39Epco4srvyLy8J/B+x2lhUdIpov7VBz++C864GAgRYHFWqrwQC -AkAAsQMEAQA= +tvHTANdAq0t/K3o+pplMVKQCBAClAwIBFakEAgIcIKqB0wSB0EMQ5938LY/ASVsV +0kStjTVOps9p3VT071bTjD3RR211+gLzBwGCk8gWNH1glJXjLAenh9E2ivDK1tYQ +3ODRdB3V46t9E78r0uAmSG/WMJ9OvkFlXyIhseYwvWW0P1cAYPI/j3Evgcyu9GIs +HSDVEKbBy9CJYCkW/SrT+2A3ouqp+wSW0XgDLFFB+mBte2Hg7wv2uILrYZ4Y0fNe +CUcTq8B+0EFEiq7p0KRGXwpSKYxNw7qZgg/Us3W85BYMnzYjfDzN0KHf+BI28VRT +Rjxuud2uBwIFANHVD/k= -----END SSL SESSION PARAMETERS-----