Missing "origin" in allowedHeaders

[ghost]

There is missing "origin" in allowedHeaders, docs says that it should be in connection.headers.

[alexander-akait]

/cc @brycekahle friendly ping

[egasimus]

@evilebottnawi @brycekahle just confirming: this causes breakage in [email protected]

[alexander-akait]

@egasimus yep

[egasimus]

@majek ?

[brycekahle]

AFAICT this is not a behavior change in sockjs-node from v0.3.19.

sockjs-node/src/transport.coffee

Line 133 in 14a5c3c

for key in ['referer', 'x-client-ip', 'x-forwarded-for', \

[alexander-akait]

@brycekahle please clarify

[brycekahle]

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

[carlosgeos]

@brycekahle you are right. There was a change in webpack-dev-server, specifically in webpack/webpack-dev-server#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 😛 a71a1e5

The commit comment says 'it is meaningless in SockJS context' so I'll look more into this

[brycekahle]

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.

[alexander-akait]

@brycekahle It was used without iframe. So it was not merged and fixed?

[brycekahle]

It was used without iframe. So it was not merged and fixed?

I don't understand. Can you rephrase?

[alexander-akait]

@brycekahle why we can't merge this PR?

[brycekahle]

@brycekahle why we can't merge this PR?

For the same reason that #247 (comment) states. Origin is meaningless, if SockJS uses an iframe middleman.

[ghost]

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

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

[alexander-akait]

@brycekahle maybe you better understand problem trying [email protected], now we use this workaround webpack/webpack-dev-server@1dfd4fb, but it is very hacky

[brycekahle]

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

[alexander-akait]

@brycekahle maybe we can add option for this?

[brycekahle]

@brycekahle maybe we can add option for this?

This is what I'm suggesting in #247 (comment) 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?

[alexander-akait]

@brycekahle can you clarify? We just need origin header 😄

[brycekahle]

can you clarify? We just need origin header

Yes, but can webpack-dev-server operate without CORS?

[alexander-akait]

@brycekahle yes, you can setup webpack-dev-server almost as you want

[mkArtakMSFT]

Hi folks. Why isn't this PR merged? It causes so much pain on the dependent frameworks. Specifically: dotnet/aspnetcore#7812 and others too.

[brycekahle]

@mkArtakMSFT Can you explain how having origin in the allowed headers would solve your issue?

[mkArtakMSFT]

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.

[brycekahle]

@mkArtakMSFT When you say "dev server", do you mean webpack-dev-server? or something else?

Without the origin header 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.

[rodrifmed]

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

[rodrifmed]

@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, referer and x-forwarded-for (and friends). We explicitly do not grant access to cookie header, 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