 Chromium Code Reviews
 Chromium Code Reviews Issue 83483012:
  [webcrypto] Add RSA private key PKCS#8 import for NSS.  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master
    
  
    Issue 83483012:
  [webcrypto] Add RSA private key PKCS#8 import for NSS.  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master| Index: content/renderer/webcrypto/webcrypto_impl_nss.cc | 
| diff --git a/content/renderer/webcrypto/webcrypto_impl_nss.cc b/content/renderer/webcrypto/webcrypto_impl_nss.cc | 
| index 7e966f16c5ee0f7a770ce51736bf4e6dc4049cde..6f3ee255ae6ea4d1cf7d83e0446ba00173d4e63f 100644 | 
| --- a/content/renderer/webcrypto/webcrypto_impl_nss.cc | 
| +++ b/content/renderer/webcrypto/webcrypto_impl_nss.cc | 
| @@ -324,12 +324,41 @@ bool ImportKeyInternalRaw( | 
| return true; | 
| } | 
| -typedef scoped_ptr_malloc< | 
| - CERTSubjectPublicKeyInfo, | 
| - crypto::NSSDestroyer<CERTSubjectPublicKeyInfo, | 
| - SECKEY_DestroySubjectPublicKeyInfo> > | 
| +typedef scoped_ptr<CERTSubjectPublicKeyInfo, | 
| + crypto::NSSDestroyer<CERTSubjectPublicKeyInfo, | 
| + SECKEY_DestroySubjectPublicKeyInfo> > | 
| ScopedCERTSubjectPublicKeyInfo; | 
| +// Validates an NSS KeyType against a WebCrypto algorithm. Some NSS KeyTypes | 
| +// contain enough information to fabricate a Web Crypto algorithm, which is | 
| +// returned if the input algorithm isNull(). This function indicates failure by | 
| +// returning a Null algorithm. | 
| +blink::WebCryptoAlgorithm ResolveNssKeyTypeWithInputAlgorithm( | 
| + KeyType key_type, | 
| + const blink::WebCryptoAlgorithm& algorithm_or_null) { | 
| + switch (key_type) { | 
| + case rsaKey: | 
| + // NSS's rsaKey KeyType maps to keys with SEC_OID_PKCS1_RSA_ENCRYPTION and | 
| + // according to RFC 4055 this can be used for both encryption and | 
| 
Ryan Sleevi
2013/11/25 06:59:44
Note the existence of http://tools.ietf.org/html/r
 
padolph
2013/11/25 23:02:58
Done.
 | 
| + // signatures. However, this is not specific enough to build a compatible | 
| + // Web Crypto algorithm, since in Web Crypto RSA encryption and signature | 
| 
Ryan Sleevi
2013/11/25 06:59:44
"since in Web Crypto" -> "since in Web Crypto,"
 
padolph
2013/11/25 23:02:58
Done.
 | 
| + // algorithms are distinct. So if the input algorithm isNull() here, we | 
| + // have to fail. | 
| + if (!algorithm_or_null.isNull() && IsAlgorithmRsa(algorithm_or_null)) | 
| + return algorithm_or_null; | 
| + break; | 
| + case dsaKey: | 
| + case ecKey: | 
| + case rsaPssKey: | 
| + case rsaOaepKey: | 
| + // TODO(padolph): Handle other key types. | 
| + break; | 
| + default: | 
| + break; | 
| + } | 
| + return blink::WebCryptoAlgorithm::createNull(); | 
| +} | 
| + | 
| bool ImportKeyInternalSpki( | 
| const unsigned char* key_data, | 
| unsigned key_data_size, | 
| @@ -342,16 +371,11 @@ bool ImportKeyInternalSpki( | 
| if (!key_data_size) | 
| return false; | 
| - | 
| DCHECK(key_data); | 
| // The binary blob 'key_data' is expected to be a DER-encoded ASN.1 Subject | 
| // Public Key Info. Decode this to a CERTSubjectPublicKeyInfo. | 
| - SECItem spki_item = { | 
| - siBuffer, | 
| - const_cast<uint8*>(key_data), | 
| - key_data_size | 
| - }; | 
| + SECItem spki_item = {siBuffer, const_cast<uint8*>(key_data), key_data_size}; | 
| const ScopedCERTSubjectPublicKeyInfo spki( | 
| SECKEY_DecodeDERSubjectPublicKeyInfo(&spki_item)); | 
| if (!spki) | 
| @@ -363,34 +387,10 @@ bool ImportKeyInternalSpki( | 
| return false; | 
| const KeyType sec_key_type = SECKEY_GetPublicKeyType(sec_public_key.get()); | 
| - | 
| - // Validate the sec_key_type against the input algorithm. Some NSS KeyType's | 
| - // contain enough information to fabricate a Web Crypto Algorithm, which will | 
| - // be used if the input algorithm isNull(). Others like 'rsaKey' do not (see | 
| - // below). | 
| blink::WebCryptoAlgorithm algorithm = | 
| - blink::WebCryptoAlgorithm::createNull(); | 
| - switch (sec_key_type) { | 
| - case rsaKey: | 
| - // NSS's rsaKey KeyType maps to keys with SEC_OID_PKCS1_RSA_ENCRYPTION and | 
| - // according to RFC 4055 this can be used for both encryption and | 
| - // signatures. However, this is not specific enough to build a compatible | 
| - // Web Crypto algorithm, since in Web Crypto RSA encryption and signature | 
| - // algorithms are distinct. So if the input algorithm isNull() here, we | 
| - // have to fail. | 
| - if (algorithm_or_null.isNull() || !IsAlgorithmRsa(algorithm_or_null)) | 
| - return false; | 
| - algorithm = algorithm_or_null; | 
| - break; | 
| - case dsaKey: | 
| - case ecKey: | 
| - case rsaPssKey: | 
| - case rsaOaepKey: | 
| - // TODO(padolph): Handle other key types. | 
| - return false; | 
| - default: | 
| - return false; | 
| - } | 
| + ResolveNssKeyTypeWithInputAlgorithm(sec_key_type, algorithm_or_null); | 
| + if (algorithm.isNull()) | 
| + return false; | 
| *key = blink::WebCryptoKey::create( | 
| new PublicKeyHandle(sec_public_key.Pass()), | 
| @@ -429,6 +429,61 @@ bool ExportKeyInternalSpki( | 
| return true; | 
| } | 
| +bool ImportKeyInternalPkcs8( | 
| + const unsigned char* key_data, | 
| + unsigned key_data_size, | 
| + const blink::WebCryptoAlgorithm& algorithm_or_null, | 
| + bool extractable, | 
| + blink::WebCryptoKeyUsageMask usage_mask, | 
| + blink::WebCryptoKey* key) { | 
| + | 
| + DCHECK(key); | 
| + | 
| + if (!key_data_size) | 
| + return false; | 
| + DCHECK(key_data); | 
| + | 
| + // The binary blob 'key_data' is expected to be a DER-encoded ASN.1 PKCS#8 | 
| + // private key info object. | 
| + SECItem pki_der = {siBuffer, const_cast<uint8*>(key_data), key_data_size}; | 
| + | 
| + crypto::ScopedPK11Slot slot(PK11_GetInternalKeySlot()); | 
| 
Ryan Sleevi
2013/11/25 06:59:44
This should be PK11_GetInternalSlot, not PK11_GetI
 
padolph
2013/11/25 23:02:58
Done.
 | 
| + if (!slot) | 
| + return false; | 
| + | 
| + SECKEYPrivateKey* seckey_private_key = NULL; | 
| + // TODO(padolph): Verify correct values of isPerm, isPrivate, and usage below. | 
| 
Ryan Sleevi
2013/11/25 06:59:44
isPerm/isPrivate is correct. Usage is (probably) w
 
padolph
2013/11/25 23:02:58
Thanks.
For usage, there are two cases to conside
 
eroman
2013/11/26 03:40:28
Setting usage to 0 seems like the right approach t
 
padolph
2013/11/27 03:35:34
Thanks for the guidance. I'll use KU_ALL because i
 | 
| + if (PK11_ImportDERPrivateKeyInfoAndReturnKey( | 
| + slot.get(), | 
| 
Ryan Sleevi
2013/11/25 06:59:44
These should all be indented an additional 4 space
 
padolph
2013/11/25 23:02:58
Done.
 | 
| + &pki_der, | 
| + NULL, | 
| + NULL, | 
| + false, // isPerm | 
| + false, // isPrivate | 
| + 0, // usage | 
| + &seckey_private_key, | 
| + NULL) != SECSuccess) { | 
| + return false; | 
| + } | 
| + DCHECK(seckey_private_key); | 
| + crypto::ScopedSECKEYPrivateKey private_key(seckey_private_key); | 
| + | 
| + const KeyType sec_key_type = SECKEY_GetPrivateKeyType(private_key.get()); | 
| + blink::WebCryptoAlgorithm algorithm = | 
| + ResolveNssKeyTypeWithInputAlgorithm(sec_key_type, algorithm_or_null); | 
| + if (algorithm.isNull()) | 
| + return false; | 
| + | 
| + *key = blink::WebCryptoKey::create( | 
| + new PrivateKeyHandle(private_key.Pass()), | 
| + blink::WebCryptoKeyTypePrivate, | 
| + extractable, | 
| + algorithm, | 
| + usage_mask); | 
| + | 
| + return true; | 
| +} | 
| + | 
| } // namespace | 
| void WebCryptoImpl::Init() { | 
| @@ -680,9 +735,14 @@ bool WebCryptoImpl::ImportKeyInternal( | 
| usage_mask, | 
| key); | 
| case blink::WebCryptoKeyFormatPkcs8: | 
| - // TODO(padolph): Handle PKCS#8 private key import | 
| - return false; | 
| + return ImportKeyInternalPkcs8(key_data, | 
| + key_data_size, | 
| + algorithm_or_null, | 
| + extractable, | 
| + usage_mask, | 
| + key); | 
| default: | 
| + // NOTE: blink::WebCryptoKeyFormatJwk is handled one level above. | 
| return false; | 
| } | 
| } |