Skip to content

Comments

TLS client: Verify leaf cert (name, time) when pinning self-signed CA#5532

Merged
RPRX merged 1 commit intomainfrom
cert-pin
Jan 13, 2026
Merged

TLS client: Verify leaf cert (name, time) when pinning self-signed CA#5532
RPRX merged 1 commit intomainfrom
cert-pin

Conversation

@Fangliding
Copy link
Member

@Fangliding Fangliding commented Jan 13, 2026

和之前讨论的一样 不用写那个verifycertinpeer啥的了了

@RPRX
Copy link
Member

RPRX commented Jan 13, 2026

之前讨论了啥

@Fangliding
Copy link
Member Author

Fangliding commented Jan 13, 2026

#5154 (comment)

@RPRX
Copy link
Member

RPRX commented Jan 13, 2026

也就是说像之前说的一样 pin 自签 CA 也还会用 SNI 验证证书和有效期了对吧

@RPRX RPRX changed the title Verify for self signed ca TLS client: Verify leaf cert (name, time) when pinning self-signed CA Jan 13, 2026
@RPRX RPRX merged commit 4bdf6e5 into main Jan 13, 2026
78 checks passed
@RPRX
Copy link
Member

RPRX commented Jan 13, 2026

还有个好处是如果 pin 了啥不受自己控制的不知名 CA 也会验证 leaf cert

@Fangliding
Copy link
Member Author

Fangliding commented Jan 13, 2026

还有个好处是如果 pin 了啥不受自己控制的不知名 CA 也会验证 leaf cert

前提是没开allow insecure
如果开了就是自签CA使用场景

@RPRX
Copy link
Member

RPRX commented Jan 13, 2026

话说 pin 的含义不应该是“只允许这个”吗，这方面有没有改

@RPRX
Copy link
Member

RPRX commented Jan 13, 2026

就是说似乎终于可以完全删掉 allowInsecure 并且完全用基于 pin 的逻辑了

@Fangliding
Copy link
Member Author

Fangliding commented Jan 13, 2026

话说 pin 的含义不应该是“只允许这个”吗，这方面有没有改

是这么设计的 如果pin ca 原根证书池会被替换

@RPRX
Copy link
Member

RPRX commented Jan 13, 2026

豁然开朗，之前真被你给绕进去了，逻辑应该是这样：

  1. 开了 pinnedPeerCertSha256 或 verifyPeerCertInNames 就自动 allowInsecure 以完全接管证书验证
  2. 如果 pin 了 leaf 证书且符合就直接通过，否则没其它验证的话就失败
  3. 如果 pin 了 CA 证书就仅信任这个/些 CA 证书，然后用 SNI 或 verifyPeerCertInNames 执行验证
  4. 如果没 pin、仅有 verifyPeerCertInNames，就用系统 CA 执行验证，以后换成内置 CA 库

在你回复前打的字，现在的逻辑貌似还是有些不符合

@RPRX
Copy link
Member

RPRX commented Jan 13, 2026

还有个好处是如果 pin 了啥不受自己控制的不知名 CA 也会验证 leaf cert

前提是没开allow insecure

就是这里不太符合

@BrewTestBot BrewTestBot mentioned this pull request Jan 13, 2026
Merged
1 task
@Fangliding
Copy link
Member Author

Fangliding commented Jan 13, 2026 •
edited
Loading

还有个好处是如果 pin 了啥不受自己控制的不知名 CA 也会验证 leaf cert

前提是没开allow insecure

就是这里不太符合

这里的逻辑是这样的
(在pin ca场景下)
1 verifyPeerCertInNames 会重新校验证书 这个pin会覆盖其使用的ca列表
2 如果没有 verifyPeerCertInNames 强行用 ServerName 重新验一遍
3 如果没开 allow insecure 则跳过第二点的重验证 这里的逻辑很简单 无allow insecure的情况下 如果这个验证函数被调用 代表go自己已经检查了证书链有效性 没必要再次验证了

@RPRX
Copy link
Member

RPRX commented Jan 14, 2026

无allow insecure的情况下 如果这个验证函数被调用 代表go自己已经检查了证书链有效性

但是 Go 自己的验证是基于系统 CA 而非 pinned CA，若有 pinned 则应只允许 pinned

@Fangliding
Copy link
Member Author

Fangliding commented Jan 14, 2026

它会去找 里面到底有没有pinned ca 如果没有就直接失败 这里只是跳过多余的验证

@RPRX
Copy link
Member

RPRX commented Jan 14, 2026

但是 pinned CA 不一定在系统 CA 库里，用户 pinned 这个行为本身就代表了信任，没必要再去验证它是否在系统 CA 库里

还是改成统一的自动 allowInsecure 然后由 pinned 接管验证吧

@RPRX
Copy link
Member

RPRX commented Jan 14, 2026

就是遵循这个逻辑 #5532 (comment)

这个逻辑更清晰，文档也好写，以后还能完全删掉 allowInsecure

@Fangliding
Copy link
Member Author

Fangliding commented Jan 14, 2026

我不是很喜欢自己包装逻辑 99%的用户都是有效证书再pin一下 这样够用(

@RPRX
Copy link
Member

RPRX commented Jan 14, 2026

改一下吧，复用验证逻辑就行，况且 allowInsecure 的描述是“跳过证书验证”但是现在又有 pinned，那到底是跳过了还是没跳过

更关键的是这会影响彻底删掉 allowInsecure 的大计

@RPRX
Copy link
Member

RPRX commented Jan 14, 2026

或者说比如，用户自己都知道 pin 的是 Let's Encrypt 的某一级证书，那么它在不在系统 CA 库中根本不重要，又不是不知道它是啥

@RPRX
Copy link
Member

RPRX commented Jan 14, 2026

pinned 这一行为本身就代表了绝对的信任，如果某台设备上系统 CA 库没及时更新还会导致 pinned 在不同设备上的行为不一致

@RPRX
Copy link
Member

RPRX commented Jan 14, 2026

如果要确保行为一致用户又要分享 allowInsecure，这意味着 VLESS 分享链接官方规范得加上它，与彻底删掉它是背道而驰

@RPRX
Copy link
Member

RPRX commented Jan 14, 2026

这个改完后隔几天可以再发一版

@RPRX RPRX mentioned this pull request Jan 14, 2026
Merged
@RPRX
Copy link
Member

RPRX commented Jan 16, 2026

既然风扇不愿意改的话我来改一下吧

@KobeArthurScofield Go 出 1.25.6 了，话说现在的 go.mod 是 1.25.5 是会导致自动编译用不上 1.25.6 吗

@KobeArthurScofield
Copy link
Contributor

KobeArthurScofield commented Jan 16, 2026

Go 出 1.25.6 了，话说现在的 go.mod 是 1.25.5 是会导致自动编译用不上 1.25.6 吗

是，正常的 GitHub Actions setup-go 有这个缺陷
Win7 的流程例外，是 setup 之后用补丁版整个覆盖，到时候发了新版会直接用上

@RPRX
Copy link
Member

RPRX commented Jan 16, 2026

https://t.me/projectXray/4612634

setup go不是可以写 >= 1.25.5吗？这样就自动下载1.25.6了啊

@KobeArthurScofield
Copy link
Contributor

KobeArthurScofield commented Jan 16, 2026

可以，到 1.26 要记得换回来
setup-go v6 可以走 go.mod 的 toolchain 参数，也可以在那里指定

看哪个比较好就用哪个就行

@RPRX
Copy link
Member

RPRX commented Jan 16, 2026

@Fangliding 似乎 pinnedPeerCertSha256 时自动把 allowInsecure 设为 true，并且删掉 len(verifiedChains) == 0 的判断就行了

有个小问题是貌似现在不支持同时 pin leaf 和 CA

@RPRX
Copy link
Member

RPRX commented Jan 16, 2026

@KobeArthurScofield 这样的话感觉写 >= 和写 latest 没啥区别

@Fangliding
Copy link
Member Author

Fangliding commented Jan 16, 2026

leaf和ca可以一起pin 不要发生一个链里同时有ca和leaf就行了 不然会比较麻烦 我觉得这种用例很少
len(verifiedChains) == 0 都可以不删 反正allowInsecure它就恒真了

@KobeArthurScofield
Copy link
Contributor

KobeArthurScofield commented Jan 16, 2026 •
edited
Loading

这样的话感觉写 >= 和写 latest 没啥区别

效果上来看是，甚至写 stable 都可以
目前是 go.mod 指定，setup-go v6 会先看 toolc hain 参数再看 go 参数
写 >= 和 latest 和 stable 之类的都会无视 go.mod

怎么会有依赖喜欢写到这么碎的小版本

@RPRX
Copy link
Member

RPRX commented Jan 16, 2026

发下个版本时我把它顺便改为 1.25.6 吧，反正 Go 1.26 也快了

@RPRX
Copy link
Member

RPRX commented Jan 16, 2026

@Fangliding 它那个 certs 是什么顺序，我想了一下把 leaf 放开头就行了

@Fangliding
Copy link
Member Author

Fangliding commented Jan 16, 2026

有正的有反的(

@RPRX
Copy link
Member

RPRX commented Jan 16, 2026

@Fangliding 不会出现中间出个 leaf 的情况吧，那判断一下第一个不是 leaf 的话 reverse 一下就行，我写一下

@Fangliding
Copy link
Member Author

Fangliding commented Jan 16, 2026 •
edited
Loading

本来就是个勉强兼容的功能 我也想改成单个 再乱整我是觉得不太好

@Fangliding
Copy link
Member Author

Fangliding commented Jan 16, 2026 •
edited
Loading

不是完全弄不了 就是得整成类似个最长匹配条目什么 找到ca不终止遍历 继续扫 扫到leaf直接回 没有leaf就返回找到ca 只是这样pin ca就必须全部扫完 有点笨比

@RPRX
Copy link
Member

RPRX commented Jan 16, 2026

@Fangliding 我本来写的是找到 leaf 后把它换到第一个，但如果 certs 顺序有正有反的话那直接 reverse 就行

RPRX added a commit that referenced this pull request Jan 16, 2026
@RPRX
TLS client: Skip TLS' built-in verification when using `pinnedPeerCer…
e7d7224 
…tSha256`

#5532 (comment)
#5532 (comment)
@RPRX
Copy link
Member

RPRX commented Jan 16, 2026

@Fangliding 测一下 e7d7224

@Fangliding
Copy link
Member Author

Fangliding commented Jan 16, 2026

这个有go test的 过了应该就没啥问题

@RPRX
Copy link
Member

RPRX commented Jan 16, 2026

@Fangliding 非也，我刚发现这两个 PR 改的逻辑有亿点小问题，return error 咋没了，我修一下，怪不得我当初写了个置为 nil

@Fangliding
Copy link
Member Author

Fangliding commented Jan 16, 2026

哦淦之前的逻辑交叉在一起了 删PinnedPeerCertificateChainSha256 不小心一块毙了

Fangliding pushed a commit that referenced this pull request Jan 16, 2026
@RPRX @Fangliding
TLS client: Skip TLS' built-in verification when using `pinnedPeerCer…
b459cee 
…tSha256`

#5532 (comment)
#5532 (comment)
@Fangliding
Copy link
Member Author

Fangliding commented Jan 16, 2026

我把这两行加回去了

RPRX added a commit that referenced this pull request Jan 16, 2026
@RPRX
TLS client: Skip TLS' built-in verification when using `pinnedPeerCer…
d182cdd 
…tSha256`; Fixes

#5532 (comment)
#5532 (comment)
#5532 (comment)
@RPRX
Copy link
Member

RPRX commented Jan 16, 2026

用我改的版本吧 d182cdd

@RPRX
Copy link
Member

RPRX commented Jan 16, 2026

（你那个 VerifyPeerCertInNames 的逻辑是错的）

@Fangliding
Copy link
Member Author

Fangliding commented Jan 16, 2026

看到了

@Fangliding
Copy link
Member Author

Fangliding commented Jan 16, 2026

等我写个testca的吧

@Fangliding Fangliding mentioned this pull request Jan 16, 2026
Merged
@RPRX
Copy link
Member

RPRX commented Jan 16, 2026

@Fangliding 我又想起了判断 VerifyPeerCertInNames 是否 nil 的真实原因是 RAW 那里可能把它置空，加了个注释防止别人改掉

RPRX added a commit that referenced this pull request Jan 16, 2026
@RPRX
TLS client: Skip TLS' built-in verification when using `pinnedPeerCer…
760223a 
…tSha256`; Fixes

#5532 (comment)
#5532 (comment)
#5532 (comment)
#5532 (comment)
RPRX pushed a commit that referenced this pull request Jan 17, 2026
@Fangliding
TLS client: Add pin_test.go for leaf and CA (#5553)
09f619d 
#5532 (comment)
This was referenced Jan 18, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Fangliding @RPRX @KobeArthurScofield