1596845 - Implement new error page for DNS errors when DoH is enabled

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P1)

Description

[(inactive) Nihanth Subramanya [:nhnt11]]

Currently the spec calls for 2 error pages: one when the TRR provider is unavailable and one when the provider tried and couldn't resolve the hostname. The former would offer a button that temporarily completely disables DoH, and the latter would offer a button to whitelist that domain.

This is currently not possible to implement since we don't have any way to distinguish between the two scenarios. Pending necko enhancements to support this, here are a few ideas that Bryan and I are considering for a single error page:

1. Offer both buttons on the same error page.
2. Offer a button to reload the page with DoH disabled.
3. Option 2, but also, offer a button to temporarily completely disable DoH after this happens N times.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The product::component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 3

[(inactive) Nihanth Subramanya [:nhnt11]]

No, this is not a necko bug, this belongs in Firefox. Maybe I'll request a DoH component for tracking frontend bugs and then we can move it over from General.

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1596845 - Pass effectiveTRRMode to HTTP child channel r=#necko

Comment 5

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1596845 - Pass new DNS record to OnLookupComplete to be able to get effectiveTRRMode and skip reason r=#necko

Depends on D164345

Comment 6

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1596845 - Make custom about:neterror page for TRR mode3 DNS failures r=pbz

Depends on D164346

Comment 7

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1596845 - Pass TRR skip reason to child channel r=#necko

Depends on D164347

Comment 8

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1596845 - Pass full trr domain into content process r=#necko,acreskey!

Previosuly we'd only pass the TRRService::ProviderKey() into the content
process. But not we need the full domain for the error page in the content
process, so we now pass the full domain. The ChildDNSService now holds on
to the full domain, but calls into TRRService to update the key for
telemetry and returns that when necessary.

Depends on D164348

Comment 9

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1596845 - Make DNSServices available as `Services.dns` r=#necko

Depends on D164347

Comment 10

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1596845 - Turn nsIDNSService dns flags into a cenum r=#necko

This allows us to use a consistent size for the dnsFlags field.
across different files (previously some would use uint16_t and some uint32_t).
It also improves type safety - making sure we don't pass in the wrong value
to DNSFlags.

Depends on D164856

Comment 11

[Andrew Creskey [:acreskey]]

Attachment: Bug 1596845 - Make effectiveTRRMode, skipReason atomic r=#necko

We encountered data races in unit tests.

Depends on D164346

Comment 12

[Kershaw Chang [:kershaw]]

Attachment: Bug 1596845 - Initialize DNS service earlier, r=acreskey

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b14a4910d73b
Pass effectiveTRRMode to HTTP child channel r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/80e5048d1f35
Pass TRR skip reason to child channel r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/8e1fe024b680
Pass full trr domain into content process r=acreskey,necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/ab300239fd69
Make custom about:neterror page for TRR mode3 DNS failures r=pbz,fluent-reviewers,settings-reviewers,flod
https://hg.mozilla.org/integration/autoland/rev/950c66dd6133
Make DNSServices available as `Services.dns` r=necko-reviewers,webdriver-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/a383f2bbcaae
Turn nsIDNSService dns flags into a cenum r=necko-reviewers,geckoview-reviewers,kershaw,m_kato
https://hg.mozilla.org/integration/autoland/rev/f1447dd8df04
Pass new DNS record to OnLookupComplete to be able to get effectiveTRRMode and skip reason r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/af6d41439c60
Make effectiveTRRMode, skipReason atomic r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/eb1e34c3041f
Initialize DNS service earlier, r=acreskey

Comment 14

[Sandor Molnar[:smolnar]]

Backed out for causing mochitest failures in browser/base/content/test/about/browser_aboutCertError_telemetry.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/a5e4362f37d6b2acbe72a62518855e80370444b1

Push with failures

Failure log

INFO - Buffered messages finished
[task 2022-12-22T22:14:34.613Z] 22:14:34     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/about/browser_aboutCertError_telemetry.js | Uncaught exception in test - at chrome://global/content/elements/browser-custom-element.js:708 - TypeError: can't access property "userTyped", this.urlbarChangeTracker is undefined
[task 2022-12-22T22:14:34.613Z] 22:14:34     INFO - Stack trace:

Comment 15

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/16f71c909bb3
Backed out 9 changesets for causing mochitest failures in browser/base/content/test/about/browser_aboutCertError_telemetry.js

Comment 16

[Andrew Creskey [:acreskey]]

Fix incoming, small change.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b171f7ac0b40
Pass effectiveTRRMode to HTTP child channel r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/f4adb3e7b8e1
Pass TRR skip reason to child channel r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/88919aaff89b
Pass full trr domain into content process r=acreskey,necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/b78bc0049605
Make custom about:neterror page for TRR mode3 DNS failures r=pbz,fluent-reviewers,settings-reviewers,flod,edgul
https://hg.mozilla.org/integration/autoland/rev/98d583f1d19e
Make DNSServices available as `Services.dns` r=necko-reviewers,webdriver-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/5cd033c9ef7c
Turn nsIDNSService dns flags into a cenum r=necko-reviewers,geckoview-reviewers,kershaw,m_kato
https://hg.mozilla.org/integration/autoland/rev/b0449eec2671
Pass new DNS record to OnLookupComplete to be able to get effectiveTRRMode and skip reason r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/7d02dad4d720
Make effectiveTRRMode, skipReason atomic r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/231acfc052bb
Initialize DNS service earlier, r=acreskey

Comment 18

[Marian-Vasile Laza]

Backed out 9 changesets (Bug 1596845) for causing xpcshell failures on test_trr_enterprise_policy.js.
Backout link
Push with failures
Failure Log

Comment 19

[Kershaw Chang [:kershaw]]

(In reply to Marian-Vasile Laza from comment #18)

Backed out 9 changesets (Bug 1596845) for causing xpcshell failures on test_trr_enterprise_policy.js.
Backout link
Push with failures
Failure Log

Not sure why this test still failed with socket process. However, we can disable this test for socket process for now and fix it later.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/726e877bdd46
Pass effectiveTRRMode to HTTP child channel r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/13d383bde6cf
Pass TRR skip reason to child channel r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/0c74e206883c
Pass full trr domain into content process r=acreskey,necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/30f730eba0c4
Make custom about:neterror page for TRR mode3 DNS failures r=pbz,fluent-reviewers,settings-reviewers,flod,edgul
https://hg.mozilla.org/integration/autoland/rev/c4400bef7b19
Make DNSServices available as `Services.dns` r=necko-reviewers,webdriver-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/ac391762ae01
Turn nsIDNSService dns flags into a cenum r=necko-reviewers,geckoview-reviewers,kershaw,m_kato
https://hg.mozilla.org/integration/autoland/rev/236b2bde7397
Pass new DNS record to OnLookupComplete to be able to get effectiveTRRMode and skip reason r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/c1d4e09eca69
Make effectiveTRRMode, skipReason atomic r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/c9c9844f1f2f
Initialize DNS service earlier, r=acreskey

Comment 21

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/726e877bdd46
https://hg.mozilla.org/mozilla-central/rev/13d383bde6cf
https://hg.mozilla.org/mozilla-central/rev/0c74e206883c
https://hg.mozilla.org/mozilla-central/rev/30f730eba0c4
https://hg.mozilla.org/mozilla-central/rev/c4400bef7b19
https://hg.mozilla.org/mozilla-central/rev/ac391762ae01
https://hg.mozilla.org/mozilla-central/rev/236b2bde7397
https://hg.mozilla.org/mozilla-central/rev/c1d4e09eca69
https://hg.mozilla.org/mozilla-central/rev/c9c9844f1f2f