Avoid relying on SSL_get_session's behavior during the handshake.

Mid-renegotiation, there are a lot of sets of TLS parameters flying
around. We need to be clear which one we want for each operation. There
were a few parts of TLS 1.2 which were relying on SSL_get_session to
abstract between the resumption session and a new session.

Implement that separately as ssl_handshake_session, so we're free to
avoid SSL_get_session returning an incomplete session mid-renegotiation.

This doesn't fixed the linked Chromium bug, but it is necessary to do
so. (I'm trying to separate the SSL_get_session change from the
dependencies within the library.)

Update-Note: SSL_generate_key_block will now fail mid-handshake. It is
ambiguous which key block to use and, in some cases, we may not even be
able to compute the right key block.

Bug: chromium:1010748
Change-Id: I30c8a683bb506310e37adbd05a28e3b8de6e6836
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/41865
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 8e11ef2..f804928 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3604,11 +3604,13 @@
                                const uint8_t **out_write_iv,
                                size_t *out_iv_len);
 
-// SSL_get_key_block_len returns the length of |ssl|'s key block.
+// SSL_get_key_block_len returns the length of |ssl|'s key block. It is an error
+// to call this function during a handshake.
 OPENSSL_EXPORT size_t SSL_get_key_block_len(const SSL *ssl);
 
 // SSL_generate_key_block generates |out_len| bytes of key material for |ssl|'s
-// current connection state.
+// current connection state. It is an error to call this function during a
+// handshake.
 OPENSSL_EXPORT int SSL_generate_key_block(const SSL *ssl, uint8_t *out,
                                           size_t out_len);
 
diff --git a/ssl/handoff.cc b/ssl/handoff.cc
index fdbac18..977fcc5 100644
--- a/ssl/handoff.cc
+++ b/ssl/handoff.cc
@@ -632,7 +632,7 @@
     case handback_after_session_resumption:
       // The write keys are installed after server Finished, but the client
       // keys must wait for ChangeCipherSpec.
-      if (!tls1_configure_aead(ssl, evp_aead_seal, &key_block, session->cipher,
+      if (!tls1_configure_aead(ssl, evp_aead_seal, &key_block, session,
                                write_iv)) {
         return false;
       }
@@ -642,9 +642,9 @@
       break;
     case handback_after_handshake:
       // The handshake is complete, so both keys are installed.
-      if (!tls1_configure_aead(ssl, evp_aead_seal, &key_block, session->cipher,
+      if (!tls1_configure_aead(ssl, evp_aead_seal, &key_block, session,
                                write_iv) ||
-          !tls1_configure_aead(ssl, evp_aead_open, &key_block, session->cipher,
+          !tls1_configure_aead(ssl, evp_aead_open, &key_block, session,
                                read_iv)) {
         return false;
       }
diff --git a/ssl/handshake.cc b/ssl/handshake.cc
index 27fc3af..7bceb1d 100644
--- a/ssl/handshake.cc
+++ b/ssl/handshake.cc
@@ -441,7 +441,7 @@
   uint8_t finished[EVP_MAX_MD_SIZE];
   size_t finished_len;
   if (!hs->transcript.GetFinishedMAC(finished, &finished_len,
-                                     SSL_get_session(ssl), !ssl->server) ||
+                                     ssl_handshake_session(hs), !ssl->server) ||
       !ssl_hash_message(hs, msg)) {
     return ssl_hs_error;
   }
@@ -484,7 +484,7 @@
 
 bool ssl_send_finished(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
-  const SSL_SESSION *session = SSL_get_session(ssl);
+  const SSL_SESSION *session = ssl_handshake_session(hs);
 
   uint8_t finished[EVP_MAX_MD_SIZE];
   size_t finished_len;
@@ -541,6 +541,13 @@
   return true;
 }
 
+const SSL_SESSION *ssl_handshake_session(const SSL_HANDSHAKE *hs) {
+  if (hs->new_session) {
+    return hs->new_session.get();
+  }
+  return hs->ssl->session.get();
+}
+
 int ssl_run_handshake(SSL_HANDSHAKE *hs, bool *out_early_return) {
   SSL *const ssl = hs->ssl;
   for (;;) {
diff --git a/ssl/internal.h b/ssl/internal.h
index 182b02f..2000409 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1940,6 +1940,11 @@
 bool ssl_send_finished(SSL_HANDSHAKE *hs);
 bool ssl_output_cert_chain(SSL_HANDSHAKE *hs);
 
+// ssl_handshake_session returns the |SSL_SESSION| corresponding to the current
+// handshake. Note, in TLS 1.2 resumptions, this session is immutable.
+const SSL_SESSION *ssl_handshake_session(const SSL_HANDSHAKE *hs);
+
+
 // SSLKEYLOGFILE functions.
 
 // ssl_log_secret logs |secret| with label |label|, if logging is enabled for
@@ -2921,13 +2926,14 @@
 // determined by |direction|) using the keys generated by the TLS KDF. The
 // |key_block_cache| argument is used to store the generated key block, if
 // empty. Otherwise it's assumed that the key block is already contained within
-// it. Returns one on success or zero on error.
-int tls1_configure_aead(SSL *ssl, evp_aead_direction_t direction,
-                        Array<uint8_t> *key_block_cache,
-                        const SSL_CIPHER *cipher,
-                        Span<const uint8_t> iv_override);
+// it. It returns true on success or false on error.
+bool tls1_configure_aead(SSL *ssl, evp_aead_direction_t direction,
+                         Array<uint8_t> *key_block_cache,
+                         const SSL_SESSION *session,
+                         Span<const uint8_t> iv_override);
 
-int tls1_change_cipher_state(SSL_HANDSHAKE *hs, evp_aead_direction_t direction);
+bool tls1_change_cipher_state(SSL_HANDSHAKE *hs,
+                              evp_aead_direction_t direction);
 int tls1_generate_master_secret(SSL_HANDSHAKE *hs, uint8_t *out,
                                 Span<const uint8_t> premaster);
 
diff --git a/ssl/t1_enc.cc b/ssl/t1_enc.cc
index 73b6544..d8b6ea2 100644
--- a/ssl/t1_enc.cc
+++ b/ssl/t1_enc.cc
@@ -189,21 +189,36 @@
   return true;
 }
 
-int tls1_configure_aead(SSL *ssl, evp_aead_direction_t direction,
-                        Array<uint8_t> *key_block_cache,
-                        const SSL_CIPHER *cipher,
-                        Span<const uint8_t> iv_override) {
+static bool generate_key_block(const SSL *ssl, Span<uint8_t> out,
+                               const SSL_SESSION *session) {
+  auto master_key =
+      MakeConstSpan(session->master_key, session->master_key_length);
+  static const char kLabel[] = "key expansion";
+  auto label = MakeConstSpan(kLabel, sizeof(kLabel) - 1);
+
+  const EVP_MD *digest = ssl_session_get_digest(session);
+  // Note this function assumes that |session|'s key material corresponds to
+  // |ssl->s3->client_random| and |ssl->s3->server_random|.
+  return tls1_prf(digest, out, master_key, label, ssl->s3->server_random,
+                  ssl->s3->client_random);
+}
+
+bool tls1_configure_aead(SSL *ssl, evp_aead_direction_t direction,
+                         Array<uint8_t> *key_block_cache,
+                         const SSL_SESSION *session,
+                         Span<const uint8_t> iv_override) {
   size_t mac_secret_len, key_len, iv_len;
-  if (!get_key_block_lengths(ssl, &mac_secret_len, &key_len, &iv_len, cipher)) {
-    return 0;
+  if (!get_key_block_lengths(ssl, &mac_secret_len, &key_len, &iv_len,
+                             session->cipher)) {
+    return false;
   }
 
   // Ensure that |key_block_cache| is set up.
   const size_t key_block_size = 2 * (mac_secret_len + key_len + iv_len);
   if (key_block_cache->empty()) {
     if (!key_block_cache->Init(key_block_size) ||
-        !SSL_generate_key_block(ssl, key_block_cache->data(), key_block_size)) {
-      return 0;
+        !generate_key_block(ssl, MakeSpan(*key_block_cache), session)) {
+      return false;
     }
   }
   assert(key_block_cache->size() == key_block_size);
@@ -224,15 +239,16 @@
 
   if (!iv_override.empty()) {
     if (iv_override.size() != iv_len) {
-      return 0;
+      return false;
     }
     iv = iv_override;
   }
 
-  UniquePtr<SSLAEADContext> aead_ctx = SSLAEADContext::Create(
-      direction, ssl->version, SSL_is_dtls(ssl), cipher, key, mac_secret, iv);
+  UniquePtr<SSLAEADContext> aead_ctx =
+      SSLAEADContext::Create(direction, ssl->version, SSL_is_dtls(ssl),
+                             session->cipher, key, mac_secret, iv);
   if (!aead_ctx) {
-    return 0;
+    return false;
   }
 
   if (direction == evp_aead_open) {
@@ -246,10 +262,10 @@
                                       /*secret_for_quic=*/{});
 }
 
-int tls1_change_cipher_state(SSL_HANDSHAKE *hs,
-                             evp_aead_direction_t direction) {
+bool tls1_change_cipher_state(SSL_HANDSHAKE *hs,
+                              evp_aead_direction_t direction) {
   return tls1_configure_aead(hs->ssl, direction, &hs->key_block,
-                             hs->new_cipher, {});
+                             ssl_handshake_session(hs), {});
 }
 
 int tls1_generate_master_secret(SSL_HANDSHAKE *hs, uint8_t *out,
@@ -286,6 +302,11 @@
 using namespace bssl;
 
 size_t SSL_get_key_block_len(const SSL *ssl) {
+  // See |SSL_generate_key_block|.
+  if (SSL_in_init(ssl)) {
+    return 0;
+  }
+
   size_t mac_secret_len, key_len, fixed_iv_len;
   if (!get_key_block_lengths(ssl, &mac_secret_len, &key_len, &fixed_iv_len,
                              SSL_get_current_cipher(ssl))) {
@@ -297,16 +318,16 @@
 }
 
 int SSL_generate_key_block(const SSL *ssl, uint8_t *out, size_t out_len) {
-  const SSL_SESSION *session = SSL_get_session(ssl);
-  auto out_span = MakeSpan(out, out_len);
-  auto master_key =
-      MakeConstSpan(session->master_key, session->master_key_length);
-  static const char kLabel[] = "key expansion";
-  auto label = MakeConstSpan(kLabel, sizeof(kLabel) - 1);
+  // Which cipher state to use is ambiguous during a handshake. In particular,
+  // there are points where read and write states are from different epochs.
+  // During a handshake, before ChangeCipherSpec, the encryption states may not
+  // match |ssl->s3->client_random| and |ssl->s3->server_random|.
+  if (SSL_in_init(ssl)) {
+    OPENSSL_PUT_ERROR(SSL, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
+    return 0;
+  }
 
-  const EVP_MD *digest = ssl_session_get_digest(session);
-  return tls1_prf(digest, out_span, master_key, label, ssl->s3->server_random,
-                  ssl->s3->client_random);
+  return generate_key_block(ssl, MakeSpan(out, out_len), SSL_get_session(ssl));
 }
 
 int SSL_export_keying_material(SSL *ssl, uint8_t *out, size_t out_len,