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

net/http: automatically add X-Content-Type-Options: nosniff #24513

Open
FiloSottile opened this issue Mar 23, 2018 · 25 comments
Open

net/http: automatically add X-Content-Type-Options: nosniff #24513

FiloSottile opened this issue Mar 23, 2018 · 25 comments

Comments

@FiloSottile
Copy link
Contributor

Some old browsers have an unfortunate mis-feature where they will detect and interpret HTML in responses that have different Content-Type. This is commonly referred to as content sniffing, and is a security risk when attacker-controlled content is served, leading to XSS.

The proposal is to automatically include the X-Content-Type-Options: nosniff header when Content-Type is one of the values known to trigger content sniffing: text/plain, application/octet-stream, application/unknown, unknown/unknown, */* or an invalid value without a slash.

It doesn't solve all issues, for example plugins are still a problem, and even older browsers don't support it, but it's almost free and mitigates the issue in IE8, the most used affected browser (according to https://caniuse.com/usage-table, https://blogs.msdn.microsoft.com/ie/2008/07/02/ie8-security-part-v-comprehensive-protection/).

There should probably be a way to disable it for users that care about the header overhead and are unconcerned by content sniffing. Maybe setting it to an empty string? Is there precedent?

See also #13150

/cc @bradfitz @andybons @rsc

@gopherbot gopherbot added this to the Proposal milestone Mar 23, 2018
@bradfitz
Copy link
Contributor

We have precedent: setting the header value to a nil slice. e.g. that's how users disable the implicit Date response header.

@pciet
Copy link
Contributor

pciet commented Mar 24, 2018

Not everybody is using web browsers with net/http, and even with just web browsers I’m seeing on Wikipedia that X-Content-Type-Options maybe only applies to Internet Explorer and Chrome.

Including secure application patterns makes sense to me (there’s a lot to learn in the http+browser history), but I think they should be optional to provide for other applications where maximized performance and elegant code is wanted. Toggling a string write to nil in every request is unfortunate at least for elegance.

@andybons
Copy link
Member

@pciet what is your proposed alternative?

@pciet
Copy link
Contributor

pciet commented Mar 27, 2018

@andybons I’d add net/http/browser or x/net/http/browser package.

// Package browser provides http functionality for serving to web browsers.
package browser

import “net/http”

// A function called in an http handler.
func SecureResponse(r *http.Request, w http.ResponseWriter) error {
    // adds X-Content-Type-Options if needed based on user-agent

Or I'd point people to a community solution like https://github.com/unrolled/secure

Although maybe this kind of function could just be in net/http. "func SecureBrowserResponse(..."

@FiloSottile
Copy link
Contributor Author

@pciet We aim to provide security by default (users that don't know to add X-Content-Type-Options wouldn't know to look for a function adding it either) and let those who know what they are doing opt out of it if they need.

User Agent sniffing might be useful here, it is usually unwieldy to maintain, but here we are only concerned about some specific old browsers, those won't change.

I'm +1 on mirroring how Date works.

@pciet
Copy link
Contributor

pciet commented Mar 27, 2018

@FiloSottile If always having this avoids a few security problems then I think it's worth it, at least until a net/http redesign happens at which point I’d propose my suggestion of separating http use cases again.

@bradfitz
Copy link
Contributor

bradfitz commented Apr 2, 2018

As long as we only do this for those weirdo/rare content-types, I'm cool with this.

I would push back if you'd proposed doing this unconditionally.

@rsc
Copy link
Contributor

rsc commented Apr 2, 2018

OK, let's do this, for the content-types listed above, with an explicit nil in the map as meaning "don't add one". Note that "text/plain; charset=utf-8" should also trigger this.

@rsc rsc modified the milestones: Proposal, Go1.11 Apr 2, 2018
@rsc rsc changed the title proposal: net/http: automatically add X-Content-Type-Options: nosniff net/http: automatically add X-Content-Type-Options: nosniff Apr 2, 2018
@bradfitz
Copy link
Contributor

bradfitz commented May 4, 2018

@FiloSottile, if this is going to happen for Go 1.11, it ideally should've already gone in. If you finish it in the next week it's probably cool.

@FiloSottile FiloSottile self-assigned this May 4, 2018
@gopherbot
Copy link

Change https://golang.org/cl/111855 mentions this issue: net/http: add X-Content-Type-Options automatically to prevent sniffing

@gopherbot
Copy link

Change https://golang.org/cl/112015 mentions this issue: http2: add X-Content-Type-Options automatically to prevent sniffing

gopherbot pushed a commit to golang/net that referenced this issue May 9, 2018
When a Content-Type that triggers content sniffing in old (but still in
significant use) browsers is sent, add the
X-Content-Type-Options: nosniff header, unless explicitly disabled.

Expose httpguts.SniffedContentType for use in the HTTP 1 implementation.

Will be tested by net/http.TestNoSniffHeader_h2.

Updates golang/go#24513

Change-Id: Id1ffea867a496393cb52c5a9f45af97d4b2fcf12
Reviewed-on: https://go-review.googlesource.com/112015
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/112256 mentions this issue: vendor, net/http: update x/net for X-Content-Type-Options: nosniff

@FiloSottile
Copy link
Contributor Author

Considering how many spurious test failures we are getting in the standard library, I'm starting to wonder if this is a bad idea without a User-Agent gate, as we will break everyone who is asserting headers in tests. @bradfitz, WDYT?

@andybons
Copy link
Member

andybons commented May 9, 2018

User-Agent detection is notoriously brittle and unreliable. I’d almost rather not have nosniff than add that.

@FiloSottile
Copy link
Contributor Author

Agreed in general, but here we have an unusually static and small target (IE 8 is not going to change its User-Agent) and it fits the threat model of XSS.

@bradfitz
Copy link
Contributor

bradfitz commented May 9, 2018

I agree User-Agent sniffing is gross, but if it's really a super small set of patterns, I'm okay with it. Even if the pattern is /MSIE/ that could be enough.

@FiloSottile
Copy link
Contributor Author

So we would only target "MSIE 8".
https://blogs.msdn.microsoft.com/ie/2009/01/09/the-internet-explorer-8-user-agent-string-updated-edition/

However, I'm now realizing that it will be little use if an attacker can load a resource with a third-party script tag and execute in the context of the target, which would be an issue also with other browsers and with other Content-Types.
https://blog.mozilla.org/security/2016/08/26/mitigating-mime-confusion-attacks-in-firefox/

But I'm out of my depth in terms of historical web platform security. Maybe @ericlaw1979 can help us here?

@ericlaw1979
Copy link

Generally speaking, content sniffing is a problem in ALL browsers, not just legacy IE.

Beyond IE9's changes in CSS handling and broadening respect for nosniff to the <script> element, the change in IE9 was limited in scope-- it simply refused to interpret responses with an explicit Content-Type: text/plain to text/html. The change did not impact scenarios where a different type (or no type at all) was specified.

While sniffing unrecognized/ambiguous types to HTML has been tightened across browsers over the years, relatively little has been done for other problems with types (particularly scenarios the target can be interpreted as Script or CSS).

There are active vulnerabilities across browsers that enable violation of the same-origin policy and reveal the content of resources that fail to declare nosniff. Sadly, I am not, at present, at liberty to reveal further details. See https://crbug.com/433049 for a higher-level discussion of Chrome's attempt to reduce the scenarios in which sniffing is permitted.

Sending correct MIME types and preventing sniffing to other types is arguably as important as ever in a world of Spectre/Meltdown. See https://textslashplain.com/2018/01/08/content-types-matter-more-than-you-think/ and in particular https://chromium.googlesource.com/chromium/src/+/master/services/network/cross_origin_read_blocking_explainer.md

@FiloSottile
Copy link
Contributor Author

FiloSottile commented May 9, 2018

@ericlaw1979 Ouch, that's way more widespread than I thought.

We can't help with setting appropriate Content-Types, but we can help setting nosniff.

How about setting nosniff every time it's not explicitly disabled and the User-Agent is a browser (look for "Mozilla")? Do you think that would be a reasonable (limited breakage), useful policy? Can we limit it to only some Content-Types or should it be applied for all of them?

@FiloSottile
Copy link
Contributor Author

This turned out to cause more churn and bring less gain than expected. Backing it out and holding to revisit in the future.

@gopherbot
Copy link

Change https://golang.org/cl/117955 mentions this issue: Revert "http2: add X-Content-Type-Options automatically to prevent sniffing"

gopherbot pushed a commit to golang/net that referenced this issue Jun 11, 2018
…iffing"

This reverts commit f73e4c9.

Reason for revert: This turned out to cause more churn and provide less
security than expected.

Updates golang/go#24513

Change-Id: I2c8d0c39f8759ec8895a3261c91a98aeb2303ede
Reviewed-on: https://go-review.googlesource.com/117955
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@FiloSottile FiloSottile modified the milestones: Go1.11, Go1.12 Jun 29, 2018
@mikesamuel
Copy link
Contributor

Let me just restate the reason we in ISE consider this an important long term goal.

  • It is convenient to host files where file uploads are processed
  • Developers often do consider it sufficient to whitelist the mime-type at upload time despite @ericlaw1979's great arguments and well thought out alternatives.
  • Even security-conscious developers forget that, because a string is an image/gif (in the set of strings allowed by the GIF spec) does not mean that it is not also in another language (set of strings)
  • Polyglots are not hard to find in the intersection of two languages.
  • Sniffers are heuristic, so different sniffers (and versions thereof) may assign different mime-types to the same input.

So real systems serve hosted content on sensitive origins with a different mime-type than that with which it was received.

This is not limited to the browser. Any system that grants privilege to files with a particular combination of (origin & mime-type) is potentially vulnerable.

@FiloSottile
Copy link
Contributor Author

@mikesamuel thanks for weighting in.

How do you feel about the conflict between setting nosniff automatically, and using nosniff as a signal to turn off server-side sniffing? (For context, the Go standard library performs Content-Type sniffing on the first 512 bytes of the response when Content-Type is unset. See #24795.)

My concern with server-side sniffing is that it's less context-aware than browser sniffing, and less likely to evolve with the browsers.

@mikesamuel
Copy link
Contributor

@FiloSottile I submitted the original patch to use nosniff as a signal to turn off server-side sniffing. I think it's a good idea since I think it's typically an error when an HTTP response both lacks a Content-type and has a single non-empty body.

Ideally, all header sets for a non-empty body would specify nosniff, but I think we could try to do this on an opt-in basis. I floated Secure Header Sets a while back to try to fill that niche:

A proposed library that provides safe defaults (with opt-out) for security-relevant HTTP response headers.

A variety of headers have been added over the years to address common security problems. Many of these headers were specified as opt-in to avoid breaking the web.

When writing new web applications, web application authors should have a way to opt-out of secure defaults based on application requirements instead of having to opt-in.


A main use case for Content-type sniffing is serving hosted content which is also where polyglot attacks are a major risk.
Since polyglot attacks depend on a string being in two languages, that use case is best served by using a less ambiguous signal, like filetype extension.

My concern with server-side sniffing is that it's less context-aware than browser sniffing, and less likely to evolve with the browsers.

Having more context can be a good thing but not necessarily when that context is attacker controlled. It's precisely the dependence on context to resolve ambiguity that lets gifjs masquerade as a .js file when used in <script src>.

@bradfitz
Copy link
Contributor

bradfitz commented Dec 4, 2018

Too late. Bumping to next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants