commit 87eab4902d1003983079881d3ee8a66ec2eaccc2
author Steven Valdez <svaldez@google.com> Mon Jun 27 16:34:59 2016 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Fri Jul 29 21:22:46 2016 +0000
tree afaac2e2b48d2bba884059a8572b3cb84a0e630a
parent 163f29af076a2fcc5564f7998ab57e3fb9b197b7

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

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index b1e3b13..6b0c16a 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -713,10 +713,10 @@
 
   assert(ssl->tlsext_hostname != NULL);
 
-  if (!ssl->hit) {
-    assert(ssl->session->tlsext_hostname == NULL);
-    ssl->session->tlsext_hostname = BUF_strdup(ssl->tlsext_hostname);
-    if (!ssl->session->tlsext_hostname) {
+  if (ssl->session == NULL) {
+    assert(ssl->s3->new_session->tlsext_hostname == NULL);
+    ssl->s3->new_session->tlsext_hostname = BUF_strdup(ssl->tlsext_hostname);
+    if (!ssl->s3->new_session->tlsext_hostname) {
       *out_alert = SSL_AD_INTERNAL_ERROR;
       return 0;
     }
@@ -759,11 +759,11 @@
   /* TODO(davidben): SNI should be resolved before resumption. We have the
    * early callback as a replacement, but we should fix the current callback
    * and avoid the need for |SSL_CTX_set_session_id_context|. */
-  if (!ssl->hit) {
-    assert(ssl->session->tlsext_hostname == NULL);
+  if (ssl->session == NULL) {
+    assert(ssl->s3->new_session->tlsext_hostname == NULL);
 
     /* Copy the hostname as a string. */
-    if (!CBS_strdup(&host_name, &ssl->session->tlsext_hostname)) {
+    if (!CBS_strdup(&host_name, &ssl->s3->new_session->tlsext_hostname)) {
       *out_alert = SSL_AD_INTERNAL_ERROR;
       return 0;
     }
@@ -775,9 +775,9 @@
 }
 
 static int ext_sni_add_serverhello(SSL *ssl, CBB *out) {
-  if (ssl->hit ||
+  if (ssl->session != NULL ||
       !ssl->s3->tmp.should_ack_sni ||
-      ssl->session->tlsext_hostname == NULL) {
+      ssl->s3->new_session->tlsext_hostname == NULL) {
     return 1;
   }
 
@@ -1210,8 +1210,8 @@
     return 0;
   }
 
-  if (!CBS_stow(&ocsp_response, &ssl->session->ocsp_response,
-                &ssl->session->ocsp_response_length)) {
+  if (!CBS_stow(&ocsp_response, &ssl->s3->new_session->ocsp_response,
+                &ssl->s3->new_session->ocsp_response_length)) {
     *out_alert = SSL_AD_INTERNAL_ERROR;
     return 0;
   }
@@ -1246,7 +1246,7 @@
 
   if (ssl3_protocol_version(ssl) < TLS1_3_VERSION) {
     /* The extension shouldn't be sent when resuming sessions. */
-    if (ssl->hit) {
+    if (ssl->session != NULL) {
       return 1;
     }
 
@@ -1436,9 +1436,11 @@
   }
 
   /* Session resumption uses the original session information. */
-  if (!ssl->hit &&
-      !CBS_stow(contents, &ssl->session->tlsext_signed_cert_timestamp_list,
-                &ssl->session->tlsext_signed_cert_timestamp_list_length)) {
+  if (ssl->session == NULL &&
+      !CBS_stow(
+          contents,
+          &ssl->s3->new_session->tlsext_signed_cert_timestamp_list,
+          &ssl->s3->new_session->tlsext_signed_cert_timestamp_list_length)) {
     *out_alert = SSL_AD_INTERNAL_ERROR;
     return 0;
   }
@@ -1453,7 +1455,7 @@
 
 static int ext_sct_add_serverhello(SSL *ssl, CBB *out) {
   /* The extension shouldn't be sent when resuming sessions. */
-  if (ssl->hit ||
+  if (ssl->session != NULL ||
       ssl->ctx->signed_cert_timestamp_list_length == 0) {
     return 1;
   }
@@ -2976,7 +2978,7 @@
   static const char kClientIDMagic[] = "TLS Channel ID signature";
   EVP_DigestUpdate(&ctx, kClientIDMagic, sizeof(kClientIDMagic));
 
-  if (ssl->hit) {
+  if (ssl->session != NULL) {
     static const char kResumptionMagic[] = "Resumption";
     EVP_DigestUpdate(&ctx, kResumptionMagic, sizeof(kResumptionMagic));
     if (ssl->session->original_handshake_hash_len == 0) {
@@ -3006,25 +3008,26 @@
 }
 
 /* tls1_record_handshake_hashes_for_channel_id records the current handshake
- * hashes in |ssl->session| so that Channel ID resumptions can sign that
+ * hashes in |ssl->s3->new_session| so that Channel ID resumptions can sign that
  * data. */
 int tls1_record_handshake_hashes_for_channel_id(SSL *ssl) {
   int digest_len;
   /* This function should never be called for a resumed session because the
    * handshake hashes that we wish to record are for the original, full
    * handshake. */
-  if (ssl->hit) {
+  if (ssl->session != NULL) {
     return -1;
   }
 
   digest_len =
-      tls1_handshake_digest(ssl, ssl->session->original_handshake_hash,
-                            sizeof(ssl->session->original_handshake_hash));
+      tls1_handshake_digest(
+          ssl, ssl->s3->new_session->original_handshake_hash,
+          sizeof(ssl->s3->new_session->original_handshake_hash));
   if (digest_len < 0) {
     return -1;
   }
 
-  ssl->session->original_handshake_hash_len = digest_len;
+  ssl->s3->new_session->original_handshake_hash_len = digest_len;
 
   return 1;
 }