Repository navigation
[Payment due @nyomanjyotisa] Update cert pinning logic - #100965
Conversation
|
@linhvovan29546 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
|
|
cc: @nyomanjyotisa since you've been reviewing #100518 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e819ea708a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Google Trust Services (GTS Root R2 is not pinned: Mozilla removed it from its root store in 2026) | ||
| "sha256/hxqRlPTu1bMS/0DITB1SSu0vd4u/8l8TjPgfaAp63Gc=", // GTS Root R1 |
There was a problem hiding this comment.
Removing this root based on Mozilla's bundle is unsafe for the mobile clients: pinning is evaluated against each device's selected system trust anchor, and those stores can continue to trust GTS Root R2 independently of Mozilla. If Cloudflare serves a GTS R2 chain to such a device, TLS will otherwise validate but the new root-only pins will not match, causing monitor failures now and blocking all affected Expensify requests once enforce mode is enabled. Retain R2 in the native/Cronet pin lists until it is no longer trusted by the supported platform stores.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fair point, on it
| + "C5+lpZ7tcVwmwQIMcRtPbsQtWLABXhQzejna0wHFr8M=", // ISRG Root X1 | ||
| + "diGVwiVYbubAI3RW4hB9xU8e/CH2GnkuvVFZE8zmgzI=", // ISRG Root X2 | ||
| + "brzvtCELCIZUo4sD/qPX0ccRtPsd3DY6RfmxpOU9oB4=", // Let's Encrypt YE1 intermediate (live ECDSA) | ||
| + // Google Trust Services - backup CA | ||
| + "hxqRlPTu1bMS/0DITB1SSu0vd4u/8l8TjPgfaAp63Gc=", // GTS Root R1 | ||
| + "Vfd95BwDeSQo+NUYxVEEIlvkOlWY2SalKK1lPhzOx78=", // GTS Root R2 |
There was a problem hiding this comment.
@staszekscp Could you regenerate this patch? The added R2 entry leaves the hunk count at 212 instead of 213, so patch-package cannot parse it. This also causes the CI installation failure.
| "ISRG_Root_X1|96:BC:EC:06:26:49:76:F3:74:60:77:9A:CF:28:C5:A7:CF:E8:A3:C0:AA:E1:1A:8F:FC:EE:05:C0:BD:DF:08:C6" | ||
| "ISRG_Root_X2|69:72:9B:8E:15:A8:6E:FC:17:7A:57:AF:B7:17:1D:FC:64:AD:D2:8C:2F:CA:8C:F1:50:7E:34:45:3C:CB:14:70" | ||
| "GTS_Root_R1|D9:47:43:2A:BD:E7:B7:FA:90:FC:2E:6B:59:10:1B:12:80:E0:E1:C7:E4:E4:0F:A3:C6:88:7F:FF:57:A7:F4:CF" | ||
| "GTS_Root_R2|8D:25:CD:97:22:9D:BF:70:35:6B:DA:4E:B3:CC:73:40:31:E2:4C:F0:0F:AF:CF:D3:2D:C7:6E:B5:84:1C:7E:A8" |
There was a problem hiding this comment.
@staszekscp The clean-checkout failure returns with R2 restored. The downloaded Mozilla bundle lacks it, and all six new tests fail on Node 26.5.0. Could we retain the pin but provide a fingerprint-verified R2 source that also works with --refresh?
| local untrusted="${tmpdir}/untrusted.pem" | ||
| cat "${tmpdir}"/cert_*.pem > "$untrusted" | ||
|
|
||
| if openssl verify -CAfile "$roots_bundle" -untrusted "$untrusted" "${tmpdir}/cert_1.pem" >/dev/null 2>&1; then |
There was a problem hiding this comment.
@staszekscp Could we restrict verification to the supplied roots? With an unpinned test CA in OpenSSL’s default trust directory, --verify reports “chain anchors at a pinned root.” Disabling the default trust sources correctly rejects the same chain.
|
The suggestions have been applied! |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid-pr100965.mp4Android: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
Hey @nyomanjyotisa! 👋🏻 How is the review going? |
|
@staszekscp I haven’t found any further blocking code issues. Could you help trigger Android and iOS ad-hoc builds so I can finish testing and complete the reviewer checklist? |
|
@mountiny @cristipaval Hey guys! Could you run an ad-hoc standalone build for testing purposes, please? 🙏🏻 |
|
🚧 cristipaval has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
nyomanjyotisa
left a comment
nyomanjyotisa
left a comment
There was a problem hiding this comment.
LGTM! Internal/QA coverage is still needed for the iOS release build (my device isn’t provisioned).
melvin-bot
Bot
changed the title
|
🎯 @nyomanjyotisa, thanks for reviewing and testing this PR! 🎉 A payment issue will be created for your review once this PR is deployed to production. If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created. |
|
@staszekscp could you please resolve the conflicts? |
|
Resolved! |
|
🚧 cristipaval has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/cristipaval in version: 9.4.92-0 🚀
|
|
@staszekscp can you please clear the steps? Thank you
|
|
Hey @izarutskaya! Maybe I should have been clearer here - what I mean by "no certificate errors" is just that the content loads as usual (the change is supposed to block network request from unauthorized sources, but if we use the app normally, there should be no change, and all content should load correctly as on other PRs) |
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.4.92-8 🚀
Bundle Size Analysis (Sentry): |
|
🤖 Payment issue created: #102151 |
5 participants
Explanation of Change
Switches NewDot's certificate pins from leaf + issuing intermediate to root CA only.
Leaf/intermediate pins have already broken twice in production — Cloudflare rotated the www/secure edge cert from Let's Encrypt to Google Trust Services (2026-07-07), and Amazon rotated CloudFront's intermediate from RSA 2048 M01 to M04 (2026-07). Leaves are re-keyed on every renewal and every CA issues from a rotating pool of intermediates, so roots are the only durable pin target. Pinning roots survives leaf rotation, intermediate rotation, and (since all three Cloudflare CAs are pinned) a CA switch, without an emergency release.
Pins are now:
Pinned hosts are unchanged: www, secure, new, integrations, travel (.expensify.com), their staging.* counterparts, and d2k5nsl2zxldvw.cloudfront.net.
Updated across all pin targets — config/certificatePinning/pins.json (canonical), CertificatePinning.kt (OkHttp), CertificatePinning.swift (TrustKit), the react-native-nitro-fetch Cronet patch, and network_security_config_enforce.xml.
Two supporting changes:
anchoredChain() in CertificatePinning.kt — a TLS server never sends its root, so the OkHttp and HttpsURLConnection monitors now rebuild the validated chain up to its trust anchor (X509TrustManagerExtensions.checkServerTrusted) before checking pins. Without it root pins could never match there and every request would report a false mismatch. Android , OkHttp enforce mode, TrustKit and Cronet include the anchor natively.
scripts/generateCertificatePins.sh — for the same reason, roots can't be read off a live handshake, so the script now downloads the Mozilla CA bundle and verifies each root against SHA-256 fingerprints committed in its ROOT_MANIFEST (the fingerprints, not the download, are the source of trust). Adds --verify (confirms each live chain anchors at a pinned root), --refresh and --android, plus ROOTS_BUNDLE= for offline/CI use. The root cache is gitignored.
Still monitor-only (enforcePinning = false everywhere): mismatches are reported to Sentry, connections are not blocked. The root pin-set for Android lives in network_security_config_enforce.xml, ready to copy over when we flip to enforce.
No JS or UI changes.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/633469
PROPOSAL:
MOBILE-EXPENSIFY: https://github.com/Expensify/Mobile-Expensify/pull/14106
Tests
Run a release build on Android and iOS (pinning is a no-op in debug). Throughout, check Sentry for certificate_pinning_* events — there should be none.
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2026-09-09.at.14.15.51.mov
Android: mWeb Chrome
iOS: Native
Simulator.Screen.Recording.-.iPhone.17.Pro.-.2026-09-09.at.14.05.34.mov
iOS: mWeb Safari
MacOS: Chrome / Safari