Merge pull request #22343 from brave/bugfix/iOS-Certs-And-Secure-State

Author: soner-yuksel

ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+ShareActivity.swift

@@ -301,10 +301,13 @@ extension BrowserViewController {
     }
 
     // Display Certificate Activity
-    if let selectedTab = tabManager.selectedTab,
-      selectedTab.secureContentState != .missingSSL && selectedTab.secureContentState != .unknown
+    if let tabURL = tabManager.selectedTab?.webView?.url,
+      tabManager.selectedTab?.webView?.serverTrust != nil
+        || ErrorPageHelper.hasCertificates(for: tabURL)
     {
-      logSecureContentState(tab: selectedTab, details: "Display Certificate Activity Settings")
+      if let selectedTab = tabManager.selectedTab {
+        logSecureContentState(tab: selectedTab, details: "Display Certificate Activity Settings")
+      }
 
       activities.append(
         BasicMenuActivity(
@@ -316,7 +319,7 @@ extension BrowserViewController {
       )
     }
 
-    // Report Web-compat Issue Actibity
+    // Report Web-compat Issue Activity
     activities.append(
       BasicMenuActivity(
         title: Strings.Shields.reportABrokenSite,

ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+WKNavigationDelegate.swift

@@ -69,21 +69,6 @@ extension BrowserViewController: WKNavigationDelegate {
     if let selectedTab = tabManager.selectedTab,
       selectedTab.url?.origin != webView.url?.origin
     {
-      if let url = webView.url {
-        if !InternalURL.isValid(url: url) {
-          // reset secure content state to unknown until page can be evaluated
-          selectedTab.sslPinningError = nil
-          selectedTab.sslPinningTrust = nil
-          selectedTab.secureContentState = .unknown
-          logSecureContentState(
-            tab: selectedTab,
-            details:
-              "DidStartProvisionalNavigation - Reset secure content state to unknown until page can be evaluated"
-          )
-
-          updateToolbarSecureContentState(.unknown)
-        }
-      }
 
       // new site has a different origin, hide wallet icon.
       tabManager.selectedTab?.isWalletIconVisible = false
@@ -746,7 +731,9 @@ extension BrowserViewController: WKNavigationDelegate {
     download.delegate = self
   }
 
-  nonisolated public func webView(
+  @MainActor
+
+  public func webView(
     _ webView: WKWebView,
     respondTo challenge: URLAuthenticationChallenge
   ) async -> (URLSession.AuthChallengeDisposition, URLCredential?) {
@@ -756,11 +743,16 @@ extension BrowserViewController: WKNavigationDelegate {
     let host = challenge.protectionSpace.host
     let origin = "\(host):\(challenge.protectionSpace.port)"
     if challenge.protectionSpace.authenticationMethod == NSURLAuthenticationMethodServerTrust,
-      let trust = challenge.protectionSpace.serverTrust,
-      let cert = (SecTrustCopyCertificateChain(trust) as? [SecCertificate])?.first,
-      profile.certStore.containsCertificate(cert, forOrigin: origin)
+      let trust = challenge.protectionSpace.serverTrust
     {
-      return (.useCredential, URLCredential(trust: trust))
+
+      let cert = await Task<SecCertificate?, Never>.detached {
+        return (SecTrustCopyCertificateChain(trust) as? [SecCertificate])?.first
+      }.value
+
+      if let cert = cert, profile.certStore.containsCertificate(cert, forOrigin: origin) {
+        return (.useCredential, URLCredential(trust: trust))
+      }
     }
 
     // Certificate Pinning
@@ -782,13 +774,8 @@ extension BrowserViewController: WKNavigationDelegate {
         if result == Int32.min {
           // Cert is POTENTIALLY invalid and cannot be pinned
 
-          await MainActor.run {
-            // Handle the potential error later in `didFailProvisionalNavigation`
-            self.tab(for: webView)?.sslPinningTrust = serverTrust
-          }
-
           // Let WebKit handle the request and validate the cert
-          // This is the same as calling `BraveCertificateUtils.evaluateTrust`
+          // This is the same as calling `BraveCertificateUtils.evaluateTrust` but with more error info provided by WebKit
           return (.performDefaultHandling, nil)
         }
 
@@ -802,7 +789,7 @@ extension BrowserViewController: WKNavigationDelegate {
           userInfo: ["_kCFStreamErrorCodeKey": Int(errorCode)]
         )
 
-        let error = await NSError(
+        let error = NSError(
           domain: kCFErrorDomainCFNetwork as String,
           code: Int(errorCode),
           userInfo: [
@@ -812,10 +799,8 @@ extension BrowserViewController: WKNavigationDelegate {
           ]
         )
 
-        await MainActor.run {
-          // Handle the error later in `didFailProvisionalNavigation`
-          self.tab(for: webView)?.sslPinningError = error
-        }
+        // Handle the error later in `didFailProvisionalNavigation`
+        self.tab(for: webView)?.sslPinningError = error
 
         return (.cancelAuthenticationChallenge, nil)
       }
@@ -825,39 +810,38 @@ extension BrowserViewController: WKNavigationDelegate {
     let protectionSpace = challenge.protectionSpace
     let credential = challenge.proposedCredential
     let previousFailureCount = challenge.previousFailureCount
-    return await Task { @MainActor in
-      guard
-        protectionSpace.authenticationMethod == NSURLAuthenticationMethodHTTPBasic
-          || protectionSpace.authenticationMethod == NSURLAuthenticationMethodHTTPDigest
-          || protectionSpace.authenticationMethod == NSURLAuthenticationMethodNTLM,
-        let tab = tab(for: webView)
-      else {
-        return (.performDefaultHandling, nil)
-      }
 
-      // The challenge may come from a background tab, so ensure it's the one visible.
-      tabManager.selectTab(tab)
+    guard
+      protectionSpace.authenticationMethod == NSURLAuthenticationMethodHTTPBasic
+        || protectionSpace.authenticationMethod == NSURLAuthenticationMethodHTTPDigest
+        || protectionSpace.authenticationMethod == NSURLAuthenticationMethodNTLM,
+      let tab = tab(for: webView)
+    else {
+      return (.performDefaultHandling, nil)
+    }
 
-      do {
-        let credentials = try await Authenticator.handleAuthRequest(
-          self,
-          credential: credential,
-          protectionSpace: protectionSpace,
-          previousFailureCount: previousFailureCount
-        )
+    // The challenge may come from a background tab, so ensure it's the one visible.
+    tabManager.selectTab(tab)
 
-        if BasicAuthCredentialsManager.validDomains.contains(host) {
-          BasicAuthCredentialsManager.setCredential(
-            origin: origin,
-            credential: credentials.credentials
-          )
-        }
+    do {
+      let credentials = try await Authenticator.handleAuthRequest(
+        self,
+        credential: credential,
+        protectionSpace: protectionSpace,
+        previousFailureCount: previousFailureCount
+      )
 
-        return (.useCredential, credentials.credentials)
-      } catch {
-        return (.rejectProtectionSpace, nil)
+      if BasicAuthCredentialsManager.validDomains.contains(host) {
+        BasicAuthCredentialsManager.setCredential(
+          origin: origin,
+          credential: credentials.credentials
+        )
       }
-    }.value
+
+      return (.useCredential, credentials.credentials)
+    } catch {
+      return (.rejectProtectionSpace, nil)
+    }
   }
 
   public func webView(_ webView: WKWebView, didCommit navigation: WKNavigation!) {
@@ -991,20 +975,6 @@ extension BrowserViewController: WKNavigationDelegate {
   ) {
     guard let tab = tab(for: webView) else { return }
 
-    // WebKit does not update certs on cancellation of a frame load
-    // So manually trigger the notification with the current cert
-    // Also, when Chromium cert validation passes, BUT Apple cert validation fails, the request is cancelled automatically by WebKit
-    // In such a case, the webView.serverTrust is `nil`. The only time we have a valid trust is when we received the challenge
-    // so we need to update the URL-Bar to show that serverTrust when WebKit's is nil.
-    logSecureContentState(tab: tab, details: "ObserveValue trigger in didFailProvisionalNavigation")
-
-    observeValue(
-      forKeyPath: KVOConstants.serverTrust.keyPath,
-      of: webView,
-      change: [.newKey: webView.serverTrust ?? tab.sslPinningTrust as Any, .kindKey: 1],
-      context: nil
-    )
-
     // Ignore the "Frame load interrupted" error that is triggered when we cancel a request
     // to open an external application and hand it over to UIApplication.openURL(). The result
     // will be that we switch to the external app, for example the app store, while keeping the
@@ -1036,23 +1006,10 @@ extension BrowserViewController: WKNavigationDelegate {
 
     if let url = error.userInfo[NSURLErrorFailingURLErrorKey] as? URL {
 
-      // The certificate came from the WebKit SSL Handshake validation and the cert is untrusted
-      if webView.serverTrust == nil, let serverTrust = tab.sslPinningTrust,
-        error.userInfo["NSErrorPeerCertificateChainKey"] == nil
-      {
-        // Build a cert chain error to display in the cert viewer in such cases, as we aren't given one by WebKit
-        var userInfo = error.userInfo
-        userInfo["NSErrorPeerCertificateChainKey"] =
-          SecTrustCopyCertificateChain(serverTrust) as? [SecCertificate] ?? []
-        userInfo["NSErrorPeerUntrustedByApple"] = true
-        error = NSError(domain: error.domain, code: error.code, userInfo: userInfo)
+      if tab == self.tabManager.selectedTab {
+        self.topToolbar.hideProgressBar()
       }
 
-      ErrorPageHelper(certStore: profile.certStore).loadPage(error, forUrl: url, inWebView: webView)
-      // Submitting same errornous URL using toolbar will cause progress bar get stuck
-      // Reseting the progress bar in case there is an error is necessary
-      topToolbar.hideProgressBar()
-
       // If the local web server isn't working for some reason (Brave cellular data is
       // disabled in settings, for example), we'll fail to load the session restore URL.
       // We rely on loading that page to get the restore callback to reset the restoring
@@ -1118,16 +1075,7 @@ extension BrowserViewController {
     // External dialog should not be shown for non-active tabs #6687 - #7835
     let isVisibleTab = tab?.isTabVisible() == true
 
-    // Check user trying to open on NTP like external link browsing
-    var isAboutHome = false
-    if let url = tab?.url {
-      isAboutHome = InternalURL(url)?.isAboutHomeURL == true
-    }
-
-    // Finally check non-active tab
-    let isNonActiveTab = isAboutHome ? false : tab?.url?.host != topToolbar.currentURL?.host
-
-    if !isVisibleTab || isNonActiveTab {
+    if !isVisibleTab {
       return false
     }
 

ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BrowserViewController.swift

@@ -2071,8 +2071,10 @@ public class BrowserViewController: UIViewController {
       tab.secureContentState = .unknown
       logSecureContentState(tab: tab, path: path)
 
-      guard let serverTrust = tab.webView?.serverTrust else {
-        if let url = tab.webView?.url ?? tab.url {
+      guard let url = webView.url,
+        let serverTrust = webView.serverTrust
+      else {
+        if let url = webView.url {
           if InternalURL.isValid(url: url),
             let internalUrl = InternalURL(url),
             internalUrl.isAboutURL || internalUrl.isAboutHomeURL
@@ -2166,8 +2168,8 @@ public class BrowserViewController: UIViewController {
         break
       }
 
-      guard let scheme = tab.webView?.url?.scheme,
-        let host = tab.webView?.url?.host
+      guard let scheme = url.scheme,
+        let host = url.host
       else {
         tab.secureContentState = .unknown
         logSecureContentState(tab: tab, path: path, details: "No webview URL host scheme)")
@@ -2177,7 +2179,7 @@ public class BrowserViewController: UIViewController {
       }
 
       let port: Int
-      if let urlPort = tab.webView?.url?.port {
+      if let urlPort = url.port {
         port = urlPort
       } else if scheme == "https" {
         port = 443
@@ -2315,15 +2317,8 @@ public class BrowserViewController: UIViewController {
     browser.tabManager.addTabsForURLs([url], zombie: false, isPrivate: isPrivate)
   }
 
-  public func switchToTabForURLOrOpen(
-    _ url: URL,
-    isPrivate: Bool = false,
-    isPrivileged: Bool,
-    isExternal: Bool = false
-  ) {
-    if !isExternal {
-      popToBVC()
-    }
+  public func switchToTabForURLOrOpen(_ url: URL, isPrivate: Bool = false, isPrivileged: Bool) {
+    popToBVC(isAnimated: false)
 
     if let tab = tabManager.getTabForURL(url, isPrivate: isPrivate) {
       tabManager.selectTab(tab)
@@ -2461,11 +2456,11 @@ public class BrowserViewController: UIViewController {
     present(settingsNavigationController, animated: true)
   }
 
-  func popToBVC(completion: (() -> Void)? = nil) {
+  func popToBVC(isAnimated: Bool = true, completion: (() -> Void)? = nil) {
     guard let currentViewController = navigationController?.topViewController else {
       return
     }
-    currentViewController.dismiss(animated: true, completion: completion)
+    currentViewController.dismiss(animated: isAnimated, completion: completion)
 
     if currentViewController != self {
       _ = self.navigationController?.popViewController(animated: true)

ios/brave-ios/Sources/Brave/Frontend/Browser/Helpers/ErrorPageHelper.swift

@@ -104,11 +104,6 @@ class ErrorPageHelper {
       URLQueryItem(name: "timestamp", value: "\(Int(Date().timeIntervalSince1970 * 1000))"),
     ]
 
-    // The error came from WebKit's internal validation and the cert is untrusted
-    if error.userInfo["NSErrorPeerUntrustedByApple"] as? Bool == true {
-      queryItems.append(URLQueryItem(name: "peeruntrusted", value: "true"))
-    }
-
     // If this is an invalid certificate, show a certificate error allowing the
     // user to go back or continue. The certificate itself is encoded and added as
     // a query parameter to the error page URL; we then read the certificate from
@@ -186,6 +181,10 @@ extension ErrorPageHelper {
     return 0
   }
 
+  static func hasCertificates(for url: URL) -> Bool {
+    return (url as NSURL).valueForQueryParameter(key: "badcerts") != nil
+  }
+
   static func serverTrust(from errorURL: URL) throws -> SecTrust? {
     guard let internalUrl = InternalURL(errorURL),
       internalUrl.isErrorPage,

ios/brave-ios/Sources/Brave/Frontend/Browser/Interstitial Pages/CertificateErrorPageHandler.swift

@@ -17,9 +17,7 @@ class CertificateErrorPageHandler: InterstitialPageHandler {
   }
 
   func response(for model: ErrorPageModel) -> (URLResponse, Data)? {
-    let hasCertificate =
-      model.components.valueForQuery("certerror") != nil
-      && model.components.valueForQuery("peeruntrusted") == nil
+    let hasCertificate = model.components.valueForQuery("certerror") != nil
 
     guard let asset = Bundle.module.path(forResource: "CertificateError", ofType: "html") else {
       assert(false)

ios/brave-ios/Sources/Brave/Frontend/Browser/NavigationRouter.swift

@@ -91,13 +91,7 @@ public enum NavigationPath: Equatable {
 
   private static func handleURL(url: URL?, isPrivate: Bool, with bvc: BrowserViewController) {
     if let newURL = url {
-      bvc.switchToTabForURLOrOpen(
-        newURL,
-        isPrivate: isPrivate,
-        isPrivileged: false,
-        isExternal: true
-      )
-      bvc.popToBVC()
+      bvc.switchToTabForURLOrOpen(newURL, isPrivate: isPrivate, isPrivileged: false)
     } else {
       bvc.openBlankNewTab(attemptLocationFieldFocus: false, isPrivate: isPrivate)
     }

ios/brave-ios/Sources/Brave/Frontend/Browser/Tab.swift

@@ -103,7 +103,6 @@ class Tab: NSObject {
 
   var secureContentState: TabSecureContentState = .unknown
   var sslPinningError: Error?
-  var sslPinningTrust: SecTrust?
 
   private let _syncTab: BraveSyncTab?
   private let _faviconDriver: FaviconDriver?

ios/brave-ios/Sources/CertificateUtilities/BraveCertificateUtils.swift

@@ -220,6 +220,7 @@ extension BraveCertificateUtils {
     return serverTrust!
   }
 
+  /// Verifies ServerTrust using Apple's APIs which validates also the X509 Certificate against the System Trusts
   public static func evaluateTrust(_ trust: SecTrust, for host: String?) async throws {
     try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Void, Error>) in
       BraveCertificateUtils.evaluationQueue.async {
@@ -241,6 +242,7 @@ extension BraveCertificateUtils {
     }
   }
 
+  /// Verifies ServerTrust using Brave-Core which verifies only SSL Pinning Status
   public static func verifyTrust(_ trust: SecTrust, host: String, port: Int) async -> Int {
     return Int(BraveCertificateUtility.verifyTrust(trust, host: host, port: port))
   }