From 329f8d6f45c69593ecf25830b70e8c1b7d5c49c7 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 23 Aug 2018 13:37:22 -0400 Subject: [PATCH 1/6] Log call session description. --- Signal/src/call/CallService.swift | 6 ++++-- Signal/src/call/PeerConnectionClient.swift | 11 +++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index a43b6a025..584e374e5 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -407,6 +407,8 @@ private class SignalCallData: NSObject { throw CallError.obsoleteCall(description: "Missing peerConnectionClient") } + Logger.info("session description for outgoing call: \(call.identifiersForLogs), sdp: \(sessionDescription.logSafeDescription).") + return peerConnectionClient.setLocalSessionDescription(sessionDescription).then { do { let offerBuilder = SSKProtoCallMessageOffer.SSKProtoCallMessageOfferBuilder(id: call.signalingId, @@ -708,12 +710,12 @@ private class SignalCallData: NSObject { // Find a sessionDescription compatible with my constraints and the remote sessionDescription return peerConnectionClient.negotiateSessionDescription(remoteDescription: offerSessionDescription, constraints: constraints) }.then { (negotiatedSessionDescription: HardenedRTCSessionDescription) in - Logger.debug("set the remote description for: \(newCall.identifiersForLogs)") - guard self.call == newCall else { throw CallError.obsoleteCall(description: "negotiateSessionDescription() response for obsolete call") } + Logger.info("session description for incoming call: \(newCall.identifiersForLogs), sdp: \(negotiatedSessionDescription.logSafeDescription).") + do { let answerBuilder = SSKProtoCallMessageAnswer.SSKProtoCallMessageAnswerBuilder(id: newCall.signalingId, sessionDescription: negotiatedSessionDescription.sdp) diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift index 3c13e9cfc..bc8ad66a0 100644 --- a/Signal/src/call/PeerConnectionClient.swift +++ b/Signal/src/call/PeerConnectionClient.swift @@ -1109,6 +1109,17 @@ class HardenedRTCSessionDescription { return RTCSessionDescription.init(type: rtcSessionDescription.type, sdp: description) } + + var logSafeDescription: String { + do { + let pattern = "\\d+\\.\\d+\\.\\d+\\.\\d+" + let regex = try NSRegularExpression(pattern: pattern, options: NSRegularExpression.Options.caseInsensitive) + let range = NSRange(location: 0, length: sdp.count) + return regex.stringByReplacingMatches(in: sdp, options: [], range: range, withTemplate: "") + } catch { + return "" + } + } } protocol VideoCaptureSettingsDelegate: class { From 2d06c05a4f0c75172bb208a0fe915bc965847807 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 23 Aug 2018 17:33:26 -0400 Subject: [PATCH 2/6] Log call session description. --- Signal/src/call/CallService.swift | 4 +- Signal/src/call/PeerConnectionClient.swift | 11 --- .../test/call/PeerConnectionClientTest.swift | 2 +- .../test/util/OWSScrubbingLogFormatterTest.m | 64 ++++++++++++++-- .../utils/OWSScrubbingLogFormatter.m | 73 ++++++++++++++++--- .../src/Contacts/PhoneNumberUtil.m | 25 ++----- 6 files changed, 131 insertions(+), 48 deletions(-) diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index 584e374e5..f5ae2086c 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -407,7 +407,7 @@ private class SignalCallData: NSObject { throw CallError.obsoleteCall(description: "Missing peerConnectionClient") } - Logger.info("session description for outgoing call: \(call.identifiersForLogs), sdp: \(sessionDescription.logSafeDescription).") + Logger.info("session description for outgoing call: \(call.identifiersForLogs), sdp: \(sessionDescription.sdp).") return peerConnectionClient.setLocalSessionDescription(sessionDescription).then { do { @@ -714,7 +714,7 @@ private class SignalCallData: NSObject { throw CallError.obsoleteCall(description: "negotiateSessionDescription() response for obsolete call") } - Logger.info("session description for incoming call: \(newCall.identifiersForLogs), sdp: \(negotiatedSessionDescription.logSafeDescription).") + Logger.info("session description for incoming call: \(newCall.identifiersForLogs), sdp: \(negotiatedSessionDescription.sdp).") do { let answerBuilder = SSKProtoCallMessageAnswer.SSKProtoCallMessageAnswerBuilder(id: newCall.signalingId, diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift index bc8ad66a0..3c13e9cfc 100644 --- a/Signal/src/call/PeerConnectionClient.swift +++ b/Signal/src/call/PeerConnectionClient.swift @@ -1109,17 +1109,6 @@ class HardenedRTCSessionDescription { return RTCSessionDescription.init(type: rtcSessionDescription.type, sdp: description) } - - var logSafeDescription: String { - do { - let pattern = "\\d+\\.\\d+\\.\\d+\\.\\d+" - let regex = try NSRegularExpression(pattern: pattern, options: NSRegularExpression.Options.caseInsensitive) - let range = NSRange(location: 0, length: sdp.count) - return regex.stringByReplacingMatches(in: sdp, options: [], range: range, withTemplate: "") - } catch { - return "" - } - } } protocol VideoCaptureSettingsDelegate: class { diff --git a/Signal/test/call/PeerConnectionClientTest.swift b/Signal/test/call/PeerConnectionClientTest.swift index a092df808..09fee6e17 100644 --- a/Signal/test/call/PeerConnectionClientTest.swift +++ b/Signal/test/call/PeerConnectionClientTest.swift @@ -130,7 +130,7 @@ class PeerConnectionClientTest: XCTestCase { XCTAssertEqual(1, clientDelegate.dataChannelMessages.count) let dataChannelMessageProto = clientDelegate.dataChannelMessages[0] - XCTAssert(dataChannelMessageProto.hasHangup) + XCTAssertNotNil(dataChannelMessageProto.hangup) let hangupProto = dataChannelMessageProto.hangup! XCTAssertEqual(123, hangupProto.id) diff --git a/Signal/test/util/OWSScrubbingLogFormatterTest.m b/Signal/test/util/OWSScrubbingLogFormatterTest.m index 0921a2b6b..27964ebeb 100644 --- a/Signal/test/util/OWSScrubbingLogFormatterTest.m +++ b/Signal/test/util/OWSScrubbingLogFormatterTest.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // #import "OWSScrubbingLogFormatter.h" @@ -9,22 +9,36 @@ NS_ASSUME_NONNULL_BEGIN @interface OWSScrubbingLogFormatterTest : XCTestCase +@property (nonatomic) NSDate *testDate; + @end @implementation OWSScrubbingLogFormatterTest +- (void)setUp +{ + [super setUp]; + + self.testDate = [NSDate new]; +} + +- (void)tearDown +{ + [super tearDown]; +} + - (DDLogMessage *)messageWithString:(NSString *)string { return [[DDLogMessage alloc] initWithMessage:string level:DDLogLevelInfo flag:0 context:0 - file:nil - function:nil + file:@"mock file name" + function:@"mock function name" line:0 tag:nil options:0 - timestamp:[NSDate new]]; + timestamp:self.testDate]; } - (void)testDataScrubbed @@ -75,7 +89,7 @@ NS_ASSUME_NONNULL_BEGIN } } -- (void)testNonPhonenumberNotScrubbed +- (void)testNonPhoneNumberNotScrubbed { OWSScrubbingLogFormatter *formatter = [OWSScrubbingLogFormatter new]; NSString *actual = @@ -85,6 +99,46 @@ NS_ASSUME_NONNULL_BEGIN XCTAssertNotEqual(NSNotFound, redactedRange.location, "Shouldn't touch non phone string."); } +- (void)testIPAddressesScrubbed +{ + id scrubbingFormatter = [OWSScrubbingLogFormatter new]; + id defaultFormatter = [DDLogFileFormatterDefault new]; + + NSDictionary *valueMap = @{ + @"0.0.0.0" : @"[ REDACTED_IP_ADDRESS:...0 ]", + @"127.0.0.1" : @"[ REDACTED_IP_ADDRESS:...1 ]", + @"255.255.255.255" : @"[ REDACTED_IP_ADDRESS:...255 ]", + @"1.2.3.4" : @"[ REDACTED_IP_ADDRESS:...4 ]", + @"0.0.0.0.0.0" : @"[ REDACTED_IP_ADDRESS:...0 ]", + @"255.255.255.255.255.255" : @"[ REDACTED_IP_ADDRESS:...255 ]", + }; + NSArray *messageFormats = @[ + @"a%@b", + @"http://%@", + @"%@ and %@ and %@", + @"%@", + ]; + + for (NSString *ipAddress in valueMap) { + NSString *redactedIPAddress = valueMap[ipAddress]; + + for (NSString *messageFormat in messageFormats) { + NSString *message = [messageFormat stringByReplacingOccurrencesOfString:@"%@" withString:ipAddress]; + + NSString *expectedRedactedMessage = [defaultFormatter + formatLogMessage:[self messageWithString:[messageFormat + stringByReplacingOccurrencesOfString:@"%@" + withString:redactedIPAddress]]]; + NSString *redactedMessage = [scrubbingFormatter formatLogMessage:[self messageWithString:message]]; + + XCTAssertEqualObjects(expectedRedactedMessage, redactedMessage); + + NSRange ipAddressRange = [redactedMessage rangeOfString:ipAddress]; + XCTAssertEqual(NSNotFound, ipAddressRange.location, "Failed to redact IP address: %@", redactedMessage); + } + } +} + @end NS_ASSUME_NONNULL_END diff --git a/SignalMessaging/utils/OWSScrubbingLogFormatter.m b/SignalMessaging/utils/OWSScrubbingLogFormatter.m index b46e46ff4..b2c812072 100644 --- a/SignalMessaging/utils/OWSScrubbingLogFormatter.m +++ b/SignalMessaging/utils/OWSScrubbingLogFormatter.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // #import "OWSScrubbingLogFormatter.h" @@ -8,14 +8,65 @@ NS_ASSUME_NONNULL_BEGIN @implementation OWSScrubbingLogFormatter +- (NSRegularExpression *)phoneRegex +{ + NSString *key = @"OWSScrubbingLogFormatter.phoneRegex"; + NSRegularExpression *_Nullable regex = NSThread.currentThread.threadDictionary[key]; + if (!regex) { + NSError *error; + regex = [NSRegularExpression regularExpressionWithPattern:@"\\+\\d{7,12}(\\d{3})" + options:NSRegularExpressionCaseInsensitive + error:&error]; + if (error || !regex) { + OWSFail(@"%@ could not compile regular expression: %@", self.logTag, error); + } + NSThread.currentThread.threadDictionary[key] = regex; + } + return regex; +} + +- (NSRegularExpression *)dataRegex +{ + NSString *key = @"OWSScrubbingLogFormatter.dataRegex"; + NSRegularExpression *_Nullable regex = NSThread.currentThread.threadDictionary[key]; + if (!regex) { + NSError *error; + regex = [NSRegularExpression regularExpressionWithPattern:@"<([\\da-f]{2})[\\da-f]{6}( [\\da-f]{8})*>" + options:NSRegularExpressionCaseInsensitive + error:&error]; + if (error || !regex) { + OWSFail(@"%@ could not compile regular expression: %@", self.logTag, error); + } + NSThread.currentThread.threadDictionary[key] = regex; + } + return regex; +} + +- (NSRegularExpression *)ipAddressRegex +{ + NSString *key = @"OWSScrubbingLogFormatter.ipAddressRegex"; + NSRegularExpression *_Nullable regex = NSThread.currentThread.threadDictionary[key]; + if (!regex) { + // Match IPv4 and IPv6 addresses. + // + // NOTE: the second group matches the last "quad/hex?" of the IPv4/IPv6 address. + NSError *error; + regex = [NSRegularExpression regularExpressionWithPattern:@"(\\d+\\.\\d+\\.)?\\d+\\.\\d+\\.\\d+\\.(\\d+)" + options:NSRegularExpressionCaseInsensitive + error:&error]; + if (error || !regex) { + OWSFail(@"%@ could not compile regular expression: %@", self.logTag, error); + } + NSThread.currentThread.threadDictionary[key] = regex; + } + return regex; +} + - (NSString *__nullable)formatLogMessage:(DDLogMessage *)logMessage { NSString *logString = [super formatLogMessage:logMessage]; - NSRegularExpression *phoneRegex = - [NSRegularExpression regularExpressionWithPattern:@"\\+\\d{7,12}(\\d{3})" - options:NSRegularExpressionCaseInsensitive - error:nil]; + NSRegularExpression *phoneRegex = self.phoneRegex; logString = [phoneRegex stringByReplacingMatchesInString:logString options:0 range:NSMakeRange(0, [logString length]) @@ -25,16 +76,18 @@ NS_ASSUME_NONNULL_BEGIN // We capture only the first two characters of the hex string for logging. // example log line: "Called someFunction with nsData: <01234567 89abcdef>" // scrubbed output: "Called someFunction with nsData: [ REDACTED_DATA:01 ]" - NSRegularExpression *dataRegex = - [NSRegularExpression regularExpressionWithPattern:@"<([\\da-f]{2})[\\da-f]{6}( [\\da-f]{8})*>" - options:NSRegularExpressionCaseInsensitive - error:nil]; - + NSRegularExpression *dataRegex = self.dataRegex; logString = [dataRegex stringByReplacingMatchesInString:logString options:0 range:NSMakeRange(0, [logString length]) withTemplate:@"[ REDACTED_DATA:$1... ]"]; + NSRegularExpression *ipAddressRegex = self.ipAddressRegex; + logString = [ipAddressRegex stringByReplacingMatchesInString:logString + options:0 + range:NSMakeRange(0, [logString length]) + withTemplate:@"[ REDACTED_IP_ADDRESS:...$2 ]"]; + return logString; } diff --git a/SignalServiceKit/src/Contacts/PhoneNumberUtil.m b/SignalServiceKit/src/Contacts/PhoneNumberUtil.m index e91cd8eff..5be24ae7d 100644 --- a/SignalServiceKit/src/Contacts/PhoneNumberUtil.m +++ b/SignalServiceKit/src/Contacts/PhoneNumberUtil.m @@ -18,28 +18,15 @@ @implementation PhoneNumberUtil -+ (NSObject *)sharedLock -{ - static dispatch_once_t onceToken; - static NSObject *lock = nil; - dispatch_once(&onceToken, ^{ - lock = [NSObject new]; - }); - return lock; -} - + (PhoneNumberUtil *)sharedThreadLocal { - @synchronized(self.sharedLock) - { - NSString *key = PhoneNumberUtil.logTag; - PhoneNumberUtil *_Nullable threadLocal = NSThread.currentThread.threadDictionary[key]; - if (!threadLocal) { - threadLocal = [PhoneNumberUtil new]; - NSThread.currentThread.threadDictionary[key] = threadLocal; - } - return threadLocal; + NSString *key = PhoneNumberUtil.logTag; + PhoneNumberUtil *_Nullable threadLocal = NSThread.currentThread.threadDictionary[key]; + if (!threadLocal) { + threadLocal = [PhoneNumberUtil new]; + NSThread.currentThread.threadDictionary[key] = threadLocal; } + return threadLocal; } - (instancetype)init { From b4539328e1c890ff550a5c49ef048cad73e9dff1 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 23 Aug 2018 17:38:32 -0400 Subject: [PATCH 3/6] Log call session description. --- .../utils/OWSScrubbingLogFormatter.m | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/SignalMessaging/utils/OWSScrubbingLogFormatter.m b/SignalMessaging/utils/OWSScrubbingLogFormatter.m index b2c812072..1cb851f67 100644 --- a/SignalMessaging/utils/OWSScrubbingLogFormatter.m +++ b/SignalMessaging/utils/OWSScrubbingLogFormatter.m @@ -10,9 +10,9 @@ NS_ASSUME_NONNULL_BEGIN - (NSRegularExpression *)phoneRegex { - NSString *key = @"OWSScrubbingLogFormatter.phoneRegex"; - NSRegularExpression *_Nullable regex = NSThread.currentThread.threadDictionary[key]; - if (!regex) { + static NSRegularExpression *regex = nil; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ NSError *error; regex = [NSRegularExpression regularExpressionWithPattern:@"\\+\\d{7,12}(\\d{3})" options:NSRegularExpressionCaseInsensitive @@ -20,16 +20,15 @@ NS_ASSUME_NONNULL_BEGIN if (error || !regex) { OWSFail(@"%@ could not compile regular expression: %@", self.logTag, error); } - NSThread.currentThread.threadDictionary[key] = regex; - } + }); return regex; } - (NSRegularExpression *)dataRegex { - NSString *key = @"OWSScrubbingLogFormatter.dataRegex"; - NSRegularExpression *_Nullable regex = NSThread.currentThread.threadDictionary[key]; - if (!regex) { + static NSRegularExpression *regex = nil; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ NSError *error; regex = [NSRegularExpression regularExpressionWithPattern:@"<([\\da-f]{2})[\\da-f]{6}( [\\da-f]{8})*>" options:NSRegularExpressionCaseInsensitive @@ -37,16 +36,15 @@ NS_ASSUME_NONNULL_BEGIN if (error || !regex) { OWSFail(@"%@ could not compile regular expression: %@", self.logTag, error); } - NSThread.currentThread.threadDictionary[key] = regex; - } + }); return regex; } - (NSRegularExpression *)ipAddressRegex { - NSString *key = @"OWSScrubbingLogFormatter.ipAddressRegex"; - NSRegularExpression *_Nullable regex = NSThread.currentThread.threadDictionary[key]; - if (!regex) { + static NSRegularExpression *regex = nil; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ // Match IPv4 and IPv6 addresses. // // NOTE: the second group matches the last "quad/hex?" of the IPv4/IPv6 address. @@ -57,8 +55,7 @@ NS_ASSUME_NONNULL_BEGIN if (error || !regex) { OWSFail(@"%@ could not compile regular expression: %@", self.logTag, error); } - NSThread.currentThread.threadDictionary[key] = regex; - } + }); return regex; } From 02daca11af01c951112e48b68a33946b60def206 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 24 Aug 2018 10:44:23 -0400 Subject: [PATCH 4/6] Redact ice-pwd from SDP. --- Signal/src/call/CallService.swift | 4 ++-- Signal/src/call/PeerConnectionClient.swift | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index f5ae2086c..584e374e5 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -407,7 +407,7 @@ private class SignalCallData: NSObject { throw CallError.obsoleteCall(description: "Missing peerConnectionClient") } - Logger.info("session description for outgoing call: \(call.identifiersForLogs), sdp: \(sessionDescription.sdp).") + Logger.info("session description for outgoing call: \(call.identifiersForLogs), sdp: \(sessionDescription.logSafeDescription).") return peerConnectionClient.setLocalSessionDescription(sessionDescription).then { do { @@ -714,7 +714,7 @@ private class SignalCallData: NSObject { throw CallError.obsoleteCall(description: "negotiateSessionDescription() response for obsolete call") } - Logger.info("session description for incoming call: \(newCall.identifiersForLogs), sdp: \(negotiatedSessionDescription.sdp).") + Logger.info("session description for incoming call: \(newCall.identifiersForLogs), sdp: \(negotiatedSessionDescription.logSafeDescription).") do { let answerBuilder = SSKProtoCallMessageAnswer.SSKProtoCallMessageAnswerBuilder(id: newCall.signalingId, diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift index 3c13e9cfc..a31b8850c 100644 --- a/Signal/src/call/PeerConnectionClient.swift +++ b/Signal/src/call/PeerConnectionClient.swift @@ -1109,6 +1109,21 @@ class HardenedRTCSessionDescription { return RTCSessionDescription.init(type: rtcSessionDescription.type, sdp: description) } + + var logSafeDescription: String { + var text = sdp + text = text.replacingOccurrences(of: "\r", with: "\n") + text = text.replacingOccurrences(of: "\n\n", with: "\n") + let lines = text.components(separatedBy: "\n") + let filteredLines: [String] = lines.map { line in + guard !line.contains("ice-pwd") else { + return "[ REDACTED ice-pwd ]" + } + return line + } + let filteredText = filteredLines.joined(separator: "\n") + return filteredText + } } protocol VideoCaptureSettingsDelegate: class { From 490ac5dd76f064e74e50c317f28786ee111dca42 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 24 Aug 2018 10:49:13 -0400 Subject: [PATCH 5/6] Redact ice-pwd from SDP. --- Signal/src/call/PeerConnectionClient.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift index a31b8850c..b295c3191 100644 --- a/Signal/src/call/PeerConnectionClient.swift +++ b/Signal/src/call/PeerConnectionClient.swift @@ -1111,6 +1111,7 @@ class HardenedRTCSessionDescription { } var logSafeDescription: String { + #if DEBUG var text = sdp text = text.replacingOccurrences(of: "\r", with: "\n") text = text.replacingOccurrences(of: "\n\n", with: "\n") @@ -1123,6 +1124,9 @@ class HardenedRTCSessionDescription { } let filteredText = filteredLines.joined(separator: "\n") return filteredText + #else + return sdp + #endif } } From 0b5b74a90167e1e1036c6ef6d64d199ef326d0ab Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 24 Aug 2018 14:08:38 -0400 Subject: [PATCH 6/6] Respond to CR. --- Signal/src/call/PeerConnectionClient.swift | 35 ++++++++++++++++++- .../test/util/OWSScrubbingLogFormatterTest.m | 20 ++++++----- .../utils/OWSScrubbingLogFormatter.m | 18 +++++----- 3 files changed, 54 insertions(+), 19 deletions(-) diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift index b295c3191..1a07f4ab2 100644 --- a/Signal/src/call/PeerConnectionClient.swift +++ b/Signal/src/call/PeerConnectionClient.swift @@ -1112,6 +1112,16 @@ class HardenedRTCSessionDescription { var logSafeDescription: String { #if DEBUG + return sdp + #else + return redactIPV6(sdp: redactIcePwd(sdp: sdp)) + #endif + } + + private func redactIcePwd(sdp: String) -> String { + #if DEBUG + return sdp + #else var text = sdp text = text.replacingOccurrences(of: "\r", with: "\n") text = text.replacingOccurrences(of: "\n\n", with: "\n") @@ -1124,8 +1134,31 @@ class HardenedRTCSessionDescription { } let filteredText = filteredLines.joined(separator: "\n") return filteredText - #else + #endif + } + + private func redactIPV6(sdp: String) -> String { + #if DEBUG return sdp + #else + + // Example values to match: + // + // * 2001:0db8:85a3:0000:0000:8a2e:0370:7334 + // * 2001:db8:85a3::8a2e:370:7334 + // * ::1 + // * :: + // * ::ffff:192.0.2.128 + // + // See: https://en.wikipedia.org/wiki/IPv6_addresshttps://en.wikipedia.org/wiki/IPv6_address + do { + let regex = try NSRegularExpression(pattern: "[\\da-f]*:[\\da-f]*:[\\da-f:\\.]*", + options: .caseInsensitive) + return regex.stringByReplacingMatches(in: sdp, options: [], range: NSRange(location: 0, length: sdp.count), withTemplate: "[ REDACTED_IPV6_ADDRESS ]") + } catch { + owsFail("Could not redact IPv6 addresses.") + return "[Could not redact IPv6 addresses.]" + } #endif } } diff --git a/Signal/test/util/OWSScrubbingLogFormatterTest.m b/Signal/test/util/OWSScrubbingLogFormatterTest.m index 27964ebeb..6aa0bf4fb 100644 --- a/Signal/test/util/OWSScrubbingLogFormatterTest.m +++ b/Signal/test/util/OWSScrubbingLogFormatterTest.m @@ -105,18 +105,20 @@ NS_ASSUME_NONNULL_BEGIN id defaultFormatter = [DDLogFileFormatterDefault new]; NSDictionary *valueMap = @{ - @"0.0.0.0" : @"[ REDACTED_IP_ADDRESS:...0 ]", - @"127.0.0.1" : @"[ REDACTED_IP_ADDRESS:...1 ]", - @"255.255.255.255" : @"[ REDACTED_IP_ADDRESS:...255 ]", - @"1.2.3.4" : @"[ REDACTED_IP_ADDRESS:...4 ]", - @"0.0.0.0.0.0" : @"[ REDACTED_IP_ADDRESS:...0 ]", - @"255.255.255.255.255.255" : @"[ REDACTED_IP_ADDRESS:...255 ]", + @"0.0.0.0" : @"[ REDACTED_IPV4_ADDRESS:...0 ]", + @"127.0.0.1" : @"[ REDACTED_IPV4_ADDRESS:...1 ]", + @"255.255.255.255" : @"[ REDACTED_IPV4_ADDRESS:...255 ]", + @"1.2.3.4" : @"[ REDACTED_IPV4_ADDRESS:...4 ]", }; NSArray *messageFormats = @[ @"a%@b", @"http://%@", + @"http://%@/", @"%@ and %@ and %@", @"%@", + @"%@ %@", + @"no ip address!", + @"", ]; for (NSString *ipAddress in valueMap) { @@ -125,16 +127,18 @@ NS_ASSUME_NONNULL_BEGIN for (NSString *messageFormat in messageFormats) { NSString *message = [messageFormat stringByReplacingOccurrencesOfString:@"%@" withString:ipAddress]; + NSString *unredactedMessage = [defaultFormatter formatLogMessage:[self messageWithString:messageFormat]]; NSString *expectedRedactedMessage = [defaultFormatter formatLogMessage:[self messageWithString:[messageFormat stringByReplacingOccurrencesOfString:@"%@" withString:redactedIPAddress]]]; NSString *redactedMessage = [scrubbingFormatter formatLogMessage:[self messageWithString:message]]; - XCTAssertEqualObjects(expectedRedactedMessage, redactedMessage); + XCTAssertEqualObjects( + expectedRedactedMessage, redactedMessage, @"Scrubbing failed for message: %@", unredactedMessage); NSRange ipAddressRange = [redactedMessage rangeOfString:ipAddress]; - XCTAssertEqual(NSNotFound, ipAddressRange.location, "Failed to redact IP address: %@", redactedMessage); + XCTAssertEqual(NSNotFound, ipAddressRange.location, "Failed to redact IP address: %@", unredactedMessage); } } } diff --git a/SignalMessaging/utils/OWSScrubbingLogFormatter.m b/SignalMessaging/utils/OWSScrubbingLogFormatter.m index 1cb851f67..c60d14cf0 100644 --- a/SignalMessaging/utils/OWSScrubbingLogFormatter.m +++ b/SignalMessaging/utils/OWSScrubbingLogFormatter.m @@ -40,16 +40,14 @@ NS_ASSUME_NONNULL_BEGIN return regex; } -- (NSRegularExpression *)ipAddressRegex +- (NSRegularExpression *)ipV4AddressRegex { static NSRegularExpression *regex = nil; static dispatch_once_t onceToken; dispatch_once(&onceToken, ^{ - // Match IPv4 and IPv6 addresses. - // - // NOTE: the second group matches the last "quad/hex?" of the IPv4/IPv6 address. + // NOTE: The group matches the last quad of the IPv4 address. NSError *error; - regex = [NSRegularExpression regularExpressionWithPattern:@"(\\d+\\.\\d+\\.)?\\d+\\.\\d+\\.\\d+\\.(\\d+)" + regex = [NSRegularExpression regularExpressionWithPattern:@"\\d+\\.\\d+\\.\\d+\\.(\\d+)" options:NSRegularExpressionCaseInsensitive error:&error]; if (error || !regex) { @@ -79,11 +77,11 @@ NS_ASSUME_NONNULL_BEGIN range:NSMakeRange(0, [logString length]) withTemplate:@"[ REDACTED_DATA:$1... ]"]; - NSRegularExpression *ipAddressRegex = self.ipAddressRegex; - logString = [ipAddressRegex stringByReplacingMatchesInString:logString - options:0 - range:NSMakeRange(0, [logString length]) - withTemplate:@"[ REDACTED_IP_ADDRESS:...$2 ]"]; + NSRegularExpression *ipV4AddressRegex = self.ipV4AddressRegex; + logString = [ipV4AddressRegex stringByReplacingMatchesInString:logString + options:0 + range:NSMakeRange(0, [logString length]) + withTemplate:@"[ REDACTED_IPV4_ADDRESS:...$1 ]"]; return logString; }