Skip to content
Merged
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 @@ -12,6 +12,7 @@
import android.util.Log;

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;

import java.io.IOException;
Expand Down Expand Up @@ -39,9 +40,13 @@
import javax.crypto.NoSuchPaddingException;
import javax.crypto.SecretKey;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.OAEPParameterSpec;
import javax.crypto.spec.PSource;
import javax.crypto.spec.SecretKeySpec;
import javax.security.auth.x500.X500Principal;

import java.security.spec.MGF1ParameterSpec;

/**
* Created by lbalmaceda on 8/24/17.
* Class to handle encryption/decryption cryptographic operations using AES and RSA algorithms in devices with API 19 or higher.
Expand All @@ -53,7 +58,25 @@ 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-1AndMGF1Padding";
/**
* !!! 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.
*/
// CodeQL suppression: This legacy constant is required for backward compatibility
// to decrypt credentials encrypted with PKCS1 before the migration to OAEP.
// It is only used for decryption (reading old data), never encryption (writing new data).
// This constant will be removed once all users have migrated to OAEP.
@SuppressWarnings("java/rsa-without-oaep")
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";
Expand All @@ -64,6 +87,17 @@ class CryptoUtil {
private static final int AES_KEY_SIZE = 256;
private static final int RSA_KEY_SIZE = 2048;

// Explicit OAEP specification for consistent behavior across JCE providers.
// Using SHA-1 for both OAEP hash and MGF1 hash as it's well-supported by Android KeyStore.
// Note: SHA-1 in OAEP/MGF1 context only requires preimage resistance (still secure),
// unlike digital signatures which require collision resistance.
private static final OAEPParameterSpec OAEP_SPEC = new OAEPParameterSpec(
"SHA-1",
"MGF1",
MGF1ParameterSpec.SHA1,
PSource.PSpecified.DEFAULT
);

private static final byte FORMAT_MARKER = 0x01;

private static final int GCM_TAG_LENGTH = 16;
Expand Down Expand Up @@ -91,6 +125,32 @@ public CryptoUtil(@NonNull Context context, @NonNull Storage storage, @NonNull S
this.storage = storage;
}

/**
* Decrypts data that was encrypted using legacy RSA/PKCS1 padding.
* <p>
* 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
@SuppressWarnings("java/rsa-without-oaep")
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.
Expand Down Expand Up @@ -130,7 +190,8 @@ KeyStore.PrivateKeyEntry getRSAKeyEntry() throws CryptoException, IncompatibleDe
.setCertificateNotBefore(start.getTime())
.setCertificateNotAfter(end.getTime())
.setKeySize(RSA_KEY_SIZE)
.setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_PKCS1)
.setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_OAEP)
.setDigests(KeyProperties.DIGEST_SHA1, KeyProperties.DIGEST_SHA256)
.setBlockModes(KeyProperties.BLOCK_MODE_ECB)
.build();
} else {
Expand Down Expand Up @@ -280,9 +341,9 @@ byte[] RSADecrypt(byte[] encryptedInput) throws IncompatibleDeviceException, Cry
try {
PrivateKey privateKey = getRSAKeyEntry().getPrivateKey();
Cipher cipher = Cipher.getInstance(RSA_TRANSFORMATION);
cipher.init(Cipher.DECRYPT_MODE, privateKey);
cipher.init(Cipher.DECRYPT_MODE, privateKey, OAEP_SPEC);
return cipher.doFinal(encryptedInput);
} catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException e) {
} catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | InvalidAlgorithmParameterException e) {
/*
* This exceptions are safe to be ignored:
*
Expand All @@ -293,6 +354,8 @@ byte[] RSADecrypt(byte[] encryptedInput) throws IncompatibleDeviceException, Cry
* 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.
*
* Read more in https://developer.android.com/reference/javax/crypto/Cipher
*/
Expand Down Expand Up @@ -329,9 +392,9 @@ byte[] RSAEncrypt(byte[] decryptedInput) throws IncompatibleDeviceException, Cry
try {
Certificate certificate = getRSAKeyEntry().getCertificate();
Cipher cipher = Cipher.getInstance(RSA_TRANSFORMATION);
cipher.init(Cipher.ENCRYPT_MODE, certificate);
cipher.init(Cipher.ENCRYPT_MODE, certificate.getPublicKey(), OAEP_SPEC);
return cipher.doFinal(decryptedInput);
} catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException e) {
} catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | InvalidAlgorithmParameterException e) {
/*
* This exceptions are safe to be ignored:
*
Expand All @@ -342,6 +405,8 @@ byte[] RSAEncrypt(byte[] decryptedInput) throws IncompatibleDeviceException, Cry
* 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.
*
* Read more in https://developer.android.com/reference/javax/crypto/Cipher
*/
Expand All @@ -362,6 +427,83 @@ byte[] RSAEncrypt(byte[] decryptedInput) throws IncompatibleDeviceException, Cry
}
}

/**
* Attempts to migrate legacy PKCS1-encrypted AES key to OAEP format.
* This method tries to decrypt the AES key using legacy PKCS1 padding,
* then re-encrypts it with OAEP and stores it for future use.
*
* @param encryptedAESBytes the encrypted AES key bytes
* @return the decrypted AES key if migration succeeds, or null if migration fails
*/
@Nullable
private byte[] attemptPKCS1Migration(byte[] encryptedAESBytes) {
try {
KeyStore keyStore = KeyStore.getInstance(ANDROID_KEY_STORE);
keyStore.load(null);

KeyStore.PrivateKeyEntry rsaKey = findRSAKeyEntry(keyStore);
if (rsaKey == null) {
Log.d(TAG, "No RSA key found for migration");
return null;
}

byte[] decryptedAESKey = RSADecryptLegacyPKCS1(encryptedAESBytes, rsaKey.getPrivateKey());

if (!isValidAESKeyLength(decryptedAESKey)) {
Log.e(TAG, "Decrypted AES key has invalid length: " + decryptedAESKey.length);
return null;
}

Log.d(TAG, "PKCS1 migration successful - deleting old keys");

deleteRSAKeys();

byte[] encryptedAESWithOAEP = RSAEncrypt(decryptedAESKey);
String encodedEncryptedAES = new String(Base64.encode(encryptedAESWithOAEP, Base64.DEFAULT), StandardCharsets.UTF_8);
storage.store(KEY_ALIAS, encodedEncryptedAES);

Log.d(TAG, "AES key re-encrypted with OAEP and stored");
return decryptedAESKey;

} catch (BadPaddingException | IllegalBlockSizeException e) {
Log.e(TAG, "PKCS1 decryption failed. Data may be corrupted.", e);
} catch (KeyStoreException | CertificateException | IOException |
NoSuchAlgorithmException | UnrecoverableEntryException |
NoSuchPaddingException | InvalidKeyException e) {
Log.e(TAG, "Migration failed due to key access error.", e);
} catch (CryptoException e) {
Log.e(TAG, "Failed to re-encrypt AES key with OAEP.", e);
}
Comment on lines +474 to +476
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

If RSAEncrypt throws IncompatibleDeviceException during migration (line 461), it will be caught by the CryptoException catch block at line 474, causing migration to fail silently by returning null. Combined with the logic in tryRecoverCurrentAESKey that re-throws IncompatibleDeviceException when migration fails, this creates a problematic scenario:

  1. OAEP decryption fails with IncompatibleDeviceException (device doesn't support OAEP)
  2. PKCS1 migration is attempted and successfully decrypts the AES key
  3. RSAEncrypt fails with IncompatibleDeviceException (device doesn't support OAEP)
  4. Migration returns null
  5. The original IncompatibleDeviceException is re-thrown to the user
  6. User data is permanently lost

Consider handling IncompatibleDeviceException separately in attemptPKCS1Migration. If RSAEncrypt fails with IncompatibleDeviceException, this indicates the device genuinely cannot support OAEP. In this case, you might want to either:

  • Keep the old PKCS1-encrypted data and continue using PKCS1 (with appropriate warnings)
  • Re-throw the IncompatibleDeviceException immediately so the caller knows the device cannot support the new encryption scheme
  • Document that devices not supporting OAEP will lose their data during migration

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This behavior is intentional. This SDK version mandates OAEP for security. If a device cannot perform OAEP encryption (RSAEncrypt fails with IncompatibleDeviceException), it is by definition incompatible with this trusted device profile.

We cannot "keep using PKCS1" because that would leave the device in a vulnerable state indefinitely. Failing the migration and marking the device as incompatible is the secure-by-design choice.

return null;
}

/**
* Finds the RSA private key entry from KeyStore, checking both current and legacy aliases.
*
* @param keyStore the initialized KeyStore instance
* @return the RSA key entry, or null if not found
*/
@Nullable
private KeyStore.PrivateKeyEntry findRSAKeyEntry(KeyStore keyStore)
throws KeyStoreException, NoSuchAlgorithmException, UnrecoverableEntryException {
if (keyStore.containsAlias(KEY_ALIAS)) {
return getKeyEntryCompat(keyStore, KEY_ALIAS);
} else if (keyStore.containsAlias(OLD_KEY_ALIAS)) {
return getKeyEntryCompat(keyStore, OLD_KEY_ALIAS);
}
return null;
}

/**
* Validates that the decrypted AES key has the correct length for AES-256.
*
* @param aesKey the decrypted AES key bytes
* @return true if the key is valid (32 bytes), false otherwise
*/
private boolean isValidAESKeyLength(byte[] aesKey) {
return aesKey != null && aesKey.length == AES_KEY_SIZE / 8;
}

/**
* Attempts to recover the existing AES Key or generates a new one if none is found.
*
Expand All @@ -371,42 +513,131 @@ byte[] RSAEncrypt(byte[] decryptedInput) throws IncompatibleDeviceException, Cry
*/
@VisibleForTesting
byte[] getAESKey() throws IncompatibleDeviceException, CryptoException {
// Step 1: Try to recover existing AES key encrypted with current format (OAEP)
byte[] aesKey = tryRecoverCurrentAESKey();
if (aesKey != null) {
return aesKey;
}

// Step 2: Try to migrate legacy AES key stored at OLD_KEY_ALIAS
aesKey = tryMigrateLegacyAESKey();
if (aesKey != null) {
return aesKey;
}

// Step 3: Generate new AES key
return generateNewAESKey();
}

/**
* Attempts to recover the AES key stored at KEY_ALIAS using OAEP decryption.
* If OAEP fails, attempts PKCS1 decryption for legacy data migration.
*
* @return the decrypted AES key, or null if no key exists or recovery failed
* @throws IncompatibleDeviceException if the device doesn't support required crypto operations
* and migration also fails
*/
@Nullable
private byte[] tryRecoverCurrentAESKey() throws IncompatibleDeviceException {
String encodedEncryptedAES = storage.retrieveString(KEY_ALIAS);
if (TextUtils.isEmpty(encodedEncryptedAES)) {
encodedEncryptedAES = storage.retrieveString(OLD_KEY_ALIAS);
return null;
}
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;
}

byte[] encryptedAESBytes = Base64.decode(encodedEncryptedAES, Base64.DEFAULT);
CryptoException oaepException = null;

try {
return RSADecrypt(encryptedAESBytes);
} catch (CryptoException e) {
// OAEP decryption failed - could be legacy PKCS1 data or device incompatibility
// Store exception to re-throw if migration also fails
oaepException = e;
Log.d(TAG, "OAEP decryption failed, attempting PKCS1 migration", e);
}

// OAEP failed - attempt PKCS1 migration
byte[] migratedKey = attemptPKCS1Migration(encryptedAESBytes);
if (migratedKey != null) {
return migratedKey;
}
//Key doesn't exist. Generate new AES

// Migration failed or wasn't attempted
// If the original error was IncompatibleDeviceException, re-throw it
if (oaepException instanceof IncompatibleDeviceException) {
throw (IncompatibleDeviceException) oaepException;
}

// Recovery failed - clean up corrupted keys
Log.w(TAG, "Could not recover AES key. Deleting corrupted keys.");
deleteRSAKeys();
deleteAESKeys();
return null;
}

/**
* Attempts to migrate a legacy AES key stored at OLD_KEY_ALIAS.
* Decrypts with PKCS1, re-encrypts with OAEP, and stores at KEY_ALIAS.
*
* @return the decrypted AES key if migration succeeds, or null otherwise
*/
@Nullable
private byte[] tryMigrateLegacyAESKey() {
String encodedOldAES = storage.retrieveString(OLD_KEY_ALIAS);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to handle both the OLD_KEY_ALIAS and KEY_ALIAS in separate block here ? Can't a single block like in the original work ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These two blocks handle different migration scenarios and must remain separate:

KEY_ALIAS block: Handles the current storage location. When OAEP decryption fails, it attempts PKCS1 decryption as a migration path (same storage key, different RSA padding).

OLD_KEY_ALIAS block: Handles a legacy storage alias from much older SDK versions. This is an alias migration (moving data from old location to new location), not just a padding migration.

The original code also had this separation - it just wasn't as clearly structured. The two scenarios are:

Padding migration: KEY_ALIAS with PKCS1 → KEY_ALIAS with OAEP
Alias migration: OLD_KEY_ALIAS → KEY_ALIAS

if (TextUtils.isEmpty(encodedOldAES)) {
return null;
}

try {
byte[] encryptedOldAESBytes = Base64.decode(encodedOldAES, Base64.DEFAULT);
KeyStore.PrivateKeyEntry rsaKeyEntry = getRSAKeyEntry();

byte[] decryptedAESKey = RSADecryptLegacyPKCS1(encryptedOldAESBytes, rsaKeyEntry.getPrivateKey());

// Re-encrypt with OAEP and store at new location
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);

Log.d(TAG, "Legacy AES key migrated successfully");
return decryptedAESKey;
} catch (CryptoException | NoSuchPaddingException | NoSuchAlgorithmException | InvalidKeyException |
BadPaddingException | IllegalBlockSizeException | IllegalArgumentException e) {
Log.e(TAG, "Could not migrate legacy AES key. Will generate new key.", e);
deleteAESKeys();
return null;
}
}

/**
* Generates a new AES-256 key, encrypts it with RSA-OAEP, and stores it.
*
* @return the newly generated AES key bytes
* @throws IncompatibleDeviceException if the device doesn't support required algorithms
* @throws CryptoException if key generation or encryption fails unexpectedly
*/
private byte[] generateNewAESKey() throws IncompatibleDeviceException, CryptoException {
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);

Log.d(TAG, "New AES key generated and stored");
return decryptedAESKey;
} catch (NoSuchAlgorithmException e) {
/*
* This exceptions are safe to be ignored:
*
* - NoSuchAlgorithmException:
* Thrown if the Algorithm implementation is not available. AES was introduced in API 1
*
* Read more in https://developer.android.com/reference/javax/crypto/KeyGenerator
*/
Log.e(TAG, "Error while creating the AES key.", e);
Log.e(TAG, "AES algorithm not available.", e);
throw new IncompatibleDeviceException(e);
} catch (CryptoException e) {
// Re-throw CryptoException and its subclasses (including IncompatibleDeviceException)
throw e;
} catch (Exception e) {
Copy link

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

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

Catching generic Exception is too broad. This catch block should handle specific exceptions that can occur during RSA encryption or storage operations, such as CryptoException or IncompatibleDeviceException.

Suggested change
} catch (Exception e) {
} catch (InvalidKeyException
| NoSuchPaddingException
| IllegalBlockSizeException
| BadPaddingException
| KeyStoreException
| UnrecoverableEntryException
| CertificateException
| IOException
| ProviderException e) {

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

made new changes on 17 dec will check on copilot review on it.

Log.e(TAG, "Unexpected error while creating new AES key.", e);
throw new CryptoException("Unexpected error while creating new AES key.", e);
}
}

Expand Down
Loading