fix(relay): 限额改为从共享策略推导,修掉信封上限过小导致的合法请求被拒
中继此前自带一份限额常量,并在注释里声称"与共享策略一致",而没有任何东西 校验这句话。其中两条已经漂移成真实缺陷: 1. 信封上限 64 KiB 是**信封**的上限,而信封约为其承载 body 的 1.8 倍(body 先 base64、再 AEAD、再 base64)。于是 body 仍在 agent 的 256 KiB maxRequestBytes 之内的请求,在中继处被拒为 relay_payload_too_large——一个 在局域网上能成功、走中继就失败的操作。响应方向同理:节点返回合法的大 body,应答在中继被拒,手机白等到超时。实测 120 KiB 的 body 就被拒。 2. mailboxDepth 只靠注释与策略"相等",改任一侧都不会有人发现,而这个深度 是 agent 重放缓存容量的依据。 现在 relay/src/limits.js 从 contracts/agent-policy.json 推导全部限额,并在 文件不可读、缺 limits、数值不可用时**拒绝启动**:中继无法区分"诚实的大请求" 与"内存耗尽尝试",上限是它唯一的工具,凭空发明一个等于在强制一个没人选过的 数字。推导结果:信封上限 513037 字节、深度 8、请求期限 30s、轮询窗口 24s。 策略摘要在健康检查里公开并逐节点对比:/healthz 返回中继自己那份的摘要,以及 每个在线节点上报的摘要与 policyMatches。它只上报不强制——中继读不到节点上的 文件,声称不一致就拒服务会把"节点跑着旧二进制"变成一次故障;而策略本来就是 agent 在强制。没有上报的节点显示 null 而不是 false,"没说"与"说了不一样的" 是两个不同的发现。 顺带修掉一个只在 sudo 配对时才暴露的缺陷:config.Save 用临时文件加重命名, 在 root 下会把配置文件变成 root 所有,而 systemd 单元以 msa-agent 用户运行, 于是配对看起来成功、节点重启后读不到自己的配置。现在重命名前保留原文件的 uid/gid,且无法保留时报错而非警告。 测试新增 test/limits.test.js(11 项),分三层:推导与策略文件一致、推导出的 上限确实容得下用真实 wire 代码构造的最坏信封(这是旧上限失败的那次测量)、 以及经真实 HTTP 面把合法大请求送过去。另有健康检查的摘要上报与 null 语义。 四处断言都做了变异验证:把上限改回 64 KiB,四项测试失败。 Node 93 项、Go 全包、Swift 99 项、端到端全部通过。
This commit is contained in:
@@ -14,6 +14,7 @@ import (
|
||||
"fmt"
|
||||
"os"
|
||||
"regexp"
|
||||
"syscall"
|
||||
)
|
||||
|
||||
var nodePattern = regexp.MustCompile(`^nd_[A-Za-z0-9_-]{8,64}$`)
|
||||
@@ -150,11 +151,19 @@ func loadFile(path string) (File, error) {
|
||||
return cfg, nil
|
||||
}
|
||||
|
||||
// Save writes the config back with 0600, atomically.
|
||||
// Save writes the config back with 0600, atomically, preserving the file's owner.
|
||||
//
|
||||
// Atomic (temp file + rename) because the pairing flow writes the device list at moments when the
|
||||
// agent may be concurrently read by systemd restarts: a half-written config is a config that fails
|
||||
// to load, which turns a completed pairing into an uncommandable node.
|
||||
//
|
||||
// Ownership is preserved because the deployment document tells the operator to run `--pair` with
|
||||
// sudo, and the installed config belongs to the `msa-agent` user. Writing a temp file as root and
|
||||
// renaming it over the config would leave it root-owned 0600, and the systemd unit — which runs as
|
||||
// `msa-agent` — could then no longer read it: the pairing would appear to succeed and the node would
|
||||
// come up unable to load its own configuration. The chown is only attempted when the process can
|
||||
// perform it (root), and a failure is an error rather than a warning, because silently leaving the
|
||||
// file unreadable by the service is the failure this is here to prevent.
|
||||
func Save(path string, cfg File) error {
|
||||
if err := cfg.Validate(true); err != nil {
|
||||
return fmt.Errorf("config: refusing to save an invalid file: %w", err)
|
||||
@@ -164,13 +173,37 @@ func Save(path string, cfg File) error {
|
||||
return err
|
||||
}
|
||||
encoded = append(encoded, '\n')
|
||||
uid, gid, hadOwner := ownerOf(path)
|
||||
temp := path + ".tmp"
|
||||
if err := os.WriteFile(temp, encoded, 0o600); err != nil {
|
||||
return err
|
||||
}
|
||||
if hadOwner && os.Geteuid() == 0 {
|
||||
if err := os.Chown(temp, uid, gid); err != nil {
|
||||
// The temp file is removed so a failed save leaves no stray copy of the config — it
|
||||
// holds the agent's private keys and the node password.
|
||||
os.Remove(temp)
|
||||
return fmt.Errorf("config: preserving ownership of %s: %w", path, err)
|
||||
}
|
||||
}
|
||||
return os.Rename(temp, path)
|
||||
}
|
||||
|
||||
// ownerOf reports the uid and gid a rewrite of path must keep. ok is false when the file does not
|
||||
// exist yet, which is the first-write case: there is no owner to preserve, and the file is created
|
||||
// by whoever is running.
|
||||
func ownerOf(path string) (uid, gid int, ok bool) {
|
||||
info, err := os.Stat(path)
|
||||
if err != nil {
|
||||
return 0, 0, false
|
||||
}
|
||||
stat, ok := info.Sys().(*syscall.Stat_t)
|
||||
if !ok {
|
||||
return 0, 0, false
|
||||
}
|
||||
return int(stat.Uid), int(stat.Gid), true
|
||||
}
|
||||
|
||||
// NewRelayToken mints a device bearer for the relay: 24 random bytes, hex. Hex because the relay
|
||||
// only requires a bearer string and hex never contains a character that needs escaping when it
|
||||
// travels through JSON or headers.
|
||||
|
||||
@@ -1,8 +1,11 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"syscall"
|
||||
"testing"
|
||||
)
|
||||
|
||||
@@ -66,6 +69,45 @@ func TestSaveRoundTripsAndIsSixHundred(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A rewrite must keep the file's owner. The deployment document tells the operator to run --pair
|
||||
// with sudo while the installed config belongs to the msa-agent user; without preserving ownership
|
||||
// the rename would leave a root-owned 0600 file that the systemd unit cannot read, so the pairing
|
||||
// would look successful and the node would come up unable to load its own configuration.
|
||||
//
|
||||
// The test cannot become root, so it asserts the property that is observable without privileges:
|
||||
// the owner after a save is the same as the owner before it, and the temp file does not survive.
|
||||
func TestSavePreservesTheFileOwner(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "config.json")
|
||||
cfg := valid()
|
||||
if err := Save(path, cfg); err != nil {
|
||||
t.Fatalf("first Save: %v", err)
|
||||
}
|
||||
before := owner(t, path)
|
||||
// A second save goes through the ownership-preserving path. The inode necessarily changes — the
|
||||
// write is a temp file plus a rename — so what is compared is the owner, which is the thing the
|
||||
// rename would otherwise reset.
|
||||
cfg.Devices = append(cfg.Devices, Device{
|
||||
ID: "ap_second0001", SignKey: cfg.Devices[0].SignKey, RelayToken: strings.Repeat("b", 48),
|
||||
})
|
||||
if err := Save(path, cfg); err != nil {
|
||||
t.Fatalf("second Save: %v", err)
|
||||
}
|
||||
after := owner(t, path)
|
||||
if before != after {
|
||||
t.Fatalf("the rewrite changed the owner from %v to %v", before, after)
|
||||
}
|
||||
if _, err := os.Stat(path + ".tmp"); !os.IsNotExist(err) {
|
||||
t.Fatal("a temp file was left behind")
|
||||
}
|
||||
reloaded, err := Load(path)
|
||||
if err != nil {
|
||||
t.Fatalf("Load after rewrite: %v", err)
|
||||
}
|
||||
if len(reloaded.Devices) != 2 {
|
||||
t.Fatalf("the rewrite lost a device: %d", len(reloaded.Devices))
|
||||
}
|
||||
}
|
||||
|
||||
// A save must not be able to write a file that the next boot would refuse: the validation runs
|
||||
// before the rename, so a caller building a device list with a bad id gets an error, not a
|
||||
// corrupted file.
|
||||
@@ -138,3 +180,17 @@ func TestLoadReportsMissingAndMalformedFiles(t *testing.T) {
|
||||
t.Fatal("a malformed file was accepted")
|
||||
}
|
||||
}
|
||||
|
||||
// owner reads the uid and gid of a path, which is what a rewrite must preserve.
|
||||
func owner(t *testing.T, path string) string {
|
||||
t.Helper()
|
||||
info, err := os.Stat(path)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
stat, ok := info.Sys().(*syscall.Stat_t)
|
||||
if !ok {
|
||||
t.Skip("stat_t is not available on this platform")
|
||||
}
|
||||
return fmt.Sprintf("%d:%d", stat.Uid, stat.Gid)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user