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

Commit

Permalink
Fix #8768: Add Logging to Track Secure Content State (#8769)
Browse files Browse the repository at this point in the history
  • Loading branch information
soner-yuksel authored Feb 27, 2024
1 parent e57c25e commit f41582f
Show file tree
Hide file tree
Showing 13 changed files with 225 additions and 92 deletions.
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
7 changes: 6 additions & 1 deletion Sources/Brave/Frontend/Settings/SettingsViewController.swift
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

0 comments on commit f41582f

Please sign in to comment.