badvpn icon indicating copy to clipboard operation
badvpn copied to clipboard

Implement optional support for SOCKS5-UDP

Open bemasc opened this issue 7 years ago • 13 comments

This change adds a new option, --socks5-udp. If this option is present, and no UDP gateway is specified, UDP packets will no longer be dropped. Instead, the client will use the SOCKS5 UDP ASSOCIATE command to route UDP packets through the proxy server.

This implementation is intended for use with any UDP data, and it includes an optimization for packets containing DNS queries. However, this implementation is currently limited to localhost SOCKS5 servers. SOCKS5-UDP does not perform well over actual network links, as it requires several roundtrips to the server and is not compatible with NAT.

This implementation is currently in use in a fork of tun2socks used by Outline (https://getoutline.org) and Intra (https://getintra.org).

Fixes https://github.com/ambrop72/badvpn/issues/56

bemasc avatar Dec 14 '18 20:12 bemasc

great, well i couldn't built it in windows?

What error message do you see? Building for Windows worked for me using Cygwin.

bemasc avatar Jan 01 '19 18:01 bemasc

sorry i tried MSVC which didn't compile in your branch but master. actually i wasn't able to compile in windows with cygwin at all. so i don't want to blame the udp feature for this. but would like to test this.

@4-FLOSS-Free-Libre-Open-Source-Software Thanks for reporting the error on Windows. I rebased this branch on @ambrop72's latest Windows build improvements, and I was able to reproduce the failure. The problem I saw was that SocksUdpClient.c was using some C99 syntax that is not supported by MSVC.

I've removed the use of non-MSVC-compatible syntax from SocksUdpClient.c, and verified that this branch now builds on linux with gcc and on Windows with MSVC using the new instructions.

This branch should be ready to merge.

bemasc avatar Jan 15 '19 22:01 bemasc

Cool that this is fixed. I will try to find time to review this pull request.

ambrop72 avatar Jan 16 '19 17:01 ambrop72

@4-FLOSS-Free-Libre-Open-Source-Software Thanks for reporting the error on Windows. verified that this branch now builds on linux with gcc and on Windows with MSVC

me too can confirm it builds fine, good job @bemasc

Could you please add a copyright notice to SocksUdpClient.h/c, i.e. "Copyright (C) your_name_or_company", above the license terms text?

ambrop72 avatar Aug 06 '19 22:08 ambrop72

Done!

bemasc avatar Aug 19 '19 16:08 bemasc

Thanks. I have already reviewed the other code, when I'm done with those last files I will merge with some fixups.

ambrop72 avatar Aug 19 '19 16:08 ambrop72

Done!

test failed,what did I do wrong?

working. QQ截图20190827164138

failed QQ截图20190827164034

failed QQ截图20190827164945

ioxuy avatar Aug 27 '19 08:08 ioxuy

@ioxuy could you get past the assertion errors until now? do you use ipv6 udp traffic on tapinterface no or? i only see ipv4 interface configured. line 119 i don´t get it. https://github.com/ambrop72/badvpn/blob/d4539b496447c8510663494a44c3a3ade9237edd/system/BDatagram_win.c#L119

@bemasc, I have some comments/questions.

However, this implementation is currently limited to localhost SOCKS5 servers. SOCKS5-UDP does not perform well over actual network links, as it requires several roundtrips to the server and is not compatible with NAT.

The round-trip should not be too bad for local (non-Internet links).

From reading the code, it seems that the localhost-only limitation is because the UDP socket is always bound to a localhost address, and this is the address sent to the SOCKS server as DST.ADDR. Anyway, I think instead of binding to localhost, we should do one of the following, selected via command line flags:

  1. Send zeros in DST.ADDR (allowed by the SOCKS5 RFC) and do not bind the UDP socket; let the kernel assign the local address when the first packet is sent. I think this has the best chance of working when NAT is involved.
  2. Get the local address of the TCP socket used for the SOCKS server and bind to some port on that address. This should work when the SOCKS server, UDP relay and tun2socks are not separated by NAT (and the UDP relay is not on a different network interface than the SOCKS server, which would be really strange).

You don't need to change the code, I plan to improve it, I'd just like to hear your reasoning and if you think the above would work.

ambrop72 avatar Dec 02 '19 19:12 ambrop72

Those options both seem reasonable to me. I don't know whether most SOCKS-UDP servers implement correct support for zero DST.ADDR, but having a user control to choose that option seems like a fine idea. For simplicity, you might want to use the #2 binding behavior in both cases, so the only thing that changes between the modes is the value of DST.ADDR. As you mentioned, this should work fine so long as the UDP and TCP proxies are routable from the same interface, which seems like a reasonable assumption*.

Just make sure to drop any downstream UDP packets whose source address doesn't match BND.ADDR, and also be careful when parsing the matching packets due to source-IP spoofing.

*One case where this would fail is TCP reverse-binding a remote SOCKS server onto localhost (e.g. with ssh -L). I think it's fine not to support that, at least initially.

bemasc avatar Dec 02 '19 19:12 bemasc