The remote address of UDP relay is not handled properly

[madeye]

In shadowsocks/shadowsocks-android#2454, we found shadowsocks-rust handles the UDP relay incorrectly, which cause STUN and DNS (with checking the source address) tests failed.

The bug is related to these lines:

shadowsocks-rust/src/relay/udprelay/association.rs

Line 411 in f1bb7c5

let _ = Address::read_from(&mut cur).await?;

Shadowsocks-rust ignores the "real" source address returned from remote, and didn't construct the relay packet with this address.

shadowsocks-rust/src/relay/udprelay/socks5_local.rs

Line 127 in f1bb7c5

let payload = assemble_packet(Address::SocketAddress(src), &pkt);

To fix this issue in socks5-local, we should assemble the packet with the source address from shadowsocks remote.

In other implementations, we simply returns the plain packet (SOCKS5 address + payload) back to the socks5 client:

https://github.com/shadowsocks/shadowsocks-libev/blob/401d32348024f7a0871aef76d436a5a847ef3b5a/src/udprelay.c#L881

In the NAT redir mode, we need to rebind the UDP socket to the the "real" address to let the client think the packet is actually sent from that address.

https://github.com/shadowsocks/shadowsocks-libev/blob/401d32348024f7a0871aef76d436a5a847ef3b5a/src/udprelay.c#L875

[madeye]

I tried to fix this via edeff21.

It's tested with ss-local and socksfy (https://www.inet.no/dante/doc/1.3.x/socksify.1.html), and dig works now.

[zonyitoo]

That's a huge bug!

[zonyitoo]

@madeye Could you make a PR for this?

[Mygod]

The commit is already in #211 right?

[madeye]

Yes, it's already in that PR.