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/ssl_lib.c

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 4da5233..c161950 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -999,17 +999,25 @@
 uint32_t SSL_get_mode(const SSL *ssl) { return ssl->mode; }
 
 X509 *SSL_get_peer_certificate(const SSL *ssl) {
-  if (ssl == NULL || ssl->session == NULL || ssl->session->peer == NULL) {
+  if (ssl == NULL) {
     return NULL;
   }
-  return X509_up_ref(ssl->session->peer);
+  SSL_SESSION *session = SSL_get_session(ssl);
+  if (session == NULL || session->peer == NULL) {
+    return NULL;
+  }
+  return X509_up_ref(session->peer);
 }
 
 STACK_OF(X509) *SSL_get_peer_cert_chain(const SSL *ssl) {
-  if (ssl == NULL || ssl->session == NULL) {
+  if (ssl == NULL) {
     return NULL;
   }
-  return ssl->session->cert_chain;
+  SSL_SESSION *session = SSL_get_session(ssl);
+  if (session == NULL) {
+    return NULL;
+  }
+  return session->cert_chain;
 }
 
 int SSL_get_tls_unique(const SSL *ssl, uint8_t *out, size_t *out_len,
@@ -1019,7 +1027,7 @@
    * https://tools.ietf.org/html/rfc5929#section-3.1. */
   const uint8_t *finished = ssl->s3->previous_client_finished;
   size_t finished_len = ssl->s3->previous_client_finished_len;
-  if (ssl->hit) {
+  if (ssl->session != NULL) {
     /* tls-unique is broken for resumed sessions unless EMS is used. */
     if (!ssl->session->extended_master_secret) {
       goto err;
@@ -1447,13 +1455,14 @@
 uint16_t SSL_get_curve_id(const SSL *ssl) {
   /* TODO(davidben): This checks the wrong session if there is a renegotiation in
    * progress. */
-  if (ssl->session == NULL ||
-      ssl->session->cipher == NULL ||
-      !SSL_CIPHER_is_ECDHE(ssl->session->cipher)) {
+  SSL_SESSION *session = SSL_get_session(ssl);
+  if (session == NULL ||
+      session->cipher == NULL ||
+      !SSL_CIPHER_is_ECDHE(session->cipher)) {
     return 0;
   }
 
-  return (uint16_t)ssl->session->key_exchange_info;
+  return (uint16_t)session->key_exchange_info;
 }
 
 int SSL_CTX_set_tmp_dh(SSL_CTX *ctx, const DH *dh) {
@@ -1679,18 +1688,19 @@
     return ssl->tlsext_hostname;
   }
 
-  if (ssl->session == NULL) {
+  SSL_SESSION *session = SSL_get_session(ssl);
+  if (session == NULL) {
     return NULL;
   }
-  return ssl->session->tlsext_hostname;
+  return session->tlsext_hostname;
 }
 
 int SSL_get_servername_type(const SSL *ssl) {
-  if (ssl->session != NULL && ssl->session->tlsext_hostname != NULL) {
-    return TLSEXT_NAMETYPE_host_name;
+  SSL_SESSION *session = SSL_get_session(ssl);
+  if (session == NULL || session->tlsext_hostname == NULL) {
+    return -1;
   }
-
-  return -1;
+  return TLSEXT_NAMETYPE_host_name;
 }
 
 void SSL_CTX_enable_signed_cert_timestamps(SSL_CTX *ctx) {
@@ -1713,7 +1723,7 @@
 
 void SSL_get0_signed_cert_timestamp_list(const SSL *ssl, const uint8_t **out,
                                          size_t *out_len) {
-  SSL_SESSION *session = ssl->session;
+  SSL_SESSION *session = SSL_get_session(ssl);
 
   *out_len = 0;
   *out = NULL;
@@ -1727,7 +1737,7 @@
 
 void SSL_get0_ocsp_response(const SSL *ssl, const uint8_t **out,
                             size_t *out_len) {
-  SSL_SESSION *session = ssl->session;
+  SSL_SESSION *session = SSL_get_session(ssl);
 
   *out_len = 0;
   *out = NULL;
@@ -2044,7 +2054,7 @@
 void ssl_update_cache(SSL *ssl, int mode) {
   SSL_CTX *ctx = ssl->initial_ctx;
   /* Never cache sessions with empty session IDs. */
-  if (ssl->session->session_id_length == 0 ||
+  if (ssl->s3->established_session->session_id_length == 0 ||
       (ctx->session_cache_mode & mode) != mode) {
     return;
   }
@@ -2056,14 +2066,16 @@
   /* A client may see new sessions on abbreviated handshakes if the server
    * decides to renew the ticket. Once the handshake is completed, it should be
    * inserted into the cache. */
-  if (!ssl->hit || (!ssl->server && ssl->tlsext_ticket_expected)) {
+  if (ssl->s3->established_session != ssl->session ||
+      (!ssl->server && ssl->tlsext_ticket_expected)) {
     if (use_internal_cache) {
-      SSL_CTX_add_session(ctx, ssl->session);
+      SSL_CTX_add_session(ctx, ssl->s3->established_session);
     }
     if (ctx->new_session_cb != NULL &&
-        !ctx->new_session_cb(ssl, SSL_SESSION_up_ref(ssl->session))) {
+        !ctx->new_session_cb(ssl, SSL_SESSION_up_ref(
+            ssl->s3->established_session))) {
       /* |new_session_cb|'s return value signals whether it took ownership. */
-      SSL_SESSION_free(ssl->session);
+      SSL_SESSION_free(ssl->s3->established_session);
     }
   }
 
@@ -2161,7 +2173,7 @@
 }
 
 int SSL_session_reused(const SSL *ssl) {
-  return ssl->hit;
+  return ssl->session != NULL;
 }
 
 const COMP_METHOD *SSL_get_current_compression(SSL *ssl) { return NULL; }
@@ -2384,13 +2396,14 @@
 unsigned SSL_get_dhe_group_size(const SSL *ssl) {
   /* TODO(davidben): This checks the wrong session if there is a renegotiation in
    * progress. */
-  if (ssl->session == NULL ||
-      ssl->session->cipher == NULL ||
-      !SSL_CIPHER_is_DHE(ssl->session->cipher)) {
+  SSL_SESSION *session = SSL_get_session(ssl);
+  if (session == NULL ||
+      session->cipher == NULL ||
+      !SSL_CIPHER_is_DHE(session->cipher)) {
     return 0;
   }
 
-  return ssl->session->key_exchange_info;
+  return session->key_exchange_info;
 }
 
 int SSL_CTX_use_psk_identity_hint(SSL_CTX *ctx, const char *identity_hint) {
@@ -2445,11 +2458,14 @@
 }
 
 const char *SSL_get_psk_identity(const SSL *ssl) {
-  if (ssl == NULL || ssl->session == NULL) {
+  if (ssl == NULL) {
     return NULL;
   }
-
-  return ssl->session->psk_identity;
+  SSL_SESSION *session = SSL_get_session(ssl);
+  if (session == NULL) {
+    return NULL;
+  }
+  return session->psk_identity;
 }
 
 void SSL_set_psk_client_callback(
@@ -2871,8 +2887,6 @@
     ssl->session = NULL;
   }
 
-  ssl->hit = 0;
-
   /* SSL_clear may be called before or after the |ssl| is initialized in either
    * accept or connect state. In the latter case, SSL_clear should preserve the
    * half and reset |ssl->state| accordingly. */