1751078 - avoid creating CERTCertificates in SSLServerCertVerification

Status: RESOLVED FIXED

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

Description

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

There are some unnecessarily-created CERTCertificates in SSLServerCertVerification (and in particular, some are off the socket thread).

Comment 1

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

Attachment: Bug 1751078 - remove SSL_SERVER_AUTH_EKU telemetry probe r?jschanck

SSL_SERVER_AUTH_EKU has served its purpose. It has demonstrated that in the web
PKI (as defined by TLS web server certificates that chain up to root
certificates in Mozilla's CA program), all server certificates will have the
EKU extension, and the extension will be valid for TLS server authentication.
We no longer need to gather this data, so this patch removes this probe.

Comment 2

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

Attachment: Bug 1751078 - remove unnecessary CERTCertificate instantiation in TLS telemetry r?jschanck

After successfully verifying a TLS server certificate, Firefox collects some
telemetry based on the built certificate chain's root certificate. Before this
patch, the implementation would unnecessarily create CERTCertificates out of
the built cert chain (unnecessary because the telemetry only relies on the
bytes of the root certificate). This patch avoids the unnecessary work.

Depends on D136676

Comment 3

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

Attachment: Bug 1751078 - use mozilla::pkix to do time comparisons in DetermineCertOverrideErrors r?jschanck

Firefox uses mozilla::pkix to implement certificate verification. Before this
patch, though, DetermineCertOverrideErrors still used NSS to determine if a
certificate had any time-related errors (in addition to the primary error
returned by verification). This wasn't great from a consistency and
attack-surface point of view, so this patch updates that function to use
mozilla::pkix as well.

Depends on D136677

Comment 4

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

Attachment: Bug 1751078 - prefer nsIX509Cert over the concrete class r?jschanck

This patch updates some uses of RefPtr<nsNSSCertificate> to
nsCOMPtr<nsIX509Cert> because it's not necessary to use the concrete class.
This patch also removes some empty files that should have been removed in a
previous bug.

Depends on D136678

Comment 5

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

Attachment: Bug 1751078 - avoid passing an extra copy of the server certificate's bytes during verification r?jschanck

Depends on D136679

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0a3e7c40e53d
remove SSL_SERVER_AUTH_EKU telemetry probe r=jschanck
https://hg.mozilla.org/integration/autoland/rev/be920676ff3b
remove unnecessary CERTCertificate instantiation in TLS telemetry r=jschanck
https://hg.mozilla.org/integration/autoland/rev/01e9db998d64
use mozilla::pkix to do time comparisons in DetermineCertOverrideErrors r=jschanck
https://hg.mozilla.org/integration/autoland/rev/0364b18c9253
prefer nsIX509Cert over the concrete class r=necko-reviewers,jschanck,kershaw
https://hg.mozilla.org/integration/autoland/rev/b34a32e1fc3e
avoid passing an extra copy of the server certificate's bytes during verification r=jschanck

Comment 7

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/0a3e7c40e53d
https://hg.mozilla.org/mozilla-central/rev/be920676ff3b
https://hg.mozilla.org/mozilla-central/rev/01e9db998d64
https://hg.mozilla.org/mozilla-central/rev/0364b18c9253
https://hg.mozilla.org/mozilla-central/rev/b34a32e1fc3e