Introduce RSAPrivateKey::SignDigest

Introduce RSAPrivateKey::SignDigest

BUG=258017
R=rsleevi@chromium.org

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

[pfeldman, 2013-07-08 13:02:35]

RSA verification is a part of raw ADB handshake. Device (Android) is sending 20
bytes SHA1 digest and expects Client (Chrome) to sign it. The handshake is
structured in a way, that the digest, not the data is being signed. Hence
RSA_sign is exposed in RSAPrivateKey on top of corresponding OpenSSL and NSS
implementations.

[Ryan Sleevi, 2013-07-08 18:09:36]

Not LGTM.

RSAPrivateKey's single responsibility is maintaining a handle to an RSA key in
an RAII-safe way and exposing properties of the keys.

Actions like creating signatures (whether PKCS#1v1.5 RSAES or PKCS#1v2.1
RSA-PSS) should be handled by SignatureCreator and SignatureVerifier.

You can look at http://crrev.com/209178 to see how PSS support was added to
SignatureVerifier, as a point of comparison for thinking about how it should be
added to SignatureCreator.

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

https://codereview.chromium.org/18697003/diff/1/crypto/rsa_private_key.h#newc...
crypto/rsa_private_key.h:227: std::vector<uint8>* output) const;
This shouldn't really hang off RSAPrivateKey.

We've intentionally created the SignatureCreator interface to disconnect the
keys from operations that may be performed with the keys.

[pfeldman, 2013-07-08 20:42:50]

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

https://codereview.chromium.org/18697003/diff/1/crypto/rsa_private_key.h#newc...
crypto/rsa_private_key.h:227: std::vector<uint8>* output) const;
On 2013/07/08 18:09:37, Ryan Sleevi wrote:
> This shouldn't really hang off RSAPrivateKey.
> 
> We've intentionally created the SignatureCreator interface to disconnect the
> keys from operations that may be performed with the keys.

Done.

[Ryan Sleevi, 2013-07-08 21:56:20]

LGTM

https://codereview.chromium.org/18697003/diff/22002/crypto/signature_creator.h
File crypto/signature_creator.h (right):

https://codereview.chromium.org/18697003/diff/22002/crypto/signature_creator....
crypto/signature_creator.h:38: // specified in PKCS #1.
comment nit: PKCS #1 v1.5.

Which at least makes it clearer that it's RSA-SSA (which everyone calls PKCS#1
v1.5, since that was the only signature form listed)

https://codereview.chromium.org/18697003/diff/22002/crypto/signature_creator_...
File crypto/signature_creator_unittest.cc (right):

https://codereview.chromium.org/18697003/diff/22002/crypto/signature_creator_...
crypto/signature_creator_unittest.cc:17: // TODO(aa): Factor this out into some
shared location.
comment nit: remove the TODO. Ain't gonna happen anytime soon :)

[Ryan Sleevi, 2013-07-08 21:57:25]

https://codereview.chromium.org/18697003/diff/22002/crypto/signature_creator_...
File crypto/signature_creator_unittest.cc (right):

https://codereview.chromium.org/18697003/diff/22002/crypto/signature_creator_...
crypto/signature_creator_unittest.cc:82: &signature));
style nit: I think it's only four spaces indent here (eg: see line 53 or 90)

[pfeldman, 2013-07-09 05:21:07]

https://codereview.chromium.org/18697003/diff/22002/crypto/signature_creator.h
File crypto/signature_creator.h (right):

https://codereview.chromium.org/18697003/diff/22002/crypto/signature_creator....
crypto/signature_creator.h:38: // specified in PKCS #1.
On 2013/07/08 21:56:20, Ryan Sleevi wrote:
> comment nit: PKCS #1 v1.5.
> 
> Which at least makes it clearer that it's RSA-SSA (which everyone calls PKCS#1
> v1.5, since that was the only signature form listed)

Done.

https://codereview.chromium.org/18697003/diff/22002/crypto/signature_creator_...
File crypto/signature_creator_unittest.cc (right):

https://codereview.chromium.org/18697003/diff/22002/crypto/signature_creator_...
crypto/signature_creator_unittest.cc:17: // TODO(aa): Factor this out into some
shared location.
On 2013/07/08 21:56:20, Ryan Sleevi wrote:
> comment nit: remove the TODO. Ain't gonna happen anytime soon :)

Done.

https://codereview.chromium.org/18697003/diff/22002/crypto/signature_creator_...
crypto/signature_creator_unittest.cc:82: &signature));
On 2013/07/08 21:57:25, Ryan Sleevi wrote:
> style nit: I think it's only four spaces indent here (eg: see line 53 or 90)

Done.

[commit-bot: I haz the power, 2013-07-09 05:21:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pfeldman@chromium.org/18697003/30001

[pfeldman, 2013-07-09 08:33:00]

Committed patchset #3 manually as r210524 (presubmit successful).