Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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

--HG--
extra : rebase_source : e674a113ae3748f45bd7efbf142b7b3ab7b03273
  • Loading branch information
martinthomson committed Mar 14, 2019
1 parent 025a747 commit ed5e4c2
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 12 deletions.
44 changes: 38 additions & 6 deletions gtests/ssl_gtest/tls_hkdf_unittest.cc
Expand Up @@ -68,6 +68,18 @@ size_t GetHashLength(SSLHashType 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:
Expand Down Expand Up @@ -136,15 +148,19 @@ class TlsHkdfTest : public ::testing::Test,
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,
Expand All @@ -157,15 +173,15 @@ class TlsHkdfTest : public ::testing::Test,
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,
Expand All @@ -191,7 +207,23 @@ class TlsHkdfTest : public ::testing::Test,
&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:
Expand Down
24 changes: 23 additions & 1 deletion lib/ssl/sslexp.h
Expand Up @@ -682,14 +682,19 @@ typedef struct SSLAeadContextStr SSLAeadContext;

/* 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", \
Expand All @@ -701,6 +706,23 @@ typedef struct SSLAeadContextStr SSLAeadContext;
(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

Expand Down
6 changes: 6 additions & 0 deletions lib/ssl/sslimpl.h
Expand Up @@ -1787,6 +1787,12 @@ SECStatus SSLExp_HkdfExpandLabel(PRUint16 version, PRUint16 cipherSuite, PK11Sym
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

Expand Down
34 changes: 29 additions & 5 deletions lib/ssl/sslprimitive.c
Expand Up @@ -45,7 +45,9 @@ tls13_GetHashAndCipher(PRUint16 version, PRUint16 cipherSuite,
return SECFailure;
}
*hash = suiteDef->prf_hash;
*cipher = cipherDef;
if (cipher != NULL) {
*cipher = cipherDef;
}
return SECSuccess;
}

Expand Down Expand Up @@ -216,9 +218,8 @@ SSLExp_HkdfExtract(PRUint16 version, PRUint16 cipherSuite,
}

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. */
}
Expand All @@ -238,13 +239,36 @@ SSLExp_HkdfExpandLabel(PRUint16 version, PRUint16 cipherSuite, PK11SymKey *prk,
}

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);
}
1 change: 1 addition & 0 deletions lib/ssl/sslsock.c
Expand Up @@ -4054,6 +4054,7 @@ struct {
EXP(InstallExtensionHooks),
EXP(HkdfExtract),
EXP(HkdfExpandLabel),
EXP(HkdfExpandLabelWithMech),
EXP(KeyUpdate),
EXP(MakeAead),
EXP(RecordLayerData),
Expand Down

0 comments on commit ed5e4c2

Please sign in to comment.