Allow a crypto::RSAPrivateKey object to be wrapped round a pre-existing

Allow a crypto::RSAPrivateKey object to be wrapped round a pre-existing
openssl key, as is currently supported for NSS.

Change-Id: I36c848884273fe8e23451259655680b6b7d46a98

BUG=412427
R=davidben@chromium.org

Committed: https://crrev.com/db7726aee7993008dff742790337f950fb371ebf
Cr-Commit-Position: refs/heads/master@{#294254}

[Ryan Sleevi, 2014-09-09 18:17:03]

rsleevi@chromium.org changed reviewers:
+ davidben@chromium.org
- rsleevi@chromium.org

[Ryan Sleevi, 2014-09-09 18:17:03]

Punting to Davidben@ to review and/or suggest alternative high-level APIs.

(FWIW, I think this is probably fine, but David should review)

[davidben, 2014-09-09 18:28:58]

lgtm with comment nit.

https://codereview.chromium.org/559623002/diff/1/crypto/rsa_private_key.h
File crypto/rsa_private_key.h (right):

https://codereview.chromium.org/559623002/diff/1/crypto/rsa_private_key.h#new...
crypto/rsa_private_key.h:221: // structure.  Does not import the key.
Nit: What does "Does not import the key" mean in this context? I'm guessing this
is from the NSS one? I would maybe replace that with:

// Create a new instance from an existing EVP_PKEY, taking a
// reference to it. |key| must be an RSA key. Returns NULL on
// failure.

[dougsteed, 2014-09-09 21:05:47]

done

https://codereview.chromium.org/559623002/diff/1/crypto/rsa_private_key.h
File crypto/rsa_private_key.h (right):

https://codereview.chromium.org/559623002/diff/1/crypto/rsa_private_key.h#new...
crypto/rsa_private_key.h:221: // structure.  Does not import the key.
On 2014/09/09 18:28:57, David Benjamin wrote:
> Nit: What does "Does not import the key" mean in this context? I'm guessing
this
> is from the NSS one? I would maybe replace that with:
> 
> // Create a new instance from an existing EVP_PKEY, taking a
> // reference to it. |key| must be an RSA key. Returns NULL on
> // failure.

Done.

[dougsteed, 2014-09-09 21:06:48]

The CQ bit was checked by dougsteed@chromium.org

[commit-bot: I haz the power, 2014-09-09 21:22:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dougsteed@chromium.org/559623002/20001

[dougsteed, 2014-09-09 22:01:32]

The CQ bit was checked by dougsteed@chromium.org

[commit-bot: I haz the power, 2014-09-09 22:06:00]

Try jobs failed on following builders:
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-09-09 22:06:25]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dougsteed@chromium.org/559623002/40001

[commit-bot: I haz the power, 2014-09-09 22:14:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-09 22:14:48]

Try jobs failed on following builders:
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dougsteed, 2014-09-09 22:25:35]

The CQ bit was checked by dougsteed@chromium.org

[commit-bot: I haz the power, 2014-09-09 22:26:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dougsteed@chromium.org/559623002/60001

[commit-bot: I haz the power, 2014-09-09 22:36:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-09 22:36:38]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Ryan Sleevi, 2014-09-09 22:40:35]

On 2014/09/09 22:36:38, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

Update description before relanding please.

This was reviewed by David.

[Ryan Sleevi, 2014-09-09 22:40:37]

On 2014/09/09 22:36:38, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

Update description before relanding please.

This was reviewed by David.

[dougsteed, 2014-09-09 23:01:26]

On 2014/09/09 22:40:37, Ryan Sleevi wrote:
> On 2014/09/09 22:36:38, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   chromium_presubmit on tryserver.chromium.linux
> >
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
> 
> Update description before relanding please.
> 
> This was reviewed by David.

I didn't realize that sometimes neither USE_NSS nor USE_OPENSSL are defined.
This means that sometimes CreateFromKey is not defined.
So the unittest that relies on it needs to be made conditional.

PTAL.

[davidben, 2014-09-10 15:51:57]

davidben@chromium.org changed reviewers:
+ rsleevi@chromium.org

[davidben, 2014-09-10 15:51:57]

USE_NSS changes lgtm, though note that this means you won't get this API on
desktop platforms outside of Linux and CrOS until we migrate everything to
OpenSSL. Is that fine for your use case?

I also don't have OWNERS coverage for crypto, so you still need a stamp from
Ryan. (Adding him back to the reviewers list for that.)

[dougsteed, 2014-09-10 16:58:58]

On 2014/09/10 15:51:57, David Benjamin wrote:
> USE_NSS changes lgtm, though note that this means you won't get this API on
> desktop platforms outside of Linux and CrOS until we migrate everything to
> OpenSSL. Is that fine for your use case?
> 
> I also don't have OWNERS coverage for crypto, so you still need a stamp from
> Ryan. (Adding him back to the reviewers list for that.)

Thanks. We actually only need this on chromecast right now, which is Linux.

[dougsteed, 2014-09-10 21:38:22]

On 2014/09/10 16:58:58, dougsteed wrote:
> On 2014/09/10 15:51:57, David Benjamin wrote:
> > USE_NSS changes lgtm, though note that this means you won't get this API on
> > desktop platforms outside of Linux and CrOS until we migrate everything to
> > OpenSSL. Is that fine for your use case?
> > 
> > I also don't have OWNERS coverage for crypto, so you still need a stamp from
> > Ryan. (Adding him back to the reviewers list for that.)
> 
> Thanks. We actually only need this on chromecast right now, which is Linux.

Ryan: David has LGTM'ed but I still need crypto OWNER approval. Sorry, still
trying to figure out the review flow protocol.

[Ryan Sleevi, 2014-09-10 21:50:15]

David needs to stop slacking and add himself to //crypto/OWNERS

LGTM though :)

[dougsteed, 2014-09-10 22:42:37]

The CQ bit was checked by dougsteed@chromium.org

[commit-bot: I haz the power, 2014-09-10 22:51:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/559623002/60001

[commit-bot: I haz the power, 2014-09-10 23:23:13]

Committed patchset #4 (id:60001) as 2eb182c3a09391f4ec4f3254e78c3f71a9c096ab

[commit-bot: I haz the power, 2014-09-10 23:27:20]

Patchset 4 (id:??) landed as
https://crrev.com/db7726aee7993008dff742790337f950fb371ebf
Cr-Commit-Position: refs/heads/master@{#294254}