[refactor] Extract the CertVerifyResult assignment of has_md2, has_md4,

[refactor] Extract the CertVerifyResult assignment of has_md2, has_md4,
has_md5, has_sha1, has_sha1_leaf to a helper function.

This change is a prelude to simplifying the
CertVerifyProcWeakDigestTest tests.

BUG=649017
Review-Url: https://codereview.chromium.org/2610903003
Cr-Commit-Position: refs/heads/master@{#441801}
Committed: https://chromium.googlesource.com/chromium/src/+/b28b2b66578c862d563727cc3f30cf84aae942f0

[eroman, 2017-01-04 19:54:51]

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

[commit-bot: I haz the power, 2017-01-04 19:55:18]

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

[eroman, 2017-01-04 20:01:29]

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

[commit-bot: I haz the power, 2017-01-04 20:01:42]

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, 2017-01-04 20:14:22]

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

[commit-bot: I haz the power, 2017-01-04 20:14:23]

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

[eroman, 2017-01-05 22:16:48]

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

[commit-bot: I haz the power, 2017-01-05 22:17:26]

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

[eroman, 2017-01-05 22:22:42]

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

[commit-bot: I haz the power, 2017-01-05 22:23:10]

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

[eroman, 2017-01-05 22:26:59]

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

[commit-bot: I haz the power, 2017-01-05 22:27:10]

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

[eroman, 2017-01-05 22:27:12]

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

[commit-bot: I haz the power, 2017-01-05 22:36:09]

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

[commit-bot: I haz the power, 2017-01-05 22:36:09]

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-...)

[eroman, 2017-01-05 22:41:00]

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

[commit-bot: I haz the power, 2017-01-05 22:41:29]

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

[Ryan Sleevi, 2017-01-05 22:48:25]

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
File net/cert/cert_verify_proc.cc (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
net/cert/cert_verify_proc.cc:782: auto hash =
X509Certificate::GetSignatureHashAlgorithm(cert);
Seems like this is skirting very close to the "Con" (banned) examples in
https://google.github.io/styleguide/cppguide.html#auto

I suspect you're relying on "When the type doesn't matter because it isn't being
used for anything other than equality comparison." but that's... subtle.

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
File net/cert/cert_verify_proc.h (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
net/cert/cert_verify_proc.h:149: bool
FillCertVerifyResultWeakSignature(X509Certificate::OSCertHandle cert,
NAMING: The "FillCertVerifyResult" feels redundant with the method signature
(being that it takes a CertVerifyResult)?

static bool CheckWeakSignature(...) ?

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
net/cert/cert_verify_proc.h:149: bool
FillCertVerifyResultWeakSignature(X509Certificate::OSCertHandle cert,
DESIGN: From the header, it's unclear why the bool return value would be
meaningful/significant at all (versus making use of the |verify_result| flags
themselves). It feels like this becomes two things - a computation and a policy
decision.

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
net/cert/cert_verify_proc.h:149: bool
FillCertVerifyResultWeakSignature(X509Certificate::OSCertHandle cert,
DESIGN: Does this need to be a free function in the public header? Why not make
it a static (protected) method and friend the test?

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
File net/cert/cert_verify_proc_mac.cc (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
net/cert/cert_verify_proc_mac.cc:219: auto hash_is_weak =
I would definitely argue this use of "auto" is not appropriate. You definitely
don't save anything over 'bool', and lose in readability.

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
File net/cert/x509_certificate.h (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
net/cert/x509_certificate.h:83: // Enumeration for weak hashing algorithms.
This feels very much like a tighter coupling.

That is, the logical question is "Why should X509Certificate know what's
considered weak or not" - and historically, it hasn't (policy is left to
CertVerifyProc).

Since we're moving this code to X509Certificate, I'm curious whether it makes to
expand this enum (to include the other algorithms), or, simply from a design
standpoint, remove the comments about 'weak' and let it be extended (if needed)

// The digest algorithm used for the certificate's signature.

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
net/cert/x509_certificate.h:341: // fails, then will return
kSignatureHashAlgorithmOther.
// Returns the digest algorithm used in |cert_handle|'s signature.
// If the digest algorithm cannot be determined, or if it is not one
// of the explicitly enumerated values, kSignatureHashAlgorithmOther
// will be returned.
// NOTE: No validation of the signature is performed, and thus invalid
// signatures may result in seemingly meaningful values.

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
File net/cert/x509_certificate_ios.cc (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
net/cert/x509_certificate_ios.cc:368: // TODO(eroman): This duplicates code with
x509_certificate_openssl.cc
x509_util_openssl is where we move the dupe code between these two (like the
name & date parsing)

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
File net/cert/x509_certificate_mac.cc (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
net/cert/x509_certificate_mac.cc:527: x509_util::CSSMFieldValue signature_field;
suggestion: newline between 526 & 527

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
net/cert/x509_certificate_mac.cc:535: // safe.
Can delete 532-535. I should have left those as CL review comments. I would
suggest a newline between 531 and 536 though.

[eroman, 2017-01-05 23:21:54]

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
File net/cert/cert_verify_proc.h (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
net/cert/cert_verify_proc.h:149: bool
FillCertVerifyResultWeakSignature(X509Certificate::OSCertHandle cert,
On 2017/01/05 22:48:24, Ryan Sleevi wrote:
> NAMING: The "FillCertVerifyResult" feels redundant with the method signature
> (being that it takes a CertVerifyResult)?
> 
> static bool CheckWeakSignature(...) ?

RE: Static method vs free-floating function
The consumers of this function are all in free-floating helper functions, so
they cannot access a protected method. I could make it a public static method,
but that mostly just makes for a longer name.

RE: bool return value
This is used only by the mac verifier, which fills a boolean if a weak signature
is used. If I use the same function signature but without the return value the
consumer needs to check each of the boolean parameters to see if any of them
were set to true, which is a bit tricky/gross.

How about instead of a bool I return the hash algorithm enum? The mac
implementation can then do a switch on that and decide whether to set the "weak
signature" bool, and the other implementations can disregard it.

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
File net/cert/cert_verify_proc_mac.cc (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
net/cert/cert_verify_proc_mac.cc:219: auto hash_is_weak =
On 2017/01/05 22:48:24, Ryan Sleevi wrote:
> I would definitely argue this use of "auto" is not appropriate. You definitely
> don't save anything over 'bool', and lose in readability.

Agreed auto is wrong here (left over from earlier iteration).

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
File net/cert/x509_certificate.h (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
net/cert/x509_certificate.h:83: // Enumeration for weak hashing algorithms.
On 2017/01/05 22:48:24, Ryan Sleevi wrote:
> This feels very much like a tighter coupling.
> 
> That is, the logical question is "Why should X509Certificate know what's
> considered weak or not" - and historically, it hasn't (policy is left to
> CertVerifyProc).
> 
> Since we're moving this code to X509Certificate, I'm curious whether it makes
to
> expand this enum (to include the other algorithms), or, simply from a design
> standpoint, remove the comments about 'weak' and let it be extended (if
needed)
> 
> // The digest algorithm used for the certificate's signature.

Sure, I will remove the comment about it being "weak" ones.

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
File net/cert/x509_certificate_ios.cc (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
net/cert/x509_certificate_ios.cc:368: // TODO(eroman): This duplicates code with
x509_certificate_openssl.cc
On 2017/01/05 22:48:24, Ryan Sleevi wrote:
> x509_util_openssl is where we move the dupe code between these two (like the
> name & date parsing)

Want me to extract as part of this CL or a follow-up?
(The code is duplicated today, I preserved that duplication in this refactor).

[eroman, 2017-01-05 23:34:26]

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

[commit-bot: I haz the power, 2017-01-05 23:35:23]

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

[eroman, 2017-01-05 23:36:31]

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
File net/cert/cert_verify_proc.cc (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
net/cert/cert_verify_proc.cc:782: auto hash =
X509Certificate::GetSignatureHashAlgorithm(cert);
On 2017/01/05 22:48:24, Ryan Sleevi wrote:
> Seems like this is skirting very close to the "Con" (banned) examples in
> https://google.github.io/styleguide/cppguide.html#auto
> 
> I suspect you're relying on "When the type doesn't matter because it isn't
being
> used for anything other than equality comparison." but that's... subtle.

Done.

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
File net/cert/cert_verify_proc.h (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
net/cert/cert_verify_proc.h:149: bool
FillCertVerifyResultWeakSignature(X509Certificate::OSCertHandle cert,
On 2017/01/05 22:48:24, Ryan Sleevi wrote:
> DESIGN: From the header, it's unclear why the bool return value would be
> meaningful/significant at all (versus making use of the |verify_result| flags
> themselves). It feels like this becomes two things - a computation and a
policy
> decision.

No longer returns a bool -- now returns the algorithm enum.

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
net/cert/cert_verify_proc.h:149: bool
FillCertVerifyResultWeakSignature(X509Certificate::OSCertHandle cert,
On 2017/01/05 22:48:24, Ryan Sleevi wrote:
> NAMING: The "FillCertVerifyResult" feels redundant with the method signature
> (being that it takes a CertVerifyResult)?
> 
> static bool CheckWeakSignature(...) ?

I haven't changed the name yet.

I am not sure about CheckWeakSignature() given that it no longer returns a bool

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
File net/cert/cert_verify_proc_mac.cc (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
net/cert/cert_verify_proc_mac.cc:219: auto hash_is_weak =
On 2017/01/05 23:21:53, eroman (slow) wrote:
> On 2017/01/05 22:48:24, Ryan Sleevi wrote:
> > I would definitely argue this use of "auto" is not appropriate. You
definitely
> > don't save anything over 'bool', and lose in readability.
> 
> Agreed auto is wrong here (left over from earlier iteration).

Done.

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
File net/cert/x509_certificate.h (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
net/cert/x509_certificate.h:83: // Enumeration for weak hashing algorithms.
On 2017/01/05 23:21:53, eroman (slow) wrote:
> On 2017/01/05 22:48:24, Ryan Sleevi wrote:
> > This feels very much like a tighter coupling.
> > 
> > That is, the logical question is "Why should X509Certificate know what's
> > considered weak or not" - and historically, it hasn't (policy is left to
> > CertVerifyProc).
> > 
> > Since we're moving this code to X509Certificate, I'm curious whether it
makes
> to
> > expand this enum (to include the other algorithms), or, simply from a design
> > standpoint, remove the comments about 'weak' and let it be extended (if
> needed)
> > 
> > // The digest algorithm used for the certificate's signature.
> 
> Sure, I will remove the comment about it being "weak" ones.

Done.

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
net/cert/x509_certificate.h:341: // fails, then will return
kSignatureHashAlgorithmOther.
On 2017/01/05 22:48:24, Ryan Sleevi wrote:
> // Returns the digest algorithm used in |cert_handle|'s signature.
> // If the digest algorithm cannot be determined, or if it is not one
> // of the explicitly enumerated values, kSignatureHashAlgorithmOther
> // will be returned.
> // NOTE: No validation of the signature is performed, and thus invalid
> // signatures may result in seemingly meaningful values.

Done.

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
File net/cert/x509_certificate_ios.cc (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
net/cert/x509_certificate_ios.cc:368: // TODO(eroman): This duplicates code with
x509_certificate_openssl.cc
On 2017/01/05 23:21:53, eroman (slow) wrote:
> On 2017/01/05 22:48:24, Ryan Sleevi wrote:
> > x509_util_openssl is where we move the dupe code between these two (like the
> > name & date parsing)
> 
> Want me to extract as part of this CL or a follow-up?
> (The code is duplicated today, I preserved that duplication in this refactor).

Thanks for the heads up, I will leave that to a follow-up.

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
File net/cert/x509_certificate_mac.cc (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
net/cert/x509_certificate_mac.cc:527: x509_util::CSSMFieldValue signature_field;
On 2017/01/05 22:48:24, Ryan Sleevi wrote:
> suggestion: newline between 526 & 527

Done.

https://codereview.chromium.org/2610903003/diff/70015/net/cert/x509_certifica...
net/cert/x509_certificate_mac.cc:535: // safe.
On 2017/01/05 22:48:24, Ryan Sleevi wrote:
> Can delete 532-535. I should have left those as CL review comments. I would
> suggest a newline between 531 and 536 though.

Done.

[Ryan Sleevi, 2017-01-05 23:46:30]

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
File net/cert/cert_verify_proc.h (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
net/cert/cert_verify_proc.h:149: bool
FillCertVerifyResultWeakSignature(X509Certificate::OSCertHandle cert,
On 2017/01/05 23:21:53, eroman (slow) wrote:
> RE: Static method vs free-floating function
> The consumers of this function are all in free-floating helper functions, so
> they cannot access a protected method. I could make it a public static method,
> but that mostly just makes for a longer name.

I'm wondering whether we could/should consider uplifting the call to this (to
populate the CertVerifyResult) into CertVerifyProc::Verify (that is, moving it
out of the respective VerifyInternal implementations)

That would/should only affect the macOS implementation, which is checking to see
if the chain is weak (in which case, it triggers the extra building logic), but
that code doesn't care about has_md* so much as knowing that the enum was
something !other, and whether that was because of the leaf or the chain.

That's a larger refactoring than perhaps you intended, but might also  solve the
bool return value thing.

Concretely, I'm thinking of something like:
SignatureHashAlgorithm X509Certificate::GetSignatureHashAlgorithm(...)

void CertVerifyProc::FillWeakKeyInfo(X509Certificate* cert, CertVerifyResult*
verify_result) {
  SomeHelperMethod(cert->os_cert_handle(), verify_result);
  verify_result->has_sha1_leaf = verify_result->has_sha1;
  for (const auto& chain_cert : cert->GetIntermediateCertificates()) {
    SomeHelperMethod(chain_cert, verify_result);
  }
}

Where SomeHelperMethod just does enum -> bool flag flips


And then we'd call that somewhere around
https://cs.chromium.org/chromium/src/net/cert/cert_verify_proc.cc?rcl=0&l=469 or
https://cs.chromium.org/chromium/src/net/cert/cert_verify_proc.cc?rcl=0&l=456

Now, for the macOS code, which needs to know whether the chain is weak, we'd
update the logic in
https://cs.chromium.org/chromium/src/net/cert/cert_verify_proc_mac.cc?rcl=0&l...
to do something like:

bool weak_chain = false;
for (CFIndex i = 1 /* not 0 */, size = CFArrayGetCount(temp_chain); i < size;
++i) {
  weak_chain |=
(X509Certificate::X509Certificate::GetSignatureHashAlgorithm(CFArrayGetValueAtIndex(temp_chain,
i)) != X509Certificate::kSignatureHashAlgorithmOther);
}


That would/should allow us to fully 'hide' a lot of these details. WDYT?

[eroman, 2017-01-06 00:47:23]

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
File net/cert/cert_verify_proc.h (right):

https://codereview.chromium.org/2610903003/diff/70015/net/cert/cert_verify_pr...
net/cert/cert_verify_proc.h:149: bool
FillCertVerifyResultWeakSignature(X509Certificate::OSCertHandle cert,
On 2017/01/05 23:46:30, Ryan Sleevi wrote:
> On 2017/01/05 23:21:53, eroman (slow) wrote:
> > RE: Static method vs free-floating function
> > The consumers of this function are all in free-floating helper functions, so
> > they cannot access a protected method. I could make it a public static
method,
> > but that mostly just makes for a longer name.
> 
> I'm wondering whether we could/should consider uplifting the call to this (to
> populate the CertVerifyResult) into CertVerifyProc::Verify (that is, moving it
> out of the respective VerifyInternal implementations)
> 
> That would/should only affect the macOS implementation, which is checking to
see
> if the chain is weak (in which case, it triggers the extra building logic),
but
> that code doesn't care about has_md* so much as knowing that the enum was
> something !other, and whether that was because of the leaf or the chain.
> 
> That's a larger refactoring than perhaps you intended, but might also  solve
the
> bool return value thing.
> 
> Concretely, I'm thinking of something like:
> SignatureHashAlgorithm X509Certificate::GetSignatureHashAlgorithm(...)
> 
> void CertVerifyProc::FillWeakKeyInfo(X509Certificate* cert, CertVerifyResult*
> verify_result) {
>   SomeHelperMethod(cert->os_cert_handle(), verify_result);
>   verify_result->has_sha1_leaf = verify_result->has_sha1;
>   for (const auto& chain_cert : cert->GetIntermediateCertificates()) {
>     SomeHelperMethod(chain_cert, verify_result);
>   }
> }
> 
> Where SomeHelperMethod just does enum -> bool flag flips
> 
> 
> And then we'd call that somewhere around
> https://cs.chromium.org/chromium/src/net/cert/cert_verify_proc.cc?rcl=0&l=469
or
> https://cs.chromium.org/chromium/src/net/cert/cert_verify_proc.cc?rcl=0&l=456
> 
> Now, for the macOS code, which needs to know whether the chain is weak, we'd
> update the logic in
>
https://cs.chromium.org/chromium/src/net/cert/cert_verify_proc_mac.cc?rcl=0&l...
> to do something like:
> 
> bool weak_chain = false;
> for (CFIndex i = 1 /* not 0 */, size = CFArrayGetCount(temp_chain); i < size;
> ++i) {
>   weak_chain |=
>
(X509Certificate::X509Certificate::GetSignatureHashAlgorithm(CFArrayGetValueAtIndex(temp_chain,
> i)) != X509Certificate::kSignatureHashAlgorithmOther);
> }
> 
> 
> That would/should allow us to fully 'hide' a lot of these details. WDYT?

That sounds plausible, however TBH I am trying to avoid too much scope creep, as
this refactor is already 2 tasks removed from what I actually want to be getting
done :)

What you described sounds like it could be an iterative change that builds on
top of this code extraction.

What if I proceeded with the change at this layer, then the accompanying
unit-test simplification, (and then the parameterization change I have in
progress), and then can circle back to improvements that internalize this into
the shared cert verify proc code? Or do you feel strongly this needs to be
tackled in this iteration?

[Ryan Sleevi, 2017-01-06 00:49:28]

On 2017/01/06 00:47:23, eroman (slow) wrote:
> That sounds plausible, however TBH I am trying to avoid too much scope creep,
as
> this refactor is already 2 tasks removed from what I actually want to be
getting
> done :)
> 
> What you described sounds like it could be an iterative change that builds on
> top of this code extraction.
> 
> What if I proceeded with the change at this layer, then the accompanying
> unit-test simplification, (and then the parameterization change I have in
> progress), and then can circle back to improvements that internalize this into
> the shared cert verify proc code? Or do you feel strongly this needs to be
> tackled in this iteration?

Yeah, I struggled between whether or not that's a necessary change.

Since this right now is blocking the unit test simplification, let's go with
this as-is, and while you work on that simplification, I'll grab the cleanup in
parallel, since I don't think it'd block your stuff?

[Ryan Sleevi, 2017-01-06 00:51:29]

LGTM as is, and once this unblocks your stuff, I'll take another pass at
reducing this code.

[commit-bot: I haz the power, 2017-01-06 00:53:46]

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

[commit-bot: I haz the power, 2017-01-06 00:53:47]

Dry run: This issue passed the CQ dry run.

[eroman, 2017-01-06 00:59:49]

The CQ bit was checked by eroman@chromium.org

[commit-bot: I haz the power, 2017-01-06 01:00:29]

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, 2017-01-06 01:44:05]

CQ is committing da patch.

Bot data: {"patchset_id": 110001, "attempt_start_ts": 1483664389068620,
"parent_rev": "9abb5f3f351b0d3831f79da9980a00db4d7d4815", "commit_rev":
"b28b2b66578c862d563727cc3f30cf84aae942f0"}

[commit-bot: I haz the power, 2017-01-06 01:44:35]

Description was changed from

==========
[refactor] Extract the CertVerifyResult assignment of has_md2, has_md4,
has_md5, has_sha1, has_sha1_leaf to a helper function.

This change is a prelude to simplifying the
CertVerifyProcWeakDigestTest tests.

BUG=649017
==========

to

==========
[refactor] Extract the CertVerifyResult assignment of has_md2, has_md4,
has_md5, has_sha1, has_sha1_leaf to a helper function.

This change is a prelude to simplifying the
CertVerifyProcWeakDigestTest tests.

BUG=649017

Review-Url: https://codereview.chromium.org/2610903003
Cr-Commit-Position: refs/heads/master@{#441801}
Committed:
https://chromium.googlesource.com/chromium/src/+/b28b2b66578c862d563727cc3f30...
==========

[commit-bot: I haz the power, 2017-01-06 01:44:36]

Committed patchset #7 (id:110001) as
https://chromium.googlesource.com/chromium/src/+/b28b2b66578c862d563727cc3f30...