Harden inbound public-key handling (first-wins / TOFU)

Meshtastic/Helpers/MapDataManager.swift

@@ -102,20 +102,69 @@ class MapDataManager: ObservableObject {
 		return metadata
 	}
 
-	/// Downloads (`http`/`https`) or reads (`file`) a GeoJSON overlay from `urlString` and imports it
-	/// through the same pipeline as `processUploadedFile`. Lets an overlay be fetched and installed
-	/// without the file-picker UI — e.g. from the `importGeoJSON` deep link (see `Router.swift`).
-	func importFromRemote(urlString: String, session: URLSession = .shared) async throws -> MapDataMetadata {
+	/// Downloads an `http`/`https` GeoJSON overlay from `urlString` and imports it through the same
+	/// pipeline as `processUploadedFile`. Lets an overlay be fetched and installed without the
+	/// file-picker UI — e.g. from the `importGeoJSON` deep link (see `Router.swift`).
+	///
+	/// Because the deep-link caller is untrusted, this rejects any non-`http(s)` scheme and blocks
+	/// loopback / link-local / private / reserved hosts (SSRF hardening). Local `file://`
+	/// imports go through `processUploadedFile` directly, not this path.
+	func importFromRemote(urlString: String, session injectedSession: URLSession? = nil) async throws -> MapDataMetadata {
 		guard let url = URL(string: urlString) else {
 			throw MapDataError.invalidDestination
 		}
 
+		// This entry point is reachable from the `importGeoJSON` deep link with an
+		// attacker-controlled URL. Only allow `http(s)` — reject everything else (closing the previous
+		// `file://` fallthrough that let a deep link reach the local-file read pipeline).
 		guard url.scheme == "http" || url.scheme == "https" else {
-			// Already local (e.g. `file://`) — hand straight to the existing pipeline.
-			return try await processUploadedFile(from: url)
-		}
+			throw MapDataError.disallowedHost
+		}
+
+		// Block SSRF to loopback / link-local / private / reserved hosts. Resolve
+		// the host and reject if any resolved address falls in a non-routable / internal range, so an
+		// attacker deep link cannot probe localhost services, LAN admin pages, or cloud metadata
+		// (169.254.169.254) from the victim's network position.
+		guard let host = url.host, !host.isEmpty else {
+			throw MapDataError.disallowedHost
+		}
+		guard !Self.isDisallowedHost(host) else {
+			throw MapDataError.disallowedHost
+		}
+
+		// The up-front host check above can be defeated by two SSRF tricks that both re-enter DNS/URL
+		// handling *after* validation: (1) an HTTP redirect to an internal host, and (2) DNS rebinding
+		// (a short-TTL record that answers with a public IP at validation time and an internal IP when
+		// `URLSession` re-resolves at connect time). When the caller doesn't inject a session (tests do),
+		// build one backed by `SSRFGuardDelegate`: redirect re-validation is the reliable control; the
+		// connected-peer check is opportunistic (see below).
+		//
+		// WARNING: an *injected* session bypasses `SSRFGuardDelegate` entirely (redirect + peer checks).
+		// Injection exists only for the test stub, which uses fixed public hostnames; never route
+		// untrusted production URLs through a caller-supplied session.
+		let guardDelegate = SSRFGuardDelegate()
+		let session: URLSession
+		let ownsSession: Bool
+		if let injectedSession {
+			session = injectedSession
+			ownsSession = false
+		} else {
+			session = URLSession(configuration: .ephemeral, delegate: guardDelegate, delegateQueue: nil)
+			ownsSession = true
+		}
+		defer { if ownsSession { session.finishTasksAndInvalidate() } }
 
 		let (data, response) = try await session.data(from: url)
+
+		// Opportunistic DNS-rebinding catch: if the connection landed on an internal peer, discard the
+		// response so its content is never imported/rendered. This only fires when `didFinishCollecting`
+		// wins the race with the `data(from:)` continuation, so it catches a subset of rebinding cases
+		// rather than all of them — redirect re-validation remains the reliable control. `URLSession`
+		// exposes no pre-connect IP hook, so the initial GET can't be blocked outright here.
+		if guardDelegate.connectedToDisallowedPeer {
+			throw MapDataError.disallowedHost
+		}
+
 		if let httpResponse = response as? HTTPURLResponse, !(200..<300).contains(httpResponse.statusCode) {
 			throw MapDataError.invalidContent
 		}
@@ -456,6 +505,183 @@ class MapDataManager: ObservableObject {
 	}
 }
 
+// MARK: - SSRF host validation
+
+extension MapDataManager {
+
+	/// Returns `true` if `host` is an internal / non-routable destination that must not be fetched from
+	/// an attacker-supplied deep link. Resolves the host and rejects if it is `localhost`, an mDNS
+	/// `.local` name, or resolves to any loopback / link-local / private / reserved IP range.
+	static func isDisallowedHost(_ host: String) -> Bool {
+		let lowered = host.lowercased()
+		// Reject bare-name internal hosts outright (localhost + mDNS `.local`).
+		if lowered == "localhost" || lowered.hasSuffix(".local") {
+			return true
+		}
+
+		// A URL host may be an IPv6 literal wrapped in brackets — strip them before resolving.
+		let bareHost = lowered.hasPrefix("[") && lowered.hasSuffix("]")
+			? String(lowered.dropFirst().dropLast())
+			: lowered
+
+		// Resolve the host (handles both literal IPs and DNS names, closing the hostname→private-IP gap).
+		var hints = addrinfo(
+			ai_flags: 0,
+			ai_family: AF_UNSPEC,
+			ai_socktype: SOCK_STREAM,
+			ai_protocol: 0,
+			ai_addrlen: 0,
+			ai_canonname: nil,
+			ai_addr: nil,
+			ai_next: nil
+		)
+		var result: UnsafeMutablePointer<addrinfo>?
+		guard getaddrinfo(bareHost, nil, &hints, &result) == 0, let first = result else {
+			// Resolution failed — fail closed and treat as disallowed.
+			return true
+		}
+		defer { freeaddrinfo(result) }
+
+		var addr: UnsafeMutablePointer<addrinfo>? = first
+		while let current = addr {
+			if let sockaddr = current.pointee.ai_addr {
+				if current.pointee.ai_family == AF_INET {
+					let ipv4 = sockaddr.withMemoryRebound(to: sockaddr_in.self, capacity: 1) {
+						UInt32(bigEndian: $0.pointee.sin_addr.s_addr)
+					}
+					if isDisallowedIPv4(ipv4) { return true }
+				} else if current.pointee.ai_family == AF_INET6 {
+					var bytes = [UInt8](repeating: 0, count: 16)
+					sockaddr.withMemoryRebound(to: sockaddr_in6.self, capacity: 1) {
+						withUnsafeBytes(of: $0.pointee.sin6_addr) { raw in
+							for i in 0..<16 { bytes[i] = raw[i] }
+						}
+					}
+					if isDisallowedIPv6(bytes) { return true }
+				}
+			}
+			addr = current.pointee.ai_next
+		}
+		return false
+	}
+
+	/// `ip` is a host-order IPv4 address. Blocks loopback, private (RFC 1918), link-local,
+	/// CGNAT, "this host", benchmarking, and other reserved ranges.
+	private static func isDisallowedIPv4(_ ip: UInt32) -> Bool {
+		func octets(_ a: UInt8, _ b: UInt8, _ c: UInt8, _ d: UInt8) -> UInt32 {
+			(UInt32(a) << 24) | (UInt32(b) << 16) | (UInt32(c) << 8) | UInt32(d)
+		}
+		func inRange(_ base: UInt32, _ prefix: Int) -> Bool {
+			let mask: UInt32 = prefix == 0 ? 0 : ~UInt32(0) << (32 - prefix)
+			return (ip & mask) == (base & mask)
+		}
+		return inRange(octets(0, 0, 0, 0), 8)        // 0.0.0.0/8  "this host"
+			|| inRange(octets(10, 0, 0, 0), 8)       // 10.0.0.0/8 private
+			|| inRange(octets(100, 64, 0, 0), 10)    // 100.64.0.0/10 CGNAT
+			|| inRange(octets(127, 0, 0, 0), 8)      // 127.0.0.0/8 loopback
+			|| inRange(octets(169, 254, 0, 0), 16)   // 169.254.0.0/16 link-local (inc. 169.254.169.254)
+			|| inRange(octets(172, 16, 0, 0), 12)    // 172.16.0.0/12 private
+			|| inRange(octets(192, 0, 0, 0), 24)     // 192.0.0.0/24 IETF protocol assignments
+			|| inRange(octets(192, 0, 2, 0), 24)     // 192.0.2.0/24 TEST-NET-1 (RFC 5737)
+			|| inRange(octets(192, 168, 0, 0), 16)   // 192.168.0.0/16 private
+			|| inRange(octets(198, 18, 0, 0), 15)    // 198.18.0.0/15 benchmarking
+			|| inRange(octets(198, 51, 100, 0), 24)  // 198.51.100.0/24 TEST-NET-2 (RFC 5737)
+			|| inRange(octets(203, 0, 113, 0), 24)   // 203.0.113.0/24 TEST-NET-3 (RFC 5737)
+			|| inRange(octets(224, 0, 0, 0), 4)      // 224.0.0.0/4 multicast
+			|| inRange(octets(240, 0, 0, 0), 4)      // 240.0.0.0/4 reserved
+			|| inRange(octets(255, 255, 255, 255), 32) // 255.255.255.255/32 limited broadcast
+	}
+
+	/// `bytes` is a 16-byte IPv6 address. Blocks loopback (`::1`), unspecified (`::`), link-local
+	/// (`fe80::/10`), unique-local (`fc00::/7`), and IPv4-mapped addresses in a blocked v4 range.
+	private static func isDisallowedIPv6(_ bytes: [UInt8]) -> Bool {
+		guard bytes.count == 16 else { return true }
+		// Loopback ::1
+		if bytes[0..<15].allSatisfy({ $0 == 0 }) && bytes[15] == 1 { return true }
+		// Unspecified ::
+		if bytes.allSatisfy({ $0 == 0 }) { return true }
+		// Link-local fe80::/10
+		if bytes[0] == 0xfe && (bytes[1] & 0xc0) == 0x80 { return true }
+		// Unique-local fc00::/7
+		if (bytes[0] & 0xfe) == 0xfc { return true }
+		// IPv4-mapped ::ffff:a.b.c.d — re-check the embedded v4 address.
+		if bytes[0..<10].allSatisfy({ $0 == 0 }) && bytes[10] == 0xff && bytes[11] == 0xff {
+			let ipv4 = (UInt32(bytes[12]) << 24) | (UInt32(bytes[13]) << 16) | (UInt32(bytes[14]) << 8) | UInt32(bytes[15])
+			return isDisallowedIPv4(ipv4)
+		}
+		// NAT64 well-known prefix 64:ff9b::/96 — the last 4 bytes embed the real IPv4 target, so on a
+		// NAT64 network `64:ff9b::a9fe:a9fe` would otherwise reach 169.254.169.254. Re-check the embed.
+		if bytes[0] == 0x00 && bytes[1] == 0x64 && bytes[2] == 0xff && bytes[3] == 0x9b
+			&& bytes[4..<12].allSatisfy({ $0 == 0 }) {
+			let ipv4 = (UInt32(bytes[12]) << 24) | (UInt32(bytes[13]) << 16) | (UInt32(bytes[14]) << 8) | UInt32(bytes[15])
+			return isDisallowedIPv4(ipv4)
+		}
+		return false
+	}
+}
+
+// MARK: - SSRF-guarded URLSession delegate
+
+/// `URLSession` delegate that hardens `MapDataManager.importFromRemote` against the two SSRF tricks
+/// the up-front hostname check can't catch on its own:
+///
+/// - **HTTP redirects** — `willPerformHTTPRedirection` re-runs `MapDataManager.isDisallowedHost` on
+///   every hop and refuses to follow a redirect to an internal host, so a public allow-listed URL
+///   can't `302` the request onto `169.254.169.254`, `localhost`, or a LAN address. This is the
+///   primary, reliable control.
+/// - **DNS rebinding** — `URLSession` re-resolves DNS itself at connect time, so a short-TTL record
+///   can pass validation with a public IP yet connect to an internal one. `didFinishCollecting`
+///   reads the real peer address from the transaction metrics and flags it; the caller then discards
+///   the response. This is best-effort defense-in-depth (metrics-based, so it prevents the fetched
+///   content from being imported rather than blocking the initial GET, which `URLSession` gives no
+///   pre-connect hook to stop).
+private final class SSRFGuardDelegate: NSObject, URLSessionTaskDelegate, @unchecked Sendable {
+	private let lock = NSLock()
+	private var disallowedPeer = false
+
+	/// `true` once any transaction in the task connected to an internal / non-routable peer.
+	var connectedToDisallowedPeer: Bool {
+		lock.lock(); defer { lock.unlock() }
+		return disallowedPeer
+	}
+
+	private func flagDisallowedPeer() {
+		lock.lock(); defer { lock.unlock() }
+		disallowedPeer = true
+	}
+
+	func urlSession(
+		_ session: URLSession,
+		task: URLSessionTask,
+		willPerformHTTPRedirection response: HTTPURLResponse,
+		newRequest request: URLRequest,
+		completionHandler: @escaping (URLRequest?) -> Void
+	) {
+		guard let host = request.url?.host, !host.isEmpty, !MapDataManager.isDisallowedHost(host) else {
+			// Disallowed or unparseable redirect target — do not follow it. Passing `nil` returns the
+			// redirect response itself, which the caller treats as a non-2xx failure.
+			completionHandler(nil)
+			return
+		}
+		completionHandler(request)
+	}
+
+	func urlSession(
+		_ session: URLSession,
+		task: URLSessionTask,
+		didFinishCollecting metrics: URLSessionTaskMetrics
+	) {
+		for transaction in metrics.transactionMetrics {
+			guard let peer = transaction.remoteAddress, !peer.isEmpty else { continue }
+			// `remoteAddress` is a literal IP; `isDisallowedHost` resolves literals locally (no network).
+			if MapDataManager.isDisallowedHost(peer) {
+				flagDisallowedPeer()
+				return
+			}
+		}
+	}
+}
+
 // MARK: - Supporting Types
 
 /// Metadata for uploaded map data files
@@ -502,6 +728,7 @@ enum MapDataError: Error, LocalizedError {
 	case invalidDestination
 	case fileNotFound
 	case saveFailed
+	case disallowedHost
 
 	var errorDescription: String? {
 		switch self {
@@ -521,6 +748,8 @@ enum MapDataError: Error, LocalizedError {
 			return "File not found."
 		case .saveFailed:
 			return "Failed to save file."
+		case .disallowedHost:
+			return String(localized: "This URL is not allowed.")
 		}
 	}
 }

Meshtastic/Helpers/MeshPackets.swift

@@ -581,11 +581,9 @@ actor MeshPackets {
 							modelContext.insert(newUserEntity)
 							fetchedNode[0].user = newUserEntity
 						}
-						// Set the public key for a user if it is empty, don't update
-						if fetchedNode[0].user?.publicKey == nil && !nodeInfo.user.publicKey.isEmpty {
-							fetchedNode[0].user?.pkiEncrypted = true
-							fetchedNode[0].user?.publicKey = nodeInfo.user.publicKey
-						}
+						// First-wins on the public key, consistent with the NodeInfo/User paths in UpdateSwiftData
+						// (previously a `== nil` guard here silently ignored mismatches). See `applyInboundPublicKey`.
+						fetchedNode[0].user?.applyInboundPublicKey(nodeInfo.user.publicKey, nodeNum: Int64(nodeInfo.num))
 						fetchedNode[0].user?.userId = nodeInfo.num.toHex()
 						fetchedNode[0].user?.num = Int64(nodeInfo.num)
 						fetchedNode[0].user?.numString = String(nodeInfo.num)

Meshtastic/Model/UserEntity.swift

@@ -6,6 +6,7 @@
 //
 
 import Foundation
+import OSLog
 import SwiftData
 
 @Model
@@ -38,3 +39,50 @@ final class UserEntity {
 
 	init() {}
 }
+
+// MARK: - First-wins public-key handling
+
+extension UserEntity {
+
+	/// The result of applying an inbound public key to this user via `applyInboundPublicKey(_:nodeNum:)`.
+	enum PublicKeyUpdateOutcome: Equatable {
+		/// The inbound key was empty — nothing to apply.
+		case ignoredEmpty
+		/// No key was on file yet, so the inbound key was stored (and `pkiEncrypted` set).
+		case stored
+		/// The inbound key matched the stored key — no change.
+		case matched
+		/// A *different* key arrived for a contact that already has one. The stored key is kept
+		/// (first-wins); `keyMatch` is cleared and `newPublicKey` records the rejected key so the UI
+		/// trust indicators surface the possible key-substitution attempt.
+		case mismatch
+	}
+
+	/// Applies **first-wins** semantics for an inbound public key.
+	///
+	/// Once a non-empty key is stored for a contact it is never silently overwritten by a different
+	/// inbound key — that would let any mesh/MQTT peer substitute a trusted contact's key and MITM the
+	/// victim's PKC direct messages. A differing key is refused and surfaced to the UI (`keyMatch =
+	/// false`, `newPublicKey = inbound`) rather than only logged; the stored key stands.
+	///
+	/// - Parameters:
+	///   - inboundKey: The public key carried by the inbound NodeInfo/User protobuf.
+	///   - nodeNum: The node number, used only for the security log line on a mismatch.
+	/// - Returns: The outcome, primarily for tests and callers that want to react to a mismatch.
+	@discardableResult
+	func applyInboundPublicKey(_ inboundKey: Data, nodeNum: Int64) -> PublicKeyUpdateOutcome {
+		guard !inboundKey.isEmpty else { return .ignoredEmpty }
+
+		if let storedKey = publicKey, !storedKey.isEmpty {
+			guard storedKey != inboundKey else { return .matched }
+			keyMatch = false
+			newPublicKey = inboundKey
+			Logger.data.error("🔐 [Security] Ignoring inbound public key change for node \(nodeNum, privacy: .public); a different key is already stored (possible key-substitution attempt).")
+			return .mismatch
+		}
+
+		pkiEncrypted = true
+		publicKey = inboundKey
+		return .stored
+	}
+}

Meshtastic/Persistence/NodeBackupManager+Import.swift

@@ -40,6 +40,11 @@ extension NodeBackupManager {
 		let backupUsers = try backupContext.fetch(FetchDescriptor<UserEntity>())
 		var usersByNum: [Int64: UserEntity] = [:]
 		for src in backupUsers {
+			// Each `dst` is a freshly-created entity that we insert into the live context — this copy never
+			// overwrites the public key of an already-populated *live* record, so the inbound-mesh
+			// first-wins key protection (UpdateSwiftData / MeshPackets) doesn't apply here. This is a
+			// user-initiated restore of the user's own trusted backup, carrying `keyMatch`/`newPublicKey`
+			// forward verbatim.
 			let dst = UserEntity()
 			dst.hwDisplayName = src.hwDisplayName
 			dst.hwModel = src.hwModel

Meshtastic/Persistence/UpdateSwiftData.swift

@@ -511,10 +511,8 @@ extension MeshPackets {
 								fetchedNode[0].user?.unmessagable = false
 							}
 						}
-						if !nodeInfoMessage.user.publicKey.isEmpty {
-							fetchedNode[0].user?.pkiEncrypted = true
-							fetchedNode[0].user?.publicKey = nodeInfoMessage.user.publicKey
-						}
+						// Security (finding H1): first-wins on the public key. See `applyInboundPublicKey`.
+						fetchedNode[0].user?.applyInboundPublicKey(nodeInfoMessage.user.publicKey, nodeNum: Int64(nodeInfoMessage.num))
 						if let user = fetchedNode.first?.user {
 							let fetchHwModel2 = Int64(user.hwModelId)
 							let hwDescriptor2 = FetchDescriptor<DeviceHardwareEntity>(
@@ -546,10 +544,8 @@ extension MeshPackets {
 						let containsRole = roles.contains(Int(fetchedNode[0].user?.role ?? -1))
 						fetchedNode[0].user?.unmessagable = containsRole
 					}
-					if !userMessage.publicKey.isEmpty {
-						fetchedNode[0].user?.pkiEncrypted = true
-						fetchedNode[0].user?.publicKey = userMessage.publicKey
-					}
+					// Security (finding H1): first-wins on the public key. See `applyInboundPublicKey`.
+					fetchedNode[0].user?.applyInboundPublicKey(userMessage.publicKey, nodeNum: Int64(packet.from))
 					if packet.hopStart != 0 && packet.hopLimit <= packet.hopStart {
 						fetchedNode[0].hopsAway = Int32(packet.hopStart - packet.hopLimit)
 					}