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

Fix #8768: Add Logging to Track Secure Content State #8769

Merged
merged 1 commit into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions App/iOS/Delegates/AppDelegate.swift
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
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,9 @@ extension BrowserViewController {
}

// Display Certificate Activity
if let secureState = tabManager.selectedTab?.secureContentState, secureState != .missingSSL && secureState != .unknown {
if let selectedTab = tabManager.selectedTab, selectedTab.secureContentState != .missingSSL && selectedTab.secureContentState != .unknown {
logSecureContentState(tab: selectedTab, details: "Display Certificate Activity Settings")

activities.append(
BasicMenuActivity(
title: Strings.displayCertificate,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ extension BrowserViewController: WKNavigationDelegate {
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)
}
}
Expand Down Expand Up @@ -715,6 +717,8 @@ extension BrowserViewController: WKNavigationDelegate {
// However, WebKit does NOT trigger the `serverTrust` observer when the URL changes, but the trust has not.
// WebKit also does NOT trigger the `serverTrust` observer when the page is actually insecure (non-https).
// So manually trigger it with the current trust.
logSecureContentState(tab: tab, details: "ObserveValue trigger in didCommit")

observeValue(forKeyPath: KVOConstants.serverTrust.keyPath,
of: webView,
change: [.newKey: webView.serverTrust as Any, .kindKey: 1],
Expand Down Expand Up @@ -823,6 +827,8 @@ extension BrowserViewController: WKNavigationDelegate {
// 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],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1834,6 +1834,7 @@ public class BrowserViewController: UIViewController {
}

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

if let url = tab.webView?.url,
Expand All @@ -1844,18 +1845,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 @@ -1868,14 +1873,17 @@ 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 {
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 @@ -1889,15 +1897,20 @@ 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 @@ -1908,6 +1921,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 @@ -1916,9 +1931,13 @@ 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 @@ -1930,6 +1949,8 @@ public class BrowserViewController: UIViewController {
guard let scheme = tab.webView?.url?.scheme,
let host = tab.webView?.url?.host else {
tab.secureContentState = .unknown
logSecureContentState(tab: tab, path: path, details: "No webview URL host scheme)")

self.updateURLBar()
return
}
Expand All @@ -1950,16 +1971,24 @@ 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 @@ -1971,6 +2000,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 updateForwardStatusIfNeeded(webView: WKWebView) {
if let forwardListItem = webView.backForwardList.forwardList.first, forwardListItem.url.isReaderModeURL {
navigationToolbar.updateForwardStatus(false)
Expand Down
16 changes: 8 additions & 8 deletions Sources/Brave/Frontend/Browser/Tab.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ protocol URLChangeDelegate {
func tab(_ tab: Tab, urlDidChangeTo url: URL)
}

enum TabSecureContentState {
case unknown
case localhost
case secure
case invalidCert
case missingSSL
case mixedContent
enum TabSecureContentState: String {
case unknown = "Unknown"
case localhost = "Localhost"
case secure = "Secure"
case invalidCert = "InvalidCertificate"
case missingSSL = "SSL Certificate Missing"
case mixedContent = "Mixed Content"

var shouldDisplayWarning: Bool {
switch self {
case .unknown, .invalidCert, .missingSSL, .mixedContent:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -809,10 +809,15 @@ class SettingsViewController: TableViewController {
let vc = AdblockDebugMenuTableViewController(style: .grouped)
self.navigationController?.pushViewController(vc, animated: true)
}, accessory: .disclosureIndicator, cellClass: MultilineValue1Cell.self),
Row(
text: "Secure Content State Debug",
selection: { [unowned self] in
self.navigationController?.pushViewController(DebugLogViewController(type: .secureState), animated: true)
}, accessory: .disclosureIndicator, cellClass: MultilineValue1Cell.self),
Row(
text: "View URP Logs",
selection: { [unowned self] in
self.navigationController?.pushViewController(UrpLogsViewController(), animated: true)
self.navigationController?.pushViewController(DebugLogViewController(type: .urp), animated: true)
}, accessory: .disclosureIndicator, cellClass: MultilineValue1Cell.self),
Row(text: "URP Code: \(UserReferralProgram.getReferralCode() ?? "--")"),
Row(
Expand Down
111 changes: 111 additions & 0 deletions Sources/BraveShared/DebugLogger.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// Copyright 2024 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/.

import UIKit

public enum LoggerType {
case urp, secureState

var prefsKey: String {
switch self {
case .urp:
return "urpLogs"
case .secureState:
return "secureStateLogs"
}
}
}

public struct DebugLogger {
public static func log(for type: LoggerType, text: String) {
var logs = UserDefaults.standard.string(forKey: type.prefsKey) ?? ""

let date = Date()
let calendar = Calendar.current
let components = calendar.dateComponents([.year, .month, .day, .hour, .minute], from: date)

guard let year = components.year, let month = components.month, let day = components.day, let hour = components.hour, let minute = components.minute else {
return
}
let time = "\(year)-\(month)-\(day) \(hour):\(minute)"

switch type {
case .secureState:
logs.append("------------------------------------\n\n")
logs.append(" [\(time)]\n\n \(text)\n")
case .urp:
logs.append("[\(time)] \(text)\n")
}

UserDefaults.standard.set(logs, forKey: type.prefsKey)
}

public static func cleanLogger(for type: LoggerType) {
UserDefaults.standard.removeObject(forKey: type.prefsKey)
}
}

public class DebugLogViewController: UIViewController {
var loggerType: LoggerType

public init(type: LoggerType) {
loggerType = type

super.init(nibName: nil, bundle: nil)
}

required init?(coder: NSCoder) {
fatalError("init(coder:) has not been implemented")
}

public override func viewDidLoad() {
super.viewDidLoad()

switch loggerType {
case .urp:
title = "URP Logs"
case .secureState:
title = "Secure Content State"
}

let rightBarButtonItem = UIBarButtonItem(barButtonSystemItem: .action, target: self, action: #selector(shareButtonTapped))
navigationItem.rightBarButtonItem = rightBarButtonItem

let textView = UITextView()
textView.translatesAutoresizingMaskIntoConstraints = false
textView.isEditable = false

view.addSubview(textView)

NSLayoutConstraint.activate([
textView.topAnchor.constraint(
equalTo: view.topAnchor, constant: 8),
textView.leadingAnchor.constraint(
equalTo: view.leadingAnchor, constant: 8),
textView.trailingAnchor.constraint(
equalTo: view.trailingAnchor, constant: -8),
textView.bottomAnchor.constraint(
equalTo: view.bottomAnchor, constant: -8)
])

guard let logs = UserDefaults.standard.string(forKey: loggerType.prefsKey) else { return }
let title = "Secure Content Logs\n\n"

textView.text = title + logs
}

@objc func shareButtonTapped() {
guard let logs = UserDefaults.standard.string(forKey: loggerType.prefsKey) else { return }

// Create an activity view controller with the text to share
let activityViewController = UIActivityViewController(activityItems: [logs], applicationActivities: nil)

// Present the activity view controller
if let popoverController = activityViewController.popoverPresentationController {
popoverController.barButtonItem = navigationItem.rightBarButtonItem
}
present(activityViewController, animated: true, completion: nil)
}
}
11 changes: 6 additions & 5 deletions Sources/Growth/DAU.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import Shared
import BraveCore
import os.log
import Preferences
import BraveShared

public class DAU {

Expand Down Expand Up @@ -92,7 +93,7 @@ public class DAU {
@objc private func sendPingToServerInternal() {
guard let paramsAndPrefs = paramsAndPrefsSetup(for: Date()) else {
Logger.module.debug("dau, no changes detected, no server ping")
UrpLog.log("dau, no changes detected, no server ping")
DebugLogger.log(for: .urp, text: "dau, no changes detected, no server ping")
return
}

Expand All @@ -112,8 +113,8 @@ public class DAU {
}

Logger.module.debug("send ping to server, url: \(pingRequestUrl)")
UrpLog.log("send ping to server, url: \(pingRequestUrl)")

DebugLogger.log(for: .urp, text: "send ping to server, url: \(pingRequestUrl)")
var request = URLRequest(url: pingRequestUrl)
for (key, value) in paramsAndPrefs.headers {
request.setValue(value, forHTTPHeaderField: key)
Expand All @@ -126,7 +127,7 @@ public class DAU {

if let e = error {
Logger.module.error("status update error: \(e.localizedDescription)")
UrpLog.log("status update error: \(e)")
DebugLogger.log(for: .urp, text: "status update error: \(e)")
return
}

Expand Down Expand Up @@ -212,7 +213,7 @@ public class DAU {

if let referralCode = UserReferralProgram.getReferralCode() {
params.append(URLQueryItem(name: "ref", value: referralCode))
UrpLog.log("DAU ping with added ref, params: \(params)")
DebugLogger.log(for: .urp, text: "DAU ping with added ref, params: \(params)")
}

let lastPingTimestamp = [Int((date).timeIntervalSince1970)]
Expand Down
Loading
Loading