feat: Public key pinning (SPKI) support - #96
Open
TseoH wants to merge 9 commits into
Open
Conversation
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- Android: safe-cast method channel arguments in the public key handlers and return a Params incorrect error instead of crashing on malformed input - Android: force Connection: close when allowCache=false so the check observes a fresh handshake despite keep-alive pooling, and release the connection in a finally block on all paths - iOS: remove unused CommonCrypto import from PublicKeyPinningTrustEvaluator - Dart: strip all whitespace in pin normalization, not just the ends - Example: define the global iOS platform (13.0) in the Podfile - README: document that the second certificate of a served chain is only the intermediate for chains of 3+, add code fence languages Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVSg1UgxvaJQxCJJbnPSKL
feat: add support for spki public key pinning
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
TseoH
marked this pull request as ready for review
July 29, 2026 11:58
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We were previously using
http_certificate_pinningwith fingerprint pinning, but we now have a case where we need to pin the public key instead of the whole certificate hash.This PR adds public key pinning (SPKI) as a separate, opt-in feature. Pins are base64-encoded SHA-256 hashes of the certificate's SubjectPublicKeyInfo.
While checking the existing issues, I came across #54 and #78 and saw your concerns about this kind of implementation (in short: pinning the key of a shared public CA would match any certificate that CA issues, and widening the existing check would silently weaken current users' pinning). These concerns are legitimate and I understand them well.
I still think a flexible approach is the right one: end users should be aware of the implications, and certificate pinning is not the kind of feature that can be implemented on the fly. This PR treats it that way. The pinned chain position is an explicit choice per call, nothing is widened silently, and the README documents the trade-offs so the decision is an informed one. And to make it clear: the current
check(through cert fingerprint) remains unchanged, independent and completely separated from the public key pinning concern, giving anyone the possibility to go with the approach that fits their use case better.What changed:
checkPublicKeys(leaf pins with intermediate fallback), plus the position-specificcheckLeaf,checkIntermediateandcheckRoot. Every method accepts anallowCacheflag (defaulttrue), useful to disable while testing pin changesPublicKeyPinningTrustEvaluator, an AlamofireServerTrustEvaluatingimplementation following the same pattern as the existing fingerprint evaluator. It takes pins per chain position, requires standard X.509 evaluation to pass first, and rebuilds the SPKI ASN.1 structure before hashing (RSA 2048/4096, ECDSA P-256/P-384), so hashes match the standard openssl pipelinePublicKeyPinningChecker, which wraps the evaluator in a configured session that fails closed (allHostsMustBeEvaluated: true), does not follow redirects, and maps failures to distinct error codes. Compatible with the package's iOS 12 targetPublicKey.getEncoded()(already DER SubjectPublicKeyInfo), with chain position logic mirroring iOS.allowCachemaps touseCaches, plusConnection: closewhen disabled so the check observes a fresh handshakeget_public_key_pins.sh, a helper script that prints the pin of every certificate in a server's chain, labeled by positioncheckRootcompares against the last certificate the server sent (some servers omit the root), and on iOS the OS TLS session cache can resume a recent session without re-triggering the pin checkTested:
allowCacheforwarding, 11 passingflutter build iosandflutter build apkboth build cleanNo breaking changes: the existing
checkAPI, its method channel contract and its behavior are completely unchanged.