-
Notifications
You must be signed in to change notification settings - Fork 332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Send "null" Origin header on cross-origin .onion requests #1351
base: main
Are you sure you want to change the base?
Conversation
1f75092
to
fefa1dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing remaining here is that if we want to have this check in at least three places, we should probably have an exported "is an onion URL" predicate that takes a URL and returns true or false instead. (Other things are tests, Mike's feedback, whether this should be optional somehow or conditional upon .onion
support...)
The other big remaining thing here is making sure this would be on track to become a part of the interoperable web, via implementation (on by default) in WebKit/Gecko/Chromium. |
Yes and trying to see whether there's agreement on the correct way to handle these was the reason I created these PRs in the first place. In the short term, Brave is going to match what the Tor Browser is doing, but ideally we'd like to see if we can align with the browsers that don't bundle Tor directly. |
On this specific point, I would suggest the answer is no, it should be always ON because there's no easy way for the browser to know that the SOCKS5 proxy it's using is the Tor daemon. |
f4dd516
to
b4aebfd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good editorially. I guess there's still the question of whether we want to abstract "is an origin URL", but that could happen later as well.
So this mainly hinges on implementer interest and resolving the question as to whether we should expose more information when CORS is used.
<li><p>If <var>request</var>'s <a for=request>current URL</a>'s <a for=url>origin</a>'s | ||
<a for=origin>host</a> ends with "<code>.onion</code>" or "<code>.onion.</code>", and | ||
is not <a>same origin</a> with <var>request</var>'s <a for=request>origin</a>, then set | ||
<var>serializedOrigin</var> to `<code>null</code>`. [[ONION]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to "byte-serializing a request origin" we'll also end up with null if they are same origin but there's a cross-origin redirect in between. Ideally we'd test that, but I guess .onion
will be hard to test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've got these two no-cors manual test cases on my test page (you can also see it at http://fmarier.com/referrer/onion.html though it's meant to run from a .onion
) :
example.onion
--POST-->example.onion
--307-->example.com
example.onion
--POST-->example.com
--307-->example.onion
For the first one, both Brave and Tor Browser send a null
origin (and omit the referrer for that matter).
For the second one, however, Brave sends a null
origin but the Tor browser sends the full Origin in that case.
Is a null
Origin the desired behavior in both cases or just in the first one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand the examples, what's the starting origin? As long as you are same-origin I would expect a referrer to be included, but once you've gone cross-origin once, it should no longer be possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The starting origin in all cases is example.onion
. It issues two 307 redirects.
In the first example, it goes from example.onion
to example.onion/anotherpage
before ending on example.com
.
In the second example, it goes from example.onion
to example.com
before ending on example.onion/anotherpage
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the user navigates to the starting origin? I guess then the scenarios would work. (Typically with fetching there's a starting document with some kind of authority and that would do the fetching, which could then result in a redirect. The initial 307 threw me off.)
Assuming the user does the navigation I'd expect:
- example.onion 307 to example.onion (Origin could be included here, e.g., in case of a POST); example.onion 307 to example.com (it would be null here)
- example.onion 307 to example.com (it would be null here); example.com to example.onion (it would continue to be null here, as it's been tainted by example.com)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the user navigates to the starting origin?
Yes, that's correct. However, I just realized that I made a mistake in my description of the test cases, which probably threw you off.
What I meant was:
- Navigate to
example.onion/index.html
. - Submit form (POST) to
example.onion/another.html
. example.onion/another.html
is a 307 redirect toexample.com
.- You end up on
example.com
with anull
Origin
header.
The second test case is:
- Navigate to
example.onion/index.html
. - Submit form (POST) to
example.com
. example.com
is a 307 redirect toexample.onion/another.html
.- You end up on
example.onion/another.html
with anull
Origin
header.
I had the same expectations as you (regarding the example.com
tainting due to the redirect) and that's what I implemented in Brave, but Tor Browser returns an Origin
of example.onion
in the second case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems like a pretty serious bug. @KershawChang @valenting ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've filed bug 1742784 for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
Fixes #1350.
(See WHATWG Working Mode: Changes for more details.)
Preview | Diff