Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
utkrishtsahu merged 1 commit into from
Oct 26, 2026
+184 −29
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 17 additions & 15 deletions auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,20 +352,22 @@ byte[] RSADecrypt(byte[] encryptedInput) throws IncompatibleDeviceException, Cry
* - InvalidAlgorithmParameterException:
* Thrown if the OAEP parameters are invalid or unsupported for this key.
*
* This is NOT a device-level incompatibility -- the key can be deleted and regenerated
* with the MGF1 digest authorized. Delete the RSA key (and the AES key it protected,
* which is now unrecoverable) and wrap as CryptoException so the caller falls through to
* key regeneration instead of permanently blocking the user with INCOMPATIBLE_DEVICE.
* If the device genuinely cannot generate a compatible key, the subsequent
* getRSAKeyEntry() will throw IncompatibleDeviceException, preserving the terminal path.
* A legacy RSA key authorized only for PKCS1 padding (generated by SDK versions before
* the OAEP switch) is rejected here in the same way.
*
* This is NOT a device-level incompatibility, so wrap as CryptoException instead of
* permanently blocking the user with INCOMPATIBLE_DEVICE. Do not delete any key here:
* tryRecoverCurrentAESKey() first attempts the PKCS1 migration, which needs the existing
* RSA key and stored AES key. If the migration fails (e.g. MGF1 digest mismatch), it
* deletes both keys and a new RSA key pair is generated with the MGF1 digest authorized.
* If the device genuinely cannot generate a compatible key, getRSAKeyEntry() throws
* IncompatibleDeviceException, preserving the terminal path.
*
* Read more in https://developer.android.com/reference/javax/crypto/Cipher
*/
Log.e(TAG, "RSA key rejected the cipher parameters (likely MGF1 digest mismatch on Keystore2). Recreating key.", e);
deleteRSAKeys();
deleteAESKeys();
Log.e(TAG, "RSA key rejected the cipher parameters (legacy PKCS1 key or MGF1 digest mismatch).", e);
throw new CryptoException(
"The RSA key's authorized parameters are incompatible with the current cipher. The keys have been recreated; please retry.", e);
"The RSA key's authorized parameters are incompatible with the current cipher.", e);
} catch (NoSuchAlgorithmException | NoSuchPaddingException e) {
/*
* - NoSuchPaddingException:
Expand All @@ -390,13 +392,13 @@ byte[] RSADecrypt(byte[] encryptedInput) throws IncompatibleDeviceException, Cry
* initialised with an OAEPWithSHA-1AndMGF1Padding cipher. On API 23-30 the
* same condition surfaces as InvalidKeyException.
*
* This is NOT a device-level incompatibility -- the key can be deleted and
* regenerated with the correct padding. Wrapping as CryptoException (rather
* than IncompatibleDeviceException) ensures the caller falls through to key
* cleanup and regeneration instead of permanently blocking the user.
* This is NOT a device-level incompatibility. Wrapping as CryptoException (rather
* than IncompatibleDeviceException) lets tryRecoverCurrentAESKey() attempt the
* PKCS1 migration first and fall back to key cleanup and regeneration, instead of
* permanently blocking the user. Keys are not deleted here: the migration needs the
* stored IV to read credentials saved in the legacy format (SDK 2.x).
*/
Log.e(TAG, "RSA key padding mismatch detected (Android 12+ Keystore2).", e);
deleteAESKeys();
throw new CryptoException(
"The RSA key's padding mode is incompatible with the current cipher.", e);
} catch (IllegalArgumentException | IllegalBlockSizeException | BadPaddingException e) {
Expand Down
181 changes: 167 additions & 14 deletions auth0/src/test/java/com/auth0/android/authentication/storage/CryptoUtilTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,8 +705,8 @@ public void shouldRSADecryptData() throws Exception {
}

@Test
public void shouldRecreateKeysAndThrowCryptoExceptionOnInvalidKeyExceptionWhenTryingToRSADecrypt() throws Exception {
Assert.assertThrows("The RSA key's authorized parameters are incompatible with the current cipher. The keys have been recreated; please retry.", CryptoException.class, () -> {
public void shouldKeepKeysAndThrowCryptoExceptionOnInvalidKeyExceptionWhenTryingToRSADecrypt() throws Exception {
Assert.assertThrows("The RSA key's authorized parameters are incompatible with the current cipher.", CryptoException.class, () -> {
byte[] sampleBytes = new byte[0];
PrivateKey privateKey = Mockito.mock(PrivateKey.class);
KeyStore.PrivateKeyEntry privateKeyEntry = Mockito.mock(KeyStore.PrivateKeyEntry.class);
Expand All @@ -718,17 +718,14 @@ public void shouldRecreateKeysAndThrowCryptoExceptionOnInvalidKeyExceptionWhenTr
cryptoUtil.RSADecrypt(sampleBytes);
});

Mockito.verify(keyStore).deleteEntry(KEY_ALIAS);
Mockito.verify(keyStore).deleteEntry(OLD_KEY_ALIAS);
Mockito.verify(storage).remove(KEY_ALIAS);
Mockito.verify(storage).remove(KEY_ALIAS + "_iv");
Mockito.verify(storage).remove(OLD_KEY_ALIAS);
Mockito.verify(storage).remove(OLD_KEY_ALIAS + "_iv");
// Keys are kept so that tryRecoverCurrentAESKey can attempt the PKCS1 migration first
Mockito.verify(keyStore, never()).deleteEntry(anyString());
Mockito.verify(storage, never()).remove(anyString());
}

@Test
public void shouldRecreateKeysAndThrowCryptoExceptionOnInvalidAlgorithmParameterExceptionWhenTryingToRSADecrypt() throws Exception {
Assert.assertThrows("The RSA key's authorized parameters are incompatible with the current cipher. The keys have been recreated; please retry.", CryptoException.class, () -> {
public void shouldKeepKeysAndThrowCryptoExceptionOnInvalidAlgorithmParameterExceptionWhenTryingToRSADecrypt() throws Exception {
Assert.assertThrows("The RSA key's authorized parameters are incompatible with the current cipher.", CryptoException.class, () -> {
byte[] sampleBytes = new byte[0];
PrivateKey privateKey = Mockito.mock(PrivateKey.class);
KeyStore.PrivateKeyEntry privateKeyEntry = Mockito.mock(KeyStore.PrivateKeyEntry.class);
Expand All @@ -740,10 +737,8 @@ public void shouldRecreateKeysAndThrowCryptoExceptionOnInvalidAlgorithmParameter
cryptoUtil.RSADecrypt(sampleBytes);
});

Mockito.verify(keyStore).deleteEntry(KEY_ALIAS);
Mockito.verify(keyStore).deleteEntry(OLD_KEY_ALIAS);
Mockito.verify(storage).remove(KEY_ALIAS);
Mockito.verify(storage).remove(OLD_KEY_ALIAS);
Mockito.verify(keyStore, never()).deleteEntry(anyString());
Mockito.verify(storage, never()).remove(anyString());
}

@Test
Expand Down Expand Up @@ -1460,6 +1455,26 @@ public void shouldThrowButNotDeleteAESKeysOnIllegalBlockSizeExceptionWhenTryingT
}


/**
* Stores an RSA key at KEY_ALIAS that stays in the KeyStore until deleteEntry(KEY_ALIAS) is
* called, as on a real device. Lets the real RSADecrypt run before the PKCS1 migration.
*/
private PrivateKey givenStatefulRSAKey(CryptoUtil cryptoUtil) throws Exception {
PrivateKey privateKey = mock(PrivateKey.class);
KeyStore.PrivateKeyEntry entry = mock(KeyStore.PrivateKeyEntry.class);
when(entry.getPrivateKey()).thenReturn(privateKey);
final boolean[] present = {true};
when(keyStore.containsAlias(KEY_ALIAS)).thenAnswer(invocation -> present[0]);
Mockito.doAnswer(invocation -> {
present[0] = false;
return null;
}).when(keyStore).deleteEntry(KEY_ALIAS);
when(keyStore.getEntry(eq(KEY_ALIAS), nullable(KeyStore.ProtectionParameter.class)))
.thenAnswer(invocation -> present[0] ? entry : null);
doReturn(entry).when(cryptoUtil).getRSAKeyEntry();
return privateKey;
}

@Test
public void shouldDetectAndMigratePKCS1KeyToOAEP() throws Exception {
CryptoUtil cryptoUtil = newCryptoUtilSpy();
Expand Down Expand Up @@ -1699,6 +1714,9 @@ public void shouldWrapProviderExceptionFromCipherInitInRSADecryptAsCryptoExcepti

cryptoUtil.RSADecrypt(new byte[]{1, 2, 3});
});

// The stored AES key and legacy IV are kept for the PKCS1 migration
Mockito.verify(storage, never()).remove(anyString());
}

@Test
Expand Down Expand Up @@ -1755,6 +1773,141 @@ public void shouldTriggerPKCS1MigrationWhenRSADecryptThrowsProviderException() t
Mockito.verify(keyStore).deleteEntry(KEY_ALIAS);
}

/*
* The tests below run the real RSADecrypt (no stub) before the PKCS1 migration, as on a device:
* a PKCS1-only RSA key from an older SDK version rejects the OAEP cipher.
*/

@Test
public void shouldMigratePKCS1KeyWhenRealRSADecryptThrowsInvalidKeyException() throws Exception {
CryptoUtil cryptoUtil = newCryptoUtilSpy();

byte[] aesKeyBytes = new byte[32];
Arrays.fill(aesKeyBytes, (byte) 0xCD);
byte[] encryptedAESPKCS1 = new byte[]{10, 11, 12, 13};
byte[] reEncryptedOAEP = new byte[]{20, 21, 22, 23};
String encodedPKCS1 = "pkcs1_encoded";
String encodedOAEP = "oaep_encoded";

when(storage.retrieveString(KEY_ALIAS)).thenReturn(encodedPKCS1);
base64Mock.when(() -> Base64.decode(encodedPKCS1, Base64.DEFAULT)).thenReturn(encryptedAESPKCS1);
base64Mock.when(() -> Base64.encode(reEncryptedOAEP, Base64.DEFAULT))
.thenReturn(encodedOAEP.getBytes(StandardCharsets.UTF_8));

PrivateKey privateKey = givenStatefulRSAKey(cryptoUtil);
doThrow(new InvalidKeyException("Incompatible padding mode"))
.when(rsaOaepCipher).init(eq(Cipher.DECRYPT_MODE), eq(privateKey), any(AlgorithmParameterSpec.class));
when(rsaPkcs1Cipher.doFinal(encryptedAESPKCS1)).thenReturn(aesKeyBytes);
doReturn(reEncryptedOAEP).when(cryptoUtil).RSAEncrypt(aesKeyBytes);

byte[] result = cryptoUtil.getAESKey();

assertThat(result, is(aesKeyBytes));
Mockito.verify(rsaPkcs1Cipher).init(Cipher.DECRYPT_MODE, privateKey);
Mockito.verify(storage).store(KEY_ALIAS, encodedOAEP);
Mockito.verify(keyGenerator, never()).generateKey();
}

@Test
public void shouldMigratePKCS1KeyAndKeepLegacyIVWhenRealRSADecryptThrowsProviderException() throws Exception {
CryptoUtil cryptoUtil = newCryptoUtilSpy();

byte[] aesKeyBytes = new byte[32];
Arrays.fill(aesKeyBytes, (byte) 0xCD);
byte[] encryptedAESPKCS1 = new byte[]{10, 11, 12, 13};
byte[] reEncryptedOAEP = new byte[]{20, 21, 22, 23};
String encodedPKCS1 = "pkcs1_encoded";
String encodedOAEP = "oaep_encoded";

when(storage.retrieveString(KEY_ALIAS)).thenReturn(encodedPKCS1);
// Credentials saved by SDK 2.x use the legacy format, whose IV is stored separately
when(storage.retrieveString(KEY_ALIAS + "_iv")).thenReturn("legacy_iv");
base64Mock.when(() -> Base64.decode(encodedPKCS1, Base64.DEFAULT)).thenReturn(encryptedAESPKCS1);
base64Mock.when(() -> Base64.encode(reEncryptedOAEP, Base64.DEFAULT))
.thenReturn(encodedOAEP.getBytes(StandardCharsets.UTF_8));

PrivateKey privateKey = givenStatefulRSAKey(cryptoUtil);
doThrow(new ProviderException(new KeyStoreException("Incompatible padding mode")))
.when(rsaOaepCipher).init(eq(Cipher.DECRYPT_MODE), eq(privateKey), any(AlgorithmParameterSpec.class));
when(rsaPkcs1Cipher.doFinal(encryptedAESPKCS1)).thenReturn(aesKeyBytes);
doReturn(reEncryptedOAEP).when(cryptoUtil).RSAEncrypt(aesKeyBytes);

byte[] result = cryptoUtil.getAESKey();

assertThat(result, is(aesKeyBytes));
Mockito.verify(storage).store(KEY_ALIAS, encodedOAEP);
Mockito.verify(storage, never()).remove(KEY_ALIAS + "_iv");
Mockito.verify(storage, never()).remove(OLD_KEY_ALIAS + "_iv");
}

@Test
public void shouldRegenerateKeysWithoutIncompatibleDeviceWhenMgf1DigestIsRejected() throws Exception {
CryptoUtil cryptoUtil = newCryptoUtilSpy();

// AES key wrapped by an OAEP RSA key whose authorized MGF1 digests exclude SHA-1:
// both the OAEP and the PKCS1 cipher are rejected, so the key can't be recovered
byte[] encryptedAESOAEP = new byte[]{10, 11, 12, 13};
String encodedOAEP = "oaep_encoded";
when(storage.retrieveString(KEY_ALIAS)).thenReturn(encodedOAEP);
when(storage.retrieveString(OLD_KEY_ALIAS)).thenReturn(null);
base64Mock.when(() -> Base64.decode(encodedOAEP, Base64.DEFAULT)).thenReturn(encryptedAESOAEP);

PrivateKey privateKey = givenStatefulRSAKey(cryptoUtil);
doThrow(new InvalidKeyException("Incompatible MGF digest"))
.when(rsaOaepCipher).init(eq(Cipher.DECRYPT_MODE), eq(privateKey), any(AlgorithmParameterSpec.class));
doThrow(new InvalidKeyException("Incompatible padding mode"))
.when(rsaPkcs1Cipher).init(Cipher.DECRYPT_MODE, privateKey);

byte[] newAESKey = new byte[32];
Arrays.fill(newAESKey, (byte) 0xEE);
SecretKey newSecretKey = mock(SecretKey.class);
when(newSecretKey.getEncoded()).thenReturn(newAESKey);
when(keyGenerator.generateKey()).thenReturn(newSecretKey);
byte[] encryptedNewAES = new byte[]{40, 41, 42};
doReturn(encryptedNewAES).when(cryptoUtil).RSAEncrypt(newAESKey);
base64Mock.when(() -> Base64.encode(encryptedNewAES, Base64.DEFAULT))
.thenReturn("fresh_key".getBytes(StandardCharsets.UTF_8));

// Must not throw IncompatibleDeviceException
byte[] result = cryptoUtil.getAESKey();

assertThat(result, is(newAESKey));
Mockito.verify(keyStore, Mockito.atLeastOnce()).deleteEntry(KEY_ALIAS);
Mockito.verify(storage).store(KEY_ALIAS, "fresh_key");
}

@Test
public void shouldMigratePKCS1KeyWhenEncryptIsTheFirstOperationAfterUpgrade() throws Exception {
CryptoUtil cryptoUtil = newCryptoUtilSpy();

byte[] aesKeyBytes = new byte[32];
Arrays.fill(aesKeyBytes, (byte) 0xCD);
byte[] encryptedAESPKCS1 = new byte[]{10, 11, 12, 13};
byte[] reEncryptedOAEP = new byte[]{20, 21, 22, 23};
String encodedPKCS1 = "pkcs1_encoded";

when(storage.retrieveString(KEY_ALIAS)).thenReturn(encodedPKCS1);
base64Mock.when(() -> Base64.decode(encodedPKCS1, Base64.DEFAULT)).thenReturn(encryptedAESPKCS1);
base64Mock.when(() -> Base64.encode(reEncryptedOAEP, Base64.DEFAULT))
.thenReturn("oaep_encoded".getBytes(StandardCharsets.UTF_8));

PrivateKey privateKey = givenStatefulRSAKey(cryptoUtil);
doThrow(new InvalidKeyException("Incompatible padding mode"))
.when(rsaOaepCipher).init(eq(Cipher.DECRYPT_MODE), eq(privateKey), any(AlgorithmParameterSpec.class));
when(rsaPkcs1Cipher.doFinal(encryptedAESPKCS1)).thenReturn(aesKeyBytes);
doReturn(reEncryptedOAEP).when(cryptoUtil).RSAEncrypt(aesKeyBytes);

when(aesCipher.doFinal(any(byte[].class))).thenReturn(new byte[]{50, 51, 52});
when(aesCipher.getIV()).thenReturn(new byte[12]);

cryptoUtil.encrypt(new byte[]{1, 2, 3});

ArgumentCaptor<SecretKey> keyCaptor = ArgumentCaptor.forClass(SecretKey.class);
Mockito.verify(aesCipher).init(eq(Cipher.ENCRYPT_MODE), keyCaptor.capture());
assertThat(keyCaptor.getValue().getEncoded(), is(aesKeyBytes));
Mockito.verify(keyGenerator, never()).generateKey();
}

@Test
public void shouldDeleteOldRSAKeyBeforeReEncryptingInTryMigrateLegacyAESKey() throws Exception {
CryptoUtil cryptoUtil = newCryptoUtilSpy();
Expand Down
Loading