Files
soft_quay/docs/review/phase1-security-review.md
T

231 lines
20 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Phase 1 安全关键原型审查(T-101 ~ T-103)
> 原始审查范围:`0d6ed05`(T-101 签名清单验签与缓存回退)、`0ffeff6`(T-102 ZIP 安全解压)、`9da72ca`(T-103 原子切换与回滚)。
> 审查视角:全栈开发工程师 + 安全审计。
> 原始审查方式:静态代码审计 + 攻击路径推演;原审查环境未安装 Go 工具链。
> 二次复核:在当前 `7efcab5` 工作区复核 Phase 1 代码及后续 T-201 接线,并执行 `cd core && go vet ./... && go test -count=1 ./...`,全部通过。
> 日期:2026-07-16
## 总体判断
Phase 1 三个原型的整体设计质量较高:
- T-101 的验签、缓存替换和回退顺序正确,未发现可伪造 Ed25519 签名或把未验证远端内容写入缓存的路径。
- T-102 对常规 Zip Slip、符号链接、特殊文件、伪造展开大小和解压失败清理处理得较完整。
- T-103 的**进程崩溃恢复**状态机成立,保持“未经健康检查的新 current 不可信”的核心不变式。
但原报告“没有找到路径穿越、zip bomb 或崩溃丢数据路径”的表述过于绝对。二次复核发现:
1. T-102 没有处理 Windows 对尾随空格/句点和 DOS 设备名的特殊路径语义,存在 staging 外写入风险。
2. `archive/zip` 在条目数检查前已经解析中央目录,仍有中央目录/元数据内存 DoS 边界。
3. P1 不仅缺 rename/journal 的父目录耐久性顺序点,解压后的 payload 文件本身也没有显式 `Sync()`。
因此 M2 应调整为**有条件成立**:T-101 与 T-103 的原型目标成立;T-102 必须先关闭 Windows 路径语义缺口,才能作为正式安装链路的可信基线。真实断电耐久性仍须在 T-302/T-601 完成。
## 逐模块核验
### T-101 · 签名验签与缓存回退(catalog)
**核心验签链正确(`verifier.go` + `canonical.go`):**
- `parseRestrictedJSON` 拒绝非 UTF-8、重复键、尾随 JSON、非整数数字和非对象根。
- 流程为:读取顶层 `signature` → 删除该字段 → 对剩余值递归规范化 → Ed25519 验签。嵌套的 package `signature` 不会被误删。
- 公钥和签名长度显式检查;公钥被复制,不会受调用者后续修改影响。
- Loader 只有在 verify + validate 全部通过后才写缓存;缓存回退会重新验签,不会盲信磁盘内容。
**需要收紧的协议表述:**
- 不同字节表示规范化成相同字节是 canonicalization 的预期行为,安全性依赖“发布端和客户端对相同输入得到完全相同语义与规范字节”,而不是规范化函数具有单射性质。
- Go `encoding/json` 对非法 UTF-16 surrogate escape 不报错,而是替换为 U+FFFD。其他语言或发布器可能选择拒绝,因此应明确拒绝非法 surrogate,或至少用跨实现测试向量冻结行为。
- `base64.StdEncoding.Strict()` 仍会忽略 CR/LF,不能等同于“拒绝所有非规范 Base64”。如果协议要求唯一文本表示,应先拒绝空白,或解码后重新编码并与原字符串比较。
- 当前测试签名由客户端同一套 `parseRestrictedJSON`/`canonicalJSON` 生成,证明了实现自洽,但不能代替 `softbox-catalog` 发布端的独立测试向量。`docs/api.md` 已披露该边界,发布前必须补齐。
**缓存原子写:** 临时文件、权限、文件 `Sync()`、current/backup rename 和主文件缺失时读取 backup 的方向正确;真实断电下的目录项耐久性归 P1。
### T-102 · ZIP 安全解压(installer/extractor)
**已经成立的防御:**
- 拒绝空名、非 UTF-8、NUL、反斜杠、冒号、绝对路径、显式 `.`/`..` 和非规范 POSIX 相对路径。
- 拒绝符号链接、特殊文件、加密条目和协议外顶层文件。
- 大小写折叠查重、全新 staging、文件 `O_CREATE|O_EXCL`、失败清理等防覆盖措施合理。
- 预检声明大小,复制阶段再用 `LimitReader(remaining+1)` 限制真实展开量,并核对实际展开大小和 ZIP CRC。
**高优先级遗漏:Windows 路径段规范化。**
原报告认为 `path.Clean` 会拒绝尾随点,该判断不成立。`path.Clean` 使用 POSIX 词法规则,不会删除路径段末尾的 ASCII 空格或句点。例如:
```text
"payload/file."
"payload/file "
"payload/.. /escape.exe"
```
这些名称能通过当前 `normalizeArchivePath`。Windows 创建/打开普通 DOS 路径时会忽略名称尾部的 ASCII 空格和句点,`".. "`可能重新取得父目录语义;随后 `filepath.Join`、`MkdirAll`、`OpenFile` 可能把目标解析到 staging 之外。Windows 还保留 `CON`、`PRN`、`AUX`、`NUL`、`COM1`~`COM9`、`LPT1`~`LPT9` 等设备名及其部分扩展/变体。
参考:
- [Microsoft:文件和目录名尾随空格/句点行为](https://learn.microsoft.com/en-us/troubleshoot/windows-client/shell-experience/file-folder-name-whitespace-characters)
- [Microsoft:Windows 文件、路径和命名空间](https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file)
- [Go:Windows reserved name 处理](https://go.dev/src/internal/filepathlite/path_windows.go)
**整改建议:**
1. 建立共享的 Windows 安全相对路径校验器,逐段拒绝尾随空格/句点、DOS 设备名、控制字符和 Windows 禁止字符。
2. 同一校验器用于 ZIP entry、entrypoint、Catalog `entry_exe`、app.json/files.json 和 installed-app 文件路径,避免各层规则漂移。
3. 增加 `payload/.. /escape.exe`、`payload/file.`、`payload/file `、`payload/NUL`、设备名带扩展和别名冲突用例。
4. 在 Win7、Win10、Win11 上执行真实文件系统测试;单纯的 `filepath.Clean` 或字符串前缀比较不足以证明 Windows 路径安全。
**中央目录资源边界:**
条目数检查发生在 `zip.OpenReader` 完成中央目录解析之后。因此当前实现防住了展开数据 bomb,但没有在标准库分配 `archive.File` 列表前限制中央目录大小。该边界已在 `docs/tasks/T-102.md` 执行记录披露,原报告不应概括为“没有 zip bomb 路径”。
T-302 应在调用 `zip.OpenReader` 前核对 Catalog 包大小,并增加 ZIP 文件/EOCD/中央目录大小预扫描上限。
### T-103 · 原子切换与回滚(installer/switcher + transaction + recovery + layout)
**进程崩溃恢复核心不变式成立:**
- journal 阶段为 `prepared → current_backed_up → staging_activated → committed`,健康失败进入 `rollback_required`。
- staging 已成为 current 但 journal 尚未 committed 时,Recover 会恢复旧 backup 或撤销首次安装,不会自动信任新版本。
- committed 后才清理 backup;非法 current/staging/backup 组合返回 `ErrRecoveryInconsistent`。
- 主 journal 缺失时可读取中断遗留 backup;journal 和管理目录会拒绝显式 symlink/非预期类型。
- rename 顺序适配了 Windows 不能直接覆盖已存在目录的行为。
现有测试覆盖的是关键步骤之间的**进程退出**,该范围内没有发现状态机错误。真实断电、文件锁、杀毒软件干扰和 Windows 目录项落盘顺序不在这些测试的证明范围内。
## 需整改/优化项(二次复核后重新排序)
### P0 · Windows 路径语义可能绕过 staging 边界
**优先级:** 阻断 T-302 正式安装整合。
按 T-102 章节建议建立共享 Windows 安全路径校验器,补真实 Windows 用例。关闭前不得把当前 Extractor 描述为“无路径穿越”。
### P1 · 真实断电耐久性未覆盖
**现状:**
- `file_cache.go`、`transaction.go` 只同步临时文件内容,rename 后未建立父目录耐久性顺序点。
- current→backup、staging→current 等目录 rename 后没有持久化屏障。
- Extractor 对 payload 文件执行 `io.Copy` 后直接 `Close`,没有显式 `Sync()`;即使 journal 已 committed,断电后新版本文件数据仍可能未完整耐久。
**建议的完整顺序:**
1. 写完每个 payload 文件并确认内容耐久。
2. 确认 staging 目录树元数据达到可接受的耐久级别。
3. 写 prepared journal 并建立顺序点。
4. 每次目录 rename 后建立父目录顺序点。
5. 写新 phase journal并建立顺序点。
6. committed 耐久后才能删除 backup 和 journal。
Windows 不直接提供与 POSIX 完全相同的目录 fsync 语义,需要评估目录句柄/`FlushFileBuffers`、写穿策略或明确的平台恢复保证,并在 T-302/T-601 用 VM/真机断电故障注入验证。
### P2 · 中央目录预解析内存 DoS
在 `archive/zip` 解析中央目录前限制 ZIP 文件大小、中央目录大小和声明条目数。Catalog `size`、下载完成文件长度和 ZIP 预扫描结果必须同时一致,但不能只依赖 `len(archive.File)`。
### P3 · Catalog 规范化的跨实现测试向量
发布端必须独立生成固定向量,至少覆盖:
- Unicode 键排序、转义字符串、`<>&`、U+2028/U+2029;
- 合法 surrogate pair 和非法 surrogate 的拒绝策略;
- `-0`、大整数、嵌套 signature;
- Base64 padding、CR/LF 和其他空白;
- 同一语义不同 JSON 表示得到相同签名字节。
客户端测试不应继续只用同一个 canonicalizer 生成并验证全部“合法”签名。
### P4 · 解压上限可能误伤合法包
默认 200:1、4 GiB、10,000 条目是合理的原型硬上限,但需按真实包分布复核。原报告建议把压缩比降为软信号过于宽松:绝对体积虽能限制磁盘占用,仍不能完全限制 CPU/IO 放大。
建议继续保留条目数、单条大小、总展开量和压缩比硬边界;根据真实包采样调整阈值,或通过可信发布策略提供受控例外,不要由未验证包内容自行放宽限制。
### 已关闭 · 原 P3 channel 校验接线
当前 `NewClient` 已用 `NewLoader(verifier, fetcher, cache, parser)` 把 Parser 注册为 DocumentValidator。`TestClientValidatesBeforeReplacingCache` 已覆盖“签名正确但 channel 错误时回退缓存且不调用 Store”。该项不再是待确认风险。
### P5 · 文件 mode 来源
ZIP mode 决定 `0o600`/`0o700` 在目标 Windows 上基本不构成安全问题,但协议语义不够干净。可以统一采用固定落盘权限;可执行入口由已验证 app.json/Catalog 定义。
### P6 · 真实健康检查
`HealthCheck` 作为 Phase 1 注入接口合理。T-402/T-403 实现启动、探活、超时和进程退出判定时,必须防止“进程短暂启动即视为健康”以及错误工作目录/错误二进制被探活。
## 结论与处理顺序
建议按以下顺序处理:
1. **[阻断 T-302 · T-605]** 修复 Windows 尾随空格/句点、DOS 设备名和路径别名问题,补 Win7/10/11 文件系统测试。
2. **[阻断 T-302]** 在 `zip.OpenReader` 前增加包大小与中央目录预扫描边界。
3. **[发布前必做]** 补 payload 文件、目录 rename 和 journal 的完整断电耐久性策略,做 VM/真机故障注入。
4. **[协议冻结前]** 完成 `softbox-catalog` 与客户端之间的独立 canonicalization/签名测试向量。
5. **[整合时复核]** 基于真实包采样调整解压硬上限,不默认降级为软信号。
6. **[实现时重点审]** 审查 T-402/T-403 的真实健康检查。
7. **[低优先]** 固定解压文件 mode。
> 声明:当前 core vet/test 全部通过,证明现有测试覆盖范围内实现一致;它不能替代 Windows 特殊路径测试、中央目录资源攻击测试或真实断电/文件锁故障注入。
## 交叉复核裁定(定稿)
> 复核视角:Claude 全栈开发工程师,对 Codex 二次复核逐条核验后裁定。
> 核验方式:重走 Windows 路径逃逸链;对照 `extractor.go`(`path.Clean`/`filepath.Join`)、`preflight` 条目数检查位置、提取循环 `Sync` 缺失确认技术主张;因 WSL 无 Go,未执行测试。
> 日期:2026-07-16
### 承认原审核错误
原报告"没有找到路径穿越、zip bomb 或崩溃丢数据路径"的表述**过于绝对,应撤回**。Codex 的头号发现(T-102 Windows 路径语义)是**真实缺陷**,已亲自走通逃逸链验证。
### 已确认的事实(可作为后续任务前提)
| 项 | 结论 | 证据 |
| --- | --- | --- |
| Windows dot-space 路径逃逸 | 属实(原审核漏报) | `normalizeArchivePath` 用 POSIX `path.Clean`;`.. `(dot-dot-空格)不被折叠而通过校验;`filepath.Join` 后**无 destination 包含性兜底**;Windows 内核剥除段尾空格/点使 `.. → ..` 解析到父目录。仅在 Windows 触发,恰是产品目标平台 |
| 中央目录 DoS 边界 | 属实 | `preflight` 的 `len(archive.File) > MaxEntries` 检查在 `zip.OpenReader` 解析完中央目录并分配 `archive.File` **之后**;严重性受下载层 Catalog 声明 size(T-301)约束 |
| payload 文件未 Sync | 属实 | 提取循环 `io.Copy` 后直接 `Close`,无 `output.Sync()`;committed 后断电仍可能数据未耐久 |
| base64 CR/LF 容忍 | 属实 | `base64.StdEncoding.Strict()` 只约束 padding 位,仍跳过 `\r`/`\n`,签名文本非唯一表示(洁净性问题,低危) |
| 非法 surrogate 替换 | 属实 | Go `encoding/json` 把非法 surrogate escape 换成 U+FFFD 而非报错,跨实现签名向量须冻结此行为 |
| channel 校验接线 | 已闭合(原 P3 关闭) | `NewClient` 用 `NewLoader(..., parser)` 注册 parser 为 DocumentValidator;`TestClientValidatesBeforeReplacingCache` 覆盖"签名正确但 channel 错误时回退且不 Store" |
### 对 Codex 二次复核的采纳与修正
- **接受** P0(Windows 路径语义)及整改方向:逐段拒绝尾部空格/点、DOS 设备名(`CON/PRN/AUX/NUL/COM1..9/LPT1..9` 及变体)、控制字符与 Windows 禁止字符;ZIP entry / entrypoint / `entry_exe` / app.json / installed-app 路径共用同一 Windows 安全校验器;在真实 Win7/10/11 文件系统上测。**补充:** 提取时对 `filepath.Join` 结果再加一道"仍在 destination 内"的包含性兜底检查作为纵深防御,成本极低。
- **接受** P1 扩展(payload 文件 + 目录 rename + journal 的完整断电耐久顺序)、P2(中央目录预扫描上限)。
- **接受并收回原建议** P4:原报告"把压缩比降为软信号"过松;保留条目数/单条/总量/压缩比硬边界,按真实包采样调阈值,例外走可信发布策略,不由未验证包内容自行放宽。Codex 的反驳成立。
- **补充威胁模型语境(Codex 遗漏,影响严重性校准而非是否修)**:提取器运行在 **Catalog 签名 + SHA-256 已验证之后**(extractor.go 注释自述)。因此路径逃逸与中央目录 DoS 的**即时可利用性**需要"恶意/被攻陷发布者或签名私钥泄露",非任意 MITM。**"必须修"的结论不变**——声称拒绝穿越的提取器在目标平台未做到即为 must-fix,且是签名管线的纵深防御;但风险定性应为**"签名内容管线的纵深防御缺口"**,而非开放 RCE。P0 阻断 T-302 的定级成立。
### 最终处理顺序(定稿)
1. **[阻断 T-302 · 最高 · T-605]** 修 Windows 尾部空格/点、DOS 设备名与路径别名:建逐段 Windows 安全路径校验器(各层共用)+ 提取时 destination 包含性兜底检查;补 Win7/10/11 真实文件系统用例。关闭前不得把 Extractor 描述为"无路径穿越"。
2. **[已关闭 · T-612]** 已在构造 `zip.Reader` 前增加同句柄包大小与中央目录/EOCD(含 ZIP64)预扫描边界;Extractor 强制接收 expected Catalog `size`,核对打开文件长度并限制原始包、中央目录与声明条目数,不只依赖 `len(archive.File)`。T-302 仍须把已验签 Catalog、完成下载文件和 SHA-256 编排为真实安装调用链。
3. **[代码顺序已关闭 · T-613;发布前环境验证仍必做]** payload 文件 Sync/Close → staging 子目录、根与父目录元数据栅栏 → journal 临时文件 Sync/Close 与 root 栅栏 → 每次 rename 后 root 栅栏 → committed 耐久后才删 backup/journal。Windows 已使用 Win7 可用的目录句柄 `CreateFile(FILE_FLAG_BACKUP_SEMANTICS)` + `FlushFileBuffers`,失败 fail closed;fake 覆盖 payload/tree/journal/rename/rollback/recovery/cleanup 失败,且原生 Windows 用例验证目录 fence 可执行。T-302/T-601 仍必须在 VM/真机进行真实断电、文件锁/杀毒干扰注入,不能以单元测试替代。
4. **[客户端协议已关闭 · T-614;外部发布端验证待协调]** 已冻结静态 corpus 与客户端 canonicalization/签名拒绝规则:Unicode 键序、转义、`<>&`、U+2028/2029、合法/非法 surrogate、`-0`/大整数/嵌套 signature、Base64 padding/CR/LF/空白、同语义不同表示同 signing bytes。`softbox-catalog` 外部仓库仍必须消费相同 corpus 并提供独立 CI 证据;本仓库没有其源码,不能将客户端回归测试表述为发布端集成已完成。
5. **[整合时复核]** 按真实包采样调解压硬上限,保留硬边界,不降级为软信号。
6. **[实现时重点审]** T-402/T-403 真实健康检查:防"进程短暂启动即判健康"、错误工作目录/错误二进制被探活。
7. **[低优先]** 固定解压落盘 mode,可执行入口由已验证 app.json/Catalog 定义。
> 裁定:Codex 二次复核成立且纠正了原审核的绝对化表述与一处过松建议;M2 调整为"有条件成立"(T-101/T-103 原型目标成立;T-102 须先关闭 Windows 路径语义缺口才能作为正式安装链路可信基线)。上述顺序为双方交叉复核后的共识。
### T-612 完成记录(2026-07-18)
- `Extractor.ExtractFile` 改为显式接收 `expectedPackageSize`,先打开同一普通文件,以其真实长度核对 Catalog 值和 4 GiB 原始包上限,再以该句柄构造 `zip.NewReader`;没有“预扫 path 后重开 path”的替换窗口。
- 新的 EOCD 预扫描只读取最多 65,557 字节尾部和固定 ZIP64 locator/end 记录,拒绝跨盘、截断、offset/size 溢出或不一致结构,在标准库解析中央目录前限制 64 MiB 中央目录及 10,000 个声明条目。现有完整 `preflight` 保留为第二道 ZIP 语义和展开数据防线。
- installer 回归覆盖 size 不一致、原始包超限、经典 EOCD 条目/目录伪造、跨盘/截断、ZIP64 有效和损坏 locator/end、非普通输入与 staging 未创建;`GOWORK=off go vet ./installer` 和 `go test -count=10 ./installer` 均通过。
- 关闭的是 Extractor 边界,不是 T-302 生产安装整合或真实下载信任链;下一项断电耐久性整改仍独立阻断 T-302。
### T-613 完成记录(2026-07-18)
- `Extractor` 在每个 payload 的 CRC/长度检查后同步并关闭文件,随后从 staging 子目录到根、再到 staging 父目录执行栅栏;任一失败删除本次 staging,不返回可安装结果。
- transaction、Switcher、rollback、Recovery 和受控删除统一经内部耐久接口:journal 临时文件先同步内容,所有 journal/目录 rename 或删除后同步 app root,未完成栅栏不得推进测试步骤或下一 phase。清理路径同时保留 `ErrRecoveryRequired` 与底层耐久错误,以便调用方可判定恢复状态。
- Windows 原生目录测试已验证读写目录句柄 `CreateFile(FILE_FLAG_BACKUP_SEMANTICS)` + `FlushFileBuffers`;Go 1.20 标准 `syscall` 路径不引入 Win10+ API 或第三方依赖。fake 覆盖 payload/tree、journal、switch、rollback/recovery 与 committed 清理的错误传播和后续 Recover 不变式。
- 关闭的是代码中的耐久顺序与 fail-closed 策略;T-302/T-601 的 Windows VM/真机物理断电、文件锁与杀毒软件故障注入仍为发布前环境验证,不宣称当前单元测试提供硬件级保证。
### T-614 完成记录(2026-07-18)
- 新增 `testdata/catalog/canonical-vectors.json` v1:公开 RFC 8032 测试公钥、固定原始 document、固定 canonical signing bytes 和 Ed25519 signature 均为静态数据。测试先比较 bytes,再用静态公钥验证静态 signature,不调用客户端 canonicalizer 或运行时私钥生成期望值。
- corpus 覆盖 Unicode 键排序/转义、`<>&`、U+2028/U+2029、合法 pair 和三类非法 surrogate、`-0`、大整数、嵌套 `signature`、标准 padding、CR/LF/space/tab、padding 缺失/额外以及两种语义相同 JSON 表示。
- 客户端现在在 JSON decoder 前拒绝孤立/不匹配 surrogate,拒绝 `-0`;Verifier 和 Parser 共享标准 padded Base64 的 decode→reencode 相等检查。关闭的是客户端协议契约;外部 `softbox-catalog` 消费 corpus 的实现/CI 证据仍需跨仓库协调。