From b4c32c450e19912f785d2fcc9fbc5d414b680cc3 Mon Sep 17 00:00:00 2001 From: "David E. Weekly" Date: Wed, 2 Sep 2026 09:30:01 -0700 Subject: [PATCH 1/2] feat(stun): STUN server deduplication and concurrent multi-provider resolution --- CHANGELOG.md | 7 ++ Sources/SwiftFTR/STUN.swift | 145 ++++++++++++++++++++++------ Tests/SwiftFTRTests/STUNTests.swift | 18 +++- 3 files changed, 137 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e9c503..e6112ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,13 @@ Unreleased - `TraceOptions` and per-operation options on `trace(to:options:)` and `traceClassified(to:vpnContext:resolver:options:)` permit overriding `maxHops` on individual traces without reconfiguring the actor. +### Changed + +- STUN public IP fallback now resolves endpoints concurrently across worker threads to avoid + serial DNS stalls. Deduplicated the fallback server list by dropping redundant `stun1.l.google.com` + (which pointed to the exact same IP as `stun.l.google.com`), ensuring genuine multi-provider failover + between independent providers (Google and Cloudflare) on distinct networks. + ### Tooling - Pull requests are now gated on the DocC documentation build: broken doc links and diff --git a/Sources/SwiftFTR/STUN.swift b/Sources/SwiftFTR/STUN.swift index 58dccb7..d8a7af0 100644 --- a/Sources/SwiftFTR/STUN.swift +++ b/Sources/SwiftFTR/STUN.swift @@ -39,12 +39,11 @@ public struct PublicIPs: Sendable { } /// Well-known public STUN servers for fallback. -/// Uses multiple providers and ports for resilience. Cloudflare and Google's -/// STUN servers resolve to both v4 and v6 records; the family preference passed -/// to the resolver determines which is used. +/// Uses multiple independent providers on distinct networks for genuine redundancy. +/// Cloudflare and Google's STUN servers resolve to both v4 and v6 records; the family +/// preference passed to the resolver determines which is used. let stunServers: [(host: String, port: UInt16)] = [ ("stun.l.google.com", 19302), // Google (port 19302) - ("stun1.l.google.com", 19302), // Google backup (port 19302) ("stun.cloudflare.com", 3478), // Cloudflare (port 3478) ] @@ -102,42 +101,47 @@ enum STUNError: Error, CustomStringConvertible { /// /// Pass `family: AF_INET6` to discover the v6 public IP; the resolver, socket, /// and bind paths all dispatch on family. The XOR-MAPPED-ADDRESS parser handles -/// both v4 (Family 0x01) and v6 (Family 0x02) per RFC 5389 ยง15.2. -internal func stunGetPublicIP( - family: Int32, +/// Resolves a STUN server host and port for the given family. +internal func resolveSTUNServer( host: String, port: UInt16, - timeout: TimeInterval = 1.0, - interface: String? = nil, - sourceIP: String? = nil, - enableLogging: Bool = false -) throws -> STUNPublicIP { - guard timeout.isFinite, timeout > 0, timeout <= TimeInterval(Int32.max) else { - throw STUNError.invalidTimeout(timeout) - } - + family: Int32 +) -> (server: sockaddr_storage, serverLen: socklen_t)? { var hints = addrinfo( ai_flags: AI_ADDRCONFIG, ai_family: family, ai_socktype: SOCK_DGRAM, ai_protocol: IPPROTO_UDP, ai_addrlen: 0, ai_canonname: nil, ai_addr: nil, ai_next: nil) var res: UnsafeMutablePointer? = nil let resolveResult = getaddrinfo(host, String(port), &hints, &res) guard resolveResult == 0, let info = res, let sa = info.pointee.ai_addr else { - let error = errno - let famName = family == AF_INET6 ? "v6" : "v4" - throw STUNError.resolveFailed( - errno: error, - details: "Failed to resolve STUN server '\(host):\(port)' (\(famName))") + return nil } defer { freeaddrinfo(info) } - // Copy the resolved sockaddr into sockaddr_storage so we can sendto from a - // single family-agnostic value. var server = sockaddr_storage() let serverLen = socklen_t(min(MemoryLayout.size, Int(info.pointee.ai_addrlen))) _ = withUnsafeMutablePointer(to: &server) { dst in memcpy(dst, sa, Int(serverLen)) } + return (server, serverLen) +} +/// Family-parameterized STUN core with pre-resolved endpoint. Builds the binding request, +/// sends it, and parses the XOR-MAPPED-ADDRESS attribute. Supports both AF_INET and AF_INET6. +internal func stunGetPublicIP( + family: Int32, + server: sockaddr_storage, + serverLen: socklen_t, + serverLabel: String, + timeout: TimeInterval = 1.0, + interface: String? = nil, + sourceIP: String? = nil, + enableLogging: Bool = false +) throws -> STUNPublicIP { + guard timeout.isFinite, timeout > 0, timeout <= TimeInterval(Int32.max) else { + throw STUNError.invalidTimeout(timeout) + } + + var server = server let fd = socket(family, SOCK_DGRAM, IPPROTO_UDP) if fd < 0 { let error = errno @@ -198,7 +202,7 @@ internal func stunGetPublicIP( guard connectResult == 0 else { let error = errno throw STUNError.connectFailed( - errno: error, details: "Failed to connect to \(host):\(port)") + errno: error, details: "Failed to connect to \(serverLabel)") } // Set timeouts. A zero timeval disables SO_RCVTIMEO, so invalid values are @@ -246,7 +250,7 @@ internal func stunGetPublicIP( if sent < 0 { let error = errno throw STUNError.sendFailed( - errno: error, details: "Failed to send STUN request to \(host):\(port)") + errno: error, details: "Failed to send STUN request to \(serverLabel)") } // The connected UDP socket accepts responses only from the selected server. @@ -358,6 +362,31 @@ internal func parseSTUNBindingResponse( throw STUNError.invalidResponse("missing mapped address for requested family") } +/// Resolves and queries a STUN server by hostname and port. +internal func stunGetPublicIP( + family: Int32, + host: String, + port: UInt16, + timeout: TimeInterval = 1.0, + interface: String? = nil, + sourceIP: String? = nil, + enableLogging: Bool = false +) throws -> STUNPublicIP { + guard timeout.isFinite, timeout > 0, timeout <= TimeInterval(Int32.max) else { + throw STUNError.invalidTimeout(timeout) + } + guard let (server, serverLen) = resolveSTUNServer(host: host, port: port, family: family) else { + let error = errno + let famName = family == AF_INET6 ? "v6" : "v4" + throw STUNError.resolveFailed( + errno: error, + details: "Failed to resolve STUN server '\(host):\(port)' (\(famName))") + } + return try stunGetPublicIP( + family: family, server: server, serverLen: serverLen, serverLabel: "\(host):\(port)", + timeout: timeout, interface: interface, sourceIP: sourceIP, enableLogging: enableLogging) +} + /// Back-compat wrapper for v4-only callers. Same signature/behavior as before /// Stage 4; delegates to the family-parameterized `stunGetPublicIP`. func stunGetPublicIPv4( @@ -381,8 +410,36 @@ func stunGetPublicIPv6( // MARK: - Multi-Server STUN Fallback +private final class STUNResolvedEndpoints: @unchecked Sendable { + private let lock = NSLock() + private var endpoints: + [(host: String, port: UInt16, server: sockaddr_storage, serverLen: socklen_t)?] + + init(count: Int) { + self.endpoints = Array(repeating: nil, count: count) + } + + func set( + _ index: Int, + value: (host: String, port: UInt16, server: sockaddr_storage, serverLen: socklen_t) + ) { + lock.lock() + defer { lock.unlock() } + endpoints[index] = value + } + + func get(_ index: Int) + -> (host: String, port: UInt16, server: sockaddr_storage, serverLen: socklen_t)? + { + lock.lock() + defer { lock.unlock() } + return endpoints[index] + } +} + /// Attempts to discover the public IP of a given family by trying multiple STUN -/// servers in sequence. Falls back through the server list until one succeeds. +/// servers in sequence with concurrent DNS resolution. Falls back through the server +/// list until one succeeds. /// /// - Parameters: /// - family: `AF_INET` for v4 or `AF_INET6` for v6. @@ -402,21 +459,47 @@ internal func stunGetPublicIPWithFallback( var lastError: Error = STUNError.recvTimeout let famName = family == AF_INET6 ? "v6" : "v4" - for (host, port) in stunServers { + if enableLogging { + print("[STUN] Resolving \(stunServers.count) servers concurrently (\(famName))...") + } + + // Resolve all endpoints concurrently across worker threads to avoid serial DNS stalls + let collector = STUNResolvedEndpoints(count: stunServers.count) + + DispatchQueue.concurrentPerform(iterations: stunServers.count) { i in + let (host, port) = stunServers[i] + if let (storage, len) = resolveSTUNServer(host: host, port: port, family: family) { + collector.set(i, value: (host, port, storage, len)) + } + } + + for i in 0.. \(result.ip)") + print("[STUN] Success from \(endpoint.host):\(endpoint.port) (\(famName)) -> \(result.ip)") } return result } catch { if enableLogging { - print("[STUN] Failed \(host):\(port) (\(famName)): \(error)") + print("[STUN] Failed \(endpoint.host):\(endpoint.port) (\(famName)): \(error)") } lastError = error continue diff --git a/Tests/SwiftFTRTests/STUNTests.swift b/Tests/SwiftFTRTests/STUNTests.swift index 09ce8a5..bfbe45f 100644 --- a/Tests/SwiftFTRTests/STUNTests.swift +++ b/Tests/SwiftFTRTests/STUNTests.swift @@ -334,11 +334,25 @@ struct STUNTests { // MARK: - Multi-Server STUN Fallback Tests - @Test("STUN server list is populated") + @Test("STUN server list is populated with unique providers") func testSTUNServerList() { - #expect(stunServers.count >= 3) + #expect(stunServers.count >= 2) #expect(stunServers.contains { $0.host.contains("google") }) #expect(stunServers.contains { $0.host.contains("cloudflare") }) + let uniqueHosts = Set(stunServers.map(\.host)) + #expect( + uniqueHosts.count == stunServers.count, "STUN servers must not contain duplicate hostnames") + } + + @Test("resolveSTUNServer resolves valid host and rejects invalid host") + func testResolveSTUNServer() { + let resolved = resolveSTUNServer(host: "127.0.0.1", port: 3478, family: AF_INET) + #expect(resolved != nil) + #expect(resolved?.serverLen == socklen_t(MemoryLayout.size)) + + let invalid = resolveSTUNServer( + host: "invalid.domain.that.does.not.exist.example", port: 3478, family: AF_INET) + #expect(invalid == nil) } @Test( From 2d36e4211e80f2b8a3a50814cc586b89fd27582b Mon Sep 17 00:00:00 2001 From: "David E. Weekly" Date: Wed, 2 Sep 2026 17:24:52 -0700 Subject: [PATCH 2/2] fix(stun): validate timeout before DNS resolution in stunGetPublicIPWithFallback --- Sources/SwiftFTR/STUN.swift | 4 ++++ Tests/SwiftFTRTests/STUNTests.swift | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/Sources/SwiftFTR/STUN.swift b/Sources/SwiftFTR/STUN.swift index d8a7af0..697536b 100644 --- a/Sources/SwiftFTR/STUN.swift +++ b/Sources/SwiftFTR/STUN.swift @@ -456,6 +456,10 @@ internal func stunGetPublicIPWithFallback( sourceIP: String? = nil, enableLogging: Bool = false ) throws -> STUNPublicIP { + guard timeout.isFinite, timeout > 0, timeout <= TimeInterval(Int32.max) else { + throw STUNError.invalidTimeout(timeout) + } + var lastError: Error = STUNError.recvTimeout let famName = family == AF_INET6 ? "v6" : "v4" diff --git a/Tests/SwiftFTRTests/STUNTests.swift b/Tests/SwiftFTRTests/STUNTests.swift index bfbe45f..f3726f8 100644 --- a/Tests/SwiftFTRTests/STUNTests.swift +++ b/Tests/SwiftFTRTests/STUNTests.swift @@ -221,6 +221,18 @@ struct STUNTests { } } + @Test( + "stunGetPublicIPWithFallback rejects invalid timeouts before DNS", + arguments: [0.0, -1.0, .infinity]) + func testFallbackInvalidTimeout(timeout: TimeInterval) { + #expect { + try stunGetPublicIPWithFallback(family: AF_INET, timeout: timeout) + } throws: { error in + guard case STUNError.invalidTimeout = error else { return false } + return true + } + } + // MARK: - Integration with SwiftFTR @Test("SwiftFTR caches STUN result")