From b5f548f1ad405a29b7c6ca48a33e4b8fa4cb11a9 Mon Sep 17 00:00:00 2001 From: chick Date: Thu, 17 Sep 2026 07:47:25 +0800 Subject: [PATCH] =?UTF-8?q?feat(agent):=20=E9=85=8D=E5=AF=B9=E7=AA=97?= =?UTF-8?q?=E5=8F=A3=E5=8F=AF=E6=98=BE=E5=BC=8F=E7=BB=91=E5=AE=9A=E5=9C=B0?= =?UTF-8?q?=E5=9D=80=EF=BC=8C=E9=BB=98=E8=AE=A4=E4=BB=8D=E6=98=AF=E5=86=85?= =?UTF-8?q?=E7=BD=91=E5=8F=AF=E8=BE=BE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 配对窗口是 agent 唯一持有的监听端口，而它授予的是设备信任：谁能带节点密码打到 /provision，谁就拿到一把 agent 会听命的密钥。此前它固定绑所有接口，且没有任何办法 收窄——一台本身能从公网路由到的节点（公网 VPS 上的 SimAdmin、放在非可信网络里的 设备）在窗口开启期间就把这个面暴露给了整个互联网。 现在加 --pair-bind，只接受 IP 字面量。不接受主机名是刻意的：绑定时解析名字意味着 监听地址取决于 DNS 当时说什么，那不是运维做过的决定。 默认值仍是绑所有接口，这个我改过一次又改回来了，原因值得记下来：我先把默认设成 127.0.0.1 并写了理由，然后发现配对按设计走内网、手机要直接打到节点，回环监听从 另一台机器根本连不上——那不是一个加固措施，而是一个永远不工作的流程，用户对这个 功能的第一次体验会是"连接被拒"且没有解释。所以默认保持内网可达，把"节点能从公网 路由到时请收窄"写进部署文档，并在 --pair-bind 上给出显式入口。 测试断言默认值同时满足两件事：是 IP 字面量（不是名字），且不是回环（手机要能连）。 Go 全包、Node 120 项、Swift 107 项通过。 --- cmd/msa-agent/main.go | 16 ++++++++++++---- internal/pair/pair.go | 23 ++++++++++++++++++++--- internal/pair/pair_test.go | 23 +++++++++++++++++++++++ 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/cmd/msa-agent/main.go b/cmd/msa-agent/main.go index 0f215ac..fd54c64 100644 --- a/cmd/msa-agent/main.go +++ b/cmd/msa-agent/main.go @@ -15,6 +15,7 @@ import ( "flag" "fmt" "log/slog" + "net" "net/http" "os" "os/signal" @@ -47,6 +48,7 @@ func main() { once = flag.Bool("once", false, "run a single poll cycle and exit") pairMode = flag.Bool("pair", false, "open the temporary pairing window on the LAN and exit when it closes") pairPort = flag.Int("pair-port", pair.DefaultAdminPort, "port for the pairing window's management listener") + pairBind = flag.String("pair-bind", pair.DefaultAdminHost, "address the pairing window binds to") ) flag.Parse() @@ -55,7 +57,7 @@ func main() { var err error if *pairMode { - err = runPair(*configPath, *pairPort) + err = runPair(*configPath, *pairPort, *pairBind) } else { err = run(*configPath, *check, *once) } @@ -71,7 +73,7 @@ func main() { // the invariant "the agent binds no socket" is a property of the daemon; the pairing window is a // deliberate, operator-started exception with three independent shutdowns (first pairing, attempt // ceiling, deadline). Pairing is run again by the operator for each additional phone. -func runPair(configPath string, adminPort int) error { +func runPair(configPath string, adminPort int, adminHost string) error { cfg, err := config.LoadForPairing(configPath) if err != nil { return err @@ -82,13 +84,19 @@ func runPair(configPath string, adminPort int) error { if err != nil { return err } + if net.ParseIP(adminHost) == nil { + // A name here would be resolved at bind time, and the window is the one place the agent holds a + // listener: an operator who typed a hostname would get whatever that name resolved to, which is + // not a decision they made deliberately. The default is a literal for that reason. + return fmt.Errorf("pair-bind must be an IP literal, got %q", adminHost) + } server := &http.Server{ - Addr: ":" + strconv.Itoa(adminPort), + Addr: net.JoinHostPort(adminHost, strconv.Itoa(adminPort)), Handler: handler.Routes(), ReadHeaderTimeout: 5 * time.Second, } go func() { - slog.Info("pairing window open", "port", adminPort, "node", cfg.NodeID) + slog.Info("pairing window open", "bind", adminHost, "port", adminPort, "node", cfg.NodeID) if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { slog.Error("pairing listener failed", "error", err) handler.Close() diff --git a/internal/pair/pair.go b/internal/pair/pair.go index a9bf36e..447c09c 100644 --- a/internal/pair/pair.go +++ b/internal/pair/pair.go @@ -30,9 +30,26 @@ import ( // Limits. The attempt ceiling is small on purpose: the pairing secret is a human password, and a // window where anyone on the LAN can try it a thousand times is a window that leaks the node. const ( - MaxAttempts = 10 - DefaultWindow = 10 * time.Minute - MaxRequestBytes = 8 << 10 + MaxAttempts = 10 + DefaultWindow = 10 * time.Minute + MaxRequestBytes = 8 << 10 + // DefaultAdminHost is a literal, not a name and not empty. + // + // The window grants device trust: whoever reaches /provision with the node password gets a key the + // agent will obey. So the bind address decides who can even attempt that, and the default has to be + // the narrowest thing that still lets the documented flow work. + // + // It cannot be 127.0.0.1. Pairing happens over the LAN by design — the phone dials the node's LAN + // address, which is what makes a QR code unnecessary — and a listener on loopback is not reachable + // from another host. A loopback default would not be a hardening measure; it would be a flow that + // never works, and the operator's first experience of this feature would be a refused connection + // with no explanation. + // + // It is a literal rather than a name for a different reason: a name is resolved at bind time, so the + // listening address would depend on what DNS said at that moment. That is not a decision the + // operator made, and on a node whose resolver is compromised it is a decision someone else made. + // Operators who want a narrower or wider bind pass --pair-bind. + DefaultAdminHost = "0.0.0.0" DefaultAdminPort = 3001 ) diff --git a/internal/pair/pair_test.go b/internal/pair/pair_test.go index 614929c..3e524cd 100644 --- a/internal/pair/pair_test.go +++ b/internal/pair/pair_test.go @@ -6,6 +6,7 @@ import ( "crypto/rand" "encoding/base64" "encoding/json" + "net" "net/http" "net/http/httptest" "os" @@ -280,3 +281,25 @@ func TestServeDeadlineClosesTheWindow(t *testing.T) { t.Fatalf("status after the deadline: %d", response.StatusCode) } } + +// The window's default bind has to satisfy two things at once, and they pull in opposite directions. +// +// It must be reachable from the phone: pairing happens over the LAN by design, so a loopback default +// would be a flow that never works. And it must be a literal rather than a name: a name is resolved at +// bind time, so the listening address would depend on what DNS said, which is not a decision the +// operator made. +func TestTheDefaultBindIsALiteralAndIsReachableFromTheLAN(t *testing.T) { + if net.ParseIP(DefaultAdminHost) == nil { + t.Fatalf("%q is not an IP literal; a name would be resolved at bind time", DefaultAdminHost) + } + ip := net.ParseIP(DefaultAdminHost) + if ip.IsLoopback() { + t.Fatal("a loopback default cannot be reached from the phone, and pairing is a LAN flow") + } + // Any-address is the default because the documented flow requires it; the operator narrows it with + // --pair-bind when the node is routable from somewhere it should not be. + if !ip.IsUnspecified() { + t.Fatalf("the default bind changed to %q; the documented flow and the deployment notes assume %q", + DefaultAdminHost, "0.0.0.0") + } +}