1748341 - instantiate CERTCertificate in nsIX509Cert on-demand instead of always

Status: RESOLVED FIXED

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

Description

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

nsIX509Cert doesn't always need to make a CERTCertificate out of the certificate bytes it's constructed with, so we could avoid some work by instantiating it on first use rather than for every certificate created. This also gives us a path towards not instantiating it at all in content processes.

Comment 1

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

Attachment: Bug 1748341 - misc nsNSSCertificate cleanup r?jschanck

Comment 2

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

Attachment: Bug 1748341 - remove superfluous nsNSSCertificate "constructors" r?jschanck

Depends on D134977

Comment 3

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

Attachment: Bug 1748341 - use mDER over mCert in nsNSSCertificate where possible r?jschanck

Depends on D134978

Comment 4

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

Attachment: Bug 1748341 - only instantiate CERTCertificate in nsNSSCertificate if needed r?jschanck

Depends on D134979

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e1f1620d1bb
misc nsNSSCertificate cleanup r=jschanck
https://hg.mozilla.org/integration/autoland/rev/7aa1f58a893e
remove superfluous nsNSSCertificate "constructors" r=necko-reviewers,kershaw,jschanck
https://hg.mozilla.org/integration/autoland/rev/4c8bf5c43e12
use mDER over mCert in nsNSSCertificate where possible r=jschanck
https://hg.mozilla.org/integration/autoland/rev/667470bfefbb
only instantiate CERTCertificate in nsNSSCertificate if needed r=jschanck

Comment 6

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/1e1f1620d1bb
https://hg.mozilla.org/mozilla-central/rev/7aa1f58a893e
https://hg.mozilla.org/mozilla-central/rev/4c8bf5c43e12
https://hg.mozilla.org/mozilla-central/rev/667470bfefbb

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/32eac1923cb2
port ´remove superfluous nsNSSCertificate "constructors"' to Thunderbird. rs=bustage-fix

Comment 8

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/d163f7c81ef3
Backed out 4 changesets for causing Bug 1749735 . a=pascal

Comment 9

[Narcis Beleuzu [:NarcisB]]

Backed out for causing Bug 1749735

Backout link: https://hg.mozilla.org/mozilla-central/rev/d163f7c81ef38ebf3d1edaff9e961c925472225f

Comment 10

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a95e0e6e62a9
Backed out changeset 32eac1923cb2 since mozilla-central part was backed out. rs=backout DONTBUILD

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/79b0615431f0
misc nsNSSCertificate cleanup r=jschanck
https://hg.mozilla.org/integration/autoland/rev/84258533642e
remove superfluous nsNSSCertificate "constructors" r=necko-reviewers,kershaw,jschanck
https://hg.mozilla.org/integration/autoland/rev/d54b59f6d31b
use mDER over mCert in nsNSSCertificate where possible r=jschanck
https://hg.mozilla.org/integration/autoland/rev/eb80fdfd96b6
only instantiate CERTCertificate in nsNSSCertificate if needed r=jschanck

Comment 12

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/79b0615431f0
https://hg.mozilla.org/mozilla-central/rev/84258533642e
https://hg.mozilla.org/mozilla-central/rev/d54b59f6d31b
https://hg.mozilla.org/mozilla-central/rev/eb80fdfd96b6

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/cb25fd710914
port ´remove superfluous nsNSSCertificate "constructors"` to Thunderbird. rs=bustage-fix

Comment 14

[Bob Clary [:bc] (inactive)]

(In reply to Pulsebot from comment #13)

This appears to have broken TLS on Thunderbird Nightly x86_64 Linux.

Regression range

Good Build 20220113104309

Bad Build 20220114122738

I can't connect to my imap servers with the latest build due to the error "non-overridable TLS error occured. Handshake error or..."

Comment 15

[Magnus Melin [:mkmelin]]

Would suspect bug 1750318? If not, please file a new bug. Today's daily at least seem to work as normal for me.

Comment 16

[Bob Clary [:bc] (inactive)]

(In reply to Magnus Melin [:mkmelin] from comment #15)

Would suspect bug 1750318? If not, please file a new bug. Today's daily at least seem to work as normal for me.

I have network.trr.mode=0 so not the same but updating to 20220115103459 appears to have fixed it. Thanks!

Comment 17

[Alex Finder]

== Change summary for alert #32957 (as of Mon, 17 Jan 2022 06:27:45 GMT) ==

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
34%	cnn-nav.world FirstVisualChange	macosx1015-64-shippable-qr	cold fission webrender	273.57 -> 367.06
26%	cnn-nav.world SpeedIndex	macosx1015-64-shippable-qr	cold fission webrender	333.63 -> 419.74

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
4%	cnn-nav.world dcf	macosx1015-64-shippable-qr	cold fission webrender	440.29 -> 423.92
4%	cnn-nav.world fcp	macosx1015-64-shippable-qr	cold fission webrender	430.23 -> 414.67

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=32957