Backed out changeset 3e803b9a08c8 NSS_TLS13_DRAFT19_BRANCH
authorMartin Thomson <martin.thomson@gmail.com>
Sun, 01 Oct 2017 09:34:29 +1100
branchNSS_TLS13_DRAFT19_BRANCH
changeset 13615 76d5048d2b8a35308808446de43e85d6cf0cb283
parent 13614 3e803b9a08c8820f035cfe9189409c101144f8a9
child 14033 a0cb1758c33fc179135d5688851105e8812375ee
push id2396
push usermartin.thomson@gmail.com
push dateSat, 30 Sep 2017 22:35:01 +0000
backs out3e803b9a08c8820f035cfe9189409c101144f8a9
Backed out changeset 3e803b9a08c8
gtests/ssl_gtest/manifest.mn
gtests/ssl_gtest/ssl_gtest.gyp
gtests/ssl_gtest/ssl_keylog_unittest.cc
lib/ssl/ssl3con.c
lib/ssl/sslimpl.h
lib/ssl/tls13con.c
--- a/gtests/ssl_gtest/manifest.mn
+++ b/gtests/ssl_gtest/manifest.mn
@@ -26,17 +26,16 @@ CPPSRCS = \
       ssl_ems_unittest.cc \
       ssl_exporter_unittest.cc \
       ssl_extension_unittest.cc \
       ssl_fragment_unittest.cc \
       ssl_fuzz_unittest.cc \
       ssl_gather_unittest.cc \
       ssl_gtest.cc \
       ssl_hrr_unittest.cc \
-      ssl_keylog_unittest.cc \
       ssl_loopback_unittest.cc \
       ssl_misc_unittest.cc \
       ssl_record_unittest.cc \
       ssl_resumption_unittest.cc \
       ssl_skip_unittest.cc \
       ssl_staticrsa_unittest.cc \
       ssl_v2_client_hello_unittest.cc \
       ssl_version_unittest.cc \
--- a/gtests/ssl_gtest/ssl_gtest.gyp
+++ b/gtests/ssl_gtest/ssl_gtest.gyp
@@ -27,17 +27,16 @@
         'ssl_ems_unittest.cc',
         'ssl_exporter_unittest.cc',
         'ssl_extension_unittest.cc',
         'ssl_fuzz_unittest.cc',
         'ssl_fragment_unittest.cc',
         'ssl_gather_unittest.cc',
         'ssl_gtest.cc',
         'ssl_hrr_unittest.cc',
-        'ssl_keylog_unittest.cc',
         'ssl_loopback_unittest.cc',
         'ssl_misc_unittest.cc',
         'ssl_record_unittest.cc',
         'ssl_resumption_unittest.cc',
         'ssl_skip_unittest.cc',
         'ssl_staticrsa_unittest.cc',
         'ssl_v2_client_hello_unittest.cc',
         'ssl_version_unittest.cc',
deleted file mode 100644
--- a/gtests/ssl_gtest/ssl_keylog_unittest.cc
+++ /dev/null
@@ -1,88 +0,0 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim: set ts=2 et sw=2 tw=80: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this file,
- * You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#include <fstream>
-#include <sstream>
-
-#include "gtest_utils.h"
-#include "tls_connect.h"
-
-namespace nss_test {
-
-static const char *keylog_file_path = "keylog.txt";
-
-class KeyLogFileTest : public TlsConnectGeneric {
- public:
-  void SetUp() {
-    TlsConnectTestBase::SetUp();
-    remove(keylog_file_path);
-    setenv("SSLKEYLOGFILE", keylog_file_path, 1);
-  }
-
-  void CheckKeyLog() {
-    std::ifstream f(keylog_file_path);
-    std::map<std::string, size_t> labels;
-    std::string last_client_random;
-    for (std::string line; std::getline(f, line);) {
-      if (line[0] == '#') {
-        continue;
-      }
-
-      std::istringstream iss(line);
-      std::string label, client_random, secret;
-      iss >> label >> client_random >> secret;
-
-      ASSERT_EQ(1U, client_random.size());
-      ASSERT_TRUE(last_client_random.empty() ||
-                  last_client_random == client_random);
-      last_client_random = client_random;
-      labels[label]++;
-    }
-
-    if (version_ < SSL_LIBRARY_VERSION_TLS_1_3) {
-      ASSERT_EQ(1U, labels["CLIENT_RANDOM"]);
-    } else {
-      ASSERT_EQ(1U, labels["CLIENT_EARLY_TRAFFIC_SECRET"]);
-      ASSERT_EQ(1U, labels["CLIENT_HANDSHAKE_TRAFFIC_SECRET"]);
-      ASSERT_EQ(1U, labels["SERVER_HANDSHAKE_TRAFFIC_SECRET"]);
-      ASSERT_EQ(1U, labels["CLIENT_TRAFFIC_SECRET_0"]);
-      ASSERT_EQ(1U, labels["SERVER_TRAFFIC_SECRET_0"]);
-      ASSERT_EQ(1U, labels["EXPORTER_SECRET"]);
-    }
-  }
-
-  void ConnectAndCheck() {
-    Connect();
-    CheckKeyLog();
-    exit(0);
-  }
-};
-
-// Tests are run in a separate process to ensure that NSS is not initialized yet
-// and can process the SSLKEYLOGFILE environment variable.
-
-TEST_P(KeyLogFileTest, KeyLogFile) {
-  testing::GTEST_FLAG(death_test_style) = "threadsafe";
-
-  ASSERT_EXIT(ConnectAndCheck(), ::testing::ExitedWithCode(0), "");
-}
-
-INSTANTIATE_TEST_CASE_P(
-    KeyLogFileDTLS12, KeyLogFileTest,
-    ::testing::Combine(TlsConnectTestBase::kTlsVariantsDatagram,
-                       TlsConnectTestBase::kTlsV11V12));
-INSTANTIATE_TEST_CASE_P(
-    KeyLogFileTLS12, KeyLogFileTest,
-    ::testing::Combine(TlsConnectTestBase::kTlsVariantsStream,
-                       TlsConnectTestBase::kTlsV10ToV12));
-#ifndef NSS_DISABLE_TLS_1_3
-INSTANTIATE_TEST_CASE_P(
-    KeyLogFileTLS13, KeyLogFileTest,
-    ::testing::Combine(TlsConnectTestBase::kTlsVariantsStream,
-                       TlsConnectTestBase::kTlsV13));
-#endif
-
-}  // namespace nss_test
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -11128,76 +11128,74 @@ ssl3_SendNextProto(sslSocket *ss)
     }
     rv = ssl3_AppendHandshakeVariable(ss, padding, padding_len, 1);
     if (rv != SECSuccess) {
         return rv; /* error code set by AppendHandshake */
     }
     return rv;
 }
 
-/* called from ssl3_SendFinished and tls13_DeriveSecretWrap.
+/* called from ssl3_SendFinished
  *
  * This function is simply a debugging aid and therefore does not return a
  * SECStatus. */
-void
-ssl3_RecordKeyLog(sslSocket *ss, const char *label, PK11SymKey *secret)
+static void
+ssl3_RecordKeyLog(sslSocket *ss)
 {
 #ifdef NSS_ALLOW_SSLKEYLOGFILE
     SECStatus rv;
     SECItem *keyData;
-    /* Longest label is "CLIENT_HANDSHAKE_TRAFFIC_SECRET", master secret is 48
-     * bytes which happens to be the largest in TLS 1.3 as well (SHA384).
-     * Maximum line length: "CLIENT_HANDSHAKE_TRAFFIC_SECRET" (31) + " " (1) +
-     * client_random (32*2) + " " (1) +
-     * traffic_secret (48*2) + "\n" (1) = 194. */
-    char buf[200];
-    unsigned int offset, len;
+    char buf[14 /* "CLIENT_RANDOM " */ +
+             SSL3_RANDOM_LENGTH * 2 /* client_random */ +
+             1 /* " " */ +
+             48 * 2 /* master secret */ +
+             1 /* new line */];
+    unsigned int j;
 
     PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss));
 
     if (!ssl_keylog_iob)
         return;
 
-    rv = PK11_ExtractKeyValue(secret);
+    rv = PK11_ExtractKeyValue(ss->ssl3.cwSpec->master_secret);
     if (rv != SECSuccess)
         return;
 
+    ssl_GetSpecReadLock(ss);
+
     /* keyData does not need to be freed. */
-    keyData = PK11_GetKeyData(secret);
-    if (!keyData || !keyData->data)
+    keyData = PK11_GetKeyData(ss->ssl3.cwSpec->master_secret);
+    if (!keyData || !keyData->data || keyData->len != 48) {
+        ssl_ReleaseSpecReadLock(ss);
         return;
-
-    len = strlen(label) + 1 +          /* label + space */
-          SSL3_RANDOM_LENGTH * 2 + 1 + /* client random (hex) + space */
-          keyData->len * 2 + 1;        /* secret (hex) + newline */
-    PORT_Assert(len <= sizeof(buf));
-    if (len > sizeof(buf))
-        return;
+    }
 
     /* https://developer.mozilla.org/en/NSS_Key_Log_Format */
 
     /* There could be multiple, concurrent writers to the
      * keylog, so we have to do everything in a single call to
      * fwrite. */
 
-    strcpy(buf, label);
-    offset = strlen(label);
-    buf[offset++] += ' ';
-    hexEncode(buf + offset, ss->ssl3.hs.client_random.rand, SSL3_RANDOM_LENGTH);
-    offset += SSL3_RANDOM_LENGTH * 2;
-    buf[offset++] = ' ';
-    hexEncode(buf + offset, keyData->data, keyData->len);
-    offset += keyData->len * 2;
-    buf[offset++] = '\n';
-
-    PORT_Assert(offset == len);
-
-    if (fwrite(buf, len, 1, ssl_keylog_iob) != 1)
+    memcpy(buf, "CLIENT_RANDOM ", 14);
+    j = 14;
+    hexEncode(buf + j, ss->ssl3.hs.client_random.rand, SSL3_RANDOM_LENGTH);
+    j += SSL3_RANDOM_LENGTH * 2;
+    buf[j++] = ' ';
+    hexEncode(buf + j, keyData->data, 48);
+    j += 48 * 2;
+    buf[j++] = '\n';
+
+    PORT_Assert(j == sizeof(buf));
+
+    ssl_ReleaseSpecReadLock(ss);
+
+    if (fwrite(buf, sizeof(buf), 1, ssl_keylog_iob) != 1)
         return;
     fflush(ssl_keylog_iob);
+    return;
 #endif
 }
 
 /* called from ssl3_SendClientSecondRound
  *             ssl3_HandleClientHello
  *             ssl3_HandleFinished
  */
 static SECStatus
@@ -11254,17 +11252,17 @@ ssl3_SendFinished(sslSocket *ss, PRInt32
         if (rv != SECSuccess)
             goto fail; /* err set by AppendHandshake. */
     }
     rv = ssl3_FlushHandshake(ss, flags);
     if (rv != SECSuccess) {
         goto fail; /* error code set by ssl3_FlushHandshake */
     }
 
-    ssl3_RecordKeyLog(ss, "CLIENT_RANDOM", ss->ssl3.cwSpec->master_secret);
+    ssl3_RecordKeyLog(ss);
 
     return SECSuccess;
 
 fail:
     return rv;
 }
 
 /* wrap the master secret, and put it into the SID.
--- a/lib/ssl/sslimpl.h
+++ b/lib/ssl/sslimpl.h
@@ -1868,19 +1868,16 @@ extern void SSL_AtomicIncrementLong(long
 SECStatus ssl3_ApplyNSSPolicy(void);
 
 extern SECStatus
 ssl3_TLSPRFWithMasterSecret(sslSocket *ss, ssl3CipherSpec *spec,
                             const char *label, unsigned int labelLen,
                             const unsigned char *val, unsigned int valLen,
                             unsigned char *out, unsigned int outLen);
 
-extern void
-ssl3_RecordKeyLog(sslSocket *ss, const char *label, PK11SymKey *secret);
-
 PRBool ssl_AlpnTagAllowed(const sslSocket *ss, const SECItem *tag);
 
 #ifdef TRACE
 #define SSL_TRACE(msg) ssl_Trace msg
 #else
 #define SSL_TRACE(msg)
 #endif
 
--- a/lib/ssl/tls13con.c
+++ b/lib/ssl/tls13con.c
@@ -62,17 +62,16 @@ tls13_SendCertificateVerify(sslSocket *s
 static SECStatus tls13_HandleCertificateVerify(
     sslSocket *ss, PRUint8 *b, PRUint32 length);
 static SECStatus tls13_RecoverWrappedSharedSecret(sslSocket *ss,
                                                   sslSessionID *sid);
 static SECStatus
 tls13_DeriveSecretWrap(sslSocket *ss, PK11SymKey *key,
                        const char *prefix,
                        const char *suffix,
-                       const char *keylogLabel,
                        PK11SymKey **dest);
 static SECStatus
 tls13_DeriveSecret(sslSocket *ss, PK11SymKey *key,
                    const char *label,
                    unsigned int labelLen,
                    const SSL3Hashes *hashes,
                    PK11SymKey **dest);
 static SECStatus tls13_SendEndOfEarlyData(sslSocket *ss);
@@ -116,23 +115,16 @@ const char kHkdfLabelApplicationTrafficS
 const char kHkdfLabelFinishedSecret[] = "finished";
 const char kHkdfLabelResumptionMasterSecret[] = "res master";
 const char kHkdfLabelExporterMasterSecret[] = "exp master";
 const char kHkdfLabelResumption[] = "resumption";
 const char kHkdfPurposeKey[] = "key";
 const char kHkdfPurposeIv[] = "iv";
 const char kKeyPhaseCleartext[] = "clear";
 
-const char keylogLabelClientEarlyTrafficSecret[] = "CLIENT_EARLY_TRAFFIC_SECRET";
-const char keylogLabelClientHsTrafficSecret[] = "CLIENT_HANDSHAKE_TRAFFIC_SECRET";
-const char keylogLabelServerHsTrafficSecret[] = "SERVER_HANDSHAKE_TRAFFIC_SECRET";
-const char keylogLabelClientTrafficSecret[] = "CLIENT_TRAFFIC_SECRET_0";
-const char keylogLabelServerTrafficSecret[] = "SERVER_TRAFFIC_SECRET_0";
-const char keylogLabelExporterSecret[] = "EXPORTER_SECRET";
-
 #define TRAFFIC_SECRET(ss, dir, name) ((ss->sec.isServer ^            \
                                         (dir == CipherSpecWrite))     \
                                            ? ss->ssl3.hs.client##name \
                                            : ss->ssl3.hs.server##name)
 
 const SSL3ProtocolVersion kTlsRecordVersion = SSL_LIBRARY_VERSION_TLS_1_0;
 const SSL3ProtocolVersion kDtlsRecordVersion = SSL_LIBRARY_VERSION_TLS_1_1;
 
@@ -825,26 +817,24 @@ tls13_ComputeHandshakeSecrets(sslSocket 
     ss->ssl3.hs.dheSecret = NULL;
     PK11_FreeSymKey(ss->ssl3.hs.currentSecret);
     ss->ssl3.hs.currentSecret = newSecret;
 
     /* Now compute |*HsTrafficSecret| */
     rv = tls13_DeriveSecretWrap(ss, ss->ssl3.hs.currentSecret,
                                 kHkdfLabelClient,
                                 kHkdfLabelHandshakeTrafficSecret,
-                                keylogLabelClientHsTrafficSecret,
                                 &ss->ssl3.hs.clientHsTrafficSecret);
     if (rv != SECSuccess) {
         LOG_ERROR(ss, SEC_ERROR_LIBRARY_FAILURE);
         return rv;
     }
     rv = tls13_DeriveSecretWrap(ss, ss->ssl3.hs.currentSecret,
                                 kHkdfLabelServer,
                                 kHkdfLabelHandshakeTrafficSecret,
-                                keylogLabelServerHsTrafficSecret,
                                 &ss->ssl3.hs.serverHsTrafficSecret);
     if (rv != SECSuccess) {
         LOG_ERROR(ss, SEC_ERROR_LIBRARY_FAILURE);
         return rv;
     }
 
     SSL_TRC(5, ("%d: TLS13[%d]: compute master secret (%s)",
                 SSL_GETPID(), ss->fd, SSL_ROLE(ss)));
@@ -877,33 +867,30 @@ tls13_ComputeHandshakeSecrets(sslSocket 
 static SECStatus
 tls13_ComputeApplicationSecrets(sslSocket *ss)
 {
     SECStatus rv;
 
     rv = tls13_DeriveSecretWrap(ss, ss->ssl3.hs.currentSecret,
                                 kHkdfLabelClient,
                                 kHkdfLabelApplicationTrafficSecret,
-                                keylogLabelClientTrafficSecret,
                                 &ss->ssl3.hs.clientTrafficSecret);
     if (rv != SECSuccess) {
         return SECFailure;
     }
     rv = tls13_DeriveSecretWrap(ss, ss->ssl3.hs.currentSecret,
                                 kHkdfLabelServer,
                                 kHkdfLabelApplicationTrafficSecret,
-                                keylogLabelServerTrafficSecret,
                                 &ss->ssl3.hs.serverTrafficSecret);
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
     rv = tls13_DeriveSecretWrap(ss, ss->ssl3.hs.currentSecret,
                                 NULL, kHkdfLabelExporterMasterSecret,
-                                keylogLabelExporterSecret,
                                 &ss->ssl3.hs.exporterSecret);
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
     return SECSuccess;
 }
 
@@ -912,17 +899,16 @@ tls13_ComputeFinalSecrets(sslSocket *ss)
 {
     SECStatus rv;
 
     PORT_Assert(!ss->ssl3.crSpec->master_secret);
     PORT_Assert(!ss->ssl3.cwSpec->master_secret);
 
     rv = tls13_DeriveSecretWrap(ss, ss->ssl3.hs.currentSecret,
                                 NULL, kHkdfLabelResumptionMasterSecret,
-                                NULL,
                                 &ss->ssl3.hs.resumptionMasterSecret);
     PK11_FreeSymKey(ss->ssl3.hs.currentSecret);
     ss->ssl3.hs.currentSecret = NULL;
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
     return SECSuccess;
@@ -1597,17 +1583,16 @@ tls13_HandleClientHelloPart2(sslSocket *
     /* Take ownership of the session. */
     ss->sec.ci.sid = sid;
     sid = NULL;
 
     if (ss->ssl3.hs.zeroRttState == ssl_0rtt_accepted) {
         rv = tls13_DeriveSecretWrap(ss, ss->ssl3.hs.currentSecret,
                                     kHkdfLabelClient,
                                     kHkdfLabelEarlyTrafficSecret,
-                                    keylogLabelClientEarlyTrafficSecret,
                                     &ss->ssl3.hs.clientEarlyTrafficSecret);
         if (rv != SECSuccess) {
             FATAL_ERROR(ss, SEC_ERROR_LIBRARY_FAILURE, internal_error);
             return SECFailure;
         }
     }
 
     ssl_GetXmitBufLock(ss);
@@ -2831,23 +2816,21 @@ tls13_DeriveSecretNullHash(sslSocket *ss
     rv = tls13_ComputeHash(ss, &hashes, buf, 0);
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
     return tls13_DeriveSecret(ss, key, label, labelLen, &hashes, dest);
 }
 
-/* Convenience wrapper that lets us supply a separate prefix and suffix.
- * It also writes a calculated secret with the given label to the keylog. */
+/* Convenience wrapper that lets us supply a separate previx and suffix. */
 static SECStatus
 tls13_DeriveSecretWrap(sslSocket *ss, PK11SymKey *key,
                        const char *prefix,
                        const char *suffix,
-                       const char *keylogLabel,
                        PK11SymKey **dest)
 {
     SECStatus rv;
     SSL3Hashes hashes;
     char buf[100];
     const char *label;
 
     if (prefix) {
@@ -2867,25 +2850,18 @@ tls13_DeriveSecretWrap(sslSocket *ss, PK
                 SSL_GETPID(), ss->fd, label));
     rv = tls13_ComputeHandshakeHashes(ss, &hashes);
     if (rv != SECSuccess) {
         PORT_Assert(0); /* Should never fail */
         ssl_MapLowLevelError(SEC_ERROR_LIBRARY_FAILURE);
         return SECFailure;
     }
 
-    rv = tls13_DeriveSecret(ss, key, label, strlen(label), &hashes, dest);
-    if (rv != SECSuccess) {
-        return SECFailure;
-    }
-
-    if (keylogLabel) {
-        ssl3_RecordKeyLog(ss, keylogLabel, *dest);
-    }
-    return SECSuccess;
+    return tls13_DeriveSecret(ss, key, label, strlen(label),
+                              &hashes, dest);
 }
 
 /* Derive traffic keys for the next cipher spec in the queue. */
 static SECStatus
 tls13_DeriveTrafficKeys(sslSocket *ss, ssl3CipherSpec *spec,
                         TrafficKeyType type,
                         CipherSpecDirection direction,
                         PRBool deleteSecret)
@@ -4832,17 +4808,16 @@ tls13_MaybeDo0RTTHandshake(sslSocket *ss
     }
 
     /* Cipher suite already set in tls13_SetupClientHello. */
     ss->ssl3.hs.preliminaryInfo = 0;
 
     rv = tls13_DeriveSecretWrap(ss, ss->ssl3.hs.currentSecret,
                                 kHkdfLabelClient,
                                 kHkdfLabelEarlyTrafficSecret,
-                                keylogLabelClientEarlyTrafficSecret,
                                 &ss->ssl3.hs.clientEarlyTrafficSecret);
     if (rv != SECSuccess)
         return SECFailure;
 
     rv = tls13_SetCipherSpec(ss, TrafficKeyEarlyApplicationData,
                              CipherSpecWrite, PR_TRUE);
     if (rv != SECSuccess) {
         return rv;