Certificate Transparency: Add UMA for whitelist status

Certificate Transparency: Add UMA for whitelist status

In order to figure out if non-compliant EV certs are non-compliant
because the client is missing the whitelist or the certificate is
not whitelisted, log the whitelist status to a histogram when a
non-compliant cert is encountered.

BUG=397458
Committed: https://crrev.com/1ab731dac180e92f8843a5b9303e6d881f4be8c6
Cr-Commit-Position: refs/heads/master@{#321302}

[Eran Messeri, 2015-03-07 20:46:25]

eranm@chromium.org changed reviewers:
+ jwd@chromium.org, rsleevi@chromium.org

[jwd, 2015-03-09 15:26:41]

https://codereview.chromium.org/985993003/diff/1/net/cert/cert_policy_enforce...
File net/cert/cert_policy_enforcer.cc (right):

https://codereview.chromium.org/985993003/diff/1/net/cert/cert_policy_enforce...
net/cert/cert_policy_enforcer.cc:252: ev_whitelist ? ev_whitelist->IsValid() :
false);
I'm concerned about combining the two false conditions here. Are you sure you
wouldn't rather track all three states (has valid, has invalid, doesn't have)
separately?

https://codereview.chromium.org/985993003/diff/1/tools/metrics/histograms/his...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/985993003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:20215: +<histogram
name="Net.SSL_EVWhitelistValidityForNonCompliantCert" enum="Boolean">
Can you use a more precise enum type, maybe BooleanValid? Or define your own.

https://codereview.chromium.org/985993003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:20221: +    Transparency requirement
encountered.
nit: "... is encountered."

[Ryan Sleevi, 2015-03-09 21:44:43]

https://codereview.chromium.org/985993003/diff/1/net/cert/cert_policy_enforce...
File net/cert/cert_policy_enforcer.cc (right):

https://codereview.chromium.org/985993003/diff/1/net/cert/cert_policy_enforce...
net/cert/cert_policy_enforcer.cc:252: ev_whitelist ? ev_whitelist->IsValid() :
false);
On 2015/03/09 15:26:41, Jesse Doherty wrote:
> I'm concerned about combining the two false conditions here. Are you sure you
> wouldn't rather track all three states (has valid, has invalid, doesn't have)
> separately?

+1

[Eran Messeri, 2015-03-10 21:16:35]

Thanks for the quick review, accepted all comments - PTAL.

https://codereview.chromium.org/985993003/diff/1/net/cert/cert_policy_enforce...
File net/cert/cert_policy_enforcer.cc (right):

https://codereview.chromium.org/985993003/diff/1/net/cert/cert_policy_enforce...
net/cert/cert_policy_enforcer.cc:252: ev_whitelist ? ev_whitelist->IsValid() :
false);
On 2015/03/09 21:44:43, Ryan Sleevi wrote:
> On 2015/03/09 15:26:41, Jesse Doherty wrote:
> > I'm concerned about combining the two false conditions here. Are you sure
you
> > wouldn't rather track all three states (has valid, has invalid, doesn't
have)
> > separately?
> 
> +1

Done - tracking all three states.

https://codereview.chromium.org/985993003/diff/1/tools/metrics/histograms/his...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/985993003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:20215: +<histogram
name="Net.SSL_EVWhitelistValidityForNonCompliantCert" enum="Boolean">
On 2015/03/09 15:26:41, Jesse Doherty wrote:
> Can you use a more precise enum type, maybe BooleanValid? Or define your own.

Done - defined my own. Can use BooleanValid if you prefer, although the name
itself is suspicious.

https://codereview.chromium.org/985993003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:20221: +    Transparency requirement
encountered.
On 2015/03/09 15:26:41, Jesse Doherty wrote:
> nit: "... is encountered."

Done.

[jwd, 2015-03-12 16:51:47]

lgtm

[Eran Messeri, 2015-03-15 16:25:54]

The CQ bit was checked by eranm@chromium.org

[commit-bot: I haz the power, 2015-03-15 16:26:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/985993003/80001

[commit-bot: I haz the power, 2015-03-15 16:43:10]

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

[commit-bot: I haz the power, 2015-03-15 16:43:11]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Ryan Sleevi, 2015-03-19 03:17:39]

LGTM so sorry about that

[Ryan Sleevi, 2015-03-19 03:17:48]

The CQ bit was checked by rsleevi@chromium.org

[commit-bot: I haz the power, 2015-03-19 03:18:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/985993003/80001

[commit-bot: I haz the power, 2015-03-19 05:43:07]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-03-19 05:43:39]

Patchset 5 (id:??) landed as
https://crrev.com/1ab731dac180e92f8843a5b9303e6d881f4be8c6
Cr-Commit-Position: refs/heads/master@{#321302}