Add unit tests for NSS-backed SSLPrivateKeys.

Add unit tests for NSS-backed SSLPrivateKeys.

BUG=673058
Committed: https://crrev.com/983d610b007a9e5dd5d59fa64fae830f8db8bdd0
Cr-Commit-Position: refs/heads/master@{#438593}

[davidben, 2016-12-14 15:28:37]

The CQ bit was checked by davidben@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-14 15:28:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-14 15:33:03]

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

[commit-bot: I haz the power, 2016-12-14 15:33:04]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[davidben, 2016-12-14 15:59:12]

The CQ bit was checked by davidben@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-14 15:59:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[davidben, 2016-12-14 16:03:05]

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

[davidben, 2016-12-14 16:03:06]

NSS's EC key situation is nuts...

https://codereview.chromium.org/2577683002/diff/20001/net/ssl/ssl_platform_ke...
File net/ssl/ssl_platform_key_chromecast.cc (left):

https://codereview.chromium.org/2577683002/diff/20001/net/ssl/ssl_platform_ke...
net/ssl/ssl_platform_key_chromecast.cc:46: SSLPrivateKey::Hash::MD5_SHA1};
MD5-SHA1 is weird and isn't advertised. (Doesn't make a difference whether we
advertise here it or not since SSLClientSocketImpl will filter it out, but other
SSLPrivateKey implementations don't list it.) The prefs are used for TLS 1.1 and
below where you always sign MD5-SHA1 for RSA.

[commit-bot: I haz the power, 2016-12-14 17:36:10]

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

[commit-bot: I haz the power, 2016-12-14 17:36:11]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[davidben, 2016-12-14 17:56:52]

The linux_chromium_asan_rel_ng failure is probably the same as
https://crbug.com/674179. Looking at that right now.

[davidben, 2016-12-14 18:21:44]

The CQ bit was checked by davidben@chromium.org to run a CQ dry run

[davidben, 2016-12-14 18:22:18]

On 2016/12/14 17:56:52, davidben wrote:
> The linux_chromium_asan_rel_ng failure is probably the same as
> https://crbug.com/674179. Looking at that right now.

Figured it out and rebased this atop it.

[commit-bot: I haz the power, 2016-12-14 18:22:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ryan Sleevi, 2016-12-14 18:42:40]

LGTM w/ 1 question

https://codereview.chromium.org/2577683002/diff/40001/net/ssl/ssl_platform_ke...
File net/ssl/ssl_platform_key_chromecast.cc (left):

https://codereview.chromium.org/2577683002/diff/40001/net/ssl/ssl_platform_ke...
net/ssl/ssl_platform_key_chromecast.cc:46: SSLPrivateKey::Hash::MD5_SHA1};
This change isn't obvious - was removing MD5_SHA1 intentional (because of the
RSA special logic)? Or is it because the TPM doesn't support it?

https://codereview.chromium.org/2577683002/diff/40001/net/ssl/ssl_platform_ke...
File net/ssl/ssl_platform_key_nss_unittest.cc (right):

https://codereview.chromium.org/2577683002/diff/40001/net/ssl/ssl_platform_ke...
net/ssl/ssl_platform_key_nss_unittest.cc:38: const char* cert_file;
const char* const is what gets it added to the RO, AIUI (can dig up the
chromium-dev thread); but I'm not sure how much that matters for a unittest

[davidben, 2016-12-14 18:49:18]

https://codereview.chromium.org/2577683002/diff/40001/net/ssl/ssl_platform_ke...
File net/ssl/ssl_platform_key_chromecast.cc (left):

https://codereview.chromium.org/2577683002/diff/40001/net/ssl/ssl_platform_ke...
net/ssl/ssl_platform_key_chromecast.cc:46: SSLPrivateKey::Hash::MD5_SHA1};
On 2016/12/14 18:42:39, Ryan Sleevi wrote:
> This change isn't obvious - was removing MD5_SHA1 intentional (because of the
> RSA special logic)? Or is it because the TPM doesn't support it?

Oh, I mentioned this on the previous patchset, but I forgot to transplant it on
re-upload. It's the first one:

MD5-SHA1 is weird and isn't advertised. (Doesn't make a difference whether we
advertise here it or not since SSLClientSocketImpl will filter it out, but other
SSLPrivateKey implementations don't list it.) The prefs are used for TLS 1.1 and
below where you always sign MD5-SHA1 for RSA. [Amended: The prefs are use for
TLS 1.2 and up. MD5-SHA1 is just a magic value for TLS 1.1 and below. Since it's
always used, we can just ask the caller to sign it and try it.]

https://codereview.chromium.org/2577683002/diff/40001/net/ssl/ssl_platform_ke...
File net/ssl/ssl_platform_key_nss_unittest.cc (right):

https://codereview.chromium.org/2577683002/diff/40001/net/ssl/ssl_platform_ke...
net/ssl/ssl_platform_key_nss_unittest.cc:38: const char* cert_file;
On 2016/12/14 18:42:39, Ryan Sleevi wrote:
> const char* const is what gets it added to the RO, AIUI (can dig up the
> chromium-dev thread); but I'm not sure how much that matters for a unittest

[Chatted out-of-band. Array is const, so we shouldn't need to individually
decorate the fields like that.]

[davidben, 2016-12-14 18:50:39]

The CQ bit was unchecked by davidben@chromium.org

[davidben, 2016-12-14 18:50:42]

The CQ bit was checked by davidben@chromium.org

[commit-bot: I haz the power, 2016-12-14 18:51:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-14 19:35:44]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481741442184890,
"parent_rev": "576cc9ba4e28f566849070bc9a174e603c2fb498", "commit_rev":
"149624c9a4bc56f7f915b52c0568939a3e6407b1"}

[commit-bot: I haz the power, 2016-12-14 19:36:12]

Description was changed from

==========
Add unit tests for NSS-backed SSLPrivateKeys.

BUG=673058
==========

to

==========
Add unit tests for NSS-backed SSLPrivateKeys.

BUG=673058

Review-Url: https://codereview.chromium.org/2577683002
==========

[commit-bot: I haz the power, 2016-12-14 19:36:14]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-14 19:39:18]

Description was changed from

==========
Add unit tests for NSS-backed SSLPrivateKeys.

BUG=673058

Review-Url: https://codereview.chromium.org/2577683002
==========

to

==========
Add unit tests for NSS-backed SSLPrivateKeys.

BUG=673058

Committed: https://crrev.com/983d610b007a9e5dd5d59fa64fae830f8db8bdd0
Cr-Commit-Position: refs/heads/master@{#438593}
==========

[commit-bot: I haz the power, 2016-12-14 19:39:19]

Patchset 3 (id:??) landed as
https://crrev.com/983d610b007a9e5dd5d59fa64fae830f8db8bdd0
Cr-Commit-Position: refs/heads/master@{#438593}