Forráskód Böngészése

Fix ServerTrustError crash. (#4553)

* Fix serverTrust crash (#4542)

* Update readme, add stale bot, add issue template, add pull request template

* Update copy from Alamofire

* Fix crash on nil serverTrust

* Expose server trust error

* add test for error throws

* Fix nullability

* Fix evaluateServerTrust determination (#4552)

Co-authored-by: Jon Shier <jon@jonshier.com>

* Revert changes making error generation public.

This reverts commit 111352b32e77167834267be1792e44287a110ae3.

# Conflicts:
#	AFNetworking/AFURLSessionManager.m

* Make static error function a method instead.

* Make trust generatio part of AFTestCase.

* Add test cases for server trust error generation.

* Mark parameters explicitly nullable.

* Fix nullability (#4551)

* Replace instead of appending to the default header (#4550)

* Replace instead of appending to the default header

* Add tests for setting HTTP headers per request

* Fix SPM usage with better publicHeadersPath. (#4554)

Co-authored-by: Jakub Kašpar <kaspikk@gmail.com>
Co-authored-by: Elf Sundae <elf.sundae@gmail.com>
Jon Shier 5 éve
szülő
commit
34b6b1b0d9

+ 29 - 17
AFNetworking/AFURLSessionManager.m

@@ -168,21 +168,6 @@ typedef void (^AFURLSessionTaskCompletionHandler)(NSURLResponse *response, id re
 
 static const void * const AuthenticationChallengeErrorKey = &AuthenticationChallengeErrorKey;
 
-static NSError * ServerTrustError(SecTrustRef serverTrust, NSURL *url)
-{
-    NSBundle *CFNetworkBundle = [NSBundle bundleWithIdentifier:@"com.apple.CFNetwork"];
-    NSString *defaultValue = @"The certificate for this server is invalid. You might be connecting to a server that is pretending to be “%@” which could put your confidential information at risk.";
-    NSString *descriptionFormat = NSLocalizedStringWithDefaultValue(@"Err-1202.w", nil, CFNetworkBundle, defaultValue, @"") ?: defaultValue;
-    NSString *localizedDescription = [descriptionFormat componentsSeparatedByString:@"%@"].count <= 2 ? [NSString localizedStringWithFormat:descriptionFormat, url.host] : descriptionFormat;
-    NSDictionary *userInfo = @{
-        NSURLErrorFailingURLErrorKey: url,
-        NSURLErrorFailingURLStringErrorKey: url.absoluteString,
-        NSURLErrorFailingURLPeerTrustErrorKey: (__bridge id)serverTrust,
-        NSLocalizedDescriptionKey: localizedDescription
-    };
-    return [NSError errorWithDomain:NSURLErrorDomain code:NSURLErrorServerCertificateUntrusted userInfo:userInfo];
-}
-
 #pragma mark - NSURLSessionTaskDelegate
 
 - (void)URLSession:(__unused NSURLSession *)session
@@ -1001,7 +986,7 @@ didReceiveChallenge:(NSURLAuthenticationChallenge *)challenge
             @throw [NSException exceptionWithName:@"Invalid Return Value" reason:@"The return value from the authentication challenge handler must be nil, an NSError, an NSURLCredential or an NSNumber." userInfo:nil];
         }
     } else {
-        evaluateServerTrust = YES;
+        evaluateServerTrust = [challenge.protectionSpace.authenticationMethod isEqualToString:NSURLAuthenticationMethodServerTrust];
     }
 
     if (evaluateServerTrust) {
@@ -1009,7 +994,9 @@ didReceiveChallenge:(NSURLAuthenticationChallenge *)challenge
             disposition = NSURLSessionAuthChallengeUseCredential;
             credential = [NSURLCredential credentialForTrust:challenge.protectionSpace.serverTrust];
         } else {
-            objc_setAssociatedObject(task, AuthenticationChallengeErrorKey, ServerTrustError(challenge.protectionSpace.serverTrust, task.currentRequest.URL), OBJC_ASSOCIATION_RETAIN);
+            objc_setAssociatedObject(task, AuthenticationChallengeErrorKey,
+                                     [self serverTrustErrorForServerTrust:challenge.protectionSpace.serverTrust url:task.currentRequest.URL],
+                                     OBJC_ASSOCIATION_RETAIN);
             disposition = NSURLSessionAuthChallengeCancelAuthenticationChallenge;
         }
     }
@@ -1019,6 +1006,31 @@ didReceiveChallenge:(NSURLAuthenticationChallenge *)challenge
     }
 }
 
+- (nonnull NSError *)serverTrustErrorForServerTrust:(nullable SecTrustRef)serverTrust url:(nullable NSURL *)url
+{
+    NSBundle *CFNetworkBundle = [NSBundle bundleWithIdentifier:@"com.apple.CFNetwork"];
+    NSString *defaultValue = @"The certificate for this server is invalid. You might be connecting to a server that is pretending to be “%@” which could put your confidential information at risk.";
+    NSString *descriptionFormat = NSLocalizedStringWithDefaultValue(@"Err-1202.w", nil, CFNetworkBundle, defaultValue, @"") ?: defaultValue;
+    NSString *localizedDescription = [descriptionFormat componentsSeparatedByString:@"%@"].count <= 2 ? [NSString localizedStringWithFormat:descriptionFormat, url.host] : descriptionFormat;
+    NSMutableDictionary *userInfo = [@{
+        NSLocalizedDescriptionKey: localizedDescription
+    } mutableCopy];
+
+    if (serverTrust) {
+        userInfo[NSURLErrorFailingURLPeerTrustErrorKey] = (__bridge id)serverTrust;
+    }
+
+    if (url) {
+        userInfo[NSURLErrorFailingURLErrorKey] = url;
+
+        if (url.absoluteString) {
+            userInfo[NSURLErrorFailingURLStringErrorKey] = url.absoluteString;
+        }
+    }
+
+    return [NSError errorWithDomain:NSURLErrorDomain code:NSURLErrorServerCertificateUntrusted userInfo:userInfo];
+}
+
 - (void)URLSession:(NSURLSession *)session
               task:(NSURLSessionTask *)task
  needNewBodyStream:(void (^)(NSInputStream *bodyStream))completionHandler

+ 48 - 0
Tests/Tests/AFHTTPSessionManagerTests.m

@@ -28,6 +28,12 @@
 @property (readwrite, nonatomic, strong) AFHTTPSessionManager *sessionManager;
 @end
 
+@interface AFURLSessionManager (Testing)
+
+- (nonnull NSError *)serverTrustErrorForServerTrust:(SecTrustRef)serverTrust url:(NSURL *)url;
+
+@end
+
 @implementation AFHTTPSessionManagerTests
 
 - (void)setUp {
@@ -643,4 +649,46 @@
     [self waitForExpectationsWithCommonTimeoutUsingHandler:nil];
 }
 
+- (void)testThatServerTrustErrorIsCreatedWithProperUserInfoWithAllParameters {
+    NSURL *url = [NSURL URLWithString:@"https://httpbin.org/get"];
+    SecTrustRef trust = AFUTTrustChainForCertsInDirectory([[[NSBundle bundleForClass:[self class]] resourcePath]
+                                                           stringByAppendingPathComponent:@"HTTPBinOrgServerTrustChain"]);
+    NSError *error = [self.sessionManager serverTrustErrorForServerTrust:trust url:url];
+
+    XCTAssertNotNil(error);
+    XCTAssertNotNil(error.userInfo[NSURLErrorFailingURLPeerTrustErrorKey]);
+    XCTAssertEqual(error.userInfo[NSURLErrorFailingURLErrorKey], url);
+    XCTAssertEqual(error.userInfo[NSURLErrorFailingURLStringErrorKey], url.absoluteString);
+}
+
+- (void)testThatServerTrustErrorIsCreatedWithProperUserInfoWhenTrustIsNil {
+    NSURL *url = [NSURL URLWithString:@"https://httpbin.org/get"];
+    NSError *error = [self.sessionManager serverTrustErrorForServerTrust:nil url:url];
+
+    XCTAssertNotNil(error);
+    XCTAssertNil(error.userInfo[NSURLErrorFailingURLPeerTrustErrorKey]);
+    XCTAssertEqual(error.userInfo[NSURLErrorFailingURLErrorKey], url);
+    XCTAssertEqual(error.userInfo[NSURLErrorFailingURLStringErrorKey], url.absoluteString);
+}
+
+- (void)testThatServerTrustErrorIsCreatedWithProperUserInfoWhenURLIsNil {
+    SecTrustRef trust = AFUTTrustChainForCertsInDirectory([[[NSBundle bundleForClass:[self class]] resourcePath]
+                                                           stringByAppendingPathComponent:@"HTTPBinOrgServerTrustChain"]);
+    NSError *error = [self.sessionManager serverTrustErrorForServerTrust:trust url:nil];
+
+    XCTAssertNotNil(error);
+    XCTAssertNotNil(error.userInfo[NSURLErrorFailingURLPeerTrustErrorKey]);
+    XCTAssertNil(error.userInfo[NSURLErrorFailingURLErrorKey]);
+    XCTAssertNil(error.userInfo[NSURLErrorFailingURLStringErrorKey]);
+}
+
+- (void)testThatServerTrustErrorIsCreatedWithProperUserInfoAllParametersAreNil {
+    NSError *error = [self.sessionManager serverTrustErrorForServerTrust:nil url:nil];
+
+    XCTAssertNotNil(error);
+    XCTAssertNil(error.userInfo[NSURLErrorFailingURLPeerTrustErrorKey]);
+    XCTAssertNil(error.userInfo[NSURLErrorFailingURLErrorKey]);
+    XCTAssertNil(error.userInfo[NSURLErrorFailingURLStringErrorKey]);
+}
+
 @end

+ 0 - 17
Tests/Tests/AFSecurityPolicyTests.m

@@ -26,23 +26,6 @@
 
 @end
 
-static SecTrustRef AFUTTrustChainForCertsInDirectory(NSString *directoryPath) {
-    NSArray *certFileNames = [[NSFileManager defaultManager] contentsOfDirectoryAtPath:directoryPath error:nil];
-    NSMutableArray *certs  = [NSMutableArray arrayWithCapacity:[certFileNames count]];
-    for (NSString *path in certFileNames) {
-        NSData *certData = [NSData dataWithContentsOfFile:[directoryPath stringByAppendingPathComponent:path]];
-        SecCertificateRef cert = SecCertificateCreateWithData(NULL, (__bridge CFDataRef)(certData));
-        [certs addObject:(__bridge_transfer id)(cert)];
-    }
-
-    SecPolicyRef policy = SecPolicyCreateBasicX509();
-    SecTrustRef trust = NULL;
-    SecTrustCreateWithCertificates((__bridge CFTypeRef)(certs), policy, &trust);
-    CFRelease(policy);
-
-    return trust;
-}
-
 static SecTrustRef AFUTHTTPBinOrgServerTrust() {
     NSString *bundlePath = [[NSBundle bundleForClass:[AFSecurityPolicyTests class]] resourcePath];
     NSString *serverCertDirectoryPath = [bundlePath stringByAppendingPathComponent:@"HTTPBinOrgServerTrustChain"];

+ 2 - 0
Tests/Tests/AFTestCase.h

@@ -21,6 +21,8 @@
 
 #import <XCTest/XCTest.h>
 
+SecTrustRef AFUTTrustChainForCertsInDirectory(NSString *directoryPath);
+
 @interface AFTestCase : XCTestCase
 
 @property (nonatomic, strong, readonly) NSURL *baseURL;

+ 17 - 0
Tests/Tests/AFTestCase.m

@@ -21,6 +21,23 @@
 
 #import "AFTestCase.h"
 
+SecTrustRef AFUTTrustChainForCertsInDirectory(NSString *directoryPath) {
+    NSArray *certFileNames = [[NSFileManager defaultManager] contentsOfDirectoryAtPath:directoryPath error:nil];
+    NSMutableArray *certs  = [NSMutableArray arrayWithCapacity:[certFileNames count]];
+    for (NSString *path in certFileNames) {
+        NSData *certData = [NSData dataWithContentsOfFile:[directoryPath stringByAppendingPathComponent:path]];
+        SecCertificateRef cert = SecCertificateCreateWithData(NULL, (__bridge CFDataRef)(certData));
+        [certs addObject:(__bridge_transfer id)(cert)];
+    }
+    
+    SecPolicyRef policy = SecPolicyCreateBasicX509();
+    SecTrustRef trust = NULL;
+    SecTrustCreateWithCertificates((__bridge CFTypeRef)(certs), policy, &trust);
+    CFRelease(policy);
+    
+    return trust;
+}
+
 @implementation AFTestCase
 
 - (void)setUp {