Skip to content

Commit

Permalink
Bug 1561637 TLS 1.3 does not work in FIPS mode
Browse files Browse the repository at this point in the history
Patch 1 of 2.
This patch updates softoken and helper functions with the new PKCS #11 v3 HKDF,
which handles all the correct key management so that we can work in FIPS mode

1) Salts can be passed in as data, as and explicit NULL (which per spec means
a zero filled buffer of length of the underlying HMAC), or through a key handle
2) A Data object can be used as a key (explicitly allowed for this mechanism by
the spec).
3) A special mechansism produces a data object rather than a key, the latter
which can be exported. Softoken does not do the optional validation on the pInfo
to verify that the requested values are supposed to be data rather than keys.
Some other tokens may.

The old hkdf mechanism has been retained for compatibility (well namely until
patch 2 is created, tls is still using it). The hkdf function has been broken
off into it's own function rather than inline in the derive function.

Note: because the base key and/or the export key could really be a data object,
our explicit handling of sensitive and extractable are adjusted to take into
account that those flags do not exist in data objects.

Differential Revision: https://phabricator.services.mozilla.com/D68940
  • Loading branch information
rjrelyea committed Mar 30, 2020
1 parent a660b2c commit f4952cf
Show file tree
Hide file tree
Showing 8 changed files with 439 additions and 177 deletions.
3 changes: 2 additions & 1 deletion automation/abi-check/expected-report-libnss3.so.txt
@@ -1,7 +1,8 @@
4 Added functions:
5 Added functions:

[A] 'function SECStatus PK11_AEADOp(PK11Context*, CK_GENERATOR_FUNCTION, int, unsigned char*, int, const unsigned char*, int, unsigned char*, int*, int, unsigned char*, int, const unsigned char*, int)' {PK11_AEADOp@@NSS_3.52}
[A] 'function SECStatus PK11_AEADRawOp(PK11Context*, void*, int, const unsigned char*, int, unsigned char*, int*, int, const unsigned char*, int)' {PK11_AEADRawOp@@NSS_3.52}
[A] 'function CK_OBJECT_HANDLE PK11_GetObjectHandle(PK11ObjectType, void*, PK11SlotInfo**)' {PK11_GetObjectHandle@@NSS_3.52}
[A] 'function PRBool _PK11_ContextGetAEADSimulation(PK11Context*)' {_PK11_ContextGetAEADSimulation@@NSS_3.52}
[A] 'function SECStatus _PK11_ContextSetAEADSimulation(PK11Context*)' {_PK11_ContextSetAEADSimulation@@NSS_3.52}

Expand Down
141 changes: 129 additions & 12 deletions gtests/pk11_gtest/pk11_hkdf_unittest.cc
Expand Up @@ -17,35 +17,141 @@

namespace nss_test {

/* different mechanisms for the tests */
typedef int HkdfTestType;
const int kNSSHkdfLegacy = 0;
const int kPkcs11HkdfDerive = 1;
const int kPkcs11HkdfDeriveDataKey = 2;
const int kPkcs11HkdfSaltDerive = 3;
const int kPkcs11HkdfData = 4;
const int kPKCS11NumTypes = 5;

class Pkcs11HkdfTest : public ::testing::TestWithParam<hkdf_vector> {
protected:
ScopedPK11SymKey ImportKey(CK_MECHANISM_TYPE mech, SECItem *ikm_item) {
CK_MECHANISM_TYPE Pk11HkdfToHash(CK_MECHANISM_TYPE nssHkdfMech) {
switch (nssHkdfMech) {
case CKM_NSS_HKDF_SHA1:
return CKM_SHA_1;
case CKM_NSS_HKDF_SHA256:
return CKM_SHA256;
case CKM_NSS_HKDF_SHA384:
return CKM_SHA384;
case CKM_NSS_HKDF_SHA512:
return CKM_SHA512;
default:
break;
}

return CKM_INVALID_MECHANISM;
}

ScopedPK11SymKey ImportKey(CK_KEY_TYPE keyType, SECItem *ikm_item) {
ScopedPK11SlotInfo slot(PK11_GetInternalSlot());
CK_MECHANISM_TYPE mech = CKM_HKDF_KEY_GEN;
if (!slot) {
ADD_FAILURE() << "Can't get slot";
return nullptr;
}
ScopedPK11SymKey ikm(
PK11_ImportSymKey(slot.get(), CKM_GENERIC_SECRET_KEY_GEN,
PK11_OriginUnwrap, CKA_SIGN, ikm_item, nullptr));
switch (keyType) {
case CKK_GENERIC_SECRET:
mech = CKM_GENERIC_SECRET_KEY_GEN;
break;
case CKK_HKDF:
mech = CKM_HKDF_KEY_GEN;
break;
}

ScopedPK11SymKey ikm(PK11_ImportSymKey(slot.get(), mech, PK11_OriginUnwrap,
CKA_SIGN, ikm_item, nullptr));

return ikm;
}

void RunTest(hkdf_vector vec) {
void RunTest(hkdf_vector vec, HkdfTestType type, CK_KEY_TYPE keyType) {
SECItem ikm_item = {siBuffer, vec.ikm.data(),
static_cast<unsigned int>(vec.ikm.size())};

CK_NSS_HKDFParams hkdf_params = {
SECItem salt_item = {siBuffer, vec.salt.data(),
static_cast<unsigned int>(vec.salt.size())};
CK_MECHANISM_TYPE derive_mech = vec.mech;
CK_NSS_HKDFParams nss_hkdf_params = {
true, vec.salt.data(), static_cast<unsigned int>(vec.salt.size()),
true, vec.info.data(), static_cast<unsigned int>(vec.info.size())};
SECItem params_item = {siBuffer, (unsigned char *)&hkdf_params,
sizeof(hkdf_params)};

ScopedPK11SymKey ikm = ImportKey(vec.mech, &ikm_item);
CK_HKDF_PARAMS hkdf_params = {true,
true,
vec.mech,
CKF_HKDF_SALT_DATA,
vec.salt.data(),
static_cast<unsigned int>(vec.salt.size()),
CK_INVALID_HANDLE,
vec.info.data(),
static_cast<unsigned int>(vec.info.size())};
SECItem params_item = {siBuffer, (unsigned char *)&nss_hkdf_params,
sizeof(nss_hkdf_params)};

ScopedPK11SymKey ikm = ImportKey(keyType, &ikm_item);
ScopedPK11SymKey salt_key = NULL;
ASSERT_NE(nullptr, ikm.get());

switch (type) {
case kNSSHkdfLegacy:
printf("kNSSHkdfLegacy\n");
break; /* already set up */
case kPkcs11HkdfDeriveDataKey: {
ScopedPK11SlotInfo slot(PK11_GetSlotFromKey(ikm.get()));
CK_OBJECT_CLASS ckoData = CKO_DATA;
CK_OBJECT_HANDLE handle;
CK_ATTRIBUTE pk11template[2] = {
{CKA_CLASS, (CK_BYTE_PTR)&ckoData, sizeof(ckoData)},
{CKA_VALUE, vec.ikm.data(), static_cast<CK_ULONG>(vec.ikm.size())}};

ScopedPK11GenericObject dataKey(PK11_CreateGenericObject(
slot.get(), pk11template, PR_ARRAY_SIZE(pk11template), PR_FALSE));
ASSERT_NE(nullptr, dataKey.get());
handle = PK11_GetObjectHandle(PK11_TypeGeneric, dataKey.get(), NULL);
ASSERT_NE((CK_ULONG)CK_INVALID_HANDLE, (CK_ULONG)handle);
/* replaces old key with our new data key */
ikm = ScopedPK11SymKey(
PK11_SymKeyFromHandle(slot.get(), NULL, PK11_OriginUnwrap,
CKM_HKDF_DERIVE, handle, PR_TRUE, NULL));
ASSERT_NE(nullptr, ikm.get());
/* generic object is freed, ikm owns the handle */
}
/* fall through */
case kPkcs11HkdfSaltDerive:
case kPkcs11HkdfDerive:
if (hkdf_params.ulSaltLen == 0) {
hkdf_params.ulSaltType = CKF_HKDF_SALT_NULL;
printf("kPkcs11HkdfNullSaltDerive\n");
} else if (type == kPkcs11HkdfSaltDerive) {
salt_key = ImportKey(keyType, &salt_item);
hkdf_params.ulSaltType = CKF_HKDF_SALT_KEY;
hkdf_params.ulSaltLen = 0;
hkdf_params.pSalt = NULL;
hkdf_params.hSaltKey = PK11_GetSymKeyHandle(salt_key.get());
printf("kPkcs11HkdfSaltDerive\n");
} else {
printf("kPkcs11HkdfDerive%s\n",
(type == kPkcs11HkdfDeriveDataKey) ? "DataKey" : "");
}
hkdf_params.prfHashMechanism = Pk11HkdfToHash(vec.mech);
params_item.data = (unsigned char *)&hkdf_params;
params_item.len = sizeof(hkdf_params);
derive_mech = CKM_HKDF_DERIVE;
break;
case kPkcs11HkdfData:
printf("kPkcs11HkdfData\n");
if (hkdf_params.ulSaltLen == 0) {
hkdf_params.ulSaltType = CKF_HKDF_SALT_NULL;
}
hkdf_params.prfHashMechanism = Pk11HkdfToHash(vec.mech);
params_item.data = (unsigned char *)&hkdf_params;
params_item.len = sizeof(hkdf_params);
derive_mech = CKM_HKDF_DATA;
break;
}
ASSERT_NE(derive_mech, CKM_INVALID_MECHANISM);
ScopedPK11SymKey okm = ScopedPK11SymKey(
PK11_Derive(ikm.get(), vec.mech, &params_item,
PK11_Derive(ikm.get(), derive_mech, &params_item,
CKM_GENERIC_SECRET_KEY_GEN, CKA_DERIVE, vec.l));
if (vec.res.expect_rv == SECSuccess) {
ASSERT_NE(nullptr, okm.get());
Expand All @@ -60,6 +166,17 @@ class Pkcs11HkdfTest : public ::testing::TestWithParam<hkdf_vector> {
ASSERT_EQ(nullptr, okm.get());
}
}
void RunTest(hkdf_vector vec) {
HkdfTestType test_type;

for (test_type = kNSSHkdfLegacy; test_type < kPKCS11NumTypes; test_type++) {
RunTest(vec, test_type, CKK_GENERIC_SECRET);
if (test_type == kPkcs11HkdfDeriveDataKey) {
continue;
}
RunTest(vec, test_type, CKK_HKDF);
}
}
};

TEST_P(Pkcs11HkdfTest, TestVectors) { RunTest(GetParam()); }
Expand Down
1 change: 1 addition & 0 deletions lib/nss/nss.def
Expand Up @@ -1169,6 +1169,7 @@ _PK11_ContextGetAEADSimulation;
_PK11_ContextSetAEADSimulation;
PK11_AEADOp;
PK11_AEADRawOp;
PK11_GetObjectHandle;
;+ local:
;+ *;
;+};
6 changes: 6 additions & 0 deletions lib/pk11wrap/pk11mech.c
Expand Up @@ -199,6 +199,8 @@ PK11_GetKeyMechanism(CK_KEY_TYPE type)
return CKM_KEA_KEY_DERIVE;
case CKK_EC: /* CKK_ECDSA is deprecated */
return CKM_ECDSA;
case CKK_HKDF:
return CKM_HKDF_DERIVE;
case CKK_GENERIC_SECRET:
default:
return CKM_SHA_1_HMAC;
Expand Down Expand Up @@ -385,6 +387,10 @@ PK11_GetKeyType(CK_MECHANISM_TYPE type, unsigned long len)
case CKM_EC_KEY_PAIR_GEN: /* aka CKM_ECDSA_KEY_PAIR_GEN */
case CKM_ECDH1_DERIVE:
return CKK_EC; /* CKK_ECDSA is deprecated */
case CKM_HKDF_KEY_GEN:
case CKM_HKDF_DERIVE:
case CKM_HKDF_DATA:
return CKK_HKDF;
case CKM_SSL3_PRE_MASTER_KEY_GEN:
case CKM_GENERIC_SECRET_KEY_GEN:
case CKM_SSL3_MASTER_KEY_DERIVE:
Expand Down
64 changes: 32 additions & 32 deletions lib/pk11wrap/pk11obj.c
Expand Up @@ -1692,18 +1692,12 @@ PK11_CreateManagedGenericObject(PK11SlotInfo *slot,
!token);
}

/*
* Change an attribute on a raw object
*/
SECStatus
PK11_WriteRawAttribute(PK11ObjectType objType, void *objSpec,
CK_ATTRIBUTE_TYPE attrType, SECItem *item)
CK_OBJECT_HANDLE
PK11_GetObjectHandle(PK11ObjectType objType, void *objSpec,
PK11SlotInfo **slotp)
{
CK_OBJECT_HANDLE handle = CK_INVALID_HANDLE;
PK11SlotInfo *slot = NULL;
CK_OBJECT_HANDLE handle = 0;
CK_ATTRIBUTE setTemplate;
CK_RV crv;
CK_SESSION_HANDLE rwsession;

switch (objType) {
case PK11_TypeGeneric:
Expand All @@ -1724,9 +1718,35 @@ PK11_WriteRawAttribute(PK11ObjectType objType, void *objSpec,
break;
case PK11_TypeCert: /* don't handle cert case for now */
default:
PORT_SetError(SEC_ERROR_UNKNOWN_OBJECT_TYPE);
break;
}
if (slotp) {
*slotp = slot;
}
/* paranoia. If the object doesn't have a slot, then it's handle isn't
* valid either */
if (slot == NULL) {
handle = CK_INVALID_HANDLE;
}
return handle;
}

/*
* Change an attribute on a raw object
*/
SECStatus
PK11_WriteRawAttribute(PK11ObjectType objType, void *objSpec,
CK_ATTRIBUTE_TYPE attrType, SECItem *item)
{
PK11SlotInfo *slot = NULL;
CK_OBJECT_HANDLE handle = 0;
CK_ATTRIBUTE setTemplate;
CK_RV crv;
CK_SESSION_HANDLE rwsession;

handle = PK11_GetObjectHandle(objType, objSpec, &slot);
if (handle == CK_INVALID_HANDLE) {
PORT_SetError(SEC_ERROR_UNKNOWN_OBJECT_TYPE);
return SECFailure;
}
Expand Down Expand Up @@ -1754,28 +1774,8 @@ PK11_ReadRawAttribute(PK11ObjectType objType, void *objSpec,
PK11SlotInfo *slot = NULL;
CK_OBJECT_HANDLE handle = 0;

switch (objType) {
case PK11_TypeGeneric:
slot = ((PK11GenericObject *)objSpec)->slot;
handle = ((PK11GenericObject *)objSpec)->objectID;
break;
case PK11_TypePrivKey:
slot = ((SECKEYPrivateKey *)objSpec)->pkcs11Slot;
handle = ((SECKEYPrivateKey *)objSpec)->pkcs11ID;
break;
case PK11_TypePubKey:
slot = ((SECKEYPublicKey *)objSpec)->pkcs11Slot;
handle = ((SECKEYPublicKey *)objSpec)->pkcs11ID;
break;
case PK11_TypeSymKey:
slot = ((PK11SymKey *)objSpec)->slot;
handle = ((PK11SymKey *)objSpec)->objectID;
break;
case PK11_TypeCert: /* don't handle cert case for now */
default:
break;
}
if (slot == NULL) {
handle = PK11_GetObjectHandle(objType, objSpec, &slot);
if (handle == CK_INVALID_HANDLE) {
PORT_SetError(SEC_ERROR_UNKNOWN_OBJECT_TYPE);
return SECFailure;
}
Expand Down
3 changes: 3 additions & 0 deletions lib/pk11wrap/pk11pub.h
Expand Up @@ -876,6 +876,9 @@ SECStatus PK11_ReadRawAttribute(PK11ObjectType type, void *object,
CK_ATTRIBUTE_TYPE attr, SECItem *item);
SECStatus PK11_WriteRawAttribute(PK11ObjectType type, void *object,
CK_ATTRIBUTE_TYPE attr, SECItem *item);
/* get the PKCS #11 handle and slot for a generic object */
CK_OBJECT_HANDLE PK11_GetObjectHandle(PK11ObjectType objType, void *objSpec,
PK11SlotInfo **slotp);

/*
* PK11_GetAllSlotsForCert returns all the slots that a given certificate
Expand Down
3 changes: 3 additions & 0 deletions lib/softoken/pkcs11.c
Expand Up @@ -427,6 +427,9 @@ static const struct mechanismList mechanisms[] = {
{ 0, 512, CKF_SN_VR },
PR_FALSE },
/* ------------------------- HKDF Operations -------------------------- */
{ CKM_HKDF_DERIVE, { 1, 255 * 64, CKF_DERIVE }, PR_TRUE },
{ CKM_HKDF_DATA, { 1, 255 * 64, CKF_DERIVE }, PR_TRUE },
{ CKM_HKDF_KEY_GEN, { 20, 64, CKF_GENERATE }, PR_TRUE },
{ CKM_NSS_HKDF_SHA1, { 1, 128, CKF_DERIVE }, PR_TRUE },
{ CKM_NSS_HKDF_SHA256, { 1, 128, CKF_DERIVE }, PR_TRUE },
{ CKM_NSS_HKDF_SHA384, { 1, 128, CKF_DERIVE }, PR_TRUE },
Expand Down

0 comments on commit f4952cf

Please sign in to comment.