[tor-bugs] #21952 [Applications/Tor Browser]: Onion-location: increasing the use of onion services through automatic redirects and aliasing
Tor Bug Tracker & Wiki
blackhole at torproject.org
Thu Jan 9 10:48:18 UTC 2020
#21952: Onion-location: increasing the use of onion services through automatic
redirects and aliasing
-------------------------------------------------+-------------------------
Reporter: linda | Owner: acat
Type: project | Status:
| needs_review
Priority: Medium | Milestone:
Component: Applications/Tor Browser | Version:
Severity: Normal | Resolution:
Keywords: ux-team, tor-hs, network-team- | Actual Points: 9
roadmap-november, tbb-9.5, |
TorBrowserTeam202001R |
Parent ID: #30024 | Points: 6
Reviewer: pospeselr, mcs, brade | Sponsor:
| Sponsor27-must
-------------------------------------------------+-------------------------
Comment (by acat):
Thanks for the reviews. Revised in https://github.com/acatarineu/tor-
browser/commits/21952+3.
\\
> Kathy and I think it would be safer to use a higher bit for
LOAD_FLAGS_ONION_REDIRECT, since we are not sure where or how the lower
bits are used. The comment "reserved for internal use by nsIWebNavigation
implementations" makes us think that other code may count on 0x1, 0x2,
0x4, and 0x8 being available/unused by the core code.
Yes, that was also my concern, but unfortunately all the other reload
flags are already used, I think. See https://github.com/acatarineu/tor-
browser/blob/3a9929a559c8505f3032f8f317fbb600d1a71e92/docshell/base/nsIWebNavigation.idl#L87.
If this is too risky, I could try to find an alternative way of doing the
"manual" redirect without the new reload flags.
\\
> Do you think that would make the patches simpler or more complex? From
Mozilla's perspective (assuming we could possibly uplift these patches
someday), the C++ approach is probably better.
I'm not so sure, to be honest. I guess it's fine to keep it C++.
\\
> Clicking the [x] has the same effect as clicking "Not Now" and therefore
the [x] does not seem necessary.
Fixed.
\\
>> Using a doorhanger for the prompt makes us think that it is safe to
click the [.onion available] button to dismiss the prompt (because that is
how most of Firefox's other doorhangers work). I am not sure how to fix
this issue though.
> Well, it *is* safe to click the [.onion available]. And actually, since
we are promoting the discovery doorhanger just the first time and
meanwhile the automatic redirect is not enabled in about:preferences, it
should get closed when the button is clicked.
So, I'm assuming this is fine as it is for now.
\\
>> There is no way to dismiss the prompt in such a way that it will be
shown a second time, which surprises Kathy and me.
> True. I made a version with a [Ask me again in the future], but it
slightly moved out-of-scope. I'm happy to explore that option in the
future (as a non-prompt/transparent re-direct flow too).
Ok, leaving this one as it is for now too.
\\
>> But I guess the "Always prioritize Onions" prompt is only intended to
serve as a form of onboarding. I wonder if the prompt should be adjusted
to open about:preferences instead of directly modifying the
privacy.prioritizeonions.enabled pref? Opening the preferences would teach
users where to find the setting in the future.
>Maybe, yes. Since it is a global feature and also if selected, the prompt
will not show anymore, we may want to open about:preferences also as an
educational opportunity.
Right now the behaviour is to flip the pref and reload the page so that it
redirects to .onion automatically. Instead of that, should the new
behaviour be just opening about:preferences without flipping the pref and
without redirecting the original page to .onion? I revised with the latter
behaviour for now, but asking to make sure.
\\
> In modules/libpref/init/StaticPrefList.h, please add a comment that
explains the purpose of the privacy.prioritizeonions.enabled preference.
Done.
\\
> We will need a torbutton.dtd patch to add the strings. Since these are
new strings, you could use a properties file instead; see our patch here
for an example: https://gitweb.torproject.org/user/brade/tor-
browser.git/diff/browser/modules/TorStrings.jsm?h=bug30237-03&id=509999fe8ef29fa3cb2915bdfb050d4512014081
Changed to use the `TorPropertyStringBundle` from the patch you pointed
to.
\\
> In OnionLocationParent.jsm inside the setOnionLocation() and
updateOnionLocationBadge() functions, is it necessary to check browser
against ...selectedBrowser? As it is now, if an https page that returns an
Onion-Location header finishes loading in the background the [.onion
available] button is not displayed.
Nice catch, thanks. I remember fixing this before, so I must have re-added
the bug when moving the code to `OnionLocationParent.jsm` (initially it
was in browser.js).
\\
> A nit: in browser/components/onionlocation/content/onionlocation-
urlbar.css, please add a space before each occurrence of !important
(easier to read, at least for Kathy and me).
Fixed.
\\
> Do we need another ticket to track creation of a new page for the
"learnMore" link or is there an existing page that we can use?
I do not see it, should I add it as a child of this bug?
\\
> So one weird thing that stands about the changes to nsHttpChannel.cpp is
that the new Onion-Location code seems to supersede all logic surrounding
the returned HTTP Status. Whether that's okay or not kind of depends on
the Onion-Location spec. How are properly configured web-servers meant to
use the Onion-Location header? Is it meant to be there in every HTTP
response sent to the client, or only in certain situations? The spec is
unclear about which HTTP status codes it is meant to be used with. It does
state that Onion-Location has the same restrictions and semantics as
Location which according to Mozilla only has meaning for 3XX and 201
responses. If we are only supposed to redirect in those contexts then the
checks for that block checking for and getting the Onion-Location header
could (probably?) go down into nsHttpChannel::AsyncProcessDirection.
It's an interesting point. One thing I was worried is that there could be
problems with 30X responses without `Location` header (and `Onion-
Location` instead). However, from the spec point of view it should be
fine, as it seems not mandatory:
https://stackoverflow.com/questions/16194988/for-which-3xx-http-codes-is-
the-location-header-mandatory. I also tested it in Firefox, 30X responses
without `Location` seem to be displayed correctly (except 304, where the
body is ignored). A different issue is whether the adoption by website
owners would be more difficult, as it's probably slightly more complicated
to change the response code to 30X in addition to setting the custom
`Onion-Location` header (and without the `Location` one). Besides, given
that the `Onion-Location` also has an "informative" behaviour (when user
has not enabled auto-redirects), it seems strange that the responses have
to always be 30x for the user to be informed about the `.onion available`.
In any case, I'm not so sure about this, and I think I have already
thought too much about it :) I think this should be discussed and we
should specify more precisely the `Onion-Location` header behaviour
depending on the response codes and whether the user has enabled automatic
.onion redirects or not. So I would suggest keeping the current behaviour
for now, until this is discussed/decided/specified. I just made one
change: if we are going to ignore the status code for 'Onion-Location' for
the redirects, I think it makes sense to set `redirectType` to a fixed one
(e.g. 308), instead of passing the actual response code.
\\
> We don't seem to check if we are *already* on the Onion site the Onion-
Location header suggests we redirect to.
I think this case is indirectly handled by the logic in
https://github.com/acatarineu/tor-
browser/commit/3a9929a559c8505f3032f8f317fbb600d1a71e92#diff-
70a990c1d10c050c5fcc69b226c33c5eR2645, since we only do the redirects if
the original url is not .onion and https and the Onion-Location is .onion.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/21952#comment:97>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
More information about the tor-bugs
mailing list