Splitting SSL session state.

To prevent configuration/established session confusion, the handshake
session state is separated into the configured session (ssl->session)
and the newly created session (ssl->s3->new_session). Upon conclusion of
the handshake, the finalized session is stored
in (ssl->s3->established_session). During the handshake, any requests
for the session (SSL_get_session) return a non-resumable session, to
prevent resumption of a partially filled session. Sessions should only
be cached upon the completion of the full handshake, using the resulting
established_session. The semantics of accessors on the session are
maintained mid-renego.

Change-Id: I4358aecb71fce4fe14a6746c5af1416a69935078
Reviewed-on: https://boringssl-review.googlesource.com/8612
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/handshake_server.c

diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 9f4aae3..caa2681 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -243,7 +243,7 @@
         if (ret <= 0) {
           goto end;
         }
-        if (ssl->hit) {
+        if (ssl->session != NULL) {
           ssl->state = SSL3_ST_SW_SESSION_TICKET_A;
         } else {
           ssl->state = SSL3_ST_SW_CERT_A;
@@ -391,16 +391,16 @@
         }
 
         ssl->method->received_flight(ssl);
-        if (ssl->hit) {
+        if (ssl->session != NULL) {
           ssl->state = SSL_ST_OK;
         } else {
           ssl->state = SSL3_ST_SW_SESSION_TICKET_A;
         }
 
-        /* If this is a full handshake with ChannelID then record the hashshake
-         * hashes in |ssl->session| in case we need them to verify a ChannelID
-         * signature on a resumption of this session in the future. */
-        if (!ssl->hit && ssl->s3->tlsext_channel_id_valid) {
+        /* If this is a full handshake with ChannelID then record the handshake
+         * hashes in |ssl->s3->new_session| in case we need them to verify a
+         * ChannelID signature on a resumption of this session in the future. */
+        if (ssl->session == NULL && ssl->s3->tlsext_channel_id_valid) {
           ret = tls1_record_handshake_hashes_for_channel_id(ssl);
           if (ret <= 0) {
             goto end;
@@ -442,7 +442,7 @@
           goto end;
         }
         ssl->state = SSL3_ST_SW_FLUSH;
-        if (ssl->hit) {
+        if (ssl->session != NULL) {
           ssl->s3->tmp.next_state = SSL3_ST_SR_CHANGE;
         } else {
           ssl->s3->tmp.next_state = SSL_ST_OK;
@@ -475,21 +475,31 @@
         ssl3_cleanup_key_block(ssl);
         ssl->method->release_current_message(ssl, 1 /* free_buffer */);
 
+        /* If we aren't retaining peer certificates then we can discard it
+         * now. */
+        if (ssl->s3->new_session != NULL &&
+            ssl->ctx->retain_only_sha256_of_client_certs) {
+          X509_free(ssl->s3->new_session->peer);
+          ssl->s3->new_session->peer = NULL;
+          sk_X509_pop_free(ssl->s3->new_session->cert_chain, X509_free);
+          ssl->s3->new_session->cert_chain = NULL;
+        }
+
+        SSL_SESSION_free(ssl->s3->established_session);
+        if (ssl->session != NULL) {
+          ssl->s3->established_session = SSL_SESSION_up_ref(ssl->session);
+        } else {
+          ssl->s3->established_session = ssl->s3->new_session;
+          ssl->s3->established_session->not_resumable = 0;
+          ssl->s3->new_session = NULL;
+        }
+
         /* remove buffering on output */
         ssl_free_wbio_buffer(ssl);
 
         ssl_handshake_free(ssl->s3->hs);
         ssl->s3->hs = NULL;
 
-        /* If we aren't retaining peer certificates then we can discard it
-         * now. */
-        if (ssl->ctx->retain_only_sha256_of_client_certs) {
-          X509_free(ssl->session->peer);
-          ssl->session->peer = NULL;
-          sk_X509_pop_free(ssl->session->cert_chain, X509_free);
-          ssl->session->cert_chain = NULL;
-        }
-
         ssl->s3->initial_handshake_complete = 1;
 
         ssl_update_cache(ssl, SSL_SESS_CACHE_SERVER);
@@ -653,7 +663,6 @@
     }
   }
 
-  ssl->hit = 0;
   int send_new_ticket = 0;
   switch (ssl_get_prev_session(ssl, &session, &send_new_ticket, &early_ctx)) {
     case ssl_session_success:
@@ -679,6 +688,7 @@
                                            &ems_data, &ems_len) &&
       ems_len == 0;
 
+  int has_session = 0;
   if (session != NULL) {
     if (session->extended_master_secret &&
         !have_extended_master_secret) {
@@ -689,7 +699,7 @@
       goto f_err;
     }
 
-    ssl->hit =
+    has_session =
         /* Only resume if the session's version matches the negotiated version:
          * most clients do not accept a mismatch. */
         ssl->version == session->ssl_version &&
@@ -698,21 +708,20 @@
         have_extended_master_secret == session->extended_master_secret;
   }
 
-  if (ssl->hit) {
-    /* Use the new session. */
-    SSL_SESSION_free(ssl->session);
+  if (has_session) {
+    /* Use the old session. */
     ssl->session = session;
     session = NULL;
-
     ssl->verify_result = ssl->session->verify_result;
   } else {
+    SSL_set_session(ssl, NULL);
     if (!ssl_get_new_session(ssl, 1 /* server */)) {
       goto err;
     }
 
     /* Clear the session ID if we want the session to be single-use. */
     if (!(ssl->ctx->session_cache_mode & SSL_SESS_CACHE_SERVER)) {
-      ssl->session->session_id_length = 0;
+      ssl->s3->new_session->session_id_length = 0;
     }
   }
 
@@ -740,7 +749,7 @@
   }
 
   /* If it is a hit, check that the cipher is in the list. */
-  if (ssl->hit) {
+  if (ssl->session != NULL) {
     size_t j;
     int found_cipher = 0;
     uint32_t id = ssl->session->cipher->id;
@@ -792,7 +801,7 @@
   }
 
   /* Given ciphers and SSL_get_ciphers, we must pick a cipher */
-  if (!ssl->hit) {
+  if (ssl->session == NULL) {
     /* Let cert callback update server certificates if required */
     if (ssl->cert->cert_cb) {
       int rv = ssl->cert->cert_cb(ssl, ssl->cert->cert_cb_arg);
@@ -813,7 +822,7 @@
       OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SHARED_CIPHER);
       goto f_err;
     }
-    ssl->session->cipher = c;
+    ssl->s3->new_session->cipher = c;
     ssl->s3->tmp.new_cipher = c;
 
     /* Determine whether to request a client certificate. */
@@ -843,16 +852,6 @@
     ssl3_free_handshake_buffer(ssl);
   }
 
-  /* we now have the following setup;
-   * client_random
-   * cipher_list        - our prefered list of ciphers
-   * ciphers            - the clients prefered list of ciphers
-   * compression        - basically ignored right now
-   * ssl version is set - sslv3
-   * ssl->session         - The ssl session has been setup.
-   * ssl->hit             - session reuse flag
-   * ssl->tmp.new_cipher  - the new cipher to use. */
-
   ret = 1;
 
   if (0) {
@@ -883,7 +882,8 @@
   /* If this is a resumption and the original handshake didn't support
    * ChannelID then we didn't record the original handshake hashes in the
    * session and so cannot resume with ChannelIDs. */
-  if (ssl->hit && ssl->session->original_handshake_hash_len == 0) {
+  if (ssl->session != NULL &&
+      ssl->session->original_handshake_hash_len == 0) {
     ssl->s3->tlsext_channel_id_valid = 0;
   }
 
@@ -911,13 +911,18 @@
     memcpy(ssl->s3->server_random + SSL3_RANDOM_SIZE - 8, kDowngradeTLS12, 8);
   }
 
+  const SSL_SESSION *session = ssl->s3->new_session;
+  if (ssl->session != NULL) {
+    session = ssl->session;
+  }
+
   CBB cbb, body, session_id;
   if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_SERVER_HELLO) ||
       !CBB_add_u16(&body, ssl->version) ||
       !CBB_add_bytes(&body, ssl->s3->server_random, SSL3_RANDOM_SIZE) ||
       !CBB_add_u8_length_prefixed(&body, &session_id) ||
-      !CBB_add_bytes(&session_id, ssl->session->session_id,
-                     ssl->session->session_id_length) ||
+      !CBB_add_bytes(&session_id, session->session_id,
+                     session->session_id_length) ||
       !CBB_add_u16(&body, ssl_cipher_get_value(ssl->s3->tmp.new_cipher)) ||
       !CBB_add_u8(&body, 0 /* no compression */) ||
       !ssl_add_serverhello_tlsext(ssl, &body) ||
@@ -1010,7 +1015,7 @@
         ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
         goto err;
       }
-      ssl->session->key_exchange_info = DH_num_bits(params);
+      ssl->s3->new_session->key_exchange_info = DH_num_bits(params);
 
       /* Set up DH, generate a key, and emit the public half. */
       DH *dh = DHparams_dup(params);
@@ -1035,7 +1040,7 @@
         ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
         goto err;
       }
-      ssl->session->key_exchange_info = group_id;
+      ssl->s3->new_session->key_exchange_info = group_id;
 
       /* Set up ECDH, generate a key, and emit the public half. */
       if (!SSL_ECDH_CTX_init(&ssl->s3->tmp.ecdh_ctx, group_id) ||
@@ -1291,9 +1296,9 @@
   CBS_init(&certificate_msg, ssl->init_msg, ssl->init_num);
   uint8_t alert;
   STACK_OF(X509) *chain = ssl_parse_cert_chain(
-      ssl, &alert,
-      ssl->ctx->retain_only_sha256_of_client_certs ? ssl->session->peer_sha256
-                                                   : NULL,
+      ssl, &alert, ssl->ctx->retain_only_sha256_of_client_certs
+                       ? ssl->s3->new_session->peer_sha256
+                       : NULL,
       &certificate_msg);
   if (chain == NULL) {
     ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
@@ -1325,7 +1330,7 @@
   } else {
     /* The hash would have been filled in. */
     if (ssl->ctx->retain_only_sha256_of_client_certs) {
-      ssl->session->peer_sha256_valid = 1;
+      ssl->s3->new_session->peer_sha256_valid = 1;
     }
 
     if (ssl_verify_cert_chain(ssl, chain) <= 0) {
@@ -1336,12 +1341,12 @@
     }
   }
 
-  X509_free(ssl->session->peer);
-  ssl->session->peer = sk_X509_shift(chain);
-  ssl->session->verify_result = ssl->verify_result;
+  X509_free(ssl->s3->new_session->peer);
+  ssl->s3->new_session->peer = sk_X509_shift(chain);
+  ssl->s3->new_session->verify_result = ssl->verify_result;
 
-  sk_X509_pop_free(ssl->session->cert_chain, X509_free);
-  ssl->session->cert_chain = chain;
+  sk_X509_pop_free(ssl->s3->new_session->cert_chain, X509_free);
+  ssl->s3->new_session->cert_chain = chain;
   /* Inconsistency alert: cert_chain does *not* include the peer's own
    * certificate, while we do include it in s3_clnt.c */
 
@@ -1402,15 +1407,15 @@
       goto f_err;
     }
 
-    if (!CBS_strdup(&psk_identity, &ssl->session->psk_identity)) {
+    if (!CBS_strdup(&psk_identity, &ssl->s3->new_session->psk_identity)) {
       al = SSL_AD_INTERNAL_ERROR;
       OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
       goto f_err;
     }
 
     /* Look up the key for the identity. */
-    psk_len = ssl->psk_server_callback(ssl, ssl->session->psk_identity, psk,
-                                       sizeof(psk));
+    psk_len = ssl->psk_server_callback(ssl, ssl->s3->new_session->psk_identity,
+                                       psk, sizeof(psk));
     if (psk_len > PSK_MAX_PSK_LEN) {
       OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
       al = SSL_AD_INTERNAL_ERROR;
@@ -1597,12 +1602,14 @@
   }
 
   /* Compute the master secret */
-  ssl->session->master_key_length = tls1_generate_master_secret(
-      ssl, ssl->session->master_key, premaster_secret, premaster_secret_len);
-  if (ssl->session->master_key_length == 0) {
+  ssl->s3->new_session->master_key_length = tls1_generate_master_secret(
+      ssl, ssl->s3->new_session->master_key, premaster_secret,
+      premaster_secret_len);
+  if (ssl->s3->new_session->master_key_length == 0) {
     goto err;
   }
-  ssl->session->extended_master_secret = ssl->s3->tmp.extended_master_secret;
+  ssl->s3->new_session->extended_master_secret =
+      ssl->s3->tmp.extended_master_secret;
 
   OPENSSL_cleanse(premaster_secret, premaster_secret_len);
   OPENSSL_free(premaster_secret);
@@ -1623,7 +1630,7 @@
 static int ssl3_get_cert_verify(SSL *ssl) {
   int al, ret = 0;
   CBS certificate_verify, signature;
-  X509 *peer = ssl->session->peer;
+  X509 *peer = ssl->s3->new_session->peer;
   EVP_PKEY *pkey = NULL;
 
   /* Only RSA and ECDSA client certificates are supported, so a
@@ -1865,8 +1872,9 @@
   /* Serialize the SSL_SESSION to be encoded into the ticket. */
   uint8_t *session = NULL;
   size_t session_len;
-  if (!SSL_SESSION_to_bytes_for_ticket(ssl->session, &session,
-                                       &session_len)) {
+  if (!SSL_SESSION_to_bytes_for_ticket(
+          ssl->session != NULL ? ssl->session : ssl->s3->new_session,
+          &session, &session_len)) {
     return -1;
   }
 
@@ -1881,7 +1889,8 @@
       /* 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. */
-      !CBB_add_u32(&body, ssl->hit ? 0 : ssl->session->timeout) ||
+      !CBB_add_u32(&body, ssl->session != NULL ? 0 :
+                   ssl->s3->new_session->timeout) ||
       !CBB_add_u16_length_prefixed(&body, &ticket)) {
     goto err;
   }