Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
URL bar falsely showing insecure state
Browse files Browse the repository at this point in the history
  • Loading branch information
@soner-yuksel
soner-yuksel committed Feb 28, 2024
1 parent 74ca7bf commit fef91c0
Show file tree
Hide file tree
Showing 16 changed files with 291 additions and 204 deletions.
6 changes: 5 additions & 1 deletion App/iOS/Delegates/AppDelegate.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
}

SystemUtils.onFirstRun()

// Clean Logger for Secure content state
DebugLogger.cleanLogger(for: .secureState)

return true
}

Expand Down Expand Up @@ -215,7 +219,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
SceneDelegate.shouldHandleUrpLookup = true
} else {
log.error("Failed to initialize user referral program")
UrpLog.log("Failed to initialize user referral program")
DebugLogger.log(for: .urp, text: "Failed to initialize user referral program")
}

if Preferences.URP.installAttributionLookupOutstanding.value == nil {
Expand Down
68 changes: 55 additions & 13 deletions Sources/Brave/Frontend/Browser/BrowserViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1812,6 +1812,7 @@ public class BrowserViewController: UIViewController {
}

tab.secureContentState = .mixedContent
logSecureContentState(tab: tab, path: path)
}

if let url = tab.webView?.url,
Expand All @@ -1822,18 +1823,22 @@ public class BrowserViewController: UIViewController {
if ErrorPageHelper.certificateError(for: url) != 0 {
// Cert validation takes precedence over all other errors
tab.secureContentState = .invalidCert
logSecureContentState(tab: tab, path: path, details: "Cert validation takes precedence over all other errors")
} else if NetworkErrorPageHandler.isNetworkError(errorCode: ErrorPageHelper.errorCode(for: url)) {
// Network error takes precedence over missing cert
// Because we cannot determine if a cert is missing yet, if we cannot connect to the server
// Our network interstitial page shows
tab.secureContentState = .localhost
logSecureContentState(tab: tab, path: path, details: "Network error takes precedence over missing cert")
} else {
// Since it's not a cert error explicitly, and it's not a network error, and the cert is missing (no serverTrust),
// then we display .missingSSL
tab.secureContentState = .missingSSL
logSecureContentState(tab: tab, path: path, details: "Certificate is missing (no serverTrust)")
}
} else if url.isReaderModeURL || InternalURL.isValid(url: url) {
tab.secureContentState = .localhost
logSecureContentState(tab: tab, path: path, details: "Reader Mode or Internal URL")
}
}

Expand All @@ -1846,14 +1851,18 @@ public class BrowserViewController: UIViewController {
}

tab.secureContentState = .unknown

guard let serverTrust = tab.webView?.serverTrust else {
if let url = tab.webView?.url ?? tab.url {
logSecureContentState(tab: tab, path: path)

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) {

tab.secureContentState = .localhost
logSecureContentState(tab: tab, path: path, details: "Internal URL aboutURL or is aboutHomeURL")

if tabManager.selectedTab === tab {
updateToolbarSecureContentState(.localhost)
}
Expand All @@ -1867,15 +1876,19 @@ public class BrowserViewController: UIViewController {
if ErrorPageHelper.certificateError(for: url) != 0 {
// Cert validation takes precedence over all other errors
tab.secureContentState = .invalidCert

logSecureContentState(tab: tab, path: path, details: "Cert validation takes precedence over all other errors")
} else if NetworkErrorPageHandler.isNetworkError(errorCode: ErrorPageHelper.errorCode(for: url)) {
// Network error takes precedence over missing cert
// Because we cannot determine if a cert is missing yet, if we cannot connect to the server
// Our network interstitial page shows
tab.secureContentState = .localhost
logSecureContentState(tab: tab, path: path, details: "Network error takes precedence over missing cert")
} else {
// Since it's not a cert error explicitly, and it's not a network error, and the cert is missing (no serverTrust),
// then we display .missingSSL
tab.secureContentState = .missingSSL
logSecureContentState(tab: tab, path: path, details: "Certificate is missing (no serverTrust)")
}

if tabManager.selectedTab === tab {
Expand All @@ -1886,6 +1899,8 @@ public class BrowserViewController: UIViewController {

if url.isReaderModeURL || InternalURL.isValid(url: url) {
tab.secureContentState = .localhost
logSecureContentState(tab: tab, path: path, details: "Reader Mode or Internal URL")

if tabManager.selectedTab === tab {
updateToolbarSecureContentState(.localhost)
}
Expand All @@ -1894,9 +1909,11 @@ public class BrowserViewController: UIViewController {

// All our checks failed, we show the page as insecure
tab.secureContentState = .missingSSL
logSecureContentState(tab: tab, path: path, details: "All our checks failed, we show the page as insecure")
} else {
// When there is no URL, it's likely a new tab.
tab.secureContentState = .localhost
logSecureContentState(tab: tab, path: path, details: "When there is no URL, it's likely a new tab")
}

if tabManager.selectedTab === tab {
Expand All @@ -1905,15 +1922,17 @@ public class BrowserViewController: UIViewController {
break
}

guard let scheme = tab.webView?.url?.scheme,
let host = tab.webView?.url?.host else {
guard let scheme = url.scheme,
let host = url.host else {
tab.secureContentState = .unknown
logSecureContentState(tab: tab, path: path, details: "No webview URL host scheme)")

self.updateURLBar()
return
}

let port: Int
if let urlPort = tab.webView?.url?.port {
if let urlPort = url.port {
port = urlPort
} else if scheme == "https" {
port = 443
Expand All @@ -1928,16 +1947,20 @@ public class BrowserViewController: UIViewController {
// Cert is valid!
if result == 0 {
tab.secureContentState = .secure
logSecureContentState(tab: tab, path: path, details: "Cert is valid!")
} else if result == Int32.min {
// Cert is valid but should be validated by the system
// Let the system handle it and we'll show an error if the system cannot validate it
try await BraveCertificateUtils.evaluateTrust(serverTrust, for: host)
tab.secureContentState = .secure
logSecureContentState(tab: tab, path: path, details: "Cert is valid but should be validated by the system")
} else {
tab.secureContentState = .invalidCert
logSecureContentState(tab: tab, path: path, details: "Invalid Cert")
}
} catch {
tab.secureContentState = .invalidCert
logSecureContentState(tab: tab, path: path, details: "Verify Trust Error")
}

self.updateURLBar()
Expand All @@ -1949,6 +1972,24 @@ public class BrowserViewController: UIViewController {
}
}

func logSecureContentState(tab: Tab, path: KVOConstants? = nil, details: String? = nil) {
var text = """
Tab URL: \(tab.url?.absoluteString ?? "Empty Tab URL")
Tab VebView URL: \(tab.webView?.url?.absoluteString ?? "Empty Webview URL")
Secure State: \(tab.secureContentState.rawValue)
"""

if let keyPath = path?.keyPath {
text.append("\n Value Observed: \(keyPath)\n")
}

if let extraDetails = details {
text.append("\n Extra Details: \(extraDetails)\n")
}

DebugLogger.log(for: .secureState, text: text)
}

func updateUIForReaderHomeStateForTab(_ tab: Tab) {
updateURLBar()
toolbarVisibilityViewModel.toolbarState = .expanded
Expand Down Expand Up @@ -2007,10 +2048,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)
Expand Down Expand Up @@ -2128,11 +2167,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)
Expand Down Expand Up @@ -2321,7 +2360,10 @@ public class BrowserViewController: UIViewController {
activities.append(addSearchEngineActivity)
}

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) {
if let selectedTab = tabManager.selectedTab {
logSecureContentState(tab: selectedTab, details: "Display Certificate Activity Settings")
}
let displayCertificateActivity = BasicMenuActivity(title: Strings.displayCertificate, braveSystemImage: "leo.lock.plain") { [weak self] in
self?.displayPageCertificateInfo()
}
Expand Down
Loading

0 comments on commit fef91c0

Please sign in to comment.