Skip to content
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

Use HTML's origin serialization, add note regarding TAO and redirects #172

Closed
wants to merge 4 commits into from

Conversation

yoavweiss
Copy link
Contributor

@yoavweiss yoavweiss commented Oct 12, 2018

Addresses some of the concerns for #152


Preview | Diff

index.html Outdated Show resolved Hide resolved
<p class="issue" data-number="152">Above "timing allow check" algorithm should also verify that
no cross-origin redirections were involved in the response, as that would
leak timing information about those origins.
</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't it do that already? As in, we run TAO check on every response in the processing model and reset the values if the check fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have the concept of a "tainted origin" until we integrate with Fetch. The current processing model talks about "the last non-redirected fetch of the resource", but as @annevk pointed out, there can be a SameOrigin=>X-Origin=>SameOrigin scenario where the spec as is can expose cross-origin information.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would it expose in this case?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that X-Origin could influence the final SameOrigin in such a way as to attack it. CORS also requires the appropriate headers in such a scenario.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One scenario could be:

  • malicious.com redirects a resource to victim.com (which unfortunately exposes a redirection endpoint, which can redirect back to any arbitrary URL). It uses the redirection endpoint to redirect back to malicious.com.
  • With current language, that would expose timing information about victim.com, which may be used to conclude's the user's login state, or other personal information.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That example doesn't really illustrate why malicious.com would need a TAO header on the final response. This is victim.com requesting something from target.com, which is either compromised or redirects to a a page on victim.com in order to confuse victim.com (or there are even more domains in the chain). And in that case we want victim.com to explicitly opt-in so that we know it agreed to be confused.

Copy link
Contributor Author

@yoavweiss yoavweiss Jan 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@annevk given the (handwavy) tackling of this in #196, do we still need an explicit note here in your opinion?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because the timing allow check itself returns "pass" if the "current resource" is same-origin with the requesting document. That means that EXAMPLE -> ATTACKER -> EXAMPLE would leak because ATTACKER would surely set the header and EXAMPLE does not need to.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@yoavweiss
Copy link
Contributor Author

This PR is obsoleted by #197

@yoavweiss yoavweiss closed this Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants