Skip to content

Commit 2d324c5

Browse files
authored
CBL-7351: Return error for AnchorTrusted in CBLCheckTrust (#3434)
* MultipeerReplicatorTest.testAuthenticateWithRootCertsFailed failed when running on iOS Device. Not sure why I didn’t catch this bug before. The test passed when running on macOS. * The cause of the issue is in CBLTrustCheck that doesn’t return an error when AnchorTrusted problem is found — it doesn’t return NO. When calling CBLTrustCheck on macOS, it failed when validating trust but for iOS, the validation error is a recoverable error so it needs additional checks. * In addition, refactor CBLTrustCheck logic to only skip host check when host is not specified. * Add some comment in CBLWebSocket and simplfy the code when calling trust check. * Update TrustCheckTest and clean up MultipeerReplicatorTest. * Note: Perform manual replication test with app service to ensure that any changes in CBLTrustCheck and CBLWebSocket are still working with the normal wss:// server.
1 parent 5d13587 commit 2d324c5

File tree

4 files changed

+51
-48
lines changed

4 files changed

+51
-48
lines changed

Objective-C/Internal/Replicator/CBLTrustCheck.mm

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,11 @@ - (NSString*) description {
8080
return [NSString stringWithFormat: @"%@[%@:%@]", self.class, (_host ?: @""), port];
8181
}
8282

83-
// Checks whether the reported problems with a SecTrust are OK.
84-
// Currently do not accept any problems.
83+
/**
84+
Validates whether the issues reported by SecTrust are acceptable if trust validation
85+
result is not kSecTrustResultProceed or kSecTrustResultUnspecified.
86+
Note: Used only when no pinned certificate or self-signed certificate restriction is set.
87+
*/
8588
- (BOOL) shouldAcceptProblems: (NSError**)outError {
8689
NSDictionary* resultDict = CFBridgingRelease(SecTrustCopyResult(_trust));
8790
NSArray* detailsArray = resultDict[@"TrustResultDetails"];
@@ -95,15 +98,21 @@ - (BOOL) shouldAcceptProblems: (NSError**)outError {
9598
[details[problem] isEqual: @[@(CSSMERR_APPLETP_HOSTNAME_MISMATCH)]])
9699
#endif
97100
) {
101+
if (!_host) {
102+
continue; // When no host specified, accept and check next problem
103+
}
98104
MYReturnError(outError, NSURLErrorServerCertificateUntrusted, NSURLErrorDomain,
99105
@"Server TLS certificate has a hostname mismatch.");
100-
} else if ([problem isEqualToString: @"AnchorTrusted"]) {
101-
MYReturnError(outError, NSURLErrorServerCertificateHasUnknownRoot, NSURLErrorDomain,
102-
@"Server TLS certificate has an unknown root or is self-signed.");
106+
return NO;
103107
} else {
104-
// Any other problem:
105-
MYReturnError(outError, NSURLErrorServerCertificateUntrusted, NSURLErrorDomain,
106-
@"Server TLS certificate is untrusted (problem = %@)", problem);
108+
if ([problem isEqualToString: @"AnchorTrusted"]) {
109+
MYReturnError(outError, NSURLErrorServerCertificateHasUnknownRoot, NSURLErrorDomain,
110+
@"Server TLS certificate has an unknown root or is self-signed.");
111+
} else {
112+
// Any other problem:
113+
MYReturnError(outError, NSURLErrorServerCertificateUntrusted, NSURLErrorDomain,
114+
@"Server TLS certificate is untrusted (problem = %@)", problem);
115+
}
107116
return NO;
108117
}
109118
}
@@ -174,38 +183,26 @@ - (nullable NSURLCredential*) checkTrust: (NSError**)outError {
174183

175184
// If using pinned cert, accept cert if it matches the pinned cert:
176185
if (_pinnedCertData) {
177-
CFIndex count = SecTrustGetCertificateCount(_trust);
178-
#if __IPHONE_OS_VERSION_MAX_REQUIRED >= 150000
179-
if (@available(iOS 15.0, *)) {
180-
NSData* certData = nil;
181-
for (CFIndex i = 0; i < count; i++) {
182-
CFArrayRef certs = SecTrustCopyCertificateChain(_trust);
183-
SecCertificateRef cert = (SecCertificateRef)CFArrayGetValueAtIndex(certs, i);
184-
certData = CFBridgingRelease(SecCertificateCopyData(cert));
186+
CFArrayRef certs = SecTrustCopyCertificateChain(_trust);
187+
CFIndex count = CFArrayGetCount(certs);
188+
for (CFIndex i = 0; i < count; i++) {
189+
SecCertificateRef cert = (SecCertificateRef)CFArrayGetValueAtIndex(certs, i);
190+
NSData* certData = CFBridgingRelease(SecCertificateCopyData(cert));
191+
if ([_pinnedCertData isEqual:certData]) {
185192
CFRelease(certs);
186-
if ([_pinnedCertData isEqual: certData]) {
187-
[self forceTrusted];
188-
return credential;
189-
}
190-
}
191-
} else
192-
#endif
193-
{
194-
for (CFIndex i = 0; i < count; i++) {
195-
#pragma clang diagnostic push
196-
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
197-
SecCertificateRef cert = SecTrustGetCertificateAtIndex(_trust, i);
198-
#pragma clang diagnostic pop
199-
if ([_pinnedCertData isEqual: CFBridgingRelease(SecCertificateCopyData(cert))]) {
200-
[self forceTrusted];
201-
return credential;
202-
}
193+
[self forceTrusted];
194+
return credential;
203195
}
204196
}
205-
MYReturnError(outError, NSURLErrorServerCertificateHasUnknownRoot, NSURLErrorDomain,
197+
198+
CFRelease(certs);
199+
MYReturnError(outError,
200+
NSURLErrorServerCertificateHasUnknownRoot,
201+
NSURLErrorDomain,
206202
@"Server TLS Certificate does not match the pinned cert.");
207203
return nil;
208204
}
205+
209206
#ifdef COUCHBASE_ENTERPRISE
210207
else if (_acceptOnlySelfSignedCert) {
211208
NSError* err;

Objective-C/Internal/Replicator/CBLWebSocket.mm

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -965,7 +965,10 @@ - (SecTrustRef) getTrustFromReadStream {
965965
kCFStreamPropertySSLPeerTrust);
966966
}
967967

968-
// Notify the received cert and verify the cert if using custom verification logic
968+
// Handles SSL certificate validation for the peer.
969+
// Performs custom trust evaluation if enabled, notifies certificate to
970+
// CBLWebSocketContext's callback and C4Socket and closes the connection on failure.
971+
// Returns YES if the certificate is accepted.
969972
- (BOOL) checkSSLCert {
970973
_shouldCheckSSLCert = NO;
971974

@@ -974,10 +977,16 @@ - (BOOL) checkSSLCert {
974977

975978
SecCertificateRef cert = [self certificateFromTrust: trust];
976979
[self notifyServerCertificateReceived: cert];
977-
980+
978981
NSError* error = nil;
982+
983+
// Validate trust only when using custom certificate validation
984+
// (kCFStreamSSLValidatesCertificateChain is disabled).
985+
// If system validation is enabled, the certificates have already been verified,
986+
// and any validation failure will trigger NSStreamEventErrorOccurred without
987+
// calling this method.
979988
if ([self usesCustomTLSCertValidation]) {
980-
if (![self validateTrust:trust error:&error]) {
989+
if (![self validateTrust: trust error: &error]) {
981990
[self closeWithError: error];
982991
return NO;
983992
}
@@ -1010,18 +1019,17 @@ - (BOOL) validateTrust: (SecTrustRef)trust error: (NSError**)error {
10101019
NSURL* url = _logic.URL;
10111020
CBLTrustCheck* check = [[CBLTrustCheck alloc] initWithTrust: trust host: url.host port: url.port.shortValue];
10121021

1013-
NSURLCredential* credentials = nil;
10141022
Value pinnedCert = _options[kC4ReplicatorOptionPinnedServerCert];
10151023
if (pinnedCert) {
10161024
check.pinnedCertData = slice(pinnedCert.asData()).copiedNSData();
1017-
credentials = [check checkTrust: error];
10181025
}
10191026
#ifdef COUCHBASE_ENTERPRISE
10201027
else if (_options[kC4ReplicatorOptionOnlySelfSignedServerCert].asBool()) {
10211028
check.acceptOnlySelfSignedCert = YES;
1022-
credentials = [check checkTrust: error];
10231029
}
10241030
#endif
1031+
1032+
NSURLCredential* credentials = [check checkTrust: error];
10251033
if (!credentials) {
10261034
CBLWarn(WebSocket, @"%@: TLS handshake failed: certificate verification error: %@", self, (*error).localizedDescription);
10271035
return false;

Objective-C/Tests/MultipeerReplicatorTest.m

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,13 @@ - (void) tearDown {
8585

8686
// TODO: Too flaky, enable after all LiteCore issues have been handled.
8787
- (BOOL) isExecutionAllowed {
88-
#if TARGET_OS_SIMULATOR
88+
#if TARGET_OS_SIMULATOR || TARGET_OS_MACCATALYST || TARGET_OS_OSX
8989
// Cannot be tested on the simulator as local network access permission is required.
9090
// See FAQ-12 : https://developer.apple.com/forums/thread/663858)
9191
return NO;
9292
#else
9393
// Disable as cannot be run on the build VM (Got POSIX Error 50, Network is down)
94-
return self.keyChainAccessAllowed && false;
94+
return self.keyChainAccessAllowed;
9595
#endif
9696
}
9797

@@ -408,6 +408,7 @@ - (void) testConfiguration {
408408
4. Check that the configuration cannot be created as the collections are empty.
409409
*/
410410
- (void) testConfigurationValidation {
411+
// When exception is thrown, the object will not get released:
411412
self.disableObjectLeakCheck = YES;
412413

413414
CBLCollection* collection = [self.db defaultCollection: nil];
@@ -1093,8 +1094,6 @@ - (void) runConflictResolverTestWithResolver: (nullable MultipeerConflictResolve
10931094
[self waitForReplicatorStatus: repl1 peerID: repl2.peerID activityLevel: kCBLReplicatorIdle];
10941095
[self waitForReplicatorStatus: repl2 peerID: repl1.peerID activityLevel: kCBLReplicatorIdle];
10951096

1096-
NSLog(@"-----------------------------------------");
1097-
10981097
// Stops:
10991098
[self stopMultipeerReplicator: repl1];
11001099
[self stopMultipeerReplicator: repl2];

Objective-C/Tests/TrustCheckTest.m

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ - (void) testTrustCHeckWithRootCerts {
228228
SecCertificateRef secCert3 = [self createSecCertFromPEM: cert3];
229229

230230
SecTrustRef trust;
231-
SecPolicyRef policy = SecPolicyCreateSSL(true, (__bridge CFStringRef)_host);
231+
SecPolicyRef policy = SecPolicyCreateBasicX509();
232232

233233
// Validate leaf cert:
234234

@@ -239,8 +239,7 @@ - (void) testTrustCHeckWithRootCerts {
239239
NSError* error;
240240
NSURLCredential* credential;
241241

242-
243-
CBLTrustCheck* trustCheck = [[CBLTrustCheck alloc] initWithTrust: trust host: _host port: 443];
242+
CBLTrustCheck* trustCheck = [[CBLTrustCheck alloc] initWithTrust: trust];
244243
credential = [trustCheck checkTrust: &error];
245244
AssertNil(credential);
246245

@@ -261,7 +260,7 @@ - (void) testTrustCHeckWithRootCerts {
261260
status = SecTrustCreateWithCertificates((__bridge CFTypeRef)certs, policy, &trust);
262261
AssertEqual(status, errSecSuccess);
263262

264-
trustCheck = [[CBLTrustCheck alloc] initWithTrust: trust host: _host port: 443];
263+
trustCheck = [[CBLTrustCheck alloc] initWithTrust: trust];
265264
credential = [trustCheck checkTrust: &error];
266265
AssertNil(credential);
267266

0 commit comments

Comments
 (0)