Closed Bug 1172615 Opened 4 years ago Closed 4 years ago
opportunistic encryption potentially perturbing certificate verification telemetry
SSL_CERT_VERIFICATION_ERRORS is reporting a huge amount of SEC_ERROR_UNKNOWN_ISSUER failures. Strangely, these numbers are not reflected in a similar measure, SSL_CERT_ERROR_OVERRIDES. After looking at the code flow, it appears we attempt to verify a certificate, note a failure (SSL_CERT_VERIFICATION_ERRORS), check if we're supposed to bypass authentication on that socket, and then gather information about overrides (SSL_CERT_ERROR_OVERRIDES). It could be the case that the number of connections using opportunistic encryption is severely throwing off our error telemetry gathering. Furthermore, it seems unnecessary to try to verify a certificate if we're going to allow it anyway after encountering an error. We should probably move that check to AuthCertificateHook.
this is interesting.. but I would be surprised if there was anywhere enough usage of OE to have this impact at this point. but hey, I live to be surprised.
Yeah, now that I think about it it seems unlikely this would account for all of the discrepancy, but it's at least a place to start. Also, this should make OE faster, since we'll skip a bunch of unnecessary operations, which seems like a win regardless.
Assignee: nobody → dkeeler
Status: NEW → ASSIGNED
Attachment #8616903 - Flags: review?(mcmanus)
this patch seems directly at odds with https://bugzilla.mozilla.org/show_bug.cgi?id=964466#c0 - which is what I used as info to implement it with.. am I misreading it? (some function names have changed in the interim, so I'm not 100% sure).. is it not the right thing todo? obviously we could always just suppress the telemetry bits, but if the optimization can be had safely that would be good too.
I think the motivation behind that comment is to provide functionality that is performance-wise as close as possible to a successful verification while not actually requiring that certificates verify correctly (so tools can use it to show performance characteristics of sites in development or something). As long as HandshakeCallback gets called (and it's my understanding that it will always be called when the handshake completes), all of the relevant bookkeeping should still happen. In short, I believe this optimization is safe.
Attachment #8616903 - Flags: review?(mcmanus) → review+
Thanks for the review. Here's a try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2075a2da8bcf
You need to log in before you can comment on or make changes to this bug.