From e2c75b7fce9f7b6b3dc1ec3ea749681542885f97 Mon Sep 17 00:00:00 2001 From: staszekscp Date: Wed, 28 Oct 2026 11:25:32 +0100 Subject: [PATCH 1/3] Add cert info to logs --- config/certificatePinning/README.md | 43 +++- cspell.json | 1 + ios/CertificatePinning.swift | 257 +++++++++++++++++++++- tests/unit/generateCertificatePinsTest.ts | 14 ++ 4 files changed, 308 insertions(+), 7 deletions(-) diff --git a/config/certificatePinning/README.md b/config/certificatePinning/README.md index 49c3b827adbc..0aa025a48d85 100644 --- a/config/certificatePinning/README.md +++ b/config/certificatePinning/README.md @@ -78,7 +78,10 @@ Pinning is **disabled in debug builds** on every layer (Android `BuildConfig.DEB ## Sentry reporting Pin failures are reported from the **native** pinning layer (TrustKit callback on iOS, monitor -interceptors on Android), tagged with: +interceptors on Android), one report per failed validation: HybridApp (Mobile-Expensify) sends a Sentry +**log** (message `Certificate pinning validation failed`) in monitor mode and a captured error in +enforce mode; the standalone NewDot layers here send a captured error in both modes. The attributes +below ride along either way, as log attributes or as tags (long values as extras): - `certificate_pinning_host` — the hostname that failed validation - `certificate_pinning_mode` — `monitor` or `enforce` - `certificate_pinning_channel` — (Android only) the networking channel: `OkHttp`, `HttpURLConnection`, `WebView`, or `cronet` @@ -89,6 +92,44 @@ interceptors on Android), tagged with: event is still worth investigating before flipping to enforce mode: OkHttp and the platform rebuild the chain the same way, so enforce mode would block that connection. +On Android the OkHttp `SSLPeerUnverifiedException` message already lists the served chain (subject and +SPKI hash per certificate). On iOS, TrustKit only reports *that* no pin matched, so the iOS reporters +(`ios/CertificatePinning.swift` here, `iOS/Expensify/CertificatePinningReporter.m` in Mobile-Expensify) +add what the device actually validated. Every value is typed so the column never flaps between boolean +and number in Sentry — the old numeric `certificate_pinning_evaluation_result` landed as `true` for +TrustKit result 1 and as `2` for result 2 and is gone: +- `certificate_pinning_result` — `no_matching_pin` (the device trusted the chain but no pinned root was + in it) or `invalid_certificate_chain` (the device itself rejected the chain; this fails with or without + pinning). Rarely `invalid_parameters` / `could_not_generate_spki_hash`. +- `certificate_pinning_sec_trust_result` — how the device classified the anchor: `proceed` means a root the + user or an MDM profile explicitly trusts (a TLS-intercepting proxy), `unspecified` a system root with no + user override (a public CA that is not pinned). This is the single most useful field for triage. +- `certificate_pinning_chain_subjects` / `certificate_pinning_chain_fingerprints` — the validated chain, + leaf first, anchor last; subjects joined with ` > `, SHA-256 certificate fingerprints (upper-case hex, + no colons — strip the colons from a `ROOT_MANIFEST` entry in `scripts/generateCertificatePins.sh` to + compare) joined with `,`. `certificate_pinning_chain_length` is the number of certificates, as a string. +- `certificate_pinning_anchor_subject` / `certificate_pinning_anchor_fingerprint` / + `certificate_pinning_anchor_self_signed` — the last certificate in that chain. A fingerprint that is not + in `ROOT_MANIFEST` and a subject naming a proxy vendor or a company is interception; a public root that + is not in `pins.json` is a pin-list gap (look the fingerprint up on crt.sh to get the certificate and + derive its SPKI pin). Certificates themselves are deliberately not sent: the fingerprint identifies the + anchor, and a PEM would embed a customer's proxy CA verbatim in every such report. +- `certificate_pinning_trust_errors` — for `invalid_certificate_chain`, the system's reasons ("expired", + "not trusted", hostname mismatch…). +- `certificate_pinning_trust_decision` — TrustKit's `allow` / `block`, confirming monitor mode did not block. +- `certificate_pinning_http_proxy_configured` / `certificate_pinning_pac_configured` / + `certificate_pinning_vpn_interface_present` / `certificate_pinning_interface` — network context. + Presence only; the proxy host and PAC URL are never sent. +- `certificate_pinning_pin_set_version` — the `_lastGenerated` value of the `pins.json` the client's + hashes were copied from, so a report can be matched to the exact pin set it was evaluated against. +- `certificate_pinning_occurrence` — per-session count of reports with the same host, anchor and result + (as a string). One device can fire several identical reports per second from parallel image loads. +- `certificate_pinning_validation_duration_us` — TrustKit's validation time in microseconds. + +Before reading these numbers, check `find_dropped_events` / the Stats page for the `logs` category: when +the log quota is exceeded Sentry drops these reports along with everything else, and a flat chart may +mean "nothing landed" rather than "nothing failed". + Reporting requires early native Sentry initialization via `SentryNativeSDKManager` in `AppDelegate.swift` / `MainApplication.kt` (standalone NewDot) or `ExpensifyAppDelegate.m` / `Expensify.java` (HybridApp), before certificate pinning. JS diff --git a/cspell.json b/cspell.json index b6ce4ae2b428..90a8321b243a 100644 --- a/cspell.json +++ b/cspell.json @@ -1140,6 +1140,7 @@ "useblacksmith", "usernotifications", "utilise", + "utun", "vcfanzfav", "vcfcitibank", "vcfmock", diff --git a/ios/CertificatePinning.swift b/ios/CertificatePinning.swift index 3b764ed02ded..ecd3d839c056 100644 --- a/ios/CertificatePinning.swift +++ b/ios/CertificatePinning.swift @@ -16,7 +16,11 @@ // Regenerate via scripts/generateCertificatePins.sh. // +import CFNetwork +import CommonCrypto import Foundation +import Network +import Security import Sentry import TrustKit @@ -26,6 +30,11 @@ enum CertificatePinning { /// Keep in sync with `enforcePinning` in config/certificatePinning/pins.json and CertificatePinning.kt. private static let enforcePinning = false + /// Identifies the pin list a client shipped with, independent of the app version: the `_lastGenerated` + /// value of config/certificatePinning/pins.json these hashes were copied from. Update it together with + /// the hashes. Reported as `certificate_pinning_pin_set_version`. + private static let pinSetVersion = "2026-09-14" + private static let certificatePinningHostTag = "certificate_pinning_host" private static let certificatePinningModeTag = "certificate_pinning_mode" @@ -103,26 +112,262 @@ enum CertificatePinning { TrustKit.initSharedInstance(withConfiguration: trustKitConfig) + // Reporting rebuilds the validated chain from TrustKit's PEM array and re-runs a trust + // evaluation to classify the anchor, so it runs on its own serial queue rather than + // TrustKit's default (main). + TrustKit.sharedInstance().pinningValidatorCallbackQueue = reportingQueue TrustKit.sharedInstance().pinningValidatorCallback = { result, hostname, _ in guard result.evaluationResult != .success else { return } - reportPinningFailure(hostname: hostname, evaluationResult: result.evaluationResult) + reportPinningFailure(result: result, hostname: hostname) } #endif } - private static func reportPinningFailure(hostname: String, evaluationResult: TSKTrustEvaluationResult) { + // MARK: - Reporting + // + // TrustKit only says *that* a pin did not match; the attributes below record *what* iOS validated + // (the full chain, the trust anchor, how the device trusts that anchor, and the network context) so + // a mismatch in Sentry can be classified as a CA we do not pin versus a TLS-intercepting proxy + // without a device in hand. Keep this set in sync with Mobile-Expensify's + // iOS/Expensify/CertificatePinningReporter.m and the table in config/certificatePinning/README.md. + // + // Sentry's attribute converter matches Bool before Int, so any number holding 0 or 1 lands as a + // boolean and the column type flaps between releases. Enum values are therefore strings, counts + // that can be 0 or 1 are strings, real flags are Bool, and the validation duration is reported in + // microseconds (never 0 or 1 in practice). + + /// Serial queue TrustKit invokes the callback on. Everything below that mutates static state runs here. + private static let reportingQueue = DispatchQueue(label: "com.expensify.certificatePinning.reporting", qos: .utility) + + private static var occurrenceCounts: [String: Int] = [:] + + /// Tags are capped at 200 characters; these ride along as extras instead. + private static let extraKeys: Set = [ + "certificate_pinning_chain_subjects", + "certificate_pinning_chain_fingerprints", + ] + private static var currentInterfaceType = "unknown" + private static let pathMonitor: NWPathMonitor = { + let monitor = NWPathMonitor() + monitor.pathUpdateHandler = { path in + if path.usesInterfaceType(.wifi) { + currentInterfaceType = "wifi" + } else if path.usesInterfaceType(.cellular) { + currentInterfaceType = "cellular" + } else if path.usesInterfaceType(.wiredEthernet) { + currentInterfaceType = "wired" + } else if path.usesInterfaceType(.other) { + currentInterfaceType = "other" // VPN or tunnel + } else { + currentInterfaceType = "unknown" + } + } + monitor.start(queue: reportingQueue) + return monitor + }() + + private static func reportPinningFailure(result: TSKPinningValidatorResult, hostname: String) { + let chain = certificates(fromPEMChain: result.certificateChain) + let subjects = chain.map(subjectSummary) + let fingerprints = chain.map(fingerprint) + let anchor = chain.last + let anchorFingerprint = fingerprints.last ?? "" + let (trustResult, trustErrors) = evaluate(chain, hostname: hostname) + + let resultName = name(of: result.evaluationResult) + let dedupKey = "\(hostname)|\(anchorFingerprint)|\(resultName)" + let occurrence = (occurrenceCounts[dedupKey] ?? 0) + 1 + occurrenceCounts[dedupKey] = occurrence + + var attributes: [String: Any] = [ + // Existing attributes, unchanged + certificatePinningHostTag: hostname, + certificatePinningModeTag: enforcePinning ? "enforce" : "monitor", + // Typed replacement for the old numeric `certificate_pinning_evaluation_result` + "certificate_pinning_result": resultName, + "certificate_pinning_trust_decision": name(of: result.finalTrustDecision), + "certificate_pinning_sec_trust_result": name(of: trustResult), + // What iOS validated, leaf first + "certificate_pinning_chain_length": String(chain.count), + "certificate_pinning_chain_subjects": subjects.joined(separator: " > "), + "certificate_pinning_chain_fingerprints": fingerprints.joined(separator: ","), + "certificate_pinning_anchor_subject": subjects.last ?? "", + "certificate_pinning_anchor_fingerprint": anchorFingerprint, + "certificate_pinning_anchor_self_signed": anchor.map(isSelfSigned) ?? false, + // Context + "certificate_pinning_pin_set_version": pinSetVersion, + "certificate_pinning_validation_duration_us": Int((result.validationDuration * 1_000_000).rounded()), + "certificate_pinning_interface": interfaceType(), + "certificate_pinning_occurrence": String(occurrence), + ] + attributes.merge(networkContext()) { current, _ in current } + + if !trustErrors.isEmpty { + attributes["certificate_pinning_trust_errors"] = trustErrors + } + + // One captured error per failed validation in both modes, exactly as before; the detail above + // rides along as tags and extras. let error = NSError( domain: "CertificatePinning", - code: evaluationResult.rawValue, + code: result.evaluationResult.rawValue, userInfo: [NSLocalizedDescriptionKey: "Certificate pinning validation failed for \(hostname)"] ) - SentrySDK.capture(error: error) { scope in - scope.setTag(value: hostname, key: certificatePinningHostTag) - scope.setTag(value: enforcePinning ? "enforce" : "monitor", key: certificatePinningModeTag) + for (key, value) in attributes { + if extraKeys.contains(key) { + scope.setExtra(value: value, key: key) + } else { + scope.setTag(value: String(describing: value), key: key) + } + } scope.setFingerprint(["certificate-pinning", hostname]) } } + + // MARK: Enum -> string (stable attribute types) + + private static func name(of result: TSKTrustEvaluationResult) -> String { + switch result { + case .success: return "success" + case .failedNoMatchingPin: return "no_matching_pin" + case .failedInvalidCertificateChain: return "invalid_certificate_chain" + case .errorInvalidParameters: return "invalid_parameters" + case .errorCouldNotGenerateSpkiHash: return "could_not_generate_spki_hash" + default: return "unknown_\(result.rawValue)" + } + } + + private static func name(of decision: TSKTrustDecision) -> String { + switch decision { + case .shouldAllowConnection: return "allow" + case .shouldBlockConnection: return "block" + case .domainNotPinned: return "domain_not_pinned" + default: return "unknown_\(decision.rawValue)" + } + } + + /// `proceed` means the anchor is explicitly trusted by the user or an MDM profile ("Enable Full Trust for + /// Root Certificates"); `unspecified` means a system root with no user override. This is the cheapest + /// single signal separating a TLS-intercepting proxy from a public CA we do not pin. + private static func name(of result: SecTrustResultType) -> String { + switch result { + case .invalid: return "invalid" + case .proceed: return "proceed" + case .deny: return "deny" + case .unspecified: return "unspecified" + case .recoverableTrustFailure: return "recoverable_trust_failure" + case .fatalTrustFailure: return "fatal_trust_failure" + case .otherError: return "other_error" + default: return "unknown_\(result.rawValue)" + } + } + + // MARK: Certificate helpers + + /// TrustKit builds `certificateChain` from the evaluated SecTrust, so it is leaf-first and includes the + /// anchor whenever iOS found one. + private static func certificates(fromPEMChain chain: [Any]) -> [SecCertificate] { + chain.compactMap { entry in + guard let pem = entry as? String else { + return nil + } + let base64 = pem + .replacingOccurrences(of: "-----BEGIN CERTIFICATE-----", with: "") + .replacingOccurrences(of: "-----END CERTIFICATE-----", with: "") + guard let der = Data(base64Encoded: base64, options: .ignoreUnknownCharacters), !der.isEmpty else { + return nil + } + return SecCertificateCreateWithData(nil, der as CFData) + } + } + + /// SHA-256 of the DER certificate, upper-case hex, no colons. Strip the colons from a ROOT_MANIFEST entry + /// in scripts/generateCertificatePins.sh and it is directly comparable. + private static func fingerprint(_ certificate: SecCertificate) -> String { + let der = SecCertificateCopyData(certificate) as Data + var digest = [UInt8](repeating: 0, count: Int(CC_SHA256_DIGEST_LENGTH)) + der.withUnsafeBytes { buffer in + _ = CC_SHA256(buffer.baseAddress, CC_LONG(der.count), &digest) + } + return digest.map { String(format: "%02X", $0) }.joined() + } + + private static func subjectSummary(_ certificate: SecCertificate) -> String { + let summary = SecCertificateCopySubjectSummary(certificate) as String? + return summary.flatMap { $0.isEmpty ? nil : $0 } ?? "" + } + + private static func isSelfSigned(_ certificate: SecCertificate) -> Bool { + guard let subject = SecCertificateCopyNormalizedSubjectSequence(certificate) as Data?, + let issuer = SecCertificateCopyNormalizedIssuerSequence(certificate) as Data? else { + return false + } + return subject == issuer + } + + /// Re-runs the system trust evaluation on the chain TrustKit validated. TrustKit 3.x does not expose the + /// original SecTrust, so this is the only way to learn how the device classified the anchor + /// (`SecTrustResultType`) and, for an untrusted chain, why ("expired", "not trusted", hostname mismatch...). + /// Revocation checking is best-effort and may touch the network, which is why this never runs on the + /// main queue. + private static func evaluate(_ certificates: [SecCertificate], hostname: String) -> (SecTrustResultType, String) { + guard !certificates.isEmpty else { + return (.invalid, "") + } + let policy = SecPolicyCreateSSL(true, hostname as CFString) + var createdTrust: SecTrust? + guard SecTrustCreateWithCertificates(certificates as CFArray, policy, &createdTrust) == errSecSuccess, + let trust = createdTrust else { + return (.invalid, "") + } + + var errors: [String] = [] + var evaluationError: CFError? + _ = SecTrustEvaluateWithError(trust, &evaluationError) + if let evaluationError { + let description = (evaluationError as Error).localizedDescription + if !description.isEmpty { + errors.append(description) + } + } + var trustResult = SecTrustResultType.invalid + SecTrustGetTrustResult(trust, &trustResult) + + if let properties = SecTrustCopyProperties(trust) as? [[String: Any]] { + for property in properties { + if let message = property[kSecPropertyTypeError as String] as? String, !message.isEmpty, !errors.contains(message) { + errors.append(message) + } + } + } + return (trustResult, errors.joined(separator: " | ")) + } + + // MARK: Network context + + /// Presence only: never the proxy host or PAC URL, which would identify the customer's network. Per-interface + /// settings under `__SCOPED__` reveal tunnels (utun/ipsec/ppp/tap), which is where per-app VPN and TLS + /// inspection usually live. + private static func networkContext() -> [String: Any] { + let proxy = CFNetworkCopySystemProxySettings()?.takeRetainedValue() as? [String: Any] ?? [:] + let httpProxy = (proxy[kCFNetworkProxiesHTTPEnable as String] as? Bool) ?? false + let pac = (proxy[kCFNetworkProxiesProxyAutoConfigEnable as String] as? Bool) ?? false + let scoped = proxy["__SCOPED__"] as? [String: Any] ?? [:] + let tunnelPrefixes = ["utun", "tun", "tap", "ppp", "ipsec"] + let tunnel = scoped.keys.contains { interface in tunnelPrefixes.contains { interface.hasPrefix($0) } } + return [ + "certificate_pinning_http_proxy_configured": httpProxy, + "certificate_pinning_pac_configured": pac, + "certificate_pinning_vpn_interface_present": tunnel, + ] + } + + /// Current interface type, cached from a path monitor started on first use so the lookup stays synchronous. + private static func interfaceType() -> String { + _ = pathMonitor + return currentInterfaceType + } } diff --git a/tests/unit/generateCertificatePinsTest.ts b/tests/unit/generateCertificatePinsTest.ts index a2d10119c96d..6a8093237873 100644 --- a/tests/unit/generateCertificatePinsTest.ts +++ b/tests/unit/generateCertificatePinsTest.ts @@ -26,11 +26,25 @@ jest.setTimeout(120000); const repoRoot = path.resolve(__dirname, '../..'); const scriptPath = path.join(repoRoot, 'scripts/generateCertificatePins.sh'); const pinsJsonPath = path.join(repoRoot, 'config/certificatePinning/pins.json'); +/** + * The HybridApp pin lists live in the Mobile-Expensify submodule and are what actually ships to the stores. + * They drifted from pins.json once already (the Sectigo roots were missing for a month and every HybridApp + * build reported the 2026-10 Sectigo chain on staging.expensify.com as a mismatch), so they are checked + * here too whenever the submodule is checked out. CI for this suite does not fetch the submodule, so that + * check only runs locally and in HybridApp builds; a missing submodule is skipped rather than failed. + */ +const hybridAppPinFiles = [ + 'Mobile-Expensify/Android/res/xml/network_security_config_enforce.xml', + 'Mobile-Expensify/Android/src/yapl/android/http/ExpensifyCertificatePinner.java', + 'Mobile-Expensify/iOS/Expensify/ExpensifyAppDelegate.m', +].filter((relativePath) => fs.existsSync(path.join(repoRoot, relativePath))); + const nativePinFiles = [ 'android/app/src/main/java/com/expensify/chat/CertificatePinning.kt', 'android/app/src/main/res/xml/network_security_config_enforce.xml', 'ios/CertificatePinning.swift', 'patches/react-native-nitro-fetch/react-native-nitro-fetch+1.6.2+001+certificate-pinning.patch', + ...hybridAppPinFiles, ]; const CLOUDFLARE_GROUP_HEADER = '=== Groups A-D: Cloudflare-fronted expensify.com hosts ==='; From 57ad86585c9fae58bdbb410012f44013be7ac761 Mon Sep 17 00:00:00 2001 From: staszekscp Date: Mon, 2 Nov 2026 15:42:05 +0100 Subject: [PATCH 2/3] Apply review --- ios/CertificatePinning.swift | 56 +++++++++++------------ tests/unit/generateCertificatePinsTest.ts | 8 ++-- 2 files changed, 30 insertions(+), 34 deletions(-) diff --git a/ios/CertificatePinning.swift b/ios/CertificatePinning.swift index ecd3d839c056..18e116153bae 100644 --- a/ios/CertificatePinning.swift +++ b/ios/CertificatePinning.swift @@ -30,9 +30,9 @@ enum CertificatePinning { /// Keep in sync with `enforcePinning` in config/certificatePinning/pins.json and CertificatePinning.kt. private static let enforcePinning = false - /// Identifies the pin list a client shipped with, independent of the app version: the `_lastGenerated` - /// value of config/certificatePinning/pins.json these hashes were copied from. Update it together with - /// the hashes. Reported as `certificate_pinning_pin_set_version`. + /// Identifies the shipped pin set independently of the app version using `_lastGenerated` + /// from config/certificatePinning/pins.json. Update it with the hashes. + /// Reported as `certificate_pinning_pin_set_version`. private static let pinSetVersion = "2026-09-14" private static let certificatePinningHostTag = "certificate_pinning_host" @@ -127,16 +127,15 @@ enum CertificatePinning { // MARK: - Reporting // - // TrustKit only says *that* a pin did not match; the attributes below record *what* iOS validated - // (the full chain, the trust anchor, how the device trusts that anchor, and the network context) so - // a mismatch in Sentry can be classified as a CA we do not pin versus a TLS-intercepting proxy - // without a device in hand. Keep this set in sync with Mobile-Expensify's - // iOS/Expensify/CertificatePinningReporter.m and the table in config/certificatePinning/README.md. + // Record TrustKit's certificate chain, a fresh system trust result, and network settings + // to help investigate pin failures in Sentry. These fields help distinguish an unpinned + // CA from a proxy, but do not establish the cause on their own. + // Keep these fields in sync with Mobile-Expensify's iOS/Expensify/CertificatePinningReporter.m + // and the field descriptions in config/certificatePinning/README.md. // - // Sentry's attribute converter matches Bool before Int, so any number holding 0 or 1 lands as a - // boolean and the column type flaps between releases. Enum values are therefore strings, counts - // that can be 0 or 1 are strings, real flags are Bool, and the validation duration is reported in - // microseconds (never 0 or 1 in practice). + // In attributes, enum names and counts are strings, flags are Bool, and validation + // duration is an integer number of microseconds. Send fields listed in extraKeys as + // extras and convert all other values to strings for tags. /// Serial queue TrustKit invokes the callback on. Everything below that mutates static state runs here. private static let reportingQueue = DispatchQueue(label: "com.expensify.certificatePinning.reporting", qos: .utility) @@ -159,7 +158,7 @@ enum CertificatePinning { } else if path.usesInterfaceType(.wiredEthernet) { currentInterfaceType = "wired" } else if path.usesInterfaceType(.other) { - currentInterfaceType = "other" // VPN or tunnel + currentInterfaceType = "other" // Virtual or otherwise unknown interface } else { currentInterfaceType = "unknown" } @@ -182,10 +181,9 @@ enum CertificatePinning { occurrenceCounts[dedupKey] = occurrence var attributes: [String: Any] = [ - // Existing attributes, unchanged certificatePinningHostTag: hostname, certificatePinningModeTag: enforcePinning ? "enforce" : "monitor", - // Typed replacement for the old numeric `certificate_pinning_evaluation_result` + // TrustKit's evaluation result and connection decision, plus the system trust result "certificate_pinning_result": resultName, "certificate_pinning_trust_decision": name(of: result.finalTrustDecision), "certificate_pinning_sec_trust_result": name(of: trustResult), @@ -196,7 +194,6 @@ enum CertificatePinning { "certificate_pinning_anchor_subject": subjects.last ?? "", "certificate_pinning_anchor_fingerprint": anchorFingerprint, "certificate_pinning_anchor_self_signed": anchor.map(isSelfSigned) ?? false, - // Context "certificate_pinning_pin_set_version": pinSetVersion, "certificate_pinning_validation_duration_us": Int((result.validationDuration * 1_000_000).rounded()), "certificate_pinning_interface": interfaceType(), @@ -208,8 +205,8 @@ enum CertificatePinning { attributes["certificate_pinning_trust_errors"] = trustErrors } - // One captured error per failed validation in both modes, exactly as before; the detail above - // rides along as tags and extras. + // Capture one error for each failed validation in either mode, with these attributes + // attached as tags and extras. let error = NSError( domain: "CertificatePinning", code: result.evaluationResult.rawValue, @@ -249,9 +246,9 @@ enum CertificatePinning { } } - /// `proceed` means the anchor is explicitly trusted by the user or an MDM profile ("Enable Full Trust for - /// Root Certificates"); `unspecified` means a system root with no user override. This is the cheapest - /// single signal separating a TLS-intercepting proxy from a public CA we do not pin. + /// `proceed` indicates a successful evaluation based on a user trust decision; + /// `unspecified` indicates success without an explicit user decision. + /// Neither result alone identifies a proxy or proves the anchor is a public CA. private static func name(of result: SecTrustResultType) -> String { switch result { case .invalid: return "invalid" @@ -308,11 +305,11 @@ enum CertificatePinning { return subject == issuer } - /// Re-runs the system trust evaluation on the chain TrustKit validated. TrustKit 3.x does not expose the - /// original SecTrust, so this is the only way to learn how the device classified the anchor - /// (`SecTrustResultType`) and, for an untrusted chain, why ("expired", "not trusted", hostname mismatch...). - /// Revocation checking is best-effort and may touch the network, which is why this never runs on the - /// main queue. + /// Evaluates TrustKit's certificate chain with an SSL policy for hostname. + /// Returns the new evaluation's SecTrustResultType and any error messages. + /// TrustKit does not expose its original SecTrust in the callback, so this evaluation + /// may differ from the original. It runs on reportingQueue because trust evaluation + /// can block while fetching certificates or checking revocation. private static func evaluate(_ certificates: [SecCertificate], hostname: String) -> (SecTrustResultType, String) { guard !certificates.isEmpty else { return (.invalid, "") @@ -348,9 +345,10 @@ enum CertificatePinning { // MARK: Network context - /// Presence only: never the proxy host or PAC URL, which would identify the customer's network. Per-interface - /// settings under `__SCOPED__` reveal tunnels (utun/ipsec/ppp/tap), which is where per-app VPN and TLS - /// inspection usually live. + /// Reports whether HTTP proxy and PAC settings are enabled, and whether `__SCOPED__` + /// contains an interface whose name starts with utun, tun, tap, ppp, or ipsec. + /// These names suggest a tunnel but do not prove this connection uses a VPN. + /// The proxy host and PAC URL are omitted to protect network details. private static func networkContext() -> [String: Any] { let proxy = CFNetworkCopySystemProxySettings()?.takeRetainedValue() as? [String: Any] ?? [:] let httpProxy = (proxy[kCFNetworkProxiesHTTPEnable as String] as? Bool) ?? false diff --git a/tests/unit/generateCertificatePinsTest.ts b/tests/unit/generateCertificatePinsTest.ts index 6a8093237873..16a012951421 100644 --- a/tests/unit/generateCertificatePinsTest.ts +++ b/tests/unit/generateCertificatePinsTest.ts @@ -27,11 +27,9 @@ const repoRoot = path.resolve(__dirname, '../..'); const scriptPath = path.join(repoRoot, 'scripts/generateCertificatePins.sh'); const pinsJsonPath = path.join(repoRoot, 'config/certificatePinning/pins.json'); /** - * The HybridApp pin lists live in the Mobile-Expensify submodule and are what actually ships to the stores. - * They drifted from pins.json once already (the Sectigo roots were missing for a month and every HybridApp - * build reported the 2026-10 Sectigo chain on staging.expensify.com as a mismatch), so they are checked - * here too whenever the submodule is checked out. CI for this suite does not fetch the submodule, so that - * check only runs locally and in HybridApp builds; a missing submodule is skipped rather than failed. + * Keep the HybridApp pin lists used in store builds in sync with pins.json. + * Include each Mobile-Expensify file that exists when this suite runs. + * Missing files are skipped so the suite can run without the submodule. */ const hybridAppPinFiles = [ 'Mobile-Expensify/Android/res/xml/network_security_config_enforce.xml', From ef9607d7712791b566029221e34dde64bb39296d Mon Sep 17 00:00:00 2001 From: staszekscp Date: Tue, 3 Nov 2026 16:12:28 +0100 Subject: [PATCH 3/3] Address review --- ios/CertificatePinning.swift | 35 ++++++++++++++++------- tests/unit/generateCertificatePinsTest.ts | 1 + 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/ios/CertificatePinning.swift b/ios/CertificatePinning.swift index 18e116153bae..e36f61962f04 100644 --- a/ios/CertificatePinning.swift +++ b/ios/CertificatePinning.swift @@ -112,6 +112,10 @@ enum CertificatePinning { TrustKit.initSharedInstance(withConfiguration: trustKitConfig) + // Start the interface monitor at launch so reports include the interface type. Its updates + // arrive on reportingQueue, so a monitor started during a report could not update that report. + _ = pathMonitor + // Reporting rebuilds the validated chain from TrustKit's PEM array and re-runs a trust // evaluation to classify the anchor, so it runs on its own serial queue rather than // TrustKit's default (main). @@ -142,10 +146,11 @@ enum CertificatePinning { private static var occurrenceCounts: [String: Int] = [:] - /// Tags are capped at 200 characters; these ride along as extras instead. + /// Sentry truncates tag values longer than 200 characters, so values that can be longer are sent as extras. private static let extraKeys: Set = [ "certificate_pinning_chain_subjects", "certificate_pinning_chain_fingerprints", + "certificate_pinning_trust_errors", ] private static var currentInterfaceType = "unknown" private static let pathMonitor: NWPathMonitor = { @@ -333,9 +338,12 @@ enum CertificatePinning { var trustResult = SecTrustResultType.invalid SecTrustGetTrustResult(trust, &trustResult) + // On iOS each property is a dictionary such as ["type": "error", "value": ]. The + // kSecPropertyKeyType and kSecPropertyKeyValue constants are unavailable on iOS, so the + // literal keys are used. if let properties = SecTrustCopyProperties(trust) as? [[String: Any]] { - for property in properties { - if let message = property[kSecPropertyTypeError as String] as? String, !message.isEmpty, !errors.contains(message) { + for property in properties where (property["type"] as? String) == (kSecPropertyTypeError as String) { + if let message = property["value"] as? String, !message.isEmpty, !errors.contains(message) { errors.append(message) } } @@ -345,15 +353,19 @@ enum CertificatePinning { // MARK: Network context - /// Reports whether HTTP proxy and PAC settings are enabled, and whether `__SCOPED__` - /// contains an interface whose name starts with utun, tun, tap, ppp, or ipsec. - /// These names suggest a tunnel but do not prove this connection uses a VPN. - /// The proxy host and PAC URL are omitted to protect network details. + /// Reports whether an HTTP or HTTPS proxy or PAC setting is enabled, in the top-level settings or + /// for any interface under `__SCOPED__`, and whether `__SCOPED__` contains an interface whose name + /// starts with utun, tun, tap, ppp, or ipsec. These names suggest a tunnel but do not prove this + /// connection uses a VPN. The proxy host and PAC URL are omitted to protect network details. private static func networkContext() -> [String: Any] { let proxy = CFNetworkCopySystemProxySettings()?.takeRetainedValue() as? [String: Any] ?? [:] - let httpProxy = (proxy[kCFNetworkProxiesHTTPEnable as String] as? Bool) ?? false - let pac = (proxy[kCFNetworkProxiesProxyAutoConfigEnable as String] as? Bool) ?? false let scoped = proxy["__SCOPED__"] as? [String: Any] ?? [:] + // A proxy configured for one Wi-Fi network or VPN can appear only in that interface's scoped entry. + let settings = [proxy] + scoped.values.compactMap { $0 as? [String: Any] } + // kCFNetworkProxiesHTTPSEnable is unavailable on iOS, but iOS proxy settings can include the key. + let proxyKeys = [kCFNetworkProxiesHTTPEnable as String, "HTTPSEnable"] + let httpProxy = settings.contains { entry in proxyKeys.contains { key in (entry[key] as? Bool) == true } } + let pac = settings.contains { entry in (entry[kCFNetworkProxiesProxyAutoConfigEnable as String] as? Bool) == true } let tunnelPrefixes = ["utun", "tun", "tap", "ppp", "ipsec"] let tunnel = scoped.keys.contains { interface in tunnelPrefixes.contains { interface.hasPrefix($0) } } return [ @@ -363,9 +375,10 @@ enum CertificatePinning { ] } - /// Current interface type, cached from a path monitor started on first use so the lookup stays synchronous. + /// Interface type cached by `pathMonitor`, which `initialize()` starts at launch. The lookup is + /// synchronous and returns "unknown" until the first update. "other" includes virtual networks and + /// interfaces of unknown types; it does not prove that a VPN is in use. private static func interfaceType() -> String { - _ = pathMonitor return currentInterfaceType } } diff --git a/tests/unit/generateCertificatePinsTest.ts b/tests/unit/generateCertificatePinsTest.ts index 16a012951421..978a3c248c90 100644 --- a/tests/unit/generateCertificatePinsTest.ts +++ b/tests/unit/generateCertificatePinsTest.ts @@ -30,6 +30,7 @@ const pinsJsonPath = path.join(repoRoot, 'config/certificatePinning/pins.json'); * Keep the HybridApp pin lists used in store builds in sync with pins.json. * Include each Mobile-Expensify file that exists when this suite runs. * Missing files are skipped so the suite can run without the submodule. + * CI does not check out the submodule, so these files are only checked locally. */ const hybridAppPinFiles = [ 'Mobile-Expensify/Android/res/xml/network_security_config_enforce.xml',