Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: should not dial bind addr directly #8

Merged
merged 1 commit into from
Jan 6, 2021
Merged

fix: should not dial bind addr directly #8

merged 1 commit into from
Jan 6, 2021

Conversation

mzz2017
Copy link
Contributor

@mzz2017 mzz2017 commented Dec 16, 2020

When server returns an any ip (0.0.0.0 or [::]), we should use conventional ip to replace the any ip given (0.0.0.0 or [::]). This behaviour adapts to most situations.

See v2fly/v2ray-core#523


Problem happens when socks5 proxy works on remote servers and different implementation will given different bind addr.

For most situations it is issues of servers. However, clients should behave normally in one case that returned bind addr is an ANYIP (0.0.0.0 or [::]), which indicate that we should use a conventional remote address to connect.

RFC does not illustrate the mechanism of bind addr; it is based on experience.

When server returns an any ip, we should use conventional ip to replace the any ip given. This behaviour adapts to most situations.
See v2fly/v2ray-core#523
@txthinking
Copy link
Owner

First of all, thanks for your PR, and I read the rfc again.

RFC does not illustrate the mechanism of bind addr;

rfc1928, part 6:

In the reply to a UDP ASSOCIATE request, the BND.PORT and BND.ADDR
fields indicate the port number/address where the client MUST send
UDP request messages to be relayed.

RFC has clarified BND.PORT and BND.ADDR.
So, I think we should not assume some non-standard behavior,

  • if a server does not support UDP, it MUST tell client through METHOD
  • if a server supports UDP, it MUST send a reply which described in rfc1928 to client

I support your point of not being clear about the part of rfc1928, but this issue BND.PORT and BND.ADDR, it is clear.
So if a server is not implemented correctly, we should submit an issue to it.

Thank you again.

@mzz2017
Copy link
Contributor Author

mzz2017 commented Jan 4, 2021

The problem is that in some NAT situation, the server may can not confirm a solid outer IP. Thus, the server may expect the client to connect the address the same as TCP's.

@txthinking
Copy link
Owner

There are many situations about UDP server address(IP:PORT) to process UDP request:

  • Same UDP IP with TCP IP and same UDP PORT with TCP PORT
  • Same UDP IP with TCP IP and random UDP PORT(port-range)
  • Different UDP IP(usually another independent machine) with TCP IP and random UDP PORT(port-range)

So MAY is not better, because MAY is a guess, not a guarantee. I think this is why RFC let server tell client its UDP IP:PORT
For guarantee, the only the server know what it will do, what it want to do, this mean we should let human config its server, here is a example project about UDP case:

https://github.com/coturn/coturn/blob/8dc5bbcb3b7f2ef561293df8b920cb21b0eaf081/examples/etc/turnserver.conf#L108

@database64128
Copy link

RFC has clarified BND.PORT and BND.ADDR.

In reality, the use case of SOCKS5 as a means of communication between censorship circumvention tools can make it difficult to determine the actual BND.ADDR and/or BND.PORT. For example, the SOCKS5 server may be behind a NAT with a dynamic IP address.

So if a server is not implemented correctly, we should submit an issue to it.

That would mean we have to change the behavior of UdpAssociate response on all these SOCKS5 servers:

You can't possibly change them all. "Fixing" the behavior would also make it more difficult to configure the SOCKS5 server. Standard compliance is important. But it's even more important to make sure the protocol actually works well under the desired use cases.

The Internet Area Working Group also acknowledged the widespread use of SOCKS5 among censorship circumvention tools in the SOCKS6 draft. So my opinion is that we should be flexible in this case and accept the widespread use of :: and 0.0.0.0 as the BND.ADDR.

@mzz2017
Copy link
Contributor Author

mzz2017 commented Jan 4, 2021

There are many situations about UDP server address(IP:PORT) to process UDP request:

NAT server exactly know the port but may not know the IP address, so if it reply 0.0.0.0 with the port specified the client will know the thing.

@txthinking txthinking merged commit 299f607 into txthinking:master Jan 6, 2021
@txthinking
Copy link
Owner

Because this is not a standard, so I added a hijack func HijackServerUDPAddr into Client

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants