CBL-7654 : Fix MultipeerCertificateAuthenticator created with root certs fails to validate peer certs (Port) (#3478)

* Directly ported the fix from release/3.3 branch (5d13587)

* The cause of the issue is in CBLTrustCheck that doesn't return an error when AnchorTrusted problem is found.

* In addition, ported MultipeerReplicatorTest enhancement that uses different CN on each test from release/3.3 branch (3da8ee8)

Author: pasin

Objective-C/Internal/Replicator/CBLTrustCheck.mm

@@ -80,8 +80,11 @@ - (NSString*) description {
     return [NSString stringWithFormat: @"%@[%@:%@]", self.class, (_host ?: @""), port];
 }
 
-// Checks whether the reported problems with a SecTrust are OK.
-// Currently do not accept any problems.
+/**
+ Validates whether the issues reported by SecTrust are acceptable if trust validation
+ result is not kSecTrustResultProceed or kSecTrustResultUnspecified.
+ Note: Used only when no pinned certificate or self-signed certificate restriction is set.
+ */
 - (BOOL) shouldAcceptProblems: (NSError**)outError {
     NSDictionary* resultDict = CFBridgingRelease(SecTrustCopyResult(_trust));
     NSArray* detailsArray = resultDict[@"TrustResultDetails"];
@@ -95,15 +98,21 @@ - (BOOL) shouldAcceptProblems: (NSError**)outError {
                     [details[problem] isEqual: @[@(CSSMERR_APPLETP_HOSTNAME_MISMATCH)]])
         #endif
             ) {
+                if (!_host) {
+                    continue; // When no host specified, accept and check next problem
+                }
                 MYReturnError(outError, NSURLErrorServerCertificateUntrusted, NSURLErrorDomain,
                                   @"Server TLS certificate has a hostname mismatch.");
-            } else if ([problem isEqualToString: @"AnchorTrusted"]) {
-                MYReturnError(outError, NSURLErrorServerCertificateHasUnknownRoot, NSURLErrorDomain,
-                              @"Server TLS certificate has an unknown root or is self-signed.");
+                return NO;
             } else {
-                // Any other problem:
-                MYReturnError(outError, NSURLErrorServerCertificateUntrusted, NSURLErrorDomain,
-                              @"Server TLS certificate is untrusted (problem = %@)", problem);
+                if ([problem isEqualToString: @"AnchorTrusted"]) {
+                    MYReturnError(outError, NSURLErrorServerCertificateHasUnknownRoot, NSURLErrorDomain,
+                                  @"Server TLS certificate has an unknown root or is self-signed.");
+                } else {
+                    // Any other problem:
+                    MYReturnError(outError, NSURLErrorServerCertificateUntrusted, NSURLErrorDomain,
+                                  @"Server TLS certificate is untrusted (problem = %@)", problem);
+                }
                 return NO;
             }
         }
@@ -174,38 +183,26 @@ - (nullable NSURLCredential*) checkTrust: (NSError**)outError {
 
     // If using pinned cert, accept cert if it matches the pinned cert:
     if (_pinnedCertData) {
-        CFIndex count = SecTrustGetCertificateCount(_trust);
-#if __IPHONE_OS_VERSION_MAX_REQUIRED >= 150000
-        if (@available(iOS 15.0, *)) {
-            NSData* certData = nil;
-            for (CFIndex i = 0; i < count; i++) {
-                CFArrayRef certs = SecTrustCopyCertificateChain(_trust);
-                SecCertificateRef cert = (SecCertificateRef)CFArrayGetValueAtIndex(certs, i);
-                certData = CFBridgingRelease(SecCertificateCopyData(cert));
+        CFArrayRef certs = SecTrustCopyCertificateChain(_trust);
+        CFIndex count = CFArrayGetCount(certs);
+        for (CFIndex i = 0; i < count; i++) {
+            SecCertificateRef cert = (SecCertificateRef)CFArrayGetValueAtIndex(certs, i);
+            NSData* certData = CFBridgingRelease(SecCertificateCopyData(cert));
+            if ([_pinnedCertData isEqual:certData]) {
                 CFRelease(certs);
-                if ([_pinnedCertData isEqual: certData]) {
-                    [self forceTrusted];
-                    return credential;
-                }
-            }
-        } else
-#endif
-        {
-            for (CFIndex i = 0; i < count; i++) {
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-                SecCertificateRef cert = SecTrustGetCertificateAtIndex(_trust, i);
-#pragma clang diagnostic pop
-                if ([_pinnedCertData isEqual: CFBridgingRelease(SecCertificateCopyData(cert))]) {
-                    [self forceTrusted];
-                    return credential;
-                }
+                [self forceTrusted];
+                return credential;
             }
         }
-        MYReturnError(outError, NSURLErrorServerCertificateHasUnknownRoot, NSURLErrorDomain,
+
+        CFRelease(certs);
+        MYReturnError(outError,
+                      NSURLErrorServerCertificateHasUnknownRoot,
+                      NSURLErrorDomain,
                       @"Server TLS Certificate does not match the pinned cert.");
         return nil;
     }
+    
 #ifdef COUCHBASE_ENTERPRISE
     else if (_acceptOnlySelfSignedCert) {
         NSError* err;

Objective-C/Internal/Replicator/CBLWebSocket.mm

@@ -973,7 +973,10 @@ - (SecTrustRef) getTrustFromReadStream {
                                                   kCFStreamPropertySSLPeerTrust);
 }
 
-// Notify the received cert and verify the cert if using custom verification logic
+// Handles SSL certificate validation for the peer.
+// Performs custom trust evaluation if enabled, notifies certificate to
+// CBLWebSocketContext's callback and C4Socket and closes the connection on failure.
+// Returns YES if the certificate is accepted.
 - (BOOL) checkSSLCert {
     _shouldCheckSSLCert = NO;
 
@@ -982,10 +985,16 @@ - (BOOL) checkSSLCert {
 
     SecCertificateRef cert = [self certificateFromTrust: trust];
     [self notifyServerCertificateReceived: cert];
-
+    
     NSError* error = nil;
+    
+    // Validate trust only when using custom certificate validation
+    // (kCFStreamSSLValidatesCertificateChain is disabled).
+    // If system validation is enabled, the certificates have already been verified,
+    // and any validation failure will trigger NSStreamEventErrorOccurred without
+    // calling this method.
     if ([self usesCustomTLSCertValidation]) {
-        if (![self validateTrust:trust error:&error]) {
+        if (![self validateTrust: trust error: &error]) {
             [self closeWithError: error];
             return NO;
         }
@@ -1018,18 +1027,17 @@ - (BOOL) validateTrust: (SecTrustRef)trust error: (NSError**)error {
     NSURL* url = _logic.URL;
     CBLTrustCheck* check = [[CBLTrustCheck alloc] initWithTrust: trust host: url.host port: url.port.shortValue];
 
-    NSURLCredential* credentials = nil;
     Value pinnedCert = _options[kC4ReplicatorOptionPinnedServerCert];
     if (pinnedCert) {
         check.pinnedCertData = slice(pinnedCert.asData()).copiedNSData();
-        credentials = [check checkTrust: error];
     }
 #ifdef COUCHBASE_ENTERPRISE
     else if (_options[kC4ReplicatorOptionOnlySelfSignedServerCert].asBool()) {
         check.acceptOnlySelfSignedCert = YES;
-        credentials = [check checkTrust: error];
     }
 #endif
+    
+    NSURLCredential* credentials = [check checkTrust: error];
     if (!credentials) {
         CBLWarn(WebSocket, @"%@: TLS handshake failed: certificate verification error: %@", self, (*error).localizedDescription);
         return false;

Objective-C/Tests/MultipeerReplicatorTest.m

@@ -129,15 +129,18 @@ - (CBLTLSIdentity*) issuer {
 
 - (CBLTLSIdentity*) createIdentity {
     Assert(_identityCount++ <= kTestMaxIdentity);
-    
     NSError* error;
-    NSString* name = [self identityNameForNumber: _identityCount];
-    NSDictionary* attrs = @{ kCBLCertAttrCommonName: name };
+    NSString* label = [self identityNameForNumber: _identityCount];
+    
+    uint64_t timestamp = (uint64_t)([[NSDate date] timeIntervalSince1970] * 1000);
+    NSString* cn = [NSString stringWithFormat: @"%@-%llu", label, timestamp];
+    NSDictionary* attrs = @{ kCBLCertAttrCommonName: cn };
+    
     CBLTLSIdentity* identity = [CBLTLSIdentity createIdentityForKeyUsages: kTestKeyUsages
                                                                attributes: attrs
                                                                expiration: nil
                                                                    issuer: [self issuer]
-                                                                    label: name
+                                                                    label: label
                                                                     error: &error];
 
     AssertNotNil(identity);
@@ -406,6 +409,7 @@ - (void) testConfiguration {
  4. Check that the configuration cannot be created as the collections are empty.
  */
 - (void) testConfigurationValidation {
+    // When exception is thrown, the object will not get released:
     self.disableObjectLeakCheck = YES;
 
     CBLTLSIdentity* identity = [self createIdentity];
@@ -1085,8 +1089,6 @@ - (void) runConflictResolverTestWithResolver: (nullable MultipeerConflictResolve
     [self waitForReplicatorStatus: repl1 peerID: repl2.peerID activityLevel: kCBLReplicatorIdle];
     [self waitForReplicatorStatus: repl2 peerID: repl1.peerID activityLevel: kCBLReplicatorIdle];
 
-    NSLog(@"-----------------------------------------");
-    
     // Stops:
     [self stopMultipeerReplicator: repl1];
     [self stopMultipeerReplicator: repl2];
@@ -1176,7 +1178,6 @@ - (void) runConflictResolverTestWithResolver: (nullable MultipeerConflictResolve
   18. Check the doc1 in both peers and verify that the its body is {"greeting": "howdy"}.
   */
 - (void) testDefaultConflictResolver {
-    CBLLogSinks.console = [[CBLConsoleLogSink alloc] initWithLevel: kCBLLogLevelVerbose];
     [self runConflictResolverTestWithResolver: nil verifyBlock: ^BOOL (CBLDocument* resolvedDoc) {
         return [[resolvedDoc stringForKey: @"greeting"] isEqualToString: @"howdy"];
     }];

Objective-C/Tests/TrustCheckTest.m

@@ -228,7 +228,7 @@ - (void) testTrustCHeckWithRootCerts {
     SecCertificateRef secCert3 = [self createSecCertFromPEM: cert3];
 
     SecTrustRef trust;
-    SecPolicyRef policy = SecPolicyCreateSSL(true, (__bridge CFStringRef)_host);
+    SecPolicyRef policy = SecPolicyCreateBasicX509();
 
     // Validate leaf cert:
 
@@ -239,8 +239,7 @@ - (void) testTrustCHeckWithRootCerts {
     NSError* error;
     NSURLCredential* credential;
 
-    
-    CBLTrustCheck* trustCheck = [[CBLTrustCheck alloc] initWithTrust: trust host: _host port: 443];
+    CBLTrustCheck* trustCheck = [[CBLTrustCheck alloc] initWithTrust: trust];
     credential = [trustCheck checkTrust: &error];
     AssertNil(credential);
 
@@ -261,7 +260,7 @@ - (void) testTrustCHeckWithRootCerts {
     status = SecTrustCreateWithCertificates((__bridge CFTypeRef)certs, policy, &trust);
     AssertEqual(status, errSecSuccess);
 
-    trustCheck = [[CBLTrustCheck alloc] initWithTrust: trust host: _host port: 443];
+    trustCheck = [[CBLTrustCheck alloc] initWithTrust: trust];
     credential = [trustCheck checkTrust: &error];
     AssertNil(credential);
 

Swift/Tests/MultipeerReplicatorTest.swift

@@ -121,14 +121,18 @@ class MultipeerReplicatorTest: CBLTestCase {
         assert(identityCount <= kTestMaxIdentity)
         identityCount += 1
 
-        let name = identityNameForNumber(identityCount)
-        let attrs = [certAttrCommonName: name]
+        let label = identityNameForNumber(identityCount)
+        
+        let timestamp = UInt64(Date().timeIntervalSince1970 * 1000)
+        let cn = "\(label)-\(timestamp)"
+        let attrs = [certAttrCommonName: cn]
+        
         return try TLSIdentity.createIdentity(
             for: kTestKeyUsages,
             attributes: attrs,
             expiration: nil,
             issuer: self.issuer,
-            label: name)
+            label: label)
     }
 
     typealias CollectionConfigBlock = (inout MultipeerCollectionConfiguration) -> Void