From b6ce79303a6f4e2c20b8e877b296713cfb1bdc1d Mon Sep 17 00:00:00 2001 From: manuroe Date: Fri, 29 Jun 2018 07:35:31 +0200 Subject: [PATCH] Crypto: Add telemetry for events unable to decrypt (UTDs) #1894 --- CHANGES.rst | 1 + Riot.xcodeproj/project.pbxproj | 12 ++ Riot/Analytics/Analytics.h | 3 +- Riot/Analytics/Analytics.m | 15 +++ Riot/Analytics/DecryptionFailure.h | 39 ++++++ Riot/Analytics/DecryptionFailure.m | 31 +++++ Riot/Analytics/DecryptionFailureTracker.h | 64 ++++++++++ Riot/Analytics/DecryptionFailureTracker.m | 138 ++++++++++++++++++++++ Riot/AppDelegate.m | 1 + Riot/Utils/EventFormatter.m | 55 +++++---- 10 files changed, 335 insertions(+), 24 deletions(-) create mode 100644 Riot/Analytics/DecryptionFailure.h create mode 100644 Riot/Analytics/DecryptionFailure.m create mode 100644 Riot/Analytics/DecryptionFailureTracker.h create mode 100644 Riot/Analytics/DecryptionFailureTracker.m diff --git a/CHANGES.rst b/CHANGES.rst index 55072c49e..fbbdeb7d1 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,6 +6,7 @@ Improvements: * RoomVC: Add a re-request keys button on message unable to decrypt (#1879). * Analytics: Move code from AppDelegate to a dedicated class: Analytics. * Analytics: Track Matrix SDK stats (time to startup the app). + * Crypto: Add telemetry for events unable to decrypt (UTDs). * Added the i18n localisation strings to the accessibility labels (#1842), thanks to @einMarco (PR#1906). * Added titles to sound files ID3 tags. diff --git a/Riot.xcodeproj/project.pbxproj b/Riot.xcodeproj/project.pbxproj index 097705c6a..20c828611 100644 --- a/Riot.xcodeproj/project.pbxproj +++ b/Riot.xcodeproj/project.pbxproj @@ -64,6 +64,8 @@ 3267EFB820E379FE00FF1CAA /* Podfile in Resources */ = {isa = PBXBuildFile; fileRef = 3267EFB420E379FD00FF1CAA /* Podfile */; }; 3267EFB920E379FE00FF1CAA /* AUTHORS.rst in Resources */ = {isa = PBXBuildFile; fileRef = 3267EFB520E379FD00FF1CAA /* AUTHORS.rst */; }; 3267EFBA20E379FE00FF1CAA /* README.rst in Resources */ = {isa = PBXBuildFile; fileRef = 3267EFB620E379FD00FF1CAA /* README.rst */; }; + 3267EFC020E4A3DD00FF1CAA /* DecryptionFailureTracker.m in Sources */ = {isa = PBXBuildFile; fileRef = 3267EFBE20E4A3DD00FF1CAA /* DecryptionFailureTracker.m */; }; + 3267EFC320E5055800FF1CAA /* DecryptionFailure.m in Sources */ = {isa = PBXBuildFile; fileRef = 3267EFC220E5055800FF1CAA /* DecryptionFailure.m */; }; 327382B51F276AD200356143 /* InfoPlist.strings in Resources */ = {isa = PBXBuildFile; fileRef = 327382A81F276AD200356143 /* InfoPlist.strings */; }; 327382B61F276AD200356143 /* Localizable.strings in Resources */ = {isa = PBXBuildFile; fileRef = 327382AA1F276AD200356143 /* Localizable.strings */; }; 327382B71F276AD200356143 /* Vector.strings in Resources */ = {isa = PBXBuildFile; fileRef = 327382AC1F276AD200356143 /* Vector.strings */; }; @@ -736,6 +738,10 @@ 3267EFB420E379FD00FF1CAA /* Podfile */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = Podfile; sourceTree = ""; }; 3267EFB520E379FD00FF1CAA /* AUTHORS.rst */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = AUTHORS.rst; sourceTree = ""; }; 3267EFB620E379FD00FF1CAA /* README.rst */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = README.rst; sourceTree = ""; }; + 3267EFBE20E4A3DD00FF1CAA /* DecryptionFailureTracker.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = DecryptionFailureTracker.m; path = Riot/Analytics/DecryptionFailureTracker.m; sourceTree = SOURCE_ROOT; }; + 3267EFBF20E4A3DD00FF1CAA /* DecryptionFailureTracker.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = DecryptionFailureTracker.h; path = Riot/Analytics/DecryptionFailureTracker.h; sourceTree = SOURCE_ROOT; }; + 3267EFC120E5055800FF1CAA /* DecryptionFailure.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = DecryptionFailure.h; path = Riot/Analytics/DecryptionFailure.h; sourceTree = SOURCE_ROOT; }; + 3267EFC220E5055800FF1CAA /* DecryptionFailure.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = DecryptionFailure.m; path = Riot/Analytics/DecryptionFailure.m; sourceTree = SOURCE_ROOT; }; 327382A91F276AD200356143 /* de */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = de; path = InfoPlist.strings; sourceTree = ""; }; 327382AB1F276AD200356143 /* de */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = de; path = Localizable.strings; sourceTree = ""; }; 327382AD1F276AD200356143 /* de */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = de; path = Vector.strings; sourceTree = ""; }; @@ -1613,6 +1619,10 @@ children = ( 3267EFB020E2A04100FF1CAA /* Analytics.h */, 3267EFB120E2A04100FF1CAA /* Analytics.m */, + 3267EFBF20E4A3DD00FF1CAA /* DecryptionFailureTracker.h */, + 3267EFBE20E4A3DD00FF1CAA /* DecryptionFailureTracker.m */, + 3267EFC120E5055800FF1CAA /* DecryptionFailure.h */, + 3267EFC220E5055800FF1CAA /* DecryptionFailure.m */, ); name = Analytics; path = "New Group"; @@ -3622,6 +3632,7 @@ F083BE2D1E7009ED00A9B29C /* ForgotPasswordInputsView.m in Sources */, F083BE7E1E7009ED00A9B29C /* InviteRecentTableViewCell.m in Sources */, F083BE3C1E7009ED00A9B29C /* RoomIncomingEncryptedAttachmentWithoutSenderInfoBubbleCell.m in Sources */, + 3267EFC320E5055800FF1CAA /* DecryptionFailure.m in Sources */, F083BE211E7009ED00A9B29C /* RoomSearchViewController.m in Sources */, 32185B311F20FA2B00752141 /* LanguagePickerViewController.m in Sources */, F083BE3E1E7009ED00A9B29C /* RoomIncomingEncryptedAttachmentWithPaginationTitleBubbleCell.m in Sources */, @@ -3686,6 +3697,7 @@ F083BE101E7009ED00A9B29C /* CountryPickerViewController.m in Sources */, F083BE581E7009ED00A9B29C /* RoomOutgoingEncryptedTextMsgWithPaginationTitleWithoutSenderNameBubbleCell.m in Sources */, F083BDF61E7009ED00A9B29C /* Contact.m in Sources */, + 3267EFC020E4A3DD00FF1CAA /* DecryptionFailureTracker.m in Sources */, F083BE961E7009ED00A9B29C /* MessagesSearchResultAttachmentBubbleCell.m in Sources */, F083BE391E7009ED00A9B29C /* RoomEncryptedDataBubbleCell.m in Sources */, F083BDF01E7009ED00A9B29C /* UIViewController+RiotSearch.m in Sources */, diff --git a/Riot/Analytics/Analytics.h b/Riot/Analytics/Analytics.h index d6b29b595..ad8466e74 100644 --- a/Riot/Analytics/Analytics.h +++ b/Riot/Analytics/Analytics.h @@ -17,11 +17,12 @@ #import #import +#import "DecryptionFailureTracker.h" /** `Analytics` sends analytics to an analytics tool. */ -@interface Analytics : NSObject +@interface Analytics : NSObject /** Returns the shared Analytics manager. diff --git a/Riot/Analytics/Analytics.m b/Riot/Analytics/Analytics.m index ad8068266..06276ce1b 100644 --- a/Riot/Analytics/Analytics.m +++ b/Riot/Analytics/Analytics.m @@ -25,6 +25,10 @@ NSString *const kAnalyticsPiwikMetricsCategory = @"Metrics"; NSString *const kAnalyticsPiwikMetricsActionPattern = @"iOS.%@"; +// E2E telemetry is stored under a Piwik category called "E2E". +NSString *const kAnalyticsPiwikE2eCategory = @"E2E"; +NSString *const kAnalyticsPiwikE2eDecryptionFailure = @"Decryption failure"; + @import PiwikTracker; @implementation Analytics @@ -166,4 +170,15 @@ NSString *const kAnalyticsPiwikMetricsActionPattern = @"iOS.%@"; url:nil]; } +#pragma mark - MXDecryptionFailureDelegate + +- (void)trackFailures:(NSUInteger)failuresCount +{ + [[PiwikTracker shared] trackWithEventWithCategory:kAnalyticsPiwikE2eCategory + action:kAnalyticsPiwikE2eDecryptionFailure + name:kAnalyticsPiwikE2eDecryptionFailure + number:@(failuresCount) + url:nil]; +} + @end diff --git a/Riot/Analytics/DecryptionFailure.h b/Riot/Analytics/DecryptionFailure.h new file mode 100644 index 000000000..df9f1af75 --- /dev/null +++ b/Riot/Analytics/DecryptionFailure.h @@ -0,0 +1,39 @@ +/* + 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 + +/** + `DecryptionFailure` represents a decryption failure. + */ +@interface DecryptionFailure : NSObject + +/** + The id of the event that was unabled to decrypt. + */ +@property (nonatomic) NSString *failedEventId; + +/** + The time the failure has been reported. + */ +@property (nonatomic, readonly) NSTimeInterval ts; + +/** + Decryption failure reason. + */ +@property (nonatomic) NSString *reason; + +@end diff --git a/Riot/Analytics/DecryptionFailure.m b/Riot/Analytics/DecryptionFailure.m new file mode 100644 index 000000000..7cb470e8c --- /dev/null +++ b/Riot/Analytics/DecryptionFailure.m @@ -0,0 +1,31 @@ +/* + 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 "DecryptionFailure.h" + +@implementation DecryptionFailure + +- (instancetype)init +{ + self = [super init]; + if (self) + { + _ts = [NSDate date].timeIntervalSince1970; + } + return self; +} + +@end diff --git a/Riot/Analytics/DecryptionFailureTracker.h b/Riot/Analytics/DecryptionFailureTracker.h new file mode 100644 index 000000000..7ddc2482f --- /dev/null +++ b/Riot/Analytics/DecryptionFailureTracker.h @@ -0,0 +1,64 @@ +/* + 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 + +@import MatrixSDK; + +@protocol MXDecryptionFailureDelegate; + +@interface DecryptionFailureTracker : NSObject + +/** + Returns the shared tracker. + + @return the shared tracker. + */ ++ (instancetype)sharedInstance; + +/** + The delegate object to receive analytics events. + */ +@property (nonatomic) id 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; + +@end + +/** + The `MXDecryptionFailureDelegate` protocol receives some stats computed by + `DecryptionFailureTracker`. + */ +@protocol MXDecryptionFailureDelegate + +/** + Stats for decryption failures. + + @param failuresCount the number of decryption failures. + */ +- (void)trackFailures:(NSUInteger)failuresCount; + +@end diff --git a/Riot/Analytics/DecryptionFailureTracker.m b/Riot/Analytics/DecryptionFailureTracker.m new file mode 100644 index 000000000..424570186 --- /dev/null +++ b/Riot/Analytics/DecryptionFailureTracker.m @@ -0,0 +1,138 @@ +/* + 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 "DecryptionFailure.h" + + +// Call `checkFailures` every `CHECK_INTERVAL` +#define CHECK_INTERVAL 5 + +// Give events a chance to be decrypted by waiting `GRACE_PERIOD` before counting +// and reporting them as failures +#define GRACE_PERIOD 60 + + +@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 memberWithUserId:userId]; + if (!myUser || myUser.membership != MXMembershipJoin) + { + return; + } + + DecryptionFailure *decryptionFailure = [[DecryptionFailure alloc] init]; + decryptionFailure.failedEventId = event.eventId; + + // TODO: Need to sync with all platforms + // decryptionFailure.reason =; + + reportedFailures[event.eventId] = decryptionFailure; +} + +#pragma mark - Private methods + +/** + Mark reported failures that occured before tsNow - GRACE_PERIOD as failures that should be + tracked. + */ +- (void)checkFailures +{ + 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 (_delegate && failuresToTrack.count) + { + NSLog(@"[DecryptionFailureTracker] trackFailures: %@", @(failuresToTrack.count)); + [_delegate trackFailures:failuresToTrack.count]; + } +} + +- (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/AppDelegate.m b/Riot/AppDelegate.m index 1ad79e6fc..4d38c59c0 100644 --- a/Riot/AppDelegate.m +++ b/Riot/AppDelegate.m @@ -464,6 +464,7 @@ NSString *const kAppDelegateNetworkStatusDidChangeNotification = @"kAppDelegateN // Configure our analytics. It will indeed start if the option is enabled [MXSDKOptions sharedInstance].analyticsDelegate = [Analytics sharedInstance]; + [DecryptionFailureTracker sharedInstance].delegate = [Analytics sharedInstance]; [[Analytics sharedInstance] start]; // Prepare Pushkit handling diff --git a/Riot/Utils/EventFormatter.m b/Riot/Utils/EventFormatter.m index 09f92f344..925259270 100644 --- a/Riot/Utils/EventFormatter.m +++ b/Riot/Utils/EventFormatter.m @@ -22,6 +22,7 @@ #import "WidgetManager.h" #import "MXDecryptionResult.h" +#import "DecryptionFailureTracker.h" #pragma mark - Constants definitions @@ -117,34 +118,42 @@ NSString *const kEventFormatterOnReRequestKeysLinkActionSeparator = @"/"; NSAttributedString *attributedString = [super attributedStringFromEvent:event withRoomState:roomState error:error]; if (event.sentState == MXEventSentStateSent - && [event.decryptionError.domain isEqualToString:MXDecryptingErrorDomain] - && event.decryptionError.code == MXDecryptingErrorUnknownInboundSessionIdCode) + && [event.decryptionError.domain isEqualToString:MXDecryptingErrorDomain]) { - // Append to the displayed error an attibuted string with a tappable link - // so that the user can try to fix the UTC - NSMutableAttributedString *attributedStringWithRerequestMessage = [attributedString mutableCopy]; - [attributedStringWithRerequestMessage appendAttributedString:[[NSAttributedString alloc] initWithString:@"\n"]]; + // Track e2e failures + dispatch_async(dispatch_get_main_queue(), ^{ + [[DecryptionFailureTracker sharedInstance] reportUnableToDecryptErrorForEvent:event withRoomState:roomState myUser:mxSession.myUser.userId]; + }); - NSString *linkActionString = [NSString stringWithFormat:@"%@%@%@", kEventFormatterOnReRequestKeysLinkAction, - kEventFormatterOnReRequestKeysLinkActionSeparator, - event.eventId]; - [attributedStringWithRerequestMessage appendAttributedString: - [[NSAttributedString alloc] initWithString:NSLocalizedStringFromTable(@"event_formatter_rerequest_keys_part1_link", @"Vector", nil) - attributes:@{ - NSLinkAttributeName: linkActionString, - NSForegroundColorAttributeName: self.sendingTextColor, - NSFontAttributeName: self.encryptedMessagesTextFont - }]]; + if (event.decryptionError.code == MXDecryptingErrorUnknownInboundSessionIdCode) + { + // Append to the displayed error an attibuted string with a tappable link + // so that the user can try to fix the UTD + NSMutableAttributedString *attributedStringWithRerequestMessage = [attributedString mutableCopy]; + [attributedStringWithRerequestMessage appendAttributedString:[[NSAttributedString alloc] initWithString:@"\n"]]; - [attributedStringWithRerequestMessage appendAttributedString: - [[NSAttributedString alloc] initWithString:NSLocalizedStringFromTable(@"event_formatter_rerequest_keys_part2", @"Vector", nil) - attributes:@{ - NSForegroundColorAttributeName: self.sendingTextColor, - NSFontAttributeName: self.encryptedMessagesTextFont - }]]; + NSString *linkActionString = [NSString stringWithFormat:@"%@%@%@", kEventFormatterOnReRequestKeysLinkAction, + kEventFormatterOnReRequestKeysLinkActionSeparator, + event.eventId]; - attributedString = attributedStringWithRerequestMessage; + [attributedStringWithRerequestMessage appendAttributedString: + [[NSAttributedString alloc] initWithString:NSLocalizedStringFromTable(@"event_formatter_rerequest_keys_part1_link", @"Vector", nil) + attributes:@{ + NSLinkAttributeName: linkActionString, + NSForegroundColorAttributeName: self.sendingTextColor, + NSFontAttributeName: self.encryptedMessagesTextFont + }]]; + + [attributedStringWithRerequestMessage appendAttributedString: + [[NSAttributedString alloc] initWithString:NSLocalizedStringFromTable(@"event_formatter_rerequest_keys_part2", @"Vector", nil) + attributes:@{ + NSForegroundColorAttributeName: self.sendingTextColor, + NSFontAttributeName: self.encryptedMessagesTextFont + }]]; + + attributedString = attributedStringWithRerequestMessage; + } } return attributedString;