From c4f8975308fb03d574a510eaa3b54dea47daf0c4 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 29 Oct 2018 21:26:40 -0600 Subject: [PATCH] Swift Exception wrap Curve25519 --- .../src/Contacts/ContactDiscoveryService.m | 4 +- .../src/Devices/OWSProvisioningCipher.m | 10 ++--- .../src/Messages/OWSMessageSender.m | 2 +- .../src/Messages/UD/OWSUDManager.swift | 37 ++++++++++--------- .../OWSPrimaryStorage+SignedPreKeyStore.m | 18 ++++++--- 5 files changed, 39 insertions(+), 32 deletions(-) diff --git a/SignalServiceKit/src/Contacts/ContactDiscoveryService.m b/SignalServiceKit/src/Contacts/ContactDiscoveryService.m index 922ecbdf6..c0d8dec35 100644 --- a/SignalServiceKit/src/Contacts/ContactDiscoveryService.m +++ b/SignalServiceKit/src/Contacts/ContactDiscoveryService.m @@ -83,9 +83,9 @@ NSErrorDomain const ContactDiscoveryServiceErrorDomain = @"SignalServiceKit.Cont NSData *ephemeralToStatic; @try { ephemeralToEphemeral = - [Curve25519 generateSharedSecretFromPublicKey:self.serverEphemeralPublic andKeyPair:self.keyPair]; + [Curve25519 try_generateSharedSecretFromPublicKey:self.serverEphemeralPublic andKeyPair:self.keyPair]; ephemeralToStatic = - [Curve25519 generateSharedSecretFromPublicKey:self.serverStaticPublic andKeyPair:self.keyPair]; + [Curve25519 try_generateSharedSecretFromPublicKey:self.serverStaticPublic andKeyPair:self.keyPair]; } @catch (NSException *exception) { OWSFailDebug(@"could not generate shared secrets: %@", exception); return NO; diff --git a/SignalServiceKit/src/Devices/OWSProvisioningCipher.m b/SignalServiceKit/src/Devices/OWSProvisioningCipher.m index 2541e4fb9..8f6746d85 100644 --- a/SignalServiceKit/src/Devices/OWSProvisioningCipher.m +++ b/SignalServiceKit/src/Devices/OWSProvisioningCipher.m @@ -54,11 +54,7 @@ NS_ASSUME_NONNULL_BEGIN - (nullable NSData *)encrypt:(NSData *)dataToEncrypt { @try { - // Exceptions can be thrown in a number of places in encryptUnsafe, e.g.: - // - // * Curve25519's generateSharedSecretFromPublicKey. - // * [HKDFKit deriveKey]. - return [self encryptUnsafe:dataToEncrypt]; + return [self try_encryptWithData:dataToEncrypt]; } @catch (NSException *exception) { OWSFailDebug(@"exception: %@ of type: %@ with reason: %@, user info: %@.", exception.description, @@ -69,10 +65,10 @@ NS_ASSUME_NONNULL_BEGIN } } -- (nullable NSData *)encryptUnsafe:(NSData *)dataToEncrypt +- (nullable NSData *)try_encryptWithData:(NSData *)dataToEncrypt { NSData *sharedSecret = - [Curve25519 generateSharedSecretFromPublicKey:self.theirPublicKey andKeyPair:self.ourKeyPair]; + [Curve25519 try_generateSharedSecretFromPublicKey:self.theirPublicKey andKeyPair:self.ourKeyPair]; NSData *infoData = [@"TextSecure Provisioning Message" dataUsingEncoding:NSASCIIStringEncoding]; NSData *nullSalt = [[NSMutableData dataWithLength:32] copy]; diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 6d87671b5..a56d1361b 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -479,7 +479,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; failure:(RetryableFailureHandler)failure { [self.udManager - ensureSenderCertificateObjCWithSuccess:^(SMKSenderCertificate *senderCertificate) { + trywrapped_ensureSenderCertificateWithSuccess:^(SMKSenderCertificate *senderCertificate) { dispatch_async([OWSDispatch sendingQueue], ^{ [self sendMessageToService:message senderCertificate:senderCertificate success:success failure:failure]; }); diff --git a/SignalServiceKit/src/Messages/UD/OWSUDManager.swift b/SignalServiceKit/src/Messages/UD/OWSUDManager.swift index 22ab6a428..f4d6a7af9 100644 --- a/SignalServiceKit/src/Messages/UD/OWSUDManager.swift +++ b/SignalServiceKit/src/Messages/UD/OWSUDManager.swift @@ -65,7 +65,7 @@ private func string(forUnidentifiedAccessMode mode: UnidentifiedAccessMode) -> S // We use completion handlers instead of a promise so that message sending // logic can access the strongly typed certificate data. @objc - func ensureSenderCertificateObjC(success:@escaping (SMKSenderCertificate) -> Void, + func trywrapped_ensureSenderCertificate(success:@escaping (SMKSenderCertificate) -> Void, failure:@escaping (Error) -> Void) // MARK: Unrestricted Access @@ -109,7 +109,9 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { guard TSAccountManager.isRegistered() else { return } - self.ensureSenderCertificate().retainUntilComplete() + + // Any error is silently ignored on startup. + self.trywrapped_ensureSenderCertificate().retainUntilComplete() } NotificationCenter.default.addObserver(self, selector: #selector(registrationStateDidChange), @@ -121,7 +123,8 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { func registrationStateDidChange() { AssertIsOnMainThread() - ensureSenderCertificate().retainUntilComplete() + // Any error is silently ignored + trywrapped_ensureSenderCertificate().retainUntilComplete() } // MARK: - @@ -263,12 +266,12 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { #if DEBUG @objc - public func hasSenderCertificate() -> Bool { - return senderCertificate() != nil + public func trywrapped_hasSenderCertificate() -> Bool { + return trywrapped_senderCertificate() != nil } #endif - private func senderCertificate() -> SMKSenderCertificate? { + private func trywrapped_senderCertificate() -> SMKSenderCertificate? { guard let certificateData = dbConnection.object(forKey: senderCertificateKey(), inCollection: kUDCollection) as? Data else { return nil } @@ -276,7 +279,7 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { do { let certificate = try SMKSenderCertificate.parse(data: certificateData) - guard isValidCertificate(certificate) else { + guard trywrapped_isValidCertificate(certificate) else { Logger.warn("Current sender certificate is not valid.") return nil } @@ -297,10 +300,10 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { } @objc - public func ensureSenderCertificateObjC(success:@escaping (SMKSenderCertificate) -> Void, - failure:@escaping (Error) -> Void) { + public func trywrapped_ensureSenderCertificate(success:@escaping (SMKSenderCertificate) -> Void, + failure:@escaping (Error) -> Void) { firstly { - ensureSenderCertificate() + trywrapped_ensureSenderCertificate() }.map { certificate in success(certificate) }.catch { error in @@ -308,15 +311,15 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { }.retainUntilComplete() } - public func ensureSenderCertificate() -> Promise { + public func trywrapped_ensureSenderCertificate() -> Promise { // If there is a valid cached sender certificate, use that. - if let certificate = senderCertificate() { + if let certificate = trywrapped_senderCertificate() { return Promise.value(certificate) } // Try to obtain a new sender certificate. return firstly { - requestSenderCertificate() + trywrapped_requestSenderCertificate() }.map { (certificateData: Data, certificate: SMKSenderCertificate) in // Cache the current sender certificate. @@ -326,13 +329,13 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { } } - private func requestSenderCertificate() -> Promise<(certificateData: Data, certificate: SMKSenderCertificate)> { + private func trywrapped_requestSenderCertificate() -> Promise<(certificateData: Data, certificate: SMKSenderCertificate)> { return firstly { SignalServiceRestClient().requestUDSenderCertificate() }.map { certificateData -> (certificateData: Data, certificate: SMKSenderCertificate) in let certificate = try SMKSenderCertificate.parse(data: certificateData) - guard self.isValidCertificate(certificate) else { + guard self.trywrapped_isValidCertificate(certificate) else { throw OWSUDError.invalidData(description: "Invalid sender certificate returned by server") } @@ -340,7 +343,7 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { } } - private func isValidCertificate(_ certificate: SMKSenderCertificate) -> Bool { + private func trywrapped_isValidCertificate(_ certificate: SMKSenderCertificate) -> Bool { // Ensure that the certificate will not expire in the next hour. // We want a threshold long enough to ensure that any outgoing message // sends will complete before the expiration. @@ -348,7 +351,7 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { let anHourFromNowMs = nowMs + kHourInMs do { - try certificateValidator.validate(senderCertificate: certificate, validationTime: anHourFromNowMs) + try certificateValidator.trywrapped_validate(senderCertificate: certificate, validationTime: anHourFromNowMs) return true } catch { OWSLogger.error("Invalid certificate") diff --git a/SignalServiceKit/src/Storage/AxolotlStore/OWSPrimaryStorage+SignedPreKeyStore.m b/SignalServiceKit/src/Storage/AxolotlStore/OWSPrimaryStorage+SignedPreKeyStore.m index 423cd3e7a..2e7394cd1 100644 --- a/SignalServiceKit/src/Storage/AxolotlStore/OWSPrimaryStorage+SignedPreKeyStore.m +++ b/SignalServiceKit/src/Storage/AxolotlStore/OWSPrimaryStorage+SignedPreKeyStore.m @@ -29,11 +29,19 @@ NSString *const OWSPrimaryStorageKeyPrekeyCurrentSignedPrekeyId = @"currentSigne // Signed prekey ids must be > 0. int preKeyId = 1 + arc4random_uniform(INT32_MAX - 1); ECKeyPair *_Nullable identityKeyPair = [[OWSIdentityManager sharedManager] identityKeyPair]; - return [[SignedPreKeyRecord alloc] - initWithId:preKeyId - keyPair:keyPair - signature:[Ed25519 sign:keyPair.publicKey.prependKeyType withKeyPair:identityKeyPair] - generatedAt:[NSDate date]]; + OWSAssert(identityKeyPair); + @try { + return [[SignedPreKeyRecord alloc] + initWithId:preKeyId + keyPair:keyPair + signature:[Ed25519 try_sign:keyPair.publicKey.prependKeyType withKeyPair:identityKeyPair] + generatedAt:[NSDate date]]; + } @catch (NSException *exception) { + // try_sign only throws when the data to sign is empty or `keyPair`. + // Neither of which should happen. + OWSFail(@"exception: %@", exception); + return nil; + } } - (SignedPreKeyRecord *)try_loadSignedPrekey:(int)signedPreKeyId