sockjs
Missing "origin" in allowedHeaders
There is missing "origin" in allowedHeaders, docs says that it should be in connection.headers.
AFAICT this is not a behavior change in sockjs-node from v0.3.19.
https://github.com/sockjs/sockjs-node/blob/14a5c3cf097126bde74436d001816c53a0593d43/src/transport.coffee#L133
@evilebottnawi Despite the docs indicating origin would be available, it appears that is has never been available (AFAICT). If the absense of origin is breaking webpack-dev-server, that is because of a change in that module, not sockjs-node.
@brycekahle you are right. There was a change in webpack-dev-server, specifically in https://github.com/webpack/webpack-dev-server/pull/1603, in order to try and check the origin header to solve this security vulnerability: https://www.npmjs.com/advisories/725.
However, sockjs-node did provide an 'origin' header in the decorateConnection method until Feb 2012 :stuck_out_tongue: https://github.com/sockjs/sockjs-node/commit/a71a1e506e772371efe2a1570bc93c69a89bf190
The commit comment says 'it is meaningless in SockJS context' so I'll look more into this
The commit comment says 'it is meaningless in SockJS context' so I'll look more into this
This is likely due to the fact that SockJS will utilize iframes to get around same origin restrictions, so the origin of the iframe will not match the origin of the parent document. See https://github.com/sockjs/sockjs-node#authorisation for more information.
@brycekahle It was used without iframe. So it was not merged and fixed?
It was used without
iframe. So it was not merged and fixed?
I don't understand. Can you rephrase?
@brycekahle why we can't merge this PR?
For the same reason that https://github.com/sockjs/sockjs-node/pull/247#issuecomment-451715393 states. Origin is meaningless, if SockJS uses an iframe middleman.
@brycekahle but it can still be optional to use maybe? Today iframe transport is in most cases useless, but let's make origin header present only by setting an option parameter
@brycekahle but it can still be optional to use maybe? Today iframe transports are in most cases useless, but let's make origin header present by setting an option parameter
I think the only secure way would be if the server knew iframe support was disabled. The disable_cors option does accomplish that, but may be too much if you still need CORS support for other transports.
@brycekahle maybe you better understand problem trying [email protected], now we use this workaround https://github.com/webpack/webpack-dev-server/commit/1dfd4fb, but it is very hacky
@evilebottnawi I understand the problem well enough, but I cannot make a change that doesn't work for all library users in a secure way.
@brycekahle maybe we can add option for this?
This is what I'm suggesting in https://github.com/sockjs/sockjs-node/pull/247#issuecomment-452929348 but I need more input on what webpack-dev-server needs. Is turning off CORS completely sufficient? Or does it need to be more selective?
can you clarify? We just need
originheader
Yes, but can webpack-dev-server operate without CORS?
@brycekahle yes, you can setup webpack-dev-server almost as you want
Hi folks. Why isn't this PR merged? It causes so much pain on the dependent frameworks. Specifically: https://github.com/aspnet/AspNetCore/issues/7812 and others too.
@mkArtakMSFT Can you explain how having origin in the allowed headers would solve your issue?
I think this comment says it all: https://github.com/webpack/webpack-dev-server/pull/1608/files#diff-15fb51940da53816af13330d8ce69b4eR47
What's happening in ASP.NET Core case, we start the dev server as a seprate process and proxy all the requests to it. Without the origin header browser can't distinguish between having a direct connection to the SPA's dev server versus a reverse-proxy.
@mkArtakMSFT When you say "dev server", do you mean webpack-dev-server? or something else?
Without the
originheader browser can't distinguish
This PR would not affect anything regarding the browser's ability to read the Origin header. This PR is for the node.js server.
Please also read the rest of this thread. There are security concerns regarding the Origin header, specifically with iframe-based transports. I cannot simply allow the Origin header for everyone, because it would not be accurate in all cases. webpack-dev-server was able to use their patch because of the constraints around how that is used. Those constraints do not apply to everyone using SockJS.
Hi @brycekahle , I'm facing a similar problem with not having the origin in the header.
My problem is to connect to the websocket we have a list of allowed origins, for security reasons.
Without know the origin and check if it is part of the allowed origin list, everyone can connect to the websocket.
Do you have a solution for this problem? I'm trying to avoid apply the workaround mentioned
@brycekahle The documentation is wrong as well, it says:
Property: headers (object) Hash containing various headers copied from last receiving request on that connection. Exposed headers include:
origin,refererandx-forwarded-for(and friends). We explicitly do not grant access tocookieheader, as using it may easily lead to security issues (for details read the section "Authorisation").
Source: https://github.com/sockjs/sockjs-node#connection-instance
But the origin is not included