Disable NSS_ALLOW_SSLKEYLOGFILE
Categories
(Firefox :: Security, defect)
Tracking
()
People
(Reporter: erahm, Assigned: erahm)
References
Details
Attachments
(1 file)
1.24 KB,
patch
|
glandium
:
review+
RyanVM
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr60+
|
Details | Diff | Splinter Review |
Due to potential shutdown hangs we'd like to temporarily disable NSS_ALLOW_SSLKEYLOGFILE [1] by default in non-nightly releases.
Assignee | ||
Comment 1•5 years ago
|
||
This disables NSS_ALLOW_SSLKEYLOGFILE in beta in release in order to avoid shutdown hangs until the NSS project has time to fix the root cause of the issue. Nathan can you review this? It's really a build system change at this point. I have a [beta simulation running](https://treeherder.mozilla.org/#/jobs?repo=try&revision=837654f11970c3269bcf0d08ec8a68d3c30e94f5) to make sure it does the right thing.
Assignee | ||
Updated•5 years ago
|
Comment 2•5 years ago
|
||
Comment on attachment 9035763 [details] [diff] [review] Disable NSS_ALLOW_SSLKEYLOGFILE in beta and release Review of attachment 9035763 [details] [diff] [review]: ----------------------------------------------------------------- Note that we're trying to do this without changing NSS at all. The Right Thing in some respect is to make this setting configurable at the NSS level, but since we're not taking that route (for now?), this was the best thing I could think of. glandium might have strong opinions about this, and I thought up the idea, so I don't think I should be the person to review this.
Comment 3•5 years ago
|
||
Comment on attachment 9035763 [details] [diff] [review] Disable NSS_ALLOW_SSLKEYLOGFILE in beta and release Review of attachment 9035763 [details] [diff] [review]: ----------------------------------------------------------------- Considering how things work for defines and gyp, this might well be the only solution without touching NSS. ::: python/mozbuild/mozbuild/frontend/gyp_reader.py @@ +250,5 @@ > if '=' in define: > name, value = define.split('=', 1) > + # The NSS gyp file doesn't expose a way to override this > + # currently, so we do so here. > + if name == 'NSS_ALLOW_SSLKEYLOGFILE' and config.substs.get('RELEASE_OR_BETA', False): You don't need that `, False`.
Pushed by erahm@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0801165e3175 Disable NSS_ALLOW_SSLKEYLOGFILE in beta and release. r=glandium
Assignee | ||
Comment 5•5 years ago
|
||
Comment on attachment 9035763 [details] [diff] [review]
Disable NSS_ALLOW_SSLKEYLOGFILE in beta and release
[Beta/Release Uplift Approval Request]
Feature/Bug causing the regression: Bug 1188657
User impact if declined: This can cause random hangs in the networking threads when working with secure connections. This particularly shows up as shutdown hangs in crash stats.
Is this code covered by automated tests?: Yes
Has the fix been verified in Nightly?: No
Needs manual test from QE?: No
If yes, steps to reproduce:
List of other uplifts needed: None
Risk to taking this patch: Low
Why is the change risky/not risky? (and alternatives if risky): We're just disabling the option to enable a feature which works around a bug in NSS related to checking if the feature is enabled.
String changes made/needed: N/A
[ESR Uplift Approval Request]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: This disables a feature that can cause networking and shutdown hangs.
User impact if declined: This can cause random hangs in the networking threads when working with secure connections. This particularly shows up as shutdown hangs in crash stats.
Fix Landed on Version: 66
Risk to taking this patch: Low
Why is the change risky/not risky? (and alternatives if risky): We're just disabling the option to enable a feature which works around a bug in NSS related to checking if the feature is enabled.
String or UUID changes made by this patch: N/A
Assignee | ||
Comment 6•5 years ago
|
||
(In reply to Eric Rahm [:erahm] from comment #1)
I have a [beta simulation
running](https://treeherder.mozilla.org/#/
jobs?repo=try&revision=837654f11970c3269bcf0d08ec8a68d3c30e94f5) to make
sure it does the right thing.
I've verified that the SSLKEYLOGFILE
string used to check if an env var is set is no longer present in a simulated beta build by running:
strings libssl3.so | grep SSLKEYLOGFILE
Comment 7•5 years ago
|
||
bugherder |
Comment 8•5 years ago
|
||
Comment on attachment 9035763 [details] [diff] [review]
Disable NSS_ALLOW_SSLKEYLOGFILE in beta and release
[Triage Comment]
Enables NSS_ALLOW_SSLKEYLOGFILE on Nightly builds only to avoid shutdown hangs. Approved for 65.0b11 and 60.5.0esr.
Comment 9•5 years ago
|
||
bugherder uplift |
Comment 10•5 years ago
|
||
bugherder uplift |
Description
•