Re: [tor-bugs] #33962 [Applications/Tor Browser]: Uplift patch for 5741 (dns leak protection)

[Tor Bug Tracker & Wiki]

#33962: Uplift patch for 5741 (dns leak protection)
-+-
 Reporter:  acat |  Owner:  acat
 Type:  task | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ReleaseTrainMigration|  Actual Points:
  TorBrowserTeam202005R  |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor58
-+-
Changes (by acat):

 * keywords:  ReleaseTrainMigration TorBrowserTeam202005 =>
 ReleaseTrainMigration TorBrowserTeam202005R
 * status:  new => needs_review


Comment:

 The followup landed: https://hg.mozilla.org/mozilla-
 central/rev/1bd7b8776812. So the two patches for review would be
 https://hg.mozilla.org/mozilla-central/rev/6bbed9a7eb4b and
 https://hg.mozilla.org/mozilla-central/rev/1bd7b8776812.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33962 [Applications/Tor Browser]: Uplift patch for 5741 (dns leak protection)

[Tor Bug Tracker & Wiki]

#33962: Uplift patch for 5741 (dns leak protection)
-+-
 Reporter:  acat |  Owner:  acat
 Type:  task | Status:  new
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ReleaseTrainMigration|  Actual Points:
  TorBrowserTeam202005   |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor58
-+-
Changes (by acat):

 * keywords:  ReleaseTrainMigration TorBrowserTeam202005R =>
 ReleaseTrainMigration TorBrowserTeam202005
 * status:  needs_review => new


Comment:

 Removing review_needed, as this second push might change the original
 patch that landed more than I expected.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33962 [Applications/Tor Browser]: Uplift patch for 5741 (dns leak protection)

[Tor Bug Tracker & Wiki]

#33962: Uplift patch for 5741 (dns leak protection)
-+-
 Reporter:  acat |  Owner:  acat
 Type:  task | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ReleaseTrainMigration|  Actual Points:
  TorBrowserTeam202005R  |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor58
-+-

Comment (by acat):

 Note: I missed a path (which should not affect us), so trying to get this
 followup landed too: https://phabricator.services.mozilla.com/D74626.

 The issue is that already cached entries (when `network.dns.disabled =
 false`) may result in network connection much later when
 `network.dns.disabled = true`, as they are asynchronously renewed when
 they expire. This should not affect us in the sense that if we set
 `network.dns.disabled = true` right from the beginning, the only cached
 entries would be IP literals, which never expire.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33962 [Applications/Tor Browser]: Uplift patch for 5741 (dns leak protection)

[Tor Bug Tracker & Wiki]

#33962: Uplift patch for 5741 (dns leak protection)
-+-
 Reporter:  acat |  Owner:  acat
 Type:  task | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ReleaseTrainMigration|  Actual Points:
  TorBrowserTeam202005R  |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor58
-+-
Changes (by acat):

 * status:  accepted => needs_review


Comment:

 This landed in https://hg.mozilla.org/mozilla-central/rev/6bbed9a7eb4b.
 Hopefully it's good enough for us to replace the #5741 patch by
 `network.dns.disabled = true`. Marking this as `needs_review` to check
 that.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33962 [Applications/Tor Browser]: Uplift patch for 5741 (dns leak protection)

[Tor Bug Tracker & Wiki]

#33962: Uplift patch for 5741 (dns leak protection)
-+-
 Reporter:  acat |  Owner:  acat
 Type:  task | Status:
 |  accepted
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ReleaseTrainMigration|  Actual Points:
  TorBrowserTeam202005R  |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor58
-+-

Comment (by acat):

 I created https://bugzilla.mozilla.org/show_bug.cgi?id=1636411, expecting
 that it has good chances to be accepted and we can just flip
 `networking.dns.disabled` to  have the same protection as this patch. I
 think the logic for proxy bypass protection when `networking.dns.disabled
 = false` can be implemented later independently of that, but I assume that
 will be not so easy to be accepted (or to get right).

 But, unless I'm missing something, I don't think we really need to support
 resolving DNS correctly if some user disables the SOCKS proxy and wants a
 direct internet connection. That can only be done via `about:prefs`, so
 that user would just need to edit one more pref to achieve that.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33962 [Applications/Tor Browser]: Uplift patch for 5741 (dns leak protection)

[Tor Bug Tracker & Wiki]

#33962: Uplift patch for 5741 (dns leak protection)
-+-
 Reporter:  acat |  Owner:  acat
 Type:  task | Status:
 |  accepted
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ReleaseTrainMigration|  Actual Points:
  TorBrowserTeam202005R  |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor58
-+-
Changes (by acat):

 * status:  needs_review => accepted
 * owner:  tbb-team => acat


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33962 [Applications/Tor Browser]: Uplift patch for 5741 (dns leak protection)

[Tor Bug Tracker & Wiki]

#33962: Uplift patch for 5741 (dns leak protection)
-+-
 Reporter:  acat |  Owner:  tbb-
 |  team
 Type:  task | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ReleaseTrainMigration|  Actual Points:
  TorBrowserTeam202005R  |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor58
-+-

Comment (by acat):

 Ok, just saw https://bugzilla.mozilla.org/show_bug.cgi?id=1618271 where
 this is being discussed already...

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33962 [Applications/Tor Browser]: Uplift patch for 5741 (dns leak protection)

[Tor Bug Tracker & Wiki]

#33962: Uplift patch for 5741 (dns leak protection)
-+-
 Reporter:  acat |  Owner:  tbb-
 |  team
 Type:  task | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ReleaseTrainMigration|  Actual Points:
  TorBrowserTeam202005R  |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor58
-+-

Comment (by acat):

 I think it makes sense, I'll do that.

 FWIW, after thinking a bit more, I'm not even sure if the patch I attached
 here would be acceptable for Mozilla: it's not clear to me that DNS should
 be disabled iff `network.proxy.socks_remote_dns = true` and
 `network.proxy.type = MANUAL`. First, there is UDP (e.g. WebRTC would stop
 working if there's a proxy with this patch). And second, I think for this
 approach you would also have to check whether `network.proxy.socks` is
 non-empty.

 Perhaps it's worth to follow this
 [https://bugzilla.mozilla.org/show_bug.cgi?id=751465#c26 suggestion] and
 just put this behind a `network.dns.disabled` pref instead of trying to
 find a logic for calculating whether DNS has to be disabled that works for
 both Mozilla and us.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33962 [Applications/Tor Browser]: Uplift patch for 5741 (dns leak protection)

[Tor Bug Tracker & Wiki]

#33962: Uplift patch for 5741 (dns leak protection)
-+-
 Reporter:  acat |  Owner:  tbb-
 |  team
 Type:  task | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ReleaseTrainMigration|  Actual Points:
  TorBrowserTeam202005R  |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor58
-+-

Comment (by gk):

 Replying to [comment:2 acat]:
 > I adapted the patch from #5741 to try to upstream it. You can find it in
 https://github.com/acatarineu/tor-browser/commit/33962
 (f27d3258eb3ca2a86774342248184c8111546dab).
 >
 > I know we briefly discussed about having this behind the `--enable-
 proxy-bypass-protection`, but I think there *might* be chances for this to
 be upstreamed as it is now, and be useful for Firefox (it wouldn't be for
 sure if it's behind the proxy bypass flag).
 >
 > I did a couple of changes with respect to the original patch. The main
 one is that the patch I attached is checking that both `network.proxy.type
 = MANUAL` and `network.proxy.socks_remote_dns = true`, while the current
 patch only checks `network.proxy.socks_remote_dns = true`. I think this
 change is needed to avoid blocking DNS when we should not, for example in
 a situation where a user sets up a SOCKS proxy (enabling DNS through
 socks), and then switches back to 'No proxy', in `about:preferences`. I
 think the patch with these changes is safe enough for Firefox, in the
 sense that it should not result in undesired breakage.
 >
 > The question is whether is also safe for us, in terms of proxy bypass
 protection. My assumption is yes, as the only additional change is that we
 also check for `network.proxy.type`, and we don't support changing this in
 Tor Browser. But I think it's a good idea for this to be reviewed before
 trying to push the patch to Firefox. I added this to 202005, but please
 feel free to re-prioritize.

 Hrm. I wonder if it would be smarter to open a bug at bugzilla in the mean
 time (I don't see one filed as child of
 https://bugzilla.mozilla.org/show_bug.cgi?id=1433504) and get feedback
 about what would be acceptable for Mozilla and then write a patch that
 would fix this bug, too). I mean we could go through the review process
 here and maybe merge your patch to our tree just to write yet another
 patch which Mozilla would accept. I have some hope, though, we can avoid
 the first part and save us some time. :) What do you think?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33962 [Applications/Tor Browser]: Uplift patch for 5741 (dns leak protection)

[Tor Bug Tracker & Wiki]

#33962: Uplift patch for 5741 (dns leak protection)
-+-
 Reporter:  acat |  Owner:  tbb-
 |  team
 Type:  task | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ReleaseTrainMigration|  Actual Points:
  TorBrowserTeam202005R  |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor58
-+-
Changes (by acat):

 * status:  new => needs_review
 * keywords:  ReleaseTrainMigration => ReleaseTrainMigration
 TorBrowserTeam202005R


Comment:

 I adapted the patch from #5741 to try to upstream it. You can find it in
 https://github.com/acatarineu/tor-browser/commit/33962 (hash).

 I know we briefly discussed about having this behind the `--enable-proxy-
 bypass-protection`, but I think there *might* be chances for this to be
 upstreamed as it is now, and be useful for Firefox (it wouldn't be for
 sure if it's behind the proxy bypass flag).

 I did a couple of changes with respect to the original patch. The main one
 is that the patch I attached is checking that both `network.proxy.type =
 MANUAL` and `network.proxy.socks_remote_dns = true`, while the current
 patch only checks `network.proxy.socks_remote_dns = true`. I think this
 change is needed to avoid blocking DNS when we should not, for example in
 a situation where a user sets up a SOCKS proxy (enabling DNS through
 socks), and then switches back to 'No proxy', in `about:preferences`. I
 think the patch with these changes is safe enough for Firefox, in the
 sense that it should not result in undesired breakage.

 The question is whether is also safe for us, in terms of proxy bypass
 protection. My assumption is yes, as the only additional change is that we
 also check for `network.proxy.type`, and we don't support changing this in
 Tor Browser. But I think it's a good idea for this to be reviewed before
 trying to push the patch to Firefox. I added this to 202005, but please
 feel free to re-prioritize.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33962 [Applications/Tor Browser]: Uplift patch for 5741 (dns leak protection)

[Tor Bug Tracker & Wiki]

#33962: Uplift patch for 5741 (dns leak protection)
--+---
 Reporter:  acat  |  Owner:  tbb-team
 Type:  task  | Status:  new
 Priority:  Medium|  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  ReleaseTrainMigration |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:  Sponsor58
--+---
Changes (by acat):

 * type:  defect => task


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs