969048 - Adjust OCSP stapling telemetry for insanity::pkix

Status: RESOLVED FIXED

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

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

The way OCSP stapling telemetry is collected currently is specific to the way that the NSS validation works in that it checks the return value of CERT_CacheOCSPResponseFromSideChannel. The call to CERT_CacheOCSPResponseFromSideChannel is irrelevant to insanity::pkix and it should be removed. More importantly, we need to count OCSP-stapling-related errors based on either the result of CertVerifier::VerifyCert or some other way, so we know if there are any OCSP(-stapling)-related problem with insanity::pkix.

Comment 1

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

Attachment: patch

The changes to SSLServerCertVerification.cpp are most clear when looked at with 'diff -w' (I can attach a patch generated like that if need be).

Comment 2

[Camilo Viecco (:cviecco)]

Comment on attachment 8382610 [details] [diff] [review]
patch

Review of attachment 8382610 [details] [diff] [review]:
-----------------------------------------------------------------

::: security/certverifier/NSSCertDBTrustDomain.cpp
@@ +177,5 @@
>      SECStatus rv = VerifyEncodedOCSPResponse(*this, cert, issuerCert, time,
>                                               stapledOCSPResponse, &validUntil);
>      if (rv == SECSuccess) {
> +      // stapled OCSP response present and good
> +      Telemetry::Accumulate(Telemetry::SSL_OCSP_STAPLING, 1);

Wow telemetry is now thread safe. Pinning telemetry then will be really easy!

Comment 3

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

Attachment: patch v1.1

rebased, carrying over r+s, inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/21ad5a221382

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/21ad5a221382