From 8a9f69dd07a542df15f65f8bf81b4fdb5415dea4 Mon Sep 17 00:00:00 2001 From: benya Date: Thu, 30 Jul 2026 01:29:55 +0300 Subject: [PATCH] fix: validate router tunnel endpoints --- .github/workflows/build.yml | 7 ++ RELEASES.md | 6 +- agent/README.md | 6 +- agent/go/cmd/rmm-agent/main.go | 78 +++++++++++++++---- agent/go/cmd/rmm-agent/main_test.go | 45 ++++++++++- agent/package/luci-app-rmm-agent/README.md | 4 +- .../package/rmm-agent-go-production/Makefile | 11 ++- .../package/rmm-agent-go-production/README.md | 2 +- agent/package/rmm-agent-go/Makefile | 4 +- agent/package/rmm-agent/Makefile | 4 +- server/internal/httpapi/luci_access_test.go | 40 +++++++--- server/internal/httpapi/server.go | 43 +++++++++- web/app.js | 2 +- 13 files changed, 211 insertions(+), 41 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d24054b..ed2da8c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -48,6 +48,13 @@ jobs: missing_file="$(grep -L 'PKG_LICENSE_FILES:=LICENSE' agent/package/*/Makefile || true)" test -z "$missing_license" test -z "$missing_file" + for file in \ + agent/package/rmm-agent/Makefile \ + agent/package/rmm-agent-go/Makefile \ + agent/package/rmm-agent-go-production/Makefile; do + grep -Eq '^[[:space:]]*DEPENDS:=.*[[:space:]]\+ip([[:space:]]|$)' "$file" + ! grep -Eq '\+ip-(tiny|full)' "$file" + done openwrt-packages: name: OpenWrt ${{ matrix.release }} ยท ${{ matrix.label }} diff --git a/RELEASES.md b/RELEASES.md index 8568202..1e9963e 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -29,7 +29,7 @@ production upgrade even when the release is marked compatible. ## Agent releases -Tags use `agent-vMAJOR.MINOR.PATCH`, for example `agent-v0.6.5`. +Tags use `agent-vMAJOR.MINOR.PATCH`, for example `agent-v0.6.6`. An agent release contains the Go runtime, LuCI application and OpenWrt IPK/APK packages. Before tagging, the tag version must match `agentVersion` in the Go source and @@ -58,8 +58,8 @@ transition period; it should not silently reuse `v1`. git tag -a server-v0.8.0 -m "OpenWrt RMM Server 0.8.0" git push origin server-v0.8.0 -git tag -a agent-v0.6.5 -m "OpenWrt RMM Agent 0.6.5" -git push origin agent-v0.6.5 +git tag -a agent-v0.6.6 -m "OpenWrt RMM Agent 0.6.6" +git push origin agent-v0.6.6 ``` Pushing a server tag publishes the container image and creates a GitHub Release. Pushing diff --git a/agent/README.md b/agent/README.md index de9141e..fef4284 100644 --- a/agent/README.md +++ b/agent/README.md @@ -1,7 +1,9 @@ # OpenWrt RMM Agent -Current stable Go agent: `0.6.5`. It reports runtime health, pending command results, -and the last heartbeat transport error after connectivity is restored. +Current stable Go agent: `0.6.6`. It reports runtime health, pending command results, +and the last heartbeat transport error after connectivity is restored. Its OpenWrt +dependency uses the virtual `ip` provider, so either `ip-tiny` or `ip-full` can satisfy it. +Production package upgrades restart an already running agent so the new binary takes effect. Production Go agent for OpenWrt, with the shell implementation retained as a fallback runtime. diff --git a/agent/go/cmd/rmm-agent/main.go b/agent/go/cmd/rmm-agent/main.go index 0c300d9..0e9ba6e 100644 --- a/agent/go/cmd/rmm-agent/main.go +++ b/agent/go/cmd/rmm-agent/main.go @@ -1,6 +1,7 @@ package main import ( + "bufio" "bytes" "context" "crypto/rand" @@ -23,7 +24,7 @@ import ( "time" ) -const agentVersion = "0.6.5" +const agentVersion = "0.6.6" type agentRuntimeHealth struct { StartedAt time.Time @@ -1028,21 +1029,56 @@ func remoteSSHTargetReachable(host string, port int) bool { if err != nil { return false } - _ = connection.Close() - return true + defer connection.Close() + if err := connection.SetReadDeadline(time.Now().Add(1500 * time.Millisecond)); err != nil { + return false + } + reader := bufio.NewReaderSize(connection, 256) + for range 4 { + line, err := reader.ReadString('\n') + if err != nil { + return false + } + line = strings.TrimSpace(line) + if strings.HasPrefix(line, "SSH-2.0-") || strings.HasPrefix(line, "SSH-1.99-") { + return true + } + } + return false } func localInterfaceIPv4Candidates() []string { - return parseLocalInterfaceIPv4Candidates(commandOutput("ip", "-4", "-o", "addr", "show")) + interfaces, err := net.Interfaces() + if err != nil { + return nil + } + var candidates []localInterfaceIPv4 + for _, networkInterface := range interfaces { + addresses, err := networkInterface.Addrs() + if err != nil { + continue + } + for _, address := range addresses { + ip, _, err := net.ParseCIDR(address.String()) + if err != nil { + continue + } + candidates = append(candidates, localInterfaceIPv4{ + interfaceName: networkInterface.Name, + ip: ip, + }) + } + } + return orderLocalInterfaceIPv4Candidates(candidates) +} + +type localInterfaceIPv4 struct { + interfaceName string + ip net.IP } func parseLocalInterfaceIPv4Candidates(output string) []string { - type candidate struct { - host string - priority int - } - var candidates []candidate - seen := map[string]struct{}{} + var candidates []localInterfaceIPv4 for _, line := range strings.Split(output, "\n") { fields := strings.Fields(line) if len(fields) < 4 || fields[2] != "inet" { @@ -1050,16 +1086,32 @@ func parseLocalInterfaceIPv4Candidates(output string) []string { } interfaceName := strings.TrimSuffix(fields[1], ":") ip, _, err := net.ParseCIDR(fields[3]) - if err != nil || ip == nil || ip.IsLoopback() || ip.IsUnspecified() || ip.IsLinkLocalUnicast() { + if err != nil { continue } - host := ip.String() + candidates = append(candidates, localInterfaceIPv4{interfaceName: interfaceName, ip: ip}) + } + return orderLocalInterfaceIPv4Candidates(candidates) +} + +func orderLocalInterfaceIPv4Candidates(values []localInterfaceIPv4) []string { + type candidate struct { + host string + priority int + } + var candidates []candidate + seen := map[string]struct{}{} + for _, value := range values { + if value.ip == nil || value.ip.To4() == nil || value.ip.IsLoopback() || value.ip.IsUnspecified() || value.ip.IsLinkLocalUnicast() { + continue + } + host := value.ip.String() if _, ok := seen[host]; ok { continue } seen[host] = struct{}{} priority := 10 - if interfaceName == "br-lan" || interfaceName == "lan" { + if value.interfaceName == "br-lan" || value.interfaceName == "lan" { priority = 0 } candidates = append(candidates, candidate{host: host, priority: priority}) diff --git a/agent/go/cmd/rmm-agent/main_test.go b/agent/go/cmd/rmm-agent/main_test.go index 5e16327..b02b978 100644 --- a/agent/go/cmd/rmm-agent/main_test.go +++ b/agent/go/cmd/rmm-agent/main_test.go @@ -2,14 +2,17 @@ package main import ( "errors" + "io" + "net" "os" "path/filepath" + "strconv" "testing" "time" ) func TestAgentVersionIsStable(t *testing.T) { - if agentVersion != "0.6.5" { + if agentVersion != "0.6.6" { t.Fatalf("unexpected agent version %q", agentVersion) } } @@ -53,6 +56,46 @@ func TestSelectRemoteSSHLocalHostDoesNotReplaceExplicitHost(t *testing.T) { } } +func TestRemoteSSHTargetReachableRequiresSSHBanner(t *testing.T) { + host, port := startBannerServer(t, "SSH-2.0-dropbear_2025.88\r\n") + if !remoteSSHTargetReachable(host, port) { + t.Fatal("SSH banner was not accepted") + } + + host, port = startBannerServer(t, "HTTP/1.1 200 OK\r\n") + if remoteSSHTargetReachable(host, port) { + t.Fatal("non-SSH TCP service was accepted") + } +} + +func startBannerServer(t *testing.T, banner string) (string, int) { + t.Helper() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _ = listener.Close() + }) + go func() { + connection, err := listener.Accept() + if err != nil { + return + } + defer connection.Close() + _, _ = io.WriteString(connection, banner) + }() + host, portText, err := net.SplitHostPort(listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + port, err := strconv.Atoi(portText) + if err != nil { + t.Fatal(err) + } + return host, port +} + func TestAgentRuntimeHealthSnapshot(t *testing.T) { spoolDir := t.TempDir() if err := os.WriteFile(filepath.Join(spoolDir, "pending.json"), []byte(`{}`), 0o600); err != nil { diff --git a/agent/package/luci-app-rmm-agent/README.md b/agent/package/luci-app-rmm-agent/README.md index a96e508..4dba03a 100644 --- a/agent/package/luci-app-rmm-agent/README.md +++ b/agent/package/luci-app-rmm-agent/README.md @@ -67,7 +67,7 @@ application to the router. Do not copy or install the shell runtime at the same cd dist/rmm-openwrt-25.12.4-ramips-mt7621 sha256sum -c SHA256SUMS scp \ - rmm-agent-go-production-0.6.5-r1.apk \ + rmm-agent-go-production-0.6.6-r1.apk \ luci-app-rmm-agent-0.2.1-r2.apk \ root@ROUTER_IP:/tmp/ ``` @@ -76,7 +76,7 @@ Then install the locally built, unsigned packages over SSH: ```sh apk add --allow-untrusted \ - /tmp/rmm-agent-go-production-0.6.5-r1.apk \ + /tmp/rmm-agent-go-production-0.6.6-r1.apk \ /tmp/luci-app-rmm-agent-0.2.1-r2.apk /etc/init.d/rpcd restart /etc/init.d/uhttpd restart diff --git a/agent/package/rmm-agent-go-production/Makefile b/agent/package/rmm-agent-go-production/Makefile index d8b2758..978286d 100644 --- a/agent/package/rmm-agent-go-production/Makefile +++ b/agent/package/rmm-agent-go-production/Makefile @@ -2,7 +2,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=rmm-agent-go-production -PKG_VERSION:=0.6.5 +PKG_VERSION:=0.6.6 PKG_RELEASE:=1 PKG_MAINTAINER:=RMM OpenWrt @@ -15,7 +15,7 @@ define Package/rmm-agent-go-production SECTION:=admin CATEGORY:=Administration TITLE:=OpenWrt RMM Go agent (production replacement) - DEPENDS:=+ca-bundle +ip-tiny +iwinfo +openssh-client +openssh-keygen + DEPENDS:=+ca-bundle +ip +iwinfo +openssh-client +openssh-keygen PROVIDES:=rmm-agent-runtime CONFLICTS:=rmm-agent endef @@ -33,6 +33,13 @@ define Package/rmm-agent-go-production/conffiles /etc/config/rmm-agent endef +define Package/rmm-agent-go-production/postinst +#!/bin/sh +if [ -z "$${IPKG_INSTROOT:-}" ] && /etc/init.d/rmm-agent running; then + /etc/init.d/rmm-agent restart +fi +endef + define Package/rmm-agent-go-production/install $(INSTALL_DIR) $(1)/usr/bin $(INSTALL_BIN) ./files/usr/bin/rmm-agent $(1)/usr/bin/rmm-agent diff --git a/agent/package/rmm-agent-go-production/README.md b/agent/package/rmm-agent-go-production/README.md index 3d6c7e2..97ee11a 100644 --- a/agent/package/rmm-agent-go-production/README.md +++ b/agent/package/rmm-agent-go-production/README.md @@ -34,5 +34,5 @@ opkg install /tmp/rmm-agent-go-production_*.ipk This package is intended for the final shell-to-Go migration when the router should keep the same RMM object identity. -Version `0.6.5` uses the cloud tunnel exclusively and no longer discovers or publishes the +Starting with version `0.6.5`, the agent uses the cloud tunnel exclusively and no longer discovers or publishes the router's public WAN addresses. diff --git a/agent/package/rmm-agent-go/Makefile b/agent/package/rmm-agent-go/Makefile index 0e1b5d8..2889a40 100644 --- a/agent/package/rmm-agent-go/Makefile +++ b/agent/package/rmm-agent-go/Makefile @@ -2,7 +2,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=rmm-agent-go -PKG_VERSION:=0.6.5 +PKG_VERSION:=0.6.6 PKG_RELEASE:=1 PKG_MAINTAINER:=RMM OpenWrt @@ -15,7 +15,7 @@ define Package/rmm-agent-go SECTION:=admin CATEGORY:=Administration TITLE:=OpenWrt RMM Go agent - DEPENDS:=+ca-bundle +ip-tiny +iwinfo +openssh-client +openssh-keygen + DEPENDS:=+ca-bundle +ip +iwinfo +openssh-client +openssh-keygen endef define Package/rmm-agent-go/description diff --git a/agent/package/rmm-agent/Makefile b/agent/package/rmm-agent/Makefile index b18e28f..2d12b3a 100644 --- a/agent/package/rmm-agent/Makefile +++ b/agent/package/rmm-agent/Makefile @@ -3,7 +3,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=rmm-agent PKG_VERSION:=0.1.1 -PKG_RELEASE:=3 +PKG_RELEASE:=4 PKG_MAINTAINER:=RMM OpenWrt PKG_LICENSE:=MIT @@ -15,7 +15,7 @@ define Package/rmm-agent SECTION:=admin CATEGORY:=Administration TITLE:=OpenWrt RMM agent - DEPENDS:=+uclient-fetch +ubus +ip-tiny + DEPENDS:=+uclient-fetch +ubus +ip PROVIDES:=rmm-agent-runtime PKGARCH:=all endef diff --git a/server/internal/httpapi/luci_access_test.go b/server/internal/httpapi/luci_access_test.go index bd6dad8..d7be28a 100644 --- a/server/internal/httpapi/luci_access_test.go +++ b/server/internal/httpapi/luci_access_test.go @@ -41,21 +41,21 @@ func TestWriteLuCIErrorRendersSafeBrowserPage(t *testing.T) { } } -func TestRemoteSessionAccessStateChecksTunnelPort(t *testing.T) { - listener, err := net.Listen("tcp", "127.0.0.1:0") - if err != nil { - t.Fatal(err) - } - defer listener.Close() - _, portText, _ := net.SplitHostPort(listener.Addr().String()) +func TestRemoteSessionAccessStateChecksLuCIResponse(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusFound) + })) + defer upstream.Close() + hostPort := strings.TrimPrefix(upstream.URL, "http://") + host, portText, _ := net.SplitHostPort(hostPort) port, _ := strconv.Atoi(portText) now := time.Now().UTC() - app := &App{tunnelHTTPHost: "127.0.0.1"} + app := &App{tunnelHTTPHost: host} session := model.RemoteSession{Status: "active", LuCIPort: port, StartedAt: &now, ExpiresAt: now.Add(time.Minute)} if got := app.remoteSessionAccessState(session); got != "ready" { t.Fatalf("access state = %q, want ready", got) } - _ = listener.Close() + upstream.Close() if got := app.remoteSessionAccessState(session); got != "starting" { t.Fatalf("fresh failed tunnel state = %q, want starting", got) } @@ -66,6 +66,28 @@ func TestRemoteSessionAccessStateChecksTunnelPort(t *testing.T) { } } +func TestRemoteSessionAccessStateRejectsBareTCPListener(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer listener.Close() + go func() { + connection, err := listener.Accept() + if err == nil { + _ = connection.Close() + } + }() + host, portText, _ := net.SplitHostPort(listener.Addr().String()) + port, _ := strconv.Atoi(portText) + now := time.Now().UTC() + app := &App{tunnelHTTPHost: host} + session := model.RemoteSession{Status: "active", LuCIPort: port, StartedAt: &now, ExpiresAt: now.Add(time.Minute)} + if got := app.remoteSessionAccessState(session); got != "starting" { + t.Fatalf("bare TCP listener state = %q, want starting", got) + } +} + func TestWriteLuCIErrorKeepsJSONForAPIClients(t *testing.T) { app := &App{deviceDomain: "rmm.example", publicScheme: "https"} req := httptest.NewRequest(http.MethodPost, "/api/devices/device-1/access", nil) diff --git a/server/internal/httpapi/server.go b/server/internal/httpapi/server.go index e755e6f..ff55327 100644 --- a/server/internal/httpapi/server.go +++ b/server/internal/httpapi/server.go @@ -1401,9 +1401,7 @@ func (a *App) remoteSessionAccessState(session model.RemoteSession) string { if session.Status != "active" || session.LuCIPort <= 0 { return "starting" } - connection, err := net.DialTimeout("tcp", net.JoinHostPort(a.tunnelHTTPHost, strconv.Itoa(session.LuCIPort)), 150*time.Millisecond) - if err == nil { - _ = connection.Close() + if a.luciTunnelReachable(session) { return "ready" } if session.StartedAt != nil && time.Since(*session.StartedAt) > 20*time.Second { @@ -1412,6 +1410,45 @@ func (a *App) remoteSessionAccessState(session model.RemoteSession) string { return "starting" } +func (a *App) luciTunnelReachable(session model.RemoteSession) bool { + scheme := session.LuCIScheme + if scheme == "" { + scheme = "http" + } + if scheme != "http" && scheme != "https" { + return false + } + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + upstream := scheme + "://" + net.JoinHostPort(a.tunnelHTTPHost, strconv.Itoa(session.LuCIPort)) + "/" + req, err := http.NewRequestWithContext(ctx, http.MethodGet, upstream, nil) + if err != nil { + return false + } + req.Host = "127.0.0.1" + + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.DisableKeepAlives = true + transport.DialContext = (&net.Dialer{Timeout: 500 * time.Millisecond}).DialContext + transport.ResponseHeaderTimeout = 1500 * time.Millisecond + if scheme == "https" { + transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} // LuCI commonly uses a router-local self-signed certificate. + } + client := &http.Client{ + Transport: transport, + Timeout: 2 * time.Second, + CheckRedirect: func(_ *http.Request, _ []*http.Request) error { + return http.ErrUseLastResponse + }, + } + resp, err := client.Do(req) + if err != nil { + return false + } + _ = resp.Body.Close() + return true +} + func writeCloudError(w http.ResponseWriter, status int, code, message string) { writeJSON(w, status, map[string]string{"error": message, "code": code}) } diff --git a/web/app.js b/web/app.js index 9fca27f..1b68114 100644 --- a/web/app.js +++ b/web/app.js @@ -38,7 +38,7 @@ const state = { let eventSource = null; let liveRefreshTimer = null; -const EXPECTED_AGENT_VERSION = "0.6.5"; +const EXPECTED_AGENT_VERSION = "0.6.6"; const els = { loginView: document.querySelector("#loginView"),