From 6c2dbbc7c346d84b3e7f48a728aad7870646efcd Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Fri, 16 Nov 2018 10:32:02 -0600 Subject: [PATCH 1/3] verify envelope source before proceeding with error handling --- .../src/Messages/OWSMessageDecrypter.m | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/SignalServiceKit/src/Messages/OWSMessageDecrypter.m b/SignalServiceKit/src/Messages/OWSMessageDecrypter.m index 960069cc7..56d1c3949 100644 --- a/SignalServiceKit/src/Messages/OWSMessageDecrypter.m +++ b/SignalServiceKit/src/Messages/OWSMessageDecrypter.m @@ -525,10 +525,16 @@ NSError *EnsureDecryptError(NSError *_Nullable error, NSString *fallbackErrorDes OWSLogError( @"Got exception: %@ of type: %@ with reason: %@", exception.description, exception.name, exception.reason); - [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { TSErrorMessage *errorMessage; + if (envelope.source.length == 0) { + TSErrorMessage *errorMessage = [TSErrorMessage corruptedMessageInUnknownThread]; + [SSKEnvironment.shared.notificationsManager notifyUserForThreadlessErrorMessage:errorMessage + transaction:transaction]; + return; + } + if ([exception.name isEqualToString:NoSessionException]) { OWSProdErrorWEnvelope([OWSAnalyticsEvents messageManagerErrorNoSession], envelope); errorMessage = [TSErrorMessage missingSessionWithEnvelope:envelope withTransaction:transaction]; @@ -551,14 +557,7 @@ NSError *EnsureDecryptError(NSError *_Nullable error, NSString *fallbackErrorDes return; } else { OWSProdErrorWEnvelope([OWSAnalyticsEvents messageManagerErrorCorruptMessage], envelope); - if (envelope.source.length > 0) { - errorMessage = [TSErrorMessage corruptedMessageWithEnvelope:envelope withTransaction:transaction]; - } else { - TSErrorMessage *errorMessage = [TSErrorMessage corruptedMessageInUnknownThread]; - [SSKEnvironment.shared.notificationsManager notifyUserForThreadlessErrorMessage:errorMessage - transaction:transaction]; - return; - } + errorMessage = [TSErrorMessage corruptedMessageWithEnvelope:envelope withTransaction:transaction]; } OWSAssertDebug(errorMessage); From f52a58e31e645f5046cb6659fdc526dc1f32aa0d Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Fri, 16 Nov 2018 13:05:14 -0600 Subject: [PATCH 2/3] Handle known sender --- Podfile | 1 - Podfile.lock | 4 +- Pods | 2 +- .../src/Messages/OWSMessageDecrypter.m | 192 +++++++++++------- 4 files changed, 125 insertions(+), 74 deletions(-) diff --git a/Podfile b/Podfile index 228be5bb6..d0199ba5c 100644 --- a/Podfile +++ b/Podfile @@ -18,7 +18,6 @@ def shared_pods # pod 'HKDFKit', path: '../HKDFKit', testspecs: ["Tests"] pod 'Curve25519Kit', git: 'https://github.com/signalapp/Curve25519Kit', testspecs: ["Tests"] # pod 'Curve25519Kit', path: '../Curve25519Kit', testspecs: ["Tests"] - # TODO: Use public repo. pod 'SignalMetadataKit', git: 'https://github.com/signalapp/SignalMetadataKit', testspecs: ["Tests"] # pod 'SignalMetadataKit', path: '../SignalMetadataKit', testspecs: ["Tests"] pod 'SignalServiceKit', path: '.', testspecs: ["Tests"] diff --git a/Podfile.lock b/Podfile.lock index e284a4cd9..0b7c433b0 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -261,7 +261,7 @@ CHECKOUT OPTIONS: :commit: b60dc7d58dfc93ca6eafbb3ea5300c6d67ebc69a :git: https://github.com/signalapp/SignalCoreKit.git SignalMetadataKit: - :commit: d6b0186cc5b15019e85c375bce3bf78e7a7fc162 + :commit: a5473c8d33602775e00253afce78eef01a69260e :git: https://github.com/signalapp/SignalMetadataKit SocketRocket: :commit: 9f9563a83cd8960503074aa8de72206f83fb7a69 @@ -296,6 +296,6 @@ SPEC CHECKSUMS: YapDatabase: b418a4baa6906e8028748938f9159807fd039af4 YYImage: 1e1b62a9997399593e4b9c4ecfbbabbf1d3f3b54 -PODFILE CHECKSUM: 820287bc7925d7c20e02a02923976c60b1f5386b +PODFILE CHECKSUM: 55c6b62a23df23853a2af68917716a56a813c3f0 COCOAPODS: 1.5.3 diff --git a/Pods b/Pods index 5821eb8e9..37cee04ee 160000 --- a/Pods +++ b/Pods @@ -1 +1 @@ -Subproject commit 5821eb8e9fd9f76fbdbedd3402892c185e65c48e +Subproject commit 37cee04eebbe0ecbf61bd415383be9b1773dccef diff --git a/SignalServiceKit/src/Messages/OWSMessageDecrypter.m b/SignalServiceKit/src/Messages/OWSMessageDecrypter.m index 56d1c3949..ab75808e4 100644 --- a/SignalServiceKit/src/Messages/OWSMessageDecrypter.m +++ b/SignalServiceKit/src/Messages/OWSMessageDecrypter.m @@ -37,6 +37,7 @@ NSError *EnsureDecryptError(NSError *_Nullable error, NSString *fallbackErrorDes if (error) { return error; } + OWSCFailDebug(@"Caller should provide specific error"); return OWSErrorWithCodeDescription(OWSErrorCodeFailedToDecryptUDMessage, fallbackErrorDescription); } @@ -434,89 +435,140 @@ NSError *EnsureDecryptError(NSError *_Nullable error, NSString *fallbackErrorDes uint32_t localDeviceId = OWSDevicePrimaryDeviceId; [self.dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - @try { - NSError *error; - SMKSecretSessionCipher *_Nullable cipher = - [[SMKSecretSessionCipher alloc] initWithSessionStore:self.primaryStorage - preKeyStore:self.primaryStorage - signedPreKeyStore:self.primaryStorage - identityStore:self.identityManager - error:&error]; - if (error || !cipher) { - OWSFailDebug(@"Could not create secret session cipher: %@", error); - error = EnsureDecryptError(error, @"Could not create secret session cipher"); + NSError *cipherError; + SMKSecretSessionCipher *_Nullable cipher = + [[SMKSecretSessionCipher alloc] initWithSessionStore:self.primaryStorage + preKeyStore:self.primaryStorage + signedPreKeyStore:self.primaryStorage + identityStore:self.identityManager + error:&cipherError]; + if (cipherError || !cipher) { + OWSFailDebug(@"Could not create secret session cipher: %@", cipherError); + cipherError = EnsureDecryptError(cipherError, @"Could not create secret session cipher"); + return failureBlock(cipherError); + } + + NSError *decryptError; + SMKDecryptResult *_Nullable decryptResult = + [cipher throwswrapped_decryptMessageWithCertificateValidator:certificateValidator + cipherTextData:encryptedData + timestamp:serverTimestamp + localRecipientId:localRecipientId + localDeviceId:localDeviceId + protocolContext:transaction + error:&decryptError]; + + if (!decryptResult) { + if (!decryptError) { + OWSFailDebug(@"Caller should provide specific error"); + NSError *error = OWSErrorWithCodeDescription( + OWSErrorCodeFailedToDecryptUDMessage, @"Could not decrypt UD message"); return failureBlock(error); } - SMKDecryptResult *_Nullable decryptResult = - [cipher throwswrapped_decryptMessageWithCertificateValidator:certificateValidator - cipherTextData:encryptedData - timestamp:serverTimestamp - localRecipientId:localRecipientId - localDeviceId:localDeviceId - protocolContext:transaction - error:&error]; - SCKRaiseIfExceptionWrapperError(error); - - if (error || !decryptResult) { - if ([error.domain isEqualToString:@"SignalMetadataKit.SMKSecretSessionCipherError"] - && error.code == SMKSecretSessionCipherErrorSelfSentMessage) { - // Self-sent messages can be safely discarded. - return failureBlock(error); - } + // Decrypt Failure Part 1: Unwrap failure details - OWSFailDebug(@"Could not decrypt UD message: %@", error); - error = EnsureDecryptError(error, @"Could not decrypt UD message"); - return failureBlock(error); - } + NSError *_Nullable underlyingError; + SSKProtoEnvelope *_Nullable identifiedEnvelope; + + if (![decryptError.domain isEqualToString:@"SignalMetadataKit.SecretSessionKnownSenderError"]) { + underlyingError = decryptError; + identifiedEnvelope = envelope; + } else { + underlyingError = decryptError.userInfo[NSUnderlyingErrorKey]; + + NSString *senderRecipientId + = decryptError.userInfo[SecretSessionKnownSenderError.kSenderRecipientIdKey]; + OWSAssert(senderRecipientId); + + NSNumber *senderDeviceId = decryptError.userInfo[SecretSessionKnownSenderError.kSenderDeviceIdKey]; + OWSAssert(senderDeviceId); + + SSKProtoEnvelopeBuilder *identifiedEnvelopeBuilder = envelope.asBuilder; + identifiedEnvelopeBuilder.source = senderRecipientId; + identifiedEnvelopeBuilder.sourceDevice = senderDeviceId.unsignedIntValue; + NSError *identifiedEnvelopeBuilderError; - if (decryptResult.messageType == SMKMessageTypePrekey) { - [TSPreKeyManager checkPreKeys]; + identifiedEnvelope = [identifiedEnvelopeBuilder buildAndReturnError:&identifiedEnvelopeBuilderError]; + if (identifiedEnvelopeBuilderError) { + OWSFailDebug(@"failure identifiedEnvelopeBuilderError: %@", identifiedEnvelopeBuilderError); + } } + OWSAssert(underlyingError); + OWSAssert(identifiedEnvelope); - NSString *source = decryptResult.senderRecipientId; - if (source.length < 1 || !source.isValidE164) { - NSString *errorDescription = @"Invalid UD sender."; - OWSFailDebug(@"%@", errorDescription); - NSError *error = OWSErrorWithCodeDescription(OWSErrorCodeFailedToDecryptUDMessage, errorDescription); - return failureBlock(error); + NSException *_Nullable underlyingException; + if ([underlyingError.domain isEqualToString:SCKExceptionWrapperErrorDomain] + && underlyingError.code == SCKExceptionWrapperErrorThrown) { + + underlyingException = underlyingError.userInfo[SCKExceptionWrapperUnderlyingExceptionKey]; + OWSAssert(underlyingException); } - long sourceDeviceId = decryptResult.senderDeviceId; - if (sourceDeviceId < 1 || sourceDeviceId > UINT32_MAX) { - NSString *errorDescription = @"Invalid UD sender device id."; - OWSFailDebug(@"%@", errorDescription); - NSError *error = OWSErrorWithCodeDescription(OWSErrorCodeFailedToDecryptUDMessage, errorDescription); - return failureBlock(error); + // Decrypt Failure Part 2: Handle unwrapped failure details + + if (underlyingException) { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + [self processException:underlyingException envelope:identifiedEnvelope]; + NSString *errorDescription = [NSString + stringWithFormat:@"Exception while decrypting ud message: %@", underlyingException.description]; + OWSLogError(@"%@", errorDescription); + NSError *error = OWSErrorWithCodeDescription(OWSErrorCodeFailedToDecryptMessage, errorDescription); + failureBlock(error); + }); + return; } - NSData *plaintextData = [decryptResult.paddedPayload removePadding]; - - SSKProtoEnvelopeBuilder *envelopeBuilder = [envelope asBuilder]; - [envelopeBuilder setSource:source]; - [envelopeBuilder setSourceDevice:(uint32_t)sourceDeviceId]; - NSData *_Nullable newEnvelopeData = [envelopeBuilder buildSerializedDataAndReturnError:&error]; - if (error || !newEnvelopeData) { - OWSFailDebug(@"Could not update UD envelope data: %@", error); - error = EnsureDecryptError(error, @"Could not update UD envelope data"); - return failureBlock(error); + + if ([underlyingError.domain isEqualToString:@"SignalMetadataKit.SMKSecretSessionCipherError"] + && underlyingError.code == SMKSecretSessionCipherErrorSelfSentMessage) { + // Self-sent messages can be safely discarded. + failureBlock(underlyingError); + return; } - OWSMessageDecryptResult *result = [OWSMessageDecryptResult resultWithEnvelopeData:newEnvelopeData - plaintextData:plaintextData - source:source - sourceDevice:(uint32_t)sourceDeviceId - isUDMessage:YES]; - successBlock(result, transaction); - } @catch (NSException *exception) { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - [self processException:exception envelope:envelope]; - NSString *errorDescription = - [NSString stringWithFormat:@"Exception while decrypting ud message: %@", exception.description]; - OWSLogError(@"%@", errorDescription); - NSError *error = OWSErrorWithCodeDescription(OWSErrorCodeFailedToDecryptMessage, errorDescription); - failureBlock(error); - }); + OWSFailDebug(@"Could not decrypt UD message: %@", underlyingError); + failureBlock(underlyingError); + return; + } + + if (decryptResult.messageType == SMKMessageTypePrekey) { + [TSPreKeyManager checkPreKeys]; + } + + NSString *source = decryptResult.senderRecipientId; + if (source.length < 1 || !source.isValidE164) { + NSString *errorDescription = @"Invalid UD sender."; + OWSFailDebug(@"%@", errorDescription); + NSError *error = OWSErrorWithCodeDescription(OWSErrorCodeFailedToDecryptUDMessage, errorDescription); + return failureBlock(error); } + + long sourceDeviceId = decryptResult.senderDeviceId; + if (sourceDeviceId < 1 || sourceDeviceId > UINT32_MAX) { + NSString *errorDescription = @"Invalid UD sender device id."; + OWSFailDebug(@"%@", errorDescription); + NSError *error = OWSErrorWithCodeDescription(OWSErrorCodeFailedToDecryptUDMessage, errorDescription); + return failureBlock(error); + } + NSData *plaintextData = [decryptResult.paddedPayload removePadding]; + + SSKProtoEnvelopeBuilder *envelopeBuilder = [envelope asBuilder]; + [envelopeBuilder setSource:source]; + [envelopeBuilder setSourceDevice:(uint32_t)sourceDeviceId]; + NSError *envelopeBuilderError; + NSData *_Nullable newEnvelopeData = [envelopeBuilder buildSerializedDataAndReturnError:&envelopeBuilderError]; + if (envelopeBuilderError || !newEnvelopeData) { + OWSFailDebug(@"Could not update UD envelope data: %@", envelopeBuilderError); + NSError *error = EnsureDecryptError(envelopeBuilderError, @"Could not update UD envelope data"); + return failureBlock(error); + } + + OWSMessageDecryptResult *result = [OWSMessageDecryptResult resultWithEnvelopeData:newEnvelopeData + plaintextData:plaintextData + source:source + sourceDevice:(uint32_t)sourceDeviceId + isUDMessage:YES]; + successBlock(result, transaction); }]; } From 9a7f08b70b5bf651df13b29846d193292e1c3e57 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Fri, 16 Nov 2018 17:25:11 -0600 Subject: [PATCH 3/3] "Bump build to 2.31.0.38." --- Signal/Signal-Info.plist | 2 +- SignalShareExtension/Info.plist | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Signal/Signal-Info.plist b/Signal/Signal-Info.plist index fd897ecb9..0d741ac9d 100644 --- a/Signal/Signal-Info.plist +++ b/Signal/Signal-Info.plist @@ -49,7 +49,7 @@ CFBundleVersion - 2.31.0.37 + 2.31.0.38 ITSAppUsesNonExemptEncryption LOGS_EMAIL diff --git a/SignalShareExtension/Info.plist b/SignalShareExtension/Info.plist index 644fc8496..32abfbd4a 100644 --- a/SignalShareExtension/Info.plist +++ b/SignalShareExtension/Info.plist @@ -19,7 +19,7 @@ CFBundleShortVersionString 2.31.0 CFBundleVersion - 2.31.0.37 + 2.31.0.38 ITSAppUsesNonExemptEncryption NSAppTransportSecurity