Closed Bug 1598676 Opened 4 years ago Closed 4 years ago

[OSX] Domains that are part of the DNS suffix list are being handled via TRR

Categories

(Core :: Networking: DNS, defect, P2)

72 Branch
All
macOS
defect

Tracking

()

VERIFIED FIXED
mozilla72
Tracking Status
firefox72 --- verified

People

(Reporter: emilghitta, Assigned: kershaw)

References

Details

(Whiteboard: [necko-triaged][trr])

Attachments

(4 files)

Attached file log.txt.moz_log

Affected versions

  • Firefox 72.0a1 (BuildId:20191122091512)

Affected platforms

  • macOS 10.14

Preconditions

  • Launch Firefox and set the network.trr.mode pref to 2.
  • Configure a local DOH server.
  • Set network.trr.uri to point out the DOH server.
  • Create a DNS suffix for a specific domain.

Steps to reproduce

  1. Access a domain that is part of the DNS suffix list.
  2. Access the about:networking#dns page.

Expected result

  • The request is not reaching the DOH server and it’s being handled via DNS.
  • The domain which was accessed at step 1 is displayed with TRR set to false.

Actual result

  • The request is reaching the DOH server.
  • The domain which was accessed at step 1 is displayed with TRR set to true.

Regression Range

  • I don’t think that this is a regression.

Notes

  • Attaching a log file for this.

Kershaw, can you take a look at this?

Blocks: 1588217
Flags: needinfo?(kershaw)
Priority: -- → P2
Whiteboard: [necko-triaged][trr]

I guess the problem is that we only refresh the DNS suffix list when getting a network change event in TRRService.

Emil, could you try to switch off and on your network interface after setting a DNS suffix and see if the domain can be accessed with TRR?
Please also add nsNotifyAddr:5 to MOZ_LOG. Thanks.

Assignee: nobody → kershaw
Flags: needinfo?(kershaw) → needinfo?(emil.ghitta)
Attached file nsNotifyAddr:5 log

I've switched my network connection off and back to on but the domains which are part of the DNS suffix list are still resolved via TRR.

Attaching the requested log.

Flags: needinfo?(emil.ghitta)

Hmm, it would seem we do detect the domains properly, but we still use TRR to get them?
It could be caused by bug 1597729, or maybe we just don't read them properly in the notification.
I would have expected to see this log line in the host resolver logs.

I've asked Emil to get the log again with MOZ_LOG=nsNotifyAddr:5,nsHostResolver:5.
The log shows that we do detect the domains.

[(null) 94483: StreamTrans #2]: D/nsNotifyAddr DNS search domain [portalsm.ro]
[(null) 94483: StreamTrans #2]: D/nsNotifyAddr DNS search domain [netflix.com]
[(null) 94483: StreamTrans #2]: D/nsNotifyAddr DNS search domain [9gag.com]

We also added the into mDNSSuffixDomains properly.

[Parent 94483: Main Thread]: D/nsHostResolver TRRService adding portalsm.ro to suffix list
[Parent 94483: Main Thread]: D/nsHostResolver TRRService adding netflix.com to suffix list
[Parent 94483: Main Thread]: D/nsHostResolver TRRService adding 9gag.com to suffix list

But, the host portalsm.ro is still resolved by TRR.

[Parent 94483: Main Thread]: D/nsHostResolver Resolving host [portalsm.ro] type 0. [this=0x10d910400]
[Parent 94483: Main Thread]: D/nsHostResolver   No usable record in cache for host [portalsm.ro] type 0.
[Parent 94483: Main Thread]: D/nsHostResolver TRR Resolve portalsm.ro type 1
[Parent 94483: Main Thread]: D/nsHostResolver   DNS lookup for host [portalsm.ro] blocking pending 'getaddrinfo' or trr query: callback [0x13aa7ede0]
[Parent 94483: Main Thread]: D/nsHostResolver Checking if host [portalsm.ro] is blacklisted
[Parent 94483: Main Thread]: D/nsHostResolver TRR::SendHTTPRequest resolve portalsm.ro type 1

I'll add more logs in TRRService::IsDomainBlacklisted to figure out the reason.

I think I've figured out the reason.
We update the DNS suffix every time when we receive NS_NETWORK_LINK_TOPIC notification.
When a link is up, we send the notification before updating the DNS suffix list. So, it is likely that we get an empty DNS suffix list in TRRService.

I think we should add a new topic network:dns-suffix-list-updated and send this notification when DNS suffix list is changed.

That's a great find. I made the change to respond to any NS_NETWORK_LINK_TOPIC notification in bug 1565004, because before we only responded to "changed" events, and that missed some notifications.

When a link is up, we send the notification before updating the DNS suffix list.

Is there any reason we can't do it before?

(In reply to Valentin Gosu [:valentin] (he/him) from comment #7)

That's a great find. I made the change to respond to any NS_NETWORK_LINK_TOPIC notification in bug 1565004, because before we only responded to "changed" events, and that missed some notifications.

When a link is up, we send the notification before updating the DNS suffix list.

Is there any reason we can't do it before?

The main reason is that we update the suffix list on a background thread, but NS_NETWORK_LINK_TOPIC notification is on main thread.
When a link is up, we send the notification immediately on main thread and dispatch a task for querying DNS suffix list on background thread.

I wonder if it's ok to just send another "changed" event once the suffix list is computed. That might cause failures like in bug 1589446 - in that case we'd have to add a new topic.

(In reply to Valentin Gosu [:valentin] (he/him) from comment #9)

I wonder if it's ok to just send another "changed" event once the suffix list is computed. That might cause failures like in bug 1589446 - in that case we'd have to add a new topic.

I'll add a new topic for this.

Emil, could you try again with this build?
https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/cvkKEB0EQ8KvVfBeysTqqA/runs/0/artifacts/public/build/target.dmg

Thanks.

Flags: needinfo?(emil.ghitta)

The try build provided in Comment 10 looks good. This issue is not reproducible on that one.

Flags: needinfo?(emil.ghitta)
Pushed by kjang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/114ac730d48a
P1: Send dns suffix list updated event (MacOS) r=valentin
https://hg.mozilla.org/integration/autoland/rev/ac9985059fae
P2: Send dns suffix list updated event (Windows/Linux/Android) r=valentin
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

This issue is verified fixed using Firefox 72.0a1 (BuildId:20191129094247). This was covered by our DNS Suffix List and TRR split horizont test run

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.