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)

diff --git a/ssl/ssl_asn1.c b/ssl/ssl_asn1.c
index 8c2afbe6afd2896b9dc9b3045ffabe15d03c242f..9af4b84d3600c46af3d8fcdc22b2edb2f0b1e8e6 100644 (file)
--- 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 23608561acb58687fd904a6eea325108b51cf99f..86c250b695f3fdad6c0ec3b5ba4d73ff34972e64 100644 (file)
--- 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 52ec670787a3d584a77399f135fe9531efd5bd29..525edb3289b7d8c7392e12f9192f61f4d33f549c 100644 (file)
--- 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 99445a65644f48e3623fbd6d3eab104ad5779530..754fedb1afa9f403ae19bcfcf762e8b307ff8eb3 100644 (file)
--- 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 c690cf0191cd9f42228198d4eb06c90190950cb1..c2976b7a3225c926e758e7b764a06ca2ec4ccaca 100644 (file)
--- 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 8b01ffcb2981f3b1b59d8967e4d8f292cb6043d1..ea0b0bcec2387e155426da29656845b65c99c854 100644 (file)
--- 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-----