Bug 1529813 - Expose HKDF-Expand-Label, r=ekr

Summary:
I forgot about packet number encryption.  This will help with that.

I decided to replace DeriveSecret with this.  No point in having that when you have this.

Reviewers: ekr

Bug #: 1529813

Differential Revision: https://phabricator.services.mozilla.com/D20937

5 files changed, 56 insertions, 45 deletions

diff --git a/gtests/ssl_gtest/tls_hkdf_unittest.cc b/gtests/ssl_gtest/tls_hkdf_unittest.cc
index ef54c0bf2..fe2038664 100644
--- a/gtests/ssl_gtest/tls_hkdf_unittest.cc
+++ b/gtests/ssl_gtest/tls_hkdf_unittest.cc
@@ -183,15 +183,12 @@ class TlsHkdfTest : public ::testing::Test,
     DumpData("Output", &output[0], output.size());
     EXPECT_EQ(0, memcmp(expected.data(), &output[0], expected.len()));
 
-    if (session_hash_len > 0) {
-      return;
-    }
-
     // Verify that the public API produces the same result.
     PRUint16 cs = GetSomeCipherSuiteForHash(base_hash);
     PK11SymKey* secret;
-    rv = SSL_HkdfDeriveSecret(SSL_LIBRARY_VERSION_TLS_1_3, cs, prk->get(),
-                              label, label_len, &secret);
+    rv = SSL_HkdfExpandLabel(SSL_LIBRARY_VERSION_TLS_1_3, cs, prk->get(),
+                             session_hash, session_hash_len, label, label_len,
+                             &secret);
     EXPECT_EQ(SECSuccess, rv);
     ASSERT_NE(nullptr, prk);
     VerifyKey(ScopedPK11SymKey(secret), expected);
@@ -347,51 +344,62 @@ TEST_P(TlsHkdfTest, BadExtractWrapperInput) {
   EXPECT_EQ(nullptr, key);
 }
 
-TEST_P(TlsHkdfTest, BadDeriveSecretWrapperInput) {
+TEST_P(TlsHkdfTest, BadExpandLabelWrapperInput) {
   PK11SymKey* key = nullptr;
   static const char* kLabel = "label";
 
   // Bad version.
-  EXPECT_EQ(SECFailure, SSL_HkdfDeriveSecret(SSL_LIBRARY_VERSION_TLS_1_2,
-                                             TLS_AES_128_GCM_SHA256, k1_.get(),
-                                             kLabel, strlen(kLabel), &key));
+  EXPECT_EQ(
+      SECFailure,
+      SSL_HkdfExpandLabel(SSL_LIBRARY_VERSION_TLS_1_2, TLS_AES_128_GCM_SHA256,
+                          k1_.get(), nullptr, 0, kLabel, strlen(kLabel), &key));
   EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
 
   // Bad ciphersuite.
-  EXPECT_EQ(SECFailure, SSL_HkdfDeriveSecret(SSL_LIBRARY_VERSION_TLS_1_3,
-                                             TLS_RSA_WITH_NULL_MD5, k1_.get(),
-                                             kLabel, strlen(kLabel), &key));
+  EXPECT_EQ(
+      SECFailure,
+      SSL_HkdfExpandLabel(SSL_LIBRARY_VERSION_TLS_1_3, TLS_RSA_WITH_NULL_MD5,
+                          k1_.get(), nullptr, 0, kLabel, strlen(kLabel), &key));
   EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
 
   // Old ciphersuite.
   EXPECT_EQ(SECFailure,
-            SSL_HkdfDeriveSecret(SSL_LIBRARY_VERSION_TLS_1_3,
-                                 TLS_RSA_WITH_AES_128_CBC_SHA, k1_.get(),
-                                 kLabel, strlen(kLabel), &key));
+            SSL_HkdfExpandLabel(SSL_LIBRARY_VERSION_TLS_1_3,
+                                TLS_RSA_WITH_AES_128_CBC_SHA, k1_.get(),
+                                nullptr, 0, kLabel, strlen(kLabel), &key));
   EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
 
   // Null PRK.
-  EXPECT_EQ(SECFailure, SSL_HkdfDeriveSecret(SSL_LIBRARY_VERSION_TLS_1_2,
-                                             TLS_AES_128_GCM_SHA256, nullptr,
-                                             kLabel, strlen(kLabel), &key));
+  EXPECT_EQ(SECFailure, SSL_HkdfExpandLabel(
+                            SSL_LIBRARY_VERSION_TLS_1_2, TLS_AES_128_GCM_SHA256,
+                            nullptr, nullptr, 0, kLabel, strlen(kLabel), &key));
   EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
 
+  // Null, non-zero-length handshake hash.
+  EXPECT_EQ(
+      SECFailure,
+      SSL_HkdfExpandLabel(SSL_LIBRARY_VERSION_TLS_1_2, TLS_AES_128_GCM_SHA256,
+                          k1_.get(), nullptr, 2, kLabel, strlen(kLabel), &key));
+
+  EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
   // Null, non-zero-length label.
-  EXPECT_EQ(SECFailure, SSL_HkdfDeriveSecret(SSL_LIBRARY_VERSION_TLS_1_3,
-                                             TLS_AES_128_GCM_SHA256, k1_.get(),
-                                             nullptr, strlen(kLabel), &key));
+  EXPECT_EQ(SECFailure,
+            SSL_HkdfExpandLabel(SSL_LIBRARY_VERSION_TLS_1_3,
+                                TLS_AES_128_GCM_SHA256, k1_.get(), nullptr, 0,
+                                nullptr, strlen(kLabel), &key));
   EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
 
   // Null, empty label.
-  EXPECT_EQ(SECFailure, SSL_HkdfDeriveSecret(SSL_LIBRARY_VERSION_TLS_1_3,
-                                             TLS_AES_128_GCM_SHA256, k1_.get(),
-                                             nullptr, 0, &key));
+  EXPECT_EQ(SECFailure, SSL_HkdfExpandLabel(SSL_LIBRARY_VERSION_TLS_1_3,
+                                            TLS_AES_128_GCM_SHA256, k1_.get(),
+                                            nullptr, 0, nullptr, 0, &key));
   EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
 
   // Null key pointer..
-  EXPECT_EQ(SECFailure, SSL_HkdfDeriveSecret(SSL_LIBRARY_VERSION_TLS_1_3,
-                                             TLS_AES_128_GCM_SHA256, k1_.get(),
-                                             kLabel, strlen(kLabel), nullptr));
+  EXPECT_EQ(SECFailure,
+            SSL_HkdfExpandLabel(SSL_LIBRARY_VERSION_TLS_1_3,
+                                TLS_AES_128_GCM_SHA256, k1_.get(), nullptr, 0,
+                                kLabel, strlen(kLabel), nullptr));
   EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
 
   EXPECT_EQ(nullptr, key);
diff --git a/lib/ssl/sslexp.h b/lib/ssl/sslexp.h
index d9d911b57..4f325e385 100644
--- a/lib/ssl/sslexp.h
+++ b/lib/ssl/sslexp.h
@@ -690,15 +690,16 @@ typedef struct SSLAeadContextStr SSLAeadContext;
                           PK11SymKey * *_keyp),                     \
                          (version, cipherSuite, salt, ikm, keyp))
 
-#define SSL_HkdfDeriveSecret(version, cipherSuite, prk,               \
-                             label, labelLen, keyp)                   \
-    SSL_EXPERIMENTAL_API("SSL_HkdfDeriveSecret",                      \
-                         (PRUint16 _version, PRUint16 _cipherSuite,   \
-                          PK11SymKey * _prk,                          \
-                          const char *_label, unsigned int _labelLen, \
-                          PK11SymKey **_keyp),                        \
-                         (version, cipherSuite, prk,                  \
-                          label, labelLen, keyp))
+#define SSL_HkdfExpandLabel(version, cipherSuite, prk,                     \
+                            hsHash, hsHashLen, label, labelLen, keyp)      \
+    SSL_EXPERIMENTAL_API("SSL_HkdfExpandLabel",                            \
+                         (PRUint16 _version, PRUint16 _cipherSuite,        \
+                          PK11SymKey * _prk,                               \
+                          const PRUint8 *_hsHash, unsigned int _hsHashLen, \
+                          const char *_label, unsigned int _labelLen,      \
+                          PK11SymKey **_keyp),                             \
+                         (version, cipherSuite, prk,                       \
+                          hsHash, hsHashLen, label, labelLen, keyp))
 
 /* Deprecated experimental APIs */
 #define SSL_UseAltServerHelloType(fd, enable) SSL_DEPRECATED_EXPERIMENTAL_API
diff --git a/lib/ssl/sslimpl.h b/lib/ssl/sslimpl.h
index a5aaef2e6..2dbf60436 100644
--- a/lib/ssl/sslimpl.h
+++ b/lib/ssl/sslimpl.h
@@ -1775,9 +1775,10 @@ SECStatus SSLExp_AeadDecrypt(const SSLAeadContext *ctx, PRUint64 counter,
 
 SECStatus SSLExp_HkdfExtract(PRUint16 version, PRUint16 cipherSuite,
                              PK11SymKey *salt, PK11SymKey *ikm, PK11SymKey **keyp);
-SECStatus SSLExp_HkdfDeriveSecret(PRUint16 version, PRUint16 cipherSuite, PK11SymKey *prk,
-                                  const char *label, unsigned int labelLen,
-                                  PK11SymKey **key);
+SECStatus SSLExp_HkdfExpandLabel(PRUint16 version, PRUint16 cipherSuite, PK11SymKey *prk,
+                                 const PRUint8 *hsHash, unsigned int hsHashLen,
+                                 const char *label, unsigned int labelLen,
+                                 PK11SymKey **key);
 
 SEC_END_PROTOS
 
diff --git a/lib/ssl/sslprimitive.c b/lib/ssl/sslprimitive.c
index 1d13476e6..811827c7d 100644
--- a/lib/ssl/sslprimitive.c
+++ b/lib/ssl/sslprimitive.c
@@ -226,9 +226,10 @@ SSLExp_HkdfExtract(PRUint16 version, PRUint16 cipherSuite,
 }
 
 SECStatus
-SSLExp_HkdfDeriveSecret(PRUint16 version, PRUint16 cipherSuite, PK11SymKey *prk,
-                        const char *label, unsigned int labelLen,
-                        PK11SymKey **keyp)
+SSLExp_HkdfExpandLabel(PRUint16 version, PRUint16 cipherSuite, PK11SymKey *prk,
+                       const PRUint8 *hsHash, unsigned int hsHashLen,
+                       const char *label, unsigned int labelLen,
+                       PK11SymKey **keyp)
 {
     if (prk == NULL || keyp == NULL ||
         label == NULL || labelLen == 0) {
@@ -243,7 +244,7 @@ SSLExp_HkdfDeriveSecret(PRUint16 version, PRUint16 cipherSuite, PK11SymKey *prk,
     if (rv != SECSuccess) {
         return SECFailure; /* Code already set. */
     }
-    return tls13_HkdfExpandLabel(prk, hash, NULL, 0, label, labelLen,
+    return tls13_HkdfExpandLabel(prk, hash, hsHash, hsHashLen, label, labelLen,
                                  tls13_GetHkdfMechanismForHash(hash),
                                  tls13_GetHashSizeForHash(hash), keyp);
 }
diff --git a/lib/ssl/sslsock.c b/lib/ssl/sslsock.c
index 6b9ffc339..735710531 100644
--- a/lib/ssl/sslsock.c
+++ b/lib/ssl/sslsock.c
@@ -4053,7 +4053,7 @@ struct {
     EXP(HelloRetryRequestCallback),
     EXP(InstallExtensionHooks),
     EXP(HkdfExtract),
-    EXP(HkdfDeriveSecret),
+    EXP(HkdfExpandLabel),
     EXP(KeyUpdate),
     EXP(MakeAead),
     EXP(RecordLayerData),