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

Sec-Fetch-Dest is not trustworthy #10

Closed
anforowicz opened this issue Jan 16, 2019 · 4 comments
Closed

Sec-Fetch-Dest is not trustworthy #10

anforowicz opened this issue Jan 16, 2019 · 4 comments

Comments

@anforowicz
Copy link

Sec-Fetch-Dest seems less trustworthy than other headers proposed in https://mikewest.github.io/sec-metadata. Browsers with Site Isolation can ensure that Sec-Fetch-Mode, Sec-Fetch-Site, Sec-Fetch-User are trustworthy (i.e. ensure that an attacker can't spoof these headers even if the attacker is able to exploit a bug in the renderer process to gain ability to execute arbitrary code within the renderer's sandbox). In contrast, the values of Sec-Fetch-Dest header are somewhat untrustworthy.

What is the utility and value of the Sec-Fetch-Dest header? Some values of the Sec-Fetch-Dest header are somewhat redundant wrt Sec-Fetch-Mode (i.e. AFAICT document and nested-document' destinations will always be associated with navigate` mode).

@anforowicz
Copy link
Author

cc @arturjanc @csreis

@arturjanc
Copy link
Contributor

That's an interesting point! Do you think browsers will be able to give more assurances about Mode than about Dest? (They are both fields on Fetch's Request object that I would expect to be treated similarly by the browser)

There are two main uses that we've been thinking about for the destination field:

  • Disambiguating between top-level and nested navigations. This would allow the server to reject requests that would end up as iframes in an attacker-controlled document; a server-side equivalent to X-Frame-Options of sorts.
  • Ensuring that the server responds with the right Content Type based on the context of the request. For example, a server which emits an authenticated image/png response could ensure that the requester will use it as an image. This is particularly useful for banning application responses from being used as plugin resources (destination=object), as these have traditionally caused content-sniffing XSS-es.

FWIW we might be okay if we consider the scope of Sec-Metadata to only cover web attacks, rather than compromised renderers. Most of the security decisions will likely be made by servers based on the Site or Mode, e.g. rejecting non-navigational cross-site requests, so even in the event of a spoofable Dest the logic based on the other fields could be useful (and Dest would still help for regular web attacks.)

@mikewest, would it make sense to have a non-normative note about this in the spec?

@anforowicz
Copy link
Author

Do you think browsers will be able to give more assurances about Mode than about Dest?

Yes! It should be possible to for the supervisor/browser-process to know and enforce the difference between document and nested-document and maybe workers (e.g. sharedworker and serviceworker), but the difference between image and script (and most other values of Sec-Fetch-Dest) has to be declared by the renderer-process/html-parsing-process.

I am not sure if the difficulty here is fundamental or potentially solvable in the future. Consider an attacker that injects an <img> tag to "spoof" Sec-Fetch-Dest and then reads the data using either Spectre or compromising the renderer - I guess we could theoretically prevent this attack by storing cross-origin / cross-site image data in a separate process, but this seems difficult in practice (seems that cross-process css would be required).

Disambiguating between top-level and nested navigations.

Would it be possible to explicitly account for these in Sec-Fetch-Mode by splitting navigate into navigate-top-level and navigate-nested? (this is just an idea - maybe doing this would help avoid having to annotate each of Sec-Fetch-Dest's specific values as secure or not).

@mikewest
Copy link
Member

Given the way the conversation has moved, this feels like a duplicate of #16. Marking it as such.

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

No branches or pull requests

3 participants