Closed
Bug 1484843
Opened 6 years ago
Closed 6 years ago
Create a policy to disable DNS over HTTPS
Categories
(Firefox :: Enterprise Policies, defect, P1)
Firefox
Enterprise Policies
Tracking
()
VERIFIED
FIXED
Firefox 64
People
(Reporter: mkaply, Assigned: mkaply)
References
Details
(Whiteboard: [trr])
Attachments
(1 file)
46 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details | Review |
Before DNS over HTTPS ships, there needs to be a policy to turn it off.
Status till now: "TRR is preffed OFF by default and you need to set a URI for an available DOH server to be able to use it." https://hg.mozilla.org/mozilla-central/file/6776d69d2f03/netwerk/test/unit/test_trr.js
Updated•6 years ago
|
Whiteboard: [trr]
Updated•6 years ago
|
Priority: P1 → P2
Comment 2•6 years ago
|
||
Update: we're hoping to ship TRR in 63 (tight schedule) so we're hoping to move on this ASAP. So we need to be clear on what pref(s) we need to set to disable TRR for enterprise. I'm guessing that this is both 1) setting "network.trr.mode" to 0 (i.e. turn off TRR) 2) Also ensure that users don't see the doorhanger asking them if they want to opt out of TRR. Asking jkt if there's a pref for #2. And just to be safe, making sure with Daniel that I understand #1 correctly (that's all we need to completely disable TRR, right?)
Flags: needinfo?(jkt)
Updated•6 years ago
|
Flags: needinfo?(daniel)
Comment 3•6 years ago
|
||
(In reply to Jason Duell [:jduell] (Regression Engineering Owner for 64; needinfo me) from comment #2) > So we need to be clear on what pref(s) we need to set to disable TRR for > enterprise. I'm guessing that this is both > > 1) setting "network.trr.mode" to 0 (i.e. turn off TRR) > > 2) Also ensure that users don't see the doorhanger asking them if they > want to opt out of TRR. > > Asking jkt if there's a pref for #2. FWIW, enterprise policies usually lock the pref, so ideally the code itself that is driving the doorhanger should check the pref value and if the pref is locked and not show itself.
Comment 4•6 years ago
|
||
You can set mode to **5** to make it "off by choice" instead of "off by default". It was explicitly implemented to make users not get prompted again.
Flags: needinfo?(daniel)
Comment 5•6 years ago
|
||
The code doesn't check the pref is locked but certainly checks if it's user mofified. Setting and then locking to mode 5 is likely the right thing here.
Flags: needinfo?(jkt)
Assignee | ||
Comment 6•6 years ago
|
||
Can someone help me with a good name for this? We don't like to use the word "Disable" for everything, so I don't really want "DisableDNSOverHTTPS" Is there a way to word this so it sounds positive instead of negative? Or should the policy just be DNSOverHTTPS and set it to true or false?
Comment 7•6 years ago
|
||
> Or should the policy just be > > DNSOverHTTPS and set it to true or false? Works for me! jkt: so if the policy code sets "network.trr.mode" to 5, will that already be enough to block the addon from popping up, or do you need to add code for that?
Flags: needinfo?(jkt)
Comment 8•6 years ago
|
||
> so if the policy code sets "network.trr.mode" to 5, will that already be enough to block the addon from popping up, or do you need to add code for that?
This works for the existing addon certainly. There are some current uncertainties about the delivery mechanism but we can target users that don't have this mode set through all mechanisms I know of. Either way whatever happens we will work around this pref being set to this value.
Flags: needinfo?(jkt)
Comment 9•6 years ago
|
||
So I’m going to take a wild guess that if DoH is enabled by Enterprise policy, we prob don’t want a banner (ie. if it’s org policy, asking individual users if they want in/out is probably not wanted). Does that sound right? Would the right fix be to add yet another pref (that indicates DoH is on because of Policy, and the banner code can check it before showing banner)?
Flags: needinfo?(jkt)
Comment 10•6 years ago
|
||
rfeeley - adding you to weigh in - this is related to the URI pref setting from last week.
Flags: needinfo?(rfeeley)
Assignee | ||
Comment 11•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Priority: P2 → P1
Comment 12•6 years ago
|
||
Had a quick chat with Mike and showed him Erica’s latest UI. He'll work on a new patch with a more extensive policy.
Flags: needinfo?(rfeeley)
Assignee | ||
Comment 13•6 years ago
|
||
What is the preference name for the provider URL?
QA Contact: felipc
Updated•6 years ago
|
QA Contact: felipc
Comment 14•6 years ago
|
||
network.trr.uri https://wiki.mozilla.org/Trusted_Recursive_Resolver
Assignee | ||
Comment 15•6 years ago
|
||
jkt: can you rereview?
Comment 17•6 years ago
|
||
Pushed by mozilla@kaply.com: https://hg.mozilla.org/integration/autoland/rev/f67a13884b97 Add policy for disabling DNS over HTTPS r=flod,Felipe,jkt
Comment 18•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f67a13884b97
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Assignee | ||
Comment 19•6 years ago
|
||
Comment on attachment 9014456 [details] Bug 1484843 - Add policy for disabling DNS over HTTPS [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: None User impact if declined: Enterprise unable to disable DNS over HTTPS via policy 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: Is being verified as part of policy work, but automated test fully covers this List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): Adds policy that sets prefs String changes made/needed: Adds string, but translation is optional (policy only - Fluent)
Attachment #9014456 -
Flags: approval-mozilla-beta?
Comment 20•6 years ago
|
||
(In reply to Mike Kaply [:mkaply] from comment #19) > Why is the change risky/not risky? (and alternatives if risky): Adds policy > that sets prefs Asking just in case but in your test file browser_policies_simple_pref_policies.js shouldn't the provider urls use https urls like other policies in the same file that use https://www.example.com/? It probably doesn't matter but I prefer to ask before accepting the uplift. lockedPrefs: { "network.trr.mode": 2, "network.trr.uri": "http://example.com/provider", },
Flags: needinfo?(mozilla)
Assignee | ||
Comment 21•6 years ago
|
||
> Thanks for asking.
It doesn't matter. This is really just an xpcshell test validating that the URLs are set correctly
As long as they are URLs, the test is good.
Flags: needinfo?(mozilla)
Comment 22•6 years ago
|
||
Comment on attachment 9014456 [details] Bug 1484843 - Add policy for disabling DNS over HTTPS Thanks Mike, contained patch adding a policy, uplift approved for our last 63 beta.
Attachment #9014456 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 23•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/66ab8622488c
Flags: in-testsuite+
Updated•6 years ago
|
Flags: qe-verify+
Comment 24•6 years ago
|
||
This bug was covered by the overall testing efforts invested in the Additional Enterprise Policies feature. Marking this as verified fixed using Firefox 64.0a1 (BuildId:20181011220118) and Firefox 63.0b14 (BuildId:20181011200118) on Windows 10 64bit and macOS 10.13.6
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Assignee | ||
Comment 25•6 years ago
|
||
FYI, it looks like the preferences aren't properly reflecting the preferences (even though they work). I'll open a separate bug.
Blocks: 1612352
You need to log in
before you can comment on or make changes to this bug.
Description
•