WebSocket support

[rubenseyer]

I too dream of a Mumble on the web, so here's my attempt at giving Grumble built-in Websocket support. It's not quite ready for prime time, and would need a full-featured client to match, but I still think we can discuss the architecture. I'm a total Go noob, so any suggestions on the code are welcome as well. Sorry for also sticking another unrelated bug I found in here as well, but I didn't think a missing exclamation mark warranted it's own pull request.

(I'm using this client to test, although it isn't feature complete.)

Websockets and the Mumble protocol are exposed on the same port, since conveniently they both run on top of TLS. Protocol suggestions through NPN/ALPN are followed, but since neither the Mumble client nor some browsers (looking at you, Chrome) perform this negotiation the code looks at the first two bytes to see if they match "GE", otherwise we assume the Mumble protocol. There is no practical difference in the code for clients speaking the raw Mumble protocol. Websocket clients are upgraded and then wrapped into a mock connection which packs and unpacks messages transparently.

Even certificates work through the web, if the clients are set up correctly. The HTTP server is set to request but not force certificates. This causes Firefox to pop up a certificate selection UI, but sadly Chrome devs prefer not to (it does work if you trick it to cache a certificate for the host, but it's not very user-friendly). It's very useful to have a valid certificate for the server because the browser can otherwise silently refuse to connect.

Websockets are TCP-only and additionally has their own framing which may mess with the packets and affect performance. However, as long as the client is fairly cooperative you can tunnel voice over them with acceptable quality, although the browser really kills the performance if the tab isn't in focus. To really have full web voice support it should ideally also communicate over WebRTC (which would be UDP), but that is more complex to implement and requires an additional negotiation on the application layer which would mean a Mumble protocol extension.

[mkrautz]

Thank you. I have wanted WebSocket support in Murmur/Grumble for a long time -- but there are too many other issues that take up my time to be able to work on it. But I have thought about it quite a bit.

Just a few drive-by comments. Haven't done a thorough review yet:

For it to really work, I think we'd need good Let's Encrypt/ACME integration. Probably with the x/crypto/acme/autocert package. But that can always be added later. :-)

I'm happy that you've tested against https://github.com/Johni0702/mumble-web. That's what I would have suggested you test against.

The negotiation/sniffer is probably too simplistic. Also, Chrome doesn't negotiate http or h2 via ALPN? Aww. That's sad.

I've always wanted to expose the WebSocket on the native Mumble port like you've done, but perhaps we should expose it via port 443 as well. Perhaps with a protocol extension allowing you to pick the "real" port. (This is future work.)

Regarding client certs. Does Firefox bring up the picker when you connect to the WebSocket? I actually hoped that it wouldn't. I think that it's best not to use client certs directly for WebSockets, and instead move client cert handling into the application layer instead of TLS. (Mostly to avoid "bad" browser UI for client certs). Browsers are pretty clear that client certs are not something they like, so trying to force use of them via TLS is not going to work in the long term. (Not requesting a client certificate obviously clashes with the ability for WebSocket support to use the same port. So perhaps the rule should be that port 443 does not request a client cert, while native Mumble ports do.)

Hope to be able to do a full review sooner rather than later! Thanks again.

[mkrautz]

The negotiation/sniffer is probably too simplistic. Also, Chrome doesn't negotiate http or h2 via ALPN? Aww. That's sad.

Actually, if it's just for the WS upgrade, I suppose it's fine? I was thinking it had to catch all HTTP verbs, but reading the code, that's probably not the case?

[rubenseyer]

Here comes a very long response... but discussing how the implementation would actually work in practice was what I wanted to do so here goes:

The negotiation/sniffer is probably too simplistic. [...]

Actually, if it's just for the WS upgrade, I suppose it's fine? I was thinking it had to catch all HTTP verbs, but reading the code, that's probably not the case?

It's more of a design decision here, I think. The WS spec says that the upgrade request must be a GET. After those two bytes are sniffed (if necessary) and the code decides to treat it as a WS connection everything is handled through the HTTP and WS implementations - there's no detection on every packet or something like that. I see it this way: if you try to use other HTTP methods, i.e. definitely not upgrade to WS, the result would be exactly the same as trying to talk HTTP to a server only speaking the Mumble protocol. It's like any Mumble server by default unless you "speak the magic words". If you want to handle every HTTP method on the initial request through the HTTP server (the design decision I referred to earlier) the current sniffer is not enough. I supposed all of this is moot if we're adding port 443 because of client cert issues anyway.

Also, Chrome doesn't negotiate http or h2 via ALPN? Aww. That's sad.

As for Chrome, for some reason they completely clear the ALPN protocol list when sending WS requests. (I thought it was a bug at first so I even tracked down the exact source line if you're curious.) No idea why they do that whatsoever. They play nice like everyone else for ordinary HTTP. Implementing HTTP/2 to force a negotiation there would be meaningless as there is no standard for WS over HTTP/2 anyway.

Regarding client certs. Does Firefox bring up the picker when you connect to the WebSocket? I actually hoped that it wouldn't. I think that it's best not to use client certs directly for WebSockets, and instead move client cert handling into the application layer instead of TLS. (Mostly to avoid "bad" browser UI for client certs). Browsers are pretty clear that client certs are not something they like, so trying to force use of them via TLS is not going to work in the long term.

Firefox does bring up the certificate UI (given that you have a cert, see below). The Chrome devs in the bug report I linked earlier speculated that this isn't deliberate and more of a coincidence, and as you say, client certs are not a priority feature on the web. I don't think decoupling client certs from the TLS connection over WS is too difficult, seeing as there really only are two spots in the code that retrieve the cert directly from the connection - the new client handling and the user information request.

(Not requesting a client certificate obviously clashes with the ability for WebSocket support to use the same port. So perhaps the rule should be that port 443 does not request a client cert, while native Mumble ports do.)

The shared port that requests a client cert could have been a good compromise, if only browsers would cooperate. If you don't have any client certs Firefox just doesn't send one and continues - works just like you'd hope to. For those few who actually have a cert installed they can click Cancel once and then they will never see it again. However, with Chrome, even if you have no certs, the WS connection just fails unless a decision already has been made for ordinary HTTP. Therefore, just browsing to the web client and trying to connect will simply fail. The aforementioned Chrome bug report said this is working as intended. This means that everything would only flow nicely if Grumble also served the client, which really isn't what I had in mind. (Also, I realised now while testing this again that the extra TLSConfig I put in the HTTP server to make sure certs were sent from browsers is entirely useless - I'll change this together with any other suggested changes after you've done a full review.)

Hope to be able to do a full review sooner rather than later!

Take your time. I am also exploring patches to the browser clients to keep me busy 😄 .

[rubenseyer]

Alright, so I thought about this a bit more and made some changes to the code after thinking about your comments. Hopefully you hadn't gotten too far with the review, because I probably threw out two-thirds of the code.

• I dropped WS on port 64738 entirely for simplicity. Seeing as you don't care for the browser client cert ui, it would only work otherwise in specific browsers, and we're going to have to listen on port 443 anyway for the Let's Encrypt integration, I can find no use case for it (although if you really wanted it I could add it back)
• Instead I moved out a "real" HTTP server on port 443. It still upgrades WebSocket connections just like before, but is no longer hidden inside handleIncomingClient (it was a nifty way to avoid adding more state to the server and slot the server under the native TLS listener, but it really was a hack). I guess it would make some API etc. possible without too much pain if one wanted that too, but that would be some (far) future work.
• Obviously, this breaks client certificates for WS on the TLS level (they're disabled on the port 443 listener). Doesn't matter if this is going to be handled in the application layer.
• I'm not a fan of having to run the server privileged (at least on Unix-based systems, I don't think this is a problem on Windows?) to be able to bind to port 443, so some sort of privilege dropping after bind would need to be added.
• The Travis CI build seems to have broken because the graceful shutdown of the HTTP server didn't yet exist in that Go version.

Again, take your time. I don't expect this to get merged until it's done for real, and that probably means adding at least the Let's Encrypt integration and the application layer client certs for WS.

[mkrautz]

FYI, I just bumped the .travis.yml to use Go 1.9. If you rebase against master, I hope the graceful http shutdown build failure should be fixed.

[rubenseyer]

All good. Thanks!

[rubenseyer]

Thanks again. (I know this is a lot of commits. I'll squash after you're done.)

[rubenseyer]

Thanks!

[rubenseyer]

Rebased everything onto the new master. If you'd like me to start on the Let's Encrypt integration, I want to know how you think it should be configured. Config file? Command-line flags? Interactive prompts? At a minimum, there has to be a mechanism for TOS acceptance (and probably for domain name input).

[mkrautz]

Rebased everything onto the new master. If you'd like me to start on the Let's Encrypt integration, I want to know how you think it should be configured. Config file? Command-line flags? Interactive prompts? At a minimum, there has to be a mechanism for TOS acceptance (and probably for domain name input).

That'd be very nice!

Not sure. I think an interactive "first run" experience would be nice, but I don't think it's a requirement.

I think a config file option would suffice. It should probably be blank by default. If it is set, you've agreed to the Let's Encrypt TOS. We can explain this in a comment in the config file, I'd think.

...But I just realized that we don't really have a config file for Grumble anymore.

Feel free to suggest something, or just do a JSON config file. I'd probably pre-process the JSON, reading each line, if a line contains "//" or "#", disregard the those characters and the rest of the line. That way we can have comments in there. Open to other suggestions.

I'd think the config file should probably just be called config.json (or whatever format we choose) and live in the data dir.

[mkrautz]

Regarding the config file: maybe, to avoid too much trouble, grumble should have a murmur emulation mode. So it'd be possible to use a regular murmur.ini with grumble.

That doesn't preclude us from doing something else alongside that, but it'd probably help a lot of people out. It'd be neat if grumble could just be a drop-in replacement for murmur in many scenarios.

[rubenseyer]

Thanks. These changes should do it. If there really is nothing else, just tell me when I can squash the WebSocket changes (or maybe if you don't care about the unrelated 19d8d35 just do it through the github ui). I think a new pull request for the config stuff afterwards would be better (but there's no code yet, so...)

Regarding everything in Stop(); it's never actually called from anywhere... (And it currently doesn't work anyway due to the double close of listeners causing it to return, but that bug is probably out of scope for this pull request.) I still thought it important to put the HTTP server shutdown there so it isn't forgotten in case the server stop will be used someday.

I'd probably pre-process the JSON, reading each line, if a line contains "//" or "#", disregard the those characters and the rest of the line. That way we can have comments in there. Regarding the config file: maybe, to avoid too much trouble, grumble should have a murmur emulation mode. So it'd be possible to use a regular murmur.ini with grumble.

I am unsure about rolling our own JSON-with-comments parser. I'd prefer just picking a format with comments in the spec instead, although JSON has the upside that it would not require an external dependency. However, if we're parsing ini-like formats anyway (because the murmur.ini compatibility is indeed useful) I think we might as well go with config.ini.

[mkrautz]

@rubenseyer Feel free to do the squash, keeping the separate commit separate.

[mkrautz]

Once again: thanks a bunch!

[NickSutton]

I would like to use the websockets functionality for the server to send out to clients which User ID is currently transmitting.

As far as I can see I will need some kind of ‘hook’ on the server to trigger the event, and obviously write a client that display’s the user ID of the person transmitting. I’m using a GoLang client (Google TalkiePi) with a PHP page on top that allows me to create PTT via touchscreen. I think it would be cool to display a pop up when other’s are transmitting. I’m building an intercom system for the house! ;-)