Closed nange closed 1 year ago
@xjasonlyu PTAL.
https://github.com/xjasonlyu/tun2socks/issues/218 这个问题我没仔细看,理论上不应该会有问题🤔
不过需要注意的是,在tun2socks里relay的时候,有一侧并不是TCPConn
,而且它似乎也不支持CloseWrite
。
嗯。 考虑了的,加上了这个方法:
func (tt *tcpTracker) CloseWrite() error {
return tt.Conn.(*net.TCPConn).CloseWrite()
}
CloseWrite
在本质上是给对方发送FIN,告诉对方,自己已经发送完毕了,是需要这样做的。 之前直接设置一个超时时间,可能提前退出了对远端连接的数据读取是不对的,应该需要等待对方发送FIN才能结束,即让io.CopyBuffer
自己退出,而不是设置超时然后退出。
218 这个问题我没仔细看,理论上不应该会有问题🤔
不过需要注意的是,在tun2socks里relay的时候,有一侧并不是
TCPConn
,而且它似乎也不支持CloseWrite
。
当时加超时机制主要是因为,有时候有一端明明已经断开了,但是netstack的连接还是没有正确关闭,即出现了连接永久挂起的问题。不知道这个解决方案会不会有这个问题。
当时加超时机制主要是因为,有时候有一端明明已经断开了,但是netstack的连接还是没有正确关闭,即出现了连接永久挂起的问题。不知道这个解决方案会不会有这个问题。
从你描述的问题以及代码看,应该是因为一方关闭断开后(其中一个io.CopyBuffer
会退出),由于没有调用CloseWrite
方法,造成对方不知道另一方已经关闭了,自己就一直没有关闭连接引起(另一个io.CopyBuffer
可能一直未完成退出)。
目前的解决方案,不会有这个问题。
不过我参考类似v2ray里的relay代码,他们也是用这种的timeout机制的。大多数客户端一般就直接close,较少才会closeWrite/Read分开。
我测试了v2ray也有同样的bug。 绝大部分应用层代码都只需要调用Close即可(因为大多数时候,都是在业务逻辑处理完成后,调用一次Close即可,后续就直接退出了,没有后续逻辑要处理了),只有做一些更底层的应用,一些较特殊的场景,可能会调用CloseWrite。 而Tun2socks是需要处理所有通用场景,因此需要考虑使用。
有道理,我再多测试一下!
看了一下现在的Clash relay代码,也是使用的类似设置超时的方式。我看看是否也存在这个问题。
OK,我测试了几次,感觉这么写似乎没有问题!
然后我觉得,可以在closeWrite的同时也setReadDeadline。
另外,我觉得不需要再在tracker里添加closeWrite方法,直接在relay里通过一个closeWriter接口判断能不能调用,可以的话直接调用就行了。
然后我觉得,可以在closeWrite的同时也setReadDeadline。
另外,我觉得不需要再在tracker里添加closeWrite方法,直接在relay里通过一个closeWriter接口判断能不能调用,可以的话直接调用就行了。
setReadDeadline
我也OK,但我觉得应该是作为兜底方案来看待。所以我建议把这个超时时间设置为足够长,比如30min。 你觉得多少合适?closeWrite
方法的话,这个功能如何生效呢?在relay判断的话, left
和right
肯定都是不满足接口断言的。 我按自己的理解重新优化了一下代码,你再看看。
关于setReadDeadline
,我想到一个更优的方法,你看看如何:
原理:如果tcpTracker
内部的Conn
支持CloseWrite
能力,则relay
里面就不需要调用setReadDeadline
方法。反之则调用setReadDeadline
方法,保持和你之前的实现一致。
实现:给tcpTracker
添加CloseWrite
方法,内部实现通过接口断言,看Conn
字段是否满足CloseWrite
接口,如果满足则正常调用,如果不满足,则返回一个错误。在relay
中,调用CloseWrite
方法,判断返回error不为空,则需要调用setReadDeadline
,并且时间设置为之前的5s。
@xjasonlyu
我昨天一直在找有没有什么更优雅的solution,然后参考了一些代码,比如 cloudflared:
它这个似乎直接就一侧完成以后就退出close掉了,我有点迷惑。
我昨天一直在找有没有什么更优雅的solution,然后参考了一些代码,比如 cloudflared:
它这个似乎直接就一侧完成以后就退出close掉了,我有点迷惑。
看上去它也没有处理CloseWrite的问题,不知道是不是cloudflared大部分场景都是用于HTTP的请求协议,HTTP请求过来的话,本身也没办法使用CloseWrite(无法支持),然后它把到目标地址的连接直接关闭了,HTTP服务这边是可以自动重用连接的。
关于
setReadDeadline
,我想到一个更优的方法,你看看如何:原理:如果
tcpTracker
内部的Conn
支持CloseWrite
能力,则relay
里面就不需要调用setReadDeadline
方法。反之则调用setReadDeadline
方法,保持和你之前的实现一致。实现:给
tcpTracker
添加CloseWrite
方法,内部实现通过接口断言,看Conn
字段是否满足CloseWrite
接口,如果满足则正常调用,如果不满足,则返回一个错误。在relay
中,调用CloseWrite
方法,判断返回error不为空,则需要调用setReadDeadline
,并且时间设置为之前的5s。@xjasonlyu
我按上面说的优化方案,更新了一版代码。
我看看。
同时setReadDeadline我也OK,但我觉得应该是作为兜底方案来看待。所以我建议把这个超时时间设置为足够长,比如30min。 你觉得多少合适?
另外,这里我觉得可以把tcp wait timeout变成一个cli 参数,这块我先去调整一下。
我发现目前的实现,似乎还是会出现这个PR https://github.com/xjasonlyu/tun2socks/pull/155#issuecomment-1211926178 里提到的问题。
我发现目前的实现,似乎还是会出现这个PR #155 (comment) 里提到的问题。
怎么能重现能说一下吗,我去试试。
似乎是用的ftp命令行操作,不过具体我也不是很清楚。
等下,我忽然想到一个问题,不知道你之前测试是基于哪个代理的;我之前做的所有测试用的代理,其实都是direct://
,但这理论上不能算是代理。
所以在我们本地实现的closeWrite/Read,如果对面代理还是直接关闭的话,似乎意义就不大了?
等下,我忽然想到一个问题,不知道你之前测试是基于哪个代理的;我之前做的所有测试用的代理,其实都是
direct://
,但这理论上不能算是代理。所以在我们本地实现的closeWrite/Read,如果对面代理还是直接关闭的话,似乎意义就不大了?
是的。如果对面代理不支持closeWrite就没用了。我之前测试是基于direct://,以及我自己写的代理:easyss。
但是首先要tun2socks这边支持,整个链条才可能通,如果对方代理不支持也是没有什么影响的,就回退到了原来的样子。
我发现目前的实现有个问题:
假设,服务端是读取来自客户端请求,然后关闭连接,
func server() {
listener, err := net.Listen("tcp", ":9878")
if err != nil {
log.Fatal(err)
}
// defer listener.Close()
conn, err := listener.Accept()
if err != nil {
log.Fatal("server", err)
os.Exit(1)
}
fmt.Println("connected from:", conn.RemoteAddr())
data := make([]byte, 1)
for {
if _, err := conn.Read(data); err != nil {
log.Println("server:", err)
break
}
}
conn.(*net.TCPConn).Close()
}
客户端出于某种原因,连接上之后没有任何读写操作就直接关闭连接,
func client() {
conn, err := net.Dial("tcp", "localhost:9878")
if err != nil {
log.Fatal("client", err)
}
// time.Sleep(3*time.Second)
conn.Close()
}
正常直连情况下,服务端读到EOF,就会自动断开:
connected from: [::1]:57079
2023/04/02 21:22:49 server: EOF
但是经过tun2socks之后,按照目前的PR实现:copyBuffer(right, left)
这侧由于left已关闭,会返回EOF,调用right.CloseWrite()
并退出;另一侧copyBuffer(left, right)
由于服务端不会主动关闭连接,则会一直卡着,造成tcp连接泄漏,这样肯定是不行的。
没明白呢,调用right.CloseWrite()
后,服务端不就收到EOF,然后服务端就关闭连接了啊,然后copyBuffer(left, right)
不就收到EOF,正常退出了?
Sorry我看走眼了。
不过我参考了一下其他的一些solution,如:https://www.excentis.com/blog/tcp-half-close-a-cool-feature-that-is-now-broken/
We suspect that some NAT devices, when seeing a FIN message, will delete the corresponding NAT entry after a short timeout even if no FIN is seen from the other side. The result is that TCP connections that are in a half–close state will stop working after a while.
I should mention that this behavior is not allowed by RFC 5382: The closing phase begins when both endpoints have terminated their half of the connection by sending a FIN packet. (The RFC also allows an idle-timeout for inactive connections after two hours and 4 minutes. But that rule did not apply in our situation because our TCP flow had only been running for a few seconds and it wasn’t idle.) So maybe this was just a bug in the NAT device implementation.
But it’s not only NAT devices that can cause problems. Many firewalls also implement a TCP half-close timeout. These timeouts can be very short: Cisco decreased the minimum half-close timeout to 30 seconds to provide better DoS protection.
And even the Linux operating system implements a half-close timeout. The default value is 60 seconds.
我觉得CloseWrite/Read可以增加进去,然后CloseWrite之后也必须要调用一次SetReadDeadline,时长使用Linux默认的60s。你觉得呢?
我按照上面的思路重新实现了一下,试了一下效果还不错,你可以试试
我看了你给的参考链接,确实考虑到可能的拒绝攻击,设置一个相对较小的超时时间是合理的。 不过我在Linux上用这个PR关联issue里面的测试代码测试了,并不会出现文章里面说的,60s超时时间, 我把服务器的sleep时间调高到180s,客户端依旧能获取到服务器返回的信息。你可以试试看,是否我的测试有问题?
最新的代码,整体我也比较赞同,有一个点我觉得需要讨论,就是CloseRead
的调用有必要吗?我目前没有看出有调用此方法的必要,因为这个方法的调用只是会影响代码层面对连接的读取,目前代码层面也没有和这个相关连的逻辑。
我看了你给的参考链接,确实考虑到可能的拒绝攻击,设置一个相对较小的超时时间是合理的。 不过我在Linux上用这个PR关联issue里面的测试代码测试了,并不会出现文章里面说的,60s超时时间, 我把服务器的sleep时间调高到180s,客户端依旧能获取到服务器返回的信息。你可以试试看,是否我的测试有问题?
这块考虑是测试的原因?不是很清楚。
最新的代码,整体我也比较赞同,有一个点我觉得需要讨论,就是CloseRead的调用有必要吗?我目前没有看出有调用此方法的必要,因为这个方法的调用只是会影响代码层面对连接的读取,目前代码层面也没有和这个相关连的逻辑。
考虑到对称一下,也没什么影响。
考虑到对称一下,也没什么影响。
嗯,问题不大,主要是个审美偏好,我是如果这个代码不是必须的,就会删除。 看你,我都能接受。
嗯,另一方面go-tun2socks
这个项目的这里也是有类似实现的:
func (h *tcpHandler) relay(lhs, rhs net.Conn) {
upCh := make(chan struct{})
cls := func(dir direction, interrupt bool) {
lhsDConn, lhsOk := lhs.(duplexConn)
rhsDConn, rhsOk := rhs.(duplexConn)
if !interrupt && lhsOk && rhsOk {
switch dir {
case dirUplink:
lhsDConn.CloseRead()
rhsDConn.CloseWrite()
case dirDownlink:
lhsDConn.CloseWrite()
rhsDConn.CloseRead()
default:
panic("unexpected direction")
}
} else {
lhs.Close()
rhs.Close()
}
}
...
感觉没什么问题,直接Merge了 :-P
还想到一个点,io.CopyBuffer
需要慎用,容易出现无谓的内存申请。
最好做一下接口断言,如果满足对应接口,则应该直接调用io.Copy
。
io.Copy
和io.CopyBuffer
底层都是io.copyBuffer
。
而且这里没什么需要担心的,因为tcpTracker
和core.tcpConn
都不存在ReadFrom
和WriteTo
方法;其次,调用这两个方法是为了实现ZeroCopy,但是前提是必须两端都是*net.TCPConn
或*os.File
,而gVisor那侧必定不是。
Ok,明白。
Fixes: #218