Merge branch 'mkirk/cds-failures' into release/2.30.2

Signal/Signal-Info.plist

@@ -60,6 +60,13 @@
 				<key>NSThirdPartyExceptionRequiresForwardSecrecy</key>
 				<false/>
 			</dict>
+			<key>api.directory.signal.org</key>
+			<dict>
+				<key>NSThirdPartyExceptionRequiresForwardSecrecy</key>
+				<false/>
+				<key>NSExceptionAllowsInsecureHTTPLoads</key>
+				<true/>
+			</dict>
 			<key>signal.org</key>
 			<dict>
 				<key>NSExceptionAllowsInsecureHTTPLoads</key>

SignalMessaging/environment/migrations/OWS106EnsureProfileComplete.swift

@@ -64,7 +64,7 @@ public class OWS106EnsureProfileComplete: OWSDatabaseMigration {
                 self.completionHandler(true)
             }.catch { error in
                 let nserror = error as NSError
-                if nserror.domain == TSNetworkManagerDomain {
+                if nserror.domain == TSNetworkManagerErrorDomain {
                     // Don't retry if we had an unrecoverable error.
                     // In particular, 401 (invalid auth) is unrecoverable.
                     let isUnrecoverableError = nserror.code == 401

SignalServiceKit/src/Account/TSAccountManager.m

@@ -386,7 +386,7 @@ NSString *const TSAccountManager_ServerSignalingKey = @"TSStorageServerSignaling
             if (!IsNSErrorNetworkFailure(error)) {
                 OWSProdError([OWSAnalyticsEvents accountsErrorVerifyAccountRequestFailed]);
             }
-            OWSAssertDebug([error.domain isEqualToString:TSNetworkManagerDomain]);
+            OWSAssertDebug([error.domain isEqualToString:TSNetworkManagerErrorDomain]);
 
             OWSLogWarn(@"Error verifying code: %@", error.debugDescription);
 

SignalServiceKit/src/Contacts/ContactDiscoveryService.h

@@ -4,6 +4,11 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+extern NSErrorDomain const ContactDiscoveryServiceErrorDomain;
+typedef NS_ERROR_ENUM(ContactDiscoveryServiceErrorDomain, ContactDiscoveryServiceError){
+    ContactDiscoveryServiceErrorAttestationFailed = 100,
+};
+
 @class ECKeyPair;
 @class OWSAES256Key;
 

SignalServiceKit/src/Contacts/ContactDiscoveryService.m

@@ -8,6 +8,7 @@
 #import "Cryptography.h"
 #import "NSData+OWS.h"
 #import "NSDate+OWS.h"
+#import "NSError+MessageSending.h"
 #import "OWSError.h"
 #import "OWSRequestFactory.h"
 #import "TSNetworkManager.h"
@@ -16,6 +17,8 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+NSErrorDomain const ContactDiscoveryServiceErrorDomain = @"SignalServiceKit.ContactDiscoveryService";
+
 @interface RemoteAttestationAuth ()
 
 @property (nonatomic) NSString *username;
@@ -357,7 +360,10 @@ NS_ASSUME_NONNULL_BEGIN
                                                                                          auth:auth];
 
                 if (!attestation) {
-                    NSError *error = OWSErrorMakeUnableToProcessServerResponseError();
+                    NSError *error = [NSError errorWithDomain:ContactDiscoveryServiceErrorDomain
+                                                         code:ContactDiscoveryServiceErrorAttestationFailed
+                                                     userInfo:nil];
+                    error.isRetryable = NO;
                     failureHandler(error);
                     return;
                 }
@@ -366,8 +372,6 @@ NS_ASSUME_NONNULL_BEGIN
             });
         }
         failure:^(NSURLSessionDataTask *task, NSError *error) {
-            NSHTTPURLResponse *response = (NSHTTPURLResponse *)task.response;
-            OWSLogVerbose(@"remote attestation failure: %lu", (unsigned long)response.statusCode);
             failureHandler(error);
         }];
 }

SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift

@@ -139,7 +139,6 @@ class LegacyContactDiscoveryBatchOperation: OWSOperation {
 enum ContactDiscoveryError: Error {
     case parseError(description: String)
     case assertionError(description: String)
-    case attestationError(underlyingError: Error)
     case clientError(underlyingError: Error)
     case serverError(underlyingError: Error)
 }
@@ -234,13 +233,7 @@ class CDSBatchOperation: OWSOperation {
         contactDiscoveryService.performRemoteAttestation(success: { (remoteAttestation: RemoteAttestation) in
             self.makeContactDiscoveryRequest(remoteAttestation: remoteAttestation)
         },
-                                                         failure: self.attestationFailure)
+                                                         failure: self.reportError)
-    }
-
-    private func attestationFailure(error: Error) {
-        let attestationError: NSError = ContactDiscoveryError.attestationError(underlyingError: error) as NSError
-        attestationError.isRetryable = false
-        self.reportError(attestationError)
     }
 
     private func makeContactDiscoveryRequest(remoteAttestation: RemoteAttestation) {
@@ -456,10 +449,13 @@ class CDSFeedbackOperation: OWSOperation {
 
         if let error = cdsOperation.failingError {
             switch error {
+            case TSNetworkManagerError.failedConnection:
+                // Don't submit feedback for connectivity errors
+                self.reportSuccess()
             case ContactDiscoveryError.serverError, ContactDiscoveryError.clientError:
-                // Server already has this information, no need to report.
+                // Server already has this information, no need submit feedback
                 self.reportSuccess()
-            case ContactDiscoveryError.attestationError:
+            case ContactDiscoveryServiceError.attestationFailed:
                 self.makeRequest(result: .attestationError)
             default:
                 self.makeRequest(result: .unexpectedError)

SignalServiceKit/src/Network/API/TSNetworkManager.h

@@ -6,7 +6,15 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-extern NSString *const TSNetworkManagerDomain;
+extern NSErrorDomain const TSNetworkManagerErrorDomain;
+typedef NS_ERROR_ENUM(TSNetworkManagerErrorDomain, TSNetworkManagerError){
+    // It's a shame to use 0 as an enum value for anything other than something like default or unknown, because it's
+    // indistinguishable from "not set" in Objc.
+    // However this value was existing behavior for connectivity errors, and since we might be using this in other
+    // places I didn't want to change it out of hand
+    TSNetworkManagerErrorFailedConnection = 0,
+    // Other TSNetworkManagerError's use HTTP status codes (e.g. 404, etc)
+};
 
 BOOL IsNSErrorNetworkFailure(NSError *_Nullable error);
 

SignalServiceKit/src/Network/API/TSNetworkManager.m

@@ -7,6 +7,7 @@
 #import "NSData+OWS.h"
 #import "NSError+messageSending.h"
 #import "NSURLSessionDataTask+StatusCode.h"
+#import "OWSError.h"
 #import "OWSSignalService.h"
 #import "SSKEnvironment.h"
 #import "TSAccountManager.h"
@@ -14,11 +15,12 @@
 #import <AFNetworking/AFNetworking.h>
 #import <SignalServiceKit/SignalServiceKit-Swift.h>
 
-NSString *const TSNetworkManagerDomain = @"org.whispersystems.signal.networkManager";
+NSErrorDomain const TSNetworkManagerErrorDomain = @"SignalServiceKit.TSNetworkManager";
 
 BOOL IsNSErrorNetworkFailure(NSError *_Nullable error)
 {
-    return ([error.domain isEqualToString:TSNetworkManagerDomain] && error.code == 0);
+    return ([error.domain isEqualToString:TSNetworkManagerErrorDomain]
+        && error.code == TSNetworkManagerErrorFailedConnection);
 }
 
 @interface TSNetworkManager ()
@@ -218,16 +220,17 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error);
 
       switch (statusCode) {
           case 0: {
-              error.isRetryable = YES;
+              NSError *connectivityError =
-
+                  [self errorWithHTTPCode:TSNetworkManagerErrorFailedConnection
-              OWSLogWarn(@"The network request failed because of a connectivity error: %@", request);
-              failureBlock(task,
-                  [self errorWithHTTPCode:statusCode
                               description:NSLocalizedString(@"ERROR_DESCRIPTION_NO_INTERNET",
                                               @"Generic error used whenever Signal can't contact the server")
                             failureReason:networkError.localizedFailureReason
                        recoverySuggestion:NSLocalizedString(@"NETWORK_ERROR_RECOVERY", nil)
-                            fallbackError:networkError]);
+                            fallbackError:networkError];
+              connectivityError.isRetryable = YES;
+
+              OWSLogWarn(@"The network request failed because of a connectivity error: %@", request);
+              failureBlock(task, connectivityError);
               break;
           }
           case 400: {
@@ -327,7 +330,10 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error);
                    description:(NSString *)description
                  failureReason:(NSString *)failureReason
             recoverySuggestion:(NSString *)recoverySuggestion
-                 fallbackError:(NSError *_Nonnull)fallbackError {
+                 fallbackError:(NSError *)fallbackError
+{
+    OWSAssertDebug(fallbackError);
+
     if (!description) {
         description = fallbackError.localizedDescription;
     }
@@ -356,7 +362,9 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error);
         [dict setObject:failureData forKey:AFNetworkingOperationFailingURLResponseDataErrorKey];
     }
 
-    return [NSError errorWithDomain:TSNetworkManagerDomain code:code userInfo:dict];
+    dict[NSUnderlyingErrorKey] = fallbackError;
+
+    return [NSError errorWithDomain:TSNetworkManagerErrorDomain code:code userInfo:dict];
 }
 
 @end