xtaci / kcp-go

A Crypto-Secure Reliable-UDP Library for golang with FEC
MIT License
4.1k stars 733 forks source link

Setting the UDP source port (adding laddr parameter to DialWithOptions in sess.go?) #51

Closed pmorch closed 7 years ago

pmorch commented 7 years ago

Hi,

In order to do UDP hole punching when using kcptun's client, I'd like to be able to set the UDP source port. This does not seem possible with the current implementation in sess.go. I first demonstrate that it is easy to hack, but would like to ask what the appropriate API should be, if this were to be considered for inclusion in kcp-go.

Setting the UDP source port would require the current call in DialWithOptions() in sess.go:

udpconn, err := net.DialUDP("udp", nil, udpaddr)

to provide a laddr parameter to net.DialUDP instead of nil. This hack makes kcptun's client_linux_amd64 use source port 20000:

> git diff
diff --git a/sess.go b/sess.go
index fac3122..418c374 100644
--- a/sess.go
+++ b/sess.go
@@ -915,7 +915,12 @@ func DialWithOptions(raddr string, block BlockCrypt, dataShards, parityShards in
                return nil, errors.Wrap(err, "net.ResolveUDPAddr")
        }

-       udpconn, err := net.DialUDP("udp", nil, udpaddr)
+       laddr, err := net.ResolveUDPAddr("udp", ":20000")
+       if err != nil {
+               return nil, errors.Wrap(err, "net.ResolveUDPAddr laddr")
+       }
+
+       udpconn, err := net.DialUDP("udp", laddr, udpaddr)
        if err != nil {
                return nil, errors.Wrap(err, "net.DialUDP")
        }

However, current the function signature of func DialWithOptions doesn't really make it possible/easy to add a laddr parameter.

// DialWithOptions connects to the remote address "raddr" on the network "udp" with packet encryption
func DialWithOptions(raddr string, block BlockCrypt, dataShards, parityShards int) (*UDPSession, error) {
        ...
}

I don't know of any alternative to or workaround for setting the UDP source port in order to do UDP hole punching and it seems like such a cool use of the KCP family of libraries so I really hope this possibility can be included. To that end I would like to ask @xtaci:

  1. Would you be open to adding the possibility of controlling the source port by providing/setting laddr?
  2. What API changes or addtions would you prefer for this?
    1. Adding a laddr parameter to func DialWithOptions?
      • Pro: This is the cleanest long-term solution
      • Con: That would break all existing callers to kcp-go's DialWithOptions
    2. My favorite: Create a type DialParams struct containing the raddr, block, dataShards, parityShards plus a new laddr element, and then create a new func DialWithParams that takes such a DialParams struct as a single parameter. Mark DialWithOptions as deprecated and implement it as a wrapper around DialWithParams.
      • Pro: This lends itself nicely to expansion in the future and it maintains backwards compatibility with DialWithOptions.
      • Con: Now, confusingly, there would be both a func DialWithParams and a deprecated func DialWithOptions.
    3. Create a new func DialWithOptionsAndLaddr that takes all the parameters of DialWithOptions + laddr? DialWithOptions would then be a wrapper around DialWithOptionsAndLaddr using a nil laddr. Choosing a good name for this func is tricky (it could also be called DialWithMoreOptions) - I guess because it ideally should be called DialWithOptions :-)
      • Pro: This follows the pattern of DialWithOptions and it maintains backwards compatibility with DialWithOptions.
      • Con: Will create a huge mess when someNewParam needs to be added in the future
    4. Some other approach you'd prefer?
AudriusButkevicius commented 7 years ago

You can already pass in net.PacketConn in one of the methods. This is redundant.

Listening on a specific upd port won't help you perform nat traversal as the UDP port given to you by tye NAT is still not known.

pmorch commented 7 years ago

You can already pass in net.PacketConn in one of the methods. This is redundant.

You are right. NewConn() does in fact let me pass in net.PacketConn. So extending or modifying DialWithOptions is not necessary. Sorry I missed that. I'll modify my derivative of kcptun/client/main.go to use NewConn() instead of DialWithOptions().

Listening on a specific upd port won't help you perform nat traversal as the UDP port given to you by the NAT is still not known.

That is what UDP hole punching is for. It involves STUN for well-behaved NATs and a coordinating server somewhere and this is a challenge I am prepared to implement code to overcome. I have it working here with my port 20000 hack above, so yes, it does in fact work. But only if I can control the UDP source port for the client.

AudriusButkevicius commented 7 years ago

I am well aware of how stun works, as I've implemented it in syncthing (yet I've hit other issues related to this library which I haven't solved it yet).

You do not need to control the client/server ports, as long as you can work out your external port mapped by the NAT via the STUN server that's all that matters.

pmorch commented 7 years ago

Then perhaps there is something I don't understand.

I'm planning on implementing this in a couple of steps, (assuming Alice wants to connect to Bob). This works when done manually:

  1. Using STUN to determine the outside (IP address, UDP port) sets for both Alice and Bob given some chosen inside UDP source ports.
  2. Communicating the outside (IP,port) of Alice to Bob, and the the outside (IP,port) of Bob to Alice.
  3. Do the UDP hole punching: Alice sends a dummy UDP packet to Bob's external (IP, Port) and Bob sends a dummy UDP packet to Alice's external (IP, Port). Both using the same internal UDP source ports.
  4. Use those same inside UDP source ports and external (IP, port) sets for KCP to establish the connection. This is where this issue, "Setting the UDP source port" becomes important.

Since the outside UDP ports depend on the inside UDP ports, how would you use STUN to determine the outside UDP port if KCP (or the OS?) later chooses a random port?

Or are you letting KCP choose an inside UDP port, and then afterwards using STUN to get the outside (IP,port) for the inside port chosen by KCP?

This is definitely getting off-topic for an issue called "Setting the UDP source port (adding laddr parameter to DialWithOptions in sess.go?)" but I would appreciate any feedback you'd care to give. Either here or to peter@morch.com.

AudriusButkevicius commented 7 years ago

Use those same inside UDP source ports and external (IP, port) sets for KCP to establish the connection. This is where this issue, "Setting the UDP source port" becomes important.

This does not matter, internal (ip, port) on Alice does not need to be the same as Bob. They don't even need to know what their internal pair is. As long as Bob knows that it's externally available on (Bip, Bp), and Alice is externally available on (Aip, Ap), Bob connects to (Aip, Ap), and Alice connects to (Bip, Bp), regardless of what the internal mapping is.

One thing you do need to do, is to make sure that you perform STUN via the same socket you are establishing a connection to the other side, so that (Xip, Xp) remains the same between discovering it and using it, though you do it not by listening on the same address pair twice (in fact, in some kernels this will even fail), you reuse the same socket descriptor you used for stun, and after stun is done, you pass it to kcp.

Actually, it's a bit more involved than that, as technically you should do test1 of stun every 20-24 seconds to keep the NAT mapping alive, because if you didn't manage to connect in the first 24 seconds, the NAP mapping is likely to get torn down, which essentially means you need to multiplex STUN and KCP over the same socket, and somehow be able to recognize the packets coming in and direct them to the right "virtual" connection.

This library had a nice interface, which allowed reading out of bound packets (packets that are not identified as being of the right protocol), which means you could share the same underlying connection for utp and offload the out of bound packets to STUN. Sadly, in my tests it wasn't a very reliable library.

Sadly, kcp does not support that and I had to roll my own library to support that.

Though as I said, I discovered other issues with KCP, where it maintains some state between connections/disconnections/reconnections, plus doesn't work very well when we have a sending socket and a receiving socket on the same underlying socket descriptor.

pmorch commented 7 years ago

Thank you for your aid, Audrius!

I find this concerning:

Though as I said, I discovered other issues with KCP, where it maintains some state between connections/disconnections/reconnections, plus doesn't work very well when we have a sending socket and a receiving socket on the same underlying socket descriptor.

I don't see any open issues or anything involving AudriusButkevicius relating to these two kcp-go issues. Did I miss something?

(I think I've found the reliability issue you had with "this library": Starts stalling under load · Issue #13 · anacrolix/utp)

pmorch commented 7 years ago

This is actually a duplicate of #36.

AudriusButkevicius commented 7 years ago

I haven't yet debugged to understand the cause, so perhapsits my own code.

imReker commented 6 years ago

Any update? Same problem here.