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

Make UDP Associate function meet RFC standard #1

Closed
wants to merge 1 commit into from

Conversation

ph4ntonn
Copy link

Hi, i happen to use your repo to test my program, and i found that something goes wrong: The UDP Associate function doesn't meet RFC standard prefectly.
The original text of the RFC1928 is:

The UDP ASSOCIATE request is used to establish an association within the UDP relay process to handle UDP datagrams. The DST.ADDR and DST.PORT fields contain the address and port that the client expects to use to send UDP datagrams on for the association. The server MAY use this information to limit access to the association. If the client is not in possesion of the information at the time of the UDP ASSOCIATE, the client MUST use a port number and address of all zeros.

And I found the original implement is to send the target's ip+port combination instead of the one client expects to use to send UDP datagrams on for the association
So, i make a little change on your repo, and hope that won't bring any mistakes 😊
Feel free to contact me if you have any problems.
Have a nice day 👋

@thinkgos
Copy link
Member

@ph4ntonn 你好,你改的是握手时的tcp Request请求中的DST.ADDR 和DST.PORT. 这不是应该发送client端要访问真实的UDP地址么?
在发送UDP association 关联的数据包,位于ccsocks5的underconn.go文件,这个也是发送的client端要访问真实的UDP地址.
我不太理解,为啥要发送Client的proxyconn tcp的本地地址?

客户端暂时我是实现了一版实现基本功能,但不是最终想要的,设计时不利于扩展.没有抽像底层连接.

@ph4ntonn
Copy link
Author

@ph4ntonn 你好,你改的是握手时的tcp Request请求中的DST.ADDR 和DST.PORT. 这不是应该发送client端要访问真实的UDP地址么?
在发送UDP association 关联的数据包,位于ccsocks5的underconn.go文件,这个也是发送的client端要访问真实的UDP地址.
我不太理解,为啥要发送Client的proxyconn tcp的本地地址?

客户端暂时我是实现了一版实现基本功能,但不是最终想要的,设计时不利于扩展.没有抽像底层连接.

因为RFC文档里面规定UDP Associate请求所携带的地址应当是client欲发送数据的地址+端口,这一个值可以被服务器用来限制来自其余地址的数据包。
你所说的真实UDP地址是在UDP Associate请求协商完成之后,在client向socks5服务器发送的数据必须加上一个特殊的数据包头,包头中携带真实的目标udp地址,服务器解析后决定将client的请求发往何处
至于为何要发送Client的proxyconn tcp的本地地址,是因为我看你的代码里面,在tcp链接协商完UDP Associate后,直接利用此socket对应的IP+port来发起对应的UDP请求

if laddr == nil {
		ad := conn.proxyConn.LocalAddr().(*net.TCPAddr)
		laddr = &net.UDPAddr{
			IP:   ad.IP,
			Port: ad.Port,
			Zone: ad.Zone,
		}
	}

	udpConn, err := net.DialUDP(network, laddr, ra)
	if err != nil {
		conn.Close()
		return nil, err
	}

所以也就是说,你的实现默认UDP Associate所需要的TCP以及UDP socket源IP+port都是一样的,所以这里就发送这个ip+port。
如果我理解错了,也请指出~

@ph4ntonn
Copy link
Author

@ph4ntonn 你好,你改的是握手时的tcp Request请求中的DST.ADDR 和DST.PORT. 这不是应该发送client端要访问真实的UDP地址么?
在发送UDP association 关联的数据包,位于ccsocks5的underconn.go文件,这个也是发送的client端要访问真实的UDP地址.
我不太理解,为啥要发送Client的proxyconn tcp的本地地址?

客户端暂时我是实现了一版实现基本功能,但不是最终想要的,设计时不利于扩展.没有抽像底层连接.

可以详细看看我上面引用的RFC文档,这里的DST.ADDR 和DST.PORT不是所谓的“目的”地址

@thinkgos
Copy link
Member

@ph4ntonn 你好,你改的是握手时的tcp Request请求中的DST.ADDR 和DST.PORT. 这不是应该发送client端要访问真实的UDP地址么?
在发送UDP association 关联的数据包,位于ccsocks5的underconn.go文件,这个也是发送的client端要访问真实的UDP地址.
我不太理解,为啥要发送Client的proxyconn tcp的本地地址?
客户端暂时我是实现了一版实现基本功能,但不是最终想要的,设计时不利于扩展.没有抽像底层连接.

可以详细看看我上面引用的RFC文档,这里的DST.ADDR 和DST.PORT不是所谓的“目的”地址

感谢,我找个时间缕一下. 我再看看RFC文档,如果的确是这样,那服务端的Associate也是同样的错误.

@ph4ntonn
Copy link
Author

@ph4ntonn 你好,你改的是握手时的tcp Request请求中的DST.ADDR 和DST.PORT. 这不是应该发送client端要访问真实的UDP地址么?
在发送UDP association 关联的数据包,位于ccsocks5的underconn.go文件,这个也是发送的client端要访问真实的UDP地址.
我不太理解,为啥要发送Client的proxyconn tcp的本地地址?
客户端暂时我是实现了一版实现基本功能,但不是最终想要的,设计时不利于扩展.没有抽像底层连接.

可以详细看看我上面引用的RFC文档,这里的DST.ADDR 和DST.PORT不是所谓的“目的”地址

感谢,我找个时间缕一下. 我再看看RFC文档,如果的确是这样,那服务端的Associate也是同样的错误.

没事hh,不过服务端的话RFC是用的MAY use,所以不提供限制源地址的功能也可以(或者给个开关) :)
同样感谢你的库hh,期待之后的更新~

@thinkgos
Copy link
Member

#33

@thinkgos thinkgos closed this Sep 11, 2023
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.

2 participants