Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -179,16 +179,33 @@ KeyStore.PrivateKeyEntry getRSAKeyEntry() throws CryptoException, IncompatibleDe
end.add(Calendar.YEAR, 25);
X500Principal principal = new X500Principal("CN=Auth0.Android,O=Auth0");

AlgorithmParameterSpec spec = new KeyGenParameterSpec.Builder(KEY_ALIAS, KeyProperties.PURPOSE_DECRYPT | KeyProperties.PURPOSE_ENCRYPT)
KeyGenParameterSpec.Builder specBuilder = new KeyGenParameterSpec.Builder(KEY_ALIAS, KeyProperties.PURPOSE_DECRYPT | KeyProperties.PURPOSE_ENCRYPT)
.setCertificateSubject(principal)
.setCertificateSerialNumber(BigInteger.ONE)
.setCertificateNotBefore(start.getTime())
.setCertificateNotAfter(end.getTime())
.setKeySize(RSA_KEY_SIZE)
.setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_OAEP)
.setDigests(KeyProperties.DIGEST_SHA1, KeyProperties.DIGEST_SHA256)
.setBlockModes(KeyProperties.BLOCK_MODE_ECB)
.build();
.setBlockModes(KeyProperties.BLOCK_MODE_ECB);

/*
* OAEP uses two independent digests: the OAEP digest (authorized above) and the MGF1
* digest. Our OAEP_SPEC uses MGF1/SHA-1. The setMgf1Digests API only exists on API 35+
* (VANILLA_ICE_CREAM); below that KeyMint defaults the MGF1 digest (historically to
* SHA-1), so leaving it unset worked. On newer Keystore2 hardware the authorized MGF1
* digest set is strictly enforced and its default is vendor-dependent (some default to
* SHA-256 only), which rejects our MGF1/SHA-1 cipher with INCOMPATIBLE_MGF_DIGEST
* ("Incompatible padding mode"). Where the API is available, authorize SHA-1 (used by the
* current cipher) and SHA-256 explicitly so the key no longer depends on the vendor
* default and remains compatible if the cipher moves to MGF1/SHA-256 in the future.
* See OAEP_SPEC.
*/
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.VANILLA_ICE_CREAM) {
specBuilder.setMgf1Digests(KeyProperties.DIGEST_SHA1, KeyProperties.DIGEST_SHA256);
}

AlgorithmParameterSpec spec = specBuilder.build();

KeyPairGenerator generator = KeyPairGenerator.getInstance(ALGORITHM_RSA, ANDROID_KEY_STORE);
generator.initialize(spec);
Expand Down Expand Up @@ -323,18 +340,42 @@ byte[] RSADecrypt(byte[] encryptedInput) throws IncompatibleDeviceException, Cry
Cipher cipher = Cipher.getInstance(RSA_TRANSFORMATION);
cipher.init(Cipher.DECRYPT_MODE, privateKey, OAEP_SPEC);
return cipher.doFinal(encryptedInput);
} catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException
| InvalidAlgorithmParameterException e) {
} catch (InvalidKeyException | InvalidAlgorithmParameterException e) {
/*
* - InvalidKeyException:
* Thrown if the stored key is inappropriate for initializing this cipher. On newer
* Keystore2 hardware (API 33+) an existing RSA key whose authorized MGF1 digest set
* does not include SHA-1 rejects our MGF1/SHA-1 cipher here, surfacing as
* "Keystore operation failed" caused by INCOMPATIBLE_MGF_DIGEST ("Incompatible
* padding mode"). Such a key was generated by an older SDK before the MGF1 digest
* was authorized explicitly (see getRSAKeyEntry).
* - 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.
*
* 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();
throw new CryptoException(
"The RSA key's authorized parameters are incompatible with the current cipher. The keys have been recreated; please retry.", e);
} catch (NoSuchAlgorithmException | NoSuchPaddingException e) {
/*
* - NoSuchPaddingException:
* Thrown if PKCS1Padding is not available. Was introduced in API 1.
* - NoSuchAlgorithmException:
* Thrown if the transformation is null, empty or invalid, or if no security provider
* implements it. Was introduced in API 1.
* - InvalidKeyException:
* Thrown if the given key is inappropriate for initializing this cipher.
* - InvalidAlgorithmParameterException:
* Thrown if the OAEP parameters are invalid or unsupported.
*
* These indicate the algorithm/padding is not available on the device at all -- a real
* device-level incompatibility.
*
* Read more in https://developer.android.com/reference/javax/crypto/Cipher
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,44 @@ public void shouldCreateRSAKeyPairIfMissingOnAPI28AndUp() throws Exception {
assertThat(entry, is(expectedEntry));
}


@Test
@Config(sdk = 30)
public void shouldNotAuthorizeMgf1DigestWhenCreatingRSAKeyPairBelowAPI33() throws Exception {
// Below API 33 setMgf1Digests does not exist; the key must be generated without it (the
// pre-existing behavior), relying on the SHA-1 default that older Keystore uses.
Mockito.when(keyStore.containsAlias(KEY_ALIAS)).thenReturn(false);
KeyStore.PrivateKeyEntry expectedEntry = Mockito.mock(KeyStore.PrivateKeyEntry.class);
Mockito.when(keyStore.getEntry(KEY_ALIAS, null)).thenReturn(expectedEntry);

ArgumentCaptor<AlgorithmParameterSpec> specCaptor = ArgumentCaptor.forClass(AlgorithmParameterSpec.class);

cryptoUtil.getRSAKeyEntry();

Mockito.verify(keyPairGenerator).initialize(specCaptor.capture());
Mockito.verify(keyPairGenerator).generateKeyPair();
KeyGenParameterSpec spec = (KeyGenParameterSpec) specCaptor.getValue();

// The rest of the spec is unchanged from the pre-fix behavior.
assertThat(spec.getEncryptionPaddings(), is(new String[]{KeyProperties.ENCRYPTION_PADDING_RSA_OAEP}));
assertThat(spec.getDigests(), is(new String[]{KeyProperties.DIGEST_SHA1, KeyProperties.DIGEST_SHA256}));

String[] mgf1Digests = readMgf1Digests(spec);
// No MGF1 digest was authorized (null/empty), matching the pre-fix key spec.
assertThat(mgf1Digests == null || mgf1Digests.length == 0, is(true));
}


private static String[] readMgf1Digests(KeyGenParameterSpec spec) {
try {
java.lang.reflect.Method m = spec.getClass().getMethod("getMgf1Digests");
Object result = m.invoke(spec);
return (String[]) result;
} catch (Throwable t) {
return null;
}
Comment on lines +251 to +285

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add API 35 coverage for MGF1 authorization.

This test only verifies the API 30 path. It does not execute setMgf1Digests. Add an @Config(sdk = 35) test that asserts SHA-1 and SHA-256 are authorized. Rename this test because the production boundary is API 35, not API 33.

As per coding guidelines, test both success and failure paths for behavior changes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@auth0/src/test/java/com/auth0/android/authentication/storage/CryptoUtilTest.java`
around lines 251 - 285, Rename
shouldNotAuthorizeMgf1DigestWhenCreatingRSAKeyPairBelowAPI33 to reflect the API
35 boundary, and retain it as the pre-API-35 coverage. Add a separate
`@Config`(sdk = 35) test around cryptoUtil.getRSAKeyEntry that captures the
KeyGenParameterSpec and asserts getMgf1Digests contains SHA-1 and SHA-256.
Ensure the new coverage exercises both successful authorization and the existing
failure/error path for MGF1 configuration.

Source: Coding guidelines

}

@Test
@Config(sdk = 28)
public void shouldCreateNewRSAKeyPairWhenExistingRSAKeyPairCannotBeRebuiltOnAPI28AndUp() throws Exception {
Expand Down Expand Up @@ -705,8 +743,8 @@ public void shouldRSADecryptData() throws Exception {
}

@Test
public void shouldThrowOnInvalidKeyExceptionWhenTryingToRSADecrypt() {
Assert.assertThrows("The device is not compatible with the CryptoUtil class", IncompatibleDeviceException.class, () -> {
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, () -> {
byte[] sampleBytes = new byte[0];
PrivateKey privateKey = Mockito.mock(PrivateKey.class);
KeyStore.PrivateKeyEntry privateKeyEntry = Mockito.mock(KeyStore.PrivateKeyEntry.class);
Expand All @@ -717,6 +755,33 @@ public void shouldThrowOnInvalidKeyExceptionWhenTryingToRSADecrypt() {

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");
}

@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, () -> {
byte[] sampleBytes = new byte[0];
PrivateKey privateKey = Mockito.mock(PrivateKey.class);
KeyStore.PrivateKeyEntry privateKeyEntry = Mockito.mock(KeyStore.PrivateKeyEntry.class);
doReturn(privateKey).when(privateKeyEntry).getPrivateKey();
doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry();
Mockito.when(Cipher.getInstance(RSA_TRANSFORMATION)).thenReturn(rsaOaepCipher);
doThrow(new InvalidAlgorithmParameterException()).when(rsaOaepCipher).init(eq(Cipher.DECRYPT_MODE), eq(privateKey), any(AlgorithmParameterSpec.class));

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);
}

@Test
Expand Down
Loading