Fixes to the SHA256 X509 Certificate fingerprint API.

Fixes to the SHA256 X509 Certificate fingerprint API.

In the original commit (285851), I missed several nits that wtc@ later pointed
out. This commit addresses those corrections and suggestions.

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

[jww, 2014-07-28 18:40:25]

Responding to wtc@'s comments in https://codereview.chromium.org/412263003/
below. rsleevi, your thoughts would be much appreciated.

On 2014/07/28 17:29:52, wtc wrote:
> Review comments on patch set 4:
> 
> 1. These SHA-256 fingerprint functions don't seem to belong in a minimal API
> because they can be implemented using existing public functions.
True although I general feel that we should be providing APIs to perform the
(correct) crypto operations for consumers elsewhere. For example, one of the
advantages of this API is we can put a big warning in the comments (as we've
done) saying "use *this* function for secure operations, not that SHA-1
business". It seems likely that commits could slip by that use the insecure
SHA-1 operation, and this might help alleviate it.
> 
> 2. The way OSCertHandles is hashed is just one possible definition -- another
> possible definition would include the number of certificates in the hash,
> similar to the code in X509Certificate::Persist. So it's not clear if the
> current definition is generally useful.
True, although this would be inconsistent with how the SHA1 API does it.

[wtc, 2014-07-28 19:26:27]

Patch set 1 LGTM. Thanks. Please undo one change I noted below before committing
this CL.

https://codereview.chromium.org/421243003/diff/1/net/cert/x509_certificate_un...
File net/cert/x509_certificate_unittest.cc (left):

https://codereview.chromium.org/421243003/diff/1/net/cert/x509_certificate_un...
net/cert/x509_certificate_unittest.cc:394: static const uint8
cert_chain3_ca_fingerprint_256[32] = {

Don't rename these three variables. They match the three variables on lines
359-371.

[jww, 2014-07-29 19:00:53]

Thanks! rsleevi, I'll wait for your lgtm.

On 2014/07/28 19:26:27, wtc wrote:
> Patch set 1 LGTM. Thanks. Please undo one change I noted below before
committing
> this CL.
> 
>
https://codereview.chromium.org/421243003/diff/1/net/cert/x509_certificate_un...
> File net/cert/x509_certificate_unittest.cc (left):
> 
>
https://codereview.chromium.org/421243003/diff/1/net/cert/x509_certificate_un...
> net/cert/x509_certificate_unittest.cc:394: static const uint8
> cert_chain3_ca_fingerprint_256[32] = {
> 
> Don't rename these three variables. They match the three variables on lines
> 359-371.

[wtc, 2014-07-29 19:24:52]

Patch set 2 LGTM.

[Ryan Sleevi, 2014-07-29 19:44:59]

On 2014/07/29 19:24:52, wtc wrote:
> Patch set 2 LGTM.

Don't wait for me

[jww, 2014-07-29 20:11:29]

The CQ bit was checked by jww@chromium.org

[commit-bot: I haz the power, 2014-07-29 20:13:27]

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

[commit-bot: I haz the power, 2014-07-29 23:20:01]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-07-30 01:25:39]

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

[commit-bot: I haz the power, 2014-07-30 01:25:39]

Try jobs failed on following builders:
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[jww, 2014-07-30 01:32:21]

The CQ bit was checked by jww@chromium.org

[commit-bot: I haz the power, 2014-07-30 01:34:11]

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

[commit-bot: I haz the power, 2014-07-30 11:24:51]

Change committed as 286477