nsNSSComponent::HasActiveSmartCards() incorrectly returning true on machine with no 3rd party PKCS11 modules installed
Categories
(Core :: Security: PSM, defect, P1)
Tracking
()
People
(Reporter: acreskey, Assigned: keeler)
References
(Blocks 1 open bug, Regression)
Details
(Keywords: perf-alert, regression, Whiteboard: [psm-assigned][psm-clientauth])
Attachments
(3 files, 1 obsolete file)
This is the behaviour that I'm seeing:
- Launch Firefox Nightly, new profile
- Observe the output of "nsNSSComponent::HasActiveSmartCards()"
- On my newish MacBook Pro, this function returns true, which is disabling the performance feature preconnect / speculative connect.
See also: https://bugzilla.mozilla.org/show_bug.cgi?id=1579023
Notes: I have not (knowingly) installed any PKCS#11 modules, although it's certainly possible that one was installed with tools?
Reporter | ||
Comment 1•2 years ago
|
||
In stepping into HasActiveSmartCards
, this is the SECMODModule
https://searchfox.org/mozilla-central/rev/861fb9abfcaff123aab45f6ac56a0106b116dc14/security/manager/ssl/nsNSSComponent.cpp#864
Reporter | ||
Comment 2•2 years ago
|
||
And this is the slot for which PK11_IsFriendly()
returns false.
Reporter | ||
Comment 3•2 years ago
|
||
Please let me know if there's anything I can debug locally to pin point the problem.
Comment 4•2 years ago
|
||
I'm also seeing PK11_IsFriendly
return false on the builtin roots module, which we always load.
I don't see any patches that would have changed that return value recently. Is it possible that this has been broken since Bug 1579023 landed?
Comment 5•2 years ago
|
||
Updated•2 years ago
|
Reporter | ||
Comment 6•2 years ago
|
||
(In reply to John Schanck [:jschanck] from comment #4)
I'm also seeing
PK11_IsFriendly
return false on the builtin roots module, which we always load.I don't see any patches that would have changed that return value recently. Is it possible that this has been broken since Bug 1579023 landed?
That seems very possible.
I'm going to spend a bit of time looking at the scope of this, and I'll update if I find anything.
Reporter | ||
Comment 7•2 years ago
•
|
||
Seeing this behaviour on Windows and Linux.
Will check Android. [yes - same behaviour]
Updated•2 years ago
|
Reporter | ||
Updated•2 years ago
|
Reporter | ||
Comment 8•2 years ago
|
||
Actually, this seems to be working fine on android, I only see desktop platforms being affected.
Comment 9•2 years ago
|
||
Thanks for looking into it. It turns out that, while my patch is sufficient on Linux, there are problems with other automatically loaded modules on Windows and OSX. So we're working on a different approach.
Updated•2 years ago
|
Comment 10•2 years ago
|
||
Set release status flags based on info from the regressing bug 1579023
Updated•2 years ago
|
Comment 11•2 years ago
|
||
:jschanck do you plan on landing this patch anytime soon?
Will it land in for 112?
Updated•2 years ago
|
Comment 12•2 years ago
|
||
We determined that my patch would only fix the issue for a fraction of users, so we're going with a different approach. It might not be done for 112.
Updated•2 years ago
|
![]() |
Assignee | |
Updated•2 years ago
|
![]() |
Assignee | |
Comment 13•2 years ago
|
||
When necko makes a speculative connection, the peer may ask for a client
authentication certificate. This patch makes it so that when this happens,
no certificate selection UI will be shown until the connection is claimed (as
in, it is no longer merely speculative).
Comment 14•2 years ago
|
||
Comment 15•2 years ago
|
||
bugherder |
Comment 16•2 years ago
|
||
The patch landed in nightly and beta is affected.
:keeler, is this bug important enough to require an uplift?
- If yes, please nominate the patch for beta approval.
- If no, please set
status-firefox113
towontfix
.
For more information, please visit auto_nag documentation.
Reporter | ||
Comment 17•2 years ago
|
||
Because we'd like to measure the performance of this change and since it's been a longstanding issue I would strongly prefer that we not uplift this right now.
Updated•2 years ago
|
![]() |
Assignee | |
Updated•2 years ago
|
Comment 19•2 years ago
|
||
(In reply to Pulsebot from comment #14)
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3155d983a779
buffer client auth certificate selection UI for speculative connections
r=valentin,jschanck,necko-reviewers,kershaw
== Change summary for alert #38158 (as of Fri, 21 Apr 2023 11:35:26 GMT) ==
Improvements:
Ratio | Test | Platform | Options | Absolute values (old vs new) |
---|---|---|---|---|
38% | tp5n main_startup_fileio | windows11-64-2009-shippable-qr | e10s fission stylo webrender-sw | 511,556.21 -> 316,187.92 |
33% | tp5n main_startup_fileio | windows11-64-2009-shippable-qr | e10s fission stylo webrender-sw | 522,650.67 -> 348,953.92 |
32% | tp5n main_startup_fileio | windows11-64-2009-shippable-qr | e10s fission stylo webrender | 519,490.00 -> 351,258.58 |
For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=38158
Reporter | ||
Comment 20•2 years ago
|
||
We were not expecting significant improvements to be picked up by the performance tests in CI since they run against an https proxy (and also since speculative connects are explicitly disabled on proxied connects in some cases).
The above alert shows an improvement in file io counts on an older startup pageload test.
However we are seeing measurable improvements by looking at the pageload_event before and after this fix landed.
Discussed here.
Updated•2 years ago
|
Comment 21•2 years ago
|
||
uplift |
Backed out in mozillla-release for causing bug 1835103
I am marking 114 as wontfix as this was backed out and 115 as fixed as this is still in central
https://hg.mozilla.org/releases/mozilla-release/rev/defb5e07935c
Description
•