1355903 - Re-enable Certificate Transparency telemetry collection

Status: NEW

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

Description

[J.C. Jones [:jcj] (he/him)]

Bug 1353216 shut off certificate transparency telemetry collection due to a performance regression. This bug should turn it back on after performance is improved.

Comment 1

[Patrick McManus [:mcmanus]]

sorry for the drive-by partially informed comment - but given that this is just for reporting is this something that can be done asynchronously queued and processed at 'idle' time? (its been a while, but iirc firefox has an event for that) so the math doesn't impact responsiveness? or is there too much state tied up in the connection to practically copy/addref for queueing?

Comment 2

[J.C. Jones [:jcj] (he/him)]

(In reply to Patrick McManus [:mcmanus] from comment #1)
> sorry for the drive-by partially informed comment - but given that this is
> just for reporting is this something that can be done asynchronously queued
> and processed at 'idle' time? (its been a while, but iirc firefox has an
> event for that) so the math doesn't impact responsiveness? or is there too
> much state tied up in the connection to practically copy/addref for queueing?

It's a good idea, but an eventual goal is to have CT enforcement, too -- which would need to be synchronous to be effective.

Comment 3

[J.C. Jones [:jcj] (he/him)]

Keeler and I discussed more what is needed to re-enable our CT telemetry collection:

== Better Tests ==

Before we get to performance testing, I think we need more significant tests in-tree, as specified in bug 1361177.

== Speedups ==

It seems that most of the performance regression was due to unnecessary cert chain validations, fixed in Bug 1356611. The unnecessary validations were made much worse by the slower-than-desired verification algorithm; Keeler filed Bug 1355993 to address speeding that up. It should be possible to get some performance increase, but given the magnitude of the fix from Bug 1356611, we _might not need_ the algorithm speedup to proceed.

We'd optimally like to see a Talos test for HTTPS that could show a regression during CT (Bug 1356073), but perhaps we can proceed with a plan that checks telemetry instead:

== Speedup Test Plan ==

When we disabled CT, the SSL_SUCCESFUL_CERT_VALIDATION_TIME_MOZILLAPKIX telemetry metric dropped precipitously [1] (the drop corresponds to the disable date) [2]. In-code, this metric is firmly tied to the CT performance.

I think it's OK to move on without the Talos test and verify whether our performance is OK using Nightly telemetry. If slows down more than a little, we'd want to wait on Bug 1355993 to proceed.

== Recommendation Summary ==

1. We need the tests in bug 1361177.
2. Then we can re-enable using this bug and monitor SSL_SUCCESFUL_CERT_VALIDATION_TIME_MOZILLAPKIX.
2.a. If performance slows down by an unacceptable amount, we back-out the enable commit.
2.b. If we back-out, then we block on Bug 1355993 to re-try.

[1] https://i.have.insufficient.coffee/ct-validation-time-fix.png
[2] https://mzl.la/2qKALHN

Comment 4

[J.C. Jones [:jcj] (he/him)]

After further discussion, I am amending my recommendation that we only re-enable after we also have Talos tests, and marking this bug as being blocked on Bug 1356073:

== Recommendation Summary (Revised) ==

1. We need the tests in bug 1361177.
2. We also need the Talos test in bug 1356073 to monitor for HTTPS performance regressions.
3. Then we can re-enable using this bug and monitor SSL_SUCCESFUL_CERT_VALIDATION_TIME_MOZILLAPKIX in Nightly.
4.a. If performance slows down by an unacceptable amount, we back-out the enable commit.
4.b. If we back-out, then we block on Bug 1355993 before re-trying.