fix: bindIP not work on udp #63
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #63 +/- ##
==========================================
+ Coverage 63.13% 65.39% +2.25%
==========================================
Files 14 14
Lines 784 705 -79
==========================================
- Hits 495 461 -34
+ Misses 230 184 -46
- Partials 59 60 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Mine. |
|
Actually I'm not so confident about this :( |
// WithBindIP is used for bind or udp associate
func WithBindIP(ip net.IP) Option {
return func(s *Server) {
if len(ip) != 0 {
s.bindIP = make(net.IP, 0, len(ip))
s.bindIP = append(s.bindIP, ip...)
}
}
} |
|
I agree that it is natural to use bindIP for UDP associate bind address. (I'm okay to revise my code like this) The last part of the third last packet include "0500 0004 0000 0000 0000 0000 0000 0000 0000 0000 b395", which invalidly replies [::]:45973 instead of 192.168.1.8:45973. redsocks terminates connection due to the invalid address. |
|
Return the local addr that tcp listener used may work,but it is just a coincidence.What if the server is behind NAT? That will return a intranet address not working to client. |
|
Yes, SOCKS5 behind NAT is rather serious problem. IMAO this is a fault of SOCKS5 protocol itself. But this is still a problem in your implementation. This problem can be solved on the client side. For example, a TPROXY-like software in Windows called Proxifyre seems to ignore the replied address and only cares the port. Using TCP local address is not logically perfect but works in most situations. On the other hand, your implementation is at least incompatible with redsocks or hev-socks5-tproxy, which are most major ones of TPROXY client in Linux, even without any NAT. |
|
No,you need to assign the right udp addr when you define the server options,and it will work well with all socks5 client. opts := []socks5.Option{
socks5.WithBindIP(udpip),
}
// Create a SOCKS5 server
server := socks5.NewServer(opts...)
if err := server.ListenAndServe("tcp", serveAddr); err != nil {
log.Fatal(err)
}Configure the optiong wrong,and the it won't work.This is not a problem |
|
Socks5 is a protocol.I don't think we should fix it incorrect to make it compatible with incorrect implementation of clients |
|
Ah you are considering socks5 listens some external IP like 192.168.1.1 ? |
|
I'm not saying altering SOCKS5 protocol. My implementation completely conforms to SOCKS5 specification and is compatible with more clients. |
|
Rather, I suggest using a single "bind address" for both TCP and UDP. Allowing to set TCP listen IP and UDP listen IP differently is wierd, isn't it? |
|
I understand what you mean.There is two conception we discussing. |
|
Hmm I'm afraid it still won't work if bindIP is 0.0.0.0 ? I think it's unnatural to return 0.0.0.0 to client. |
|
I have told you,bindIP is only for assign udp ip for return to client |
|
As I explained, returning 0.0.0.0 to client is problematic because the client cannot connect to 0.0.0.0, resulting in incompatibillity with redsocks or hev-socks5-tproxy. |
|
Yeah,but that's a para error |
|
I hope this software can listen all IPs at once and process clients accessing on different IPs. My implementation conform to SOCKS5 spec and no need to make it inconvenient. |
|
You didn't get my point,I won't reply you any more. |
|
Okay. Again, in my opinion, this is basically due to the fault of SOCKS5 proxy specification. |
|
Both implementation is not good enough for now,I don't care who's PR is accepted. |
|
What I want to say is that, in your implementation, we cannot serve SOCKS5 on two extenal IPs at once, whatever bindIP you may choose. However, we may be able to serve two separate SOCKS5 on each IP. |
|
@fregie How about using TCP local addr only if |
|
https://github.com/ginuerzh/gost/blob/fd57e80709aba9581757b1cd63b7d8f75e2385d2/socks.go#L1141 |
|
I sent a very similar PR (ginuerzh/gost#1030) to gost and now it's merged (though go-socks doesn't have to obey gost). |
#64
udp associate not return a correct ip to client,cause
Server.bindIPnot used as localAddr whennet.ListenUDP