Skip to content

[sshx] ParseSinglePublicKey 的 doc 承诺「拒一行塞两把」与实现/测试互相矛盾——rest 检查是不可达死代码,函数名与 doc 都不兑现「恰好一把」 #151

Closed

Description

@zy84338719

位置

  • sshx/key.go:112-127(ParseSinglePublicKey 的 doc 与两条承诺)
  • sshx/key.go:133-135(多行拒绝)
  • sshx/key.go:136-141(rest 检查)
  • sshx/key_test.go:174-185(仓内测试把「一行两把」钉成应当通过)

问题

doc 说这个函数比 ParsePublicKey 严,拒两种输入,其中第二种是:

一行里塞了两把:authorized_keys 是一行一把;rest 非空说明后面还有内容。

并给出存在理由:

本函数回答的是「这里面只有一把能用的钥匙吗」……要把这段字符串原样存进某个系统时,后者才是该问的那个。

但实现里这条防线不存在:

  1. key.go:140 的判据 len(strings.TrimSpace(string(rest))) > 0 依赖 ssh.ParseAuthorizedKey 返回的 rest。x/crypto 的 rest 语义是**「第一行之后的余量」**(golang.org/x/crypto/ssh/keys.go:234-243:end := bytes.IndexByte(in, '\n'); if end != -1 { rest = in[end+1:] } else { rest = nil })——单行输入的 rest 恒为 nil。而 :133 已把含换行的输入全部拒掉 ⇒ :140 永远不会命中,是一段不可达的死代码。
  2. 实际结果:「一行塞两把」时,第二把公钥被 ssh.ParseAuthorizedKey 当成 comment 字段吞掉(authorized_keys 的 comment 是自由文本),函数返回 err == nil。
  3. 仓内测试 sshx/key_test.go:174-185(子用例名「一行两把钥匙:第二把按语法算注释」,断言在 :181)把这一行为显式钉成「应当通过」,理由是「authorized_keys 的语法是「[options] type base64 [comment]」⇒ 一行里第二个 type/base64 形状的东西是注释,语法上分不出来」,并附一条 ⚠️:「堡垒机怎么存这一行没有实测过——本函数只保证「语法上是一把钥匙」」。

⇒ doc 说「拒」、测试说「不拒」、函数名说「Single」、doc 的存在理由说「回答「只有一把吗」」——契约面与行为面互相矛盾,且 doc 给出的推理(rest 非空)本身是错的。

实测

探针(自建 module,replace 指向 origin/main 快照 v0.9.2):/tmp/aud2/p-net,逐字输出:

=== sshx.ParseSinglePublicKey(契约:一行塞两把必须被拒)===
  一把           -> err=<nil>  返回公钥非nil=true
  一行塞两把        -> err=<nil>  返回公钥非nil=true
  两行各一把        -> err=the input holds more than one key  返回公钥非nil=false
=== 对照:x/crypto 原始解析(rest 语义 + comment 吞第二把)===
  err=<nil>  type=ssh-ed25519
  comment="ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAID1U564aCx5bPF/U3wNC30WXrZwd0r2J1jBnQTsVKNG9"
  rest 长度=0

对照组「两行各一把」被 :133 正确拒掉;实验组「一行塞两把」被接受。两者只差「两把钥匙在同一行还是两行」——doc 承诺的拒斥对两者一视同仁。

「rest 长度=0」直接证明 :140 的判据在单行输入上恒不成立(该判据不可达)。

为什么值得看

  • 调用方按 doc 的存在理由用这个函数替自己做「里面只有一把钥匙吗」的判断,然后把整个输入字符串原样存进某个系统(堡垒机 / 密钥库入口的典型形态)。doc 自己指出这类漏洞「极难查」,而 doc 给出的保护其实不存在。
  • 函数名 ParseSinglePublicKey + doc 首句「解析恰好一把 SSH 公钥」会让调用方以为拿到了「恰好一把」的保证。doc 里那句「rest 非空说明后面还有内容」是事实错误,照它写的检查代码永远是死代码——后来者按 doc 复核会得出「这一类已覆盖」的错觉。
  • 仓内测试的 ⚠️ 已承认「堡垒机怎么存这一行没有实测过」⇒ 这一格到底是「语法上就是一把」还是「实际存了两把」,正是 doc 承诺要回答、而 doc 实际没回答的问题。

建议修复方向

二选一(由维护者裁决,本条不预设):

  • (a) 兑现 doc:把拒斥判据从「rest 非空」换成「comment 本身像一把公钥」——例如 comment 以 ssh- / ecdsa- / sk- 开头(OpenSSH 公钥类型前缀)即判定为「一行塞了两把」并报错;或先用 strings.Fields 切分输入,若第 3 个及之后的字段能被 ssh.ParseAuthorizedKey 解析成公钥,则拒斥。
  • (b) 改 doc 兑现现实:doc 改成「解析出语法上第一把公钥;authorized_keys 语法允许 comment 自由文本,一行两把在语法上不可分」——但这样函数名里的 "Single" 与 doc 里「要把这段字符串原样存进某个系统时,后者才是该问的那个」需一并改写,并明确「本函数不保证只有一把」。

无论选哪条,:140 的不可达判据与 :122-123 的错误推理都应修掉——它们现在给读者一个「这一类已覆盖」的错觉。若选 (a),key_test.go:174-185 的子用例需相应翻转(并补一条「单行两把被拒」的断言)。

Activity

  1. moved this from Todo to In Progress in oekit Issue 大盘on Nov 10, 2026
  2. zy84338719 commented on Nov 10, 2026

    @zy84338719
    ContributorAuthor

    已在 v0.10.1 修复并发布 ✅

    变更内容

    本条修复(在 v0.10.1 的版本节里定位到):

    • fix(sshx): ParseSinglePublicKey 兑现「恰好一把」,拒「一行塞两把」([sshx] ParseSinglePublicKey 的 doc 承诺「拒一行塞两把」与实现/测试互相矛盾——rest 检查是不可达死代码,函数名与 doc 都不兑现「恰好一把」 #151)——doc 承诺拒两种输入,但第二种「一行里塞了两把」的防线(rest 非空)是不可达死代码:x/crypto 的 rest 是「第一行之后的余量」,单行输入恒为空、多行已被前一条判据拒掉 ⇒ 实际「一行塞两把」被放行,第二把被 authorized_keys 的 comment 字段吞下(doc 的推理「rest 非空说明后面还有内容」本身是错的),仓内测试还把该行为钉成应当通过。现判据换成「comment 里还藏着一把能解析的公钥」——第二把落进 comment 是语法必然,逐后缀试解析连「注释文字后跟第二把」也命中,判据用「能解析」而非类型前缀(ssh-/ecdsa-/sk-),注释里「ssh 跳板机」字样不误伤;rest 死代码与错误推理一并修掉,测试翻转为应当拒绝并补「普通注释放行」对照(不标 BREAKING:签名与既有合法输入行为不变,唯一变化是此前违背 doc/函数名承诺被放行的「一行塞两把」开始被拒,即收紧到既有契约;若存在依赖旧宽松行为——第二把被当 comment 吞掉——的调用方,会开始收到 error,此类调用方应按 BREAKING 对待)
    该版本 `v0.10.1` 的其余变更

    关联问题单:#151。

    Release: https://github.com/oekit/network/releases/tag/v0.10.1

    复核确认:修复(PR#153)已合入 main 且随 v0.10.1 发版,main CI 全绿。

  3. moved this from In Progress to Done in oekit Issue 大盘on Nov 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions