Add support for logging out TLSv1.3 secrets

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2287)

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index f027f1a7d4d9ddd8f82e6d41e280c252ae762149..42d49d0ca8dca9052400488df202029b8c6087af 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -4441,32 +4441,16 @@ int ssl_log_rsa_client_key_exchange(SSL *ssl,
                           premaster_len);
 }
 
-int ssl_log_master_secret(SSL *ssl,
-                          const uint8_t *client_random,
-                          size_t client_random_len,
-                          const uint8_t *master,
-                          size_t master_len)
+int ssl_log_secret(SSL *ssl,
+                   const char *label,
+                   const uint8_t *secret,
+                   size_t secret_len)
 {
-    /*
-     * TLSv1.3 changes the derivation of the master secret compared to earlier
-     * TLS versions, meaning that logging it out is less useful. Instead we
-     * want to log out other secrets: specifically, the handshake and
-     * application traffic secrets. For this reason, if this function is called
-     * for TLSv1.3 we don't bother logging, and just return success
-     * immediately.
-     */
-    if (SSL_IS_TLS13(ssl)) return 1;
-
-    if (client_random_len != 32) {
-        SSLerr(SSL_F_SSL_LOG_MASTER_SECRET, ERR_R_INTERNAL_ERROR);
-        return 0;
-    }
-
-    return nss_keylog_int("CLIENT_RANDOM",
+    return nss_keylog_int(label,
                           ssl,
-                          client_random,
-                          client_random_len,
-                          master,
-                          master_len);
+                          ssl->s3->client_random,
+                          SSL3_RANDOM_SIZE,
+                          secret,
+                          secret_len);
 }
 

diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 26580b06a737d62256daa76ca6d41c9e4d33406d..53a33e9fdfe80f92ec354808cd8f734ff313eeab 100644 (file)
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -2287,13 +2287,19 @@ __owur int ssl_log_rsa_client_key_exchange(SSL *ssl,
                                            const uint8_t *premaster,
                                            size_t premaster_len);
 
-/* ssl_log_master_secret logs |master| to the SSL_CTX associated with |ssl|, if
- * logging is enabled. It returns one on success and zero on failure. The entry
- * is identified by |client_random|.
+/*
+ * ssl_log_secret logs |secret| to the SSL_CTX associated with |ssl|, if
+ * logging is available. It returns one on success and zero on failure. It tags
+ * the entry with |label|.
  */
-__owur int ssl_log_master_secret(SSL *ssl, const uint8_t *client_random,
-                                 size_t client_random_len,
-                                 const uint8_t *master, size_t master_len);
+__owur int ssl_log_secret(SSL *ssl, const char *label,
+                          const uint8_t *secret, size_t secret_len);
+
+#define MASTER_SECRET_LABEL "CLIENT_RANDOM"
+#define CLIENT_HANDSHAKE_LABEL "CLIENT_HANDSHAKE_TRAFFIC_SECRET"
+#define SERVER_HANDSHAKE_LABEL "SERVER_HANDSHAKE_TRAFFIC_SECRET"
+#define CLIENT_APPLICATION_LABEL "CLIENT_TRAFFIC_SECRET_0"
+#define SERVER_APPLICATION_LABEL "SERVER_TRAFFIC_SECRET_0"
 
 /* s3_cbc.c */
 __owur char ssl3_cbc_record_digest_supported(const EVP_MD_CTX *ctx);

diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index e21a1027daee70d6bd3410695e74959d825653c5..4b021f906e7e7fde78949aec5bdca077f9961595 100644 (file)
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -470,10 +470,13 @@ int tls_construct_finished(SSL *s, WPACKET *pkt)
         goto err;
     }
 
-    /* Log the master secret, if logging is enabled. */
-    if (!ssl_log_master_secret(s, s->s3->client_random, SSL3_RANDOM_SIZE,
-                               s->session->master_key,
-                               s->session->master_key_length))
+    /*
+     * Log the master secret, if logging is enabled. We don't log it for
+     * TLSv1.3: there's a different key schedule for that.
+     */
+    if (!SSL_IS_TLS13(s) && !ssl_log_secret(s, MASTER_SECRET_LABEL,
+                                            s->session->master_key,
+                                            s->session->master_key_length))
         return 0;
 
     /*

diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c
index 7c217c11d31cb62f116ca3fa1a87398633cf032e..0d29dae0428c70d047475565ec7e96f6755f201b 100644 (file)
--- a/ssl/tls13_enc.c
+++ b/ssl/tls13_enc.c
@@ -261,6 +261,7 @@ int tls13_change_cipher_state(SSL *s, int which)
     unsigned char *hash = hashval;
     unsigned char *insecret;
     unsigned char *finsecret = NULL;
+    const char *log_label = NULL;
     EVP_CIPHER_CTX *ciph_ctx;
     const EVP_CIPHER *ciph = s->s3->tmp.new_sym_enc;
     size_t ivlen, keylen, finsecretlen = 0;
@@ -306,10 +307,12 @@ int tls13_change_cipher_state(SSL *s, int which)
             finsecretlen = EVP_MD_size(ssl_handshake_md(s));
             label = client_handshake_traffic;
             labellen = sizeof(client_handshake_traffic) - 1;
+            log_label = CLIENT_HANDSHAKE_LABEL;
         } else {
             insecret = s->master_secret;
             label = client_application_traffic;
             labellen = sizeof(client_application_traffic) - 1;
+            log_label = CLIENT_APPLICATION_LABEL;
             /*
              * For this we only use the handshake hashes up until the server
              * Finished hash. We do not include the client's Finished, which is
@@ -325,10 +328,12 @@ int tls13_change_cipher_state(SSL *s, int which)
             finsecretlen = EVP_MD_size(ssl_handshake_md(s));
             label = server_handshake_traffic;
             labellen = sizeof(server_handshake_traffic) - 1;
+            log_label = SERVER_HANDSHAKE_LABEL;
         } else {
             insecret = s->master_secret;
             label = server_application_traffic;
             labellen = sizeof(server_application_traffic) - 1;
+            log_label = SERVER_APPLICATION_LABEL;
         }
     }
 
@@ -370,6 +375,11 @@ int tls13_change_cipher_state(SSL *s, int which)
     keylen = EVP_CIPHER_key_length(ciph);
     ivlen = EVP_CIPHER_iv_length(ciph);
 
+    if (!ssl_log_secret(s, log_label, secret, hashlen)) {
+        SSLerr(SSL_F_TLS13_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
     if (!tls13_derive_key(s, secret, key, keylen)
             || !tls13_derive_iv(s, secret, iv, ivlen)
             || (finsecret != NULL && !tls13_derive_finishedkey(s,