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

Add a security consideration about content sniffing. #348

Merged
merged 3 commits into from
Dec 19, 2018

Conversation

jyasskin
Copy link
Member

@jyasskin jyasskin commented Dec 8, 2018

This fixes #321, I think. @molnarg, how do you feel about this?

I'll need to add this to bundles too, but I want to get the SXG text right first.


Preview | Diff

@jyasskin jyasskin requested a review from twifkak December 8, 2018 00:55
twifkak added a commit to twifkak/amppackager that referenced this pull request Dec 10, 2018
This is to mitigate the content-sniffing risk outlined in
WICG/webpackage#348.
twifkak added a commit to twifkak/amppackager that referenced this pull request Dec 10, 2018
This is to mitigate the content-sniffing risk outlined in
WICG/webpackage#348.
@jyasskin jyasskin force-pushed the anti-content-sniffing branch 2 times, most recently from 308e7be to a70242c Compare December 10, 2018 23:02
Copy link
Collaborator

@nyaxt nyaxt left a comment

Choose a reason for hiding this comment

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

still lg

@nyaxt
Copy link
Collaborator

nyaxt commented Dec 13, 2018

@annevk
Copy link

annevk commented Dec 13, 2018

I need some help understanding the model.

Is the idea that application/signed-exchange and application/webbundle responses cannot be sniffed as scripts or same-origin quirks mode style sheets? If so, we could add them to https://fetch.spec.whatwg.org/#should-response-to-request-be-blocked-due-to-mime-type?. I don't think it matters for other nosniff contexts per https://mimesniff.spec.whatwg.org/#determining-the-computed-mime-type-of-a-resource.

Or is the idea that the responses they encompass (i.e., those in the bundle/archive) cannot be sniffed? If so, we could "dynamically inject" this header when we create those responses ensuring "determine nosniff" always returns true for them downstream.

@jyasskin
Copy link
Member Author

@annevk One of the bigger risks is that they might be sniffed as PDFs, Flash, or other plugin-recognized types. The risky file types don't necessarily follow the Fetch spec, so the header isn't guaranteed to work, but we thought it'd improve our chances.

@mikewest may have other answers. He suggested requiring nosniff here. We also want to make internal responses automatically nosniff, but that'll come in a separate change.

@annevk
Copy link

annevk commented Dec 13, 2018

Ah plugins, that is indeed a poorly specified area. Note that currently we don't have any language that suggests nosniff would work for plugins.

@jyasskin
Copy link
Member Author

Yep. I don't personally have evidence that this improves things for any particular plugin; we're just hoping. I don't have strong feelings about whether this is the right thing to do.

@annevk
Copy link

annevk commented Dec 13, 2018

I think we need to know concretely what this will help with so it can be tested for (and specified if it isn't already). In particular as we'd have to do this for other new formats too, presumably (e.g., Wasm)? (I see the benefits for the contained non-HTTP responses, but as you said that's separate.)

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Dec 14, 2018
To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
@mikewest
Copy link
Member

In general, I'd like for us to be strict about new mechanisms we're adding to the platform that might result in executable code. I don't have any concrete examples of places where plugins could misinterpret an SXG response as something unfortunate, but there are a number of examples of plugins' propensity to zealously sniff incoming content into executable code. I'd like to make that less likely.

That said, when I was talking with @jyasskin about this a few days ago, I incorrectly assumed that nosniff applied to plugin content. In the meantime, @jyasskin pointed out that https://html.spec.whatwg.org/multipage/iframe-embed-object.html#object-type-detection makes strictly obeying Content-Type headers entirely optional for plugin resources, and that we apparently don't do so in Chrome. I think we should change that (though I don't have any data that would suggest that it's safe to do so; I'll add some metrics). If we don't, then I think I agree with @annevk that the Fetch bits of this change wouldn't have any teeth.

Or is the idea that the responses they encompass (i.e., those in the bundle/archive) cannot be sniffed? If so, we could "dynamically inject" this header when we create those responses ensuring "determine nosniff" always returns true for them downstream.

As above, I'd like for us to strictly enforce MIME type checks for resources contained in SXG responses. Implicitly injecting a nosniff requirement seems like it would indeed be simpler for developers, so I'd be on board with that kind of mechanism. It seems like requiring developers to include the header has a higher chance of increasing usage even in browsers that don't support SXG (as presumably developers would inject the header themselves on both the SXG-encoded resource, and the plain ol' resource), but that might be asking too much.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Dec 19, 2018
To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Dec 19, 2018
To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Dec 19, 2018
To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Dec 19, 2018
To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Dec 19, 2018
To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
Reviewed-on: https://chromium-review.googlesource.com/c/1373430
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617780}
aarongable pushed a commit to chromium/chromium that referenced this pull request Dec 19, 2018
To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
Reviewed-on: https://chromium-review.googlesource.com/c/1373430
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617780}
foolip pushed a commit to web-platform-tests/wpt that referenced this pull request Dec 19, 2018
To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
Reviewed-on: https://chromium-review.googlesource.com/c/1373430
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617780}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Dec 19, 2018
To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
Reviewed-on: https://chromium-review.googlesource.com/c/1373430
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617780}
@jyasskin
Copy link
Member Author

I'm going to revert the discussion of X-Content-Type-Options:nosniff in this PR, to land the general anti-sniffing advice, and then open a new PR that deals specifically with that header, both for the SXG itself and for its contents.

@jyasskin jyasskin merged commit f030d43 into WICG:master Dec 19, 2018
@jyasskin jyasskin deleted the anti-content-sniffing branch December 19, 2018 22:43
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 5, 2019
… response headers, a=testonly

Automatic update from web-platform-tests
SignedExchange: Require nosniff in outer response headers

To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
Reviewed-on: https://chromium-review.googlesource.com/c/1373430
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617780}

--

wpt-commits: 675ade14e7e8db49c13da4d4a8684568cedb10d7
wpt-pr: 14522
mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Feb 6, 2019
… response headers, a=testonly

Automatic update from web-platform-tests
SignedExchange: Require nosniff in outer response headers

To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
Reviewed-on: https://chromium-review.googlesource.com/c/1373430
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617780}

--

wpt-commits: 675ade14e7e8db49c13da4d4a8684568cedb10d7
wpt-pr: 14522
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 7, 2019
… response headers, a=testonly

Automatic update from web-platform-tests
SignedExchange: Require nosniff in outer response headers

To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
Reviewed-on: https://chromium-review.googlesource.com/c/1373430
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617780}

--

wpt-commits: 675ade14e7e8db49c13da4d4a8684568cedb10d7
wpt-pr: 14522
mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Feb 8, 2019
… response headers, a=testonly

Automatic update from web-platform-tests
SignedExchange: Require nosniff in outer response headers

To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
Reviewed-on: https://chromium-review.googlesource.com/c/1373430
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617780}

--

wpt-commits: 675ade14e7e8db49c13da4d4a8684568cedb10d7
wpt-pr: 14522
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
… response headers, a=testonly

Automatic update from web-platform-tests
SignedExchange: Require nosniff in outer response headers

To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
Reviewed-on: https://chromium-review.googlesource.com/c/1373430
Commit-Queue: Kouhei Ueno <kouheichromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamotochromium.org>
Reviewed-by: Kinuko Yasuda <kinukochromium.org>
Cr-Commit-Position: refs/heads/master{#617780}

--

wpt-commits: 675ade14e7e8db49c13da4d4a8684568cedb10d7
wpt-pr: 14522

UltraBlame original commit: 8e0c10dc58255a9a29577813535a805fcf1999c0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
… response headers, a=testonly

Automatic update from web-platform-tests
SignedExchange: Require nosniff in outer response headers

To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
Reviewed-on: https://chromium-review.googlesource.com/c/1373430
Commit-Queue: Kouhei Ueno <kouheichromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamotochromium.org>
Reviewed-by: Kinuko Yasuda <kinukochromium.org>
Cr-Commit-Position: refs/heads/master{#617780}

--

wpt-commits: 675ade14e7e8db49c13da4d4a8684568cedb10d7
wpt-pr: 14522

UltraBlame original commit: 768b0b422609bacdc7b00036185329a90b2fdaba
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
… response headers, a=testonly

Automatic update from web-platform-tests
SignedExchange: Require nosniff in outer response headers

To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
Reviewed-on: https://chromium-review.googlesource.com/c/1373430
Commit-Queue: Kouhei Ueno <kouheichromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamotochromium.org>
Reviewed-by: Kinuko Yasuda <kinukochromium.org>
Cr-Commit-Position: refs/heads/master{#617780}

--

wpt-commits: 675ade14e7e8db49c13da4d4a8684568cedb10d7
wpt-pr: 14522

UltraBlame original commit: 8e0c10dc58255a9a29577813535a805fcf1999c0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
… response headers, a=testonly

Automatic update from web-platform-tests
SignedExchange: Require nosniff in outer response headers

To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
Reviewed-on: https://chromium-review.googlesource.com/c/1373430
Commit-Queue: Kouhei Ueno <kouheichromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamotochromium.org>
Reviewed-by: Kinuko Yasuda <kinukochromium.org>
Cr-Commit-Position: refs/heads/master{#617780}

--

wpt-commits: 675ade14e7e8db49c13da4d4a8684568cedb10d7
wpt-pr: 14522

UltraBlame original commit: 768b0b422609bacdc7b00036185329a90b2fdaba
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 4, 2019
… response headers, a=testonly

Automatic update from web-platform-tests
SignedExchange: Require nosniff in outer response headers

To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
Reviewed-on: https://chromium-review.googlesource.com/c/1373430
Commit-Queue: Kouhei Ueno <kouheichromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamotochromium.org>
Reviewed-by: Kinuko Yasuda <kinukochromium.org>
Cr-Commit-Position: refs/heads/master{#617780}

--

wpt-commits: 675ade14e7e8db49c13da4d4a8684568cedb10d7
wpt-pr: 14522

UltraBlame original commit: 8e0c10dc58255a9a29577813535a805fcf1999c0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 4, 2019
… response headers, a=testonly

Automatic update from web-platform-tests
SignedExchange: Require nosniff in outer response headers

To encourage servers to include the nosniff header, this CL makes
Chromium reject SXG served without the "X-Content-Type-Options: nosniff"
header.

Bug: WICG/webpackage#348, 916362
Change-Id: I5343a8d13a42a3c9144f05d871777d35a20a77b7
Reviewed-on: https://chromium-review.googlesource.com/c/1373430
Commit-Queue: Kouhei Ueno <kouheichromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamotochromium.org>
Reviewed-by: Kinuko Yasuda <kinukochromium.org>
Cr-Commit-Position: refs/heads/master{#617780}

--

wpt-commits: 675ade14e7e8db49c13da4d4a8684568cedb10d7
wpt-pr: 14522

UltraBlame original commit: 768b0b422609bacdc7b00036185329a90b2fdaba
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.

Reflecting the URL leads to content sniffing bugs
5 participants