Add TCP support

[pion]

Add TCP support

[Sean-Der]

Hey @rstinear

TCP support is merged into master. I am going to work on bringing up the code coverage ~80% and fixing bugs before I announce though!

If you have any opinions/thoughts I would love to hear them. The design has dramatically changed, but I believe this goes down the path of solving issues with TURN servers people have been fighting.

[Sean-Der]

Ok I feel pretty good about this, check it out when you get a chance! We have a simple example here

I would love peoples feedback. I also have the v2 release notes to see everything that went into this release. I would love peoples feedback before I announce to the greater world.

I am going to try and put as much polish as possible into pion/turn, but will get dragged away in about a week. So if anyone has things they can think of now I would love to hear!

[Sean-Der]

Totally understand! I will do /v2 if we don't like it we can start over.

I really love your feedback though, lots of great ideas have come out of it! I have made so many tech debt mistakes, so nice to get others people opinion.

thanks @enobufs! Will try to have something this weekend, I really want to have integration tests before I call TCP support done.

[Sean-Der]

@rstinear hey! I am REALLY shooting to have this done this weekend.

Technically the work isn't hard just struggling with designing a good API, I want to measure twice cut once hopefully :) I will have a PR you can test with that supports UDP, DTLS, TCP and TLS by the end of this weekend!

[iwittkau]

I tried to use pion-turn with SimpleWebRTC clients. They weren't able to connect to each other and the server logged several:

YYYY/MM/DD HH:MM:SS Failed to handle ALLOCATE-REQUEST from XXX.XXX.XXX.XXX:1650: MessageIntegrity mismatch 227dde3a5dfea88f12e8ac08d5e4497a ace4ee917261ed5d5037e8a9da505039fe0ecf54

Is that related? If so, I think for compatibility reasons TCP should be implemented.

[Sean-Der]

Hey @iwittkau

MessageIntegrity is for authentication, what browser were you using? At one time we had a bug that broke FireFox c0352cc it could be broken in other ways.

I will improve the error message so it says to double check the username/credential also

[iwittkau]

I was using Chrome 65 on macOS.
I now tested it again with the WebRTC samples Trickle ICE Test which said "Authentication failed?". Same error in pion-turn: "MessageIntegrity mismatch".

[Sean-Der]

Is it possible the username/password is wrong?

I did make modifications to simple-turn with 04bf642 but I have been using the TURN server and auth has worked fine for me so far!

[iwittkau]

I was able to get it to work now: it wasn't an authentication failure. I had pion-turn listen on a port that was also opened for TCP requests in the firewall. So I disabled TCP for the pion-turn port in my firewall.
It seems like TURN clients try to use TCP first, if available?

Anyway, I can now report that it works flawlessly with andyet's SimlpeWebRTC clients and signalmaster!

[Sean-Der]

@iwittkau Thanks again for being such a fantastic user/tester :) that is great to hear!

I will start a branch with TCP support this weekend! It shouldn't be that hard, just been distracted working on pions/webrtc which has been a lot of fun.

[iwittkau]

@Sean-Der nice! Hope I can test that aswell soon :)

[Sean-Der]

@iwittkau hey! sorry I didn't get to this during the weekend, was caught up doing DTLS stuff :(

Promise that this week will start on it, I am really excited to get this done. Then there shouldn't be any reason people can't use us!

[oec]

@Sean-Der , any update on the TCP-support? Beeing able to run STUN via TCP on port 443 would allow us to communicate with many people behind corporate firewalls using only our self-hosted components.

[Sean-Der]

Hey @oec
Sorry for the slow response, I am going through and triaging all our issues!

pion/webrtc really took off as far as users requests and interest, and has taken up most of my time. Since it has production users (and most interest) it has gotten most of my attention.

I do think we will get to this by the end of the summer though! We use pion/turn to test pion/webrtc and we have had a few requests for TCP TURN support. I will update this ticket when I start on that though.

thanks for the patience, and if I can ever be helpful always happy to talk on Slack

[enobufs]

@Sean-Der in case it helps, here are my thoughts for what it's worth...

Framing

We will need to synchronize the STUN packets over TCP. I do not think RFC 5766 does not mention anything about how the synchronization should work. I believe, we have to follow STUN RFC Sec 7.2.2 for it, which seems to indicate that we do not have to have additional framing layer (use STUN packet's length field, also length field of Channel Bind). In addition to that, RFC 5766 requires padding for packets over TCP stream. (RFC 5766 Sec 11.5) - probably the hardest part of TCP support.

Retransmission

RFC 5766 does not mention anything about how to deal with retransmission of STUN packets over TCP. Probably we will need to follow STUN RFC.

   Reliability of STUN over TCP and TLS-over-TCP is handled by TCP
   itself, and there are no retransmissions at the STUN protocol level.

So, we should turn off the retransmissions implemented in the "PerformTransaction" method when TCP is used..

[Sean-Der]

pion/webrtc now supports TCP!

I will know working on adding server support to pion/turn so we can have integration tests

[Sean-Der]

@enobufs I started looking at this, what do you think of these API ideas?

I would like the API to be like how we have Client people pass in Conn/Listeners, and then we just loop over them. That will give people the flexibility they need to do UDP, DTLS, TCP and TLS. It will require API breakage though.

turn.ServerConfig{
    PacketConns []*net.PacketConn
    PacketConnListeners []func() (*net.PacketConn err)
}

Then users can populate the PacketConns with UDP sockets. PacketConnListeners will be full of blocking functions that return a TCP or TLS socket that is wrapped with a STUNConn so it will appear to be a PacketConn so we can keep the internal code simple still.

---

Then I will make our interface iteration code and update the examples to use them.

[enobufs]

@Sean-Der
I have actually been wondering if turn.Client should have completely owned the conn.
The reason is because, when you try to close, turn.Client, it has to close the conn to have conn.Read() unblock. (this problem may be happening in CandidateRelay...) I also thought about using conn.SetReadDeadline to unblock Read(), but then turn.Client is changing the behavior of conn that it does not own.

Any thoughts?

[Sean-Der]

Oof that is a tough one, I don't know which is better.

I think closing the passed socket is 'least' surprising. Otherwise we will be leaking a goroutine. It will also be pretty easy for the user to debug/tell that the socket was closed if they were reading it in another thread.

I wish Go had the ability to cancel Reads, that would make this a lot easier! Something like select would be great

[Sean-Der]

Personally I like how Client is, it gives us the flexibility to do a lot more. After we land TCP support I am going to update the docs and talk more about the 'vision' and setup auto-building.

I think it is really powerful if we can provide an 'API for building TURN servers'. At work scaling coturn is really painful, I would love if we could get pion/turn to the point that we could easily scale it. So I think having hooks (but maybe less new dev friendly) is the way to go.

[enobufs]

I am fine with decoupling conn from Client and Server as long as we can do so safely.

Looking at turn.Client code, it attempts both patterns, deatch and non-detached conn, but with one problem as I mentioned above, where you pass conn to turn.Client, but if you use Listen(), you cannot unblock Read() unless you close the conn you attempted to detach. Too bad golang's net is designed this way and not as flexible as C's select() / poll(), etc. (I agree with you), but my instinct says, we should follow the Go way, or it would bite you back later.

Therefore, I think if we should have the following design policy:

• When conn is not detached, turn.Client/Server should, create the conn, read/write and manage the entire lifecycle of the conn. (needless to say)
• When conn is decoupled, turn.Client/Server should not directly read data from conn. Have the user code read the data, pass the read data to HandleInbound().

Current turn.Client does not create the conn by itself but provides Listen() method which reads data from the conn, which is the problem I see, and we will need to fix it.

We could support both patterns. But I personally feel we could do the decoupling when it becomes absolutely necessary.

Maybe I just do not understand how decompling of conn helps "scaling" you mentioned...

[Sean-Der]

My vote is we only do HandleInbound()then! Would you be ok with getting rid of Listen I would prefer we only have one pattern of doing this.

I don't mind verbosity, I think this way accomplishes what I want the most (Flexibility, Explicit/Not Bug Prone)

re: scaling. I don't like how coturn forces you to do integrations (with stuff like redis). Lots of companies have existing proprietary systems, so I want to make that easy. If people want easy 'drop in TURN server' that problem has already been solved :)

Would you be ok with me starting a v2 branch, and when we are ready we can make that master? It will be a decent amount of breaking changes, but it will enable TCP for the server and get rid of Listen and we will only have HandleInbound

[enobufs]

It's your call!

It's just me trying not to invest anything until I know exactly what the goal is. I know you have some idea!

[rstinear]

Thanks for this project, really useful. Is there any more progress on TCP support?

[rstinear]

How have you gotten on @Sean-Der ? I'm still keen to take a look at a PR when you get the chance, thanks!

[Sean-Der]

@rstinear yes! It is a real mess right now, but I have been prototyping on https://github.com/pion/turn/blob/v2/examples/turn-server/main.go#L59-L81 it is a mess, but I am playing around with ideas I have wanted to do for a while.

You pass Conn and PacketConn directly into the server

• You can intercept/inspect traffic easier. This could let people do interesting things.
• You can do more then just TCP/UDP/TLS/DTLS. People can do testing/simulations easier.

Having a RelayAddressGenerator. You can generate the `relay-address

• It will make load balancing easier if you want to share your allocations across many servers
• It should work out of the box for the typical AWS user. If you put it behind a NAT it will work, no scripting to setup the external-address

And maybe more ideas! Sorry I have gotten more aggressive with the refactoring, I just wanted to build something that solved problems I have learned about in the past year :)