Security: require authenticated TLS for public listener - #653
Conversation
|
PR Title: Security: require authenticated TLS for public lis... Commit: 该 PR 对 daemon 的 HTTP 监听进行安全加固：新增 HttpTLSCertFile/HttpTLSKeyFile 配置项；非 loopback 的 HTTP_LISTEN 现在强制要求 AGENT_COMPOSE_AUTH_TOKEN 和 TLS 证书对，并由 daemon 直接以 TLS 提供服务（拒绝暴露明文 h2c）；同时在 serve() 中为每个 http.Server 调用 http2.ConfigureServer 以启用 ALPN HTTP/2。配置校验（pkg/config）与监听逻辑（cmd/agent-compose/cli_daemon.go）均有对应改动，测试和文档同步更新。 整体方向正确（修复“明文 h2c 暴露 + token 可被截获重放”的问题）。但存在三个值得关注的问题：(1) 新 TLS 监听器的 Handler 仍被 h2c.NewHandler 包裹，结合新增的 http2.ConfigureServer，ALPN 协商出的 HTTP/2 请求会被 h2c 处理器当作 prior-knowledge 前缀重新进入 HTTP/2 服务，导致 TLS 下的 HTTP/2（含 Connect 双向流）流量不可用（高）；(2) .env.example 的默认配置（非 loopback HTTP_LISTEN + 空 token + TLS 注释掉）与新的启动校验冲突，照抄示例会直接启动失败（中）；(3) CLI 客户端连接 daemon 的 HTTP 路径仍使用明文 h2c、且无自定义 CA 信任配置，未同步适配非 loopback 的 TLS-only 监听（中）。 |
![monkeyscan[bot]](https://avatars.githubusercontent.com/u/231811742?s=60&v=4){kind=small}
|
已按最新评论修订：
本地 |
|
PR Title: Security: require authenticated TLS for public lis... Commit: 本改动是对此前一轮审查发现的定向修复，核心在 cmd/agent-compose/cli_daemon.go：
新增测试 TestDaemonTLSServerUsesHTTP2AndCustomCA 覆盖 TLS listener 上协商出 HTTP/2.0 + 客户端通过自定义 CA 完成 TLS，恰好能捕获此前两个历史缺陷。 处置结论：两条历史 finding（TLS 上 h2c 双重接管；CLI 未适配 TLS-only 远程 daemon/无法信任私有 CA）在本次修订中均已得到针对性修复，修复方式正确且有集成测试覆盖。经对服务端 TLS 握手/ALPN 派发、客户端 http.Transport 自动 HTTP/2 配置、unix/http/https 三条路径的逐一核对，未发现新引入的高置信度正确性、安全或回归问题。总体评估：改动方向正确、范围聚焦、文档同步，可通过。 |
|
CI 发现上一版修订使用了不存在的
修复提交： |
|
PR Title: Security: require authenticated TLS for public lis... Commit: 本改动对 cmd/agent-compose/cli_daemon.go 中 daemon 监听器的 TLS 判定方式做了一次纯重构：原来由 daemonServers.add 在运行时通过 listener.(*tls.Listener) 类型断言自动推断 usesTLS，现改为在 HTTP_LISTEN 调用点按 tlsConfig != nil 显式选择新增的 addTLS（usesTLS=true）或既有 add（usesTLS=false），两者都委托给新拆出的私有方法 addWithTLS(name,...,usesTLS)。经核查本文件内全部三个调用点（AGENT_COMPOSE_SOCKET unix socket、HTTP_LISTEN TLS、HTTP_LISTEN 明文），改动前后各监听项的最终 usesTLS/tls 取值完全一致：unix socket 仍走 h2c 包裹、item.tls=false；TLS HTTP 监听器不再被 h2c 包裹、item.tls=true 使 serve() 对其实施 http2.ConfigureServer，HTTP/2-over-TLS 经 ALPN 直接进入 a.Echo；明文 HTTP 监听器仍走 h2c.NewHandler。整体为行为保持且提升可读性/健壮性的重构（显式分类不再依赖易碎的动态类型判断），未发现本增量改动引入的新缺陷。两条历史发现属于更早引入 TLS-only 监听等功能的提交，超出本增量 diff 范围，此处不再重复上报。 |
|
已修复最新 CI 的镜像 Smoke 失败：PR 的新启动策略要求非 Loopback listener 配置认证和 TLS，但 Docker image E2E fixture 仍使用镜像默认的 修复内容：
提交： |
|
PR Title: Security: require authenticated TLS for public lis... Commit: 该改动将 e2e 测试（image_docker_lifecycle_test.go）中访问 daemon 的方式从明文 HTTP 切换为 HTTPS，并引入 Bearer Token 认证：新增 imageDockerTLSConfig 结构体与 newImageDockerTLSConfig helper（通过 httptest.NewTLSServer 生成自签证书/私钥并写入临时目录）；startImageDockerStartupDaemon 与 startImageDockerLifecycleDaemon 均把证书/私钥 bind-mount 进容器并通过 HTTP_TLS_CERT_FILE/HTTP_TLS_KEY_FILE/AGENT_COMPOSE_AUTH_TOKEN 环境变量启用 TLS 与 token；baseURL 改为 https；imageDockerHTTPClient 改为信任自签 CA 并通过 imageDockerAuthTransport 为每个请求附加 Authorization 头。整体结构与既有 fixture/cleanup 模式一致，TLS 证书自洽（同一证书同时作为服务端证书与客户端信任根），happy path 逻辑无明显错误；主要问题集中在测试辅助实现方式（无用的监听服务）以及缺少对鉴权负向路径的验证。 |
|
补充修订：将镜像 E2E 的证书挂载目标从可能不存在的 证书仍以只读方式挂载，daemon 仍使用真实 TLS 和 Bearer Token，客户端仍通过临时 CA 验证证书。 |
|
PR Title: Security: require authenticated TLS for public lis... Commit: 本次变更仅涉及 test/e2e/image_docker_lifecycle_test.go，将两个 daemon e2e fixture（startImageDockerStartupDaemon 与 startImageDockerLifecycleDaemon）中 TLS 证书/私钥在容器内的挂载目标路径从 /run/agent-compose/server.crt|.key 调整为 /tmp/agent-compose-server.crt|.key，并同步更新了 HTTP_TLS_CERT_FILE / HTTP_TLS_KEY_FILE 环境变量。变更共 4 个 hunk，全部为成对的环境变量+Mount.Target 修改，测试内部自洽：env 指向的路径与 bind-mount 目标一致，且主机侧源文件由 newImageDockerTLSConfig 提前写入（0600 权限），ContainerCreate 前即存在。未发现残留的旧路径引用。此类文件级 bind-mount 到 /tmp 下在 Docker 中行为正常，读-only 挂载且证书为一次性自签测试证书，不构成安全或功能回归风险。整体评估：低风险、内部一致的测试调整，未发现由该变更新引入的可操作问题。历史已确认发现（缺少鉴权负向断言、httptest TLS 服务仅作证书来源）仍存在，但与本路径变更无关，未予重复提交。 |
|
已处理最新 Review 建议：
提交： |
|
PR Title: Security: require authenticated TLS for public lis... Commit: 本 PR 仅修改 test/e2e/image_docker_lifecycle_test.go，旨在修复两条历史评审意见：
总体评估：改动正确且聚焦。证书自签名且同时充当客户端信任根，Go 的 x509/TLS 验证路径成立（与本改动之前 httptest 自签证书作为信任根的行为一致）；两个 fixture（no-KVM 启动与沙箱生命周期）均设置了 AGENT_COMPOSE_AUTH_TOKEN 及 HTTP_TLS_CERT/KEY_FILE，因此负向断言在两条 e2e 路径中都会实际执行；新证书有效期 1 小时，对分钟级 e2e 足够；文件权限保持 0600，无回归。两条历史 finding 均已基本解决。 仍存在一个较小的测试完备性缺口（见附带 finding）：负向断言只覆盖「缺失 token」，未验证「携带错误/伪造 token」也应被拒绝，因此无法发现 daemon 侧「仅检查 header 存在、不校验 token 值」的回归。 |
|
已补充认证测试覆盖：镜像 daemon 就绪后，现在分别验证：
两种请求都必须返回 提交： |
|
PR Title: Security: require authenticated TLS for public lis... Commit: 本次改动仅涉及 test/e2e/image_docker_lifecycle_test.go 中的 assertImageDockerAuthEnforced 辅助函数。改动将原先"只发送一个不带 Authorization 头的 GET /api/version 请求并断言返回 401/403"的负向鉴权校验，改为在一个循环中对两种凭证形态分别断言：(1) 完全不带 Authorization 头（缺失 token）；(2) 携带错误的 Bearer token（wrong-image-docker-token）。两种场景都期望 daemon 返回 401 或 403。 设计意图清晰：此前确认的历史 finding 0a46ac1a 指出负向断言只覆盖"缺失 token"，无法发现"daemon 仅校验 Authorization 头存在性而不校验 token 值"的回归；本次改动正好补齐了"错误 token 被拒绝"这一负向用例，使 token 值校验这一安全属性获得回归保护，是对既有 finding 的针对性修复，历史 finding 已被本次改动解决。 安全性与正确性核查：failImageDockerFixture 内部调用 t.Fatalf，故 client.Do 返回错误后不会继续执行 resp.Body.Close()，不存在 nil 解引用风险；body 未读取即关闭仅影响连接复用，且请求次数少、客户端短命，不构成资源泄漏。整体改动低风险，未发现功能/安全层面的高置信缺陷。 仅存的次要问题：循环内失败信息仍沿用 "unauthenticated ..." 文案且未附带当前 token 值，导致错误 token 迭代失败时信息失真（实际已携带 Bearer 头却被描述为未认证），并且 CI 失败时无法区分是缺失-token 场景还是错误-token 场景触发了回归，削弱该安全负向断言失败时的可诊断性。建议在失败信息中输出当前 token。已作为低严重度、可维护性类 finding 提交。 |
| _ = resp.Body.Close() | ||
| if resp.StatusCode != http.StatusUnauthorized && resp.StatusCode != http.StatusForbidden { | ||
| failImageDockerFixture(t, fixture, "unauthenticated /api/version status = %d, want 401 or 403", resp.StatusCode) | ||
| } |
There was a problem hiding this comment.
鉴权负向断言循环失败时无法区分两种 token 场景，且错误 token 场景被误标为 unauthenticated
本次改动把单一未认证请求改为循环遍历 ["", "wrong-image-docker-token"] 两种凭证形态，但两处失败路径仍沿用改写前的文案：client.Do 出错时报 "unauthenticated /api/version request: %v"，状态码不符时报 "unauthenticated /api/version status = %d, want 401 or 403"，且均未附带当前 token。由此带来两个具体问题：(1) 在第二个迭代（实际已设置 Authorization: Bearer wrong-image-docker-token）失败时，日志将携带凭证的请求描述为 unauthenticated，信息失真；(2) 若 daemon 鉴权出现回归，导致两个迭代中的某一个返回了非 401/403 状态，CI 输出无法指明是“缺失 token”还是“错误 token”场景触发了失败。考虑到该测试的产物就是失败信号本身，而循环引入前单条消息是准确的，这个可诊断性缺口是本改动直接引入的。
Problem code:
Changed code at test/e2e/image_docker_lifecycle_test.go:536-551
Recommendation:
在失败信息中包含当前 token 值（或至少标识迭代场景），并将文案从 unauthenticated 改为中性的鉴权校验描述，使 CI 失败时能立即定位是哪个负向用例被突破。例如在请求错误与状态码断言处将 token 作为格式化参数传入。
Suggested diff:
--- a/test/e2e/image_docker_lifecycle_test.go
+++ b/test/e2e/image_docker_lifecycle_test.go
@@ -536,22 +536,22 @@ func assertImageDockerAuthEnforced(t *testing.T, ctx context.Context, fixture *i
client := &http.Client{Timeout: 5 * time.Second, Transport: transport}
for _, token := range []string{"", "wrong-image-docker-token"} {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, fixture.baseURL+"/api/version", nil)
if err != nil {
- t.Fatalf("create unauthenticated request: %v", err)
+ t.Fatalf("create auth-enforcement request (token %q): %v", token, err)
}
if token != "" {
req.Header.Set("Authorization", "Bearer "+token)
}
resp, err := client.Do(req)
if err != nil {
- failImageDockerFixture(t, fixture, "unauthenticated /api/version request: %v", err)
+ failImageDockerFixture(t, fixture, "/api/version auth-enforcement (token %q) request: %v", token, err)
}
_ = resp.Body.Close()
if resp.StatusCode != http.StatusUnauthorized && resp.StatusCode != http.StatusForbidden {
- failImageDockerFixture(t, fixture, "unauthenticated /api/version status = %d, want 401 or 403", resp.StatusCode)
+ failImageDockerFixture(t, fixture, "/api/version auth-enforcement (token %q) status = %d, want 401 or 403", token, resp.StatusCode)
}
}
}
1 participant
问题
非 Loopback 的
HTTP_LISTEN过去会启动明文 h2c；当AGENT_COMPOSE_AUTH_TOKEN未配置时，daemon HTTP 控制面还会 fail-open，远程请求可直接访问管理 API。影响
攻击者可以在网络层窃取 Bearer Token，或在无 Token 配置时直接调用控制面 API，进一步执行 Sandbox 命令、修改项目状态和访问敏感数据。涉及 CWE-306、CWE-319。
修复内容
HTTP_LISTEN强制要求AGENT_COMPOSE_AUTH_TOKEN。HTTP_TLS_CERT_FILE和HTTP_TLS_KEY_FILE。验证
go test ./pkg/configgit diff --check说明：完整
go test ./...当前受工作区已有的 Protobuf 生成文件与.proto源码不同步影响，表现为缺少K8SDriverSpec、RunTimeout等生成符号；本 PR 未修改该生成基线。