[webcrypto] Add RSA private key PKCS#8 import for NSS.

[webcrypto] Add RSA private key PKCS#8 import for NSS.

Depends on Issue 62633004.

BUG=245025
TEST=content_unittests --gtest_filter="WebCryptoImpl*"

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=237523

[Ryan Sleevi, 2013-11-25 06:59:43]

NSS functions LGTM, but please wait for eroman on the Web Crypto side

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
File content/renderer/webcrypto/webcrypto_impl_nss.cc (right):

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
content/renderer/webcrypto/webcrypto_impl_nss.cc:342: // according to RFC 4055
this can be used for both encryption and
Note the existence of http://tools.ietf.org/html/rfc5756

"according to RFCs 4055/5756" should be sufficient.

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
content/renderer/webcrypto/webcrypto_impl_nss.cc:344: // Web Crypto algorithm,
since in Web Crypto RSA encryption and signature
"since in Web Crypto" -> "since in Web Crypto,"

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
content/renderer/webcrypto/webcrypto_impl_nss.cc:450: crypto::ScopedPK11Slot
slot(PK11_GetInternalKeySlot());
This should be PK11_GetInternalSlot, not PK11_GetInternalKeySlot.

The distinction is because there's this ugly API to set the internal key slot,
which we *don't* want affecting the WebCrypto Impl.

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
content/renderer/webcrypto/webcrypto_impl_nss.cc:455: // TODO(padolph): Verify
correct values of isPerm, isPrivate, and usage below.
isPerm/isPrivate is correct. Usage is (probably) wrong, IIRC.

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
content/renderer/webcrypto/webcrypto_impl_nss.cc:457: slot.get(),
These should all be indented an additional 4 spaces.

[padolph, 2013-11-25 23:02:58]

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
File content/renderer/webcrypto/webcrypto_impl_nss.cc (right):

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
content/renderer/webcrypto/webcrypto_impl_nss.cc:342: // according to RFC 4055
this can be used for both encryption and
On 2013/11/25 06:59:44, Ryan Sleevi wrote:
> Note the existence of http://tools.ietf.org/html/rfc5756
> 
> "according to RFCs 4055/5756" should be sufficient.

Done.

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
content/renderer/webcrypto/webcrypto_impl_nss.cc:344: // Web Crypto algorithm,
since in Web Crypto RSA encryption and signature
On 2013/11/25 06:59:44, Ryan Sleevi wrote:
> "since in Web Crypto" -> "since in Web Crypto,"

Done.

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
content/renderer/webcrypto/webcrypto_impl_nss.cc:450: crypto::ScopedPK11Slot
slot(PK11_GetInternalKeySlot());
On 2013/11/25 06:59:44, Ryan Sleevi wrote:
> This should be PK11_GetInternalSlot, not PK11_GetInternalKeySlot.
> 
> The distinction is because there's this ugly API to set the internal key slot,
> which we *don't* want affecting the WebCrypto Impl.

Done.

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
content/renderer/webcrypto/webcrypto_impl_nss.cc:455: // TODO(padolph): Verify
correct values of isPerm, isPrivate, and usage below.
On 2013/11/25 06:59:44, Ryan Sleevi wrote:
> isPerm/isPrivate is correct. Usage is (probably) wrong, IIRC.

Thanks.

For usage, there are two cases to consider. If the input algorithm is non-Null,
I will set the NSS usage flags just like we do in ImportKeyInternalRaw(): set
them to all known possible usages for a given algorithm, with the assumption
that Blink will further restrict usage.

But this API can receive a Null algorithm, as you and Eric discussed earlier. In
that case I think the best approach is to just take the input (Blink) usage
flags and transfer them to the NSS key. Do you agree?

One alternative is to run PK11_ImportDERPrivateKeyInfoAndReturnKey() TWICE. Once
to get the NSS KeyType from inside the DER from with usage can be deduced, then 
destroy the key and run the function again with the correct usage flags. Seems
really silly.

Or I could parse the PKI ASN.1 manually to get the KeyType first and use that to
determine usage for the PK11_ImportDERPrivateKeyInfoAndReturnKey(). I don't like
that either.

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
content/renderer/webcrypto/webcrypto_impl_nss.cc:457: slot.get(),
On 2013/11/25 06:59:44, Ryan Sleevi wrote:
> These should all be indented an additional 4 spaces.

Done.

[eroman, 2013-11-26 03:40:28]

This LGTM, however please upload the patchset which addresses Ryan's comments.
Thanks!

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
File content/renderer/webcrypto/webcrypto_impl_nss.cc (right):

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
content/renderer/webcrypto/webcrypto_impl_nss.cc:455: // TODO(padolph): Verify
correct values of isPerm, isPrivate, and usage below.
On 2013/11/25 23:02:58, padolph wrote:
> On 2013/11/25 06:59:44, Ryan Sleevi wrote:
> > isPerm/isPrivate is correct. Usage is (probably) wrong, IIRC.
> 
> Thanks.
> 
> For usage, there are two cases to consider. If the input algorithm is
non-Null,
> I will set the NSS usage flags just like we do in ImportKeyInternalRaw(): set
> them to all known possible usages for a given algorithm, with the assumption
> that Blink will further restrict usage.
> 
> But this API can receive a Null algorithm, as you and Eric discussed earlier.
In
> that case I think the best approach is to just take the input (Blink) usage
> flags and transfer them to the NSS key. Do you agree?
> 
> One alternative is to run PK11_ImportDERPrivateKeyInfoAndReturnKey() TWICE.
Once
> to get the NSS KeyType from inside the DER from with usage can be deduced,
then 
> destroy the key and run the function again with the correct usage flags. Seems
> really silly.
> 
> Or I could parse the PKI ASN.1 manually to get the KeyType first and use that
to
> determine usage for the PK11_ImportDERPrivateKeyInfoAndReturnKey(). I don't
like
> that either.

Setting usage to 0 seems like the right approach to me (or KU_ALL if 0 is
failing). Blink enforces its own granularity of usage for permitting operations
using the key, so mirroring in NSS isn't critical other than to make operations
work.

Note that usages is non-optional in WebCrypto, so even if the algorithm is Null
we could synthesize a nssKeyUsage from usage_mask. (Although we wouldn't be able
to verify if the usages make sense without knowing the algorithm).

That said, I am not sure how useful doing that is since the nssKeyUsage does not
have a 1:1 mapping with the WebCrypto key usages, so ultimately it would still
be a superset of usages (judging from how keyUsage is used in
http://mxr.mozilla.org/nss/source/lib/pk11wrap/pk11pk12.c#283)

[padolph, 2013-11-27 03:35:34]

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
File content/renderer/webcrypto/webcrypto_impl_nss.cc (right):

https://codereview.chromium.org/83483012/diff/1/content/renderer/webcrypto/we...
content/renderer/webcrypto/webcrypto_impl_nss.cc:455: // TODO(padolph): Verify
correct values of isPerm, isPrivate, and usage below.
On 2013/11/26 03:40:28, eroman wrote:
> On 2013/11/25 23:02:58, padolph wrote:
> > On 2013/11/25 06:59:44, Ryan Sleevi wrote:
> > > isPerm/isPrivate is correct. Usage is (probably) wrong, IIRC.
> > 
> > Thanks.
> > 
> > For usage, there are two cases to consider. If the input algorithm is
> non-Null,
> > I will set the NSS usage flags just like we do in ImportKeyInternalRaw():
set
> > them to all known possible usages for a given algorithm, with the assumption
> > that Blink will further restrict usage.
> > 
> > But this API can receive a Null algorithm, as you and Eric discussed
earlier.
> In
> > that case I think the best approach is to just take the input (Blink) usage
> > flags and transfer them to the NSS key. Do you agree?
> > 
> > One alternative is to run PK11_ImportDERPrivateKeyInfoAndReturnKey() TWICE.
> Once
> > to get the NSS KeyType from inside the DER from with usage can be deduced,
> then 
> > destroy the key and run the function again with the correct usage flags.
Seems
> > really silly.
> > 
> > Or I could parse the PKI ASN.1 manually to get the KeyType first and use
that
> to
> > determine usage for the PK11_ImportDERPrivateKeyInfoAndReturnKey(). I don't
> like
> > that either.
> 
> Setting usage to 0 seems like the right approach to me (or KU_ALL if 0 is
> failing). Blink enforces its own granularity of usage for permitting
operations
> using the key, so mirroring in NSS isn't critical other than to make
operations
> work.
> 
> Note that usages is non-optional in WebCrypto, so even if the algorithm is
Null
> we could synthesize a nssKeyUsage from usage_mask. (Although we wouldn't be
able
> to verify if the usages make sense without knowing the algorithm).
> 
> That said, I am not sure how useful doing that is since the nssKeyUsage does
not
> have a 1:1 mapping with the WebCrypto key usages, so ultimately it would still
> be a superset of usages (judging from how keyUsage is used in
> http://mxr.mozilla.org/nss/source/lib/pk11wrap/pk11pk12.c#283)

Thanks for the guidance. I'll use KU_ALL because it looks like 0 would restrict
usage.

[commit-bot: I haz the power, 2013-11-27 03:51:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/padolph@netflix.com/83483012/100001

[commit-bot: I haz the power, 2013-11-27 05:43:10]

Change committed as 237523