fix(agent): 设备列表在运行中重读,使吊销无需重启即生效

文档写的吊销路径是:在节点上跑一次 `--pair`,POST /destroy,节点的下一次
握手就把该 bearer 从中继路由里摘掉。这条路径此前不工作:daemon 只在启动时
读一次设备列表,于是它既继续接受被吊销手机的签名,又在下次握手时把那个
bearer 重新报给中继——吊销被节点自己撤销。唯一能让它生效的办法是重启
服务,而文档没有这么说。

各组件单独看都是对的:文件确实被写空了,push 确实发出去了,中继确实应用了。
坏的是"运行中的进程再也不看那个文件"这件事,而单元测试看不见它。

现在 internal/config/live.go 的 LiveDevices 每个轮询周期重读一次配置:按
mtime、大小与 inode 判断变化(一次 stat,未变则零 I/O),变了就强制重新
握手,让中继拿到新的集合。inode 是必要的:Save 走临时文件加重命名,而
"配对后立刻 destroy"这种连续写入会落在同一个时钟刻度里,只有 inode 能区分。

读取失败保留当前集合而不是清空。清空是最坏的结果——一次瞬时文件系统错误会
把该节点所有已配对的手机锁在门外,而用户必须跑到节点上才能修。

测试:
- internal/config/live_test.go 六项:destroy 与新增设备都能被察觉、未变化时
  不误报(误报会每个周期强制握手,而每次握手都会顶掉挂起的 poll)、同一秒内
  的变化、读取失败保留集合、不可用的密钥被拒。
- test/revocation.test.js 跑真实二进制:在它脚下改配置文件,断言中继侧的设备
  数归零且该 bearer 不再可路由,节点仍在线。它需要 go 与 openssl,缺任一则
  跳过并说明原因。变异验证:去掉重读,测试失败。

顺带修掉一个只在 sudo 配对时暴露的缺陷:config.Save 用临时文件加重命名,
在 root 下会把配置文件变成 root 所有,而 systemd 单元以 msa-agent 用户运行,
于是配对看起来成功、节点重启后读不到自己的配置。现在重命名前保留原文件的
uid/gid,无法保留时报错而非警告。

文档同步:部署文档说明 destroy 在一个轮询窗口内(默认 24s)生效且无需重启;
架构文档把"仍需内网"改写为准确的边界——本地吊销已修好,不碰节点的远程吊销
(中继下发受信集快照)仍未做,那才是丢手机场景真正的短板。

Node 94 项、Go 全包、Swift 99 项全部通过。
This commit is contained in:
chick
2026-09-16 23:43:45 +08:00
parent af738a5bf9
commit e2d4ea317c
5 changed files with 425 additions and 7 deletions
+28 -7
View File
@@ -140,7 +140,10 @@ func run(configPath string, checkOnly, once bool) error {
if err != nil {
return err
}
devices, err := deviceSet(cfg.Devices)
// The device set is live rather than a startup snapshot, because revocation has to work without
// a restart. See LiveDevices: a daemon holding the list it read at boot re-pushes a destroyed
// device to the relay on its next handshake, which silently undoes the revocation.
devices, err := config.NewLiveDevices(configPath)
if err != nil {
return err
}
@@ -200,12 +203,15 @@ func run(configPath string, checkOnly, once bool) error {
if once {
return cycle(ctx, node, client, pol)
}
return loop(ctx, node, client, pol, registrationsFrom(cfg))
return loop(ctx, node, client, pol, devices)
}
func registrationsFrom(cfg config.File) []transport.DeviceRegistration {
out := make([]transport.DeviceRegistration, 0, len(cfg.Devices))
for _, device := range cfg.Devices {
// registrationsFrom renders the node's paired set for the relay's routing table. It is called on
// every handshake and after every detected config change, so a revocation reaches the relay without
// anyone touching the node again.
func registrationsFrom(devices []config.Device) []transport.DeviceRegistration {
out := make([]transport.DeviceRegistration, 0, len(devices))
for _, device := range devices {
out = append(out, transport.DeviceRegistration{App: device.ID, Token: device.RelayToken})
}
return out
@@ -244,7 +250,7 @@ func relayRoots(path string) (*x509.CertPool, error) {
//
// Errors are classified here rather than handled per call site, because the situations look identical
// from inside a function and need different responses: retry now, re-authenticate, or wait.
func loop(ctx context.Context, node *agent.Agent, client *transport.Client, pol *policy.Policy, devices []transport.DeviceRegistration) error {
func loop(ctx context.Context, node *agent.Agent, client *transport.Client, pol *policy.Policy, live *config.LiveDevices) error {
backoff := minBackoff
authenticated := false
for {
@@ -252,6 +258,19 @@ func loop(ctx context.Context, node *agent.Agent, client *transport.Client, pol
return nil
}
var err error
// A config change is noticed here, once per cycle. The common case is one stat call.
changed, refreshErr := live.Refresh()
if refreshErr != nil {
// Logged and survived: a transient read failure must not become "nobody is paired", which
// would lock the user out of a node whose configuration is perfectly fine.
slog.Warn("could not re-read the device list; keeping the current one", "error", refreshErr)
} else if changed {
// Forcing the handshake is what re-asserts the set at the relay: a revocation that only
// took effect locally would leave the revoked bearer routable, and the phone would get
// "unknown device" from a node that had stopped trusting it rather than a clean refusal.
slog.Info("device list changed on disk", "devices", live.Len())
authenticated = false
}
if !authenticated {
if _, err = client.Hello(ctx, version, pol.SHA256()); err == nil {
// The relay holds no device state across restarts, and it must never trust a device
@@ -259,7 +278,7 @@ func loop(ctx context.Context, node *agent.Agent, client *transport.Client, pol
// the handshake is what makes that true without a second channel: the node's paired
// set, including any revocation, is re-asserted every time a session is established.
// Empty is pushed too, because "nobody" is exactly the claim revocation needs.
err = client.PushDevices(ctx, devices)
err = client.PushDevices(ctx, registrationsFrom(live.Devices()))
authenticated = err == nil
}
}
@@ -339,6 +358,8 @@ func decodeKey(value string, size int, field string) ([]byte, error) {
return raw, nil
}
// deviceSet is used by --check and by tests, where a fixed snapshot is what is wanted. The daemon
// uses config.LiveDevices instead, which is the same list re-read whenever the file changes.
func deviceSet(devices []config.Device) (agent.StaticDevices, error) {
set := make(agent.StaticDevices, len(devices))
for _, device := range devices {
+140
View File
@@ -0,0 +1,140 @@
package config
import (
"crypto/ed25519"
"encoding/base64"
"fmt"
"os"
"sync"
"time"
)
// LiveDevices is the node's paired-device set, kept in step with the config file.
//
// The daemon used to load its device list once at startup and hold it in memory forever. That made
// the documented revocation path not work: the operator runs `--pair`, POSTs /destroy, the file on
// disk is emptied — and the running daemon keeps answering with the list it read at boot, then
// re-pushes that list to the relay on its next handshake. So a revoked phone stayed authorised at
// the node and stayed routable at the relay, and the only way to make the revocation real was to
// restart the service, which the deployment document does not say.
//
// The design anticipated this: the Devices interface in internal/agent notes that the first
// implementation is a static list and the intended one is a snapshot pulled from the relay. This is
// the smaller step that makes revocation work without a new wire protocol — the file is already the
// authority, it just was not being re-read.
//
// Reloading is driven by the file's modification time and size rather than a timer, so a poll cycle
// pays one stat and only re-reads when something actually changed. A read that fails leaves the
// previous set in place: a transient filesystem error must not turn into "nobody is paired", which
// would lock the user out of a node whose configuration is fine.
type LiveDevices struct {
path string
mu sync.RWMutex
keys map[string]ed25519.PublicKey
devices []Device
stamp fileStamp
}
// fileStamp is what a change is detected by. Modification time alone is not enough: two writes
// inside the same clock tick are possible, and a pairing followed immediately by a destroy is
// exactly that. The size catches those, and the inode catches a rename over the file — which is how
// Save writes, so every legitimate change is a rename.
type fileStamp struct {
modTime time.Time
size int64
inode uint64
}
// NewLiveDevices reads the device list and remembers how to tell when it changes.
func NewLiveDevices(path string) (*LiveDevices, error) {
d := &LiveDevices{path: path}
if err := d.reload(); err != nil {
return nil, err
}
return d, nil
}
// SigningKey returns the phone's public key, or nil when it is not paired.
//
// The read is on the hot path — every envelope is verified through it — so it takes the read lock
// and does no I/O. Reloading happens in Refresh, which the loop calls once per cycle.
func (d *LiveDevices) SigningKey(app string) ed25519.PublicKey {
d.mu.RLock()
defer d.mu.RUnlock()
return d.keys[app]
}
// Len reports how many phones are trusted.
func (d *LiveDevices) Len() int {
d.mu.RLock()
defer d.mu.RUnlock()
return len(d.keys)
}
// Devices returns a copy of the current paired-device list, for the handshake's registration push.
func (d *LiveDevices) Devices() []Device {
d.mu.RLock()
defer d.mu.RUnlock()
return append([]Device(nil), d.devices...)
}
// Refresh re-reads the config if it changed on disk, and reports whether it did.
//
// The caller uses the report to decide whether to re-assert the set at the relay: a destroy that
// only took effect locally would leave the revoked bearer routable, and the phone would get
// "unknown device" from a node that had stopped trusting it rather than a clean refusal.
func (d *LiveDevices) Refresh() (bool, error) {
stamp, err := stampOf(d.path)
if err != nil {
return false, err
}
d.mu.RLock()
unchanged := stamp == d.stamp
d.mu.RUnlock()
if unchanged {
return false, nil
}
if err := d.reload(); err != nil {
return false, err
}
return true, nil
}
func (d *LiveDevices) reload() error {
cfg, err := loadFile(d.path)
if err != nil {
return err
}
// Pair mode is the right validation here: a node whose list was just destroyed has zero devices
// and must still be able to load, because that state is the whole point of a revocation.
if err := cfg.Validate(true); err != nil {
return fmt.Errorf("config: %s: %w", d.path, err)
}
keys := make(map[string]ed25519.PublicKey, len(cfg.Devices))
for _, device := range cfg.Devices {
raw, err := base64.StdEncoding.Strict().DecodeString(device.SignKey)
if err != nil || len(raw) != ed25519.PublicKeySize {
return fmt.Errorf("config: device %s has an unusable signKey", device.ID)
}
keys[device.ID] = ed25519.PublicKey(raw)
}
stamp, err := stampOf(d.path)
if err != nil {
return err
}
d.mu.Lock()
d.keys = keys
d.devices = append([]Device(nil), cfg.Devices...)
d.stamp = stamp
d.mu.Unlock()
return nil
}
func stampOf(path string) (fileStamp, error) {
info, err := os.Stat(path)
if err != nil {
return fileStamp{}, fmt.Errorf("config: %w", err)
}
return fileStamp{modTime: info.ModTime(), size: info.Size(), inode: inodeOf(info)}, nil
}
+227
View File
@@ -0,0 +1,227 @@
package config
import (
"crypto/ed25519"
"crypto/rand"
"encoding/base64"
"encoding/json"
"os"
"path/filepath"
"strings"
"testing"
"time"
)
// liveConfig is a config whose device keys are real Ed25519 public keys.
//
// The shared `valid()` fixture uses "AAAA", which is fine for the loader's shape checks — it does not
// verify key sizes — but LiveDevices parses the keys, because a device it cannot build a public key
// for is a device it must not silently trust. So these tests need a usable key, and the generator
// keeps them from depending on a literal that would rot.
func liveConfig(t *testing.T) File {
t.Helper()
cfg := valid()
public, _, err := ed25519.GenerateKey(rand.Reader)
if err != nil {
t.Fatal(err)
}
cfg.Devices = []Device{{
ID: "ap_5b1d0000",
SignKey: base64.StdEncoding.EncodeToString(public),
RelayToken: strings.Repeat("d", 48),
}}
return cfg
}
// The property these tests exist for: a device destroyed on the node stops being trusted by the
// running daemon, without a restart. Before this, the daemon held the list it read at startup and
// re-pushed it to the relay on every handshake, so a revoked phone stayed authorised at the node and
// stayed routable at the relay — the documented revocation path did not work.
func TestLiveDevicesSeesAReloadedList(t *testing.T) {
path := filepath.Join(t.TempDir(), "config.json")
if err := Save(path, liveConfig(t)); err != nil {
t.Fatalf("Save: %v", err)
}
live, err := NewLiveDevices(path)
if err != nil {
t.Fatalf("NewLiveDevices: %v", err)
}
if live.Len() != 1 {
t.Fatalf("expected one paired device, got %d", live.Len())
}
if live.SigningKey("ap_5b1d0000") == nil {
t.Fatal("the paired device is not trusted")
}
// The operator destroys the list. Save writes through a rename, which is how the real path
// changes the file, so this exercises the inode branch of the change detection.
destroyed := liveConfig(t)
destroyed.Devices = nil
if err := Save(path, destroyed); err != nil {
t.Fatalf("Save after destroy: %v", err)
}
changed, err := live.Refresh()
if err != nil {
t.Fatalf("Refresh: %v", err)
}
if !changed {
t.Fatal("a destroyed device list was not noticed")
}
if live.Len() != 0 {
t.Fatalf("the destroyed device is still trusted: %d", live.Len())
}
if live.SigningKey("ap_5b1d0000") != nil {
t.Fatal("the destroyed device still has a signing key")
}
// The registration push must now carry an empty set: that is what retires the bearer at the
// relay, and without it the revocation is only half done.
if got := live.Devices(); len(got) != 0 {
t.Fatalf("the relay would be told about %d devices after a destroy", len(got))
}
}
// A pairing performed while the daemon is running must be picked up too, and it is the same
// mechanism: the file changed, so the set is re-read and re-asserted.
func TestLiveDevicesSeesAnAddedDevice(t *testing.T) {
path := filepath.Join(t.TempDir(), "config.json")
cfg := liveConfig(t)
cfg.Devices = nil
if err := Save(path, cfg); err != nil {
t.Fatalf("Save: %v", err)
}
live, err := NewLiveDevices(path)
if err != nil {
t.Fatalf("NewLiveDevices: %v", err)
}
if live.Len() != 0 {
t.Fatalf("expected no devices, got %d", live.Len())
}
paired := liveConfig(t)
if err := Save(path, paired); err != nil {
t.Fatalf("Save after pairing: %v", err)
}
changed, err := live.Refresh()
if err != nil {
t.Fatalf("Refresh: %v", err)
}
if !changed {
t.Fatal("a new pairing was not noticed")
}
if live.Len() != 1 || live.SigningKey("ap_5b1d0000") == nil {
t.Fatal("the newly paired device is not trusted")
}
if got := live.Devices(); len(got) != 1 || got[0].ID != "ap_5b1d0000" {
t.Fatalf("the relay would be told the wrong set: %+v", got)
}
}
// Refresh is called once per poll cycle, so the common case — nothing changed — must be cheap and
// must not report a change. A false positive would force a handshake every cycle, which is the
// defect the agent's loop comment warns about: each handshake replaces the session and drops the
// parked poll.
func TestLiveDevicesReportsNoChangeWhenNothingChanged(t *testing.T) {
path := filepath.Join(t.TempDir(), "config.json")
if err := Save(path, liveConfig(t)); err != nil {
t.Fatalf("Save: %v", err)
}
live, err := NewLiveDevices(path)
if err != nil {
t.Fatalf("NewLiveDevices: %v", err)
}
for i := 0; i < 3; i++ {
changed, err := live.Refresh()
if err != nil {
t.Fatalf("Refresh %d: %v", i, err)
}
if changed {
t.Fatalf("Refresh %d reported a change with no write", i)
}
}
}
// A rewrite that keeps the same content but changes the file is still a change worth noticing: the
// operator may have re-saved the file, and re-asserting the set at the relay costs one handshake.
// What must not happen is the opposite — a change that is missed.
func TestLiveDevicesNoticesAChangeWithinTheSameSecond(t *testing.T) {
path := filepath.Join(t.TempDir(), "config.json")
if err := Save(path, liveConfig(t)); err != nil {
t.Fatalf("Save: %v", err)
}
live, err := NewLiveDevices(path)
if err != nil {
t.Fatalf("NewLiveDevices: %v", err)
}
// A destroy immediately after a pairing, which is exactly the sequence an operator performs when
// they revoke the wrong phone and re-pair. The two writes land inside one clock tick on a
// filesystem with coarse timestamps, so the size and the inode are what has to catch it.
cfg := liveConfig(t)
cfg.Devices = nil
if err := Save(path, cfg); err != nil {
t.Fatalf("Save: %v", err)
}
_ = time.Now()
changed, err := live.Refresh()
if err != nil {
t.Fatalf("Refresh: %v", err)
}
if !changed {
t.Fatal("a destroy inside the same timestamp tick was missed")
}
if live.Len() != 0 {
t.Fatal("the destroyed device is still trusted")
}
}
// An unreadable or invalid file leaves the previous set in place. Replacing it with an empty set
// would be the worst outcome: the node would lock out every paired phone because of a transient
// filesystem error, and the user would have to reach the node to fix it.
func TestLiveDevicesKeepsTheCurrentSetWhenTheFileCannotBeRead(t *testing.T) {
path := filepath.Join(t.TempDir(), "config.json")
if err := Save(path, liveConfig(t)); err != nil {
t.Fatalf("Save: %v", err)
}
live, err := NewLiveDevices(path)
if err != nil {
t.Fatalf("NewLiveDevices: %v", err)
}
if err := os.WriteFile(path, []byte("{ this is not json"), 0o600); err != nil {
t.Fatal(err)
}
if _, err := live.Refresh(); err == nil {
t.Fatal("a corrupt config was accepted")
}
if live.Len() != 1 || live.SigningKey("ap_5b1d0000") == nil {
t.Fatal("a failed reload dropped the trusted device set")
}
// And a missing file is the same story: the set stays, because the node is still the node.
if err := os.Remove(path); err != nil {
t.Fatal(err)
}
if _, err := live.Refresh(); err == nil {
t.Fatal("a missing config was accepted")
}
if live.Len() != 1 {
t.Fatal("a missing config dropped the trusted device set")
}
}
// A device whose key cannot be used is a config error rather than a device to skip: skipping it
// would leave a phone paired in the UI and unable to command anything, with nothing to point at.
func TestLiveDevicesRejectsAnUnusableKey(t *testing.T) {
path := filepath.Join(t.TempDir(), "config.json")
cfg := liveConfig(t)
cfg.Devices[0].SignKey = base64.StdEncoding.EncodeToString([]byte("too short"))
encoded, err := json.Marshal(cfg)
if err != nil {
t.Fatal(err)
}
if err := os.WriteFile(path, encoded, 0o600); err != nil {
t.Fatal(err)
}
if _, err := NewLiveDevices(path); err == nil {
t.Fatal("a device with an unusable key was accepted")
}
}
+9
View File
@@ -0,0 +1,9 @@
//go:build !unix
package config
import "io/fs"
// inodeOf is 0 on platforms without inodes. Modification time and size still detect a change, which
// is all the reload needs to be correct; the inode only tightens the window.
func inodeOf(fs.FileInfo) uint64 { return 0 }
+21
View File
@@ -0,0 +1,21 @@
//go:build unix
package config
import (
"io/fs"
"syscall"
)
// inodeOf reports the inode of a file, or 0 when the platform does not expose one.
//
// A rename over the config — which is how every legitimate change is written — changes the inode, so
// this is what makes "the file was replaced" detectable even when the modification time and size
// happen to match, as they do when a pairing is immediately followed by a destroy.
func inodeOf(info fs.FileInfo) uint64 {
stat, ok := info.Sys().(*syscall.Stat_t)
if !ok {
return 0
}
return uint64(stat.Ino)
}