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

Sketch out a CORP-only mode. #893

Closed
wants to merge 3 commits into from
Closed

Sketch out a CORP-only mode. #893

wants to merge 3 commits into from

Conversation

mikewest
Copy link
Member

A straw-proposal to flesh out the discussion in whatwg/html#4175 (comment).

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

My main worry here is still that depending on how things shake out long term this could effectively be CORS, without it being clear and without the additional credentials opt-in we require there. (I.e., this is as simple to configure as Access-Control-Allow-Origin: * while potentially having greater risk.)

<var>request</var>'s <a for=request>tainted origin flag</a> is not checked.
<li><p><var>request</var> is a <a>navigation request</a> whose <a for=request>reserved client</a>
is an <a for=/>environment</a> whose <a for=environment>target browsing context</a> is neither a
<a>nested browsing context</a> nor an <a>auxiliary browsing context</a>.
Copy link
Member

Choose a reason for hiding this comment

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

This should explain why the auxiliary navigating the non-auxiliary is not a problem.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, hrm. I guess that actually is a problem without process isolation. attacker.site could open attacker.site in a new window, the latter could navigate the former to victim.site, and sadness would ensue. Perhaps this should instead ask whether we're navigating within a unit of related browsing contexts? Or whatever the new hotness is? "User agent cluster"?

Copy link
Member

Choose a reason for hiding this comment

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

Without process isolation on the agent cluster level, right.

I think the navigation checks only make sense when the flag is set on a document. And that flag only makes sense if COOP is also set. (COOP guaranteeing process isolation on the browsing context group level.)

Copy link
Member

Choose a reason for hiding this comment

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

fetch.bs Outdated

<li>
<p>If the following are true
<p>If <var>policy</var> is null, and <a>request</a>'s <a for=request>client</a>'s
Copy link
Member

Choose a reason for hiding this comment

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

Is this what we want? I kinda feel like an erroneous policy should fail closed when this TBD boolean is set.

Copy link
Member Author

Choose a reason for hiding this comment

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

I want to be able to support new values for this header in the future (for example, a list of origins a la #760). I'm happy to tighten this up, but I'd like to figure out how to do so in a forward-compatible way.

Copy link
Member

Choose a reason for hiding this comment

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

I guess the question then is what the migration strategy has to be. In equivalent cases we've failed for potentially new syntax and new servers will have to update (e.g., all the recentish and proposed CORS changes).

@annevk
Copy link
Member

annevk commented Jul 6, 2020

9ff55e4 from PR #1030 addressed this.

@annevk annevk closed this Jul 6, 2020
@annevk annevk deleted the corp-only branch July 6, 2020 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants