From d16166ca88878b8d00abb86c6e0c2159877c5baa Mon Sep 17 00:00:00 2001 From: Prince Mathew Date: Fri, 21 Nov 2025 17:29:49 +0530 Subject: [PATCH 1/5] fix: Fixing the IV overwrite conflict while storing multiple encrypted credentials --- .../authentication/storage/CryptoUtil.java | 139 +++++++++++++++--- .../storage/SecureCredentialsManager.kt | 28 ++-- 2 files changed, 131 insertions(+), 36 deletions(-) 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 1d6fe95df..45bf8ebc9 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 @@ -64,6 +64,8 @@ class CryptoUtil { private static final int AES_KEY_SIZE = 256; private static final int RSA_KEY_SIZE = 2048; + private static final byte FORMAT_MARKER = 0x01; + private final String OLD_KEY_ALIAS; private final String OLD_KEY_IV_ALIAS; private final String KEY_ALIAS; @@ -156,7 +158,9 @@ KeyStore.PrivateKeyEntry getRSAKeyEntry() throws CryptoException, IncompatibleDe generator.generateKeyPair(); return getKeyEntryCompat(keyStore, KEY_ALIAS); - } catch (CertificateException | InvalidAlgorithmParameterException | NoSuchProviderException | NoSuchAlgorithmException | KeyStoreException | ProviderException e) { + } catch (CertificateException | InvalidAlgorithmParameterException | + NoSuchProviderException | NoSuchAlgorithmException | KeyStoreException | + ProviderException e) { /* * This exceptions are safe to be ignored: * @@ -240,7 +244,8 @@ private void deleteRSAKeys() { keyStore.deleteEntry(KEY_ALIAS); keyStore.deleteEntry(OLD_KEY_ALIAS); Log.d(TAG, "Deleting the existing RSA key pair from the KeyStore."); - } catch (KeyStoreException | CertificateException | IOException | NoSuchAlgorithmException e) { + } catch (KeyStoreException | CertificateException | IOException | + NoSuchAlgorithmException e) { Log.e(TAG, "Failed to remove the RSA KeyEntry from the Android KeyStore.", e); } } @@ -403,7 +408,7 @@ byte[] getAESKey() throws IncompatibleDeviceException, CryptoException { /** - * Encrypts the given input bytes using a symmetric key (AES). + * Decrypts the given input bytes using a symmetric key (AES). * The AES key is stored protected by an asymmetric key pair (RSA). * * @param encryptedInput the input bytes to decrypt. There's no limit in size. @@ -415,18 +420,15 @@ public byte[] decrypt(byte[] encryptedInput) throws CryptoException, Incompatibl try { SecretKey key = new SecretKeySpec(getAESKey(), ALGORITHM_AES); Cipher cipher = Cipher.getInstance(AES_TRANSFORMATION); - String encodedIV = storage.retrieveString(KEY_IV_ALIAS); - if (TextUtils.isEmpty(encodedIV)) { - encodedIV = storage.retrieveString(OLD_KEY_IV_ALIAS); - if (TextUtils.isEmpty(encodedIV)) { - //AES key was JUST generated. If anything existed before, should be encrypted again first. - throw new CryptoException("The encryption keys changed recently. You need to re-encrypt something first.", null); - } + + // Detect format and decrypt accordingly to maintain backward compatibility + if (isNewFormat(encryptedInput)) { + return decryptNewFormat(encryptedInput, cipher, key); + } else { + return decryptLegacyFormat(encryptedInput, cipher, key); } - byte[] iv = Base64.decode(encodedIV, Base64.DEFAULT); - cipher.init(Cipher.DECRYPT_MODE, key, new IvParameterSpec(iv)); - return cipher.doFinal(encryptedInput); - } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | InvalidAlgorithmParameterException e) { + } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | + InvalidAlgorithmParameterException e) { /* * This exceptions are safe to be ignored: * @@ -456,12 +458,104 @@ public byte[] decrypt(byte[] encryptedInput) throws CryptoException, Incompatibl } } + /** + * Checks if the encrypted input uses the new format with bundled IV. + * New format structure: [FORMAT_MARKER][IV_LENGTH][IV][ENCRYPTED_DATA] + * + * @param encryptedInput the encrypted data to check + * @return true if new format, false if legacy format + */ + @VisibleForTesting + boolean isNewFormat(byte[] encryptedInput) { + if (encryptedInput[0] != FORMAT_MARKER) { + return false; + } + + // Check IV length is valid for AES-GCM (12 or 16 bytes) + int ivLength = encryptedInput[1] & 0xFF; + if (ivLength != 12 && ivLength != 16) { + return false; + } + + // Verify minimum total length + // Need: marker(1) + length(1) + IV(12-16) + GCM tag(16) + data(1+) + int minLength = 2 + ivLength + 16 + 1; + return encryptedInput.length >= minLength; + } + + /** + * Decrypts data in the new format (IV bundled with encrypted data). + * + * @param encryptedInput the encrypted input in new format + * @param cipher the cipher instance + * @param key the secret key + * @return the decrypted data + * @throws InvalidKeyException if the key is invalid + * @throws InvalidAlgorithmParameterException if the IV is invalid + * @throws IllegalBlockSizeException if the block size is invalid + * @throws BadPaddingException if padding is incorrect + */ + @VisibleForTesting + private byte[] decryptNewFormat(byte[] encryptedInput, Cipher cipher, SecretKey key) + throws InvalidKeyException, InvalidAlgorithmParameterException, + IllegalBlockSizeException, BadPaddingException { + + // Read IV length (byte 1) + int ivLength = encryptedInput[1] & 0xFF; + + // Extract IV (bytes 2 to 2+ivLength) + byte[] iv = new byte[ivLength]; + System.arraycopy(encryptedInput, 2, iv, 0, ivLength); + + int encryptedDataOffset = 2 + ivLength; + int encryptedDataLength = encryptedInput.length - encryptedDataOffset; + + cipher.init(Cipher.DECRYPT_MODE, key, new IvParameterSpec(iv)); + return cipher.doFinal(encryptedInput, encryptedDataOffset, encryptedDataLength); + } + + /** + * Decrypts data in the legacy format (IV stored separately in storage). + * This maintains backward compatibility with credentials encrypted before the fix. + * + * @param encryptedInput the encrypted input in legacy format + * @param cipher the cipher instance + * @param key the secret key + * @return the decrypted data + * @throws InvalidKeyException if the key is invalid + * @throws InvalidAlgorithmParameterException if the IV is invalid + * @throws IllegalBlockSizeException if the block size is invalid + * @throws BadPaddingException if padding is incorrect + * @throws CryptoException if the IV cannot be found in storage + */ + @VisibleForTesting + private byte[] decryptLegacyFormat(byte[] encryptedInput, Cipher cipher, SecretKey key) + throws InvalidKeyException, InvalidAlgorithmParameterException, + IllegalBlockSizeException, BadPaddingException, CryptoException { + // Retrieve IV from storage (legacy behavior) + String encodedIV = storage.retrieveString(KEY_IV_ALIAS); + if (TextUtils.isEmpty(encodedIV)) { + encodedIV = storage.retrieveString(OLD_KEY_IV_ALIAS); + if (TextUtils.isEmpty(encodedIV)) { + throw new CryptoException("The encryption keys changed recently. You need to re-encrypt something first.", null); + } + } + + byte[] iv = Base64.decode(encodedIV, Base64.DEFAULT); + cipher.init(Cipher.DECRYPT_MODE, key, new IvParameterSpec(iv)); + return cipher.doFinal(encryptedInput); + } + /** * Encrypts the given input bytes using a symmetric key (AES). * The AES key is stored protected by an asymmetric key pair (RSA). + *

+ * The encrypted output uses a new format that bundles the IV with the encrypted data + * to prevent IV collision issues when multiple credentials are stored. + * Format: [FORMAT_MARKER(1)][IV_LENGTH(1)][IV(12-16)][ENCRYPTED_DATA(variable)] * * @param decryptedInput the input bytes to encrypt. There's no limit in size. - * @return the encrypted output bytes + * @return the encrypted output bytes with bundled IV * @throws CryptoException if the RSA Key pair was deemed invalid and got deleted. Operation can be retried. * @throws IncompatibleDeviceException in the event the device can't understand the cryptographic settings required */ @@ -471,10 +565,17 @@ public byte[] encrypt(byte[] decryptedInput) throws CryptoException, Incompatibl Cipher cipher = Cipher.getInstance(AES_TRANSFORMATION); cipher.init(Cipher.ENCRYPT_MODE, key); byte[] encrypted = cipher.doFinal(decryptedInput); - byte[] encodedIV = Base64.encode(cipher.getIV(), Base64.DEFAULT); - //Save IV for Decrypt stage - storage.store(KEY_IV_ALIAS, new String(encodedIV, StandardCharsets.UTF_8)); - return encrypted; + byte[] iv = cipher.getIV(); + + // NEW FORMAT: Bundle IV with encrypted data to prevent collision issues + // Format: [FORMAT_MARKER][IV_LENGTH][IV][ENCRYPTED_DATA] + byte[] output = new byte[1 + 1 + iv.length + encrypted.length]; + output[0] = FORMAT_MARKER; + output[1] = (byte) iv.length; + System.arraycopy(iv, 0, output, 2, iv.length); + System.arraycopy(encrypted, 0, output, 2 + iv.length, encrypted.length); + + return output; } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException e) { /* * This exceptions are safe to be ignored: diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt index 919c5e8f7..40be4c1f8 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt @@ -164,6 +164,7 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT DefaultLocalAuthenticationManagerFactory() ) + /** * Saves the given credentials in the Storage. * @@ -703,20 +704,6 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT continueGetCredentials(scope, minTtl, parameters, headers, forceRefresh, callback) } - private val localAuthenticationResultCallback = - { scope: String?, minTtl: Int, parameters: Map, headers: Map, forceRefresh: Boolean, callback: Callback -> - object : Callback { - override fun onSuccess(result: Boolean) { - continueGetCredentials( - scope, minTtl, parameters, headers, forceRefresh, callback - ) - } - - override fun onFailure(error: CredentialsManagerException) { - callback.onFailure(error) - } - } - } /** * Retrieves API credentials from storage and automatically renews them using the refresh token if the access @@ -742,6 +729,11 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT if (fragmentActivity != null && localAuthenticationOptions != null && localAuthenticationManagerFactory != null) { + if (isBiometricSessionValid()) { + continueGetApiCredentials(audience, scope, minTtl, parameters, headers, callback) + return + } + fragmentActivity.get()?.let { fragmentActivity -> startBiometricAuthentication( fragmentActivity, @@ -975,10 +967,11 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT serialExecutor.execute { val encryptedEncodedJson = storage.retrieveString(audience) //Check if existing api credentials are present and valid + encryptedEncodedJson?.let { encryptedEncoded -> - val encrypted = Base64.decode(encryptedEncoded, Base64.DEFAULT) + val decoded = Base64.decode(encryptedEncoded, Base64.DEFAULT) val json: String = try { - String(crypto.decrypt(encrypted)) + String(crypto.decrypt(decoded)) } catch (e: IncompatibleDeviceException) { callback.onFailure( CredentialsManagerException( @@ -1102,6 +1095,7 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT CredentialsManagerException.Code.INCOMPATIBLE_DEVICE, e ) } catch (e: CryptoException) { + clearCredentials() throw CredentialsManagerException( CredentialsManagerException.Code.CRYPTO_EXCEPTION, e ) @@ -1203,7 +1197,7 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT internal fun isBiometricSessionValid(): Boolean { val lastAuth = lastBiometricAuthTime.get() if (lastAuth == NO_SESSION) return false // No session exists - + val policy = localAuthenticationOptions?.policy ?: BiometricPolicy.Always return when (policy) { is BiometricPolicy.Session, From 20810d9e04dafa9702bc4111632d2b5103854368 Mon Sep 17 00:00:00 2001 From: Prince Mathew Date: Sat, 22 Nov 2025 17:36:49 +0530 Subject: [PATCH 2/5] Added UTs for the new changes --- .../storage/CryptoUtilTest.java | 318 ++++++++++++++++-- 1 file changed, 292 insertions(+), 26 deletions(-) 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 d38035bdd..9633b5fd3 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 @@ -971,26 +971,37 @@ public void shouldAESEncryptData() throws Exception { byte[] aesKey = "aes-decrypted-key".getBytes(); byte[] data = "data".getBytes(); byte[] encryptedData = new byte[]{0, 1, 2, 3, 4, 5}; - byte[] iv = new byte[]{99, 99, 11, 11}; - byte[] encodedIv = "iv-data".getBytes(); + byte[] iv = new byte[]{99, 99, 11, 11, 22, 22, 33, 33, 44, 44, 55, 55}; // 12-byte IV for AES-GCM doReturn(aesKey).when(cryptoUtil).getAESKey(); doReturn(encryptedData).when(aesCipher).doFinal(data); PowerMockito.when(aesCipher.doFinal(data)).thenReturn(encryptedData); PowerMockito.when(aesCipher.getIV()).thenReturn(iv); - PowerMockito.mockStatic(Base64.class); - PowerMockito.when(Base64.encode(iv, Base64.DEFAULT)).thenReturn(encodedIv); final byte[] encrypted = cryptoUtil.encrypt(data); - Mockito.verify(aesCipher).init(eq(Cipher.ENCRYPT_MODE), secretKeyCaptor.capture()); assertThat(secretKeyCaptor.getValue(), is(notNullValue())); assertThat(secretKeyCaptor.getValue().getAlgorithm(), is(ALGORITHM_AES)); assertThat(secretKeyCaptor.getValue().getEncoded(), is(aesKey)); - Mockito.verify(storage).store(KEY_ALIAS + "_iv", "iv-data"); - assertThat(encrypted, is(encryptedData)); + // IV is NO LONGER stored in storage - it's bundled with the encrypted data + Mockito.verify(storage, never()).store(eq(KEY_ALIAS + "_iv"), anyString()); + + assertThat(encrypted, is(notNullValue())); + assertThat(encrypted.length, is(1 + 1 + iv.length + encryptedData.length)); + assertThat(encrypted[0], is((byte) 0x01)); + assertThat(encrypted[1], is((byte) iv.length)); + + // Verify IV is correctly embedded + byte[] extractedIV = new byte[iv.length]; + System.arraycopy(encrypted, 2, extractedIV, 0, iv.length); + assertThat(extractedIV, is(iv)); + + // Verify encrypted data is correctly embedded + byte[] extractedEncrypted = new byte[encryptedData.length]; + System.arraycopy(encrypted, 2 + iv.length, extractedEncrypted, 0, encryptedData.length); + assertThat(extractedEncrypted, is(encryptedData)); } @Test @@ -1117,6 +1128,256 @@ public void shouldThrowButNotDeleteAESKeysOnIllegalBlockSizeExceptionWhenTryingT } + /* + * NEW FORMAT tests + */ + @Test + public void shouldDetectNewFormatWithValidMarkerAndIVLength12() { + // Create new format data: [0x01][12][IV(12 bytes)][encrypted+tag(17 bytes minimum)] + // Min length: 1 + 1 + 12 + 16 (tag) + 1 (data) = 31 bytes + byte[] newFormatData = new byte[31]; + newFormatData[0] = 0x01; // FORMAT_MARKER + newFormatData[1] = 12; // IV length + for (int i = 2; i < newFormatData.length; i++) { + newFormatData[i] = (byte) i; + } + + boolean result = cryptoUtil.isNewFormat(newFormatData); + + assertThat(result, is(true)); + } + + @Test + public void shouldDetectNewFormatWithValidMarkerAndIVLength16() { + // Create new format data: [0x01][16][IV(16 bytes)][encrypted+tag(17 bytes minimum)] + // Min length: 1 + 1 + 16 + 16 (tag) + 1 (data) = 35 bytes + byte[] newFormatData = new byte[35]; + newFormatData[0] = 0x01; // FORMAT_MARKER + newFormatData[1] = 16; // IV length + // Fill with dummy data + for (int i = 2; i < newFormatData.length; i++) { + newFormatData[i] = (byte) i; + } + + boolean result = cryptoUtil.isNewFormat(newFormatData); + + assertThat(result, is(true)); + } + + @Test + public void shouldNotDetectNewFormatWithInvalidMarker() { + // Create data with wrong marker + byte[] invalidData = new byte[30]; + invalidData[0] = 0x02; // Wrong marker + invalidData[1] = 12; // Valid IV length + + boolean result = cryptoUtil.isNewFormat(invalidData); + + assertThat(result, is(false)); + } + + @Test + public void shouldNotDetectNewFormatWithInvalidIVLength() { + // Create data with invalid IV length + byte[] invalidData = new byte[30]; + invalidData[0] = 0x01; // Valid marker + invalidData[1] = 10; // Invalid IV length (not 12 or 16) + + boolean result = cryptoUtil.isNewFormat(invalidData); + + assertThat(result, is(false)); + } + + @Test + public void shouldExtractIVFromNewFormatCorrectly() { + byte[] iv = new byte[]{10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 110, 120}; + byte[] encryptedPayload = new byte[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17}; // At least 17 bytes (16 tag + 1 data) + + byte[] newFormatData = new byte[1 + 1 + iv.length + encryptedPayload.length]; + newFormatData[0] = 0x01; + newFormatData[1] = (byte) iv.length; + System.arraycopy(iv, 0, newFormatData, 2, iv.length); + System.arraycopy(encryptedPayload, 0, newFormatData, 2 + iv.length, encryptedPayload.length); + + // Verify format detection + assertThat(cryptoUtil.isNewFormat(newFormatData), is(true)); + + // Manually extract and verify IV + int ivLength = newFormatData[1] & 0xFF; + assertThat(ivLength, is(12)); + + byte[] extractedIV = new byte[ivLength]; + System.arraycopy(newFormatData, 2, extractedIV, 0, ivLength); + assertThat(extractedIV, is(iv)); + + // Verify encrypted payload position + int dataOffset = 2 + ivLength; + int dataLength = newFormatData.length - dataOffset; + assertThat(dataLength, is(encryptedPayload.length)); + } + + @Test + public void shouldVerifyMinimumLengthRequirements() { + // Minimum valid new format with 12-byte IV: + // 1 (marker) + 1 (length) + 12 (IV) + 16 (GCM tag) + 1 (data) = 31 bytes + byte[] minValid12 = new byte[31]; + minValid12[0] = 0x01; + minValid12[1] = 12; + assertThat(cryptoUtil.isNewFormat(minValid12), is(true)); + + // One byte less should fail + byte[] tooShort12 = new byte[30]; + tooShort12[0] = 0x01; + tooShort12[1] = 12; + assertThat(cryptoUtil.isNewFormat(tooShort12), is(false)); + + // Minimum valid new format with 16-byte IV: + // 1 (marker) + 1 (length) + 16 (IV) + 16 (GCM tag) + 1 (data) = 35 bytes + byte[] minValid16 = new byte[35]; + minValid16[0] = 0x01; + minValid16[1] = 16; + assertThat(cryptoUtil.isNewFormat(minValid16), is(true)); + + // One byte less should fail + byte[] tooShort16 = new byte[34]; + tooShort16[0] = 0x01; + tooShort16[1] = 16; + assertThat(cryptoUtil.isNewFormat(tooShort16), is(false)); + } + + /* + * MIGRATION SCENARIO tests - Testing backward compatibility and format coexistence + */ + @Test + public void shouldDecryptLegacyFormatDataWithIVInStorage() throws Exception { + ArgumentCaptor ivCaptor = ArgumentCaptor.forClass(IvParameterSpec.class); + byte[] aesKey = "aes-decrypted-key".getBytes(); + byte[] originalData = "sensitive-data".getBytes(); + byte[] encryptedData = new byte[]{10, 20, 30, 40, 50, 60}; // Old format + byte[] iv = new byte[]{99, 99, 11, 11, 22, 22, 33, 33, 44, 44, 55, 55}; // 12-byte IV + + // Setup: Old format has IV stored separately in storage + doReturn(aesKey).when(cryptoUtil).getAESKey(); + PowerMockito.when(storage.retrieveString(KEY_ALIAS + "_iv")).thenReturn("encoded-iv-data"); + PowerMockito.mockStatic(Base64.class); + PowerMockito.when(Base64.decode("encoded-iv-data", Base64.DEFAULT)).thenReturn(iv); + PowerMockito.mockStatic(Cipher.class); + PowerMockito.when(Cipher.getInstance(AES_TRANSFORMATION)).thenReturn(aesCipher); + PowerMockito.when(aesCipher.doFinal(encryptedData)).thenReturn(originalData); + + // Execute: Decrypt old format data (should be detected as legacy format) + final byte[] decrypted = cryptoUtil.decrypt(encryptedData); + + // Verify: Should detect as legacy format and use IV from storage + assertThat(cryptoUtil.isNewFormat(encryptedData), is(false)); + Mockito.verify(storage).retrieveString(KEY_ALIAS + "_iv"); + Mockito.verify(aesCipher).init(eq(Cipher.DECRYPT_MODE), any(SecretKey.class), ivCaptor.capture()); + assertThat(ivCaptor.getValue().getIV(), is(iv)); + assertThat(decrypted, is(originalData)); + } + + @Test + public void shouldMigrateFromLegacyFormatToNewFormat() throws Exception { + byte[] aesKey = "aes-decrypted-key".getBytes(); + byte[] originalData = "sensitive-data".getBytes(); + byte[] oldEncryptedData = new byte[]{10, 20, 30, 40, 50, 60}; + byte[] oldIv = new byte[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}; + // New encrypted data must be at least 17 bytes (16-byte GCM tag + 1+ bytes data) + byte[] newEncryptedData = new byte[20]; // 20 bytes to be safe + for (int i = 0; i < newEncryptedData.length; i++) { + newEncryptedData[i] = (byte) (50 + i); + } + byte[] newIv = new byte[]{11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22}; + + doReturn(aesKey).when(cryptoUtil).getAESKey(); + PowerMockito.mockStatic(Cipher.class); + PowerMockito.when(Cipher.getInstance(AES_TRANSFORMATION)).thenReturn(aesCipher); + PowerMockito.mockStatic(Base64.class); + + // Step 1: Decrypt old format (IV from storage) + PowerMockito.when(storage.retrieveString(KEY_ALIAS + "_iv")).thenReturn("old-encoded-iv"); + PowerMockito.when(Base64.decode("old-encoded-iv", Base64.DEFAULT)).thenReturn(oldIv); + PowerMockito.when(aesCipher.doFinal(oldEncryptedData)).thenReturn(originalData); + + byte[] decryptedOld = cryptoUtil.decrypt(oldEncryptedData); + assertThat(decryptedOld, is(originalData)); + assertThat(cryptoUtil.isNewFormat(oldEncryptedData), is(false)); + + // Step 2: Re-encrypt in new format (IV bundled) + PowerMockito.when(aesCipher.doFinal(originalData)).thenReturn(newEncryptedData); + PowerMockito.when(aesCipher.getIV()).thenReturn(newIv); + + byte[] reEncrypted = cryptoUtil.encrypt(originalData); + + // Verify new format structure + assertThat(reEncrypted[0], is((byte) 0x01)); // FORMAT_MARKER + assertThat(reEncrypted[1], is((byte) newIv.length)); + assertThat(cryptoUtil.isNewFormat(reEncrypted), is(true)); + + // Extract and verify IV is bundled + byte[] extractedIV = new byte[newIv.length]; + System.arraycopy(reEncrypted, 2, extractedIV, 0, newIv.length); + assertThat(extractedIV, is(newIv)); + + // Step 3: Decrypt new format (IV bundled in data) + PowerMockito.when(aesCipher.doFinal(any(byte[].class), anyInt(), anyInt())).thenReturn(originalData); + + byte[] decryptedNew = cryptoUtil.decrypt(reEncrypted); + assertThat(decryptedNew, is(originalData)); + + // Verify IV not stored in storage for new format + Mockito.verify(storage, never()).store(eq(KEY_ALIAS + "_iv"), anyString()); + } + + @Test + public void shouldDecryptBothLegacyAndNewFormatInSameSession() throws Exception { + byte[] aesKey = "aes-decrypted-key".getBytes(); + byte[] dataA = "credential-A".getBytes(); + byte[] dataB = "credential-B".getBytes(); + + // Old format encrypted data (no format marker, starts with random byte) + byte[] oldEncrypted = new byte[]{10, 20, 30, 40, 50, 60}; + byte[] oldIv = new byte[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}; + + // New format encrypted data (with format marker and bundled IV) + byte[] newIv = new byte[]{99, 98, 97, 96, 95, 94, 93, 92, 91, 90, 89, 88}; + byte[] newEncryptedPayload = new byte[17]; // 17 bytes (16 tag + 1 data min) + for (int i = 0; i < newEncryptedPayload.length; i++) { + newEncryptedPayload[i] = (byte) (70 + i * 10); + } + byte[] newEncrypted = new byte[1 + 1 + newIv.length + newEncryptedPayload.length]; + newEncrypted[0] = 0x01; // FORMAT_MARKER + newEncrypted[1] = (byte) newIv.length; + System.arraycopy(newIv, 0, newEncrypted, 2, newIv.length); + System.arraycopy(newEncryptedPayload, 0, newEncrypted, 2 + newIv.length, newEncryptedPayload.length); + + doReturn(aesKey).when(cryptoUtil).getAESKey(); + PowerMockito.mockStatic(Cipher.class); + PowerMockito.when(Cipher.getInstance(AES_TRANSFORMATION)).thenReturn(aesCipher); + PowerMockito.mockStatic(Base64.class); + + // Decrypt old format first + PowerMockito.when(storage.retrieveString(KEY_ALIAS + "_iv")).thenReturn("old-iv-encoded"); + PowerMockito.when(Base64.decode("old-iv-encoded", Base64.DEFAULT)).thenReturn(oldIv); + PowerMockito.when(aesCipher.doFinal(oldEncrypted)).thenReturn(dataA); + + byte[] decryptedOld = cryptoUtil.decrypt(oldEncrypted); + assertThat(decryptedOld, is(dataA)); + + // Decrypt new format next + PowerMockito.when(aesCipher.doFinal(any(byte[].class), anyInt(), anyInt())).thenReturn(dataB); + + byte[] decryptedNew = cryptoUtil.decrypt(newEncrypted); + assertThat(decryptedNew, is(dataB)); + + // Verify format detection worked correctly for both + assertThat(cryptoUtil.isNewFormat(oldEncrypted), is(false)); + assertThat(cryptoUtil.isNewFormat(newEncrypted), is(true)); + + // Verify storage was only accessed for old format + Mockito.verify(storage, Mockito.atLeastOnce()).retrieveString(KEY_ALIAS + "_iv"); + } + /* * MAIN DECRYPT (AES) tests */ @@ -1126,28 +1387,33 @@ public void shouldAESDecryptData() throws Exception { ArgumentCaptor secretKeyCaptor = ArgumentCaptor.forClass(SecretKey.class); ArgumentCaptor ivParameterSpecCaptor = ArgumentCaptor.forClass(IvParameterSpec.class); byte[] aesKey = "aes-decrypted-key".getBytes(); - byte[] data = "data".getBytes(); - byte[] decryptedData = new byte[]{0, 1, 2, 3, 4, 5}; - String encodedIv = "iv-data"; + byte[] originalData = "data".getBytes(); + byte[] encryptedPayload = new byte[]{10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 110, 120, 13, 14, 15, 16, 17}; // 17 bytes (16 tag + 1 data) + byte[] iv = new byte[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}; // 12-byte IV - doReturn(aesKey).when(cryptoUtil).getAESKey(); - doReturn(decryptedData).when(aesCipher).doFinal(data); - PowerMockito.when(aesCipher.doFinal(data)).thenReturn(decryptedData); - PowerMockito.when(storage.retrieveString(KEY_ALIAS + "_iv")).thenReturn(encodedIv); - PowerMockito.mockStatic(Base64.class); - PowerMockito.when(Base64.decode(encodedIv, Base64.DEFAULT)).thenReturn(encodedIv.getBytes()); + // Build new format data: [0x01][IV_LENGTH][IV][ENCRYPTED_DATA] + byte[] newFormatData = new byte[1 + 1 + iv.length + encryptedPayload.length]; + newFormatData[0] = 0x01; // FORMAT_MARKER + newFormatData[1] = (byte) iv.length; // IV length + System.arraycopy(iv, 0, newFormatData, 2, iv.length); + System.arraycopy(encryptedPayload, 0, newFormatData, 2 + iv.length, encryptedPayload.length); - final byte[] decrypted = cryptoUtil.decrypt(data); + doReturn(aesKey).when(cryptoUtil).getAESKey(); + PowerMockito.mockStatic(Cipher.class); + PowerMockito.when(Cipher.getInstance(AES_TRANSFORMATION)).thenReturn(aesCipher); + PowerMockito.when(aesCipher.doFinal(any(byte[].class), anyInt(), anyInt())).thenReturn(originalData); + final byte[] decrypted = cryptoUtil.decrypt(newFormatData); + assertThat(cryptoUtil.isNewFormat(newFormatData), is(true)); + Mockito.verify(aesCipher).init(eq(Cipher.DECRYPT_MODE), secretKeyCaptor.capture(), ivParameterSpecCaptor.capture()); assertThat(secretKeyCaptor.getValue(), is(notNullValue())); assertThat(secretKeyCaptor.getValue().getAlgorithm(), is(ALGORITHM_AES)); assertThat(secretKeyCaptor.getValue().getEncoded(), is(aesKey)); assertThat(ivParameterSpecCaptor.getValue(), is(notNullValue())); - assertThat(ivParameterSpecCaptor.getValue().getIV(), is(encodedIv.getBytes())); - - assertThat(decrypted, is(decryptedData)); + assertThat(ivParameterSpecCaptor.getValue().getIV(), is(iv)); + assertThat(decrypted, is(originalData)); } @Test @@ -1215,7 +1481,7 @@ public void shouldThrowOnNoSuchAlgorithmExceptionWhenTryingToAESDecrypt() { } @Test - public void shouldThrowOnEmptyInitializationVectorWhenTryingToAESDecrypt() { + public void shouldThrowOnEmptyInitializationVectorWhenTryingToAESDecryptWithOldFormat() { Assert.assertThrows("The encryption keys changed recently. You need to re-encrypt something first.", CryptoException.class, () -> { doReturn(new byte[]{11, 22, 33}).when(cryptoUtil).getAESKey(); PowerMockito.mockStatic(Cipher.class); @@ -1223,7 +1489,7 @@ public void shouldThrowOnEmptyInitializationVectorWhenTryingToAESDecrypt() { PowerMockito.when(storage.retrieveString(KEY_ALIAS + "_iv")).thenReturn(""); PowerMockito.when(storage.retrieveString(BASE_ALIAS + "_iv")).thenReturn(""); - cryptoUtil.decrypt(new byte[0]); + cryptoUtil.decrypt(new byte[]{12,1,3,14,15,16,17}); }); } @@ -1246,7 +1512,7 @@ public void shouldThrowOnInvalidKeyExceptionWhenTryingToAESDecrypt() throws Exce doThrow(new InvalidKeyException()).when(aesCipher).init(eq(Cipher.DECRYPT_MODE), secretKeyArgumentCaptor.capture(), ivParameterSpecArgumentCaptor.capture()); - cryptoUtil.decrypt(new byte[0]); + cryptoUtil.decrypt(new byte[]{12,13,14,15,16}); } catch (IncompatibleDeviceException e) { exception = e; } @@ -1275,7 +1541,7 @@ public void shouldThrowOnInvalidAlgorithmParameterExceptionWhenTryingToAESDecryp PowerMockito.when(Base64.decode("a_valid_iv", Base64.DEFAULT)).thenReturn(ivBytes); doThrow(new InvalidAlgorithmParameterException()).when(aesCipher).init(eq(Cipher.DECRYPT_MODE), secretKeyArgumentCaptor.capture(), ivParameterSpecArgumentCaptor.capture()); - cryptoUtil.decrypt(new byte[0]); + cryptoUtil.decrypt(new byte[]{12,13,14,15,16,17}); } catch (IncompatibleDeviceException e) { exception = e; } @@ -1302,7 +1568,7 @@ public void shouldThrowButNotDeleteAESKeysOnBadPaddingExceptionWhenTryingToAESDe doThrow(new BadPaddingException()).when(aesCipher).doFinal(any(byte[].class)); - cryptoUtil.decrypt(new byte[0]); + cryptoUtil.decrypt(new byte[]{12,13,14,15,16,17}); }); Mockito.verify(keyStore, never()).deleteEntry(KEY_ALIAS); @@ -1328,7 +1594,7 @@ public void shouldThrowButNotDeleteAESKeysOnIllegalBlockSizeExceptionWhenTryingT doThrow(new IllegalBlockSizeException()).when(aesCipher).doFinal(any(byte[].class)); - cryptoUtil.decrypt(new byte[0]); + cryptoUtil.decrypt(new byte[]{12,13,14,15,16,17}); }); Mockito.verify(keyStore, never()).deleteEntry(KEY_ALIAS); From 4769c6248dfcfa7e1d0f7f46a1eabf2de53df9dc Mon Sep 17 00:00:00 2001 From: Prince Mathew Date: Sun, 23 Nov 2025 17:51:40 +0530 Subject: [PATCH 3/5] addressed feedbacks from claude code PR review --- .../authentication/storage/CryptoUtil.java | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) 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 45bf8ebc9..57cdc1618 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 @@ -66,6 +66,9 @@ class CryptoUtil { private static final byte FORMAT_MARKER = 0x01; + private static final int GCM_TAG_LENGTH = 16; + private static final int MIN_DATA_LENGTH = 1; + private final String OLD_KEY_ALIAS; private final String OLD_KEY_IV_ALIAS; private final String KEY_ALIAS; @@ -467,11 +470,22 @@ public byte[] decrypt(byte[] encryptedInput) throws CryptoException, Incompatibl */ @VisibleForTesting boolean isNewFormat(byte[] encryptedInput) { + + // Boundary check + if (encryptedInput == null || encryptedInput.length < 2) { + return false; + } + if (encryptedInput[0] != FORMAT_MARKER) { return false; } // Check IV length is valid for AES-GCM (12 or 16 bytes) + // AES is a 128 block size cipher ,which is 16 bytes + // AES in GCM mode the recommended IV length is 12 bytes. + // This 12-byte IV is then combined with a 4-byte internal counter to form the full 16-byte + // input block for the underlying AES block cipher in counter mode (CTR), which GCM utilizes. + // Thus checking for a 12 or 16 byte length int ivLength = encryptedInput[1] & 0xFF; if (ivLength != 12 && ivLength != 16) { return false; @@ -479,7 +493,7 @@ boolean isNewFormat(byte[] encryptedInput) { // Verify minimum total length // Need: marker(1) + length(1) + IV(12-16) + GCM tag(16) + data(1+) - int minLength = 2 + ivLength + 16 + 1; + int minLength = 2 + ivLength + GCM_TAG_LENGTH + MIN_DATA_LENGTH; return encryptedInput.length >= minLength; } From b5cda3476bb617caf2b3c078c3dc9bb639ef9695 Mon Sep 17 00:00:00 2001 From: Prince Mathew Date: Sun, 23 Nov 2025 19:51:28 +0530 Subject: [PATCH 4/5] More review comments addressed --- .../com/auth0/android/authentication/storage/CryptoUtil.java | 5 +++-- .../authentication/storage/SecureCredentialsManager.kt | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) 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 57cdc1618..959723a31 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 @@ -68,6 +68,7 @@ class CryptoUtil { private static final int GCM_TAG_LENGTH = 16; private static final int MIN_DATA_LENGTH = 1; + private static final int FORMAT_HEADER_LENGTH = 2; private final String OLD_KEY_ALIAS; private final String OLD_KEY_IV_ALIAS; @@ -469,7 +470,7 @@ public byte[] decrypt(byte[] encryptedInput) throws CryptoException, Incompatibl * @return true if new format, false if legacy format */ @VisibleForTesting - boolean isNewFormat(byte[] encryptedInput) { + private boolean isNewFormat(byte[] encryptedInput) { // Boundary check if (encryptedInput == null || encryptedInput.length < 2) { @@ -493,7 +494,7 @@ boolean isNewFormat(byte[] encryptedInput) { // Verify minimum total length // Need: marker(1) + length(1) + IV(12-16) + GCM tag(16) + data(1+) - int minLength = 2 + ivLength + GCM_TAG_LENGTH + MIN_DATA_LENGTH; + int minLength = FORMAT_HEADER_LENGTH + ivLength + GCM_TAG_LENGTH + MIN_DATA_LENGTH; return encryptedInput.length >= minLength; } diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt index 40be4c1f8..cd209c259 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt @@ -969,9 +969,9 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT //Check if existing api credentials are present and valid encryptedEncodedJson?.let { encryptedEncoded -> - val decoded = Base64.decode(encryptedEncoded, Base64.DEFAULT) + val encrypted = Base64.decode(encryptedEncoded, Base64.DEFAULT) val json: String = try { - String(crypto.decrypt(decoded)) + String(crypto.decrypt(encrypted)) } catch (e: IncompatibleDeviceException) { callback.onFailure( CredentialsManagerException( From 7784bf9abf990d823b1a64be660183e8e5a862d6 Mon Sep 17 00:00:00 2001 From: Prince Mathew Date: Sun, 23 Nov 2025 20:04:49 +0530 Subject: [PATCH 5/5] Added new test case for corner case of iv length --- .../authentication/storage/CryptoUtil.java | 4 +-- .../storage/CryptoUtilTest.java | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) 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 959723a31..846228b24 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 @@ -469,8 +469,8 @@ public byte[] decrypt(byte[] encryptedInput) throws CryptoException, Incompatibl * @param encryptedInput the encrypted data to check * @return true if new format, false if legacy format */ - @VisibleForTesting - private boolean isNewFormat(byte[] encryptedInput) { + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + boolean isNewFormat(byte[] encryptedInput) { // Boundary check if (encryptedInput == null || encryptedInput.length < 2) { 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 9633b5fd3..466de58c2 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 @@ -1245,6 +1245,34 @@ public void shouldVerifyMinimumLengthRequirements() { assertThat(cryptoUtil.isNewFormat(tooShort16), is(false)); } + @Test + public void shouldRejectInvalidIVLengthsInNewFormat() { + byte[] ivLength0 = new byte[19]; + ivLength0[0] = 0x01; + ivLength0[1] = 0; + assertThat(cryptoUtil.isNewFormat(ivLength0), is(false)); + + byte[] ivLength13 = new byte[32]; + ivLength13[0] = 0x01; + ivLength13[1] = 13; + assertThat(cryptoUtil.isNewFormat(ivLength13), is(false)); + + byte[] ivLength14 = new byte[33]; + ivLength14[0] = 0x01; + ivLength14[1] = 14; + assertThat(cryptoUtil.isNewFormat(ivLength14), is(false)); + + byte[] ivLength15 = new byte[34]; + ivLength15[0] = 0x01; + ivLength15[1] = 15; + assertThat(cryptoUtil.isNewFormat(ivLength15), is(false)); + + byte[] ivLength255 = new byte[274]; + ivLength255[0] = 0x01; + ivLength255[1] = (byte) 255; + assertThat(cryptoUtil.isNewFormat(ivLength255), is(false)); + } + /* * MIGRATION SCENARIO tests - Testing backward compatibility and format coexistence */