Closed Bug 1475420 Opened Last year Closed Last year

TRR timeout when using TRR to open TRR channel

Categories

(Core :: Networking: DNS, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox61 --- unaffected
firefox62 --- fixed
firefox63 --- fixed

People

(Reporter: mcmanus, Assigned: mcmanus)

Details

(Whiteboard: [trr])

Attachments

(1 file)

the initial trr channel falls back to native dns beacuse the NS check has never passed.

however, its possible to get into a state where
 1] there is no open trr channel
 2] the origin is not in the dns cache
 3] the ns check succeeded in the past

and then we get into a deadlock of the trr channel trying to look itself up using trr. it times out and life goes on - this is a little more obvious on the esni branch but this is a general trr bug.

this patch marks the TRR channel as DISABLE_TRR to avoid the loop.

I confirmed the patch works ok in TRR_ONLY mode because the bootstrap code doesn't look at channel flags at all.
Attachment #8991780 - Flags: review?(valentin.gosu)
Comment on attachment 8991780 [details]
Bug 1475420 - mark the TRR connection as DISABLE_TRR to avoid TXT deadlock

https://reviewboard.mozilla.org/r/256706/#review263660
Attachment #8991780 - Flags: review?(valentin.gosu) → review+
Pushed by mcmanus@ducksong.com:
https://hg.mozilla.org/integration/autoland/rev/cbcbae48a940
mark the TRR connection as DISABLE_TRR to avoid TXT deadlock r=valentin
https://hg.mozilla.org/mozilla-central/rev/cbcbae48a940
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Comment on attachment 8991780 [details]
Bug 1475420 - mark the TRR connection as DISABLE_TRR to avoid TXT deadlock

https://reviewboard.mozilla.org/r/256706/#review263876
Attachment #8991780 - Flags: review?(daniel) → review+
Assignee: nobody → mcmanus
Version: 50 Branch → unspecified
Comment on attachment 8991780 [details]
Bug 1475420 - mark the TRR connection as DISABLE_TRR to avoid TXT deadlock

Approval Request Comment
[Feature/Bug causing the regression]: TRR (DoH) - original bug on 62, not a regression
[User impact if declined]: pauses in DNS resolution when DoH is enabled, especially after hard refreshes
[Is this code covered by automated tests?]: yes, though the timing is not verified
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]:  no
[List of other uplifts needed for the feature/fix]: na
[Is the change risky?]: no
[Why is the change risky/not risky?]: DoH is not enabled by default on 62, but we'd like to do pref/shield studies that target it
[String changes made/needed]: none
Attachment #8991780 - Flags: approval-mozilla-beta?
Comment on attachment 8991780 [details]
Bug 1475420 - mark the TRR connection as DISABLE_TRR to avoid TXT deadlock

Looks like DoH studies are coming soon for 63, while I don't see anything in the works for 62, this seems OK to uplift in case we plan one during 62 release.
Attachment #8991780 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
thanks! some background - the goal is to have the 62 doh code be as complete as the 63 code. Primarily to support manual testing. (obviously there are limits to that, but this is a low risk change).

There will likely be a beta-based DoH study on the horizon - though I suspect beta will be on 63 when that happens..  but its nice to be prepared just in case.
You need to log in before you can comment on or make changes to this bug.