Bug 1529813 - Expose Hkdf-Expand-Label with mechanism, r=ekr NSS_3_43_BRANCH NSS_3_43_BETA4
authorMartin Thomson <mt@lowentropy.net>
Fri, 15 Mar 2019 09:07:53 +1100
branchNSS_3_43_BRANCH
changeset 15053 cf30a49bf53e
parent 15051 2c1cd23c3718
child 15054 d6b58187222c
push id3301
push usermartin.thomson@gmail.com
push dateThu, 14 Mar 2019 23:54:38 +0000
reviewersekr
bugs1529813
Bug 1529813 - Expose Hkdf-Expand-Label with mechanism, r=ekr Summary: It turns out that leaf keys sometimes need to be exposed with different mechanisms and sizes. The default function provides something good enough for use with the AEAD functions that were exposed, but if you want to use the key directly, that isn't enough. So here we are: new arguments for specifying the mechanism and key size are needed. Reviewers: ekr Tags: #secure-revision Bug #: 1529813 Differential Revision: https://phabricator.services.mozilla.com/D23596
gtests/ssl_gtest/tls_hkdf_unittest.cc
lib/ssl/sslexp.h
lib/ssl/sslimpl.h
lib/ssl/sslprimitive.c
lib/ssl/sslsock.c
--- a/gtests/ssl_gtest/tls_hkdf_unittest.cc
+++ b/gtests/ssl_gtest/tls_hkdf_unittest.cc
@@ -63,16 +63,28 @@ size_t GetHashLength(SSLHashType hash) {
   size_t i = static_cast<size_t>(hash);
   if (i < PR_ARRAY_SIZE(kHashLength)) {
     return kHashLength[i];
   }
   ADD_FAILURE() << "Unknown hash: " << hash;
   return 0;
 }
 
+CK_MECHANISM_TYPE GetHkdfMech(SSLHashType hash) {
+  switch (hash) {
+    case ssl_hash_sha256:
+      return CKM_NSS_HKDF_SHA256;
+    case ssl_hash_sha384:
+      return CKM_NSS_HKDF_SHA384;
+    default:
+      ADD_FAILURE() << "Unknown hash: " << hash;
+  }
+  return CKM_INVALID_MECHANISM;
+}
+
 PRUint16 GetSomeCipherSuiteForHash(SSLHashType hash) {
   switch (hash) {
     case ssl_hash_sha256:
       return TLS_AES_128_GCM_SHA256;
     case ssl_hash_sha384:
       return TLS_AES_256_GCM_SHA384;
     default:
       ADD_FAILURE() << "Unknown hash: " << hash;
@@ -131,46 +143,50 @@ class TlsHkdfTest : public ::testing::Te
     }
   }
 
   void SetUp() {
     ImportKey(&k1_, kKey1, hash_type_, slot_.get());
     ImportKey(&k2_, kKey2, hash_type_, slot_.get());
   }
 
-  void VerifyKey(const ScopedPK11SymKey& key, const DataBuffer& expected) {
+  void VerifyKey(const ScopedPK11SymKey& key, CK_MECHANISM_TYPE expected_mech,
+                 const DataBuffer& expected_value) {
+    EXPECT_EQ(expected_mech, PK11_GetMechanism(key.get()));
+
     SECStatus rv = PK11_ExtractKeyValue(key.get());
     ASSERT_EQ(SECSuccess, rv);
 
     SECItem* key_data = PK11_GetKeyData(key.get());
     ASSERT_NE(nullptr, key_data);
 
-    EXPECT_EQ(expected.len(), key_data->len);
-    EXPECT_EQ(0, memcmp(expected.data(), key_data->data, expected.len()));
+    EXPECT_EQ(expected_value.len(), key_data->len);
+    EXPECT_EQ(
+        0, memcmp(expected_value.data(), key_data->data, expected_value.len()));
   }
 
   void HkdfExtract(const ScopedPK11SymKey& ikmk1, const ScopedPK11SymKey& ikmk2,
                    SSLHashType base_hash, const DataBuffer& expected) {
     std::cerr << "Hash = " << kHashName[base_hash] << std::endl;
 
     PK11SymKey* prk = nullptr;
     SECStatus rv = tls13_HkdfExtract(ikmk1.get(), ikmk2.get(), base_hash, &prk);
     ASSERT_EQ(SECSuccess, rv);
     ScopedPK11SymKey prkk(prk);
 
     DumpKey("Output", prkk);
-    VerifyKey(prkk, expected);
+    VerifyKey(prkk, GetHkdfMech(base_hash), expected);
 
     // Now test the public wrapper.
     PRUint16 cs = GetSomeCipherSuiteForHash(base_hash);
     rv = SSL_HkdfExtract(SSL_LIBRARY_VERSION_TLS_1_3, cs, ikmk1.get(),
                          ikmk2.get(), &prk);
     ASSERT_EQ(SECSuccess, rv);
     ASSERT_NE(nullptr, prk);
-    VerifyKey(ScopedPK11SymKey(prk), expected);
+    VerifyKey(ScopedPK11SymKey(prk), GetHkdfMech(base_hash), expected);
   }
 
   void HkdfExpandLabel(ScopedPK11SymKey* prk, SSLHashType base_hash,
                        const uint8_t* session_hash, size_t session_hash_len,
                        const char* label, size_t label_len,
                        const DataBuffer& expected) {
     std::cerr << "Hash = " << kHashName[base_hash] << std::endl;
 
@@ -186,17 +202,33 @@ class TlsHkdfTest : public ::testing::Te
     // Verify that the public API produces the same result.
     PRUint16 cs = GetSomeCipherSuiteForHash(base_hash);
     PK11SymKey* 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);
+    VerifyKey(ScopedPK11SymKey(secret), GetHkdfMech(base_hash), expected);
+
+    // Verify that a key can be created with a different key type and size.
+    rv = SSL_HkdfExpandLabelWithMech(
+        SSL_LIBRARY_VERSION_TLS_1_3, cs, prk->get(), session_hash,
+        session_hash_len, label, label_len, CKM_DES3_CBC_PAD, 24, &secret);
+    EXPECT_EQ(SECSuccess, rv);
+    ASSERT_NE(nullptr, prk);
+    ScopedPK11SymKey with_mech(secret);
+    EXPECT_EQ(static_cast<CK_MECHANISM_TYPE>(CKM_DES3_CBC_PAD),
+              PK11_GetMechanism(with_mech.get()));
+    // Just verify that the key is the right size.
+    rv = PK11_ExtractKeyValue(with_mech.get());
+    ASSERT_EQ(SECSuccess, rv);
+    SECItem* key_data = PK11_GetKeyData(with_mech.get());
+    ASSERT_NE(nullptr, key_data);
+    EXPECT_EQ(24U, key_data->len);
   }
 
  protected:
   ScopedPK11SymKey k1_;
   ScopedPK11SymKey k2_;
   SSLHashType hash_type_;
 
  private:
--- a/lib/ssl/sslexp.h
+++ b/lib/ssl/sslexp.h
@@ -677,33 +677,55 @@ typedef struct SSLAeadContextStr SSLAead
 
 #define SSL_DestroyAead(ctx)                      \
     SSL_EXPERIMENTAL_API("SSL_DestroyAead",       \
                          (SSLAeadContext * _ctx), \
                          (ctx))
 
 /* SSL_HkdfExtract and SSL_HkdfExpandLabel implement the functions from TLS,
  * using the version and ciphersuite to set parameters. This allows callers to
- * use these TLS functions as a KDF. This is only supported for TLS 1.3. */
+ * use these TLS functions as a KDF. This is only supported for TLS 1.3.
+ *
+ * SSL_HkdfExtract produces a key with a mechanism that is suitable for input to
+ * SSL_HkdfExpandLabel (and SSL_HkdfExpandLabelWithMech). */
 #define SSL_HkdfExtract(version, cipherSuite, salt, ikm, keyp)      \
     SSL_EXPERIMENTAL_API("SSL_HkdfExtract",                         \
                          (PRUint16 _version, PRUint16 _cipherSuite, \
                           PK11SymKey * _salt, PK11SymKey * _ikm,    \
                           PK11SymKey * *_keyp),                     \
                          (version, cipherSuite, salt, ikm, keyp))
 
+/* SSL_HkdfExpandLabel produces a key with a mechanism that is suitable for
+ * input to SSL_HkdfExpandLabel or SSL_MakeAead. */
 #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))
 
+/* SSL_HkdfExpandLabelWithMech uses the KDF from the selected TLS version and
+ * cipher suite, as with the other calls, but the provided mechanism and key
+ * size. This allows the key to be used more widely. */
+#define SSL_HkdfExpandLabelWithMech(version, cipherSuite, prk,             \
+                                    hsHash, hsHashLen, label, labelLen,    \
+                                    mech, keySize, keyp)                   \
+    SSL_EXPERIMENTAL_API("SSL_HkdfExpandLabelWithMech",                    \
+                         (PRUint16 _version, PRUint16 _cipherSuite,        \
+                          PK11SymKey * _prk,                               \
+                          const PRUint8 *_hsHash, unsigned int _hsHashLen, \
+                          const char *_label, unsigned int _labelLen,      \
+                          CK_MECHANISM_TYPE _mech, unsigned int _keySize,  \
+                          PK11SymKey **_keyp),                             \
+                         (version, cipherSuite, prk,                       \
+                          hsHash, hsHashLen, label, labelLen,              \
+                          mech, keySize, keyp))
+
 /* Deprecated experimental APIs */
 #define SSL_UseAltServerHelloType(fd, enable) SSL_DEPRECATED_EXPERIMENTAL_API
 
 SEC_END_PROTOS
 
 #endif /* __sslexp_h_ */
--- a/lib/ssl/sslimpl.h
+++ b/lib/ssl/sslimpl.h
@@ -1774,16 +1774,22 @@ SECStatus SSLExp_AeadDecrypt(const SSLAe
                              PRUint8 *out, unsigned int *outLen, unsigned int maxOut);
 
 SECStatus SSLExp_HkdfExtract(PRUint16 version, PRUint16 cipherSuite,
                              PK11SymKey *salt, PK11SymKey *ikm, PK11SymKey **keyp);
 SECStatus SSLExp_HkdfExpandLabel(PRUint16 version, PRUint16 cipherSuite, PK11SymKey *prk,
                                  const PRUint8 *hsHash, unsigned int hsHashLen,
                                  const char *label, unsigned int labelLen,
                                  PK11SymKey **key);
+SECStatus
+SSLExp_HkdfExpandLabelWithMech(PRUint16 version, PRUint16 cipherSuite, PK11SymKey *prk,
+                               const PRUint8 *hsHash, unsigned int hsHashLen,
+                               const char *label, unsigned int labelLen,
+                               CK_MECHANISM_TYPE mech, unsigned int keySize,
+                               PK11SymKey **keyp);
 
 SEC_END_PROTOS
 
 #if defined(XP_UNIX) || defined(XP_OS2) || defined(XP_BEOS)
 #define SSL_GETPID getpid
 #elif defined(WIN32)
 extern int __cdecl _getpid(void);
 #define SSL_GETPID _getpid
--- a/lib/ssl/sslprimitive.c
+++ b/lib/ssl/sslprimitive.c
@@ -40,17 +40,19 @@ tls13_GetHashAndCipher(PRUint16 version,
     }
     const ssl3CipherSuiteDef *suiteDef = ssl_LookupCipherSuiteDef(cipherSuite);
     const ssl3BulkCipherDef *cipherDef = ssl_GetBulkCipherDef(suiteDef);
     if (cipherDef->type != type_aead) {
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
         return SECFailure;
     }
     *hash = suiteDef->prf_hash;
-    *cipher = cipherDef;
+    if (cipher != NULL) {
+        *cipher = cipherDef;
+    }
     return SECSuccess;
 }
 
 SECStatus
 SSLExp_MakeAead(PRUint16 version, PRUint16 cipherSuite, PK11SymKey *secret,
                 const char *labelPrefix, unsigned int labelPrefixLen,
                 SSLAeadContext **ctx)
 {
@@ -211,19 +213,18 @@ SSLExp_HkdfExtract(PRUint16 version, PRU
                    PK11SymKey *salt, PK11SymKey *ikm, PK11SymKey **keyp)
 {
     if (keyp == NULL) {
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
         return SECFailure;
     }
 
     SSLHashType hash;
-    const ssl3BulkCipherDef *cipher; /* Unused here. */
     SECStatus rv = tls13_GetHashAndCipher(version, cipherSuite,
-                                          &hash, &cipher);
+                                          &hash, NULL);
     if (rv != SECSuccess) {
         return SECFailure; /* Code already set. */
     }
     return tls13_HkdfExtract(salt, ikm, hash, keyp);
 }
 
 SECStatus
 SSLExp_HkdfExpandLabel(PRUint16 version, PRUint16 cipherSuite, PK11SymKey *prk,
@@ -233,18 +234,41 @@ SSLExp_HkdfExpandLabel(PRUint16 version,
 {
     if (prk == NULL || keyp == NULL ||
         label == NULL || labelLen == 0) {
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
         return SECFailure;
     }
 
     SSLHashType hash;
-    const ssl3BulkCipherDef *cipher; /* Unused here. */
     SECStatus rv = tls13_GetHashAndCipher(version, cipherSuite,
-                                          &hash, &cipher);
+                                          &hash, NULL);
     if (rv != SECSuccess) {
         return SECFailure; /* Code already set. */
     }
     return tls13_HkdfExpandLabel(prk, hash, hsHash, hsHashLen, label, labelLen,
                                  tls13_GetHkdfMechanismForHash(hash),
                                  tls13_GetHashSizeForHash(hash), keyp);
 }
+
+SECStatus
+SSLExp_HkdfExpandLabelWithMech(PRUint16 version, PRUint16 cipherSuite, PK11SymKey *prk,
+                               const PRUint8 *hsHash, unsigned int hsHashLen,
+                               const char *label, unsigned int labelLen,
+                               CK_MECHANISM_TYPE mech, unsigned int keySize,
+                               PK11SymKey **keyp)
+{
+    if (prk == NULL || keyp == NULL ||
+        label == NULL || labelLen == 0 ||
+        mech == CKM_INVALID_MECHANISM || keySize == 0) {
+        PORT_SetError(SEC_ERROR_INVALID_ARGS);
+        return SECFailure;
+    }
+
+    SSLHashType hash;
+    SECStatus rv = tls13_GetHashAndCipher(version, cipherSuite,
+                                          &hash, NULL);
+    if (rv != SECSuccess) {
+        return SECFailure; /* Code already set. */
+    }
+    return tls13_HkdfExpandLabel(prk, hash, hsHash, hsHashLen, label, labelLen,
+                                 mech, keySize, keyp);
+}
--- a/lib/ssl/sslsock.c
+++ b/lib/ssl/sslsock.c
@@ -4049,16 +4049,17 @@ struct {
     EXP(EncodeESNIKeys),
     EXP(GetCurrentEpoch),
     EXP(GetExtensionSupport),
     EXP(GetResumptionTokenInfo),
     EXP(HelloRetryRequestCallback),
     EXP(InstallExtensionHooks),
     EXP(HkdfExtract),
     EXP(HkdfExpandLabel),
+    EXP(HkdfExpandLabelWithMech),
     EXP(KeyUpdate),
     EXP(MakeAead),
     EXP(RecordLayerData),
     EXP(RecordLayerWriteCallback),
     EXP(SecretCallback),
     EXP(SendCertificateRequest),
     EXP(SendSessionTicket),
     EXP(SetESNIKeyPair),