From 95fd61f1016d28996c74ac65a621749f118145ec Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 29 Mar 2024 11:03:28 +0100 Subject: [PATCH 1/4] convert DecryptionFailureTracker to swift + tests --- Riot/Modules/Analytics/Analytics.swift | 55 +++-- .../Modules/Analytics/DecryptionFailure.swift | 5 +- .../Analytics/DecryptionFailureTracker.h | 55 ----- .../Analytics/DecryptionFailureTracker.m | 174 ---------------- .../Analytics/DecryptionFailureTracker.swift | 138 ++++++++++++ Riot/Modules/Application/LegacyAppDelegate.m | 1 - Riot/Utils/EventFormatter.m | 1 - RiotTests/DecryptionFailureTrackerTests.swift | 196 ++++++++++++++++++ RiotTests/FakeUtils.swift | 109 ++++++++++ 9 files changed, 479 insertions(+), 255 deletions(-) delete mode 100644 Riot/Modules/Analytics/DecryptionFailureTracker.h delete mode 100644 Riot/Modules/Analytics/DecryptionFailureTracker.m create mode 100644 Riot/Modules/Analytics/DecryptionFailureTracker.swift create mode 100644 RiotTests/DecryptionFailureTrackerTests.swift create mode 100644 RiotTests/FakeUtils.swift diff --git a/Riot/Modules/Analytics/Analytics.swift b/Riot/Modules/Analytics/Analytics.swift index 123b09cd5a..849efd0b87 100644 --- a/Riot/Modules/Analytics/Analytics.swift +++ b/Riot/Modules/Analytics/Analytics.swift @@ -213,6 +213,38 @@ import AnalyticsEvents } } +@objc +protocol E2EAnalytics { + func trackE2EEError(_ reason: DecryptionFailureReason, context: String) +} + + +@objc extension Analytics: E2EAnalytics { + + /// Track an E2EE error that occurred + /// - Parameters: + /// - reason: The error that occurred. + /// - context: Additional context of the error that occured + func trackE2EEError(_ reason: DecryptionFailureReason, context: String) { + let event = AnalyticsEvent.Error( + context: context, + cryptoModule: .Rust, + cryptoSDK: AnalyticsEvent.Error.CryptoSDK.Rust, + domain: .E2EE, + // XXX not yet supported. + eventLocalAgeMillis: nil, + isFederated: nil, + isMatrixDotOrg: nil, + name: reason.errorName, + timeToDecryptMillis: nil, + userTrustsOwnIdentity: nil, + wasVisibleToUser: nil + ) + capture(event: event) + } + +} + // MARK: - Public tracking methods // The following methods are exposed for compatibility with Objective-C as // the `capture` method and the generated events cannot be bridged from Swift. @@ -266,28 +298,7 @@ extension Analytics { func trackInteraction(_ uiElement: AnalyticsUIElement) { trackInteraction(uiElement, interactionType: .Touch, index: nil) } - - /// Track an E2EE error that occurred - /// - Parameters: - /// - reason: The error that occurred. - /// - context: Additional context of the error that occured - func trackE2EEError(_ reason: DecryptionFailureReason, context: String) { - let event = AnalyticsEvent.Error( - context: context, - cryptoModule: .Rust, - cryptoSDK: .Rust, - domain: .E2EE, - // XXX not yet supported. - eventLocalAgeMillis: nil, - isFederated: nil, - isMatrixDotOrg: nil, - name: reason.errorName, - timeToDecryptMillis: nil, - userTrustsOwnIdentity: nil, - wasVisibleToUser: nil - ) - capture(event: event) - } + /// Track when a user becomes unauthenticated without pressing the `sign out` button. /// - Parameters: diff --git a/Riot/Modules/Analytics/DecryptionFailure.swift b/Riot/Modules/Analytics/DecryptionFailure.swift index 1c991db88f..b5804d258a 100644 --- a/Riot/Modules/Analytics/DecryptionFailure.swift +++ b/Riot/Modules/Analytics/DecryptionFailure.swift @@ -38,15 +38,16 @@ import AnalyticsEvents /// The id of the event that was unabled to decrypt. let failedEventId: String /// The time the failure has been reported. - let ts: TimeInterval = Date().timeIntervalSince1970 + let ts: TimeInterval /// Decryption failure reason. let reason: DecryptionFailureReason /// Additional context of failure let context: String - init(failedEventId: String, reason: DecryptionFailureReason, context: String) { + init(failedEventId: String, reason: DecryptionFailureReason, context: String, ts: TimeInterval) { self.failedEventId = failedEventId self.reason = reason self.context = context + self.ts = ts } } diff --git a/Riot/Modules/Analytics/DecryptionFailureTracker.h b/Riot/Modules/Analytics/DecryptionFailureTracker.h deleted file mode 100644 index b8f9ca467e..0000000000 --- a/Riot/Modules/Analytics/DecryptionFailureTracker.h +++ /dev/null @@ -1,55 +0,0 @@ -/* - Copyright 2018 New Vector Ltd - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - */ - -#import - -@class DecryptionFailureTracker; - -@class Analytics; -@import MatrixSDK; - -@interface DecryptionFailureTracker : NSObject - -/** - Returns the shared tracker. - - @return the shared tracker. - */ -+ (instancetype)sharedInstance; - -/** - The delegate object to receive analytics events. - */ -@property (nonatomic, weak) Analytics *delegate; - -/** - Report an event unable to decrypt. - - This error can be momentary. The DecryptionFailureTracker will check if it gets - fixed. Else, it will generate a failure (@see `trackFailures`). - - @param event the event. - @param roomState the room state when the event was received. - @param userId my user id. - */ -- (void)reportUnableToDecryptErrorForEvent:(MXEvent*)event withRoomState:(MXRoomState*)roomState myUser:(NSString*)userId; - -/** - Flush current data. - */ -- (void)dispatch; - -@end diff --git a/Riot/Modules/Analytics/DecryptionFailureTracker.m b/Riot/Modules/Analytics/DecryptionFailureTracker.m deleted file mode 100644 index 4a749b71aa..0000000000 --- a/Riot/Modules/Analytics/DecryptionFailureTracker.m +++ /dev/null @@ -1,174 +0,0 @@ -/* - Copyright 2018 New Vector Ltd - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - */ - -#import "DecryptionFailureTracker.h" -#import "GeneratedInterface-Swift.h" - - -// Call `checkFailures` every `CHECK_INTERVAL` -#define CHECK_INTERVAL 2 - -// Give events a chance to be decrypted by waiting `GRACE_PERIOD` before counting -// and reporting them as failures -#define GRACE_PERIOD 4 - -// E2E failures analytics category. -NSString *const kDecryptionFailureTrackerAnalyticsCategory = @"e2e.failure"; - -@interface DecryptionFailureTracker() -{ - // Reported failures - // Every `CHECK_INTERVAL`, this list is checked for failures that happened - // more than`GRACE_PERIOD` ago. Those that did are reported to the delegate. - NSMutableDictionary *reportedFailures; - - // Event ids of failures that were tracked previously - NSMutableSet *trackedEvents; - - // Timer for periodic check - NSTimer *checkFailuresTimer; -} -@end - -@implementation DecryptionFailureTracker - -+ (instancetype)sharedInstance -{ - static DecryptionFailureTracker *sharedInstance = nil; - static dispatch_once_t onceToken; - - dispatch_once(&onceToken, ^{ - sharedInstance = [[DecryptionFailureTracker alloc] init]; - }); - - return sharedInstance; -} - -- (instancetype)init -{ - self = [super init]; - if (self) - { - reportedFailures = [NSMutableDictionary dictionary]; - trackedEvents = [NSMutableSet set]; - - [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(eventDidDecrypt:) name:kMXEventDidDecryptNotification object:nil]; - - checkFailuresTimer = [NSTimer scheduledTimerWithTimeInterval:CHECK_INTERVAL - target:self - selector:@selector(checkFailures) - userInfo:nil - repeats:YES]; - } - return self; -} - -- (void)reportUnableToDecryptErrorForEvent:(MXEvent *)event withRoomState:(MXRoomState *)roomState myUser:(NSString *)userId -{ - if (reportedFailures[event.eventId] || [trackedEvents containsObject:event.eventId]) - { - return; - } - - // Filter out "expected" UTDs - // We cannot decrypt messages sent before the user joined the room - MXRoomMember *myUser = [roomState.members memberWithUserId:userId]; - if (!myUser || myUser.membership != MXMembershipJoin) - { - return; - } - - NSString *failedEventId = event.eventId; - DecryptionFailureReason reason; - - // Categorise the error - switch (event.decryptionError.code) - { - case MXDecryptingErrorUnknownInboundSessionIdCode: - reason = DecryptionFailureReasonOlmKeysNotSent; - break; - - case MXDecryptingErrorOlmCode: - reason = DecryptionFailureReasonOlmIndexError; - break; - - default: - // All other error codes will be tracked as `OlmUnspecifiedError` and will include `context` containing - // the actual error code and localized description - reason = DecryptionFailureReasonUnspecified; - break; - } - - NSString *context = [NSString stringWithFormat:@"code: %ld, description: %@", event.decryptionError.code, event.decryptionError.localizedDescription]; - reportedFailures[event.eventId] = [[DecryptionFailure alloc] initWithFailedEventId:failedEventId - reason:reason - context:context]; -} - -- (void)dispatch -{ - [self checkFailures]; -} - -#pragma mark - Private methods - -/** - Mark reported failures that occured before tsNow - GRACE_PERIOD as failures that should be - tracked. - */ -- (void)checkFailures -{ - if (!_delegate) - { - return; - } - - NSTimeInterval tsNow = [NSDate date].timeIntervalSince1970; - - NSMutableArray *failuresToTrack = [NSMutableArray array]; - - for (DecryptionFailure *reportedFailure in reportedFailures.allValues) - { - if (reportedFailure.ts < tsNow - GRACE_PERIOD) - { - [failuresToTrack addObject:reportedFailure]; - [reportedFailures removeObjectForKey:reportedFailure.failedEventId]; - [trackedEvents addObject:reportedFailure.failedEventId]; - } - } - - if (failuresToTrack.count) - { - // Sort failures by error reason - NSMutableDictionary *failuresCounts = [NSMutableDictionary dictionary]; - for (DecryptionFailure *failure in failuresToTrack) - { - failuresCounts[@(failure.reason)] = @(failuresCounts[@(failure.reason)].unsignedIntegerValue + 1); - [self.delegate trackE2EEError:failure.reason context:failure.context]; - } - - MXLogDebug(@"[DecryptionFailureTracker] trackFailures: %@", failuresCounts); - } -} - -- (void)eventDidDecrypt:(NSNotification *)notif -{ - // Could be an event in the reportedFailures, remove it - MXEvent *event = notif.object; - [reportedFailures removeObjectForKey:event.eventId]; -} - -@end diff --git a/Riot/Modules/Analytics/DecryptionFailureTracker.swift b/Riot/Modules/Analytics/DecryptionFailureTracker.swift new file mode 100644 index 0000000000..f33f2b7c52 --- /dev/null +++ b/Riot/Modules/Analytics/DecryptionFailureTracker.swift @@ -0,0 +1,138 @@ +// +// Copyright 2024 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + + +// Protocol to get the current time. Used for easy testing +protocol TimeProvider { + func nowTs() -> TimeInterval +} + +class DefaultTimeProvider: TimeProvider { + + func nowTs() -> TimeInterval { + return Date.now.timeIntervalSince1970 + } + +} + + +@objc +class DecryptionFailureTracker: NSObject { + + let GRACE_PERIOD: TimeInterval = 4 + // Call `checkFailures` every `CHECK_INTERVAL` + let CHECK_INTERVAL: TimeInterval = 2 + + @objc weak var delegate: E2EAnalytics? + + // Reported failures + var reportedFailures = [String /* eventId */: DecryptionFailure]() + + // Event ids of failures that were tracked previously + var trackedEvents = Set() + + var checkFailuresTimer: Timer? + + @objc static let sharedInstance = DecryptionFailureTracker() + + var timeProvider: TimeProvider = DefaultTimeProvider() + + override init() { + super.init() + + NotificationCenter.default.addObserver(self, + selector: #selector(eventDidDecrypt(_:)), + name: .mxEventDidDecrypt, + object: nil) + + Timer.scheduledTimer(withTimeInterval: CHECK_INTERVAL, repeats: true) { [weak self] _ in + self?.checkFailures() + } + } + + @objc + func reportUnableToDecryptError(forEvent event: MXEvent, withRoomState roomState: MXRoomState, myUser userId: String) { + if reportedFailures[event.eventId] != nil || trackedEvents.contains(event.eventId) { + return + } + + // Filter out "expected" UTDs + // We cannot decrypt messages sent before the user joined the room + guard let myUser = roomState.members.member(withUserId: userId) else { return } + if myUser.membership != MXMembership.join { + return + } + + guard let failedEventId = event.eventId else { return } + + guard let error = event.decryptionError as? NSError else { return } + + var reason = DecryptionFailureReason.unspecified + + if error.code == MXDecryptingErrorUnknownInboundSessionIdCode.rawValue { + reason = DecryptionFailureReason.olmKeysNotSent + } else if error.code == MXDecryptingErrorOlmCode.rawValue { + reason = DecryptionFailureReason.olmIndexError + } + + let context = String(format: "code: %ld, description: %@", error.code, event.decryptionError.localizedDescription) + + reportedFailures[failedEventId] = DecryptionFailure(failedEventId: failedEventId, reason: reason, context: context, ts: self.timeProvider.nowTs()) + } + + @objc + func dispatch() { + self.checkFailures() + } + + @objc + func eventDidDecrypt(_ notification: Notification) { + guard let event = notification.object as? MXEvent else { return } + + // Could be an event in the reportedFailures, remove it + reportedFailures.removeValue(forKey: event.eventId) + } + + /** + Mark reported failures that occured before tsNow - GRACE_PERIOD as failures that should be + tracked. + */ + @objc + func checkFailures() { + guard let delegate = self.delegate else {return} + + + let tsNow = self.timeProvider.nowTs() + var failuresToCheck = [DecryptionFailure]() + + for reportedFailure in self.reportedFailures.values { + let ellapsed = tsNow - reportedFailure.ts + if ellapsed > GRACE_PERIOD { + failuresToCheck.append(reportedFailure) + reportedFailures.removeValue(forKey: reportedFailure.failedEventId) + trackedEvents.insert(reportedFailure.failedEventId) + } + } + + for failure in failuresToCheck { + delegate.trackE2EEError(failure.reason, context: failure.context) + } + + } + +} diff --git a/Riot/Modules/Application/LegacyAppDelegate.m b/Riot/Modules/Application/LegacyAppDelegate.m index ab17eeac5c..79d263cef1 100644 --- a/Riot/Modules/Application/LegacyAppDelegate.m +++ b/Riot/Modules/Application/LegacyAppDelegate.m @@ -33,7 +33,6 @@ #import "ContactDetailsViewController.h" #import "BugReportViewController.h" -#import "DecryptionFailureTracker.h" #import "Tools.h" #import "WidgetManager.h" diff --git a/Riot/Utils/EventFormatter.m b/Riot/Utils/EventFormatter.m index d25655a226..4494bdbb2d 100644 --- a/Riot/Utils/EventFormatter.m +++ b/Riot/Utils/EventFormatter.m @@ -23,7 +23,6 @@ #import "WidgetManager.h" #import "MXDecryptionResult.h" -#import "DecryptionFailureTracker.h" #import diff --git a/RiotTests/DecryptionFailureTrackerTests.swift b/RiotTests/DecryptionFailureTrackerTests.swift new file mode 100644 index 0000000000..9b5fe94ea4 --- /dev/null +++ b/RiotTests/DecryptionFailureTrackerTests.swift @@ -0,0 +1,196 @@ +// +// Copyright 2024 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +import XCTest +@testable import Element + + +class DecryptionFailureTrackerTests: XCTestCase { + + class TimeShifter: TimeProvider { + + var timestamp = TimeInterval(0) + + func nowTs() -> TimeInterval { + return timestamp + } + } + + class AnalyticsDelegate : E2EAnalytics { + var reportedFailure: Element.DecryptionFailureReason?; + + func trackE2EEError(_ reason: Element.DecryptionFailureReason, context: String) { + print("Error Tracked: ", reason) + reportedFailure = reason + } + + } + + let timeShifter = TimeShifter() + + func test_grace_period() { + + let myUser = "test@example.com"; + + let decryptionFailureTracker = DecryptionFailureTracker(); + decryptionFailureTracker.timeProvider = timeShifter; + + let testDelegate = AnalyticsDelegate(); + + decryptionFailureTracker.delegate = testDelegate; + + timeShifter.timestamp = TimeInterval(0) + + let fakeEvent = FakeEvent(id: "$0000"); + fakeEvent.decryptionError = NSError(domain: MXDecryptingErrorDomain, code: Int(MXDecryptingErrorUnknownInboundSessionIdCode.rawValue)) + + + let fakeRoomState = FakeRoomState(); + fakeRoomState.mockMembers = FakeRoomMembers(joined: [myUser]) + decryptionFailureTracker.reportUnableToDecryptError(forEvent: fakeEvent, withRoomState: fakeRoomState, myUser: myUser); + + timeShifter.timestamp = TimeInterval(2) + + decryptionFailureTracker.checkFailures(); + + XCTAssertNil(testDelegate.reportedFailure); + + // Pass the grace period + timeShifter.timestamp = TimeInterval(5) + + decryptionFailureTracker.checkFailures(); + + XCTAssertEqual(testDelegate.reportedFailure, DecryptionFailureReason.olmKeysNotSent); + } + + func test_do_not_double_report() { + + let myUser = "test@example.com"; + + let decryptionFailureTracker = DecryptionFailureTracker(); + decryptionFailureTracker.timeProvider = timeShifter; + + let testDelegate = AnalyticsDelegate(); + + decryptionFailureTracker.delegate = testDelegate; + + timeShifter.timestamp = TimeInterval(0) + + let fakeEvent = FakeEvent(id: "$0000"); + fakeEvent.decryptionError = NSError(domain: MXDecryptingErrorDomain, code: Int(MXDecryptingErrorUnknownInboundSessionIdCode.rawValue)) + + + let fakeRoomState = FakeRoomState(); + fakeRoomState.mockMembers = FakeRoomMembers(joined: [myUser]) + + decryptionFailureTracker.reportUnableToDecryptError(forEvent: fakeEvent, withRoomState: fakeRoomState, myUser: myUser); + + // Pass the grace period + timeShifter.timestamp = TimeInterval(5) + + decryptionFailureTracker.checkFailures(); + + XCTAssertEqual(testDelegate.reportedFailure, DecryptionFailureReason.olmKeysNotSent); + + // Try to report again the same event + testDelegate.reportedFailure = nil + decryptionFailureTracker.reportUnableToDecryptError(forEvent: fakeEvent, withRoomState: fakeRoomState, myUser: myUser); + // Pass the grace period + timeShifter.timestamp = TimeInterval(10) + + decryptionFailureTracker.checkFailures(); + + XCTAssertNil(testDelegate.reportedFailure); + } + + + func test_ignore_not_member() { + + let myUser = "test@example.com"; + + let decryptionFailureTracker = DecryptionFailureTracker(); + decryptionFailureTracker.timeProvider = timeShifter; + + let testDelegate = AnalyticsDelegate(); + + decryptionFailureTracker.delegate = testDelegate; + + timeShifter.timestamp = TimeInterval(0) + + let fakeEvent = FakeEvent(id: "$0000"); + fakeEvent.decryptionError = NSError(domain: MXDecryptingErrorDomain, code: Int(MXDecryptingErrorUnknownInboundSessionIdCode.rawValue)) + + + let fakeRoomState = FakeRoomState(); + let fakeMembers = FakeRoomMembers() + fakeMembers.mockMembers[myUser] = MXMembership.ban + fakeRoomState.mockMembers = fakeMembers + + decryptionFailureTracker.reportUnableToDecryptError(forEvent: fakeEvent, withRoomState: fakeRoomState, myUser: myUser); + + // Pass the grace period + timeShifter.timestamp = TimeInterval(5) + + decryptionFailureTracker.checkFailures(); + + XCTAssertNil(testDelegate.reportedFailure); + } + + + + func test_notification_center() { + + let myUser = "test@example.com"; + + let decryptionFailureTracker = DecryptionFailureTracker(); + decryptionFailureTracker.timeProvider = timeShifter; + + let testDelegate = AnalyticsDelegate(); + + decryptionFailureTracker.delegate = testDelegate; + + timeShifter.timestamp = TimeInterval(0) + + let fakeEvent = FakeEvent(id: "$0000"); + fakeEvent.decryptionError = NSError(domain: MXDecryptingErrorDomain, code: Int(MXDecryptingErrorUnknownInboundSessionIdCode.rawValue)) + + + let fakeRoomState = FakeRoomState(); + fakeRoomState.mockMembers = FakeRoomMembers(joined: [myUser]) + + decryptionFailureTracker.reportUnableToDecryptError(forEvent: fakeEvent, withRoomState: fakeRoomState, myUser: myUser); + + // Shift time below GRACE_PERIOD + timeShifter.timestamp = TimeInterval(2) + + // Simulate event gets decrypted + NotificationCenter.default.post(name: .mxEventDidDecrypt, object: fakeEvent) + + + // Shift time after GRACE_PERIOD + timeShifter.timestamp = TimeInterval(6) + + + decryptionFailureTracker.checkFailures(); + + // Event should have been graced + XCTAssertNil(testDelegate.reportedFailure); + } + +} + diff --git a/RiotTests/FakeUtils.swift b/RiotTests/FakeUtils.swift new file mode 100644 index 0000000000..7bd350e4bc --- /dev/null +++ b/RiotTests/FakeUtils.swift @@ -0,0 +1,109 @@ +// +// Copyright 2024 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + + +class FakeEvent: MXEvent { + + var mockEventId: String; + var mockSender: String!; + var mockDecryptionError: Error? + + init(id: String) { + mockEventId = id + super.init() + } + + required init?(coder: NSCoder) { + fatalError() + } + + override var sender: String! { + get { return mockSender } + set { mockSender = newValue } + } + + override var eventId: String! { + get { return mockEventId } + set { mockEventId = newValue } + } + + override var decryptionError: Error? { + get { return mockDecryptionError } + set { mockDecryptionError = newValue } + } + +} + + +class FakeRoomState: MXRoomState { + + var mockMembers: MXRoomMembers? + + override var members: MXRoomMembers? { + get { return mockMembers } + set { mockMembers = newValue } + } + +} + +class FakeRoomMember: MXRoomMember { + var mockMembership: MXMembership = MXMembership.join + var mockUserId: String! + var mockMembers: MXRoomMembers? = FakeRoomMembers() + + init(mockUserId: String!) { + self.mockUserId = mockUserId + super.init() + } + + override var membership: MXMembership { + get { return mockMembership } + set { mockMembership = newValue } + } + + override var userId: String!{ + get { return mockUserId } + set { mockUserId = newValue } + } + +} + + +class FakeRoomMembers: MXRoomMembers { + + var mockMembers = [String : MXMembership]() + + init(joined: [String] = [String]()) { + for userId in joined { + self.mockMembers[userId] = MXMembership.join + } + super.init() + } + + override func member(withUserId userId: String!) -> MXRoomMember? { + let membership = mockMembers[userId] + if membership != nil { + let mockMember = FakeRoomMember(mockUserId: userId) + mockMember.mockMembership = membership! + return mockMember + } else { + return nil + } + } + +} From 0a6528703d987efe993ffec945fb18ec9348f24a Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 29 Mar 2024 18:14:15 +0100 Subject: [PATCH 2/4] DecryptionTracker: Permanent vs Temporary UTD --- Riot/Modules/Analytics/Analytics.swift | 19 +-- .../DecryptionFailure+Analytics.swift | 44 ++++++ .../Modules/Analytics/DecryptionFailure.swift | 3 + .../Analytics/DecryptionFailureTracker.swift | 52 +++++- RiotTests/DecryptionFailureTrackerTests.swift | 148 +++++++++++++++++- 5 files changed, 235 insertions(+), 31 deletions(-) create mode 100644 Riot/Modules/Analytics/DecryptionFailure+Analytics.swift diff --git a/Riot/Modules/Analytics/Analytics.swift b/Riot/Modules/Analytics/Analytics.swift index 849efd0b87..d9d68e2f85 100644 --- a/Riot/Modules/Analytics/Analytics.swift +++ b/Riot/Modules/Analytics/Analytics.swift @@ -215,7 +215,7 @@ import AnalyticsEvents @objc protocol E2EAnalytics { - func trackE2EEError(_ reason: DecryptionFailureReason, context: String) + func trackE2EEError(_ failure: DecryptionFailure) } @@ -225,21 +225,8 @@ protocol E2EAnalytics { /// - Parameters: /// - reason: The error that occurred. /// - context: Additional context of the error that occured - func trackE2EEError(_ reason: DecryptionFailureReason, context: String) { - let event = AnalyticsEvent.Error( - context: context, - cryptoModule: .Rust, - cryptoSDK: AnalyticsEvent.Error.CryptoSDK.Rust, - domain: .E2EE, - // XXX not yet supported. - eventLocalAgeMillis: nil, - isFederated: nil, - isMatrixDotOrg: nil, - name: reason.errorName, - timeToDecryptMillis: nil, - userTrustsOwnIdentity: nil, - wasVisibleToUser: nil - ) + func trackE2EEError(_ failure: DecryptionFailure) { + let event = failure.toAnalyticsEvent() capture(event: event) } diff --git a/Riot/Modules/Analytics/DecryptionFailure+Analytics.swift b/Riot/Modules/Analytics/DecryptionFailure+Analytics.swift new file mode 100644 index 0000000000..cd129aee35 --- /dev/null +++ b/Riot/Modules/Analytics/DecryptionFailure+Analytics.swift @@ -0,0 +1,44 @@ +// +// Copyright 2024 New Vector Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import AnalyticsEvents + +extension DecryptionFailure { + + public func toAnalyticsEvent() -> AnalyticsEvent.Error { + + let timeToDecryptMillis: Int = if self.timeToDecrypt != nil { + Int(self.timeToDecrypt! * 1000) + } else { + -1 + } + return AnalyticsEvent.Error( + context: self.context, + cryptoModule: .Rust, + cryptoSDK: .Rust, + domain: .E2EE, + + eventLocalAgeMillis: nil, + isFederated: nil, + isMatrixDotOrg: nil, + name: self.reason.errorName, + timeToDecryptMillis: timeToDecryptMillis, + userTrustsOwnIdentity: nil, + wasVisibleToUser: nil + ) + } +} diff --git a/Riot/Modules/Analytics/DecryptionFailure.swift b/Riot/Modules/Analytics/DecryptionFailure.swift index b5804d258a..9e0ca57858 100644 --- a/Riot/Modules/Analytics/DecryptionFailure.swift +++ b/Riot/Modules/Analytics/DecryptionFailure.swift @@ -44,6 +44,9 @@ import AnalyticsEvents /// Additional context of failure let context: String + /// UTDs can be permanent or temporary. If temporary, this field will contain the time it took to decrypt the message in milliseconds. If permanent should be nil + var timeToDecrypt: TimeInterval? + init(failedEventId: String, reason: DecryptionFailureReason, context: String, ts: TimeInterval) { self.failedEventId = failedEventId self.reason = reason diff --git a/Riot/Modules/Analytics/DecryptionFailureTracker.swift b/Riot/Modules/Analytics/DecryptionFailureTracker.swift index f33f2b7c52..19b8afb19c 100644 --- a/Riot/Modules/Analytics/DecryptionFailureTracker.swift +++ b/Riot/Modules/Analytics/DecryptionFailureTracker.swift @@ -36,7 +36,10 @@ class DecryptionFailureTracker: NSObject { let GRACE_PERIOD: TimeInterval = 4 // Call `checkFailures` every `CHECK_INTERVAL` - let CHECK_INTERVAL: TimeInterval = 2 + let CHECK_INTERVAL: TimeInterval = 15 + + // The maximum time to wait for a late decryption before reporting as permanent UTD + let MAX_WAIT_FOR_LATE_DECRYPTION: TimeInterval = 60 @objc weak var delegate: E2EAnalytics? @@ -60,9 +63,6 @@ class DecryptionFailureTracker: NSObject { name: .mxEventDidDecrypt, object: nil) - Timer.scheduledTimer(withTimeInterval: CHECK_INTERVAL, repeats: true) { [weak self] _ in - self?.checkFailures() - } } @objc @@ -93,6 +93,14 @@ class DecryptionFailureTracker: NSObject { let context = String(format: "code: %ld, description: %@", error.code, event.decryptionError.localizedDescription) reportedFailures[failedEventId] = DecryptionFailure(failedEventId: failedEventId, reason: reason, context: context, ts: self.timeProvider.nowTs()) + + // Start the ticker if needed. There is no need to have a ticker if no failures are tracked + if checkFailuresTimer == nil { + self.checkFailuresTimer = Timer.scheduledTimer(withTimeInterval: CHECK_INTERVAL, repeats: true) { [weak self] _ in + self?.checkFailures() + } + } + } @objc @@ -104,8 +112,30 @@ class DecryptionFailureTracker: NSObject { func eventDidDecrypt(_ notification: Notification) { guard let event = notification.object as? MXEvent else { return } - // Could be an event in the reportedFailures, remove it + guard let reportedFailure = self.reportedFailures[event.eventId] else { return } + + let now = self.timeProvider.nowTs() + let ellapsedTime = now - reportedFailure.ts + + if ellapsedTime < 4 { + // event is graced + reportedFailures.removeValue(forKey: event.eventId) + } else { + // It's a late decrypt must be reported as a late decrypt + reportedFailure.timeToDecrypt = ellapsedTime + self.delegate?.trackE2EEError(reportedFailure) + } + // Remove from reported failures + self.trackedEvents.insert(event.eventId) reportedFailures.removeValue(forKey: event.eventId) + + // Check if we still need the ticker timer + if reportedFailures.isEmpty { + // Invalidate the current timer, nothing to check for + self.checkFailuresTimer?.invalidate() + self.checkFailuresTimer = nil + } + } /** @@ -116,23 +146,29 @@ class DecryptionFailureTracker: NSObject { func checkFailures() { guard let delegate = self.delegate else {return} - let tsNow = self.timeProvider.nowTs() var failuresToCheck = [DecryptionFailure]() for reportedFailure in self.reportedFailures.values { let ellapsed = tsNow - reportedFailure.ts - if ellapsed > GRACE_PERIOD { + if ellapsed > MAX_WAIT_FOR_LATE_DECRYPTION { failuresToCheck.append(reportedFailure) + reportedFailure.timeToDecrypt = nil reportedFailures.removeValue(forKey: reportedFailure.failedEventId) trackedEvents.insert(reportedFailure.failedEventId) } } for failure in failuresToCheck { - delegate.trackE2EEError(failure.reason, context: failure.context) + delegate.trackE2EEError(failure) } + // Check if we still need the ticker timer + if reportedFailures.isEmpty { + // Invalidate the current timer, nothing to check for + self.checkFailuresTimer?.invalidate() + self.checkFailuresTimer = nil + } } } diff --git a/RiotTests/DecryptionFailureTrackerTests.swift b/RiotTests/DecryptionFailureTrackerTests.swift index 9b5fe94ea4..3175ef19e4 100644 --- a/RiotTests/DecryptionFailureTrackerTests.swift +++ b/RiotTests/DecryptionFailureTrackerTests.swift @@ -32,10 +32,9 @@ class DecryptionFailureTrackerTests: XCTestCase { } class AnalyticsDelegate : E2EAnalytics { - var reportedFailure: Element.DecryptionFailureReason?; + var reportedFailure: Element.DecryptionFailure?; - func trackE2EEError(_ reason: Element.DecryptionFailureReason, context: String) { - print("Error Tracked: ", reason) + func trackE2EEError(_ reason: Element.DecryptionFailure) { reportedFailure = reason } @@ -66,6 +65,9 @@ class DecryptionFailureTrackerTests: XCTestCase { timeShifter.timestamp = TimeInterval(2) + // simulate decrypted in the grace period + NotificationCenter.default.post(name: .mxEventDidDecrypt, object: fakeEvent) + decryptionFailureTracker.checkFailures(); XCTAssertNil(testDelegate.reportedFailure); @@ -73,11 +75,71 @@ class DecryptionFailureTrackerTests: XCTestCase { // Pass the grace period timeShifter.timestamp = TimeInterval(5) + decryptionFailureTracker.checkFailures(); + XCTAssertNil(testDelegate.reportedFailure); + + } + + func test_report_ratcheted_key_utd() { + + let myUser = "test@example.com"; + + let decryptionFailureTracker = DecryptionFailureTracker(); + decryptionFailureTracker.timeProvider = timeShifter; + + let testDelegate = AnalyticsDelegate(); + + decryptionFailureTracker.delegate = testDelegate; + + timeShifter.timestamp = TimeInterval(0) + + let fakeEvent = FakeEvent(id: "$0000"); + fakeEvent.decryptionError = NSError(domain: MXDecryptingErrorDomain, code: Int(MXDecryptingErrorOlmCode.rawValue)) + + + let fakeRoomState = FakeRoomState(); + fakeRoomState.mockMembers = FakeRoomMembers(joined: [myUser]) + decryptionFailureTracker.reportUnableToDecryptError(forEvent: fakeEvent, withRoomState: fakeRoomState, myUser: myUser); + + // Pass the max period + timeShifter.timestamp = TimeInterval(70) + + decryptionFailureTracker.checkFailures(); + + XCTAssertEqual(testDelegate.reportedFailure?.reason, DecryptionFailureReason.olmIndexError); + } + + func test_report_unspecified_error() { + + let myUser = "test@example.com"; + + let decryptionFailureTracker = DecryptionFailureTracker(); + decryptionFailureTracker.timeProvider = timeShifter; + + let testDelegate = AnalyticsDelegate(); + + decryptionFailureTracker.delegate = testDelegate; + + timeShifter.timestamp = TimeInterval(0) + + let fakeEvent = FakeEvent(id: "$0000"); + fakeEvent.decryptionError = NSError(domain: MXDecryptingErrorDomain, code: Int(MXDecryptingErrorBadRoomCode.rawValue)) + + + let fakeRoomState = FakeRoomState(); + fakeRoomState.mockMembers = FakeRoomMembers(joined: [myUser]) + decryptionFailureTracker.reportUnableToDecryptError(forEvent: fakeEvent, withRoomState: fakeRoomState, myUser: myUser); + + // Pass the max period + timeShifter.timestamp = TimeInterval(70) + decryptionFailureTracker.checkFailures(); - XCTAssertEqual(testDelegate.reportedFailure, DecryptionFailureReason.olmKeysNotSent); + XCTAssertEqual(testDelegate.reportedFailure?.reason, DecryptionFailureReason.unspecified); } + + func test_do_not_double_report() { let myUser = "test@example.com"; @@ -100,12 +162,12 @@ class DecryptionFailureTrackerTests: XCTestCase { decryptionFailureTracker.reportUnableToDecryptError(forEvent: fakeEvent, withRoomState: fakeRoomState, myUser: myUser); - // Pass the grace period - timeShifter.timestamp = TimeInterval(5) + // Pass the max period + timeShifter.timestamp = TimeInterval(70) decryptionFailureTracker.checkFailures(); - XCTAssertEqual(testDelegate.reportedFailure, DecryptionFailureReason.olmKeysNotSent); + XCTAssertEqual(testDelegate.reportedFailure?.reason, DecryptionFailureReason.olmKeysNotSent); // Try to report again the same event testDelegate.reportedFailure = nil @@ -192,5 +254,77 @@ class DecryptionFailureTrackerTests: XCTestCase { XCTAssertNil(testDelegate.reportedFailure); } + + func test_should_report_late_decrypt() { + + let myUser = "test@example.com"; + + let decryptionFailureTracker = DecryptionFailureTracker(); + decryptionFailureTracker.timeProvider = timeShifter; + + let testDelegate = AnalyticsDelegate(); + + decryptionFailureTracker.delegate = testDelegate; + + timeShifter.timestamp = TimeInterval(0) + + let fakeEvent = FakeEvent(id: "$0000"); + fakeEvent.decryptionError = NSError(domain: MXDecryptingErrorDomain, code: Int(MXDecryptingErrorUnknownInboundSessionIdCode.rawValue)) + + + let fakeRoomState = FakeRoomState(); + fakeRoomState.mockMembers = FakeRoomMembers(joined: [myUser]) + + decryptionFailureTracker.reportUnableToDecryptError(forEvent: fakeEvent, withRoomState: fakeRoomState, myUser: myUser); + + // Simulate succesful decryption after grace period but before max wait + timeShifter.timestamp = TimeInterval(20) + + // Simulate event gets decrypted + NotificationCenter.default.post(name: .mxEventDidDecrypt, object: fakeEvent) + + + decryptionFailureTracker.checkFailures(); + + // Event should have been reported as a late decrypt + XCTAssertEqual(testDelegate.reportedFailure?.reason, DecryptionFailureReason.olmKeysNotSent); + XCTAssertEqual(testDelegate.reportedFailure?.timeToDecrypt, TimeInterval(20)); + + } + + + + func test_should_report_permanent_decryption_error() { + + let myUser = "test@example.com"; + + let decryptionFailureTracker = DecryptionFailureTracker(); + decryptionFailureTracker.timeProvider = timeShifter; + + let testDelegate = AnalyticsDelegate(); + + decryptionFailureTracker.delegate = testDelegate; + + timeShifter.timestamp = TimeInterval(0) + + let fakeEvent = FakeEvent(id: "$0000"); + fakeEvent.decryptionError = NSError(domain: MXDecryptingErrorDomain, code: Int(MXDecryptingErrorUnknownInboundSessionIdCode.rawValue)) + + + let fakeRoomState = FakeRoomState(); + fakeRoomState.mockMembers = FakeRoomMembers(joined: [myUser]) + + decryptionFailureTracker.reportUnableToDecryptError(forEvent: fakeEvent, withRoomState: fakeRoomState, myUser: myUser); + + // Simulate succesful decryption after max wait + timeShifter.timestamp = TimeInterval(70) + + decryptionFailureTracker.checkFailures(); + + // Event should have been reported as a late decrypt + XCTAssertEqual(testDelegate.reportedFailure?.reason, DecryptionFailureReason.olmKeysNotSent); + XCTAssertNil(testDelegate.reportedFailure?.timeToDecrypt); + + } } From 1d633e8fa5dcdf20ff113502661dcf4dafa115f6 Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 29 Mar 2024 18:21:56 +0100 Subject: [PATCH 3/4] Add more test for convertion to analytics error --- RiotTests/DecryptionFailureTrackerTests.swift | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/RiotTests/DecryptionFailureTrackerTests.swift b/RiotTests/DecryptionFailureTrackerTests.swift index 3175ef19e4..7cd9bf480f 100644 --- a/RiotTests/DecryptionFailureTrackerTests.swift +++ b/RiotTests/DecryptionFailureTrackerTests.swift @@ -290,6 +290,11 @@ class DecryptionFailureTrackerTests: XCTestCase { XCTAssertEqual(testDelegate.reportedFailure?.reason, DecryptionFailureReason.olmKeysNotSent); XCTAssertEqual(testDelegate.reportedFailure?.timeToDecrypt, TimeInterval(20)); + // Assert that it's converted to millis for reporting + let analyticsError = testDelegate.reportedFailure!.toAnalyticsEvent() + + XCTAssertEqual(analyticsError.timeToDecryptMillis, 20000) + } @@ -325,6 +330,12 @@ class DecryptionFailureTrackerTests: XCTestCase { XCTAssertEqual(testDelegate.reportedFailure?.reason, DecryptionFailureReason.olmKeysNotSent); XCTAssertNil(testDelegate.reportedFailure?.timeToDecrypt); + + // Assert that it's converted to -1 for reporting + let analyticsError = testDelegate.reportedFailure!.toAnalyticsEvent() + + XCTAssertEqual(analyticsError.timeToDecryptMillis, -1) + } } From 56e66539d87f3a6cef6cc0343ce9e6021d166692 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 2 Apr 2024 11:13:10 +0200 Subject: [PATCH 4/4] post merge fix --- .../Analytics/DecryptionFailureTracker.m | 174 ------------------ 1 file changed, 174 deletions(-) delete mode 100644 Riot/Modules/Analytics/DecryptionFailureTracker.m diff --git a/Riot/Modules/Analytics/DecryptionFailureTracker.m b/Riot/Modules/Analytics/DecryptionFailureTracker.m deleted file mode 100644 index 86b847f221..0000000000 --- a/Riot/Modules/Analytics/DecryptionFailureTracker.m +++ /dev/null @@ -1,174 +0,0 @@ -/* - Copyright 2018 New Vector Ltd - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - */ - -#import "DecryptionFailureTracker.h" -#import "GeneratedInterface-Swift.h" - - -// Call `checkFailures` every `CHECK_INTERVAL` -#define CHECK_INTERVAL 10 - -// Give events a chance to be decrypted by waiting `GRACE_PERIOD` before counting -// and reporting them as failures -#define GRACE_PERIOD 30 - -// E2E failures analytics category. -NSString *const kDecryptionFailureTrackerAnalyticsCategory = @"e2e.failure"; - -@interface DecryptionFailureTracker() -{ - // Reported failures - // Every `CHECK_INTERVAL`, this list is checked for failures that happened - // more than`GRACE_PERIOD` ago. Those that did are reported to the delegate. - NSMutableDictionary *reportedFailures; - - // Event ids of failures that were tracked previously - NSMutableSet *trackedEvents; - - // Timer for periodic check - NSTimer *checkFailuresTimer; -} -@end - -@implementation DecryptionFailureTracker - -+ (instancetype)sharedInstance -{ - static DecryptionFailureTracker *sharedInstance = nil; - static dispatch_once_t onceToken; - - dispatch_once(&onceToken, ^{ - sharedInstance = [[DecryptionFailureTracker alloc] init]; - }); - - return sharedInstance; -} - -- (instancetype)init -{ - self = [super init]; - if (self) - { - reportedFailures = [NSMutableDictionary dictionary]; - trackedEvents = [NSMutableSet set]; - - [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(eventDidDecrypt:) name:kMXEventDidDecryptNotification object:nil]; - - checkFailuresTimer = [NSTimer scheduledTimerWithTimeInterval:CHECK_INTERVAL - target:self - selector:@selector(checkFailures) - userInfo:nil - repeats:YES]; - } - return self; -} - -- (void)reportUnableToDecryptErrorForEvent:(MXEvent *)event withRoomState:(MXRoomState *)roomState myUser:(NSString *)userId -{ - if (reportedFailures[event.eventId] || [trackedEvents containsObject:event.eventId]) - { - return; - } - - // Filter out "expected" UTDs - // We cannot decrypt messages sent before the user joined the room - MXRoomMember *myUser = [roomState.members memberWithUserId:userId]; - if (!myUser || myUser.membership != MXMembershipJoin) - { - return; - } - - NSString *failedEventId = event.eventId; - DecryptionFailureReason reason; - - // Categorise the error - switch (event.decryptionError.code) - { - case MXDecryptingErrorUnknownInboundSessionIdCode: - reason = DecryptionFailureReasonOlmKeysNotSent; - break; - - case MXDecryptingErrorOlmCode: - reason = DecryptionFailureReasonOlmIndexError; - break; - - default: - // All other error codes will be tracked as `OlmUnspecifiedError` and will include `context` containing - // the actual error code and localized description - reason = DecryptionFailureReasonUnspecified; - break; - } - - NSString *context = [NSString stringWithFormat:@"code: %ld, description: %@", event.decryptionError.code, event.decryptionError.localizedDescription]; - reportedFailures[event.eventId] = [[DecryptionFailure alloc] initWithFailedEventId:failedEventId - reason:reason - context:context]; -} - -- (void)dispatch -{ - [self checkFailures]; -} - -#pragma mark - Private methods - -/** - Mark reported failures that occured before tsNow - GRACE_PERIOD as failures that should be - tracked. - */ -- (void)checkFailures -{ - if (!_delegate) - { - return; - } - - NSTimeInterval tsNow = [NSDate date].timeIntervalSince1970; - - NSMutableArray *failuresToTrack = [NSMutableArray array]; - - for (DecryptionFailure *reportedFailure in reportedFailures.allValues) - { - if (reportedFailure.ts < tsNow - GRACE_PERIOD) - { - [failuresToTrack addObject:reportedFailure]; - [reportedFailures removeObjectForKey:reportedFailure.failedEventId]; - [trackedEvents addObject:reportedFailure.failedEventId]; - } - } - - if (failuresToTrack.count) - { - // Sort failures by error reason - NSMutableDictionary *failuresCounts = [NSMutableDictionary dictionary]; - for (DecryptionFailure *failure in failuresToTrack) - { - failuresCounts[@(failure.reason)] = @(failuresCounts[@(failure.reason)].unsignedIntegerValue + 1); - [self.delegate trackE2EEError:failure.reason context:failure.context]; - } - - MXLogDebug(@"[DecryptionFailureTracker] trackFailures: %@", failuresCounts); - } -} - -- (void)eventDidDecrypt:(NSNotification *)notif -{ - // Could be an event in the reportedFailures, remove it - MXEvent *event = notif.object; - [reportedFailures removeObjectForKey:event.eventId]; -} - -@end