[tor-bugs] #28005 [Applications/Tor Browser]: Officially support onions in HTTPS-Everywhere

[Tor Bug Tracker & Wiki]

#28005: Officially support onions in HTTPS-Everywhere
-------------------------------------------------+-------------------------
 Reporter:  asn                                  |          Owner:  tbb-
                                                 |  team
     Type:  defect                               |         Status:
                                                 |  needs_review
 Priority:  Medium                               |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tor-hs, https-everywhere, network-   |  Actual Points:  17
  team-roadmap-november, network-team-roadmap-   |
  2020Q1, TorBrowserTeam202003R, ux-team         |
Parent ID:  #30029                               |         Points:  20
 Reviewer:  mcs, sysrqb, antonela                |        Sponsor:
                                                 |  Sponsor27-must
-------------------------------------------------+-------------------------
Changes (by acat):

 * keywords:
     tor-hs, https-everywhere, network-team-roadmap-november, network-team-
     roadmap-2020Q1, TorBrowserTeam202002, ux-team
     =>
     tor-hs, https-everywhere, network-team-roadmap-november, network-team-
     roadmap-2020Q1, TorBrowserTeam202003R, ux-team
 * actualpoints:  15 => 17


Comment:

 Thanks for the review. I addressed the issues in
 https://github.com/acatarineu/tor-browser/commit/28005+1.

 Apart from this, I also added support for `get_simple_rules_ending_with`
 which was introduced in https://github.com/EFForg/https-
 everywhere/pull/18994. I think this handles your concern

 >In browser/components/onionalias/OnionAliasStore.jsm, it seems that
 nothing is ever removed from _onionMap while the browser is running. Will
 this cause any problems? For example, what happens if an HTTPS-E rule
 update removes some mappings?

 better than the previous patch. However, since this is still not in the
 release version of HTTPS Everywhere there is a fallback using the first
 patch approach, a http observer. I hope these are not too many changes for
 a second review. Most of the code for this was added in
 `OnionAliasStore.jsm`, and some in `HttpsEverywhereControl.jsm`. I also
 added some logging in `OnionAliasStore.jsm` and added some more comments
 too.

 The only comment I did not address yet was

 > UX: For the circuit display, does it make sense to show the full onion
 address on hover? (in addition to the "Click to Copy" functionality). That
 could be done with a tooltip and would allow users to see the complete
 name without pasting somewhere else.

 as I thought we could wait for antonela's thoughts about it.
 ----

 > We wish that the tor-browser patch did not have to touch the Firefox
 code in so many places. That might be unavoidable though. Please add some
 comments near the added codeblocks; for example, to explain the places you
 perform origin checks.

 Indeed, me too. I think we could avoid these changes if it would be fine
 to treat the case when user types a .tor.onion **exactly** the same way as
 when the user types the .onion directly, meaning that both end up showing
 the same memorable URL in the urlbar, and there is no other UI element
 that is shown in one case and not in the other, for example. In this case,
 I think we would not need to maintain the state of whether the user
 started the navigation with a .tor.onion, and we could get rid of all the
 code tracking this for all the subnavigations. In part, this should now be
 possible because of the new `"get_simple_rules_ending_with"` (once this is
 released in https everywhere).

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/28005#comment:28>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online