1599816 - Requests are being handled via TRR while connected to a PPTP VPN server

Status: VERIFIED FIXED

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

Description

[Emil Ghitta, QA [:emilghitta]]

Affected versions

• Firefox 72.0a1 (BuildId:20191126212708)

Affected platforms

• Windows 10 64bit.

Preconditions

• Connect to a VPN server:
  Steps to configure VPN server: https://my.ibvpn.com/knowledgebase/267/Set-up-the-PPTP-on-Windows-10.html
  Free VPN servers: https://www.vpnbook.com/

Steps to reproduce

1. Launch Firefox.
2. Access the about:config page.
3. Set the network.trr.mode pref to 2.
4. Restart Firefox.
5. Access a random domain.

Expected result

• A VPN connection is detected and the request is not being handled via TRR.

Actual result

• The request is being handled via TRR.

Regression Range

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

Comment 1

[Honza Bambas (:mayhemer)]

Not sure about a priority, but there should be a code to detect VPN and turn off TRR already. Michal or Valentin may know more. This should be fixed in bug 1565004.

Comment 2

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

This is Windows only, using instructions provided by collaborators at Microsoft.
I'll try to diagnose why this doesn't work.

Comment 3

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

Hi Emil,

I just tested this on windows 10 - Built from https://hg.mozilla.org/mozilla-central/rev/329340b1608d79266217ae37b098fa270f444c89
When I turn on the PPTP VPN (de4.vpnbook.com from https://www.vpnbook.com/ ) new domains appear with TRR=false in about:networking
I also checked the logs and it says

2019-12-09 10:48:04.161000 UTC - [Parent 12708: Socket Thread]: D/nsHostResolver Resolving host [www.reddit.com] type 0. [this=000002CE1A575C10]
2019-12-09 10:48:04.161000 UTC - [Parent 12708: Socket Thread]: D/nsHostResolver www.reddit.com is excluded from TRR because of platform indications

So it seems to me like this works.
Could you give it another go? And if you can reproduce it, can you get some HTTP logs with MOZ_LOG=sync,timestamp,nsNotifyAddr:5,nsHostResolver:5

Thanks!

Comment 4

[Emil Ghitta, QA [:emilghitta]]

I can confirm that this issue is not reproducible on the build provided in comment 3. The domains are no longer handled via TRR while a PPTP VPN connection is in place.

Comment 5

[Emil Ghitta, QA [:emilghitta]]

Attachment: log.txt.moz_log

Please Ignore my previous comment (comment 4). I am able to reproduce this issue using Firefox 73.0a1 (BuildId:20191209095039).

Attaching log file.

Comment 6

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

Hi Emil,

Great work tracking this down. The cause is that after computing the VPN/Suffix we send a NS_DNS_SUFFIX_LIST_UPDATED_TOPIC, but the TRRService only updates its VPN status when a network change topic is received. Sometimes those might come at close times, and it works, sometimes they might happen separately, and we get this bug.

I'll write up a patch ASAP.

Comment 7

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

Attachment: Bug 1599816 - TRR: Make sure we recheck for VPNs on all relevant observer topics r=kershaw

This bug is caused by bug 1598676 (Adds NS_DNS_SUFFIX_LIST_UPDATED_TOPIC), and
bug 1598676 (makes CheckAdaptersAddresses be called at startup)

While at startup it's sometimes the case that the NS_NETWORK_LINK_TOPIC and
NS_DNS_SUFFIX_LIST_UPDATED_TOPIC are both called, due to coalescing and
other link service quirks (or bugs) that doesn't happen for every network
change.

This means that we end up calling RebuildSuffixList after every
CheckAdaptersAddresses call, but we don't do that for CheckPlatformDNSStatus.

This patch makes it so that we always call both methods regardless which of
the two observer notifications was received.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53c316fbea27
TRR: Make sure we recheck for VPNs on all relevant observer topics r=kershaw

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/53c316fbea27

Comment 10

[Julien Cristau [:jcristau]]

Should we uplift this to beta?

Comment 11

[Emil Ghitta, QA [:emilghitta]]

This issue is verified fixed using Firefox 73.0a1 (BuildId:20191211094640) on Windows 10 64bit. Tried to reproduce this issue a couple of times but failed to do so. It looks that domain requests are no longer being handled via TRR if a PPTP VPN connection is used.

Pending for a possible beta uplift.

Comment 12

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

Comment on attachment 9114675 [details]
Bug 1599816 - TRR: Make sure we recheck for VPNs on all relevant observer topics r=kershaw

Beta/Release Uplift Approval Request

• User impact if declined: VPN detection will only happen on the first network change leading to the possibility that the user will be sending TRR requests even though there is an active VPN.
• Is this code covered by automated tests?: Unknown
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Configure and start a VPN connection (comment 0)
  Start firefox
  Browse around
  Check about:networking#dns that no requests used TRR
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Since bug 1598676 and bug 1598676 are both in fx 72 we should uplift this too, so users don't have an inconsistent experience.
• String changes made/needed:

Comment 13

[Julien Cristau [:jcristau]]

Comment on attachment 9114675 [details]
Bug 1599816 - TRR: Make sure we recheck for VPNs on all relevant observer topics r=kershaw

DoH vs VPN fix, approved for 72.0b7

Comment 14

[Julien Cristau [:jcristau]]

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

• Why is the change risky/not risky? (and alternatives if risky): Since bug 1598676 and bug 1598676 are both in fx 72 we should uplift this too, so users don't have an inconsistent experience.

I assume one of these 1598676 should have been some other bug number :)

Comment 15

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/releases/mozilla-beta/rev/34077af5ca8b

Comment 16

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

This issue is verified fixed using Firefox 72.0b7 on Windows 10 64bit.