-
Notifications
You must be signed in to change notification settings - Fork 13
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
Support for custom headers for handshake #16
Comments
We'd need implementer interest in this before moving forward, similar to whatwg/html#2177. |
Mozilla - https://support.mozilla.org/en-US/questions/1176810 @domenic I'll add feature request for other browsers soon. Hopefully some of them will show some interest. |
Thank you Misiu for filing the feature request and the bugs for each vendor. Regarding complexity, for the web platform, this would require non-trivial amount of work at both spec and impl side. To introduce custom header feature, we need to make it issue a CORS preflight. Though this is not an HTTP API, considering the original reason why we introduced the CORS preflight, we need it for WebSocket + custom headers. On Chromium, we have separate stacks for WebSocket and XHR/fetch() (for which we have CORS logic). Some non-trivial refactoring and gluing is needed. |
I am one of the authors and maintainers of Chrome's WebSocket implementation. I oppose this proposal. Reasons:
|
@tyoshino @ricea thank You guys for Your reply. If You look at SignalR repo You will notice that many people are looking for a way to pass authentication via WebSocket - aspnet/SignalR#888 (comment) Right now the only option to do this is to add token as part of query string. |
@Misiu In the specific case of WebSockets I think passing an authentication token in the URL is okay. The reason is that, unlike HTTP URLs, wss: URLs are never exposed to the user. They can't bookmark them or copy-and-paste them. This minimises the risk of accidental sharing. In addition, their appearance in other web APIs is minimal [1]. For example, they won't appear in history. This reduces the risk of leakage via JS APIs. The best thing from a security perspective would probably be to perform authentication after the WebSocket is connected, but I realise it is undesirable from a resource-usage perspective to permit unauthorised connections to be established in the first place. Given the portability difficulties with using cookies or http auth, a short-lived authentication token in the URL is probably your best option at the moment. A specific note with respect to the [1] I actually can't think of any other APIs that expose ws: or wss: URLs at all. Certainly they don't appear in resource timing. |
I cannot use WebSocketSharp because it's missing the ability to customize authorization headers. I am trying to integrate with a WebSocket api that takes in OAuth2 bearer tokens. |
@ricea That's not the main reason why you don't put authentication values in URLs. It's because of server logs. Auth tokens should be treated with the same security concern as username and password combos. You don't put username and password combinations in urls, so neither should auth tokens. If you have a vulnerability and a user downloads your http logs, they then have access to virtually all of your accounts. |
@ricea sadly implementers don't get to make the decision of what is required to use an api. I am currently trying to use an api that REQUIRES oauth2 bearer tokens in the authorization header for server-to-server communication. They offer cookies for browser based security, but I do not have that luxury because I have no sessionids for the user. So all you're doing by saying "we should not do this" is saying "you have to look elsewhere to implement with this api". Which would simply just drive people away from using this implementation. You aren't "saving" anyone but not allowing this, just alienating users. |
I encountered this issue looking for solution on my problem - how to remove OWASP Zap report is suggesting sid param removal from URL, also this question on stackoverflow is refering that CloudFlare is also suggesting sid param removal from URL: https://stackoverflow.com/questions/42759556/how-to-remove-socket-io-sid-parameter-from-url So my question is, @ricea, do you think that this reported issue is completely irrelevant because of fact that ws/wss urls are never exposed to users? |
Security measures need to be considered in terms of what threats they are intended to protect against and to what extent they are effective in mitigating those threats. In the case of the Cloudflare vulnerability, placing the Quoting https://bugs.chromium.org/p/project-zero/issues/detail?id=1139:
This is the trouble with checklist security. If you removed the @digitalpacman If someone has access to your server logs, they can do much worse things than just steal credentials. There's usually all kinds of personally-identifiable information in there. Access needs to be tightly restricted just as with any other kind of user data. |
@ricea there is only personally-identifiable information in there if you aren't doing good practices. At most there should be an IP address, but you can opt to not record that either. Not putting SECURITY CREDENTIALS in the url is an extremely popular best practice. Infact, governing bodies for security will instantly fail you if you are passing them in the URL. Please look at the OAuth2 spec for example and explanation of these vulnerabilities. There is a reason it REQUIRES them in the POST body. |
@ricea is this perhaps something we can reconsider if https://wicg.github.io/origin-policy/ becomes a thing? That should nullify most of the CORS preflight cost. The reason I'm somewhat sympathetic to the requests is because I learned that middleboxes basically ruin full duplex HTTP so WebSocket will likely stick around. |
@annevk I think origin policy will probably be implemented in the browser in the same place as CORS, so it would still mean re-wiring the handshake to go down the same code path. The WebSocket handshake started out as purely a mechanism to negotiate a WebSocket connection. Over time we've added HTTP features like cookies and authentication, but we've paid a high price in implementation complexity. The "right" way to do WebSocket authentication is to do it at the application layer after the handshake completes. No-one asks how to put an oauth token in a TCP/IP SYN packet. But the reality is that we've ended up in a fuzzy middle-ground that is hard to explain. |
@ricea given that we do add browser-supplied cookies and authentication, it's quite a reasonable request that we also enable headers, since much WebSocket server-side infrastructure probably depends on such information being in the handshake. And at least as far as the specification is concerned the handshake shares many aspects with Fetch, to enable mixed content blocking, HSTS, CSP, etc. So from that perspective this is not that much of a stretch. (I wonder if Chrome's approach is shared by other browsers.) |
Can you elaborate here. I don't understand why the security model hinges on not allowing headers to be set in the client API? I'm not sure I understand this. |
@davidfowl part of the same-origin policy is that we don't send "attacker-controlled" headers to cross-origin URLs. That's why if you want to do that you need to use CORS, which uses a CORS preflight to make an explicit check that the cross-origin URL is okay with the headers about to be transmitted. The objection here, at least from the Chrome team, is that supporting a CORS preflight for WebSocket is too costly. |
@annevk Thanks for that clarity! So attacker controlled headers are seen as more dangerous than attacker controlled query string. Would it help if we restricted the types of headers that could be sent? |
Yes, you can basically reach any arbitrary URL (whatever the query string or path), but you can only control headers to a very limited extent (and methods too, only HEAD/GET/POST). The request headers we allow "attackers" to control are listed at https://fetch.spec.whatwg.org/#cors-safelisted-request-header, but note that we plan to lock that down some more in whatwg/fetch#736. For the kinds of use cases that people seem to have I don't think allowing these would be sufficient (or it would lead to hacks where you put authorization data in |
@annevk The current design forces people to send what would typically go in the the Authorization header in the query string. Is that any less secure than allowing say the authorization header to bet set on the upgrade request? (even cross origin) |
The concern is not with the security of the application making the request, it's with the security of the remote server. The remote server will have to be robust against arbitrary URLs, but it does not have to be robust against arbitrary headers, since it'll assume (due to the long history of the web and early establishment of the same-origin policy) that those cannot come from browsers. |
I see, understand the push back now. So there needs to be a pre-flight request integrated into the flow and that's expensive in chrome because of the current implementation split between websockets and xhr/fetch. Is that a fair summary? |
Excellent summary. |
Should we get some of the other browser implementors to chime in? I can probably get somebody from the edge team to chime in here about the difficulty there. I'm not sure how to go about getting the other browser vendors interested enough to look at this issue (safari, firefox, anything else?) |
I'm kinda surprised people aren't smuggling this through subprotocol - seems obvious enough. architecturally speaking, a CORS preflight wouldn't be a big burden for firefox to implement. otoh, designs that result in a lot of preflights suck so I'm not convinced we want to enable this rather than pushing it into the post websocket-handshake data. |
Then its a per web app custom protocol implementation; open to a slow loris type, slow auth issue from an unknown user; depending on how the application developer implements it; vs a well understood header handling implementation by the webserver? |
Why would any WebSocket server support that today though? |
It's common for HTTP servers to have WebSocket functionality, so it's not outrageous to think they might already support preflights. If there are environments where it would actually work, that would be an argument in favour of supporting custom headers. Conversely, if none of the people who want custom headers are actually going to be able to use it, it's not worth spending time on. |
That presumes that if browsers add support for WebSocket + CORS, WebSocket servers will not want to update. I don't see how that is a reasonable assumption. |
Preflight would be on the HTTP(s) UPGRADE request; the first request of a WebSocket is regular HTTP (and so same origin rules could still apply, or different origin for CORS) |
I don't see why it's necessary to find WebSocket servers that currently support CORS preflights. It's quite easy to write a websocket server that adds CORS preflight logic: g := gin.Default()
g.Use(func(c *gin.Context) {
c.Writer.Header().Set("Access-Control-Allow-Origin", "*")
c.Writer.Header().Set("Access-Control-Allow-Credentials", "true")
c.Writer.Header().Set("Access-Control-Allow-Headers", "Authorization, Upgrade")
c.Writer.Header().Set("Access-Control-Allow-Methods", "OPTIONS, GET")
if c.Request.Method == "OPTIONS" {
c.AbortWithStatus(204)
} else {
c.Next()
}
})
g.GET("/ws", func(c *gin.Context){
conn, _ := websocket.Upgrader{
ReadBufferSize: 1024,
WriteBufferSize: 1024,
}.Upgrade(c.Writer, c.Request, nil)
conn.SetPingHandler(
// Handle ping
)
// Other business logic here
}) |
A weird thing I noticed is that you could have a preflight over HTTP/1.1 and then have the real WebSocket connection go over HTTP/2. HTTP/2 uses I think the solution is for servers to send
even over HTTP/1.1 where it makes no sense, just in case the real connection happens to be on a different version of HTTP. Should user agents enforce this? Or should they assume that the server probably intended to allow |
Not everyone has the ability to modify their WebSocket server implementation. Even if they request an update from the vendor, there may be a very long time before it is usable in production. |
That's an interesting case. What would |
|
Yeah that seems reasonable. And maybe we wouldn't output |
A quick note to people that encounter this thread that token smuggling in the I reiterate the multiple voices here that said that adding a token in the URL goes completely against best practices (mainly because of logs, which include not only the servers in the service but ingresses and proxy services). It feels like by being strict about the risks in the handshake ( #16 (comment) ) might be leading to thousands of people engaging in less safe practices, resulting in a net negative outcome for security of web services. Not allowing custom headers is especially hard for me to to understand given that the RFC6455 clearly mentions additional headers, and given how many other frameworks already allow it. |
@sergioisidoro thanks for that note! FWIW, I'm looking at this right now, and the So if request has:
and the response comes back with
Chrome accepts the connection because |
I would suggest to spec the alternate constructor signature from #16 (comment), e.g. similar treatment than new WebSocket(url, protocols)
new WebSocket(url, {headers, protocols}) |
I've got an alternate way to address this, by reusing much of the Readable streams would gain an extra argument to
To set up a WebSocket, you'd do
I looked in the spec, and much of the infrastructure appears to already exist. So this might be the path of least resistance. |
This is similar to the [WebSocketStream API] Although it is attractive to re-use the |
One benefit of this is that it makes it possible to receive info from failed WebSocket handshake requests. If the
Allowing passing |
@ricea That's fair. And I myself have some general ergonomic hesitations with this API, and almost stated that in the initial comment posting it. (The
@randomstuff That was part of my motivation for this, actually. It offers a standard API that can expose that info. |
Hi,
please consider adding ability to add custom headers for handshake.
In RFC6455 there one interesting point:
I've found an example how to add custom header to handshake: https://blog.heckel.xyz/2014/10/30/http-basic-auth-for-websocket-connections-with-undertow/ but this is for Java and unfortunately isn't possible in HTML5.
When searching over the net I found many places question about this option, for example:
sta/websocket-sharp#22
https://stackoverflow.com/questions/4361173/http-headers-in-websockets-client-api/4361358#4361358
aspnet/SignalR#888
For example in Python this is possible https://stackoverflow.com/questions/15381414/sending-custom-headers-in-websocket-handshake. Other languages also support this. Last place missing is the browser.
Please consider adding this into specification. Having this even as a draft would allow us to consider browser vendors to add support for it.
If this is incorrect place for adding request about specification please forgive me and please point me to right place.
The text was updated successfully, but these errors were encountered: