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

Navigation algorithm consults filtered header list #8740

Closed
domenic opened this issue Jan 18, 2023 · 4 comments · Fixed by whatwg/fetch#1618
Closed

Navigation algorithm consults filtered header list #8740

domenic opened this issue Jan 18, 2023 · 4 comments · Fixed by whatwg/fetch#1618

Comments

@domenic
Copy link
Member

domenic commented Jan 18, 2023

See discussion in #8686 (comment). I'm not sure I understand this perfectly but I think the issue is: whenever the navigation algorithm looks at a response, it actually is looking at the filtered response. This is not good because the filtered response is missing lots of headers.

I'm not sure whether this is redirect-specific or general.

#8686 fixes one instance of this. But we should probably do a more comprehensive fix, and perhaps revert that.

Ideas for comprehensive fixes:

  • Audit every time we look at a response in the navigate algorithm and make sure the filtering doesn't cause problems. If it does, reach into the unsafe response.
  • Stop filtering responses, at the Fetch level, whenever mode = "navigate".
  • In the HTML level, immediately pull out the unsafe response early in the navigate fetching algorithms, and always operate on that

@annevk, can you suggest a strategy? I'd be happy to work on PRs to implement it, I just am a bit out of my depth.

domenic pushed a commit that referenced this issue Jan 18, 2023
Using the filtered response's header list in case of a redirect meant we wouldn't get access to the COOP header.

#8740 tracks the more general issue here.
@annevk
Copy link
Member

annevk commented Jan 18, 2023

I think using a basic filtered response for navigation responses is the way to go.


Assume a browser architecture whereby there is a Browser process managing UI and navigations and zero or more Website processes responsible for agent clusters. Ideally most of the navigation, including redirect handling is handled by the Browser process and only at the end of that a decision is made to allocate a Website process for whatever authority the navigated to URL has.

With such an architecture the final response will not have have access to the redirect responses and it is evidently same-origin with its own response so any kind of filtering is mostly in vain, but filtering out HTTP cookies still seems valuable. (Now to be clear, this assumes a kind of filtering that's currently not implemented. In most implementations the filters are not robust against process-level attacks, though perhaps for HTTP cookies some are.)

I think there is a future where we could be more ambitious about filtering. E.g., essentially safelist the headers we know the navigate algorithm needs and block everything else. It's similar to opaque and poking select holes through it, but makes it much more clear to implementers and reviewers what the holes are. But you could essentially only test it through a Spectre attack so perhaps that's more of a "best in class" achievement for implementations to aim for and not something to define in the specification.

(@arturjanc these kind of musings prolly interest you.)

@domenic
Copy link
Member Author

domenic commented Jan 18, 2023

So is that the "audit and reach in" option from my list?

@annevk
Copy link
Member

annevk commented Jan 19, 2023

No, if we change Fetch to return a basic filtered response for navigations HTML doesn't need any changes because it doesn't inspect Set-Cookie headers. (A basic filtered response only filters out Set-Cookie and Set-Cookie2 headers.)

Everything below that was rationale for why that would be okay and how we could improve upon it, but also why that improvement wouldn't be black-box-distinguishable and therefore somewhat questionable as a time investment.

@domenic
Copy link
Member Author

domenic commented Jan 19, 2023

Got it. So the issue is that although the initial response's response tainting mode is "basic", step 7 of HTTP fetch doesn't currently do the right thing:

Set response to an opaque-redirect filtered response whose internal response is actualResponse. If request’s mode is "navigate", then set fetchParams’s controller’s next manual redirect steps to run HTTP-redirect fetch given fetchParams and response.

It should set it to a basic filtered response if mode = navigate. I'll give it a shot.

domenic added a commit to whatwg/fetch that referenced this issue Mar 8, 2023
domenic added a commit that referenced this issue Mar 8, 2023
This reverts most of commit 63e2f17 (except for a typo fix). Now that the more general issue is fixed (see #8740 and whatwg/fetch#1618), we no longer need to look at unsafe responses inside the navigate algorithm.
domenic added a commit that referenced this issue Mar 8, 2023
This reverts most of commit 63e2f17 (except for a typo fix). Now that the more general issue is fixed (see #8740 and whatwg/fetch#1618), we no longer need to look at unsafe responses inside the navigate algorithm.
domenic added a commit to whatwg/fetch that referenced this issue Mar 24, 2023
annevk pushed a commit to whatwg/fetch that referenced this issue Mar 24, 2023
annevk pushed a commit that referenced this issue Mar 24, 2023
This reverts most of commit 63e2f17 (except for a typo fix). Now that the more general issue is fixed (see #8740 and whatwg/fetch#1618), we no longer need to look at unsafe responses inside the navigate algorithm.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants