Implements RSAPrivateKey for openssl.

Implements RSAPrivateKey for openssl.
Leaves the declared NSS-only methods (CreateSensitive.. & FindFromPublicKeyInfo) as UNIMPLEMENTED.

BUG=None
TEST=base_unittests --gtest_filter=RSA*

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=66438

[bulach, 2010-11-17 11:52:29]

LGTM

thanks joth! few suggestions below:

http://codereview.chromium.org/5047003/diff/2001/base/crypto/rsa_private_key_...
File base/crypto/rsa_private_key_openssl.cc (right):

http://codereview.chromium.org/5047003/diff/2001/base/crypto/rsa_private_key_...
base/crypto/rsa_private_key_openssl.cc:30: int res = (export_private ?
i2d_PKCS8PrivateKeyInfo_bio : i2d_PUBKEY_bio)(
the object code would probably be the same, by I think it'd  be more readable to
split:

typedef int (Function)(BIO*, EVP_PKEY*);

// Choose the appropriate function depending on the flag.
Function* f = export_private ? i2d_PKCS8PrivateKeyInfo_bio : i2d_PUBKEY_bio;

int res = (*f)(bio.get(), key);

http://codereview.chromium.org/5047003/diff/2001/base/crypto/rsa_private_key_...
base/crypto/rsa_private_key_openssl.cc:57: scoped_ptr<RSAPrivateKey> self(new
RSAPrivateKey);
not really "self", perhaps new_key ?

http://codereview.chromium.org/5047003/diff/2001/base/crypto/rsa_private_key_...
base/crypto/rsa_private_key_openssl.cc:91: scoped_ptr<RSAPrivateKey> self(new
RSAPrivateKey);
ditto

http://codereview.chromium.org/5047003/diff/2001/base/crypto/rsa_private_key_...
base/crypto/rsa_private_key_openssl.cc:120: EVP_PKEY_free(key_);
we should really add release to the ScopedOpenSSL..

[joth, 2010-11-17 14:31:24]

Thanks! Let me know if you has another idea about ScopedOpenSSL....?

http://codereview.chromium.org/5047003/diff/2001/base/crypto/rsa_private_key_...
File base/crypto/rsa_private_key_openssl.cc (right):

http://codereview.chromium.org/5047003/diff/2001/base/crypto/rsa_private_key_...
base/crypto/rsa_private_key_openssl.cc:30: int res = (export_private ?
i2d_PKCS8PrivateKeyInfo_bio : i2d_PUBKEY_bio)(
On 2010/11/17 11:52:30, bulach wrote:
> the object code would probably be the same, by I think it'd  be more readable
to
> split:
> 
> typedef int (Function)(BIO*, EVP_PKEY*);
> 
> // Choose the appropriate function depending on the flag.
> Function* f = export_private ? i2d_PKCS8PrivateKeyInfo_bio : i2d_PUBKEY_bio;
> 
> int res = (*f)(bio.get(), key);

Once we have the typedef, I think it's even clearer to just pass the function
pointer in instead of a bool.
-> Done.

http://codereview.chromium.org/5047003/diff/2001/base/crypto/rsa_private_key_...
base/crypto/rsa_private_key_openssl.cc:57: scoped_ptr<RSAPrivateKey> self(new
RSAPrivateKey);
On 2010/11/17 11:52:30, bulach wrote:
> not really "self", perhaps new_key ?

Huh, I've got a habit of using self as a surrogate 'this' in static factory
functions. The other platforms use result, so switched to that.

http://codereview.chromium.org/5047003/diff/2001/base/crypto/rsa_private_key_...
base/crypto/rsa_private_key_openssl.cc:91: scoped_ptr<RSAPrivateKey> self(new
RSAPrivateKey);
On 2010/11/17 11:52:30, bulach wrote:
> ditto

Done.

http://codereview.chromium.org/5047003/diff/2001/base/crypto/rsa_private_key_...
base/crypto/rsa_private_key_openssl.cc:120: EVP_PKEY_free(key_);
On 2010/11/17 11:52:30, bulach wrote:
> we should really add release to the ScopedOpenSSL..

I think you mean reset() ?
I'm doing that in my next CL, but it won't really help here as I'd then have to
pull openssl_util.h and worse <openssl/evp.h> into the cross platform header
that declares this class, which doesn't sound good.

[agl, 2010-11-17 14:32:32]

LGTM

http://codereview.chromium.org/5047003/diff/7001/base/crypto/rsa_private_key_...
File base/crypto/rsa_private_key_openssl.cc (right):

http://codereview.chromium.org/5047003/diff/7001/base/crypto/rsa_private_key_...
base/crypto/rsa_private_key_openssl.cc:26: // |export_private| is true the full
key is exported, otherwise only the
export_private doesn't appear to be an argument.

[joth, 2010-11-17 14:35:19]

Thanks! Yep I forgot to update that comment...

On 17 November 2010 14:32, <agl@chromium.org> wrote:

> LGTM
>
>
>
>
http://codereview.chromium.org/5047003/diff/7001/base/crypto/rsa_private_key_...
>
> File base/crypto/rsa_private_key_openssl.cc (right):
>
>
>
http://codereview.chromium.org/5047003/diff/7001/base/crypto/rsa_private_key_...
> base/crypto/rsa_private_key_openssl.cc:26: // |export_private| is true
> the full key is exported, otherwise only the
> export_private doesn't appear to be an argument.
>
>
> http://codereview.chromium.org/5047003/
>