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

修复socks5密码可以被绕过的漏洞 #3371

Merged
merged 5 commits into from
May 22, 2024
Merged

Conversation

Fangliding
Copy link
Member

@Fangliding Fangliding commented May 17, 2024

虽然这个漏洞无关痛痒不过多少是个安全漏洞 还是想办法修了
不改变现有的UDP处理结构(UDP还是听在TCP同端口)

@Fangliding
Copy link
Member Author

@yuhan6665
Copy link
Member

@RPRX

@Fangliding
Copy link
Member Author

我按视频里确实复现了xray有这个问题 这个也确实可以拦截非法请求并且在日志输出

@dyhkwong
Copy link
Contributor

FYI 你可以通过 类似这里的方式 为每个会话动态分配随机的 UDP 端口,何必这样。炒 几年前的冷饭 有什么意思呢。

@Fangliding
Copy link
Member Author

FYI 你可以通过 类似这里的方式 为每个会话动态分配随机的 UDP 端口,何必这样。炒 几年前的冷饭 有什么意思呢。

要抄的话sing里的socks5处理更严谨一点
我也觉得炒冷饭没意思不想多动所以弄了一个很简单的方式解决问题(

@RPRX
Copy link
Member

RPRX commented May 20, 2024

幸好看了下,600 秒太短了,而且这样改有点影响性能

服务端设了密码时,sync.Map 存一下来源 ip 就行了,无 timeout

服务端没设密码时,绕过这个新增的机制,不影响绝大多数场景下(本地、内网 socks5)的性能

@RPRX
Copy link
Member

RPRX commented May 21, 2024

GKD,这个和 #3323 弄完后又可以发个小版本

@Fangliding
Copy link
Member Author

我没有怎么考虑性能问题 因为实际上这是从ss2022的盐过滤器改装来的 对于ss2022来说每个新连接都会在这个池子新增条目和过滤已有的盐 开销比简单的过滤IP高多了(正常来说这个池子里只会有一个IP 新的请求只是刷新过期时间)

@RPRX
Copy link
Member

RPRX commented May 21, 2024

说到“刷新过期时间”,现在这个好像只有 Add 没有 Update,效果是每 600 秒 UDP 都会断一次,所以说太短了

但是加上 Update 的话开销更大了,虽然 *ray 里这种东西已经不少了

反正 *ray 的实现都不标准,简单加一个 IP 无 timeout,确保大聪明们开在公网的 socks5 不会轻易被人扫去转发 UDP 就行

然后 Xray 发个小版本,大聪明们无意间升级一下,免费 UDP 转发器数量骤减

@Fangliding
Copy link
Member Author

好了 改成sync.Map了(

@RPRX
Copy link
Member

RPRX commented May 22, 2024

好了 改成sync.Map了(

#3295 (comment)

@RPRX
Copy link
Member

RPRX commented May 22, 2024

学一下我的代码风格,不要学 *ray 的,风扇看到我的简化后会

还有,公网上 UDP 乱飞的,AtError() 会造成大量无用日志,改成最次要的 AtDebug() 了

@RPRX
Copy link
Member

RPRX commented May 22, 2024

顺便看了下 shadowsocks 的 server.go,不小心把它的代码复制上去了

UDPFilter 无 timeout,有个小问题是有心者可以用大量 IPv6 地址非正常地不断 Add,但这样的话可以封他号,所以不是大问题

@RPRX RPRX merged commit 9ee9a06 into XTLS:main May 22, 2024
34 checks passed
@Fangliding
Copy link
Member Author

我还真没考虑过这个也会被其他udp流量影响
道理上把检查移到handleUDPPayload读完socks5的请求头再检查就行了()

@RPRX
Copy link
Member

RPRX commented May 22, 2024

道理上把检查移到handleUDPPayload读完socks5的请求头再检查就行了()

现在来源不明的 UDP 直接扔了也挺好

@RPRX
Copy link
Member

RPRX commented May 24, 2024

Xray-core v1.8.13 安全更新:Socks5 入站有密码时,丢弃未认证过的来源 IP 的 UDP 请求 #3371 @Fangliding @RPRX

总算把这句话改到位了,我觉得 Socks5 UDP 固定端口时,这是个比较恰当的解决方案,Clash.Meta 也更新一下吧 @H1JK

@RPRX
Copy link
Member

RPRX commented May 24, 2024

还想到一件事,Socks5 的标准 UDP Associate 是客户端开 TCP 告知自己这边要用的 UDP IP 和端口,那么我们这里的实现是可以在这个 IP 不为空时把这个 IP 加进白名单,但我觉得没有必要,因为实践中会留空(未知公网的 IP 和端口),而且客户端那个 IP 能发 UDP 就应当能发 TCP 来认证,否则有点异常,并且我们不希望客户端甚至不需要大量 IP 就可以乱塞白名单,虽然可以封他号

leninalive pushed a commit to amnezia-vpn/amnezia-xray-core that referenced this pull request Oct 29, 2024
Co-authored-by: RPRX <63339210+RPRX@users.noreply.github.com>
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.

4 participants