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 <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6415)
This commit is contained in:
Matt Caswell 2018-06-05 12:23:28 +01:00
parent 4ff1a52666
commit 6cf2dbd9fa
6 changed files with 25 additions and 63 deletions

View file

@ -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) {

View file

@ -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;

View file

@ -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));

View file

@ -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 */

View file

@ -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)

View file

@ -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-----