diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java b/auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java index 846228b2..c6923261 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java @@ -53,7 +53,20 @@ class CryptoUtil { // Transformations available since API 18 // https://developer.android.com/training/articles/keystore.html#SupportedCiphers - private static final String RSA_TRANSFORMATION = "RSA/ECB/PKCS1Padding"; + private static final String RSA_TRANSFORMATION = "RSA/ECB/OAEPWithSHA-256AndMGF1Padding"; + /** + * !!! WARNING !!! + * "RSA/ECB/PKCS1Padding" is cryptographically deprecated due to vulnerabilities + * (e.g. Bleichenbacher padding oracle attacks) and MUST NOT be used for encrypting + * new data or for any general-purpose RSA operations. + * + * This transformation exists solely to DECRYPT pre-existing legacy data that was + * originally encrypted with PKCS#1 v1.5 padding, so that it can be re-encrypted + * using the secure OAEP-based {@link #RSA_TRANSFORMATION}. Once all legacy data has + * been migrated, support for this constant and any code paths that use it should be + * removed. + */ + private static final String LEGACY_PKCS1_RSA_TRANSFORMATION = "RSA/ECB/PKCS1Padding"; // https://developer.android.com/reference/javax/crypto/Cipher.html @SuppressWarnings("SpellCheckingInspection") private static final String AES_TRANSFORMATION = "AES/GCM/NOPADDING"; @@ -62,7 +75,7 @@ class CryptoUtil { private static final String ALGORITHM_RSA = "RSA"; private static final String ALGORITHM_AES = "AES"; private static final int AES_KEY_SIZE = 256; - private static final int RSA_KEY_SIZE = 2048; + private static final int RSA_KEY_SIZE = 4096; private static final byte FORMAT_MARKER = 0x01; @@ -91,6 +104,31 @@ public CryptoUtil(@NonNull Context context, @NonNull Storage storage, @NonNull S this.storage = storage; } + /** + * Decrypts data that was encrypted using legacy RSA/PKCS1 padding. + *
+ * WARNING: This must only be used for decrypting legacy data during migration.
+ * New code must always use OAEP padding for RSA encryption/decryption.
+ *
+ * @param encryptedData The data encrypted with PKCS1 padding
+ * @param privateKey The private key for decryption
+ * @return The decrypted data
+ * @throws NoSuchPaddingException If PKCS1 padding is not available
+ * @throws NoSuchAlgorithmException If RSA algorithm is not available
+ * @throws InvalidKeyException If the private key is invalid
+ * @throws BadPaddingException If the encrypted data has invalid padding
+ * @throws IllegalBlockSizeException If the encrypted data size is invalid
+ */
+ @NonNull
+ private static byte[] RSADecryptLegacyPKCS1(@NonNull byte[] encryptedData,
+ @NonNull PrivateKey privateKey)
+ throws NoSuchPaddingException, NoSuchAlgorithmException, InvalidKeyException,
+ BadPaddingException, IllegalBlockSizeException {
+ Cipher rsaPkcs1Cipher = Cipher.getInstance(LEGACY_PKCS1_RSA_TRANSFORMATION);
+ rsaPkcs1Cipher.init(Cipher.DECRYPT_MODE, privateKey);
+ return rsaPkcs1Cipher.doFinal(encryptedData);
+ }
+
/**
* Attempts to recover the existing RSA Private Key entry or generates a new one as secure as
* this device and Android version allows it if none is found.
@@ -372,30 +410,106 @@ byte[] RSAEncrypt(byte[] decryptedInput) throws IncompatibleDeviceException, Cry
@VisibleForTesting
byte[] getAESKey() throws IncompatibleDeviceException, CryptoException {
String encodedEncryptedAES = storage.retrieveString(KEY_ALIAS);
- if (TextUtils.isEmpty(encodedEncryptedAES)) {
- encodedEncryptedAES = storage.retrieveString(OLD_KEY_ALIAS);
+ if (!TextUtils.isEmpty(encodedEncryptedAES)) {
+ byte[] encryptedAESBytes = Base64.decode(encodedEncryptedAES, Base64.DEFAULT);
+ try {
+ return RSADecrypt(encryptedAESBytes);
+ } catch (IncompatibleDeviceException e) {
+ String fullMessage = e.toString();
+ Throwable cause = e.getCause();
+ while (cause != null) {
+ fullMessage += "\n" + cause.toString();
+ cause = cause.getCause();
+ }
+
+ if (fullMessage.contains("Incompatible padding mode") ||
+ fullMessage.contains("INCOMPATIBLE_PADDING_MODE")) {
+ try {
+ KeyStore keyStore = KeyStore.getInstance(ANDROID_KEY_STORE);
+ keyStore.load(null);
+
+ // Get the RSA key from KeyStore (could be at KEY_ALIAS or OLD_KEY_ALIAS)
+ KeyStore.PrivateKeyEntry rsaKey = null;
+ String keyAliasUsed = null;
+
+ if (keyStore.containsAlias(KEY_ALIAS)) {
+ rsaKey = getKeyEntryCompat(keyStore, KEY_ALIAS);
+ keyAliasUsed = KEY_ALIAS;
+ } else if (keyStore.containsAlias(OLD_KEY_ALIAS)) {
+ rsaKey = getKeyEntryCompat(keyStore, OLD_KEY_ALIAS);
+ keyAliasUsed = OLD_KEY_ALIAS;
+ }
+
+ if (rsaKey != null && keyAliasUsed != null) {
+ // WARNING: Using PKCS1 padding here is intentional and ONLY for decrypting legacy data.
+ // This cipher must NEVER be used for encryption or for any new data; always use OAEP instead.
+ byte[] decryptedAESKey = RSADecryptLegacyPKCS1(
+ encryptedAESBytes,
+ rsaKey.getPrivateKey()
+ );
+ deleteRSAKeys();
+
+ // Re-encrypt AES key with NEW OAEP RSA key (4096-bit)
+ byte[] encryptedAESWithOAEP = RSAEncrypt(decryptedAESKey);
+ String newEncodedEncryptedAES = new String(
+ Base64.encode(encryptedAESWithOAEP, Base64.DEFAULT),
+ StandardCharsets.UTF_8
+ );
+ storage.store(KEY_ALIAS, newEncodedEncryptedAES);
+
+ return decryptedAESKey;
+ }
+ } catch (CryptoException | KeyStoreException | CertificateException |
+ IOException | NoSuchAlgorithmException | UnrecoverableEntryException |
+ NoSuchPaddingException | InvalidKeyException |
+ IllegalBlockSizeException | BadPaddingException ex) {
+ Log.e(TAG, "Could not migrate. A new key will be generated.", ex);
+ deleteRSAKeys();
+ deleteAESKeys();
+ }
+ } else {
+ throw e;
+ }
+ } catch (CryptoException e) {
+ // RSA decryption failed - the encrypted AES key is corrupted or the RSA key is invalid
+ // Delete keys and regenerate them
+ Log.w(TAG, "RSA decryption failed with CryptoException. Keys may be corrupted. Will regenerate.", e);
+ deleteRSAKeys();
+ deleteAESKeys();
+ }
}
- if (encodedEncryptedAES != null) {
- //Return existing key
- byte[] encryptedAES = Base64.decode(encodedEncryptedAES, Base64.DEFAULT);
- byte[] existingAES = RSADecrypt(encryptedAES);
- final int aesExpectedLengthInBytes = AES_KEY_SIZE / 8;
- //Prevent returning an 'Empty key' (invalid/corrupted) that was mistakenly saved
- if (existingAES != null && existingAES.length == aesExpectedLengthInBytes) {
- //Key exists and has the right size
- return existingAES;
+ String encodedOldAES = storage.retrieveString(OLD_KEY_ALIAS);
+ if (!TextUtils.isEmpty(encodedOldAES)) {
+ try {
+ byte[] encryptedOldAESBytes = Base64.decode(encodedOldAES, Base64.DEFAULT);
+ KeyStore.PrivateKeyEntry rsaKeyEntry = getRSAKeyEntry();
+ // WARNING: Using PKCS1 padding here is intentional and ONLY for decrypting legacy data.
+ // This cipher must NEVER be used for encryption or for any new data; always use OAEP padding instead.
+ byte[] decryptedAESKey = RSADecryptLegacyPKCS1(
+ encryptedOldAESBytes,
+ rsaKeyEntry.getPrivateKey()
+ );
+
+ byte[] encryptedAESWithOAEP = RSAEncrypt(decryptedAESKey);
+ String newEncodedEncryptedAES = new String(Base64.encode(encryptedAESWithOAEP, Base64.DEFAULT), StandardCharsets.UTF_8);
+ storage.store(KEY_ALIAS, newEncodedEncryptedAES);
+ storage.remove(OLD_KEY_ALIAS);
+ return decryptedAESKey;
+ } catch (Exception e) {
+ Log.e(TAG, "Could not migrate the legacy AES key. A new key will be generated.", e);
+ deleteAESKeys();
}
}
- //Key doesn't exist. Generate new AES
+
try {
KeyGenerator keyGen = KeyGenerator.getInstance(ALGORITHM_AES);
keyGen.init(AES_KEY_SIZE);
- byte[] aes = keyGen.generateKey().getEncoded();
- //Save encrypted encoded version
- byte[] encryptedAES = RSAEncrypt(aes);
- String encodedEncryptedAESText = new String(Base64.encode(encryptedAES, Base64.DEFAULT), StandardCharsets.UTF_8);
- storage.store(KEY_ALIAS, encodedEncryptedAESText);
- return aes;
+ byte[] decryptedAESKey = keyGen.generateKey().getEncoded();
+
+ byte[] encryptedNewAES = RSAEncrypt(decryptedAESKey);
+ String encodedEncryptedNewAESText = new String(Base64.encode(encryptedNewAES, Base64.DEFAULT), StandardCharsets.UTF_8);
+ storage.store(KEY_ALIAS, encodedEncryptedNewAESText);
+ return decryptedAESKey;
} catch (NoSuchAlgorithmException e) {
/*
* This exceptions are safe to be ignored:
@@ -407,6 +521,9 @@ byte[] getAESKey() throws IncompatibleDeviceException, CryptoException {
*/
Log.e(TAG, "Error while creating the AES key.", e);
throw new IncompatibleDeviceException(e);
+ } catch (Exception e) {
+ Log.e(TAG, "Unexpected error while creating the new AES key.", e);
+ throw new CryptoException("Unexpected error while creating the new AES key.", e);
}
}
diff --git a/auth0/src/test/java/com/auth0/android/authentication/storage/CryptoUtilTest.java b/auth0/src/test/java/com/auth0/android/authentication/storage/CryptoUtilTest.java
index 466de58c..4963b328 100644
--- a/auth0/src/test/java/com/auth0/android/authentication/storage/CryptoUtilTest.java
+++ b/auth0/src/test/java/com/auth0/android/authentication/storage/CryptoUtilTest.java
@@ -27,6 +27,7 @@
import java.io.IOException;
import java.math.BigInteger;
+import java.nio.charset.StandardCharsets;
import java.security.InvalidAlgorithmParameterException;
import java.security.InvalidKeyException;
import java.security.KeyPairGenerator;
@@ -63,10 +64,12 @@
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
import static org.mockito.Mockito.when;
import static org.powermock.api.mockito.PowerMockito.doReturn;
import static org.powermock.api.mockito.PowerMockito.doThrow;
import static org.powermock.api.mockito.PowerMockito.mock;
+import static org.powermock.api.mockito.PowerMockito.verifyPrivate;
/**
* In the rest of the test files we use Mockito as that's enough for most cases. However,
@@ -82,15 +85,18 @@
@PrepareForTest({CryptoUtil.class, KeyGenerator.class, TextUtils.class, Build.VERSION.class, Base64.class, Cipher.class, Log.class})
public class CryptoUtilTest {
- private static final String RSA_TRANSFORMATION = "RSA/ECB/PKCS1Padding";
+ private static final String RSA_TRANSFORMATION = "RSA/ECB/OAEPWithSHA-256AndMGF1Padding";
+ private static final String OLD_RSA_PKCS1_TRANSFORMATION = "RSA/ECB/PKCS1Padding";
private static final String AES_TRANSFORMATION = "AES/GCM/NOPADDING";
private static final String CERTIFICATE_PRINCIPAL = "CN=Auth0.Android,O=Auth0";
private static final String ANDROID_KEY_STORE = "AndroidKeyStore";
private static final String ALGORITHM_AES = "AES";
private static final String ALGORITHM_RSA = "RSA";
+ private static final int RSA_KEY_SIZE = 4096;
private final Storage storage = PowerMockito.mock(Storage.class);
- private final Cipher rsaCipher = PowerMockito.mock(Cipher.class);
+ private final Cipher rsaOaepCipher = PowerMockito.mock(Cipher.class);
+ private final Cipher rsaPkcs1Cipher = PowerMockito.mock(Cipher.class);
private final Cipher aesCipher = PowerMockito.mock(Cipher.class);
private final KeyStore keyStore = PowerMockito.mock(KeyStore.class);
private final KeyPairGenerator keyPairGenerator = PowerMockito.mock(KeyPairGenerator.class);
@@ -158,7 +164,7 @@ public void shouldNotCreateProtectedRSAKeyPairIfMissingAndLockScreenEnabledOnAPI
final KeyStore.PrivateKeyEntry entry = cryptoUtil.getRSAKeyEntry();
- Mockito.verify(builder).setKeySize(2048);
+ Mockito.verify(builder).setKeySize(4096);
Mockito.verify(builder).setSubject(principalCaptor.capture());
Mockito.verify(builder).setAlias(KEY_ALIAS);
Mockito.verify(builder).setSerialNumber(BigInteger.ONE);
@@ -209,7 +215,7 @@ public void shouldCreateUnprotectedRSAKeyPairIfMissingAndLockScreenDisabledOnAPI
final KeyStore.PrivateKeyEntry entry = cryptoUtil.getRSAKeyEntry();
- Mockito.verify(builder).setKeySize(2048);
+ Mockito.verify(builder).setKeySize(4096);
Mockito.verify(builder).setSubject(principalCaptor.capture());
Mockito.verify(builder).setAlias(KEY_ALIAS);
Mockito.verify(builder).setSerialNumber(BigInteger.ONE);
@@ -260,7 +266,7 @@ public void shouldCreateProtectedRSAKeyPairIfMissingAndLockScreenEnabledOnAPI21(
final KeyStore.PrivateKeyEntry entry = cryptoUtil.getRSAKeyEntry();
- Mockito.verify(builder).setKeySize(2048);
+ Mockito.verify(builder).setKeySize(4096);
Mockito.verify(builder).setSubject(principalCaptor.capture());
Mockito.verify(builder).setAlias(KEY_ALIAS);
Mockito.verify(builder).setSerialNumber(BigInteger.ONE);
@@ -307,7 +313,7 @@ public void shouldCreateRSAKeyPairIfMissingOnAPI23AndUp() throws Exception {
final KeyStore.PrivateKeyEntry entry = cryptoUtil.getRSAKeyEntry();
- Mockito.verify(builder).setKeySize(2048);
+ Mockito.verify(builder).setKeySize(4096);
Mockito.verify(builder).setCertificateSubject(principalCaptor.capture());
Mockito.verify(builder).setCertificateSerialNumber(BigInteger.ONE);
Mockito.verify(builder).setCertificateNotBefore(startDateCaptor.capture());
@@ -353,7 +359,7 @@ public void shouldCreateRSAKeyPairIfMissingOnAPI28AndUp() throws Exception {
final KeyStore.PrivateKeyEntry entry = cryptoUtil.getRSAKeyEntry();
- Mockito.verify(builder).setKeySize(2048);
+ Mockito.verify(builder).setKeySize(4096);
Mockito.verify(builder).setCertificateSubject(principalCaptor.capture());
Mockito.verify(builder).setCertificateSerialNumber(BigInteger.ONE);
Mockito.verify(builder).setCertificateNotBefore(startDateCaptor.capture());
@@ -408,7 +414,7 @@ public void shouldCreateNewRSAKeyPairWhenExistingRSAKeyPairCannotBeRebuiltOnAPI2
final KeyStore.PrivateKeyEntry entry = cryptoUtil.getRSAKeyEntry();
- Mockito.verify(builder).setKeySize(2048);
+ Mockito.verify(builder).setKeySize(4096);
Mockito.verify(builder).setCertificateSubject(principalCaptor.capture());
Mockito.verify(builder).setCertificateSerialNumber(BigInteger.ONE);
Mockito.verify(builder).setCertificateNotBefore(startDateCaptor.capture());
@@ -699,7 +705,7 @@ public void shouldCreateAESKeyIfStoredOneIsEmpty() throws BadPaddingException, I
KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class);
doReturn(privateKey).when(privateKeyEntry).getPrivateKey();
doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry();
- doReturn(sampleOutput).when(rsaCipher).doFinal(sampleBytes);
+ doReturn(sampleOutput).when(rsaOaepCipher).doFinal(sampleBytes);
SecretKey secretKey = PowerMockito.mock(SecretKey.class);
PowerMockito.when(secretKey.getEncoded()).thenReturn(sampleBytes);
@@ -758,11 +764,11 @@ public void shouldRSAEncryptData() throws Exception {
KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class);
doReturn(certificate).when(privateKeyEntry).getCertificate();
doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry();
- doReturn(sampleOutput).when(rsaCipher).doFinal(sampleInput);
+ doReturn(sampleOutput).when(rsaOaepCipher).doFinal(sampleInput);
final byte[] output = cryptoUtil.RSAEncrypt(sampleInput);
- Mockito.verify(rsaCipher).init(Cipher.ENCRYPT_MODE, certificate);
+ Mockito.verify(rsaOaepCipher).init(Cipher.ENCRYPT_MODE, certificate);
assertThat(output, is(sampleOutput));
}
@@ -775,8 +781,8 @@ public void shouldThrowOnInvalidKeyExceptionWhenTryingToRSAEncrypt() {
doReturn(certificate).when(privateKeyEntry).getCertificate();
doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry();
PowerMockito.mockStatic(Cipher.class);
- PowerMockito.when(Cipher.getInstance(RSA_TRANSFORMATION)).thenReturn(rsaCipher);
- doThrow(new InvalidKeyException()).when(rsaCipher).init(Cipher.ENCRYPT_MODE, certificate);
+ PowerMockito.when(Cipher.getInstance(RSA_TRANSFORMATION)).thenReturn(rsaOaepCipher);
+ doThrow(new InvalidKeyException()).when(rsaOaepCipher).init(Cipher.ENCRYPT_MODE, certificate);
cryptoUtil.RSAEncrypt(sampleBytes);
});
@@ -792,8 +798,8 @@ public void shouldDeleteAESKeysAndThrowOnBadPaddingExceptionWhenTryingToRSAEncry
doReturn(certificate).when(privateKeyEntry).getCertificate();
doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry();
PowerMockito.mockStatic(Cipher.class);
- PowerMockito.when(Cipher.getInstance(RSA_TRANSFORMATION)).thenReturn(rsaCipher);
- PowerMockito.when(rsaCipher.doFinal(sampleBytes)).thenThrow(new BadPaddingException());
+ PowerMockito.when(Cipher.getInstance(RSA_TRANSFORMATION)).thenReturn(rsaOaepCipher);
+ PowerMockito.when(rsaOaepCipher.doFinal(sampleBytes)).thenThrow(new BadPaddingException());
cryptoUtil.RSAEncrypt(sampleBytes);
});
@@ -814,8 +820,8 @@ public void shouldDeleteAESKeysAndThrowOnIllegalBlockSizeExceptionWhenTryingToRS
doReturn(certificate).when(privateKeyEntry).getCertificate();
doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry();
PowerMockito.mockStatic(Cipher.class);
- PowerMockito.when(Cipher.getInstance(RSA_TRANSFORMATION)).thenReturn(rsaCipher);
- PowerMockito.when(rsaCipher.doFinal(any(byte[].class))).thenThrow(new IllegalBlockSizeException());
+ PowerMockito.when(Cipher.getInstance(RSA_TRANSFORMATION)).thenReturn(rsaOaepCipher);
+ PowerMockito.when(rsaOaepCipher.doFinal(any(byte[].class))).thenThrow(new IllegalBlockSizeException());
cryptoUtil.RSAEncrypt(new byte[0]);
});
@@ -869,11 +875,11 @@ public void shouldRSADecryptData() throws Exception {
KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class);
doReturn(privateKey).when(privateKeyEntry).getPrivateKey();
doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry();
- doReturn(sampleOutput).when(rsaCipher).doFinal(sampleInput);
+ doReturn(sampleOutput).when(rsaOaepCipher).doFinal(sampleInput);
final byte[] output = cryptoUtil.RSADecrypt(sampleInput);
- Mockito.verify(rsaCipher).init(Cipher.DECRYPT_MODE, privateKey);
+ Mockito.verify(rsaOaepCipher).init(Cipher.DECRYPT_MODE, privateKey);
assertThat(output, is(sampleOutput));
}
@@ -886,8 +892,8 @@ public void shouldThrowOnInvalidKeyExceptionWhenTryingToRSADecrypt() {
doReturn(privateKey).when(privateKeyEntry).getPrivateKey();
doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry();
PowerMockito.mockStatic(Cipher.class);
- PowerMockito.when(Cipher.getInstance(RSA_TRANSFORMATION)).thenReturn(rsaCipher);
- doThrow(new InvalidKeyException()).when(rsaCipher).init(Cipher.DECRYPT_MODE, privateKey);
+ PowerMockito.when(Cipher.getInstance(RSA_TRANSFORMATION)).thenReturn(rsaOaepCipher);
+ doThrow(new InvalidKeyException()).when(rsaOaepCipher).init(Cipher.DECRYPT_MODE, privateKey);
cryptoUtil.RSADecrypt(sampleBytes);
});
@@ -929,7 +935,7 @@ public void shouldDeleteAESKeysAndThrowOnBadPaddingExceptionWhenTryingToRSADecry
doReturn(privateKey).when(privateKeyEntry).getPrivateKey();
doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry();
- doThrow(new BadPaddingException()).when(rsaCipher).doFinal(any(byte[].class));
+ doThrow(new BadPaddingException()).when(rsaOaepCipher).doFinal(any(byte[].class));
cryptoUtil.RSADecrypt(new byte[0]);
});
@@ -949,7 +955,7 @@ public void shouldDeleteAESKeysAndThrowOnIllegalBlockSizeExceptionWhenTryingToRS
doReturn(privateKey).when(privateKeyEntry).getPrivateKey();
doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry();
- doThrow(new IllegalBlockSizeException()).when(rsaCipher).doFinal(any(byte[].class));
+ doThrow(new IllegalBlockSizeException()).when(rsaOaepCipher).doFinal(any(byte[].class));
cryptoUtil.RSADecrypt(new byte[0]);
});
@@ -1677,7 +1683,9 @@ private CryptoUtil newCryptoUtilSpy() throws Exception {
PowerMockito.when(Cipher.getInstance(anyString())).then((Answer