Skip to content
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

WebSocket Origin restrictions #20

Closed
joewalnes opened this issue Dec 15, 2013 · 13 comments
Closed

WebSocket Origin restrictions #20

joewalnes opened this issue Dec 15, 2013 · 13 comments
Labels

Comments

@joewalnes
Copy link
Owner

Background

WebSockets are cross domain by default. e.g. A page on http://foo can open a WebSocket on ws://bar.

User defined WebSocket endpoints can see the originating origin through the Origin HTTP header (exposed as HTTP_ORIGIN environment variable to scripts). If they wanted to restrict to certain origins, they could check at the beginning of the script and exit appropriately.

Problem

The above works, but:

  • It's tedious. If there are many scripts it would have to be applied to each of them.
  • Security should be "on" by default.
  • Deployment configuration (eg hostnames) gets buried in runtime code.

Proposed solution

websocketd should perform origin checking and reject mismatches before invoking user scripts. Configuration via command line flags.

Examples:

  • Default: restrict to same origin only.
  • "--origin=foo,bar:1234": only allow if origin is from http://foo (any port) or http://bar:1234
  • "--anyorigin": Allow for any origin
@asbjornenge
Copy link

I'm having a possibly related/inverted issue. I want to connect via a nodejs websocket. When trying to connect directly to the IP websocketd throws a 403 back at me. If I however use the a proper hostname I can connect.

My problem is that I need to be able to connect using the IP. So ideally I need the --anyorigin...?

@asergeyev
Copy link
Collaborator

Default needs explanation.

  1. Should be same hostname but any port in most cases? If I said --address=1.2.3.4 and I see Origin: hostname.com; should I resolve hostname.com and see if 1.2.3.4 is in the list of addresses I can fetch? Looks slightly expensive to do this on all requests but manageable.
  2. What if Origin is "file://" ? Should websocketd be checking if localhost is in allowed origin list?
  3. What if websocket listens on "", "0.0.0.0" or "::" .. should we allow any origin? I think that in this case "security is ON by default" gets harder to implement.

@joewalnes
Copy link
Owner Author

  1. No resolution should occur. If user wants to allow from origin by IP address and hostname they should specify multiple options.
  2. I think file should be a special case. By default not, but we can add a --fileorigin.
  3. For the same origin case, rather than comparing the Origin header to the server listen, we should be comparing it to the sent Host header (i.e. what the browser thinks its talking to). The websocket bind address is not used.

@asergeyev
Copy link
Collaborator

How about --origin=* instead of anyorigin? Trying to minimize number of flags would be good at this point :)

@asbjornenge
Copy link

👍 for --origin=*

@joewalnes
Copy link
Owner Author

Thing about * is that most shells would attempt to wildcard complete. You'd
need to do --origin=* which looks messy.

On Thursday, April 3, 2014, Alex Sergeyev notifications@github.com wrote:

How about --origin=* instead of anyorigin? Trying to minimize number of
flags would be good at this point :)

Reply to this email directly or view it on GitHubhttps://github.com//issues/20#issuecomment-39446966
.

@asbjornenge
Copy link

--origin=any or all ?

@asergeyev
Copy link
Collaborator

See, "any" could actually represent a hostname for some fancy people.

As an option to avoid shell wildcard it could be then --origin=- (minus) or --origin=. (dot) but it might be that --anyorigin is what we need to settle with... (just going to be another set of conflicting flags as --devconsole with cgidir/staticdir)

@asbjornenge
Copy link

I would actually like to see any be the default. And if users want to specify, pass the --origin flag.

@joewalnes
Copy link
Owner Author

I can get behind that. It's convenient. In that case we'd need to add a
--sameorigin flag for the currently proposed default behavior.

On Thu, Apr 3, 2014 at 9:52 AM, asbjornenge notifications@github.comwrote:

I would actually like to see any be the default. And if users want to
specify, pass the --origin flag.

Reply to this email directly or view it on GitHubhttps://github.com//issues/20#issuecomment-39460608
.

@asergeyev
Copy link
Collaborator

Ok... usecases:

  1. "--sameorigin" only: allow upgrade from any origin that matches currently requested Host header
  2. "--origin foo,bar:12345": allow upgrade only when origin is foo:* or bar:12345
  3. "--origin foo,bar:12345 --sameorigin": alow upgrade only when origin is foo:* or bar:12345 but only when it matches Host header
  4. none of above - allow any origin

would that be correct?

@asergeyev
Copy link
Collaborator

Also question... Should origin directives cause fatal failure if neither command nor --dir are specified?

@asergeyev
Copy link
Collaborator

asergeyev/websocketd@master...originchecks
My vision of origin checks. It required slight refactor of things to prevent adding more and more arguments to acceptWebsocket and createEnv.

IMO this will eventually lead to create proper "request" type that implements http.Request stuff, URLInfo, ... and defines libwebsocketd specific things. But for now there is just requestInfo handicap struct. This does not look bad, just probably needs to be armored by better comments.

Comments welcome. I added http_test to start actually checking things not by running examples but by verifying logic :) Did not get far there though :(

asergeyev added a commit that referenced this issue May 16, 2014
Originchecks per #20, closes #58 too...

--origin and --sameorigin flags added, code refactored to separate some logical parts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants