Fixup certificate parsing tests

Skip failure when running tests when we're explicitly testing failure cases.

Be more specific about failure conditions via NSError param

Signal/test/util/CDSSigningCertificateTest.m

@@ -17,15 +17,20 @@
 - (void)testParsing_good
 {
     NSString *pem = [self certificatesPem_good];
-    CDSSigningCertificate *_Nullable certificate = [CDSSigningCertificate parseCertificateFromPem:pem];
+    NSError *error;
+    CDSSigningCertificate *_Nullable certificate = [CDSSigningCertificate parseCertificateFromPem:pem error:&error];
     XCTAssertNotNil(certificate);
+    XCTAssertNil(error);
 }
 
 - (void)testParsing_bad
 {
     NSString *pem = [self certificatesPem_bad];
-
+    NSError *error;
-    XCTAssertThrows([CDSSigningCertificate parseCertificateFromPem:pem]);
+    CDSSigningCertificate *_Nullable certificate = [CDSSigningCertificate parseCertificateFromPem:pem error:&error];
+    XCTAssertNil(certificate);
+    XCTAssertNotNil(error);
+    XCTAssertEqual(error.code, CDSSigningCertificateError_InvalidDistinguishedName);
 }
 
 - (void)testVerification_good
@@ -33,9 +38,11 @@
     NSString *pem = [self certificatesPem_good];
     NSData *signature = [self signature_good];
     NSString *bodyString = [self bodyString_good];
-    
+
-    CDSSigningCertificate *_Nullable certificate = [CDSSigningCertificate parseCertificateFromPem:pem];
+    NSError *error;
+    CDSSigningCertificate *_Nullable certificate = [CDSSigningCertificate parseCertificateFromPem:pem error:&error];
     XCTAssertNotNil(certificate);
+    XCTAssertNil(error);
 
     BOOL result = [certificate verifySignatureOfBody:bodyString signature:signature];
     XCTAssertTrue(result);
@@ -46,11 +53,13 @@
     NSString *pem = [self certificatesPem_good];
     NSData *signature = [self signature_good];
     NSString *bodyString = [self bodyString_bad];
-    
+
-    CDSSigningCertificate *_Nullable certificate = [CDSSigningCertificate parseCertificateFromPem:pem];
+    NSError *error;
+    CDSSigningCertificate *_Nullable certificate = [CDSSigningCertificate parseCertificateFromPem:pem error:&error];
     XCTAssertNotNil(certificate);
+    XCTAssertNil(error);
 
-    XCTAssertThrows([certificate verifySignatureOfBody:bodyString signature:signature]);
+    XCTAssertFalse([certificate verifySignatureOfBody:bodyString signature:signature]);
 }
 
 - (void)testVerification_badSignature
@@ -58,13 +67,17 @@
     NSString *pem = [self certificatesPem_good];
     NSData *signature = [self signature_bad];
     NSString *bodyString = [self bodyString_good];
-    
+
-    CDSSigningCertificate *_Nullable certificate = [CDSSigningCertificate parseCertificateFromPem:pem];
+    NSError *error;
+    CDSSigningCertificate *_Nullable certificate = [CDSSigningCertificate parseCertificateFromPem:pem error:&error];
     XCTAssertNotNil(certificate);
+    XCTAssertNil(error);
 
-    XCTAssertThrows([certificate verifySignatureOfBody:bodyString signature:signature]);
+    XCTAssertFalse([certificate verifySignatureOfBody:bodyString signature:signature]);
 }
 
+#pragma mark - test values
+
 - (NSString *)certificatesPem_good {
     return @"-----BEGIN CERTIFICATE----- \
     MIIEoTCCAwmgAwIBAgIJANEHdl0yo7CWMA0GCSqGSIb3DQEBCwUAMH4xCzAJBgNV \

SignalServiceKit/src/Contacts/CDSSigningCertificate.h

@@ -4,9 +4,24 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+typedef NS_ENUM(NSUInteger, CDSSigningCertificateErrorCode) {
+    // AssertionError's indicate either developer or some serious system error that should never happen.
+    //
+    // Do not use this for an "expected" error, e.g. something that could be induced by user input which
+    // we specifically need to handle gracefull.
+    CDSSigningCertificateError_AssertionError = 1,
+
+    CDSSigningCertificateError_InvalidPEMSupplied,
+    CDSSigningCertificateError_CouldNotExtractLeafCertificate,
+    CDSSigningCertificateError_InvalidDistinguishedName,
+    CDSSigningCertificateError_UntrustedCertificate
+};
+
+NSError *CDSSigningCertificateErrorMake(CDSSigningCertificateErrorCode code, NSString *localizedDescription);
+
 @interface CDSSigningCertificate : NSObject
 
-+ (nullable CDSSigningCertificate *)parseCertificateFromPem:(NSString *)certificatePem;
++ (nullable CDSSigningCertificate *)parseCertificateFromPem:(NSString *)certificatePem error:(NSError **)error;
 
 - (BOOL)verifySignatureOfBody:(NSString *)body signature:(NSData *)theirSignature;
 

SignalServiceKit/src/Contacts/CDSSigningCertificate.m

@@ -10,6 +10,13 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+NSError *CDSSigningCertificateErrorMake(CDSSigningCertificateErrorCode code, NSString *localizedDescription)
+{
+    return [NSError errorWithDomain:@"CDSSigningCertificate"
+                               code:code
+                           userInfo:@{ NSLocalizedDescriptionKey : localizedDescription }];
+}
+
 @interface CDSSigningCertificate ()
 
 @property (nonatomic) SecPolicyRef policy;
@@ -49,15 +56,18 @@ NS_ASSUME_NONNULL_BEGIN
     }
 }
 
-+ (nullable CDSSigningCertificate *)parseCertificateFromPem:(NSString *)certificatePem
++ (nullable CDSSigningCertificate *)parseCertificateFromPem:(NSString *)certificatePem error:(NSError **)error
 {
     OWSAssertDebug(certificatePem);
+    *error = nil;
 
     CDSSigningCertificate *signingCertificate = [CDSSigningCertificate new];
 
     NSArray<NSData *> *_Nullable anchorCertificates = [self anchorCertificates];
     if (anchorCertificates.count < 1) {
         OWSFailDebug(@"Could not load anchor certificates.");
+        *error = CDSSigningCertificateErrorMake(
+            CDSSigningCertificateError_AssertionError, @"Could not load anchor certificates.");
         return nil;
     }
 
@@ -65,6 +75,7 @@ NS_ASSUME_NONNULL_BEGIN
 
     if (certificateDerDatas.count < 1) {
         OWSFailDebug(@"Could not parse PEM.");
+        *error = CDSSigningCertificateErrorMake(CDSSigningCertificateError_InvalidPEMSupplied, @"Could not parse PEM.");
         return nil;
     }
 
@@ -72,10 +83,14 @@ NS_ASSUME_NONNULL_BEGIN
     NSData *_Nullable leafCertificateData = [certificateDerDatas firstObject];
     if (!leafCertificateData) {
         OWSFailDebug(@"Could not extract leaf certificate data.");
+        *error = CDSSigningCertificateErrorMake(
+            CDSSigningCertificateError_CouldNotExtractLeafCertificate, @"Could not extract leaf certificate data.");
         return nil;
     }
     if (![self verifyDistinguishedNameOfCertificate:leafCertificateData]) {
         OWSFailDebug(@"Leaf certificate has invalid name.");
+        *error = CDSSigningCertificateErrorMake(
+            CDSSigningCertificateError_InvalidDistinguishedName, @"Could not extract leaf certificate data.");
         return nil;
     }
 
@@ -83,7 +98,9 @@ NS_ASSUME_NONNULL_BEGIN
     for (NSData *certificateDerData in certificateDerDatas) {
         SecCertificateRef certificate = SecCertificateCreateWithData(NULL, (__bridge CFDataRef)(certificateDerData));
         if (!certificate) {
-            OWSFailDebug(@"Could not load DER.");
+            OWSFailDebug(@"Could not create SecCertificate.");
+            *error = CDSSigningCertificateErrorMake(
+                CDSSigningCertificateError_AssertionError, @"Could not create SecCertificate.");
             return nil;
         }
         [certificates addObject:(__bridge_transfer id)certificate];
@@ -93,6 +110,7 @@ NS_ASSUME_NONNULL_BEGIN
     signingCertificate.policy = policy;
     if (!policy) {
         OWSFailDebug(@"Could not create policy.");
+        *error = CDSSigningCertificateErrorMake(CDSSigningCertificateError_AssertionError, @"Could not create policy.");
         return nil;
     }
 
@@ -100,23 +118,30 @@ NS_ASSUME_NONNULL_BEGIN
     OSStatus status = SecTrustCreateWithCertificates((__bridge CFTypeRef)certificates, policy, &trust);
     signingCertificate.trust = trust;
     if (status != errSecSuccess) {
-        OWSFailDebug(@"trust could not be created.");
+        OWSFailDebug(@"Creating trust did not succeed.");
+        *error = CDSSigningCertificateErrorMake(
+            CDSSigningCertificateError_AssertionError, @"Creating trust did not succeed.");
         return nil;
     }
     if (!trust) {
         OWSFailDebug(@"Could not create trust.");
+        *error = CDSSigningCertificateErrorMake(CDSSigningCertificateError_AssertionError, @"Could not create trust.");
         return nil;
     }
 
     status = SecTrustSetNetworkFetchAllowed(trust, NO);
     if (status != errSecSuccess) {
         OWSFailDebug(@"trust fetch could not be configured.");
+        *error = CDSSigningCertificateErrorMake(
+            CDSSigningCertificateError_AssertionError, @"trust fetch could not be configured.");
         return nil;
     }
 
     status = SecTrustSetAnchorCertificatesOnly(trust, YES);
     if (status != errSecSuccess) {
         OWSFailDebug(@"trust anchor certs could not be configured.");
+        *error = CDSSigningCertificateErrorMake(
+            CDSSigningCertificateError_AssertionError, @"trust anchor certs could not be configured.");
         return nil;
     }
 
@@ -124,7 +149,9 @@ NS_ASSUME_NONNULL_BEGIN
     for (NSData *certificateData in anchorCertificates) {
         SecCertificateRef certificate = SecCertificateCreateWithData(NULL, (__bridge CFDataRef)(certificateData));
         if (!certificate) {
-            OWSFailDebug(@"Could not load DER.");
+            OWSFailDebug(@"Could not create pinned SecCertificate.");
+            *error = CDSSigningCertificateErrorMake(
+                CDSSigningCertificateError_AssertionError, @"Could not create pinned SecCertificate.");
             return nil;
         }
 
@@ -133,6 +160,8 @@ NS_ASSUME_NONNULL_BEGIN
     status = SecTrustSetAnchorCertificates(trust, (__bridge CFArrayRef)pinnedCertificates);
     if (status != errSecSuccess) {
         OWSFailDebug(@"The anchor certificates couldn't be set.");
+        *error = CDSSigningCertificateErrorMake(
+            CDSSigningCertificateError_AssertionError, @"The anchor certificates couldn't be set.");
         return nil;
     }
 
@@ -140,6 +169,8 @@ NS_ASSUME_NONNULL_BEGIN
     status = SecTrustEvaluate(trust, &result);
     if (status != errSecSuccess) {
         OWSFailDebug(@"Could not evaluate certificates.");
+        *error = CDSSigningCertificateErrorMake(
+            CDSSigningCertificateError_AssertionError, @"Could not evaluate certificates.");
         return nil;
     }
 
@@ -147,7 +178,9 @@ NS_ASSUME_NONNULL_BEGIN
     // See the comments in the header where it is defined.
     BOOL isValid = (result == kSecTrustResultUnspecified || result == kSecTrustResultProceed);
     if (!isValid) {
-        OWSFailDebug(@"Certificate evaluation failed.");
+        OWSFailDebug(@"Certificate was not trusted.");
+        *error = CDSSigningCertificateErrorMake(
+            CDSSigningCertificateError_UntrustedCertificate, @"Certificate was not trusted.");
         return nil;
     }
 
@@ -155,6 +188,8 @@ NS_ASSUME_NONNULL_BEGIN
     signingCertificate.publicKey = publicKey;
     if (!publicKey) {
         OWSFailDebug(@"Could not extract public key.");
+        *error = CDSSigningCertificateErrorMake(
+            CDSSigningCertificateError_AssertionError, @"Could not extract public key.");
         return nil;
     }
 

SignalServiceKit/src/Contacts/ContactDiscoveryService.m

@@ -509,7 +509,14 @@ NSErrorDomain const ContactDiscoveryServiceErrorDomain = @"SignalServiceKit.Cont
     OWSAssertDebug(signature.length > 0);
     OWSAssertDebug(quoteData);
 
-    CDSSigningCertificate *_Nullable certificate = [CDSSigningCertificate parseCertificateFromPem:certificates];
+    NSError *error;
+    CDSSigningCertificate *_Nullable certificate =
+        [CDSSigningCertificate parseCertificateFromPem:certificates error:&error];
+    if (error) {
+        OWSFailDebug(@"error when parsing signing certificate. %@", error.localizedDescription);
+        return NO;
+    }
+
     if (!certificate) {
         OWSFailDebug(@"could not parse signing certificate.");
         return NO;