Restrict server-side remote import targets - #508
Conversation
|
PR Title: Restrict server-side remote import targets Commit: 本次变更为服务端远程包导入（remote package import）新增 SSRF 防护。 改动内容：
总体评估：设计合理，归档下载路径采用“URL 校验 + 拨号层直连 IP + 重定向复检”的纵深防御，DNS rebinding 防护到位。主要问题集中在 git 路径：git 由自身二进制解析建连，校验与建连之间存在 DNS rebinding/TOCTOU 缝隙；此外归档下载无响应体大小上限（迁移时原样保留）可能造成磁盘耗尽；在配置出站代理的环境下，克隆传输层保留的 ProxyFromEnvironment 会让拨号层校验代理而非目标，构成 SSRF 绕过。共提交 3 条发现。 |
![monkeyscan[bot]](https://avatars.githubusercontent.com/u/231811742?s=60&v=4){kind=small}
| if err != nil { | ||
| return err | ||
| } | ||
| _, copyErr := io.Copy(out, resp.Body) |
There was a problem hiding this comment.
远程归档下载无响应体大小上限，存在磁盘耗尽风险
downloadRemoteArchive 使用 io.Copy(out, resp.Body) 将响应体无任何大小上限地写入磁盘（remote_security.go 第 173 行）。服务端导入的 source 是用户可控的，而校验器只限制目标地址，不限制响应体大小。任意公网 URL 可以在 remoteImportTimeout（10 分钟）内持续返回数据，导致磁盘被写满；这是把远程导入作为服务端功能的可用性/稳定性风险。该函数从 importer.go 迁移到新模块时原样保留了无上限拷贝，本次 SSRF 加固并未覆盖这一点。
Problem code:
Changed code at internal/packageimport/remote_security.go:173
Recommendation:
对下载响应体设置大小上限：使用 io.LimitReader 包裹 resp.Body（例如基于 Content-Length 或固定上限，超出即中止并清理已写文件），并在后续解压/展开阶段同样限制总大小与条目数量，防止磁盘耗尽。
Suggested diff:
if _, copyErr := io.Copy(out, io.LimitReader(resp.Body, maxRemoteArchiveBytes)); copyErr != nil {
out.Close()
os.Remove(artifactPath)
return fmt.Errorf("download remote package %q: %w", redactedRemoteArchiveSource(source), copyErr)
}
|
PR Title: Restrict server-side remote import targets Commit: 本次改动是对远程包导入的 SSRF 防护加固,目标有三:(1) 修复归档下载在配置了 HTTP(S)_PROXY 环境变量时,请求经外部代理转发从而绕过目标校验的历史问题 —— 通过在 newRemoteHTTPClient 中设置 transport.Proxy = nil;(2) 为 git 子进程引入 validatedGitProxy:当 Importer.RemoteTargetValidator 非空时,在 127.0.0.1 起一个只接受 CONNECT 的本地代理,并把 -c http.proxy 注入 git,使 git 的 HTTPS 抓取也必须经过拨号前的目标校验(dialValidatedRemote 复用 DefaultRemoteTargetValidator),同时保留原始主机名以便 TLS 证书校验;(3) 新增一个针对 HTTP client 关闭 ambient proxy 的单元测试。 整体设计思路合理:归档路径用“禁用代理 + 拨号时校验”,git 路径用“本地 CONNECT 代理 + 拨号时校验”,两者都在拨号层拦截内网/特殊地址并保留主机名校验。审查中发现以下需要关注的问题:(a) validatedGitProxy 手工拼写的 HTTP 状态行字符串疑似把 CRLF 过度转义成字面 \r\n 文本,会使 git/libcurl 无法解析 CONNECT 应答(无测试覆盖该路径,难以被发现);(b) 在“必须经出站代理才能上网”的网络里,禁用 ambient proxy 且本地代理直连目标会直接导致远程导入不可用,属于相对此前的行为回归;(c) 安全关键的 CONNECT 代理/拨号校验逻辑没有任何直接测试。总体评估:方向正确,但代理实现缺少真实 git 集成验证,且对出站代理环境的兼容性需要明确权衡。 |
| } | ||
| // Do not let HTTP(S)_PROXY redirect a validated request to an | ||
| // unvalidated destination through an external proxy. | ||
| transport.Proxy = nil |
There was a problem hiding this comment.
强制出站代理环境下,禁用 ambient proxy 与 git 本地代理直连会导致远程导入不可用(行为回归)
本次改动为了堵住“经外部出站代理转发从而绕过 SSRF 校验”的漏洞,archive 路径直接把 transport.Proxy 置为 nil(newRemoteHTTPClient),git 路径则把流量引到本地 validatedGitProxy 后由 dialValidatedRemote 用 net.Dialer 直连目标。两者都完全绕过了进程环境里的 HTTP_PROXY/HTTPS_PROXY。改动前,如果部署环境把这些环境变量指向一个强制的出站代理(例如仅允许经代理上外网、防火墙封锁直连的企业网络),归档下载和 git fetch 都能通过该代理正常工作;改动后这类环境下 daemon 会尝试直连公网而被防火墙拦截,远程导入(归档和 git)将整体不可用。换言之,修复 SSRF 的方式把原本“经代理可用”的环境变成了“完全不可用”,这是相对基线的行为回归。
Problem code:
Changed code at internal/packageimport/remote_security.go:113-115
Recommendation:
在必须使用出站代理的网络模型中,应让本地代理(及 HTTP 传输)在保留“目标主机名校验/私网解析拦截”的同时,仍然把实际连接经由配置的出站代理转发(即校验目标后再连代理),而不是直接置空 Proxy/直连;至少应在文档或配置中明确该安全模式与强制出站代理环境不兼容,并评估是否需要对这类环境提供链式代理支持。
| } | ||
| } | ||
| return nil, fmt.Errorf("unable to connect to allowed address for %s", host) | ||
| } |
There was a problem hiding this comment.
validatedGitProxy/dialValidatedRemote 安全关键逻辑没有任何直接测试覆盖
本次改动新增的安全核心是 validatedGitProxy/startValidatedGitProxy/serve/handle/dialValidatedRemote(约 90 行手写 CONNECT 代理:解析 HTTP 请求、手工拼 405/403/200 应答、拨号前校验、双向 io.Copy 隧道),但新增测试只有 TestRemoteHTTPClientDisablesAmbientProxy,仅仅断言 http.Client.Transport.Proxy 为 nil;既没有覆盖 CONNECT 成功/403 拒绝私有目标/405 拒绝非 CONNECT 请求,也没有断言 git 子进程真的通过 http.proxy 走本地代理、更没有任何真实 git fetch 的端到端用例。现有 TestPrepareGitSourceRejectsPrivateRemoteBeforeGitFetch 在启动 git fetch 前就因私有地址被初始校验拒绝,完全不会触达代理应答与隧道逻辑,因此这套代理实现处于“无任何有效测试”的状态。手写 HTTP 状态行、CONNECT 解析与隧道生命周期都属于容易出错且一旦出错会让整个安全模式失效或导入挂起的代码,回归风险很高(例如状态行转义错误、Host 解析、半关闭语义等)。
Problem code:
Changed code at internal/packageimport/remote_security.go:151-245
Recommendation:
为 validatedGitProxy 补充单元/集成测试:1) 用本地 HTTP server 作为目标,验证 CONNECT 到公网目标返回 200 并能双向转发数据;2) 验证 CONNECT 到 127.0.0.1/169.254.169.254 等私有目标返回 403;3) 验证非 CONNECT 方法返回 405;4) 在 prepareGitSource 中用一个本地假 git 服务器验证 git fetch 实际经由代理并成功/按策略失败。
|
PR Title: Restrict server-side remote import targets Commit: 本提交是对 validatedGitProxy 的小型定向修复(remote_security.go +10/-5、git_source.go +1/-1):此前 prepareGitSource 在配置了自定义 i.RemoteTargetValidator 时虽会启动 validatedGitProxy,但代理内部的 CONNECT 校验一直硬编码使用 DefaultRemoteTargetValidator,导致 daemon 配置的 host/IP 网络策略在 git fetch 的拨号阶段并未生效。本次改动把 validate 函数注入 proxy 结构体,并由 handle → dialValidatedRemote 在 CONNECT 时调用自定义校验器,同时保留了 validate==nil 时回退到 DefaultRemoteTargetValidator 的防御。 整体评估:改动方向正确且范围克制。git 源只允许 https(parseGitSource/classifySource 限制),拨号阶段的 isForbiddenRemoteIP 过滤无条件保留,因此私有 IP 的 SSRF 防护未被削弱;默认配置(校验器即 DefaultRemoteTargetValidator)行为等价,无回归。主要风险点在于:CONNECT 阶段无法携带原始 URL 的 path/凭据,只能用 "https://host:port\" 重新合成入参调用自定义校验器,与 prepareGitSource 初始校验传入完整 CredentialURL 的既有约定不一致,可能影响路径敏感的校验策略,已作为一条 medium 置信度的 functional correctness finding 提交。既有的 4 条历史 finding(CRLF 应答转义、代理无测试、禁用 ambient proxy 的行为回归、归档无响应体大小上限)均为本提交未触及的存量问题,不予重复提交。 |
| if validate == nil { | ||
| validate = DefaultRemoteTargetValidator | ||
| } | ||
| if err := validate(ctx, "https://"+net.JoinHostPort(host, port)); err != nil { |
There was a problem hiding this comment.
代理 CONNECT 阶段改用自定义 RemoteTargetValidator,但入参是丢失 path/凭据的合成 URL,与初始校验约定不一致
本提交将 validatedGitProxy 的 CONNECT 校验从固定 DefaultRemoteTargetValidator 改为调用 i.RemoteTargetValidator(prepareGitSource 现在把自定义校验器传入 startValidatedGitProxy),dialValidatedRemote 第 228-231 行以 "https://"+net.JoinHostPort(host,port) 合成 URL 调用该校验器。问题在于:CONNECT 请求本身只携带 host:port,合成 URL 丢失了原始仓库 URL 的 path(以及 userinfo 凭据),而同一校验器在 prepareGitSource 中是用完整 src.CredentialURL(含 path,例如 https://github.com/octocat/hello.git)调用的。二者入参语义不一致:若 daemon 配置的自定义校验器是按完整 URL/path 前缀做放行(如只允许 https://github.com/octocat/*),或对 URL 做精确匹配/端口归一化,则初始校验通过后 git fetch 的每一个 CONNECT 都会被该校验器以 "https://host:port" 拒绝,代理返回 403,导致配置了自定义校验策略的部署中 git 导入整体不可用。本提交正是为'自定义校验器生效于 git 拨号路径'而做,因此受影响的就是这类部署。该路径在 remote_security_test.go 中没有任何直接测试覆盖(现有 TestPrepareGitSourceRejectsPrivateRemoteBeforeGitFetch 在 git fetch 前即被拒绝,不会触达 CONNECT 校验)。
Problem code:
Changed code at internal/packageimport/remote_security.go:228-231
Recommendation:
明确并统一 RemoteTargetValidator 在代理阶段的入参约定:由于 CONNECT 只能暴露 host:port,无法还原原始 path,若网络策略应仅基于 host/IP,则代理阶段校验应使用与拨号层 isForbiddenRemoteIP 一致的 host 级语义,path 级策略留在 prepareGitSource 的完整 URL 校验即可;同时应在 Importer.RemoteTargetValidator 的文档注释中说明其可能收到 "https://host:port" 形式的合成 URL,避免调用方假设始终是完整 URL。建议补充一个端到端测试:配置一个仅允许特定 host 的自定义 validator,断言 git fetch 能成功且对该 host 之外的 CONNECT 返回 403,以固定本次改动引入的新行为。
|
Follow-up fixes after security review:
Verification: remote-target and Git validation tests pass. |
|
PR Title: Restrict server-side remote import targets Commit: 本次改动仅涉及 internal/packageimport/remote_security.go 的 validatedGitProxy.handle（+3/-2）。 改动内容：把 http.ReadRequest 的一次性 bufio.NewReader(client) 提取为具名 reader，并在 client→remote 隧道转发时改用 io.Copy(remote, io.MultiReader(reader, client))。 分析与评估：
结论：本提交是一个正确、低风险、无回归的读缓冲修复，未引入新的正确性/安全/并发/可靠性问题，无可新增的 actionable finding。文件内其余历史 confirmed finding（CONNECT 校验入参语义、状态行文本、测试缺失、禁用出站代理、归档无大小上限等）均位于本 diff 未触碰的区域，属既有问题，不重复提交。 |
1 participant
问题
服务端远程归档和 HTTPS Git 导入直接访问用户提供的 URL，未限制私网地址，也未校验重定向目标。
影响
攻击者可借助 Admin 导入接口访问 loopback、RFC1918、链路本地地址或云元数据服务，形成 SSRF，并可能探测或调用内部服务。Git 和 HTTP 重定向还可能绕过只检查初始地址的防护。
修复内容
验证
go test ./internal/packageimport -run 'Test(DefaultRemoteTargetValidator|RemoteHTTPClientRevalidatesRedirects|PrepareGitSourceRejectsPrivateRemote)'通过。go test ./cmd/octobus通过。