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

Demonstrating websocket support? #77

Closed
abl opened this issue Apr 4, 2019 · 15 comments · Fixed by #151
Closed

Demonstrating websocket support? #77

abl opened this issue Apr 4, 2019 · 15 comments · Fixed by #151
Assignees
Labels
bug Something isn't working

Comments

@abl
Copy link

abl commented Apr 4, 2019

Describe the bug

I am unable to confirm that websocket support works in 0.0.3.

To Reproduce

  1. Deployed Pomerium 0.0.3
  2. Tried to connect to otherwise-working proxied apps where websocket-based features failed
  3. Websockets were unable to connect
  4. Added a proxy from https://websocket.corp.my-redacted-domain.com to https://websockets.org
  5. Demo was unable to connect to wss://websocket.corp.my-redacted-domain.com/
    6, Deployed Kaazing gateway at http://kaazing.corp.my-redacted-domain.com:8000
  6. Successfully demonstrated insecure websockets on port 8000
  7. Configured a policy to point at local gateway
  8. Local gateway cannot connect to websocket (this might be a Kaazing issue; it's expecting a particular host value, I think.)

Expected behavior

Websocket support works.

Environment:

  • Pomerium version (retrieve with pomerium --version): v0.0.3+41c42f5
  • Server Operating System/Architecture/Cloud: Synology x86 NAS running docker

Configuration file(s):

policy.yaml:

- from: kaazing.corp.my-redacted-domain.com
  to: http://kaazing.corp.my-redacted-domain.com:8000
  allowed_users:
  - <me>
- from: httpbin.corp.my-redacted-domain.com
  to: https://httpbin.org
  allowed_users:
  - <me>
- from: websocket.corp.my-redacted-domain.com
  to: https://websocket.org/
  allowed_users:
  - <me>

Logs(s):

There appear to be no logs associated with websocket connection attempts.

Additional context

Pomerium is bound directly to port 443.

Happy to add any Pomerium folks who want to try these routes to my allowed users (and share with them the actual URLs involved.) This is not a production system so I'm happy to make sweeping changes.

Connecting to wss://httpbin.corp.my-redacted-domain.com/anything yields an error 200; curiously, nothing is in the logs for this request. POMERIUM_DEBUG is true and LOG_LEVEL is default; l I am seeing DBG logs.

@abl
Copy link
Author

abl commented Apr 4, 2019

Trying to set up codercom/code-server and generated a self-signed certificate. I'm now seeing lots of lines like:

1:25AM ERR http: proxy error: can't switch protocols using non-Hijacker ResponseWriter type *http.timeoutWriter proxy=192.168.1.2:8943

Where 192.168.1.2:8943 is the locally bound port. Non-WSS content works fine.

@desimone
Copy link
Contributor

desimone commented Apr 4, 2019

That you for the report @abl ; sorry this isn't working as expected.

Just to confirm, you were unable to ever connect to a web-socket service behind pomerium but all normal HTTP services ran fine? (e.g. when you say you successfully demonstrated insecure websockets on port 8000, was that by directly connecting without pomerium)?

If you don't mind detailing what docker images you used, I can try testing this on my end.

@desimone
Copy link
Contributor

desimone commented Apr 4, 2019

Ok. I've reproduced the issue. This is a bug.

Quick hunch. Pomerium uses HTTP/2 by default and muxes both gRPC and HTTP on the same port depending on what type of payload it sees coming down the pipe.

What makes this tricky is web-sockets (seem to be?) incompatible with http2. Web-sockets were tested before pomerium used gRPC/HTTP2 on the same port. Hopefully we can still support downgraded http1.1.

@desimone desimone added the bug Something isn't working label Apr 4, 2019
@desimone desimone self-assigned this Apr 4, 2019
@abl
Copy link
Author

abl commented Apr 4, 2019

@desimone thanks for the quick reply! Happy to send my complete configuration over.

Yes, all normal HTTP/S services were fine. Successful insecure websockets were, as you said, by bypassing pomerium and connecting directly.

Tricky sounds right; I hadn't considered HTTP2.

@desimone desimone mentioned this issue Apr 7, 2019
3 tasks
@desimone
Copy link
Contributor

desimone commented May 1, 2019

@fightforlife
Copy link

Just for anyone googling this as a note. (keywords)

To me it seems the following services are not working correctly because of this issue:
Guacamole, Node-Red, Nextcloud, Elkarbackup,...

Services working just fine:
Portainer, Dupliati, Jellyfin,..

@nareddyt
Copy link
Contributor

Also having issues with Home Assistant and Jupyter Notebook

@desimone are there any known workarounds for websocket support? Is it difficult to downgrade the connection to HTTP/1.1?

@desimone
Copy link
Contributor

desimone commented May 20, 2019 via email

@nareddyt
Copy link
Contributor

I started looking into this myself, and I think you're on the right track. For demonstration purposes, when I remove the TimeoutHandler in this line, the error message changes to flushWriter instead of timeoutWriter:

ERR http: proxy error: can't switch protocols using non-Hijacker ResponseWriter type *log.flushWriter proxy=localhost:8123

Looking at internal/log/wrap_writer.go, I see the basicWriter serves as a wrapper around ResponseWriter. I believe we have implemented this wrapper incorrectly, and that is breaking hijacking.

go-martini/martini#45 mentions the following:

The wrapped handler needs to to implement http.Hijackerinterface. It currently does not. Below is a relevant thread on Golang-nuts:
https://groups.google.com/forum/#!topic/golang-nuts/zq_i3Hf7Nbs

So if we rewrite basicWriter and flushWriter to wrap the ResponseWriter correctly, I think hijacking should be supported. Thoughts? I can experiment with this a bit more in the next few days.

Of course, even if this did work, we still need a workaround for TimeoutHandler. I'll look into that if I can correctly re-implement the wrapper.

@nareddyt nareddyt mentioned this issue May 31, 2019
4 tasks
@nareddyt
Copy link
Contributor

Nevermind, I was actually able to get websocket support working without having to re-write the wrapper. All I did was change the hardcoded HTTP protocol to 1 instead of 2 (in addition to removing TimeoutHandler).

Any ideas on how to proceed forward to enable websocket support based on this working prototype? I have created PR #151 to demonstrate the minimal code changes needed to support websockets. It would be great if you could mark that as a WIP PR.

@desimone
Copy link
Contributor

Looking at internal/log/wrap_writer.go, I see the basicWriter serves as a wrapper around ResponseWriter. I believe we have implemented this wrapper incorrectly, and that is breaking hijacking.

Solid sleuthing @nareddyt . I think you are right. I'm going to follow up on your PR with thoughts.

@desimone
Copy link
Contributor

desimone commented Jul 8, 2019

Hi @abl -- wanted to check in to see if this solved your issue. Thanks!

@abl
Copy link
Author

abl commented Jul 8, 2019

@desimone @nareddyt thanks! I deployed the changes but haven't had a chance to test them fully yet - getting a different error than before with some apps but others appear to be working (but aren't showing up as websocket requests anymore in Chrome, which is...odd).

@desimone
Copy link
Contributor

desimone commented Jul 8, 2019

Sounds good. Let me know if I can help or jump in chat on slack. I believe you were using this to try out visual studio code in the browser?
Screen Shot 2019-07-07 at 10 43 55 PM

@abl
Copy link
Author

abl commented Jul 8, 2019

awesome! Yes, that was my primary use case - the errors I'm currently getting are from Synology's streaming console/terminal. It makes a lot of assumptions and I'm in the process of deprecating it in favor of proper Docker tooling anyways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants