1254653 - Add telemetry to measure how often we encounter EV certificates

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Richard Barnes [:rbarnes]]

It would be good to know how much usage EV is actually getting in practice.

Comment 1

[Richard Barnes [:rbarnes]]

Attachment: MozReview Request: Bug 1254653 - Add telemetry to measure how often we encounter EV certificates r?keeler

Review commit: https://reviewboard.mozilla.org/r/38789/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38789/

Comment 2

[Dana Keeler (she/her) [:keeler]]

Comment on attachment 8728115 [details]
MozReview Request: Bug 1254653 - Add telemetry to measure how often we encounter EV certificates r?keeler

https://reviewboard.mozilla.org/r/38789/#review35469

::: security/manager/ssl/SSLServerCertVerification.cpp:1246
(Diff revision 1)
> +  uint32_t evStatus = ((rv == SECSuccess)? 2 : 0) +

From an API standpoint, if rv != SECSuccess, evOidPolicy won't necessarily have a meaningful value. Looking at the actual code, this would work in cases where the certificate successfully validates as EV but then isn't valid for the given hostname (for example). That said, I'm not sure we want to break the "if the return value indicates failure, no output variables are valid" guideline for this. Would having 3 buckets work for the data you want? 0: failed to validate, 1: DV, 2: EV?

::: security/manager/ssl/SSLServerCertVerification.cpp:1247
(Diff revision 1)
> +                      ((evOidPolicy != SEC_OID_UNKNOWN)? 1: 0);

nit: spaces around operators: a ? b : c

::: toolkit/components/telemetry/Histograms.json:8154
(Diff revision 1)
> +  "CERT_EV_STATUS": {

I think technically you need a privacy peer to sign off on this: https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Adding_a_new_Telemetry_probe

Comment 3

[Richard Barnes [:rbarnes]]

Comment on attachment 8728115 [details]
MozReview Request: Bug 1254653 - Add telemetry to measure how often we encounter EV certificates r?keeler

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38789/diff/1-2/

Comment 4

[Richard Barnes [:rbarnes]]

https://reviewboard.mozilla.org/r/38789/#review35469

> From an API standpoint, if rv != SECSuccess, evOidPolicy won't necessarily have a meaningful value. Looking at the actual code, this would work in cases where the certificate successfully validates as EV but then isn't valid for the given hostname (for example). That said, I'm not sure we want to break the "if the return value indicates failure, no output variables are valid" guideline for this. Would having 3 buckets work for the data you want? 0: failed to validate, 1: DV, 2: EV?

Yep, that'll be fine.

> nit: spaces around operators: a ? b : c

Borrowing a pattern from AccumulateNonECCKeySize instead.

Comment 5

[Benjamin Smedberg]

https://reviewboard.mozilla.org/r/38789/#review35601

::: toolkit/components/telemetry/Histograms.json:8160
(Diff revision 2)
> +    "description": "How often do we encounter EV certificates? 0=invalid/DV, 1=invalid/EV, 2=valid/DV, 3=valid/EV"

Could you phrase this in the form of a statement instead of a question? e.g.

"Recorded for every TLS connection attempted."

Obviously I made this up because I don't know if there is caching or other bits involved. Or is this only recorded for a subset of TLS connections that have already passed other checks?

Who is going to monitor this permanently and how will that be done? Note that telemetry alerts are going to be useless for this; they are currently only useful for scalar data such as performance. Do you intend to monitor the total volume of certificates or just the ratios between them? Is there somebody I can ping in particular every 6 months to make sure that this data is still valuable?

Comment 6

[Richard Barnes [:rbarnes]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #5)
> https://reviewboard.mozilla.org/r/38789/#review35601
> 
> ::: toolkit/components/telemetry/Histograms.json:8160
> (Diff revision 2)
> > +    "description": "How often do we encounter EV certificates? 0=invalid/DV, 1=invalid/EV, 2=valid/DV, 3=valid/EV"
> 
> Could you phrase this in the form of a statement instead of a question? e.g.
> 
> "Recorded for every TLS connection attempted."

Done.  Also updated the values to match the revisions after keeler's comments.

> Obviously I made this up because I don't know if there is caching or other
> bits involved. Or is this only recorded for a subset of TLS connections that
> have already passed other checks?

This is done once per TLS connection; no caching.


> Who is going to monitor this permanently and how will that be done? Note
> that telemetry alerts are going to be useless for this; they are currently
> only useful for scalar data such as performance. Do you intend to monitor
> the total volume of certificates or just the ratios between them? Is there
> somebody I can ping in particular every 6 months to make sure that this data
> is still valuable?

This is going to be one of the longitudinal metrics that we use to monitor the certificate ecosystem, like HTTP_PAGELOAD_IS_SSL.  User benefit is not coming from solving a specific problem in the browser, but rather by helping us keep track of how Firefox sees the PKI in general.

I know alerts are useless here.  I just included alert_emails because it was required :)

Comment 7

[Richard Barnes [:rbarnes]]

Comment on attachment 8728115 [details]
MozReview Request: Bug 1254653 - Add telemetry to measure how often we encounter EV certificates r?keeler

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38789/diff/2-3/

Comment 8

[Benjamin Smedberg]

Comment on attachment 8728115 [details]
MozReview Request: Bug 1254653 - Add telemetry to measure how often we encounter EV certificates r?keeler

data-review=me

If you have a bug report specifically for dashboards or longitudinal monitoring, I'd love to be cc'ed on them.

Comment 9

[Dana Keeler (she/her) [:keeler]]

Comment on attachment 8728115 [details]
MozReview Request: Bug 1254653 - Add telemetry to measure how often we encounter EV certificates r?keeler

https://reviewboard.mozilla.org/r/38789/#review35633

LGTM.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2cd3d18cdb2e

Comment 11

[Richard Barnes [:rbarnes]]

Comment on attachment 8728115 [details]
MozReview Request: Bug 1254653 - Add telemetry to measure how often we encounter EV certificates r?keeler

Approval Request Comment
[Feature/regressing bug #]: n/a
[User impact if declined]: Delay in getting measurements
[Describe test coverage new/current, TreeHerder]: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=2cd3d18cdb2e
[Risks and why]: Very minor; no logic change, just a telemetry probe
[String/UUID change made/needed]: n/a

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2cd3d18cdb2e

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8728115 [details]
MozReview Request: Bug 1254653 - Add telemetry to measure how often we encounter EV certificates r?keeler

New telemetry probe for TLS, taking it.

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5c529f1c218f