Skip to content

fix(storage): keep RSA key for PKCS1 migration on padding mismatch - #1110

Merged
utkrishtsahu merged 1 commit into
mainfrom
fix/1696-keep-rsa-key-for-pkcs1-migration-4x
Oct 26, 2026
Merged

utkrishtsahu merged 1 commit into
mainfrom
fix/1696-keep-rsa-key-for-pkcs1-migration-4x

Conversation

@utkrishtsahu

@utkrishtsahu utkrishtsahu commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Changes

Cherry-pick of #1107 (3.x) / #1108 (v5_development) to main (4.x). The code change is identical.

Users who saved credentials with Auth0.Android 3.12.1 or older (incl. 2.x) are logged out on the first launch after updating to 4.2.0 (and 3.21.1 / 3.21.2).

Root cause. Those versions created a PKCS1-only RSA key. Since #1092 (4.x port of #1048), when RSADecrypt gets InvalidKeyException (Keystore INCOMPATIBLE_PADDING_MODE), it deletes the RSA and AES keys before throwing. Its only caller, tryRecoverCurrentAESKey, then runs attemptPKCS1Migration, which needs that RSA key. The key is already gone (No RSA key found for migration), so new keys are generated and the stored credentials are cleared.

The ProviderException branch has a smaller version of the same problem: deleteAESKeys() also removes the legacy IV. Credentials saved by SDK 2.x use the legacy format, so on devices that report the padding mismatch as ProviderException, the AES key migrates but the credentials can't be decrypted afterwards.

Fix (CryptoUtil.RSADecrypt only):

  • InvalidKeyException | InvalidAlgorithmParameterException: no longer deletes keys; still throws CryptoException (never IncompatibleDeviceException, as introduced in fix(storage): authorize MGF1 digest to prevent INCOMPATIBLE_DEVICE on Keystore2 #1092).
  • ProviderException: no longer calls deleteAESKeys(); still throws CryptoException.
  • Comments and log messages updated to cover both causes (legacy PKCS1 key, MGF1 digest mismatch).

Recovery stays in tryRecoverCurrentAESKey: it tries the PKCS1 migration first, and if that fails it deletes both keys and a new key pair is generated.

Case Before After
Legacy PKCS1 key (2.x / ≤ 3.12.x) Keys deleted → logged out Migrated to OAEP, stays logged in
PKCS1 key + ProviderException, 2.x data Legacy IV deleted → logged out IV kept, stays logged in
OAEP key with MGF1 digest mismatch (#1016 / #1092) Keys regenerated Unchanged: PKCS1 attempt fails, keys regenerated (deleted one step later)
OAEP key (3.13+), fresh install Works Unchanged

No public API changes.

References

Testing

Unit tests (CryptoUtilTest): same tests as #1108. The new tests run the real RSADecrypt before the migration, with a KeyStore mock where a deleted key is really gone.

Test Type Asserts
shouldKeepKeysAndThrowCryptoExceptionOnInvalidKeyExceptionWhenTryingToRSADecrypt Updated CryptoException, no key deleted
shouldKeepKeysAndThrowCryptoExceptionOnInvalidAlgorithmParameterExceptionWhenTryingToRSADecrypt Updated CryptoException, no key deleted
shouldWrapProviderExceptionFromCipherInitInRSADecryptAsCryptoException Updated CryptoException, storage untouched
shouldMigratePKCS1KeyWhenRealRSADecryptThrowsInvalidKeyException New Original AES key returned and re-wrapped with OAEP
shouldMigratePKCS1KeyAndKeepLegacyIVWhenRealRSADecryptThrowsProviderException New Original AES key returned, legacy IV not removed
shouldMigratePKCS1KeyWhenEncryptIsTheFirstOperationAfterUpgrade New encrypt() uses the migrated key, no new AES key
shouldRegenerateKeysWithoutIncompatibleDeviceWhenMgf1DigestIsRejected New (guards #1092) Keys deleted, new AES key stored, no IncompatibleDeviceException

./gradlew testReleaseUnitTest lintRelease on main + this change: all unit tests pass (1,491; CryptoUtilTest 87), lint 0 errors.

Device (Pixel 10, Android 17 / API 37, TEE-backed Keystore2). Sample app using SecureCredentialsManager with a real login against a test tenant. Every upgrade is an in-place update (adb install -r, app data kept), followed by a cold start that calls getCredentials().

# Scenario Steps Key logs Result
1 2.10.2 → 4.2.0 + this fix Log in on 2.10.2 (key [PKCS1Padding], legacy key_iv), update, launch RSA key rejected … (legacy PKCS1 key or MGF1 digest mismatch) → PKCS1 migration successful → AES key re-encrypted with OAEP and stored ✅ Stays logged in; key now [OAEPPadding], key_iv kept; cold start: no migration, success

The full device matrix (3.12.0 → fix, MGF1 mismatch on 3.21.1 vs fix, 3.21.0 OAEP key → fix, credentials already cleared → fix) ran against the 3.x build of the same CryptoUtil change; see #1107.

  • This change adds unit test coverage
  • This change adds integration test coverage
  • This change has been tested on the latest version of the platform/language or why not

Checklist

Summary by CodeRabbit

  • Bug Fixes
    • RSA decryption failures no longer immediately remove stored keys or AES data, helping preserve credentials for recovery.
    • Improved compatibility with legacy credentials: older encryption formats can be migrated during decryption or encryption, while retaining the stored initialization data when needed.
    • Keys are regenerated when both supported decryption methods fail.

RSADecrypt deleted the RSA/AES keys on InvalidKeyException (and the AES key + legacy IV on ProviderException) before tryRecoverCurrentAESKey could run the PKCS1 migration, logging out users upgrading from SDK <= 3.12.x / 2.x. RSADecrypt now only throws CryptoException; recovery and cleanup stay in tryRecoverCurrentAESKey, so the MGF1 regeneration from #1048 is unchanged.

Fixes auth0/react-native-auth0#1696

Co-Authored-By: Claude
(cherry picked from commit 15b91f1)
@utkrishtsahu
utkrishtsahu requested a review from a team as a code owner October 26, 2026 05:24
@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 599cf656-94f8-4d3d-9192-ec9bb9d94b3f
Commits

Reviewing files that changed from the base of the PR and between d28b1ea and ca52e2f.

Files selected for processing: 2
  • auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java
  • auth0/src/test/java/com/auth0/android/authentication/storage/CryptoUtilTest.java

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

Walkthrough

RSA decryption failures now preserve existing RSA and AES key data. Tests cover PKCS1 recovery after OAEP failures, migration of recovered keys, IV retention, fallback key regeneration, and encryption as the first operation after an upgrade.

Changes: RSA decryption recovery

Layer / File(s) Summary
Preserve key data on decryption errors
auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java, auth0/src/test/java/com/auth0/android/authentication/storage/CryptoUtilTest.java
RSA decryption initialization failures no longer delete RSA or AES data. Tests verify that KeyStore entries and stored values remain present.
Cover PKCS1 migration and fallback
auth0/src/test/java/com/auth0/android/authentication/storage/CryptoUtilTest.java
Tests cover PKCS1 recovery after OAEP failure, OAEP key migration, IV retention, fallback key regeneration, and encryption as the first post-upgrade operation.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant RSADecrypt
  participant AndroidKeystore
  participant AESStorage
  RSADecrypt->>AndroidKeystore: Attempt OAEP decryption
  AndroidKeystore-->>RSADecrypt: Return initialization failure
  RSADecrypt->>AndroidKeystore: Attempt PKCS1 decryption
  AndroidKeystore-->>RSADecrypt: Return recovered AES key
  RSADecrypt->>AndroidKeystore: Encrypt recovered key with OAEP
  RSADecrypt->>AESStorage: Store migrated key and retain IV
Loading

Possibly related issues

  • auth0/react-native-auth0 issue 1696: The report attributes logout after upgrades to OAEP decryption removing the PKCS1 key before migration. This change retains key data so PKCS1 recovery can proceed.

Suggested reviewers: pmathew92

Merge Risk: Minimal · up to ca52e

The key-retention change preserves the data needed for credential migration, and the inspected failure path retains its cleanup fallback. No identified issue blocks merging after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: retaining the RSA key to support PKCS1 migration after a padding mismatch.
Linked Issues check Passed No active directly linked issue targets remain. Issue #1092 is closed and supplies historical context only. Therefore, this pull request has no linked-issue coding requirements to satisfy.
Out of Scope Changes check Passed The diff changes only CryptoUtil.RSADecrypt and CryptoUtilTest. The production change preserves keys for the PKCS1 migration and keeps CryptoException behavior. The tests cover migration, legacy…
Full details: Out of Scope Changes check

Explanation

The diff changes only CryptoUtil.RSADecrypt and CryptoUtilTest. The production change preserves keys for the PKCS1 migration and keeps CryptoException behavior. The tests cover migration, legacy IV retention, regeneration, and encryption-first recovery. These changes match the current pull request intent and show no unrelated scope.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@utkrishtsahu
utkrishtsahu merged commit f963502 into main Oct 26, 2026
7 checks passed
@utkrishtsahu
utkrishtsahu deleted the fix/1696-keep-rsa-key-for-pkcs1-migration-4x branch October 26, 2026 06:39
@utkrishtsahu utkrishtsahu mentioned this pull request Oct 26, 2026
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