Closed Bug 1320567 Opened 8 years ago Closed 8 years ago

Certificate Transparency - telemetry reports of CT Policy compliance

Categories

(Core :: Security: PSM, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: sergei.cv, Assigned: sergei.cv)

References

(Blocks 1 open bug)

Details

(Whiteboard: [psm-assigned])

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.98 Safari/537.36
Blocks: 1281469
Priority: -- → P2
Whiteboard: [psm-backlog]
This patch introduces new telemetry probes related to CT Policy compliance of secure connections: 1. Compliance status of connections with EV certificates. 2. Counts of compliant and non-compliant connections per root CA.
Comment on attachment 8831741 [details] Bug 1320567 - Certificate Transparency - telemetry reports of CT Policy compliance. https://reviewboard.mozilla.org/r/108288/#review109490 Awesome! I think all you need is data-review from a telemetry peer? ::: security/manager/ssl/SSLServerCertVerification.cpp (Diff revision 1) > - if (CERT_LIST_END(endEntityNode, certList) || > - CERT_LIST_END(rootNode, certList)) { > - return; > - } > - CERTCertificate* cert = endEntityNode->cert; > - MOZ_ASSERT(cert); I think it would probably be best to keep the assertions/null checks in these functions.
Attachment #8831741 - Flags: review?(dkeeler) → review+
Assignee: nobody → sergei.cv
Priority: P2 → P1
Whiteboard: [psm-backlog] → [psm-assigned]
Telemetry Review ================ We'd like to gather telemetry data on Certificate Transparency (CT) policy compliance of some kinds of secure connections. Currently, CT policy compliance is evaluated but not enforced, and before enabling the enforcement we'd like to understand the implications. The probes we are interested in are: 1. CT policy compliance of EV certificates. The intention is to eventually revoke the EV status from non-compliant certificates. 2. Per-CA counts of compliant and non-compliant certificates. This is to require CT compliance of certificates issued by CAs which are known to be CT compliant. Because of the implications of the policy enforcement, we need the collection to be opt-out. Per-CA probes also need to be whitelisted for the number of histogram buckets. The CT policy we are evaluating is expected to change along the way; there is currently no ETA for turning the enforcement on. If limiting the collection by date, please allow us a longer collection period and an option to extend it later if needed. Please see the Histograms.json / histogram-whitelists.json files in ReviewBoard. Thanks!
Flags: needinfo?(benjamin)
Comment on attachment 8831741 [details] Bug 1320567 - Certificate Transparency - telemetry reports of CT Policy compliance. https://reviewboard.mozilla.org/r/108288/#review111542 ::: toolkit/components/telemetry/Histograms.json:8513 (Diff revision 2) > + "expires_in_version": "never", > + "kind": "enumerated", > + "n_values": 10, > + "bug_numbers": [1320567], > + "releaseChannelCollection": "opt-out", > + "description": "Certificate Transparency Policy compliance of SSL connections with EV certificate (1=Compliant, 2=Insufficient number of SCTs, 3=Insufficient diversity of CT Log operators)" Now that we have categorical histograms, that might be a better choice than enumerated, but it's not required. ::: toolkit/components/telemetry/Histograms.json:8522 (Diff revision 2) > + "expires_in_version": "never", > + "kind": "enumerated", > + "n_values": 256, > + "bug_numbers": [1320567], > + "releaseChannelCollection": "opt-out", > + "description": "SSL connections compliant with the Certificate Transparency Policy, by CA (see RootHashes.inc for names of CAs)" please include either a full path ( security/manager/ssl/RootHashes.inc ) or a DXR link here and below, so that people on t.m.o can find this file. Please be more specific about when this is recorded: once for every successfully-established TLS connection? Is anything at all recorded for user-added/customized roots? That should be documented as well. ::: toolkit/components/telemetry/histogram-whitelists.json:1830 (Diff revision 2) > "CERT_PINNING_MOZ_TEST_RESULTS_BY_HOST", > "GFX_CRASH", > "GC_REASON_2", > "GC_MINOR_REASON", > - "GC_MINOR_REASON_LONG" > + "GC_MINOR_REASON_LONG", > + "SSL_CT_POLICY_COMPLIANT_CONNECTIONS_BY_CA", Please do not add any new histograms to this whitelist. You shouldn't need to anyway.
In terms of the timeframe for collection, this seems like an good candidate for permanent data collection if your team is planning on monitoring it permanently. Can you briefly describe what kind of dashboards or alerting system you're going to use for permanent monitoring? If you aren't sure about the dashboards, let's start out with 6 months (expires in FF59) and you can renew and come up with a plan for permanent monitoring.
Flags: needinfo?(benjamin)
I'd like to see the answer about user-added/customized roots before marking final data-r.
Flags: needinfo?(sergei.cv)
(In reply to Benjamin Smedberg [:bsmedberg] from comment #7) > In terms of the timeframe for collection, this seems like an good candidate > for permanent data collection if your team is planning on monitoring it > permanently. Can you briefly describe what kind of dashboards or alerting > system you're going to use for permanent monitoring? We (used to) have a dashboard for per-CA verification (with and without pinning) successes/failures at https://people-mozilla.org/~dkeeler/ca-telemetry-dashboard/ but it's been broken by changes to the telemetry API, so it will have to be fixed. When it's working, we can extend it to monitor CT. > If you aren't sure about the dashboards, let's start out with 6 months > (expires in FF59) and you can renew and come up with a plan for permanent > monitoring. I think we'll want until at least the end of 2017, so maybe more like 9-10 months would be best. (In reply to Benjamin Smedberg [:bsmedberg] from comment #8) > I'd like to see the answer about user-added/customized roots before marking > final data-r. Any user-added roots are unknown to the collection system and get mapped to bucket 0.
See David's answer above. Uploaded a fix to ReviewBoard with improved descriptions, no whitelisting, and expiration of the EV probe set to FF62 (added 3-4 months). Take a look if that's OK?
Flags: needinfo?(sergei.cv) → needinfo?(benjamin)
Note: try builds fail because of whitelisting error: "New histogram "SSL_CT_POLICY_COMPLIANT_CONNECTIONS_BY_CA" is not permitted to have more than 100 buckets. Histograms with large numbers of buckets use disproportionately high amounts of resources. Contact the Telemetry team (e.g. in #telemetry) if you think an exception ought to be made." It builds fine on my dev machine. Should we add the whitelists anyway for now, so we can submit?
Oh! that is an implementation/technical issue, not a question of data policy. I'll defer that to gfritzsche: as it says, 256 buckets can be expensive in terms of storage/cost/pipeline processing.
Flags: needinfo?(benjamin) → needinfo?(gfritzsche)
This seems reasonable to me here. Frank, any concerns from the aggregator?
Flags: needinfo?(gfritzsche) → needinfo?(fbertsch)
Should be fine on aggregation side.
Flags: needinfo?(fbertsch)
Added the probes to the whitelist, things seem to be working fine now. From the telemetry perspective, can we proceed now with landing the patch?
Flags: needinfo?(benjamin)
Yes go ahead.
Flags: needinfo?(benjamin)
Great, thanks all!
Keywords: checkin-needed
Autoland can't push this until all pending issues in MozReview are marked as resolved.
Flags: needinfo?(sergei.cv)
Keywords: checkin-needed
Forgot to update MozReview - fixed now.
Flags: needinfo?(sergei.cv)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/5bf71e06cef7 Certificate Transparency - telemetry reports of CT Policy compliance. r=keeler
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: