Fix Catalyst SecTrust API Deprecation Warning (#2977)

* Refactor some trust evaluation for Catalyst and deprecations.

* Remove logging.

* Make tests reliable on all platforms.

* Make test more reliable.

* Documentation cleanup.

Source/AFError.swift

@@ -154,6 +154,8 @@ public enum AFError: Error {
         case settingAnchorCertificatesFailed(status: OSStatus, certificates: [SecCertificate])
         /// During evaluation, creation of the revocation policy failed.
         case revocationPolicyCreationFailed
+        /// `SecTrust` evaluation failed with the associated `Error`, if one was produced.
+        case trustEvaluationFailed(error: Error?)
         /// Default evaluation failed with the associated `Output`.
         case defaultEvaluationFailed(output: Output)
         /// Host validation failed with the associated `Output`.
@@ -597,6 +599,7 @@ extension AFError.ServerTrustFailureReason {
              .policyApplicationFailed,
              .settingAnchorCertificatesFailed,
              .revocationPolicyCreationFailed,
+             .trustEvaluationFailed,
              .certificatePinningFailed,
              .publicKeyPinningFailed,
              .customEvaluationFailed:
@@ -608,6 +611,8 @@ extension AFError.ServerTrustFailureReason {
         switch self {
         case let .customEvaluationFailed(error):
             return error
+        case let .trustEvaluationFailed(error):
+            return error
         case .noRequiredEvaluator,
              .noCertificatesFound,
              .noPublicKeysFound,
@@ -657,8 +662,8 @@ extension AFError: LocalizedError {
             """
         case let .sessionInvalidated(error):
             return "Session was invalidated with error: \(error?.localizedDescription ?? "No description.")"
-        case .serverTrustEvaluationFailed:
-            return "Server trust evaluation failed."
+        case let .serverTrustEvaluationFailed(reason):
+            return "Server trust evaluation failed due to reason: \(reason.localizedDescription)"
         case let .urlRequestValidationFailed(reason):
             return "URLRequest validation failed due to reason: \(reason.localizedDescription)"
         case let .createUploadableFailed(error):
@@ -804,6 +809,8 @@ extension AFError.ServerTrustFailureReason {
             return "Attempting to set the provided certificates as anchor certificates failed."
         case .revocationPolicyCreationFailed:
             return "Attempting to create a revocation policy failed."
+        case let .trustEvaluationFailed(error):
+            return "SecTrust evaluation failed with error: \(error?.localizedDescription ?? "None")"
         case let .defaultEvaluationFailed(output):
             return "Default evaluation failed for host \(output.host)."
         case let .hostValidationFailed(output):

Source/ServerTrustEvaluation.swift

@@ -178,8 +178,12 @@ public final class RevocationTrustEvaluator: ServerTrustEvaluating {
             try trust.af.performValidation(forHost: host)
         }
 
-        try trust.af.validate(policy: SecPolicy.af.revocation(options: options)) { status, result in
-            AFError.serverTrustEvaluationFailed(reason: .revocationCheckFailed(output: .init(host, trust, status, result), options: options))
+        if #available(iOS 12, macOS 10.14, tvOS 12, watchOS 5, *) {
+            try trust.af.evaluate(afterApplying: SecPolicy.af.revocation(options: options))
+        } else {
+            try trust.af.validate(policy: SecPolicy.af.revocation(options: options)) { status, result in
+                AFError.serverTrustEvaluationFailed(reason: .revocationCheckFailed(output: .init(host, trust, status, result), options: options))
+            }
         }
     }
 }
@@ -389,12 +393,26 @@ public extension AlamofireExtension where ExtendedType: Bundle {
 
 extension SecTrust: AlamofireExtended {}
 public extension AlamofireExtension where ExtendedType == SecTrust {
-    /// Attempts to validate `self` using the policy provided and transforming any error produced using the closure passed.
+    /// Evaluates `self` after applying the `SecPolicy` value provided.
+    ///
+    /// - Parameter policy: The `SecPolicy` to apply to `self` before evaluation.
+    ///
+    /// - Throws:           Any `Error` from applying the `SecPolicy` or from evaluation.
+    @available(iOS 12, macOS 10.14, tvOS 12, watchOS 5, *)
+    func evaluate(afterApplying policy: SecPolicy) throws {
+        try apply(policy: policy).af.evaluate()
+    }
+
+    /// Attempts to validate `self` using the `SecPolicy` provided and transforming any error produced using the closure passed.
     ///
     /// - Parameters:
     ///   - policy:        The `SecPolicy` used to evaluate `self`.
     ///   - errorProducer: The closure used transform the failed `OSStatus` and `SecTrustResultType`.
-    /// - Throws:          Any error from applying the `policy`, or the result of `errorProducer` if validation fails.
+    /// - Throws:          Any `Error` from applying the `policy`, or the result of `errorProducer` if validation fails.
+    @available(iOS, introduced: 10, deprecated: 12, renamed: "evaluate(afterApplying:)")
+    @available(macOS, introduced: 10.12, deprecated: 10.14, renamed: "evaluate(afterApplying:)")
+    @available(tvOS, introduced: 10, deprecated: 12, renamed: "evaluate(afterApplying:)")
+    @available(watchOS, introduced: 3, deprecated: 5, renamed: "evaluate(afterApplying:)")
     func validate(policy: SecPolicy, errorProducer: (_ status: OSStatus, _ result: SecTrustResultType) -> Error) throws {
         try apply(policy: policy).af.validate(errorProducer: errorProducer)
     }
@@ -417,11 +435,29 @@ public extension AlamofireExtension where ExtendedType == SecTrust {
         return type
     }
 
+    /// Evaluate `self`, throwing an `Error` if evaluation fails.
+    ///
+    /// - Throws: `AFError.serverTrustEvaluationFailed` with reason `.trustValidationFailed` and associated error from
+    ///           the underlying evaluation.
+    @available(iOS 12, macOS 10.14, tvOS 12, watchOS 5, *)
+    func evaluate() throws {
+        var error: CFError?
+        let evaluationSucceeded = SecTrustEvaluateWithError(type, &error)
+
+        if !evaluationSucceeded {
+            throw AFError.serverTrustEvaluationFailed(reason: .trustEvaluationFailed(error: error))
+        }
+    }
+
     /// Validate `self`, passing any failure values through `errorProducer`.
     ///
     /// - Parameter errorProducer: The closure used to transform the failed `OSStatus` and `SecTrustResultType` into an
     ///                            `Error`.
     /// - Throws:                  The `Error` produced by the `errorProducer` closure.
+    @available(iOS, introduced: 10, deprecated: 12, renamed: "evaluate()")
+    @available(macOS, introduced: 10.12, deprecated: 10.14, renamed: "evaluate()")
+    @available(tvOS, introduced: 10, deprecated: 12, renamed: "evaluate()")
+    @available(watchOS, introduced: 3, deprecated: 5, renamed: "evaluate()")
     func validate(errorProducer: (_ status: OSStatus, _ result: SecTrustResultType) -> Error) throws {
         var result = SecTrustResultType.invalid
         let status = SecTrustEvaluate(type, &result)
@@ -473,8 +509,12 @@ public extension AlamofireExtension where ExtendedType == SecTrust {
     /// - Parameter host: The hostname, used only in the error output if validation fails.
     /// - Throws: An `AFError.serverTrustEvaluationFailed` instance with a `.defaultEvaluationFailed` reason.
     func performDefaultValidation(forHost host: String) throws {
-        try validate(policy: SecPolicy.af.default) { status, result in
-            AFError.serverTrustEvaluationFailed(reason: .defaultEvaluationFailed(output: .init(host, type, status, result)))
+        if #available(iOS 12, macOS 10.14, tvOS 12, watchOS 5, *) {
+            try evaluate(afterApplying: SecPolicy.af.default)
+        } else {
+            try validate(policy: SecPolicy.af.default) { status, result in
+                AFError.serverTrustEvaluationFailed(reason: .defaultEvaluationFailed(output: .init(host, type, status, result)))
+            }
         }
     }
 
@@ -484,8 +524,12 @@ public extension AlamofireExtension where ExtendedType == SecTrust {
     /// - Parameter host: The hostname to use in the validation.
     /// - Throws:         An `AFError.serverTrustEvaluationFailed` instance with a `.defaultEvaluationFailed` reason.
     func performValidation(forHost host: String) throws {
-        try validate(policy: SecPolicy.af.hostname(host)) { status, result in
-            AFError.serverTrustEvaluationFailed(reason: .hostValidationFailed(output: .init(host, type, status, result)))
+        if #available(iOS 12, macOS 10.14, tvOS 12, watchOS 5, *) {
+            try evaluate(afterApplying: SecPolicy.af.hostname(host))
+        } else {
+            try validate(policy: SecPolicy.af.hostname(host)) { status, result in
+                AFError.serverTrustEvaluationFailed(reason: .hostValidationFailed(output: .init(host, type, status, result)))
+            }
         }
     }
 }

Tests/AFError+AlamofireTests.swift

@@ -355,6 +355,11 @@ extension AFError.ServerTrustFailureReason {
         return false
     }
 
+    var isTrustEvaluationFailed: Bool {
+        if case .trustEvaluationFailed = self { return true }
+        return false
+    }
+
     var isDefaultEvaluationFailed: Bool {
         if case .defaultEvaluationFailed = self { return true }
         return false

Tests/SessionTests.swift

@@ -1484,25 +1484,38 @@ final class SessionCancellationTestCase: BaseTestCase {
     func testThatAutomaticallyResumedRequestsCanBeMassCancelled() {
         // Given
         let count = 100
-        let session = Session()
-        var responses: [DataResponse<Data?, AFError>] = []
         let completion = expectation(description: "all requests should finish")
         completion.expectedFulfillmentCount = count
+        let createdTasks = expectation(description: "all tasks created")
+        createdTasks.expectedFulfillmentCount = count
+        let gatheredMetrics = expectation(description: "metrics gathered for all tasks")
+        gatheredMetrics.expectedFulfillmentCount = count
         let cancellation = expectation(description: "cancel all requests should be called")
+        let monitor = ClosureEventMonitor()
+        monitor.requestDidCreateTask = { _, _ in createdTasks.fulfill() }
+        monitor.requestDidGatherMetrics = { _, _ in gatheredMetrics.fulfill() }
+        let session = Session(eventMonitors: [monitor])
+        let request = URLRequest.makeHTTPBinRequest(path: "delay/1")
+        var requests: [DataRequest] = []
+        var responses: [DataResponse<Data?, AFError>] = []
 
         // When
-        for _ in 1...count {
-            let request = URLRequest.makeHTTPBinRequest(path: "delay/1")
-            session.request(request).response { response in
+        requests = (0..<count).map { _ in session.request(request) }
+
+        wait(for: [createdTasks], timeout: timeout)
+
+        requests.forEach { request in
+            request.response { response in
                 responses.append(response)
                 completion.fulfill()
             }
         }
+
         session.cancelAllRequests {
             cancellation.fulfill()
         }
 
-        waitForExpectations(timeout: timeout)
+        wait(for: [gatheredMetrics, cancellation, completion], timeout: timeout)
 
         // Then
         XCTAssertTrue(responses.allSatisfy { $0.error?.isExplicitlyCancelledError == true })
@@ -1515,25 +1528,35 @@ final class SessionCancellationTestCase: BaseTestCase {
     func testThatManuallyResumedRequestsCanBeMassCancelled() {
         // Given
         let count = 100
-        let session = Session(startRequestsImmediately: false)
-        let request = URLRequest.makeHTTPBinRequest(path: "delay/1")
-        var responses: [DataResponse<Data?, AFError>] = []
         let completion = expectation(description: "all requests should finish")
         completion.expectedFulfillmentCount = count
+        let createdTasks = expectation(description: "all tasks created")
+        createdTasks.expectedFulfillmentCount = count
+        let gatheredMetrics = expectation(description: "metrics gathered for all tasks")
+        gatheredMetrics.expectedFulfillmentCount = count
         let cancellation = expectation(description: "cancel all requests should be called")
+        let monitor = ClosureEventMonitor()
+        monitor.requestDidCreateTask = { _, _ in createdTasks.fulfill() }
+        monitor.requestDidGatherMetrics = { _, _ in gatheredMetrics.fulfill() }
+        let session = Session(startRequestsImmediately: false, eventMonitors: [monitor])
+        let request = URLRequest.makeHTTPBinRequest(path: "delay/1")
+        var responses: [DataResponse<Data?, AFError>] = []
 
         // When
-        for _ in 1...count {
+        for _ in 0..<count {
             session.request(request).response { response in
                 responses.append(response)
                 completion.fulfill()
             }
         }
+
+        wait(for: [createdTasks], timeout: timeout)
+
         session.cancelAllRequests {
             cancellation.fulfill()
         }
 
-        waitForExpectations(timeout: timeout)
+        wait(for: [gatheredMetrics, cancellation, completion], timeout: timeout)
 
         // Then
         XCTAssertTrue(responses.allSatisfy { $0.error?.isExplicitlyCancelledError == true })
@@ -1560,13 +1583,15 @@ final class SessionCancellationTestCase: BaseTestCase {
         let cancellation = expectation(description: "cancel all requests should be called")
         let createTask = expectation(description: "should create task twice")
         createTask.expectedFulfillmentCount = 2
+        var tasksCreated = 0
         monitor.requestDidCreateTask = { _, _ in
+            tasksCreated += 1
             createTask.fulfill()
-        }
-        // Cancel when retry starts.
-        monitor.requestIsRetrying = { _ in
-            session.cancelAllRequests {
-                cancellation.fulfill()
+            // Cancel after the second task is created to ensure proper lifetime events.
+            if tasksCreated == 2 {
+                session.cancelAllRequests {
+                    cancellation.fulfill()
+                }
             }
         }
 

Tests/TLSEvaluationTests.swift

@@ -146,7 +146,11 @@ final class TLSEvaluationExpiredLeafCertificateTestCase: BaseTestCase {
 
         XCTAssertEqual(error?.isServerTrustEvaluationError, true)
         if case let .serverTrustEvaluationFailed(reason)? = error {
-            XCTAssertTrue(reason.isHostValidationFailed, "should be .hostValidationFailed")
+            if #available(iOS 12, macOS 10.14, tvOS 12, watchOS 5, *) {
+                XCTAssertTrue(reason.isTrustEvaluationFailed, "should be .trustEvaluationFailed")
+            } else {
+                XCTAssertTrue(reason.isHostValidationFailed, "should be .hostValidationFailed")
+            }
         } else {
             XCTFail("error should be .serverTrustEvaluationFailed")
         }
@@ -212,7 +216,11 @@ final class TLSEvaluationExpiredLeafCertificateTestCase: BaseTestCase {
         XCTAssertEqual(error?.isServerTrustEvaluationError, true)
 
         if case let .serverTrustEvaluationFailed(reason)? = error {
-            XCTAssertTrue(reason.isDefaultEvaluationFailed, "should be .defaultEvaluationFailed")
+            if #available(iOS 12, macOS 10.14, tvOS 12, watchOS 5, *) {
+                XCTAssertTrue(reason.isTrustEvaluationFailed, "should be .trustEvaluationFailed")
+            } else {
+                XCTAssertTrue(reason.isDefaultEvaluationFailed, "should be .defaultEvaluationFailed")
+            }
         } else {
             XCTFail("error should be .serverTrustEvaluationFailed")
         }
@@ -244,9 +252,13 @@ final class TLSEvaluationExpiredLeafCertificateTestCase: BaseTestCase {
         XCTAssertEqual(error?.isServerTrustEvaluationError, true)
 
         if case let .serverTrustEvaluationFailed(reason)? = error {
-            // Test seems flaky and can result in either of these failures, perhaps due to the OS actually checking?
-            XCTAssertTrue(reason.isDefaultEvaluationFailed || reason.isRevocationCheckFailed,
-                          "should be .defaultEvaluationFailed or .revocationCheckFailed")
+            if #available(iOS 12, macOS 10.14, tvOS 12, watchOS 5, *) {
+                XCTAssertTrue(reason.isTrustEvaluationFailed, "should be .trustEvaluationFailed")
+            } else {
+                // Test seems flaky and can result in either of these failures, perhaps due to the OS actually checking?
+                XCTAssertTrue(reason.isDefaultEvaluationFailed || reason.isRevocationCheckFailed,
+                              "should be .defaultEvaluationFailed or .revocationCheckFailed")
+            }
         } else {
             XCTFail("error should be .serverTrustEvaluationFailed")
         }
@@ -279,7 +291,11 @@ final class TLSEvaluationExpiredLeafCertificateTestCase: BaseTestCase {
         XCTAssertEqual(error?.isServerTrustEvaluationError, true)
 
         if case let .serverTrustEvaluationFailed(reason)? = error {
-            XCTAssertTrue(reason.isDefaultEvaluationFailed, "should be .defaultEvaluationFailed")
+            if #available(iOS 12, macOS 10.14, tvOS 12, watchOS 5, *) {
+                XCTAssertTrue(reason.isTrustEvaluationFailed, "should be .trustEvaluationFailed")
+            } else {
+                XCTAssertTrue(reason.isDefaultEvaluationFailed, "should be .defaultEvaluationFailed")
+            }
         } else {
             XCTFail("error should be .serverTrustEvaluationFailed")
         }
@@ -314,7 +330,11 @@ final class TLSEvaluationExpiredLeafCertificateTestCase: BaseTestCase {
         XCTAssertEqual(error?.isServerTrustEvaluationError, true)
 
         if case let .serverTrustEvaluationFailed(reason)? = error {
-            XCTAssertTrue(reason.isDefaultEvaluationFailed, "should be .defaultEvaluationFailed")
+            if #available(iOS 12, macOS 10.14, tvOS 12, watchOS 5, *) {
+                XCTAssertTrue(reason.isTrustEvaluationFailed, "should be .trustEvaluationFailed")
+            } else {
+                XCTAssertTrue(reason.isDefaultEvaluationFailed, "should be .defaultEvaluationFailed")
+            }
         } else {
             XCTFail("error should be .serverTrustEvaluationFailed")
         }
@@ -423,7 +443,11 @@ final class TLSEvaluationExpiredLeafCertificateTestCase: BaseTestCase {
         XCTAssertEqual(error?.isServerTrustEvaluationError, true)
 
         if case let .serverTrustEvaluationFailed(reason)? = error {
-            XCTAssertTrue(reason.isDefaultEvaluationFailed, "should be .defaultEvaluationFailed")
+            if #available(iOS 12, macOS 10.14, tvOS 12, watchOS 5, *) {
+                XCTAssertTrue(reason.isTrustEvaluationFailed, "should be .trustEvaluationFailed")
+            } else {
+                XCTAssertTrue(reason.isDefaultEvaluationFailed, "should be .defaultEvaluationFailed")
+            }
         } else {
             XCTFail("error should be .serverTrustEvaluationFailed")
         }