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

@@ -268,7 +268,10 @@ extension BrowserViewController {
     }
 
     // Display Certificate Activity
-    if let secureState = tabManager.selectedTab?.secureContentState, secureState != .missingSSL && secureState != .unknown {
+    if let tabURL = tabManager.selectedTab?.webView?.url,
+      tabManager.selectedTab?.webView?.serverTrust != nil
+        || ErrorPageHelper.hasCertificates(for: tabURL)
+    {
       activities.append(
         BasicMenuActivity(
           title: Strings.displayCertificate,
@@ -278,7 +281,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

@@ -66,17 +66,9 @@ extension BrowserViewController: WKNavigationDelegate {
 
     // check if web view is loading a different origin than the one currently loaded
     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
-          updateToolbarSecureContentState(.unknown)
-        }
-      }
-      
+      selectedTab.url?.origin != webView.url?.origin
+    {
+
       // new site has a different origin, hide wallet icon.
       tabManager.selectedTab?.isWalletIconVisible = false
       // new site, reset connected addresses
@@ -606,16 +598,28 @@ extension BrowserViewController: WKNavigationDelegate {
     download.delegate = self
   }
 
-  nonisolated public func webView(_ webView: WKWebView, respondTo challenge: URLAuthenticationChallenge) async -> (URLSession.AuthChallengeDisposition, URLCredential?) {
+
+  @MainActor
+
+  public func webView(
+    _ webView: WKWebView,
+    respondTo challenge: URLAuthenticationChallenge
+  ) async -> (URLSession.AuthChallengeDisposition, URLCredential?) {
 
     // If this is a certificate challenge, see if the certificate has previously been
     // accepted by the user.
     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) {
-      return (.useCredential, URLCredential(trust: trust))
+      let trust = challenge.protectionSpace.serverTrust
+    {
+      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
@@ -636,36 +640,35 @@ extension BrowserViewController: WKNavigationDelegate {
         // Let the system handle it and we'll show an error if the system cannot validate it
         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)
         }
 
         // Cert is invalid and cannot be pinned
         Logger.module.error("CERTIFICATE_INVALID")
         let errorCode = CFNetworkErrors.braveCertificatePinningFailed.rawValue
-        
-        let underlyingError = NSError(domain: kCFErrorDomainCFNetwork as String,
-                                      code: Int(errorCode),
-                                      userInfo: ["_kCFStreamErrorCodeKey": Int(errorCode)])
-        
-        let error = await NSError(domain: kCFErrorDomainCFNetwork as String,
-                                  code: Int(errorCode),
-                                  userInfo: [NSURLErrorFailingURLErrorKey: webView.url as Any,
-                                             "NSErrorPeerCertificateChainKey": certificateChain,
-                                                     NSUnderlyingErrorKey: underlyingError])
-        
-        await MainActor.run {
-          // Handle the error later in `didFailProvisionalNavigation`
-          self.tab(for: webView)?.sslPinningError = error
-        }
-        
+
+        let underlyingError = NSError(
+          domain: kCFErrorDomainCFNetwork as String,
+          code: Int(errorCode),
+          userInfo: ["_kCFStreamErrorCodeKey": Int(errorCode)]
+        )
+
+        let error = NSError(
+          domain: kCFErrorDomainCFNetwork as String,
+          code: Int(errorCode),
+          userInfo: [
+            NSURLErrorFailingURLErrorKey: webView.url as Any,
+            "NSErrorPeerCertificateChainKey": certificateChain,
+            NSUnderlyingErrorKey: underlyingError,
+          ]
+        )
+
+        // Handle the error later in `didFailProvisionalNavigation`
+        self.tab(for: webView)?.sslPinningError = error
+
         return (.cancelAuthenticationChallenge, nil)
       }
     }
@@ -674,35 +677,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)
-
-      do {
-        let credentials = try await Authenticator.handleAuthRequest(
-          self,
-          credential: credential,
-          protectionSpace: protectionSpace,
-          previousFailureCount: previousFailureCount
+
+    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)
+
+    do {
+      let credentials = try await Authenticator.handleAuthRequest(
+        self,
+        credential: credential,
+        protectionSpace: protectionSpace,
+        previousFailureCount: previousFailureCount
+      )
+
+      if BasicAuthCredentialsManager.validDomains.contains(host) {
+        BasicAuthCredentialsManager.setCredential(
+          origin: origin,
+          credential: credentials.credentials
         )
-        
-        if BasicAuthCredentialsManager.validDomains.contains(host) {
-          BasicAuthCredentialsManager.setCredential(origin: origin, credential: credentials.credentials)
-        }
-        
-        return (.useCredential, credentials.credentials)
-      } catch {
-        return (.rejectProtectionSpace, nil)
       }
-    }.value
+
+      return (.useCredential, credentials.credentials)
+    } catch {
+      return (.rejectProtectionSpace, nil)
+    }
   }
 
   public func webView(_ webView: WKWebView, didCommit navigation: WKNavigation!) {
@@ -817,17 +823,7 @@ extension BrowserViewController: WKNavigationDelegate {
   /// Invoked when an error occurs while starting to load data for the main frame.
   public func webView(_ webView: WKWebView, didFailProvisionalNavigation navigation: WKNavigation!, withError error: Error) {
     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.
-    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
@@ -858,20 +854,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.
@@ -912,55 +898,48 @@ extension BrowserViewController {
   private func handleExternalURL(
     _ url: URL,
     tab: Tab?,
-    navigationAction: WKNavigationAction) async -> Bool {
-      // Do not open external links for child tabs automatically
-      // The user must tap on the link to open it.
-      if tab?.parent != nil && navigationAction.navigationType != .linkActivated {
-        return false
-      }
-      
-      // Check if the current url of the caller has changed
-      if let domain = tab?.url?.baseDomain,
-         domain != tab?.externalAppURLDomain {
-        tab?.externalAppAlertCounter = 0
-        tab?.isExternalAppAlertSuppressed = false
-      }
-      
-      tab?.externalAppURLDomain = tab?.url?.baseDomain
-      
-      // Do not try to present over existing warning
-      if tab?.isExternalAppAlertPresented == true || tab?.isExternalAppAlertSuppressed == true {
-        return false
-      }
-      
-      // 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 {
-        return false
-      }
-      
-      var alertTitle = Strings.openExternalAppURLGenericTitle
-      
-      if let displayHost = tab?.url?.withoutWWW.host {
-        alertTitle = String(format: Strings.openExternalAppURLTitle, displayHost)
-      }
-      
-      // Handling condition when Tab is empty when handling an external URL we should remove the tab once the user decides
-      let removeTabIfEmpty = { [weak self] in
-        if let tab = tab, tab.url == nil {
-          self?.tabManager.removeTab(tab)
-        }
+    navigationAction: WKNavigationAction
+  ) async -> Bool {
+    // Do not open external links for child tabs automatically
+    // The user must tap on the link to open it.
+    if tab?.parent != nil && navigationAction.navigationType != .linkActivated {
+      return false
+    }
+
+    // Check if the current url of the caller has changed
+    if let domain = tab?.url?.baseDomain,
+      domain != tab?.externalAppURLDomain
+    {
+      tab?.externalAppAlertCounter = 0
+      tab?.isExternalAppAlertSuppressed = false
+    }
+
+    tab?.externalAppURLDomain = tab?.url?.baseDomain
+
+    // Do not try to present over existing warning
+    if tab?.isExternalAppAlertPresented == true || tab?.isExternalAppAlertSuppressed == true {
+      return false
+    }
+
+    // External dialog should not be shown for non-active tabs #6687 - #7835
+    let isVisibleTab = tab?.isTabVisible() == true
+
+    if !isVisibleTab {
+      return false
+    }
+
+    var alertTitle = Strings.openExternalAppURLGenericTitle
+
+    if let displayHost = tab?.url?.withoutWWW.host {
+      alertTitle = String(format: Strings.openExternalAppURLTitle, displayHost)
+    }
+
+    // Handling condition when Tab is empty when handling an external URL we should remove the tab once the user decides
+    let removeTabIfEmpty = { [weak self] in
+      if let tab = tab, tab.url == nil {
+        self?.tabManager.removeTab(tab)
       }
+    }
 
       // Show the external sceheme invoke alert
       @MainActor