Skip to content

fix: install security provider before tls use - #1416

Draft
Jasonvdb wants to merge 2 commits into
masterfrom
fix/security-provider-startup-race
Draft

Jasonvdb wants to merge 2 commits into
masterfrom
fix/security-provider-startup-race

Conversation

@Jasonvdb

@Jasonvdb Jasonvdb commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

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

  • Fixes paykit refusing every TLS connection for the rest of the session after most cold launches made while signed in to a Pubky profile. The provider swap left a window of about a second in which no provider offered the "BKS" keystore. When paykit's first HTTPS call landed in that window, its certificate verifier class failed to initialise, and Java never retries a class whose initialiser failed
  • Moves the swap into Crypto.installSecurityProvider() and calls it before super.onCreate() in App, 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 nothing
  • Builds the replacement provider before removing the old one, so the gap with no "BC" provider drops from about a second to under 1 ms

Root cause

  • Crypto's init runs on the main thread when MainActivity's ViewModels are created. It removed Android's built-in "BC" provider and only then constructed BouncyCastleProvider(), which took 0.8–1.1 s on the emulator. During that time no provider offered "BKS".
  • Meanwhile App.onCreate → super.onCreate() injects PubkyAuthHandlerRegistrar → PubkyRepo.init → PaykitSdkService.initialize(), which launches republishIdentityIfNeeded(). That makes paykit's first HTTPS call on an IO thread 1.6–2.8 s after process start.
  • Paykit bundles rustls-platform-verifier. The static initialiser of org.rustls.platformverifier.CertificateVerifier starts with KeyStore.getInstance(KeyStore.getDefaultType()), and the default type on Android is "BKS". Inside the window it throws KeyStoreException: BKS not found, the class is marked as failed for the whole process, and every later paykit certificate check throws NoClassDefFoundError until 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 found or the NoClassDefFoundError.

Build Broken launches
master 6ba44a4 9/10
master 6ba44a4 with timing logs 8/8
#1399 13/15
master 6ba44a4 with this change 0/10
  • 0/10 against 9/10 gives Fisher's exact p ≈ 1e-4.
  • All 8 failures in the logged runs fell inside the removal window.
  • With this change the provider is built 1.0–1.4 s after process start, and the removal and insertion happen in the same millisecond.
  • This branch is based on 907f546. There Crypto.kt is unchanged and App only gained SubscriptionClockOffsetSync, but the change has not been re-measured on that base.
  • Time to first frame stayed in the same range: 6.0–7.5 s with the change, against 6.8–8.1 s for the timing-log build. The constructor already ran on the main thread before the first frame; it now just runs earlier.
  • Process starts with no UI and no push, such as a widget refresh or a worker, now also pay that cost in App.onCreate. Push starts already did, because FcmService injects Crypto.

Security

  • Only the order changes. Certificate verification runs the same code with the same trust roots.
  • Today's failure fails closed: TLS is refused, never weakened. This PR deliberately does not catch-and-retry, skip or soften verification, or switch paykit to bundled roots.
  • A guard inside paykit would have to cover every native library that uses the platform verifier. Doing the swap first in App covers 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's CertPathValidator.getInstance("PKIX") resolves to BouncyCastle after the swap. The cleaner long-term fix is a private BouncyCastleProvider instance passed explicitly to Crypto's getInstance calls for key generation, key factories and ECDH. The Cipher calls 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

  • Install, sign in to a Pubky profile, then cold-launch the app 10 times while online → logcat shows no BKS not found and no CertificateVerifier error on any launch — repeated cold launches checked through logcat are not in Capabilities

Automated Checks

  • updated CryptoTest.kt — installSecurityProvider adds 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 from Crypto's constructor

@ovitrif ovitrif added this to the 2.6.0 milestone Oct 2, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants