golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.4k stars 17.71k forks source link

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

Open FiloSottile opened 6 years ago

FiloSottile commented 6 years ago

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

bradfitz commented 6 years ago

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

pciet commented 6 years ago

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 commented 6 years ago

@pciet what is your proposed alternative?

pciet commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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.

bradfitz commented 6 years ago

@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.

gopherbot commented 6 years ago

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

gopherbot commented 6 years ago

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

gopherbot commented 6 years ago

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

FiloSottile commented 6 years ago

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 commented 6 years ago

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

FiloSottile commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 Githubissues.

  • Githubissues is a development platform for aggregating issues.