Same origin policy for websockets breaks JS in external websites

[esseks]

6c46d1f by @thypon removes same origin check for /ws endpoint.

The same origin policy prevents external websites from connecting to /ws. Non-browser code is not affected. Removing such a check has both pros and cons:

pros: malicious JS from external websites cannot forge requests. cons: external websites cannot access /ws for the legitimate purpose of displaying status updates.

At the moment the channel is read-only, so malicious JS would have no target, apart from a creative and not-that-effective DDoS via XSS. Anyway, punching a hole in security might result in a future issue if this is not taken into consideration in the future.

Currently, there are no in-browser JS applications using /ws, but there might be in the future.

Do we keep this?

[thypon]

Some Background:

• During an undocumented update we started using 4.0 tornado in production.
• My "update" is a two line revert to have 3.0 tornado behaviour.

The discussion could be: it's needed to break API compatibility for an unforseen future where other informations pass on that channel?

[esseks]

Tornado 4.0 is the current stable branch, Tornado 3.0 is no more supported AFAIK Our policy is to track the latest stable release and that's what happened. I acknowledge that the README was not up to date, but you were welcome to bring it on and/or report that as a bug as well. On the contrary, setup.py install script contains no hard reference to Tornado 3.0. But anyway that's not the point.

The fact that SOP was not enforced prior to the update was a bug and the current behaviour is what I expect. Currently we don't break any application I am aware of, but I understand that accessing /ws from other hosts could have a legitimate use.

[thypon]

Not enforcing SOP (or server side enforcing) was in the design document and was one of the reason why we choose Websocket instead of a most common long lived HTTP Request or an HTTP chunked request. Changing that behaviour because "we are using an updated library that claims to be secure" is unacceptable (for me).

Now explain, why not using HTTP chunked removing Websocket; it's more secure, It's supported in IE>7. What's the drawback (apart going against design document without discussion)?

What you propose is not about security: it's about a semantic bug, introduced by using a new library major release w/o doing regression tests, and a future security enchancement. What i fixed was a bug which we don't have to discuss on.

If we want to discuss about security enchancement we open a new issue about the probably added security enforcing SOP. Meanwhile we have to close this semantic bug, giving the opportunity to clients to use the no SOP feature.

I'll repeat, this is a bugfix

[thypon]

I removed Security tag because discussing about security is discussing about protecting something from someone. What you propose is protecting null from someone. With the actual pushing protocol there is nothing to protect.

[esseks]

All in all I guess there is no downside in having that change now. I deployed it to the server. Closing.