Skip to content

QUIC sniffer: Full support for handling multiple initial packets#4642

Merged
RPRX merged 10 commits intoXTLS:mainfrom
j2rong4cn:quic-sniffer
Apr 28, 2025
Merged

QUIC sniffer: Full support for handling multiple initial packets#4642
RPRX merged 10 commits intoXTLS:mainfrom
j2rong4cn:quic-sniffer

Conversation

@j2rong4cn
Copy link
Contributor

j2rong4cn and others added 2 commits April 22, 2025 17:08
大部分代码来自 v2fly/v2ray-core@8ceba34
v2fly/v2ray-core@c8c1120

Co-authored-by: Vigilans <vigilans@foxmail.com>
Co-authored-by: Shelikhoo <xiaokangwang@outlook.com>
@RPRX
Copy link
Member

RPRX commented Apr 23, 2025

整个 QUIC sniffer 的代码都是从 v2fly 那边搬来的,你要搬就全搬了

@j2rong4cn
Copy link
Contributor Author

整个 QUIC sniffer 的代码都是从 v2fly 那边搬来的,你要搬就全搬了

@ddatsh
Copy link
Contributor

ddatsh commented Apr 23, 2025

想提个问题请教啊,测试数据里那些hexData,有没办法是用代码来生成呀

@RPRX
Copy link
Member

RPRX commented Apr 23, 2025

想提个问题请教啊,测试数据里那些hexData,有没办法是用代码来生成呀

说实话我没看过

@j2rong4cn
Copy link
Contributor Author

想提个问题请教啊,测试数据里那些hexData,有没办法是用代码来生成呀

我是在手机的xray加个出站到电脑端得到的

@RPRX
Copy link
Member

RPRX commented Apr 23, 2025

@j2rong4cn 你现在有空完善这个 PR 吗?我可以等等然后很快合并

@j2rong4cn
Copy link
Contributor Author

@j2rong4cn 你现在有空完善这个 PR 吗?我可以等等然后很快合并

现在身边没电脑

@Fangliding
Copy link
Member

Fangliding commented Apr 23, 2025

想提个问题请教啊,测试数据里那些hexData,有没办法是用代码来生成呀

可以的 丢个fake conn给QUIC就行了 但是好像不是很有必要 加之奇奇怪怪的QUIC包主要来自chrome的chaos protection 要生成它们就有点麻烦了(

@j2rong4cn
Copy link
Contributor Author

@RPRX 好了

@j2rong4cn
Copy link
Contributor Author

应该没bug了

@Fangliding
Copy link
Member

最好全部搬过来然后移除掉那个defer recover

@j2rong4cn
Copy link
Contributor Author

最好全部搬过来然后移除掉那个defer recover

搬完了

onediram
onediram previously approved these changes Apr 23, 2025
@RPRX
Copy link
Member

RPRX commented Apr 28, 2025

看了下修改后的 buffer.go,用标识 bytespools 取代 if cap(p) == Size { 是合理的,NewWithSize() Alloc 的也能正确 Free 了

潜在 bug 是 NewExisted() 会把 unmanaged 和 bytespools 转为 managed,然后在 Release() 中被 pool.Put(p),以前有判断还好

但看了下 NewExisted() 只有 gRPC 传输层有两处调用,随便吧懒得再往上追了,gRPC 传输层都快成弃子了,交给众测

@j2rong4cn
Copy link
Contributor Author

潜在 bug 是 NewExisted() 会把 unmanaged 和 bytespools 转为 managed,然后在 Release() 中被 pool.Put(p),以前有判断还好

是直接从v2ray搬来的,没修改这个文件就没怎么看代码

@RPRX
Copy link
Member

RPRX commented Apr 28, 2025

@j2rong4cn 我不确定你这项新的修改合不合适,根据之前的代码,我感觉 NewExisted() 的本意可能就是复用 New() 的 buffer?不然和 FromBytes() 的区别就只剩确保了 capacity

@RPRX
Copy link
Member

RPRX commented Apr 28, 2025

@j2rong4cn 此外 if cap(p) == Size { 这个判断应该没必要加回来了,能 New 的就那五个函数,我之前看了一遍逻辑没有冲突

@j2rong4cn
Copy link
Contributor Author

j2rong4cn commented Apr 28, 2025

@j2rong4cn 我不确定你这项新的修改合不合适,根据之前的代码,我感觉 NewExisted() 的本意可能就是复用 New() 的 buffer?不然和 FromBytes() 的区别就只剩确保了 capacity

buffer.New之后不都会buffer.Release,除非忘记了Release,应该没问题吧,我去看看代码

感觉只加 if cap(p) == Size 判断最合适了

@RPRX
Copy link
Member

RPRX commented Apr 28, 2025

感觉只加 if cap(p) == Size 判断最合适了

这样也行,走原逻辑,至少 buffer.go 这种最重要的基础设施之一别改出啥问题

至于 QUIC sniffer 的部分我是没有看的,交给众测,以及我计划把针对 UDP/443 的默认 block 扩展至更大范围,还没定插哪合适

这个 PR 的 title 需要 rename 一下以匹配现在的 files changed,不用提及 buffer.go

@j2rong4cn
Copy link
Contributor Author

这个 PR 的 title 需要 rename 一下以匹配现在的 files changed,不用提及 buffer.go

你改吧

@RPRX
Copy link
Member

RPRX commented Apr 28, 2025

我没看 QUIC sniffer 的相关 changes 所以改不了,你改吧

@j2rong4cn
Copy link
Contributor Author

j2rong4cn commented Apr 28, 2025

至于 QUIC sniffer 的部分我是没有看的,交给众测

我在安卓设备上测试了几个chrome内核的主流浏览器都都可以识别了

@j2rong4cn j2rong4cn closed this Apr 28, 2025
@j2rong4cn j2rong4cn reopened this Apr 28, 2025
@j2rong4cn j2rong4cn changed the title QUIC sniffer: handle PacketNumberLength great 1 QUIC sniffer: Full support for handle multiple initial packets Apr 28, 2025
@RPRX RPRX changed the title QUIC sniffer: Full support for handle multiple initial packets QUIC sniffer: Full support for handling multiple initial packets Apr 28, 2025
@RPRX RPRX merged commit 58c4866 into XTLS:main Apr 28, 2025
70 checks passed
@RPRX
Copy link
Member

RPRX commented Apr 28, 2025

感谢 PR,以及 v2fly 的三位贡献者

@j2rong4cn
Copy link
Contributor Author

感谢 PR,以及 v2fly 的三位贡献者

又发现潜在bug了😅emm

@RPRX
Copy link
Member

RPRX commented Apr 28, 2025

其实之前我还是简单对比了一下的,我发现 v2fly 的代码没有你这里加的 wipeBytes(),看到你是在 "perf" 那个 commit 加的就没有说什么,而且你给 v2fly 那边也提了一个 PR,所以与它的 main 代码不一致也正常?所以就没提出

你再看看代码,可以开个 PR 修 bug,如果没把握的话就改成和 v2fly 的 main 保持一致吧

@j2rong4cn
Copy link
Contributor Author

其实之前我还是简单对比了一下的,我发现 v2fly 的代码没有你这里加的 wipeBytes(),看到你是在 "perf" 那个 commit 加的就没有说什么,而且你给 v2fly 那边也提了一个 PR,所以与它的 main 代码不一致也正常?所以就没提出

你再看看代码,可以开个 PR 修 bug,如果没把握的话就改成和 v2fly 的 main 保持一致吧

是buffer.New()得到了有数据buf,导致数据错乱

@RPRX
Copy link
Member

RPRX commented Apr 28, 2025

是buffer.New()得到了有数据buf,导致数据错乱

按理来说 buf.New() 不是默认 length 0、capacity 8k 吗?为什么还会得到有数据的 buf

@j2rong4cn
Copy link
Contributor Author

是buffer.New()得到了有数据buf,导致数据错乱

按理来说 buf.New() 不是默认 length 0、capacity 8k 吗?为什么还会得到有数据的 buf

因为buf是从pool里拿出来的,有时候会得到有数据的buf

@RPRX
Copy link
Member

RPRX commented Apr 28, 2025

因为buf是从pool里拿出来的,有时候会得到有数据的buf

只要 length 0,capacity 有数据也无所谓啊,难道 extend 加的是 length


nonce := cache.Extend(int32(cipher.NonceSize()))
binary.BigEndian.PutUint64(nonce[len(nonce)-8:], uint64(packetNumber))
_, err = buffer.Read(nonce[len(nonce)-packetNumberLength:])
Copy link
Contributor Author

@j2rong4cn j2rong4cn Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RPRX 因为这里把数据读取到nonce末尾

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

原本的代码就有这个潜在bug

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个 buffer.Read 没问题吧,上面 cache.Extend 可能有问题

@RPRX
Copy link
Member

RPRX commented Apr 28, 2025

我看了下 buf.Extend() 真就是只加 length,坏了

@j2rong4cn j2rong4cn deleted the quic-sniffer branch April 29, 2025 08:55
maoxikun pushed a commit to maoxikun/Xray-core that referenced this pull request Aug 23, 2025
…S#4642)

Co-authored-by: RPRX <63339210+RPRX@users.noreply.github.com>
Co-authored-by: Vigilans <vigilans@foxmail.com>
Co-authored-by: Shelikhoo <xiaokangwang@outlook.com>
Co-authored-by: dyhkwong <50692134+dyhkwong@users.noreply.github.com>
(cherry picked from commit 58c4866)
it2konst pushed a commit to it2konst/gametunnel-core that referenced this pull request Mar 1, 2026
…S#4642)

Co-authored-by: RPRX <63339210+RPRX@users.noreply.github.com>
Co-authored-by: Vigilans <vigilans@foxmail.com>
Co-authored-by: Shelikhoo <xiaokangwang@outlook.com>
Co-authored-by: dyhkwong <50692134+dyhkwong@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.

5 participants