Splitting handshake traffic derivation from key change.

This is in preparation for implementing 0-RTT where, like
with client_traffic_secret_0, client_handshake_secret must
be derived slightly earlier than it is used. (The secret is
derived at ServerHello, but used at server Finished.)

Change-Id: I6a186b84829800704a62fda412992ac730422110
Reviewed-on: https://boringssl-review.googlesource.com/12920
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/internal.h

@@ -833,10 +833,9 @@ int tls13_set_traffic_key(SSL *ssl, enum evp_aead_direction_t direction,
                           const uint8_t *traffic_secret,
                           size_t traffic_secret_len);
 
-/* tls13_set_handshake_traffic derives the handshake traffic secret and
- * switches both read and write traffic to it. It returns one on success and
- * zero on error. */
-int tls13_set_handshake_traffic(SSL_HANDSHAKE *hs);
+/* tls13_derive_handshake_secrets derives the handshake traffic secret. It
+ * returns one on success and zero on error. */
+int tls13_derive_handshake_secrets(SSL_HANDSHAKE *hs);
 
 /* tls13_rotate_traffic_key derives the next read or write traffic secret. It
  * returns one on success and zero on error. */
@@ -914,6 +913,8 @@ struct ssl_handshake_st {
 
   size_t hash_len;
   uint8_t secret[EVP_MAX_MD_SIZE];
+  uint8_t client_handshake_secret[EVP_MAX_MD_SIZE];
+  uint8_t server_handshake_secret[EVP_MAX_MD_SIZE];
   uint8_t client_traffic_secret_0[EVP_MAX_MD_SIZE];
   uint8_t server_traffic_secret_0[EVP_MAX_MD_SIZE];
 

ssl/s3_both.c

@@ -149,6 +149,10 @@ void ssl_handshake_free(SSL_HANDSHAKE *hs) {
   }
 
   OPENSSL_cleanse(hs->secret, sizeof(hs->secret));
+  OPENSSL_cleanse(hs->client_handshake_secret,
+                  sizeof(hs->client_handshake_secret));
+  OPENSSL_cleanse(hs->server_handshake_secret,
+                  sizeof(hs->server_handshake_secret));
   OPENSSL_cleanse(hs->client_traffic_secret_0,
                   sizeof(hs->client_traffic_secret_0));
   OPENSSL_cleanse(hs->server_traffic_secret_0,

ssl/tls13_client.c

@@ -311,7 +311,11 @@ static enum ssl_hs_wait_t do_process_server_hello(SSL_HANDSHAKE *hs) {
     return ssl_hs_error;
   }
 
-  if (!tls13_set_handshake_traffic(hs)) {
+  if (!tls13_derive_handshake_secrets(hs) ||
+      !tls13_set_traffic_key(ssl, evp_aead_open, hs->server_handshake_secret,
+                             hs->hash_len) ||
+      !tls13_set_traffic_key(ssl, evp_aead_seal, hs->client_handshake_secret,
+                             hs->hash_len)) {
     return ssl_hs_error;
   }
 

ssl/tls13_enc.c

@@ -185,39 +185,18 @@ static const char kTLS13LabelClientApplicationTraffic[] =
 static const char kTLS13LabelServerApplicationTraffic[] =
     "server application traffic secret";
 
-int tls13_set_handshake_traffic(SSL_HANDSHAKE *hs) {
+int tls13_derive_handshake_secrets(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
-  uint8_t client_traffic_secret[EVP_MAX_MD_SIZE];
-  uint8_t server_traffic_secret[EVP_MAX_MD_SIZE];
-  if (!derive_secret(hs, client_traffic_secret, hs->hash_len,
-                     (const uint8_t *)kTLS13LabelClientHandshakeTraffic,
-                     strlen(kTLS13LabelClientHandshakeTraffic)) ||
-      !ssl_log_secret(ssl, "CLIENT_HANDSHAKE_TRAFFIC_SECRET",
-                      client_traffic_secret, hs->hash_len) ||
-      !derive_secret(hs, server_traffic_secret, hs->hash_len,
-                     (const uint8_t *)kTLS13LabelServerHandshakeTraffic,
-                     strlen(kTLS13LabelServerHandshakeTraffic)) ||
-      !ssl_log_secret(ssl, "SERVER_HANDSHAKE_TRAFFIC_SECRET",
-                      server_traffic_secret, hs->hash_len)) {
-    return 0;
-  }
-
-  if (ssl->server) {
-    if (!tls13_set_traffic_key(ssl, evp_aead_open, client_traffic_secret,
-                               hs->hash_len) ||
-        !tls13_set_traffic_key(ssl, evp_aead_seal, server_traffic_secret,
-                               hs->hash_len)) {
-      return 0;
-    }
-  } else {
-    if (!tls13_set_traffic_key(ssl, evp_aead_open, server_traffic_secret,
-                               hs->hash_len) ||
-        !tls13_set_traffic_key(ssl, evp_aead_seal, client_traffic_secret,
-                               hs->hash_len)) {
-      return 0;
-    }
-  }
-  return 1;
+  return derive_secret(hs, hs->client_handshake_secret, hs->hash_len,
+                       (const uint8_t *)kTLS13LabelClientHandshakeTraffic,
+                       strlen(kTLS13LabelClientHandshakeTraffic)) &&
+         ssl_log_secret(ssl, "CLIENT_HANDSHAKE_TRAFFIC_SECRET",
+                        hs->client_handshake_secret, hs->hash_len) &&
+         derive_secret(hs, hs->server_handshake_secret, hs->hash_len,
+                       (const uint8_t *)kTLS13LabelServerHandshakeTraffic,
+                       strlen(kTLS13LabelServerHandshakeTraffic)) &&
+         ssl_log_secret(ssl, "SERVER_HANDSHAKE_TRAFFIC_SECRET",
+                        hs->server_handshake_secret, hs->hash_len);
 }
 
 static const char kTLS13LabelExporter[] = "exporter master secret";

ssl/tls13_server.c

@@ -404,7 +404,11 @@ err:
 
 static enum ssl_hs_wait_t do_send_encrypted_extensions(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
-  if (!tls13_set_handshake_traffic(hs)) {
+  if (!tls13_derive_handshake_secrets(hs) ||
+      !tls13_set_traffic_key(ssl, evp_aead_open, hs->client_handshake_secret,
+                             hs->hash_len) ||
+      !tls13_set_traffic_key(ssl, evp_aead_seal, hs->server_handshake_secret,
+                             hs->hash_len)) {
     return ssl_hs_error;
   }