+
+
+ +
Test | +Result | +Time | +
---|
diff --git a/dom/crypto/CryptoKey.cpp b/dom/crypto/CryptoKey.cpp index 1ed86388512f..526d34500ba0 100644 --- a/dom/crypto/CryptoKey.cpp +++ b/dom/crypto/CryptoKey.cpp @@ -63,9 +63,23 @@ StringToUsage(const nsString& aUsage, CryptoKey::KeyUsage& aUsageOut) return NS_OK; } +// This helper function will release the memory backing a SECKEYPrivateKey and +// any resources acquired in its creation. It will leave the backing PKCS#11 +// object untouched, however. This should only be called from +// PrivateKeyFromPrivateKeyTemplate. +static void +DestroyPrivateKeyWithoutDestroyingPKCS11Object(SECKEYPrivateKey* key) +{ + PK11_FreeSlot(key->pkcs11Slot); + PORT_FreeArena(key->arena, PR_TRUE); +} + +// To protect against key ID collisions, PrivateKeyFromPrivateKeyTemplate +// generates a random ID for each key. The given template must contain an +// attribute slot for a key ID, but it must consist of a null pointer and have a +// length of 0. SECKEYPrivateKey* -PrivateKeyFromPrivateKeyTemplate(SECItem* aObjID, - CK_ATTRIBUTE* aTemplate, +PrivateKeyFromPrivateKeyTemplate(CK_ATTRIBUTE* aTemplate, CK_ULONG aTemplateSize) { // Create a generic object with the contents of the key @@ -74,16 +88,63 @@ PrivateKeyFromPrivateKeyTemplate(SECItem* aObjID, return nullptr; } + // Generate a random 160-bit object ID. This ID must be unique. + ScopedSECItem objID(::SECITEM_AllocItem(nullptr, nullptr, 20)); + SECStatus rv = PK11_GenerateRandomOnSlot(slot, objID->data, objID->len); + if (rv != SECSuccess) { + return nullptr; + } + // Check if something is already using this ID. + SECKEYPrivateKey* preexistingKey = PK11_FindKeyByKeyID(slot, objID, nullptr); + if (preexistingKey) { + // Note that we can't just call SECKEY_DestroyPrivateKey here because that + // will destroy the PKCS#11 object that is backing a preexisting key (that + // we still have a handle on somewhere else in memory). If that object were + // destroyed, cryptographic operations performed by that other key would + // fail. + DestroyPrivateKeyWithoutDestroyingPKCS11Object(preexistingKey); + // Try again with a new ID (but only once - collisions are very unlikely). + rv = PK11_GenerateRandomOnSlot(slot, objID->data, objID->len); + if (rv != SECSuccess) { + return nullptr; + } + preexistingKey = PK11_FindKeyByKeyID(slot, objID, nullptr); + if (preexistingKey) { + DestroyPrivateKeyWithoutDestroyingPKCS11Object(preexistingKey); + return nullptr; + } + } + + CK_ATTRIBUTE* idAttributeSlot = nullptr; + for (CK_ULONG i = 0; i < aTemplateSize; i++) { + if (aTemplate[i].type == CKA_ID) { + if (aTemplate[i].pValue != nullptr || aTemplate[i].ulValueLen != 0) { + return nullptr; + } + idAttributeSlot = aTemplate + i; + break; + } + } + if (!idAttributeSlot) { + return nullptr; + } + + idAttributeSlot->pValue = objID->data; + idAttributeSlot->ulValueLen = objID->len; ScopedPK11GenericObject obj(PK11_CreateGenericObject(slot, aTemplate, aTemplateSize, PR_FALSE)); + // Unset the ID attribute slot's pointer and length so that data that only + // lives for the scope of this function doesn't escape. + idAttributeSlot->pValue = nullptr; + idAttributeSlot->ulValueLen = 0; if (!obj) { return nullptr; } // Have NSS translate the object to a private key. - return PK11_FindKeyByKeyID(slot, aObjID, nullptr); + return PK11_FindKeyByKeyID(slot, objID, nullptr); } CryptoKey::CryptoKey(nsIGlobalObject* aGlobal) @@ -262,22 +323,10 @@ CryptoKey::AddPublicKeyData(SECKEYPublicKey* aPublicKey) nsNSSShutDownPreventionLock locker; - ScopedPK11SlotInfo slot(PK11_GetInternalSlot()); - if (!slot) { - return NS_ERROR_DOM_OPERATION_ERR; - } - - // Generate a random 160-bit object ID. - ScopedSECItem objID(::SECITEM_AllocItem(nullptr, nullptr, 20)); - SECStatus rv = PK11_GenerateRandomOnSlot(slot, objID->data, objID->len); - if (rv != SECSuccess) { - return NS_ERROR_DOM_OPERATION_ERR; - } - // Read EC params. ScopedSECItem params(::SECITEM_AllocItem(nullptr, nullptr, 0)); - rv = PK11_ReadRawAttribute(PK11_TypePrivKey, mPrivateKey, CKA_EC_PARAMS, - params); + SECStatus rv = PK11_ReadRawAttribute(PK11_TypePrivKey, mPrivateKey, + CKA_EC_PARAMS, params); if (rv != SECSuccess) { return NS_ERROR_DOM_OPERATION_ERR; } @@ -300,13 +349,14 @@ CryptoKey::AddPublicKeyData(SECKEYPublicKey* aPublicKey) { CKA_TOKEN, &falseValue, sizeof(falseValue) }, { CKA_SENSITIVE, &falseValue, sizeof(falseValue) }, { CKA_PRIVATE, &falseValue, sizeof(falseValue) }, - { CKA_ID, objID->data, objID->len }, + // PrivateKeyFromPrivateKeyTemplate sets the ID. + { CKA_ID, nullptr, 0 }, { CKA_EC_PARAMS, params->data, params->len }, { CKA_EC_POINT, point->data, point->len }, { CKA_VALUE, value->data, value->len }, }; - mPrivateKey = PrivateKeyFromPrivateKeyTemplate(objID, keyTemplate, + mPrivateKey = PrivateKeyFromPrivateKeyTemplate(keyTemplate, PR_ARRAY_SIZE(keyTemplate)); NS_ENSURE_TRUE(mPrivateKey, NS_ERROR_DOM_OPERATION_ERR); @@ -727,13 +777,6 @@ CryptoKey::PrivateKeyFromJwk(const JsonWebKey& aJwk, return nullptr; } - // Compute the ID for this key - // This is generated with a SHA-1 hash, so unlikely to collide - ScopedSECItem objID(PK11_MakeIDFromPubKey(ecPoint)); - if (!objID.get()) { - return nullptr; - } - // Populate template from parameters CK_KEY_TYPE ecValue = CKK_EC; CK_ATTRIBUTE keyTemplate[9] = { @@ -742,13 +785,14 @@ CryptoKey::PrivateKeyFromJwk(const JsonWebKey& aJwk, { CKA_TOKEN, &falseValue, sizeof(falseValue) }, { CKA_SENSITIVE, &falseValue, sizeof(falseValue) }, { CKA_PRIVATE, &falseValue, sizeof(falseValue) }, - { CKA_ID, objID->data, objID->len }, + // PrivateKeyFromPrivateKeyTemplate sets the ID. + { CKA_ID, nullptr, 0 }, { CKA_EC_PARAMS, params->data, params->len }, { CKA_EC_POINT, ecPoint->data, ecPoint->len }, { CKA_VALUE, (void*) d.Elements(), (CK_ULONG) d.Length() }, }; - return PrivateKeyFromPrivateKeyTemplate(objID, keyTemplate, + return PrivateKeyFromPrivateKeyTemplate(keyTemplate, PR_ARRAY_SIZE(keyTemplate)); } @@ -771,18 +815,6 @@ CryptoKey::PrivateKeyFromJwk(const JsonWebKey& aJwk, return nullptr; } - // Compute the ID for this key - // This is generated with a SHA-1 hash, so unlikely to collide - SECItem nItem = { siBuffer, nullptr, 0 }; - if (!n.ToSECItem(arena, &nItem)) { - return nullptr; - } - - ScopedSECItem objID(PK11_MakeIDFromPubKey(&nItem)); - if (!objID.get()) { - return nullptr; - } - // Populate template from parameters CK_KEY_TYPE rsaValue = CKK_RSA; CK_ATTRIBUTE keyTemplate[14] = { @@ -791,7 +823,8 @@ CryptoKey::PrivateKeyFromJwk(const JsonWebKey& aJwk, { CKA_TOKEN, &falseValue, sizeof(falseValue) }, { CKA_SENSITIVE, &falseValue, sizeof(falseValue) }, { CKA_PRIVATE, &falseValue, sizeof(falseValue) }, - { CKA_ID, objID->data, objID->len }, + // PrivateKeyFromPrivateKeyTemplate sets the ID. + { CKA_ID, nullptr, 0 }, { CKA_MODULUS, (void*) n.Elements(), (CK_ULONG) n.Length() }, { CKA_PUBLIC_EXPONENT, (void*) e.Elements(), (CK_ULONG) e.Length() }, { CKA_PRIVATE_EXPONENT, (void*) d.Elements(), (CK_ULONG) d.Length() }, @@ -802,7 +835,7 @@ CryptoKey::PrivateKeyFromJwk(const JsonWebKey& aJwk, { CKA_COEFFICIENT, (void*) qi.Elements(), (CK_ULONG) qi.Length() }, }; - return PrivateKeyFromPrivateKeyTemplate(objID, keyTemplate, + return PrivateKeyFromPrivateKeyTemplate(keyTemplate, PR_ARRAY_SIZE(keyTemplate)); } diff --git a/dom/crypto/test/mochitest.ini b/dom/crypto/test/mochitest.ini index 4091334b7bec..2547c77215b8 100644 --- a/dom/crypto/test/mochitest.ini +++ b/dom/crypto/test/mochitest.ini @@ -16,6 +16,7 @@ skip-if = toolkit == 'android' # bug 1200570 [test_WebCrypto_ECDH.html] [test_WebCrypto_ECDSA.html] [test_WebCrypto_HKDF.html] +[test_WebCrypto_Import_Multiple_Identical_Keys.html] [test_WebCrypto_JWK.html] [test_WebCrypto_Normalize.html] [test_WebCrypto_PBKDF2.html] diff --git a/dom/crypto/test/test_WebCrypto_Import_Multiple_Identical_Keys.html b/dom/crypto/test/test_WebCrypto_Import_Multiple_Identical_Keys.html new file mode 100644 index 000000000000..6913bb78e34b --- /dev/null +++ b/dom/crypto/test/test_WebCrypto_Import_Multiple_Identical_Keys.html @@ -0,0 +1,119 @@ + + + +
+Test | +Result | +Time | +
---|