From 75c3b385b2719361b3a417b0d4602c6d613ac6a3 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 20 Jul 2018 10:32:15 -0400 Subject: [PATCH] Respond to CR. --- .../src/Contacts/CDSSigningCertificate.m | 20 ++++++--- .../src/Contacts/ContactDiscoveryService.m | 45 ++++++++++++++----- .../API/Requests/CDSAttestationRequest.h | 2 + .../API/Requests/CDSAttestationRequest.m | 2 + .../Network/API/Requests/OWSRequestFactory.h | 1 + .../Network/API/Requests/OWSRequestFactory.m | 3 ++ .../src/Network/API/TSNetworkManager.m | 7 +-- 7 files changed, 56 insertions(+), 24 deletions(-) diff --git a/SignalServiceKit/src/Contacts/CDSSigningCertificate.m b/SignalServiceKit/src/Contacts/CDSSigningCertificate.m index c975378ef..e25b4ad56 100644 --- a/SignalServiceKit/src/Contacts/CDSSigningCertificate.m +++ b/SignalServiceKit/src/Contacts/CDSSigningCertificate.m @@ -200,13 +200,19 @@ NS_ASSUME_NONNULL_BEGIN + (nullable NSArray *)anchorCertificates { - // We need to use an Intel certificate as the anchor for IAS verification. - NSData *_Nullable anchorCertificate = [self certificateDataForService:@"ias-root"]; - if (!anchorCertificate) { - OWSProdLogAndFail(@"%@ could not load anchor certificate.", self.logTag); - return nil; - } - return @[ anchorCertificate ]; + static NSArray *anchorCertificates = nil; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + // We need to use an Intel certificate as the anchor for IAS verification. + NSData *_Nullable anchorCertificate = [self certificateDataForService:@"ias-root"]; + if (!anchorCertificate) { + OWSProdLogAndFail(@"%@ could not load anchor certificate.", self.logTag); + OWSRaiseException(@"OWSSignalService_CouldNotLoadCertificate", @"%s", __PRETTY_FUNCTION__); + } else { + anchorCertificates = @[ anchorCertificate ]; + } + }); + return anchorCertificates; } + (nullable NSData *)certificateDataForService:(NSString *)service diff --git a/SignalServiceKit/src/Contacts/ContactDiscoveryService.m b/SignalServiceKit/src/Contacts/ContactDiscoveryService.m index c0ad3f105..aa738d7b4 100644 --- a/SignalServiceKit/src/Contacts/ContactDiscoveryService.m +++ b/SignalServiceKit/src/Contacts/ContactDiscoveryService.m @@ -14,6 +14,21 @@ NS_ASSUME_NONNULL_BEGIN +@interface RemoteAttestationAuth : NSObject + +@property (nonatomic) NSString *username; +@property (nonatomic) NSString *authToken; + +@end + +#pragma mark - + +@implementation RemoteAttestationAuth + +@end + +#pragma mark - + @interface RemoteAttestationKeys : NSObject @property (nonatomic) ECKeyPair *keyPair; @@ -195,12 +210,12 @@ NS_ASSUME_NONNULL_BEGIN DDLogVerbose(@"%@ remote attestation auth success: %@", self.logTag, responseDict); dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - NSString *_Nullable authToken = [self parseAuthToken:responseDict]; - if (!authToken) { - DDLogError(@"%@ remote attestation auth missing token: %@", self.logTag, responseDict); + RemoteAttestationAuth *_Nullable auth = [self parseAuthToken:responseDict]; + if (!auth) { + DDLogError(@"%@ remote attestation auth could not be parsed: %@", self.logTag, responseDict); return; } - [self performRemoteAttestationWithToken:authToken]; + [self performRemoteAttestationWithAuth:auth]; }); } failure:^(NSURLSessionDataTask *task, NSError *error) { @@ -209,7 +224,7 @@ NS_ASSUME_NONNULL_BEGIN }]; } -- (nullable NSString *)parseAuthToken:(id)response +- (nullable RemoteAttestationAuth *)parseAuthToken:(id)response { if (![response isKindOfClass:[NSDictionary class]]) { return nil; @@ -218,35 +233,41 @@ NS_ASSUME_NONNULL_BEGIN NSDictionary *responseDict = response; NSString *_Nullable token = responseDict[@"token"]; if (![token isKindOfClass:[NSString class]]) { + OWSProdLogAndFail(@"%@ missing or invalid token.", self.logTag); return nil; } if (token.length < 1) { + OWSProdLogAndFail(@"%@ empty token.", self.logTag); return nil; } NSString *_Nullable username = responseDict[@"username"]; if (![username isKindOfClass:[NSString class]]) { + OWSProdLogAndFail(@"%@ missing or invalid username.", self.logTag); return nil; } if (username.length < 1) { + OWSProdLogAndFail(@"%@ empty username.", self.logTag); return nil; } - // To work around an idiosyncracy of the service implementation, - // we need to repeat the username twice in the token. - NSString *modifiedToken = [username stringByAppendingFormat:@":%@", token]; - DDLogVerbose(@"%@ attestation modified token: %@", self.logTag, modifiedToken); - return modifiedToken; + RemoteAttestationAuth *result = [RemoteAttestationAuth new]; + result.username = username; + result.authToken = token; + return result; } -- (void)performRemoteAttestationWithToken:(NSString *)authToken +- (void)performRemoteAttestationWithAuth:(RemoteAttestationAuth *)auth { ECKeyPair *keyPair = [Curve25519 generateKeyPair]; // TODO: NSString *enclaveId = @"cd6cfc342937b23b1bdd3bbf9721aa5615ac9ff50a75c5527d441cd3276826c9"; - TSRequest *request = [OWSRequestFactory remoteAttestationRequest:keyPair enclaveId:enclaveId authToken:authToken]; + TSRequest *request = [OWSRequestFactory remoteAttestationRequest:keyPair + enclaveId:enclaveId + username:auth.username + authToken:auth.authToken]; [[TSNetworkManager sharedManager] makeRequest:request success:^(NSURLSessionDataTask *task, id responseJson) { DDLogVerbose(@"%@ remote attestation success: %@", self.logTag, responseJson); diff --git a/SignalServiceKit/src/Network/API/Requests/CDSAttestationRequest.h b/SignalServiceKit/src/Network/API/Requests/CDSAttestationRequest.h index 4882e2db2..f34a0cffd 100644 --- a/SignalServiceKit/src/Network/API/Requests/CDSAttestationRequest.h +++ b/SignalServiceKit/src/Network/API/Requests/CDSAttestationRequest.h @@ -9,12 +9,14 @@ NS_ASSUME_NONNULL_BEGIN @interface CDSAttestationRequest : TSRequest @property (nonatomic, readonly) NSString *authToken; +@property (nonatomic, readonly) NSString *username; - (instancetype)init NS_UNAVAILABLE; - (TSRequest *)initWithURL:(NSURL *)URL method:(NSString *)method parameters:(nullable NSDictionary *)parameters + username:(NSString *)username authToken:(NSString *)authToken; @end diff --git a/SignalServiceKit/src/Network/API/Requests/CDSAttestationRequest.m b/SignalServiceKit/src/Network/API/Requests/CDSAttestationRequest.m index e79679722..fd769a6b0 100644 --- a/SignalServiceKit/src/Network/API/Requests/CDSAttestationRequest.m +++ b/SignalServiceKit/src/Network/API/Requests/CDSAttestationRequest.m @@ -11,11 +11,13 @@ NS_ASSUME_NONNULL_BEGIN - (TSRequest *)initWithURL:(NSURL *)URL method:(NSString *)method parameters:(nullable NSDictionary *)parameters + username:(NSString *)username authToken:(NSString *)authToken { OWSAssert(authToken.length > 0); if (self = [super initWithURL:URL method:method parameters:parameters]) { + _username = username; _authToken = authToken; } diff --git a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.h b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.h index 5ca8442ac..9b5442505 100644 --- a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.h +++ b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.h @@ -72,6 +72,7 @@ typedef NS_ENUM(NSUInteger, TSVerificationTransport) { TSVerificationTransportVo + (TSRequest *)remoteAttestationRequest:(ECKeyPair *)keyPair enclaveId:(NSString *)enclaveId + username:(NSString *)username authToken:(NSString *)authToken; + (TSRequest *)remoteAttestationAuthRequest; diff --git a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m index 628083469..3a51dd89b 100644 --- a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m +++ b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m @@ -278,10 +278,12 @@ NS_ASSUME_NONNULL_BEGIN + (TSRequest *)remoteAttestationRequest:(ECKeyPair *)keyPair enclaveId:(NSString *)enclaveId + username:(NSString *)username authToken:(NSString *)authToken { OWSAssert(keyPair); OWSAssert(enclaveId.length > 0); + OWSAssert(username.length > 0); OWSAssert(authToken.length > 0); NSString *path = @@ -292,6 +294,7 @@ NS_ASSUME_NONNULL_BEGIN // We DO NOT prepend the "key type" byte. @"clientPublic" : [keyPair.publicKey base64EncodedStringWithOptions:0], } + username:username authToken:authToken]; } diff --git a/SignalServiceKit/src/Network/API/TSNetworkManager.m b/SignalServiceKit/src/Network/API/TSNetworkManager.m index e73e77217..123868117 100644 --- a/SignalServiceKit/src/Network/API/TSNetworkManager.m +++ b/SignalServiceKit/src/Network/API/TSNetworkManager.m @@ -116,11 +116,8 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); } else { if ([request isKindOfClass:[CDSAttestationRequest class]]) { CDSAttestationRequest *attestationRequest = (CDSAttestationRequest *)request; - NSData *basicAuthCredentials = [attestationRequest.authToken dataUsingEncoding:NSUTF8StringEncoding]; - NSString *base64AuthCredentials = - [basicAuthCredentials base64EncodedStringWithOptions:(NSDataBase64EncodingOptions)0]; - [sessionManager.requestSerializer setValue:[NSString stringWithFormat:@"Basic %@", base64AuthCredentials] - forHTTPHeaderField:@"Authorization"]; + [sessionManager.requestSerializer setAuthorizationHeaderFieldWithUsername:attestationRequest.username + password:attestationRequest.authToken]; } else if (request.shouldHaveAuthorizationHeaders) { [sessionManager.requestSerializer setAuthorizationHeaderFieldWithUsername:[TSAccountManager localNumber]