Conversation
This branch has not been deployed
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.
This PR installs the bundled BouncyCastle security provider as the first step of
App.onCreate, so swapping security providers can no longer race paykit's first TLS connection at start-up.I found this while testing #1399. It is older than that PR: master fails at the same rate.
Description
Crypto.installSecurityProvider()and calls it beforesuper.onCreate()inApp, where Hilt has not built anything yet, so the swap finishes before any service can open a connection.Crypto's constructor still calls it, and that second call finds BouncyCastle in place and does nothingRoot cause
Crypto'sinitruns on the main thread when MainActivity's ViewModels are created. It removed Android's built-in "BC" provider and only then constructedBouncyCastleProvider(), which took 0.8–1.1 s on the emulator. During that time no provider offered "BKS".App.onCreate→super.onCreate()injectsPubkyAuthHandlerRegistrar→PubkyRepo.init→PaykitSdkService.initialize(), which launchesrepublishIdentityIfNeeded(). That makes paykit's first HTTPS call on an IO thread 1.6–2.8 s after process start.rustls-platform-verifier. The static initialiser oforg.rustls.platformverifier.CertificateVerifierstarts withKeyStore.getInstance(KeyStore.getDefaultType()), and the default type on Android is "BKS". Inside the window it throwsKeyStoreException: BKS not found, the class is marked as failed for the whole process, and every later paykit certificate check throwsNoClassDefFoundErroruntil the app restarts.Measurements
These were online cold launches on an Android emulator while signed in to a Pubky profile. A launch counts as broken when logcat shows
BKS not foundor theNoClassDefFoundError.Crypto.ktis unchanged andApponly gainedSubscriptionClockOffsetSync, but the change has not been re-measured on that base.App.onCreate. Push starts already did, becauseFcmServiceinjectsCrypto.Security
Appcovers all of them.Out of Scope
Crypto.kt: putting the full BouncyCastle first in the global provider list changes JCA choices for the whole process. For example, paykit'sCertPathValidator.getInstance("PKIX")resolves to BouncyCastle after the swap. The cleaner long-term fix is a privateBouncyCastleProviderinstance passed explicitly toCrypto'sgetInstancecalls for key generation, key factories and ECDH. TheCiphercalls should get it too: without BouncyCastle first they would resolve to Conscrypt, which might reject Blocktank's 16-byte GCM IVs. That needs its own tests, so it is left for a follow-up.Design
N/A — no UI changes.
Preview
N/A
QA Notes
Journeys
N/A — not drivable; see Manual Tests.
Manual Tests
BKS not foundand noCertificateVerifiererror on any launch — repeated cold launches checked through logcat are not in CapabilitiesAutomated Checks
CryptoTest.kt—installSecurityProvideradds BouncyCastle when no BC provider exists, replaces an outdated BC provider at position 1, and leaves the installed provider in place on later calls, including the one fromCrypto's constructor