From d583782ee322a6faa1fe87ae52455e0d349de586 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 14 Feb 2026 17:17:46 +0100 Subject: [PATCH] fix(security): harden discovery routing and TLS pins --- CHANGELOG.md | 1 + .../java/ai/openclaw/android/NodeRuntime.kt | 12 +- .../android/node/ConnectionManager.kt | 86 ++++++++------ .../android/node/ConnectionManagerTest.kt | 77 +++++++++++++ .../Gateway/GatewayConnectionController.swift | 108 +++++++++++------- .../Gateway/GatewayServiceResolver.swift | 55 +++++++++ .../GatewayConnectionSecurityTests.swift | 105 +++++++++++++++++ .../OpenClaw/GatewayDiscoveryHelpers.swift | 18 ++- .../Sources/OpenClaw/GeneralSettings.swift | 4 +- .../OpenClaw/OnboardingView+Actions.swift | 4 +- .../GatewayDiscoveryModel.swift | 77 +++++++++---- docs/gateway/bonjour.md | 6 + docs/gateway/bridge-protocol.md | 4 +- docs/gateway/discovery.md | 6 + src/cli/gateway-cli/discover.test.ts | 35 ++++++ src/cli/gateway-cli/discover.ts | 7 +- src/commands/onboard-remote.ts | 8 +- 17 files changed, 503 insertions(+), 110 deletions(-) create mode 100644 apps/android/app/src/test/java/ai/openclaw/android/node/ConnectionManagerTest.kt create mode 100644 apps/ios/Sources/Gateway/GatewayServiceResolver.swift create mode 100644 apps/ios/Tests/GatewayConnectionSecurityTests.swift create mode 100644 src/cli/gateway-cli/discover.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b2e58ecc94..ffd4ce5baf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Docs: https://docs.openclaw.ai - Security/Agents: scope CLI process cleanup to owned child PIDs to avoid killing unrelated processes on shared hosts. Thanks @aether-ai-agent. - Security: fix Chutes manual OAuth login state validation (thanks @aether-ai-agent). (#16058) +- Security/Discovery: stop treating Bonjour TXT records as authoritative routing (prefer resolved service endpoints) and prevent discovery from overriding stored TLS pins; autoconnect now requires a previously trusted gateway. Thanks @simecek. - macOS: hard-limit unkeyed `openclaw://agent` deep links and ignore `deliver` / `to` / `channel` unless a valid unattended key is provided. Thanks @Cillian-Collins. - Plugins: suppress false duplicate plugin id warnings when the same extension is discovered via multiple paths (config/workspace/global vs bundled), while still warning on genuine duplicates. (#16222) Thanks @shadril238. - Security/Google Chat: deprecate `users/` allowlists (treat `users/...` as immutable user id only); keep raw email allowlists for usability. Thanks @vincentkoc. diff --git a/apps/android/app/src/main/java/ai/openclaw/android/NodeRuntime.kt b/apps/android/app/src/main/java/ai/openclaw/android/NodeRuntime.kt index 51daeff5ab..965472c89b 100644 --- a/apps/android/app/src/main/java/ai/openclaw/android/NodeRuntime.kt +++ b/apps/android/app/src/main/java/ai/openclaw/android/NodeRuntime.kt @@ -405,8 +405,11 @@ class NodeRuntime(context: Context) { scope.launch(Dispatchers.Default) { gateways.collect { list -> if (list.isNotEmpty()) { - // Persist the last discovered gateway (best-effort UX parity with iOS). - prefs.setLastDiscoveredStableId(list.last().stableId) + // Security: don't let an unauthenticated discovery feed continuously steer autoconnect. + // UX parity with iOS: only set once when unset. + if (lastDiscoveredStableId.value.trim().isEmpty()) { + prefs.setLastDiscoveredStableId(list.first().stableId) + } } if (didAutoConnect) return@collect @@ -425,6 +428,11 @@ class NodeRuntime(context: Context) { val targetStableId = lastDiscoveredStableId.value.trim() if (targetStableId.isEmpty()) return@collect val target = list.firstOrNull { it.stableId == targetStableId } ?: return@collect + + // Security: autoconnect only to previously trusted gateways (stored TLS pin). + val storedFingerprint = prefs.loadGatewayTlsFingerprint(target.stableId)?.trim().orEmpty() + if (storedFingerprint.isEmpty()) return@collect + didAutoConnect = true connect(target) } diff --git a/apps/android/app/src/main/java/ai/openclaw/android/node/ConnectionManager.kt b/apps/android/app/src/main/java/ai/openclaw/android/node/ConnectionManager.kt index 3b413d2d68..33d5f9d531 100644 --- a/apps/android/app/src/main/java/ai/openclaw/android/node/ConnectionManager.kt +++ b/apps/android/app/src/main/java/ai/openclaw/android/node/ConnectionManager.kt @@ -26,6 +26,59 @@ class ConnectionManager( private val hasRecordAudioPermission: () -> Boolean, private val manualTls: () -> Boolean, ) { + companion object { + internal fun resolveTlsParamsForEndpoint( + endpoint: GatewayEndpoint, + storedFingerprint: String?, + manualTlsEnabled: Boolean, + ): GatewayTlsParams? { + val stableId = endpoint.stableId + val stored = storedFingerprint?.trim().takeIf { !it.isNullOrEmpty() } + val isManual = stableId.startsWith("manual|") + + if (isManual) { + if (!manualTlsEnabled) return null + if (!stored.isNullOrBlank()) { + return GatewayTlsParams( + required = true, + expectedFingerprint = stored, + allowTOFU = false, + stableId = stableId, + ) + } + return GatewayTlsParams( + required = true, + expectedFingerprint = null, + allowTOFU = true, + stableId = stableId, + ) + } + + // Prefer stored pins. Never let discovery-provided TXT override a stored fingerprint. + if (!stored.isNullOrBlank()) { + return GatewayTlsParams( + required = true, + expectedFingerprint = stored, + allowTOFU = false, + stableId = stableId, + ) + } + + val hinted = endpoint.tlsEnabled || !endpoint.tlsFingerprintSha256.isNullOrBlank() + if (hinted) { + // TXT is unauthenticated. Do not treat the advertised fingerprint as authoritative. + return GatewayTlsParams( + required = true, + expectedFingerprint = null, + allowTOFU = true, + stableId = stableId, + ) + } + + return null + } + } + fun buildInvokeCommands(): List = buildList { add(OpenClawCanvasCommand.Present.rawValue) @@ -130,37 +183,6 @@ class ConnectionManager( fun resolveTlsParams(endpoint: GatewayEndpoint): GatewayTlsParams? { val stored = prefs.loadGatewayTlsFingerprint(endpoint.stableId) - val hinted = endpoint.tlsEnabled || !endpoint.tlsFingerprintSha256.isNullOrBlank() - val manual = endpoint.stableId.startsWith("manual|") - - if (manual) { - if (!manualTls()) return null - return GatewayTlsParams( - required = true, - expectedFingerprint = endpoint.tlsFingerprintSha256 ?: stored, - allowTOFU = stored == null, - stableId = endpoint.stableId, - ) - } - - if (hinted) { - return GatewayTlsParams( - required = true, - expectedFingerprint = endpoint.tlsFingerprintSha256 ?: stored, - allowTOFU = stored == null, - stableId = endpoint.stableId, - ) - } - - if (!stored.isNullOrBlank()) { - return GatewayTlsParams( - required = true, - expectedFingerprint = stored, - allowTOFU = false, - stableId = endpoint.stableId, - ) - } - - return null + return resolveTlsParamsForEndpoint(endpoint, storedFingerprint = stored, manualTlsEnabled = manualTls()) } } diff --git a/apps/android/app/src/test/java/ai/openclaw/android/node/ConnectionManagerTest.kt b/apps/android/app/src/test/java/ai/openclaw/android/node/ConnectionManagerTest.kt new file mode 100644 index 0000000000..3704f93bd5 --- /dev/null +++ b/apps/android/app/src/test/java/ai/openclaw/android/node/ConnectionManagerTest.kt @@ -0,0 +1,77 @@ +package ai.openclaw.android.node + +import ai.openclaw.android.gateway.GatewayEndpoint +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +class ConnectionManagerTest { + @Test + fun resolveTlsParamsForEndpoint_prefersStoredPinOverAdvertisedFingerprint() { + val endpoint = + GatewayEndpoint( + stableId = "_openclaw-gw._tcp.|local.|Test", + name = "Test", + host = "10.0.0.2", + port = 18789, + tlsEnabled = true, + tlsFingerprintSha256 = "attacker", + ) + + val params = + ConnectionManager.resolveTlsParamsForEndpoint( + endpoint, + storedFingerprint = "legit", + manualTlsEnabled = false, + ) + + assertEquals("legit", params?.expectedFingerprint) + assertEquals(false, params?.allowTOFU) + } + + @Test + fun resolveTlsParamsForEndpoint_doesNotTrustAdvertisedFingerprintWhenNoStoredPin() { + val endpoint = + GatewayEndpoint( + stableId = "_openclaw-gw._tcp.|local.|Test", + name = "Test", + host = "10.0.0.2", + port = 18789, + tlsEnabled = true, + tlsFingerprintSha256 = "attacker", + ) + + val params = + ConnectionManager.resolveTlsParamsForEndpoint( + endpoint, + storedFingerprint = null, + manualTlsEnabled = false, + ) + + assertNull(params?.expectedFingerprint) + assertEquals(true, params?.allowTOFU) + } + + @Test + fun resolveTlsParamsForEndpoint_manualRespectsManualTlsToggle() { + val endpoint = GatewayEndpoint.manual(host = "example.com", port = 443) + + val off = + ConnectionManager.resolveTlsParamsForEndpoint( + endpoint, + storedFingerprint = null, + manualTlsEnabled = false, + ) + assertNull(off) + + val on = + ConnectionManager.resolveTlsParamsForEndpoint( + endpoint, + storedFingerprint = null, + manualTlsEnabled = true, + ) + assertNull(on?.expectedFingerprint) + assertEquals(true, on?.allowTOFU) + } +} + diff --git a/apps/ios/Sources/Gateway/GatewayConnectionController.swift b/apps/ios/Sources/Gateway/GatewayConnectionController.swift index 34af7f1dc0..579513e365 100644 --- a/apps/ios/Sources/Gateway/GatewayConnectionController.swift +++ b/apps/ios/Sources/Gateway/GatewayConnectionController.swift @@ -23,6 +23,7 @@ final class GatewayConnectionController { private let discovery = GatewayDiscoveryModel() private weak var appModel: NodeAppModel? private var didAutoConnect = false + private var pendingServiceResolvers: [String: GatewayServiceResolver] = [:] init(appModel: NodeAppModel, startDiscovery: Bool = true) { self.appModel = appModel @@ -57,21 +58,30 @@ final class GatewayConnectionController { } func connect(_ gateway: GatewayDiscoveryModel.DiscoveredGateway) async { + await self.connectDiscoveredGateway(gateway, allowTOFU: true) + } + + private func connectDiscoveredGateway( + _ gateway: GatewayDiscoveryModel.DiscoveredGateway, + allowTOFU: Bool) async + { let instanceId = UserDefaults.standard.string(forKey: "node.instanceId")? .trimmingCharacters(in: .whitespacesAndNewlines) ?? "" let token = GatewaySettingsStore.loadGatewayToken(instanceId: instanceId) let password = GatewaySettingsStore.loadGatewayPassword(instanceId: instanceId) - guard let host = self.resolveGatewayHost(gateway) else { return } - let port = gateway.gatewayPort ?? 18789 - let tlsParams = self.resolveDiscoveredTLSParams(gateway: gateway) + + // Resolve the service endpoint (SRV/A/AAAA). TXT is unauthenticated; do not route via TXT. + guard let target = await self.resolveServiceEndpoint(gateway.endpoint) else { return } + + let tlsParams = self.resolveDiscoveredTLSParams(gateway: gateway, allowTOFU: allowTOFU) guard let url = self.buildGatewayURL( - host: host, - port: port, + host: target.host, + port: target.port, useTLS: tlsParams?.required == true) else { return } GatewaySettingsStore.saveLastGatewayConnection( - host: host, - port: port, + host: target.host, + port: target.port, useTLS: tlsParams?.required == true, stableID: gateway.stableID) self.didAutoConnect = true @@ -254,36 +264,26 @@ final class GatewayConnectionController { self.gateways.contains(where: { $0.stableID == id }) }) { guard let target = self.gateways.first(where: { $0.stableID == targetStableID }) else { return } - guard let host = self.resolveGatewayHost(target) else { return } - let port = target.gatewayPort ?? 18789 - let tlsParams = self.resolveDiscoveredTLSParams(gateway: target) - guard let url = self.buildGatewayURL(host: host, port: port, useTLS: tlsParams?.required == true) - else { return } + // Security: autoconnect only to previously trusted gateways (stored TLS pin). + guard GatewayTLSStore.loadFingerprint(stableID: target.stableID) != nil else { return } self.didAutoConnect = true - self.startAutoConnect( - url: url, - gatewayStableID: target.stableID, - tls: tlsParams, - token: token, - password: password) + Task { [weak self] in + guard let self else { return } + await self.connectDiscoveredGateway(target, allowTOFU: false) + } return } if self.gateways.count == 1, let gateway = self.gateways.first { - guard let host = self.resolveGatewayHost(gateway) else { return } - let port = gateway.gatewayPort ?? 18789 - let tlsParams = self.resolveDiscoveredTLSParams(gateway: gateway) - guard let url = self.buildGatewayURL(host: host, port: port, useTLS: tlsParams?.required == true) - else { return } + // Security: autoconnect only to previously trusted gateways (stored TLS pin). + guard GatewayTLSStore.loadFingerprint(stableID: gateway.stableID) != nil else { return } self.didAutoConnect = true - self.startAutoConnect( - url: url, - gatewayStableID: gateway.stableID, - tls: tlsParams, - token: token, - password: password) + Task { [weak self] in + guard let self else { return } + await self.connectDiscoveredGateway(gateway, allowTOFU: false) + } return } } @@ -339,15 +339,27 @@ final class GatewayConnectionController { } } - private func resolveDiscoveredTLSParams(gateway: GatewayDiscoveryModel.DiscoveredGateway) -> GatewayTLSParams? { + private func resolveDiscoveredTLSParams( + gateway: GatewayDiscoveryModel.DiscoveredGateway, + allowTOFU: Bool) -> GatewayTLSParams? + { let stableID = gateway.stableID let stored = GatewayTLSStore.loadFingerprint(stableID: stableID) - if gateway.tlsEnabled || gateway.tlsFingerprintSha256 != nil || stored != nil { + // Never let unauthenticated discovery (TXT) override a stored pin. + if let stored { return GatewayTLSParams( required: true, - expectedFingerprint: gateway.tlsFingerprintSha256 ?? stored, - allowTOFU: stored == nil, + expectedFingerprint: stored, + allowTOFU: false, + storeKey: stableID) + } + + if gateway.tlsEnabled || gateway.tlsFingerprintSha256 != nil { + return GatewayTLSParams( + required: true, + expectedFingerprint: nil, + allowTOFU: allowTOFU, storeKey: stableID) } @@ -371,14 +383,19 @@ final class GatewayConnectionController { return nil } - private func resolveGatewayHost(_ gateway: GatewayDiscoveryModel.DiscoveredGateway) -> String? { - if let tailnet = gateway.tailnetDns?.trimmingCharacters(in: .whitespacesAndNewlines), !tailnet.isEmpty { - return tailnet + private func resolveServiceEndpoint(_ endpoint: NWEndpoint) async -> (host: String, port: Int)? { + guard case let .service(name, type, domain, _) = endpoint else { return nil } + let key = "\(domain)|\(type)|\(name)" + return await withCheckedContinuation { continuation in + let resolver = GatewayServiceResolver(name: name, type: type, domain: domain) { [weak self] result in + Task { @MainActor in + self?.pendingServiceResolvers[key] = nil + continuation.resume(returning: result) + } + } + self.pendingServiceResolvers[key] = resolver + resolver.start() } - if let lanHost = gateway.lanHost?.trimmingCharacters(in: .whitespacesAndNewlines), !lanHost.isEmpty { - return lanHost - } - return nil } private func buildGatewayURL(host: String, port: Int, useTLS: Bool) -> URL? { @@ -662,5 +679,16 @@ extension GatewayConnectionController { func _test_triggerAutoConnect() { self.maybeAutoConnect() } + + func _test_didAutoConnect() -> Bool { + self.didAutoConnect + } + + func _test_resolveDiscoveredTLSParams( + gateway: GatewayDiscoveryModel.DiscoveredGateway, + allowTOFU: Bool) -> GatewayTLSParams? + { + self.resolveDiscoveredTLSParams(gateway: gateway, allowTOFU: allowTOFU) + } } #endif diff --git a/apps/ios/Sources/Gateway/GatewayServiceResolver.swift b/apps/ios/Sources/Gateway/GatewayServiceResolver.swift new file mode 100644 index 0000000000..882a4e7d05 --- /dev/null +++ b/apps/ios/Sources/Gateway/GatewayServiceResolver.swift @@ -0,0 +1,55 @@ +import Foundation + +// NetService-based resolver for Bonjour services. +// Used to resolve the service endpoint (SRV + A/AAAA) without trusting TXT for routing. +final class GatewayServiceResolver: NSObject, NetServiceDelegate { + private let service: NetService + private let completion: ((host: String, port: Int)?) -> Void + private var didFinish = false + + init( + name: String, + type: String, + domain: String, + completion: @escaping ((host: String, port: Int)?) -> Void) + { + self.service = NetService(domain: domain, type: type, name: name) + self.completion = completion + super.init() + self.service.delegate = self + } + + func start(timeout: TimeInterval = 2.0) { + self.service.schedule(in: .main, forMode: .common) + self.service.resolve(withTimeout: timeout) + } + + func netServiceDidResolveAddress(_ sender: NetService) { + let host = Self.normalizeHost(sender.hostName) + let port = sender.port + guard let host, !host.isEmpty, port > 0 else { + self.finish(result: nil) + return + } + self.finish(result: (host: host, port: port)) + } + + func netService(_ sender: NetService, didNotResolve errorDict: [String: NSNumber]) { + self.finish(result: nil) + } + + private func finish(result: ((host: String, port: Int))?) { + guard !self.didFinish else { return } + self.didFinish = true + self.service.stop() + self.service.remove(from: .main, forMode: .common) + self.completion(result) + } + + private static func normalizeHost(_ raw: String?) -> String? { + let trimmed = raw?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + if trimmed.isEmpty { return nil } + return trimmed.hasSuffix(".") ? String(trimmed.dropLast()) : trimmed + } +} + diff --git a/apps/ios/Tests/GatewayConnectionSecurityTests.swift b/apps/ios/Tests/GatewayConnectionSecurityTests.swift new file mode 100644 index 0000000000..c05add2aed --- /dev/null +++ b/apps/ios/Tests/GatewayConnectionSecurityTests.swift @@ -0,0 +1,105 @@ +import Foundation +import Network +import Testing +@testable import OpenClaw + +@Suite(.serialized) struct GatewayConnectionSecurityTests { + private func clearTLSFingerprint(stableID: String) { + let suite = UserDefaults(suiteName: "ai.openclaw.shared") ?? .standard + suite.removeObject(forKey: "gateway.tls.\(stableID)") + } + + @Test @MainActor func discoveredTLSParams_prefersStoredPinOverAdvertisedTXT() async { + let stableID = "test|\(UUID().uuidString)" + defer { clearTLSFingerprint(stableID: stableID) } + clearTLSFingerprint(stableID: stableID) + + GatewayTLSStore.saveFingerprint("11", stableID: stableID) + + let endpoint: NWEndpoint = .service(name: "Test", type: "_openclaw-gw._tcp", domain: "local.", interface: nil) + let gateway = GatewayDiscoveryModel.DiscoveredGateway( + name: "Test", + endpoint: endpoint, + stableID: stableID, + debugID: "debug", + lanHost: "evil.example.com", + tailnetDns: "evil.example.com", + gatewayPort: 12345, + canvasPort: nil, + tlsEnabled: true, + tlsFingerprintSha256: "22", + cliPath: nil) + + let appModel = NodeAppModel() + let controller = GatewayConnectionController(appModel: appModel, startDiscovery: false) + + let params = controller._test_resolveDiscoveredTLSParams(gateway: gateway, allowTOFU: true) + #expect(params?.expectedFingerprint == "11") + #expect(params?.allowTOFU == false) + } + + @Test @MainActor func discoveredTLSParams_doesNotTrustAdvertisedFingerprint() async { + let stableID = "test|\(UUID().uuidString)" + defer { clearTLSFingerprint(stableID: stableID) } + clearTLSFingerprint(stableID: stableID) + + let endpoint: NWEndpoint = .service(name: "Test", type: "_openclaw-gw._tcp", domain: "local.", interface: nil) + let gateway = GatewayDiscoveryModel.DiscoveredGateway( + name: "Test", + endpoint: endpoint, + stableID: stableID, + debugID: "debug", + lanHost: nil, + tailnetDns: nil, + gatewayPort: nil, + canvasPort: nil, + tlsEnabled: true, + tlsFingerprintSha256: "22", + cliPath: nil) + + let appModel = NodeAppModel() + let controller = GatewayConnectionController(appModel: appModel, startDiscovery: false) + + let params = controller._test_resolveDiscoveredTLSParams(gateway: gateway, allowTOFU: true) + #expect(params?.expectedFingerprint == nil) + #expect(params?.allowTOFU == true) + } + + @Test @MainActor func autoconnectRequiresStoredPinForDiscoveredGateways() async { + let stableID = "test|\(UUID().uuidString)" + defer { clearTLSFingerprint(stableID: stableID) } + clearTLSFingerprint(stableID: stableID) + + let defaults = UserDefaults.standard + defaults.set(true, forKey: "gateway.autoconnect") + defaults.set(false, forKey: "gateway.manual.enabled") + defaults.removeObject(forKey: "gateway.last.host") + defaults.removeObject(forKey: "gateway.last.port") + defaults.removeObject(forKey: "gateway.last.tls") + defaults.removeObject(forKey: "gateway.last.stableID") + defaults.removeObject(forKey: "gateway.preferredStableID") + defaults.set(stableID, forKey: "gateway.lastDiscoveredStableID") + + let endpoint: NWEndpoint = .service(name: "Test", type: "_openclaw-gw._tcp", domain: "local.", interface: nil) + let gateway = GatewayDiscoveryModel.DiscoveredGateway( + name: "Test", + endpoint: endpoint, + stableID: stableID, + debugID: "debug", + lanHost: "test.local", + tailnetDns: nil, + gatewayPort: 18789, + canvasPort: nil, + tlsEnabled: true, + tlsFingerprintSha256: nil, + cliPath: nil) + + let appModel = NodeAppModel() + let controller = GatewayConnectionController(appModel: appModel, startDiscovery: false) + controller._test_setGateways([gateway]) + controller._test_triggerAutoConnect() + + #expect(controller._test_didAutoConnect() == false) + } +} + diff --git a/apps/macos/Sources/OpenClaw/GatewayDiscoveryHelpers.swift b/apps/macos/Sources/OpenClaw/GatewayDiscoveryHelpers.swift index 4becd8b13c..a533b92ebb 100644 --- a/apps/macos/Sources/OpenClaw/GatewayDiscoveryHelpers.swift +++ b/apps/macos/Sources/OpenClaw/GatewayDiscoveryHelpers.swift @@ -15,19 +15,29 @@ enum GatewayDiscoveryHelpers { static func directUrl(for gateway: GatewayDiscoveryModel.DiscoveredGateway) -> String? { self.directGatewayUrl( - tailnetDns: gateway.tailnetDns, + serviceHost: gateway.serviceHost, + servicePort: gateway.servicePort, lanHost: gateway.lanHost, gatewayPort: gateway.gatewayPort) } static func directGatewayUrl( - tailnetDns: String?, + serviceHost: String?, + servicePort: Int?, lanHost: String?, gatewayPort: Int?) -> String? { - if let tailnetDns = self.sanitizedTailnetHost(tailnetDns) { - return "wss://\(tailnetDns)" + // Security: do not route using unauthenticated TXT hints (tailnetDns/lanHost/gatewayPort). + // Prefer the resolved service endpoint (SRV + A/AAAA). + if let host = self.trimmed(serviceHost), !host.isEmpty, + let port = servicePort, port > 0 + { + let scheme = port == 443 ? "wss" : "ws" + let portSuffix = port == 443 ? "" : ":\(port)" + return "\(scheme)://\(host)\(portSuffix)" } + + // Legacy fallback (best-effort): keep existing behavior when we couldn't resolve SRV. guard let lanHost = self.trimmed(lanHost), !lanHost.isEmpty else { return nil } let port = gatewayPort ?? 18789 return "ws://\(lanHost):\(port)" diff --git a/apps/macos/Sources/OpenClaw/GeneralSettings.swift b/apps/macos/Sources/OpenClaw/GeneralSettings.swift index 03855b7698..40a105d1cb 100644 --- a/apps/macos/Sources/OpenClaw/GeneralSettings.swift +++ b/apps/macos/Sources/OpenClaw/GeneralSettings.swift @@ -683,7 +683,9 @@ extension GeneralSettings { host: host, port: gateway.sshPort) self.state.remoteCliPath = gateway.cliPath ?? "" - OpenClawConfigFile.setRemoteGatewayUrl(host: host, port: gateway.gatewayPort) + OpenClawConfigFile.setRemoteGatewayUrl( + host: gateway.serviceHost ?? host, + port: gateway.servicePort ?? gateway.gatewayPort) } } } diff --git a/apps/macos/Sources/OpenClaw/OnboardingView+Actions.swift b/apps/macos/Sources/OpenClaw/OnboardingView+Actions.swift index bfffc39f15..47cce949db 100644 --- a/apps/macos/Sources/OpenClaw/OnboardingView+Actions.swift +++ b/apps/macos/Sources/OpenClaw/OnboardingView+Actions.swift @@ -35,7 +35,9 @@ extension OnboardingView { user: user, host: host, port: gateway.sshPort) - OpenClawConfigFile.setRemoteGatewayUrl(host: host, port: gateway.gatewayPort) + OpenClawConfigFile.setRemoteGatewayUrl( + host: gateway.serviceHost ?? host, + port: gateway.servicePort ?? gateway.gatewayPort) } self.state.remoteCliPath = gateway.cliPath ?? "" diff --git a/apps/macos/Sources/OpenClawDiscovery/GatewayDiscoveryModel.swift b/apps/macos/Sources/OpenClawDiscovery/GatewayDiscoveryModel.swift index c8cde804ec..27548d9065 100644 --- a/apps/macos/Sources/OpenClawDiscovery/GatewayDiscoveryModel.swift +++ b/apps/macos/Sources/OpenClawDiscovery/GatewayDiscoveryModel.swift @@ -20,6 +20,9 @@ public final class GatewayDiscoveryModel { public struct DiscoveredGateway: Identifiable, Equatable, Sendable { public var id: String { self.stableID } public var displayName: String + // Resolved service endpoint (SRV + A/AAAA). Used for routing; do not trust TXT for routing. + public var serviceHost: String? + public var servicePort: Int? public var lanHost: String? public var tailnetDns: String? public var sshPort: Int @@ -31,6 +34,8 @@ public final class GatewayDiscoveryModel { public init( displayName: String, + serviceHost: String? = nil, + servicePort: Int? = nil, lanHost: String? = nil, tailnetDns: String? = nil, sshPort: Int, @@ -41,6 +46,8 @@ public final class GatewayDiscoveryModel { isLocal: Bool) { self.displayName = displayName + self.serviceHost = serviceHost + self.servicePort = servicePort self.lanHost = lanHost self.tailnetDns = tailnetDns self.sshPort = sshPort @@ -62,8 +69,8 @@ public final class GatewayDiscoveryModel { private var localIdentity: LocalIdentity private let localDisplayName: String? private let filterLocalGateways: Bool - private var resolvedTXTByID: [String: [String: String]] = [:] - private var pendingTXTResolvers: [String: GatewayTXTResolver] = [:] + private var resolvedServiceByID: [String: ResolvedGatewayService] = [:] + private var pendingServiceResolvers: [String: GatewayServiceResolver] = [:] private var wideAreaFallbackTask: Task? private var wideAreaFallbackGateways: [DiscoveredGateway] = [] private let logger = Logger(subsystem: "ai.openclaw", category: "gateway-discovery") @@ -133,9 +140,9 @@ public final class GatewayDiscoveryModel { self.resultsByDomain = [:] self.gatewaysByDomain = [:] self.statesByDomain = [:] - self.resolvedTXTByID = [:] - self.pendingTXTResolvers.values.forEach { $0.cancel() } - self.pendingTXTResolvers = [:] + self.resolvedServiceByID = [:] + self.pendingServiceResolvers.values.forEach { $0.cancel() } + self.pendingServiceResolvers = [:] self.wideAreaFallbackTask?.cancel() self.wideAreaFallbackTask = nil self.wideAreaFallbackGateways = [] @@ -154,6 +161,8 @@ public final class GatewayDiscoveryModel { local: self.localIdentity) return DiscoveredGateway( displayName: beacon.displayName, + serviceHost: beacon.host, + servicePort: beacon.port, lanHost: beacon.lanHost, tailnetDns: beacon.tailnetDns, sshPort: beacon.sshPort ?? 22, @@ -195,7 +204,8 @@ public final class GatewayDiscoveryModel { let decodedName = BonjourEscapes.decode(name) let stableID = GatewayEndpointID.stableID(result.endpoint) - let resolvedTXT = self.resolvedTXTByID[stableID] ?? [:] + let resolved = self.resolvedServiceByID[stableID] + let resolvedTXT = resolved?.txt ?? [:] let txt = Self.txtDictionary(from: result).merging( resolvedTXT, uniquingKeysWith: { _, new in new }) @@ -208,8 +218,10 @@ public final class GatewayDiscoveryModel { let parsedTXT = Self.parseGatewayTXT(txt) - if parsedTXT.lanHost == nil || parsedTXT.tailnetDns == nil { - self.ensureTXTResolution( + // Always attempt NetService resolution for the endpoint (host/port and TXT). + // TXT is unauthenticated; do not use it for routing. + if resolved == nil { + self.ensureServiceResolution( stableID: stableID, serviceName: name, type: type, @@ -224,6 +236,8 @@ public final class GatewayDiscoveryModel { local: self.localIdentity) return DiscoveredGateway( displayName: prettyName, + serviceHost: resolved?.host, + servicePort: resolved?.port, lanHost: parsedTXT.lanHost, tailnetDns: parsedTXT.tailnetDns, sshPort: parsedTXT.sshPort, @@ -421,16 +435,16 @@ public final class GatewayDiscoveryModel { return target } - private func ensureTXTResolution( + private func ensureServiceResolution( stableID: String, serviceName: String, type: String, domain: String) { - guard self.resolvedTXTByID[stableID] == nil else { return } - guard self.pendingTXTResolvers[stableID] == nil else { return } + guard self.resolvedServiceByID[stableID] == nil else { return } + guard self.pendingServiceResolvers[stableID] == nil else { return } - let resolver = GatewayTXTResolver( + let resolver = GatewayServiceResolver( name: serviceName, type: type, domain: domain, @@ -438,10 +452,10 @@ public final class GatewayDiscoveryModel { { [weak self] result in Task { @MainActor in guard let self else { return } - self.pendingTXTResolvers[stableID] = nil + self.pendingServiceResolvers[stableID] = nil switch result { - case let .success(txt): - self.resolvedTXTByID[stableID] = txt + case let .success(resolved): + self.resolvedServiceByID[stableID] = resolved self.updateGatewaysForAllDomains() self.recomputeGateways() case .failure: @@ -450,7 +464,7 @@ public final class GatewayDiscoveryModel { } } - self.pendingTXTResolvers[stableID] = resolver + self.pendingServiceResolvers[stableID] = resolver resolver.start() } @@ -607,9 +621,15 @@ public final class GatewayDiscoveryModel { } } -final class GatewayTXTResolver: NSObject, NetServiceDelegate { +struct ResolvedGatewayService: Equatable, Sendable { + var txt: [String: String] + var host: String? + var port: Int? +} + +final class GatewayServiceResolver: NSObject, NetServiceDelegate { private let service: NetService - private let completion: (Result<[String: String], Error>) -> Void + private let completion: (Result) -> Void private let logger: Logger private var didFinish = false @@ -618,7 +638,7 @@ final class GatewayTXTResolver: NSObject, NetServiceDelegate { type: String, domain: String, logger: Logger, - completion: @escaping (Result<[String: String], Error>) -> Void) + completion: @escaping (Result) -> Void) { self.service = NetService(domain: domain, type: type, name: name) self.completion = completion @@ -633,24 +653,27 @@ final class GatewayTXTResolver: NSObject, NetServiceDelegate { } func cancel() { - self.finish(result: .failure(GatewayTXTResolverError.cancelled)) + self.finish(result: .failure(GatewayServiceResolverError.cancelled)) } func netServiceDidResolveAddress(_ sender: NetService) { let txt = Self.decodeTXT(sender.txtRecordData()) + let host = Self.normalizeHost(sender.hostName) + let port = sender.port > 0 ? sender.port : nil if !txt.isEmpty { let payload = self.formatTXT(txt) self.logger.debug( "discovery: resolved TXT for \(sender.name, privacy: .public): \(payload, privacy: .public)") } - self.finish(result: .success(txt)) + let resolved = ResolvedGatewayService(txt: txt, host: host, port: port) + self.finish(result: .success(resolved)) } func netService(_ sender: NetService, didNotResolve errorDict: [String: NSNumber]) { - self.finish(result: .failure(GatewayTXTResolverError.resolveFailed(errorDict))) + self.finish(result: .failure(GatewayServiceResolverError.resolveFailed(errorDict))) } - private func finish(result: Result<[String: String], Error>) { + private func finish(result: Result) { guard !self.didFinish else { return } self.didFinish = true self.service.stop() @@ -671,6 +694,12 @@ final class GatewayTXTResolver: NSObject, NetServiceDelegate { return out } + private static func normalizeHost(_ raw: String?) -> String? { + let trimmed = raw?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + if trimmed.isEmpty { return nil } + return trimmed.hasSuffix(".") ? String(trimmed.dropLast()) : trimmed + } + private func formatTXT(_ txt: [String: String]) -> String { txt.sorted(by: { $0.key < $1.key }) .map { "\($0.key)=\($0.value)" } @@ -678,7 +707,7 @@ final class GatewayTXTResolver: NSObject, NetServiceDelegate { } } -enum GatewayTXTResolverError: Error { +enum GatewayServiceResolverError: Error { case cancelled case resolveFailed([String: NSNumber]) } diff --git a/docs/gateway/bonjour.md b/docs/gateway/bonjour.md index 608d3b11ab..0f5d287f8d 100644 --- a/docs/gateway/bonjour.md +++ b/docs/gateway/bonjour.md @@ -100,6 +100,12 @@ The Gateway advertises small non‑secret hints to make UI flows convenient: - `cliPath=` (optional; absolute path to a runnable `openclaw` entrypoint) - `tailnetDns=` (optional hint when Tailnet is available) +Security notes: + +- Bonjour/mDNS TXT records are **unauthenticated**. Clients must not treat TXT as authoritative routing. +- Clients should route using the resolved service endpoint (SRV + A/AAAA). Treat `lanHost`, `tailnetDns`, `gatewayPort`, and `gatewayTlsSha256` as hints only. +- TLS pinning must never allow an advertised `gatewayTlsSha256` to override a previously stored pin. + ## Debugging on macOS Useful built‑in tools: diff --git a/docs/gateway/bridge-protocol.md b/docs/gateway/bridge-protocol.md index 1c23e38186..850de1c2d5 100644 --- a/docs/gateway/bridge-protocol.md +++ b/docs/gateway/bridge-protocol.md @@ -35,7 +35,9 @@ Legacy `bridge.*` config keys are no longer part of the config schema. - Legacy default listener port was `18790` (current builds do not start a TCP bridge). When TLS is enabled, discovery TXT records include `bridgeTls=1` plus -`bridgeTlsSha256` so nodes can pin the certificate. +`bridgeTlsSha256` as a non-secret hint. Note that Bonjour/mDNS TXT records are +unauthenticated; clients must not treat the advertised fingerprint as an +authoritative pin without explicit user intent or other out-of-band verification. ## Handshake + pairing diff --git a/docs/gateway/discovery.md b/docs/gateway/discovery.md index 6212df61e0..4bf7884d1c 100644 --- a/docs/gateway/discovery.md +++ b/docs/gateway/discovery.md @@ -68,6 +68,12 @@ Troubleshooting and beacon details: [Bonjour](/gateway/bonjour). - `cliPath=` (optional; absolute path to a runnable `openclaw` entrypoint or binary) - `tailnetDns=` (optional hint; auto-detected when Tailscale is available) +Security notes: + +- Bonjour/mDNS TXT records are **unauthenticated**. Clients must treat TXT values as UX hints only. +- Routing (host/port) should prefer the **resolved service endpoint** (SRV + A/AAAA) over TXT-provided `lanHost`, `tailnetDns`, or `gatewayPort`. +- TLS pinning must never allow an advertised `gatewayTlsSha256` to override a previously stored pin. For first-time connections, require explicit user intent (TOFU or other out-of-band verification). + Disable/override: - `OPENCLAW_DISABLE_BONJOUR=1` disables advertising. diff --git a/src/cli/gateway-cli/discover.test.ts b/src/cli/gateway-cli/discover.test.ts new file mode 100644 index 0000000000..bda8b70920 --- /dev/null +++ b/src/cli/gateway-cli/discover.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from "vitest"; +import type { GatewayBonjourBeacon } from "../../infra/bonjour-discovery.js"; +import { pickBeaconHost, pickGatewayPort } from "./discover.js"; + +describe("gateway discover routing helpers", () => { + it("prefers resolved service host over TXT hints", () => { + const beacon: GatewayBonjourBeacon = { + instanceName: "Test", + host: "10.0.0.2", + lanHost: "evil.example.com", + tailnetDns: "evil.example.com", + }; + expect(pickBeaconHost(beacon)).toBe("10.0.0.2"); + }); + + it("prefers resolved service port over TXT gatewayPort", () => { + const beacon: GatewayBonjourBeacon = { + instanceName: "Test", + host: "10.0.0.2", + port: 18789, + gatewayPort: 12345, + }; + expect(pickGatewayPort(beacon)).toBe(18789); + }); + + it("falls back to TXT host/port when resolve data is missing", () => { + const beacon: GatewayBonjourBeacon = { + instanceName: "Test", + lanHost: "test-host.local", + gatewayPort: 18789, + }; + expect(pickBeaconHost(beacon)).toBe("test-host.local"); + expect(pickGatewayPort(beacon)).toBe(18789); + }); +}); diff --git a/src/cli/gateway-cli/discover.ts b/src/cli/gateway-cli/discover.ts index 51a3b12f7d..8465cf449c 100644 --- a/src/cli/gateway-cli/discover.ts +++ b/src/cli/gateway-cli/discover.ts @@ -30,12 +30,15 @@ export function parseDiscoverTimeoutMs(raw: unknown, fallbackMs: number): number } export function pickBeaconHost(beacon: GatewayBonjourBeacon): string | null { - const host = beacon.tailnetDns || beacon.lanHost || beacon.host; + // Security: TXT records are unauthenticated. Prefer the resolved service endpoint (SRV/A/AAAA) + // over TXT-provided routing hints. + const host = beacon.host || beacon.tailnetDns || beacon.lanHost; return host?.trim() ? host.trim() : null; } export function pickGatewayPort(beacon: GatewayBonjourBeacon): number { - const port = beacon.gatewayPort ?? 18789; + // Security: TXT records are unauthenticated. Prefer the resolved service port over TXT gatewayPort. + const port = beacon.port ?? beacon.gatewayPort ?? 18789; return port > 0 ? port : 18789; } diff --git a/src/commands/onboard-remote.ts b/src/commands/onboard-remote.ts index 1ecea324e4..077f5fa6be 100644 --- a/src/commands/onboard-remote.ts +++ b/src/commands/onboard-remote.ts @@ -8,12 +8,14 @@ import { detectBinary } from "./onboard-helpers.js"; const DEFAULT_GATEWAY_URL = "ws://127.0.0.1:18789"; function pickHost(beacon: GatewayBonjourBeacon): string | undefined { - return beacon.tailnetDns || beacon.lanHost || beacon.host; + // Security: TXT is unauthenticated. Prefer the resolved service endpoint host. + return beacon.host || beacon.tailnetDns || beacon.lanHost; } function buildLabel(beacon: GatewayBonjourBeacon): string { const host = pickHost(beacon); - const port = beacon.gatewayPort ?? beacon.port ?? 18789; + // Security: Prefer the resolved service endpoint port. + const port = beacon.port ?? beacon.gatewayPort ?? 18789; const title = beacon.displayName ?? beacon.instanceName; const hint = host ? `${host}:${port}` : "host unknown"; return `${title} (${hint})`; @@ -80,7 +82,7 @@ export async function promptRemoteGatewayConfig( if (selectedBeacon) { const host = pickHost(selectedBeacon); - const port = selectedBeacon.gatewayPort ?? 18789; + const port = selectedBeacon.port ?? selectedBeacon.gatewayPort ?? 18789; if (host) { const mode = await prompter.select({ message: "Connection method",