From 7f7e6d534b228b161b1b10b1792c904cd308299c Mon Sep 17 00:00:00 2001 From: Maciek Grzybowski Date: Tue, 26 Apr 2022 15:20:36 +0200 Subject: [PATCH 1/2] RUMM-2153 Prevent Kronos logic from querying privte IPs --- Datadog/Datadog.xcodeproj/project.pbxproj | 6 + .../Datadog/Kronos/KronosDNSResolver.swift | 1 + .../Kronos/KronosInternetAddress.swift | 46 +++++++ .../Kronos/KronosInternetAddressTests.swift | 120 ++++++++++++++++++ .../SystemFrameworks/FoundationMocks.swift | 6 +- 5 files changed, 177 insertions(+), 2 deletions(-) create mode 100644 Tests/DatadogTests/Datadog/Kronos/KronosInternetAddressTests.swift diff --git a/Datadog/Datadog.xcodeproj/project.pbxproj b/Datadog/Datadog.xcodeproj/project.pbxproj index 8393e3f558..9eb20cfbf5 100644 --- a/Datadog/Datadog.xcodeproj/project.pbxproj +++ b/Datadog/Datadog.xcodeproj/project.pbxproj @@ -353,6 +353,8 @@ 61B7885D25C180CB002675B5 /* DatadogCrashReporting.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 61B7885425C180CB002675B5 /* DatadogCrashReporting.framework */; }; 61B7886225C180CB002675B5 /* DDCrashReportingPluginTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61B7886125C180CB002675B5 /* DDCrashReportingPluginTests.swift */; }; 61B7886425C180CB002675B5 /* DatadogCrashReporting.h in Headers */ = {isa = PBXBuildFile; fileRef = 61B7885625C180CB002675B5 /* DatadogCrashReporting.h */; settings = {ATTRIBUTES = (Public, ); }; }; + 61B8BA91281812F60068AFF4 /* KronosInternetAddressTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61B8BA90281812F60068AFF4 /* KronosInternetAddressTests.swift */; }; + 61B8BA92281812F60068AFF4 /* KronosInternetAddressTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61B8BA90281812F60068AFF4 /* KronosInternetAddressTests.swift */; }; 61B9ED1C2461E12000C0DCFF /* SendLogsFixtureViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61B9ED1A2461E12000C0DCFF /* SendLogsFixtureViewController.swift */; }; 61B9ED1D2461E12000C0DCFF /* SendTracesFixtureViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61B9ED1B2461E12000C0DCFF /* SendTracesFixtureViewController.swift */; }; 61B9ED1F2461E57700C0DCFF /* UITestsHelpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61B9ED1E2461E57700C0DCFF /* UITestsHelpers.swift */; }; @@ -1549,6 +1551,7 @@ 61B7885C25C180CB002675B5 /* DatadogCrashReportingTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = DatadogCrashReportingTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; 61B7886125C180CB002675B5 /* DDCrashReportingPluginTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DDCrashReportingPluginTests.swift; sourceTree = ""; }; 61B7886325C180CB002675B5 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; + 61B8BA90281812F60068AFF4 /* KronosInternetAddressTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KronosInternetAddressTests.swift; sourceTree = ""; }; 61B9ED1A2461E12000C0DCFF /* SendLogsFixtureViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SendLogsFixtureViewController.swift; sourceTree = ""; }; 61B9ED1B2461E12000C0DCFF /* SendTracesFixtureViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SendTracesFixtureViewController.swift; sourceTree = ""; }; 61B9ED1E2461E57700C0DCFF /* UITestsHelpers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UITestsHelpers.swift; sourceTree = ""; }; @@ -3603,6 +3606,7 @@ children = ( 61D3E0DF277B3D92008BE766 /* KronosNTPPacketTests.swift */, 61D3E0E2277B3D92008BE766 /* KronosTimeStorageTests.swift */, + 61B8BA90281812F60068AFF4 /* KronosInternetAddressTests.swift */, ); path = Kronos; sourceTree = ""; @@ -5182,6 +5186,7 @@ D248ED4A28081D7500B315B4 /* RUMTelemetryTests.swift in Sources */, 61B0387D252724AB00518F3C /* URLSessionSwizzlerTests.swift in Sources */, 617CD0DD24CEDDD300B0B557 /* RUMUserActionScopeTests.swift in Sources */, + 61B8BA91281812F60068AFF4 /* KronosInternetAddressTests.swift in Sources */, D29889C9273413ED00A4D1A9 /* RUMViewsHandlerTests.swift in Sources */, 61C5A8A024509C1100DA608C /* Casting+Tracing.swift in Sources */, 61133C662423990D00786299 /* LogSanitizerTests.swift in Sources */, @@ -5801,6 +5806,7 @@ D2CB6ED727C520D400A62B57 /* URLSessionSwizzlerTests.swift in Sources */, D248ED4B28081D7E00B315B4 /* RUMTelemetryTests.swift in Sources */, D2CB6ED827C520D400A62B57 /* RUMUserActionScopeTests.swift in Sources */, + 61B8BA92281812F60068AFF4 /* KronosInternetAddressTests.swift in Sources */, D2CB6ED927C520D400A62B57 /* RUMViewsHandlerTests.swift in Sources */, D2CB6EDA27C520D400A62B57 /* Casting+Tracing.swift in Sources */, D2CB6EDB27C520D400A62B57 /* LogSanitizerTests.swift in Sources */, diff --git a/Sources/Datadog/Kronos/KronosDNSResolver.swift b/Sources/Datadog/Kronos/KronosDNSResolver.swift index 042de51de6..2ff6cfab84 100644 --- a/Sources/Datadog/Kronos/KronosDNSResolver.swift +++ b/Sources/Datadog/Kronos/KronosDNSResolver.swift @@ -49,6 +49,7 @@ internal final class KronosDNSResolver { let IPs = (addresses.takeUnretainedValue() as NSArray) .compactMap { $0 as? NSData } .compactMap(KronosInternetAddress.init) + .filter { ip in !ip.isPrivate } // to avoid querying private IPs, see: https://github.com/DataDog/dd-sdk-ios/issues/647 resolver.completion?(IPs) retainedSelf.release() diff --git a/Sources/Datadog/Kronos/KronosInternetAddress.swift b/Sources/Datadog/Kronos/KronosInternetAddress.swift index 9c912a87ff..9c894cb669 100644 --- a/Sources/Datadog/Kronos/KronosInternetAddress.swift +++ b/Sources/Datadog/Kronos/KronosInternetAddress.swift @@ -43,6 +43,52 @@ internal enum KronosInternetAddress: Hashable { } } + /// If the address is reserved for private internets (local / private IP). + var isPrivate: Bool { + guard let host = host else { + return false + } + + switch self { + case .ipv6: + // Ref.: https://datatracker.ietf.org/doc/html/rfc4193#section-3 + // +--------+-+------------+-----------+----------------------------+ + // | 7 bits |1| 40 bits | 16 bits | 64 bits | + // +--------+-+------------+-----------+----------------------------+ + // | Prefix |L| Global ID | Subnet ID | Interface ID | + // +--------+-+------------+-----------+----------------------------+ + // + // Local IP is expected to have FC00::/7 prefix (7 bits) and L byte set to 1, + // which effectively means `fd` prefix for local IPs. + let localPrefix = "fd" + + // Ref.: https://datatracker.ietf.org/doc/html/rfc4291#section-2.4 + let multicastPrefix = "ff" + + let hostLowercased = host.lowercased() + return hostLowercased.starts(with: localPrefix) + || hostLowercased.starts(with: multicastPrefix) + case .ipv4: + // Ref.: https://datatracker.ietf.org/doc/html/rfc1918#section-3 + // Local IPs have predefined ranges: + // - class A: 10.0.0.0 — 10.255.255.255 + // - class B: 172.16.0.0 — 172.31.255.255 + // - class C: 192.168.0.0 — 192.168.255.255 + let classABCregex = #"^((10\.)|(172\.1[6-9]\.)|(172\.2[0-9]\.)|(172\.3[0-1]\.)|(192\.168\.))"# + + // Ref.: https://datatracker.ietf.org/doc/html/rfc5771#section-3 + // Multicast address (range 224.0.0.0 - 239.255.255.255) are considered to be local network + // addresses too (https://developer.apple.com/forums/thread/663848) + // + let multicastRegex = #"^((22[4-9]\.)|(23[0-9]\.))"# + let broadcastIP = "255.255.255.255" + + return host.range(of: classABCregex, options: .regularExpression) != nil + || host.range(of: multicastRegex, options: .regularExpression) != nil + || host == broadcastIP + } + } + func hash(into hasher: inout Hasher) { hasher.combine(self.host) } diff --git a/Tests/DatadogTests/Datadog/Kronos/KronosInternetAddressTests.swift b/Tests/DatadogTests/Datadog/Kronos/KronosInternetAddressTests.swift new file mode 100644 index 0000000000..a432ac3b7a --- /dev/null +++ b/Tests/DatadogTests/Datadog/Kronos/KronosInternetAddressTests.swift @@ -0,0 +1,120 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2019-2020 Datadog, Inc. + */ + +import XCTest +@testable import Datadog + +class KronosInternetAddressTests: XCTestCase { + func testIfIPv4AddressIsPrivate() throws { + let privateIPs: [KronosInternetAddress] = try (0..<50).flatMap { _ in + return [ + // random private IPs of class A: 10.0.0.0 — 10.255.255.255 + try .mockIPv4([10, .mockRandom(), .mockRandom(), .mockRandom()]), + // random private IPs of class B: 172.16.0.0 — 172.31.255.255 + try .mockIPv4([172, .mockRandom(min: 16, max: 31), .mockRandom(), .mockRandom()]), + // random private IPs of class C: 192.168.0.0 — 192.168.255.255 + try .mockIPv4([192, 168, .mockRandom(), .mockRandom()]), + // multicast IPs 224.0.0.0 - 239.255.255.255 + try .mockIPv4([.mockRandom(min: 224, max: 239), .mockRandom(), .mockRandom(), .mockRandom()]), + // broadcast IP 255.255.255.255 + try .mockIPv4([255, 255, 255, 255]), + ] + } + let publicIPs: [KronosInternetAddress] = try (0..<50).flatMap { _ in + return [ + try .mockIPv4([.mockRandom(otherThan: Set([10, 172, 192, 255] + (224...239))), .mockRandom(), .mockRandom(), .mockRandom()]), + try .mockIPv4([172, .mockRandom(min: 0, max: 15), .mockRandom(), .mockRandom()]), + try .mockIPv4([172, .mockRandom(min: 32, max: 255), .mockRandom(), .mockRandom()]), + try .mockIPv4([192, .mockRandom(otherThan: [168]), .mockRandom(), .mockRandom()]), + try .mockIPv4([255, .mockRandom(max: 254), .mockRandom(), .mockRandom()]), + try .mockIPv4([255, .mockRandom(), .mockRandom(max: 254), .mockRandom()]), + try .mockIPv4([255, .mockRandom(), .mockRandom(), .mockRandom(max: 254)]), + ] + } + + privateIPs.forEach { ip in + XCTAssertTrue(ip.isPrivate, "\(ip.host ?? "nil") should be private IP") + } + publicIPs.forEach { ip in + XCTAssertFalse(ip.isPrivate, "\(ip.host ?? "nil") should not be private IP") + } + } + + func testIfIPv6AddressIsPrivate() throws { + let privateIPs: [KronosInternetAddress] = try (0..<50).flatMap { _ in + return [ + // random private IP starting with `fd` prefix + try .mockIPv6([0xfd] + (0..<15).map({ _ in .mockRandom() })), + // random multicast IP starting with `ff` prefix + try .mockIPv6([0xff] + (0..<15).map({ _ in .mockRandom() })), + ] + } + let publicIPs: [KronosInternetAddress] = try (0..<50).flatMap { _ in + return [ + // first byte is mocked to avoid having `fd` or `ff` prefix + try .mockIPv6([.mockRandom(min: 0xf0, otherThan: [0xfd, 0xff])] + (0..<15).map({ _ in .mockRandom() })), + try .mockIPv6([.mockRandom(max: 0xfc, otherThan: [0xf])] + (0..<15).map({ _ in .mockRandom() })), + ] + } + + privateIPs.forEach { ip in + XCTAssertTrue(ip.isPrivate, "\(ip.host ?? "nil") should be private IP") + } + publicIPs.forEach { ip in + XCTAssertFalse(ip.isPrivate, "\(ip.host ?? "nil") should not be private IP") + } + } +} + +// MARK: - Mocks + +private extension KronosInternetAddress { + static func mockIPv4(_ bytes: [UInt8]) throws -> KronosInternetAddress { + precondition(bytes.count == 4, "Expected 4 bytes") + let numbers = bytes.map { String($0) } + let ipv4String = numbers.joined(separator: ".") // e.g. '192.168.1.1' + let address: KronosInternetAddress? = .mockWith(ipv4String: ipv4String) + return try XCTUnwrap(address, "\(ipv4String) is not a valid IPv4 string") + } + + static func mockIPv6(_ bytes: [UInt8]) throws -> KronosInternetAddress { + precondition(bytes.count == 16, "Expected 16 bytes") + let groups: [String] = (0..<8).map { idx in + let hexA = String(bytes[idx * 2], radix: 16) + let hexB = String(bytes[idx * 2 + 1], radix: 16) + return hexA + hexB + } + let ipv6String = groups.joined(separator: ":") // e.g. 'ab:ab:ab:ab:ab:ab:ab:ab' + let address: KronosInternetAddress? = .mockWith(ipv6String: ipv6String.randomcased()) + return try XCTUnwrap(address, "\(ipv6String) is not a valid IPv6 string") + } + + static func mockWith(ipv4String: String) -> KronosInternetAddress? { + var inaddr = in_addr() + guard ipv4String.withCString({ inet_pton(AF_INET, $0, &inaddr) }) == 1 else { + return nil // likely, not an IPv4 string + } + + var addr = sockaddr_in() + addr.sin_len = UInt8(MemoryLayout.size(ofValue: addr)) + addr.sin_family = sa_family_t(AF_INET) + addr.sin_addr = inaddr + return .ipv4(addr) + } + + static func mockWith(ipv6String: String) -> KronosInternetAddress? { + var inaddr = in6_addr() + guard ipv6String.withCString({ inet_pton(AF_INET6, $0, &inaddr) }) == 1 else { + return nil // likely, not an IPv6 string + } + + var addr = sockaddr_in6() + addr.sin6_len = UInt8(MemoryLayout.size(ofValue: addr)) + addr.sin6_family = sa_family_t(AF_INET6) + addr.sin6_addr = inaddr + return .ipv6(addr) + } +} diff --git a/Tests/DatadogTests/Datadog/Mocks/SystemFrameworks/FoundationMocks.swift b/Tests/DatadogTests/Datadog/Mocks/SystemFrameworks/FoundationMocks.swift index 3cc6de5267..2a25b94152 100644 --- a/Tests/DatadogTests/Datadog/Mocks/SystemFrameworks/FoundationMocks.swift +++ b/Tests/DatadogTests/Datadog/Mocks/SystemFrameworks/FoundationMocks.swift @@ -270,8 +270,10 @@ extension FixedWidthInteger where Self: RandomMockable { return .random(in: min...max) } - static func mockRandom(min: Self = .min, max: Self = .max) -> Self { - return .random(in: min...max) + static func mockRandom(min: Self = .min, max: Self = .max, otherThan values: Set = []) -> Self { + var random: Self = .random(in: min...max) + while values.contains(random) { random = .random(in: min...max) } + return random } } From 25d9eaecac43a1384648fafc79bfcf05610b443e Mon Sep 17 00:00:00 2001 From: Maciek Grzybowski Date: Wed, 27 Apr 2022 11:53:16 +0200 Subject: [PATCH 2/2] RUMM-2153 Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51d2595817..1f0fa32e76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +* [BUGFIX] Fix rare problem with bringing up the "Local Network Permission" alert. See [#830][] + # 1.11.0-beta1 / 04-26-2022 ### Changes @@ -347,6 +349,7 @@ [#795]: https://github.com/DataDog/dd-sdk-ios/issues/795 [#797]: https://github.com/DataDog/dd-sdk-ios/issues/797 [#815]: https://github.com/DataDog/dd-sdk-ios/issues/815 +[#830]: https://github.com/DataDog/dd-sdk-ios/issues/830 [@00FA9A]: https://github.com/00FA9A [@Britton-Earnin]: https://github.com/Britton-Earnin [@Hengyu]: https://github.com/Hengyu