From 607cb38d383cabcdfcbc293471adf41cc074a4af Mon Sep 17 00:00:00 2001 From: Luciano Balmaceda Date: Tue, 15 Jan 2019 17:30:26 -0300 Subject: [PATCH 1/6] delete keys and stored credentials on unrecoverable use cases --- .../authentication/storage/CryptoUtil.java | 15 +++++---------- .../storage/SecureCredentialsManager.java | 7 +++++++ .../storage/UnrecoverableContentException.java | 11 +++++++++++ 3 files changed, 23 insertions(+), 10 deletions(-) create mode 100644 auth0/src/main/java/com/auth0/android/authentication/storage/UnrecoverableContentException.java 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 8181da312..5d262f7fa 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 @@ -79,7 +79,7 @@ public CryptoUtil(@NonNull Context context, @NonNull Storage storage, @NonNull S } @VisibleForTesting - KeyStore.PrivateKeyEntry getRSAKeyEntry() throws KeyException { + KeyStore.PrivateKeyEntry getRSAKeyEntry() throws KeyException, UnrecoverableEntryException { try { KeyStore keyStore = KeyStore.getInstance(ANDROID_KEY_STORE); keyStore.load(null); @@ -138,11 +138,6 @@ KeyStore.PrivateKeyEntry getRSAKeyEntry() throws KeyException { } catch (KeyStoreException | IOException | NoSuchProviderException | InvalidAlgorithmParameterException | NoSuchAlgorithmException | CertificateException e) { Log.e(TAG, "An error occurred while trying to obtain the RSA KeyPair Entry from the Android KeyStore.", e); throw new KeyException("An error occurred while trying to obtain the RSA KeyPair Entry from the Android KeyStore.", e); - } catch (UnrecoverableEntryException e) { - //Remove keys and Retry - Log.w(TAG, "RSA KeyPair was deemed unrecoverable. Deleting the existing entry and trying again."); - deleteKeys(); - return getRSAKeyEntry(); } } @@ -185,11 +180,11 @@ byte[] RSADecrypt(byte[] encryptedInput) throws CryptoException { return cipher.doFinal(encryptedInput); } catch (NoSuchAlgorithmException | NoSuchPaddingException | KeyException e) { throw new CryptoException("Couldn't decrypt the input using the RSA Key.", e); - } catch (BadPaddingException | IllegalBlockSizeException e) { + } catch (BadPaddingException | IllegalBlockSizeException | UnrecoverableEntryException e) { deleteKeys(); Log.e(TAG, "The input contained unexpected content, probably because it was encrypted using a different key. " + "The existing keys have been deleted and a new pair will be created next time. Please try to encrypt the content again.", e); - return new byte[]{}; + throw new CryptoException("Couldn't decrypt the input using the RSA Key.", new UnrecoverableContentException(e)); } } @@ -203,11 +198,11 @@ byte[] RSAEncrypt(byte[] decryptedInput) throws CryptoException { return cipher.doFinal(decryptedInput); } catch (NoSuchAlgorithmException | NoSuchPaddingException | KeyException e) { throw new CryptoException("Couldn't encrypt the input using the RSA Key.", e); - } catch (BadPaddingException | IllegalBlockSizeException e) { + } catch (BadPaddingException | IllegalBlockSizeException | UnrecoverableEntryException e) { deleteKeys(); Log.e(TAG, "The input contained unexpected content and it was deemed unrecoverable." + " The existing keys have been deleted and a new pair will be created next time.", e); - return new byte[]{}; + throw new CryptoException("Couldn't encrypt the input using the RSA Key.", new UnrecoverableContentException(e)); } } diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java index e9f1fb9df..386a137e1 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java @@ -148,6 +148,9 @@ public void saveCredentials(@NonNull Credentials credentials) throws Credentials storage.store(KEY_CREDENTIALS, encryptedEncoded); storage.store(KEY_EXPIRES_AT, expiresAt); storage.store(KEY_CAN_REFRESH, canRefresh); + } catch (UnrecoverableContentException e) { + //If keys were invalidated, a retry will work fine for the "save credentials" use case. + saveCredentials(credentials); } catch (CryptoException e) { throw new CredentialsManagerException("An error occurred while encrypting the credentials.", e); } @@ -207,6 +210,10 @@ private void continueGetCredentials(final BaseCallback Date: Wed, 16 Jan 2019 16:59:16 -0300 Subject: [PATCH 2/6] fix existing and add new crypto tests --- .../storage/SecureCredentialsManager.java | 1 + .../storage/CryptoUtilTest.java | 151 +++++++++++-- .../storage/SecureCredentialsManagerTest.java | 213 +++++++++++------- 3 files changed, 274 insertions(+), 91 deletions(-) diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java index 386a137e1..b4d2aaf6b 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java @@ -215,6 +215,7 @@ private void continueGetCredentials(final BaseCallback varargsCaptor = ArgumentCaptor.forClass(Object.class); PowerMockito.when(keyStore.containsAlias(KEY_ALIAS)).thenReturn(true); + PowerMockito.when(keyStore.getKey(KEY_ALIAS, null)).thenReturn(privateKey); + PowerMockito.when(keyStore.getCertificate(KEY_ALIAS)).thenReturn(certificate); + PowerMockito.whenNew(KeyStore.PrivateKeyEntry.class).withAnyArguments().thenReturn(entry); + + KeyStore.PrivateKeyEntry rsaEntry = cryptoUtil.getRSAKeyEntry(); + PowerMockito.verifyNew(KeyStore.PrivateKeyEntry.class).withArguments(varargsCaptor.capture()); + assertThat(rsaEntry, is(notNullValue())); + assertThat(rsaEntry, is(entry)); + assertThat(varargsCaptor.getAllValues(), is(notNullValue())); + PrivateKey capturedPrivateKey = (PrivateKey) varargsCaptor.getAllValues().get(0); + Certificate[] capturedCertificatesArray = (Certificate[]) varargsCaptor.getAllValues().get(1); + assertThat(capturedPrivateKey, is(privateKey)); + assertThat(capturedCertificatesArray[0], is(certificate)); + assertThat(capturedCertificatesArray.length, is(1)); + } + + @RequiresApi(api = Build.VERSION_CODES.P) + @Test + @Config(sdk = 28) + public void shouldUseExistingRSAKeyPairOnAPI28AndUp() throws Exception { + ReflectionHelpers.setStaticField(Build.VERSION.class, "SDK_INT", 28); + KeyStore.PrivateKeyEntry entry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); PowerMockito.when(keyStore.getEntry(KEY_ALIAS, null)).thenReturn(entry); + PrivateKey privateKey = null; + PowerMockito.when(keyStore.containsAlias(KEY_ALIAS)).thenReturn(true); + PowerMockito.when(keyStore.getKey(KEY_ALIAS, null)).thenReturn(privateKey); KeyStore.PrivateKeyEntry rsaEntry = cryptoUtil.getRSAKeyEntry(); assertThat(rsaEntry, is(notNullValue())); assertThat(rsaEntry, is(entry)); } + @RequiresApi(api = Build.VERSION_CODES.O_MR1) @Test - public void shouldDeleteKeysOnUnrecoverableErrorWhenTryingToObtainRSAKeys() throws Exception { + @Config(sdk = 27) + public void shouldUseExistingRSAKeyPairOnAPI27AndDown() throws Exception { + ReflectionHelpers.setStaticField(Build.VERSION.class, "SDK_INT", 27); + KeyStore.PrivateKeyEntry entry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); + PowerMockito.when(keyStore.containsAlias(KEY_ALIAS)).thenReturn(true); + PowerMockito.when(keyStore.getEntry(KEY_ALIAS, null)).thenReturn(entry); + + KeyStore.PrivateKeyEntry rsaEntry = cryptoUtil.getRSAKeyEntry(); + assertThat(rsaEntry, is(notNullValue())); + assertThat(rsaEntry, is(entry)); + } + + @Test + public void shouldThrowUnrecoverableEntryExceptionOnUnrecoverableErrorWhenTryingToObtainRSAKeys() throws Exception { + exception.expect(UnrecoverableEntryException.class); + KeyStore.PrivateKeyEntry entry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); PowerMockito.when(keyStore.containsAlias(KEY_ALIAS)).thenReturn(true); PowerMockito.when(keyStore.getEntry(KEY_ALIAS, null)) @@ -396,10 +445,6 @@ public void shouldDeleteKeysOnUnrecoverableErrorWhenTryingToObtainRSAKeys() thro .thenReturn(entry); cryptoUtil.getRSAKeyEntry(); - - Mockito.verify(keyStore).deleteEntry(KEY_ALIAS); - Mockito.verify(storage).remove(KEY_ALIAS); - Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); } @Test @@ -651,7 +696,11 @@ public void shouldThrowOnNoSuchPaddingErrorWhenTryingToRSADecrypt() throws Excep } @Test - public void shouldDeleteKeysOnErrorWhenTryingToRSADecrypt() throws Exception { + public void shouldDeleteKeysOnBadPaddingExceptionWhenTryingToRSADecrypt() throws Exception { + exception.expect(CryptoException.class); + exception.expectMessage("Couldn't decrypt the input using the RSA Key."); + exception.expectCause(CoreMatchers.instanceOf(UnrecoverableContentException.class)); + PrivateKey privateKey = PowerMockito.mock(PrivateKey.class); KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); doReturn(privateKey).when(privateKeyEntry).getPrivateKey(); @@ -659,16 +708,54 @@ public void shouldDeleteKeysOnErrorWhenTryingToRSADecrypt() throws Exception { doThrow(new BadPaddingException()).when(rsaCipher).doFinal(any(byte[].class)); cryptoUtil.RSADecrypt(new byte[0]); + + Mockito.verify(keyStore).deleteEntry(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); + } + + @Test + public void shouldDeleteKeysOnIllegalBlockSizeExceptionWhenTryingToRSADecrypt() throws Exception { + exception.expect(CryptoException.class); + exception.expectMessage("Couldn't decrypt the input using the RSA Key."); + exception.expectCause(CoreMatchers.instanceOf(UnrecoverableContentException.class)); + + PrivateKey privateKey = PowerMockito.mock(PrivateKey.class); + KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); + doReturn(privateKey).when(privateKeyEntry).getPrivateKey(); + doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry(); + doThrow(new IllegalBlockSizeException()).when(rsaCipher).doFinal(any(byte[].class)); cryptoUtil.RSADecrypt(new byte[0]); - Mockito.verify(keyStore, Mockito.times(2)).deleteEntry(KEY_ALIAS); - Mockito.verify(storage, Mockito.times(2)).remove(KEY_ALIAS); - Mockito.verify(storage, Mockito.times(2)).remove(KEY_ALIAS + "_iv"); + Mockito.verify(keyStore).deleteEntry(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); + } + + @Test + public void shouldDeleteKeysOnUnrecoverableEntryExceptionWhenTryingToRSADecrypt() throws Exception { + exception.expect(CryptoException.class); + exception.expectMessage("Couldn't decrypt the input using the RSA Key."); + exception.expectCause(CoreMatchers.instanceOf(UnrecoverableContentException.class)); + + PrivateKey privateKey = PowerMockito.mock(PrivateKey.class); + KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); + doReturn(privateKey).when(privateKeyEntry).getPrivateKey(); + doThrow(new UnrecoverableEntryException()).when(cryptoUtil).getRSAKeyEntry(); + cryptoUtil.RSADecrypt(new byte[0]); + + Mockito.verify(keyStore).deleteEntry(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); } @Test - public void shouldDeleteKeysOnErrorWhenTryingToRSAEncrypt() throws Exception { + public void shouldDeleteKeysOnBadPaddingExceptionWhenTryingToRSAEncrypt() throws Exception { + exception.expect(CryptoException.class); + exception.expectMessage("Couldn't encrypt the input using the RSA Key."); + exception.expectCause(CoreMatchers.instanceOf(UnrecoverableContentException.class)); + Certificate certificate = PowerMockito.mock(Certificate.class); KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); doReturn(certificate).when(privateKeyEntry).getCertificate(); @@ -676,12 +763,48 @@ public void shouldDeleteKeysOnErrorWhenTryingToRSAEncrypt() throws Exception { doThrow(new BadPaddingException()).when(rsaCipher).doFinal(any(byte[].class)); cryptoUtil.RSAEncrypt(new byte[0]); + + Mockito.verify(keyStore).deleteEntry(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); + } + + @Test + public void shouldDeleteKeysOnIllegalBlockSizeExceptionWhenTryingToRSAEncrypt() throws Exception { + exception.expect(CryptoException.class); + exception.expectMessage("Couldn't encrypt the input using the RSA Key."); + exception.expectCause(CoreMatchers.instanceOf(UnrecoverableContentException.class)); + + Certificate certificate = PowerMockito.mock(Certificate.class); + KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); + doReturn(certificate).when(privateKeyEntry).getCertificate(); + doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry(); + doThrow(new IllegalBlockSizeException()).when(rsaCipher).doFinal(any(byte[].class)); cryptoUtil.RSAEncrypt(new byte[0]); - Mockito.verify(keyStore, Mockito.times(2)).deleteEntry(KEY_ALIAS); - Mockito.verify(storage, Mockito.times(2)).remove(KEY_ALIAS); - Mockito.verify(storage, Mockito.times(2)).remove(KEY_ALIAS + "_iv"); + Mockito.verify(keyStore).deleteEntry(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); + } + + @Test + public void shouldDeleteKeysOnUnrecoverableEntryExceptionWhenTryingToRSAEncrypt() throws Exception { + exception.expect(CryptoException.class); + exception.expectMessage("Couldn't encrypt the input using the RSA Key."); + exception.expectCause(CoreMatchers.instanceOf(UnrecoverableContentException.class)); + + Certificate certificate = PowerMockito.mock(Certificate.class); + KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); + doReturn(certificate).when(privateKeyEntry).getCertificate(); + doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry(); + + doThrow(new UnrecoverableEntryException()).when(cryptoUtil).getRSAKeyEntry(); + cryptoUtil.RSAEncrypt(new byte[0]); + + Mockito.verify(keyStore).deleteEntry(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); } @Test diff --git a/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.java b/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.java index d2e08444f..77dfb3b01 100644 --- a/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.java +++ b/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.java @@ -19,6 +19,7 @@ import com.google.gson.Gson; import org.hamcrest.core.Is; +import org.hamcrest.core.IsInstanceOf; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -50,6 +51,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; @@ -159,7 +161,34 @@ public void shouldSaveNonRefreshableCredentialsInStorage() throws Exception { } @Test - public void shouldThrowOnSetIfCryptoError() throws Exception { + public void shouldRetryOnceOnSaveIfUnrecoverableContentException() throws Exception { + long expirationTime = CredentialsMock.CURRENT_TIME_MS + 123456 * 1000; + Credentials credentials = new CredentialsMock("idToken", "accessToken", "type", null, new Date(expirationTime), "scope"); + String json = gson.toJson(credentials); + when(crypto.encrypt(json.getBytes())).thenThrow(new UnrecoverableContentException(null)).thenReturn(json.getBytes()); + + manager.saveCredentials(credentials); + + verify(crypto, times(2)).encrypt(any(byte[].class)); + verify(storage).store(eq("com.auth0.credentials"), stringCaptor.capture()); + verify(storage).store("com.auth0.credentials_expires_at", expirationTime); + verify(storage).store("com.auth0.credentials_can_refresh", false); + verifyNoMoreInteractions(storage); + final String encodedJson = stringCaptor.getValue(); + assertThat(encodedJson, is(notNullValue())); + final byte[] decoded = Base64.decode(encodedJson, Base64.DEFAULT); + Credentials storedCredentials = gson.fromJson(new String(decoded), Credentials.class); + assertThat(storedCredentials.getAccessToken(), is("accessToken")); + assertThat(storedCredentials.getIdToken(), is("idToken")); + assertThat(storedCredentials.getRefreshToken(), is(nullValue())); + assertThat(storedCredentials.getType(), is("type")); + assertThat(storedCredentials.getExpiresAt(), is(notNullValue())); + assertThat(storedCredentials.getExpiresAt().getTime(), is(expirationTime)); + assertThat(storedCredentials.getScope(), is("scope")); + } + + @Test + public void shouldThrowOnSaveIfCryptoError() throws Exception { exception.expect(CredentialsManagerException.class); exception.expectMessage("An error occurred while encrypting the credentials."); @@ -170,7 +199,7 @@ public void shouldThrowOnSetIfCryptoError() throws Exception { } @Test - public void shouldThrowOnSetIfCredentialsDoesNotHaveIdTokenOrAccessToken() throws Exception { + public void shouldThrowOnSaveIfCredentialsDoesNotHaveIdTokenOrAccessToken() throws Exception { exception.expect(CredentialsManagerException.class); exception.expectMessage("Credentials must have a valid date of expiration and a valid access_token or id_token value."); @@ -180,7 +209,7 @@ public void shouldThrowOnSetIfCredentialsDoesNotHaveIdTokenOrAccessToken() throw @SuppressWarnings("ConstantConditions") @Test - public void shouldThrowOnSetIfCredentialsDoesNotHaveExpiresAt() throws Exception { + public void shouldThrowOnSaveIfCredentialsDoesNotHaveExpiresAt() throws Exception { exception.expect(CredentialsManagerException.class); exception.expectMessage("Credentials must have a valid date of expiration and a valid access_token or id_token value."); @@ -190,30 +219,65 @@ public void shouldThrowOnSetIfCredentialsDoesNotHaveExpiresAt() throws Exception } @Test - public void shouldNotThrowOnSetIfCredentialsHaveAccessTokenAndExpiresIn() throws Exception { + public void shouldNotThrowOnSaveIfCredentialsHaveAccessTokenAndExpiresIn() throws Exception { Credentials credentials = new CredentialsMock(null, "accessToken", "type", "refreshToken", 123456L); when(crypto.encrypt(any(byte[].class))).thenReturn(new byte[]{12, 34, 56, 78}); manager.saveCredentials(credentials); } @Test - public void shouldNotThrowOnSetIfCredentialsHaveIdTokenAndExpiresIn() throws Exception { + public void shouldNotThrowOnSaveIfCredentialsHaveIdTokenAndExpiresIn() throws Exception { Credentials credentials = new CredentialsMock("idToken", null, "type", "refreshToken", 123456L); when(crypto.encrypt(any(byte[].class))).thenReturn(new byte[]{12, 34, 56, 78}); manager.saveCredentials(credentials); } @Test - public void shouldFailOnGetCredentialsWhenNoAccessTokenOrIdTokenWasSaved() throws Exception { + public void shouldFailOnGetCredentialsWhenCryptoExceptionIsThrown() throws Exception { verifyNoMoreInteractions(client); - long expirationTime = CredentialsMock.CURRENT_TIME_MS + 123456L * 1000; - Credentials storedCredentials = new Credentials(null, null, "type", "refreshToken", new Date(expirationTime), "scope"); - String storedJson = gson.toJson(storedCredentials); - String encoded = new String(Base64.encode(storedJson.getBytes(), Base64.DEFAULT)); - when(crypto.decrypt(storedJson.getBytes())).thenReturn(storedJson.getBytes()); - when(storage.retrieveString("com.auth0.credentials")).thenReturn(encoded); + Date expiresAt = new Date(CredentialsMock.CURRENT_TIME_MS + 123456L * 1000); + String storedJson = insertTestCredentials(true, true, true, expiresAt); + when(crypto.decrypt(storedJson.getBytes())).thenThrow(new CryptoException("This just happened", null)); + manager.getCredentials(callback); + verify(callback).onFailure(exceptionCaptor.capture()); + CredentialsManagerException exception = exceptionCaptor.getValue(); + assertThat(exception, is(notNullValue())); + assertThat(exception.getCause(), IsInstanceOf.instanceOf(CryptoException.class)); + assertThat(exception.getMessage(), is("An error occurred while decrypting the existing credentials.")); + + verify(storage, never()).remove("com.auth0.credentials"); + verify(storage, never()).remove("com.auth0.credentials_expires_at"); + verify(storage, never()).remove("com.auth0.credentials_can_refresh"); + } + + @Test + public void shouldFailOnGetCredentialsAndClearStoredCredentialsWhenUnrecoverableContentExceptionIsThrown() throws Exception { + verifyNoMoreInteractions(client); + + Date expiresAt = new Date(CredentialsMock.CURRENT_TIME_MS + 123456L * 1000); + String storedJson = insertTestCredentials(true, true, true, expiresAt); + when(crypto.decrypt(storedJson.getBytes())).thenThrow(new UnrecoverableContentException(null)); + manager.getCredentials(callback); + + verify(callback).onFailure(exceptionCaptor.capture()); + CredentialsManagerException exception = exceptionCaptor.getValue(); + assertThat(exception, is(notNullValue())); + assertThat(exception.getCause(), IsInstanceOf.instanceOf(UnrecoverableContentException.class)); + assertThat(exception.getMessage(), is("An error occurred while decrypting the existing credentials.")); + + verify(storage).remove("com.auth0.credentials"); + verify(storage).remove("com.auth0.credentials_expires_at"); + verify(storage).remove("com.auth0.credentials_can_refresh"); + } + + @Test + public void shouldFailOnGetCredentialsWhenNoAccessTokenOrIdTokenWasSaved() throws Exception { + verifyNoMoreInteractions(client); + + Date expiresAt = new Date(CredentialsMock.CURRENT_TIME_MS + 123456L * 1000); + insertTestCredentials(false, false, true, expiresAt); manager.getCredentials(callback); verify(callback).onFailure(exceptionCaptor.capture()); @@ -226,13 +290,7 @@ public void shouldFailOnGetCredentialsWhenNoAccessTokenOrIdTokenWasSaved() throw public void shouldFailOnGetCredentialsWhenNoExpirationTimeWasSaved() throws Exception { verifyNoMoreInteractions(client); - Date expiresAt = null; - Credentials storedCredentials = new Credentials("idToken", "accessToken", "type", "refreshToken", expiresAt, "scope"); - String storedJson = gson.toJson(storedCredentials); - String encoded = new String(Base64.encode(storedJson.getBytes(), Base64.DEFAULT)); - when(crypto.decrypt(storedJson.getBytes())).thenReturn(storedJson.getBytes()); - when(storage.retrieveString("com.auth0.credentials")).thenReturn(encoded); - + insertTestCredentials(false, false, true, null); manager.getCredentials(callback); verify(callback).onFailure(exceptionCaptor.capture()); @@ -247,14 +305,7 @@ public void shouldFailOnGetCredentialsWhenExpiredAndNoRefreshTokenWasSaved() thr verifyNoMoreInteractions(client); Date expiresAt = new Date(CredentialsMock.CURRENT_TIME_MS); //Same as current time --> expired - Credentials storedCredentials = new Credentials("idToken", "accessToken", "type", null, expiresAt, "scope"); - when(storage.retrieveLong("com.auth0.credentials_expires_at")).thenReturn(expiresAt.getTime()); - when(storage.retrieveBoolean("com.auth0.credentials_can_refresh")).thenReturn(false); - String storedJson = gson.toJson(storedCredentials); - String encoded = new String(Base64.encode(storedJson.getBytes(), Base64.DEFAULT)); - when(crypto.decrypt(storedJson.getBytes())).thenReturn(storedJson.getBytes()); - when(storage.retrieveString("com.auth0.credentials")).thenReturn(encoded); - + insertTestCredentials(true, true, false, expiresAt); manager.getCredentials(callback); verify(callback).onFailure(exceptionCaptor.capture()); @@ -268,13 +319,7 @@ public void shouldGetNonExpiredCredentialsFromStorage() throws Exception { verifyNoMoreInteractions(client); Date expiresAt = new Date(CredentialsMock.CURRENT_TIME_MS + 123456L * 1000); - Credentials storedCredentials = new Credentials("idToken", "accessToken", "type", "refreshToken", expiresAt, "scope"); - String storedJson = gson.toJson(storedCredentials); - String encoded = new String(Base64.encode(storedJson.getBytes(), Base64.DEFAULT)); - when(crypto.decrypt(storedJson.getBytes())).thenReturn(storedJson.getBytes()); - when(storage.retrieveString("com.auth0.credentials")).thenReturn(encoded); - when(storage.retrieveLong("com.auth0.credentials_expires_at")).thenReturn(expiresAt.getTime()); - when(storage.retrieveBoolean("com.auth0.credentials_can_refresh")).thenReturn(false); + insertTestCredentials(true, true, true, expiresAt); manager.getCredentials(callback); verify(callback).onSuccess(credentialsCaptor.capture()); @@ -295,13 +340,8 @@ public void shouldGetNonExpiredCredentialsFromStorageWhenOnlyIdTokenIsAvailable( verifyNoMoreInteractions(client); Date expiresAt = new Date(CredentialsMock.CURRENT_TIME_MS + 123456L * 1000); - Credentials storedCredentials = new Credentials("idToken", null, "type", "refreshToken", expiresAt, "scope"); - String storedJson = gson.toJson(storedCredentials); - String encoded = new String(Base64.encode(storedJson.getBytes(), Base64.DEFAULT)); - when(crypto.decrypt(storedJson.getBytes())).thenReturn(storedJson.getBytes()); - when(storage.retrieveString("com.auth0.credentials")).thenReturn(encoded); - when(storage.retrieveLong("com.auth0.credentials_expires_at")).thenReturn(expiresAt.getTime()); - when(storage.retrieveBoolean("com.auth0.credentials_can_refresh")).thenReturn(false); + insertTestCredentials(true, false, true, expiresAt); + manager.getCredentials(callback); verify(callback).onSuccess(credentialsCaptor.capture()); @@ -322,13 +362,7 @@ public void shouldGetNonExpiredCredentialsFromStorageWhenOnlyAccessTokenIsAvaila verifyNoMoreInteractions(client); Date expiresAt = new Date(CredentialsMock.CURRENT_TIME_MS + 123456L * 1000); - Credentials storedCredentials = new Credentials(null, "accessToken", "type", "refreshToken", expiresAt, "scope"); - String storedJson = gson.toJson(storedCredentials); - String encoded = new String(Base64.encode(storedJson.getBytes(), Base64.DEFAULT)); - when(crypto.decrypt(storedJson.getBytes())).thenReturn(storedJson.getBytes()); - when(storage.retrieveString("com.auth0.credentials")).thenReturn(encoded); - when(storage.retrieveLong("com.auth0.credentials_expires_at")).thenReturn(expiresAt.getTime()); - when(storage.retrieveBoolean("com.auth0.credentials_can_refresh")).thenReturn(false); + insertTestCredentials(false, true, true, expiresAt); manager.getCredentials(callback); verify(callback).onSuccess(credentialsCaptor.capture()); @@ -348,13 +382,7 @@ public void shouldGetNonExpiredCredentialsFromStorageWhenOnlyAccessTokenIsAvaila @Test public void shouldGetAndSuccessfullyRenewExpiredCredentials() throws Exception { Date expiresAt = new Date(CredentialsMock.CURRENT_TIME_MS); - Credentials storedCredentials = new Credentials(null, "accessToken", "type", "refreshToken", expiresAt, "scope"); - String storedJson = gson.toJson(storedCredentials); - String encoded = new String(Base64.encode(storedJson.getBytes(), Base64.DEFAULT)); - when(crypto.decrypt(storedJson.getBytes())).thenReturn(storedJson.getBytes()); - when(storage.retrieveString("com.auth0.credentials")).thenReturn(encoded); - when(storage.retrieveLong("com.auth0.credentials_expires_at")).thenReturn(expiresAt.getTime()); - when(storage.retrieveBoolean("com.auth0.credentials_can_refresh")).thenReturn(true); + insertTestCredentials(false, true, true, expiresAt); when(client.renewAuth("refreshToken")).thenReturn(request); manager.getCredentials(callback); @@ -401,13 +429,7 @@ public void shouldGetAndSuccessfullyRenewExpiredCredentials() throws Exception { @Test public void shouldGetAndFailToRenewExpiredCredentials() throws Exception { Date expiresAt = new Date(CredentialsMock.CURRENT_TIME_MS); - Credentials storedCredentials = new Credentials(null, "accessToken", "type", "refreshToken", expiresAt, "scope"); - String storedJson = gson.toJson(storedCredentials); - String encoded = new String(Base64.encode(storedJson.getBytes(), Base64.DEFAULT)); - when(crypto.decrypt(storedJson.getBytes())).thenReturn(storedJson.getBytes()); - when(storage.retrieveString("com.auth0.credentials")).thenReturn(encoded); - when(storage.retrieveLong("com.auth0.credentials_expires_at")).thenReturn(expiresAt.getTime()); - when(storage.retrieveBoolean("com.auth0.credentials_can_refresh")).thenReturn(true); + insertTestCredentials(false, true, true, expiresAt); when(client.renewAuth("refreshToken")).thenReturn(request); manager.getCredentials(callback); @@ -566,13 +588,7 @@ public void shouldRequireAuthenticationIfAPI23AndLockScreenEnabled() throws Exce @Test public void shouldGetCredentialsAfterAuthentication() throws Exception { Date expiresAt = new Date(CredentialsMock.CURRENT_TIME_MS + 123456L * 1000); - Credentials storedCredentials = new Credentials("idToken", "accessToken", "type", "refreshToken", expiresAt, "scope"); - String storedJson = gson.toJson(storedCredentials); - String encoded = new String(Base64.encode(storedJson.getBytes(), Base64.DEFAULT)); - when(crypto.decrypt(storedJson.getBytes())).thenReturn(storedJson.getBytes()); - when(storage.retrieveString("com.auth0.credentials")).thenReturn(encoded); - when(storage.retrieveLong("com.auth0.credentials_expires_at")).thenReturn(expiresAt.getTime()); - when(storage.retrieveBoolean("com.auth0.credentials_can_refresh")).thenReturn(false); + insertTestCredentials(true, true, false, expiresAt); //Require authentication Activity activity = spy(Robolectric.buildActivity(Activity.class).create().start().resume().get()); @@ -600,23 +616,21 @@ public void shouldGetCredentialsAfterAuthentication() throws Exception { assertThat(retrievedCredentials, is(notNullValue())); assertThat(retrievedCredentials.getAccessToken(), is("accessToken")); assertThat(retrievedCredentials.getIdToken(), is("idToken")); - assertThat(retrievedCredentials.getRefreshToken(), is("refreshToken")); + assertThat(retrievedCredentials.getRefreshToken(), is(nullValue())); assertThat(retrievedCredentials.getType(), is("type")); assertThat(retrievedCredentials.getExpiresAt(), is(notNullValue())); assertThat(retrievedCredentials.getExpiresAt().getTime(), is(expiresAt.getTime())); assertThat(retrievedCredentials.getScope(), is("scope")); + + //A second call to checkAuthenticationResult should fail as callback is set to null + final boolean retryCheck = manager.checkAuthenticationResult(123, Activity.RESULT_OK); + assertThat(retryCheck, is(false)); } @Test public void shouldNotGetCredentialsAfterCanceledAuthentication() throws Exception { Date expiresAt = new Date(CredentialsMock.CURRENT_TIME_MS + 123456L * 1000); - Credentials storedCredentials = new Credentials("idToken", "accessToken", "type", "refreshToken", expiresAt, "scope"); - String storedJson = gson.toJson(storedCredentials); - String encoded = new String(Base64.encode(storedJson.getBytes(), Base64.DEFAULT)); - when(crypto.decrypt(storedJson.getBytes())).thenReturn(storedJson.getBytes()); - when(storage.retrieveString("com.auth0.credentials")).thenReturn(encoded); - when(storage.retrieveLong("com.auth0.credentials_expires_at")).thenReturn(expiresAt.getTime()); - when(storage.retrieveBoolean("com.auth0.credentials_can_refresh")).thenReturn(false); + insertTestCredentials(true, true, false, expiresAt); //Require authentication Activity activity = spy(Robolectric.buildActivity(Activity.class).create().start().resume().get()); @@ -645,4 +659,49 @@ public void shouldNotGetCredentialsAfterCanceledAuthentication() throws Exceptio assertThat(exceptionCaptor.getValue(), is(notNullValue())); assertThat(exceptionCaptor.getValue().getMessage(), is("The user didn't pass the authentication challenge.")); } + + @Test + public void shouldNotGetCredentialsOnDifferentAuthenticationRequestCode() throws Exception { + Date expiresAt = new Date(CredentialsMock.CURRENT_TIME_MS + 123456L * 1000); + insertTestCredentials(true, true, false, expiresAt); + + //Require authentication + Activity activity = spy(Robolectric.buildActivity(Activity.class).create().start().resume().get()); + KeyguardManager kService = mock(KeyguardManager.class); + when(activity.getSystemService(Context.KEYGUARD_SERVICE)).thenReturn(kService); + when(kService.isKeyguardSecure()).thenReturn(true); + Intent confirmCredentialsIntent = mock(Intent.class); + when(kService.createConfirmDeviceCredentialIntent("theTitle", "theDescription")).thenReturn(confirmCredentialsIntent); + boolean willRequireAuthentication = manager.requireAuthentication(activity, 100, "theTitle", "theDescription"); + assertThat(willRequireAuthentication, is(true)); + + manager.getCredentials(callback); + + ArgumentCaptor intentCaptor = ArgumentCaptor.forClass(Intent.class); + verify(activity).startActivityForResult(intentCaptor.capture(), eq(100)); + assertThat(intentCaptor.getValue(), is(confirmCredentialsIntent)); + + + //Continue after successful authentication + verifyNoMoreInteractions(callback); + final boolean processed = manager.checkAuthenticationResult(123, Activity.RESULT_OK); + assertThat(processed, is(false)); + + } + + + /** + * Used to simplify the tests length + */ + private String insertTestCredentials(boolean hasIdToken, boolean hasAccessToken, boolean hasRefreshToken, Date willExpireAt) { + Credentials storedCredentials = new Credentials(hasIdToken ? "idToken" : null, hasAccessToken ? "accessToken" : null, "type", + hasRefreshToken ? "refreshToken" : null, willExpireAt, "scope"); + String storedJson = gson.toJson(storedCredentials); + String encoded = new String(Base64.encode(storedJson.getBytes(), Base64.DEFAULT)); + when(crypto.decrypt(storedJson.getBytes())).thenReturn(storedJson.getBytes()); + when(storage.retrieveString("com.auth0.credentials")).thenReturn(encoded); + when(storage.retrieveLong("com.auth0.credentials_expires_at")).thenReturn(willExpireAt != null ? willExpireAt.getTime() : null); + when(storage.retrieveBoolean("com.auth0.credentials_can_refresh")).thenReturn(hasRefreshToken); + return storedJson; + } } \ No newline at end of file From bb59b1399902a726d31a6b2a78068ce120661c21 Mon Sep 17 00:00:00 2001 From: Luciano Balmaceda Date: Mon, 21 Jan 2019 17:54:04 -0300 Subject: [PATCH 3/6] refactor CryptoUtil with better exception handling --- .../authentication/storage/CryptoUtil.java | 258 +++++++++++++++--- .../storage/IncompatibleDeviceException.java | 11 + .../storage/SecureCredentialsManager.java | 33 ++- .../UnrecoverableContentException.java | 11 - 4 files changed, 244 insertions(+), 69 deletions(-) create mode 100644 auth0/src/main/java/com/auth0/android/authentication/storage/IncompatibleDeviceException.java delete mode 100644 auth0/src/main/java/com/auth0/android/authentication/storage/UnrecoverableContentException.java 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 5d262f7fa..c32c99007 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 @@ -17,7 +17,7 @@ import java.io.IOException; import java.math.BigInteger; import java.security.InvalidAlgorithmParameterException; -import java.security.KeyException; +import java.security.InvalidKeyException; import java.security.KeyPairGenerator; import java.security.KeyStore; import java.security.KeyStoreException; @@ -42,6 +42,7 @@ /** * 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. */ @SuppressWarnings("WeakerAccess") @RequiresApi(api = Build.VERSION_CODES.KITKAT) @@ -78,8 +79,16 @@ public CryptoUtil(@NonNull Context context, @NonNull Storage storage, @NonNull S this.storage = storage; } + /** + * 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. + * + * @return a valid RSA Private Key entry + * @throws CryptoException if the stored keys can't be recovered and should be deemed invalid + * @throws IncompatibleDeviceException in the event the device can't understand the cryptographic settings required by this method + */ @VisibleForTesting - KeyStore.PrivateKeyEntry getRSAKeyEntry() throws KeyException, UnrecoverableEntryException { + KeyStore.PrivateKeyEntry getRSAKeyEntry() throws CryptoException, IncompatibleDeviceException { try { KeyStore keyStore = KeyStore.getInstance(ANDROID_KEY_STORE); keyStore.load(null); @@ -95,7 +104,6 @@ KeyStore.PrivateKeyEntry getRSAKeyEntry() throws KeyException, UnrecoverableEntr X500Principal principal = new X500Principal("CN=Auth0.Android,O=Auth0"); if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - //Following code is for API 23+ spec = new KeyGenParameterSpec.Builder(KEY_ALIAS, KeyProperties.PURPOSE_DECRYPT | KeyProperties.PURPOSE_ENCRYPT) .setCertificateSubject(principal) .setCertificateSerialNumber(BigInteger.ONE) @@ -117,15 +125,14 @@ KeyStore.PrivateKeyEntry getRSAKeyEntry() throws KeyException, UnrecoverableEntr .setEndDate(end.getTime()); KeyguardManager kManager = (KeyguardManager) context.getSystemService(Context.KEYGUARD_SERVICE); - Intent authIntent = null; if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.LOLLIPOP) { //The next call can return null when the LockScreen is not configured - authIntent = kManager.createConfirmDeviceCredentialIntent(null, null); - } - boolean keyguardEnabled = Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP && kManager.isKeyguardSecure() && authIntent != null; - if (keyguardEnabled) { - //If a ScreenLock is setup, protect this key pair. - specBuilder.setEncryptionRequired(); + Intent authIntent = kManager.createConfirmDeviceCredentialIntent(null, null); + boolean keyguardEnabled = kManager.isKeyguardSecure() && authIntent != null; + if (keyguardEnabled) { + //If a ScreenLock is setup, protect this key pair. + specBuilder.setEncryptionRequired(); + } } spec = specBuilder.build(); } @@ -135,12 +142,55 @@ KeyStore.PrivateKeyEntry getRSAKeyEntry() throws KeyException, UnrecoverableEntr generator.generateKeyPair(); return getKeyEntryCompat(keyStore); - } catch (KeyStoreException | IOException | NoSuchProviderException | InvalidAlgorithmParameterException | NoSuchAlgorithmException | CertificateException e) { - Log.e(TAG, "An error occurred while trying to obtain the RSA KeyPair Entry from the Android KeyStore.", e); - throw new KeyException("An error occurred while trying to obtain the RSA KeyPair Entry from the Android KeyStore.", e); + } catch (CertificateException | InvalidAlgorithmParameterException | NoSuchProviderException | NoSuchAlgorithmException | KeyStoreException e) { + /* + * This exceptions are safe to be ignored: + * + * - CertificateException: + * Thrown when certificate has expired (25 years..) or couldn't be loaded + * - KeyStoreException: + * - NoSuchProviderException: + * Thrown when "AndroidKeyStore" is not available. Was introduced on API 18. + * - NoSuchAlgorithmException: + * Thrown when "RSA" algorithm is not available. Was introduced on API 18. + * - InvalidAlgorithmParameterException: + * Thrown if Key Size is other than 512, 768, 1024, 2048, 3072, 4096 + * or if Padding is other than RSA/ECB/PKCS1Padding, introduced on API 18 + * or if Block Mode is other than ECB + * + * However if any of this exceptions happens to be thrown (OEMs often change their Android distribution source code), + * all the checks performed in this class wouldn't matter and the device would not be compatible at all with it. + * + * Read more in https://developer.android.com/training/articles/keystore#SupportedAlgorithms + */ + Log.e(TAG, "The device can't generate a new RSA Key pair.", e); + throw new IncompatibleDeviceException(e); + } catch (IOException | UnrecoverableEntryException e) { + /* + * Any of this exceptions mean the old key pair is somehow corrupted. + * We can delete it and let the user retry the operation. + * + * - IOException: + * Thrown when there is an I/O or format problem with the keystore data. + * - UnrecoverableEntryException: + * Thrown when the key cannot be recovered. Probably because it was invalidated by a Lock Screen change. + */ + deleteKeys(); + throw new CryptoException("The existing RSA key pair could not be recovered and has been deleted. " + + "This occasionally happens when the Lock Screen settings are changed. You can safely retry this operation.", e); } } + /** + * Helper method compatible with older Android versions to load the Private Key Entry from + * the KeyStore using the {@link #KEY_ALIAS}. + * + * @param keyStore the KeyStore instance. Must be initialized (loaded). + * @return the key entry stored in the KeyStore. + * @throws KeyStoreException if the keystore was not initialized. + * @throws NoSuchAlgorithmException if device is not compatible with RSA algorithm. RSA is available since API 18. + * @throws UnrecoverableEntryException if key cannot be recovered. Probably because it was invalidated by a Lock Screen change. + */ private KeyStore.PrivateKeyEntry getKeyEntryCompat(KeyStore keyStore) throws KeyStoreException, NoSuchAlgorithmException, UnrecoverableEntryException { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) { return (KeyStore.PrivateKeyEntry) keyStore.getEntry(KEY_ALIAS, null); @@ -157,57 +207,108 @@ private KeyStore.PrivateKeyEntry getKeyEntryCompat(KeyStore keyStore) throws Key return new KeyStore.PrivateKeyEntry(privateKey, new Certificate[]{certificate}); } - //Used to delete recreate the key pair in case of error + /** + * Removes the AES and RSA keys generated in a previous execution. + * Used when we want the next call to {@link #encrypt(byte[])} or {@link #decrypt(byte[])} + * to recreate the keys. + */ private void deleteKeys() { + storage.remove(KEY_ALIAS); + storage.remove(KEY_IV_ALIAS); try { KeyStore keyStore = KeyStore.getInstance(ANDROID_KEY_STORE); keyStore.load(null); keyStore.deleteEntry(KEY_ALIAS); - storage.remove(KEY_ALIAS); - storage.remove(KEY_IV_ALIAS); + Log.d(TAG, "Deleting the existing RSA key pair from the KeyStore."); } catch (KeyStoreException | CertificateException | IOException | NoSuchAlgorithmException e) { Log.e(TAG, "Failed to remove the RSA KeyEntry from the Android KeyStore.", e); } } - //Only used to decrypt AES key + /** + * Decrypts the given input using a generated RSA Private Key. + * Used to decrypt the AES key for later usage. + * + * @param encryptedInput the input bytes to decrypt + * @return the decrypted bytes output + * @throws IncompatibleDeviceException in the event the device can't understand the cryptographic settings required + */ @VisibleForTesting - byte[] RSADecrypt(byte[] encryptedInput) throws CryptoException { + byte[] RSADecrypt(byte[] encryptedInput) throws IncompatibleDeviceException { try { PrivateKey privateKey = getRSAKeyEntry().getPrivateKey(); Cipher cipher = Cipher.getInstance(RSA_TRANSFORMATION); cipher.init(Cipher.DECRYPT_MODE, privateKey); return cipher.doFinal(encryptedInput); - } catch (NoSuchAlgorithmException | NoSuchPaddingException | KeyException e) { - throw new CryptoException("Couldn't decrypt the input using the RSA Key.", e); - } catch (BadPaddingException | IllegalBlockSizeException | UnrecoverableEntryException e) { - deleteKeys(); - Log.e(TAG, "The input contained unexpected content, probably because it was encrypted using a different key. " + - "The existing keys have been deleted and a new pair will be created next time. Please try to encrypt the content again.", e); - throw new CryptoException("Couldn't decrypt the input using the RSA Key.", new UnrecoverableContentException(e)); + } catch (NoSuchAlgorithmException | IllegalBlockSizeException | BadPaddingException | NoSuchPaddingException | InvalidKeyException e) { + /* + * This exceptions are safe to be ignored: + * + * - 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. + * - IllegalBlockSizeException: + * Thrown only on encrypt mode. + * - BadPaddingException: + * Thrown if the input doesn't contain the proper padding bytes. + * + * Read more in https://developer.android.com/reference/javax/crypto/Cipher + */ + Log.e(TAG, "The device can't decrypt input using a RSA Key.", e); + throw new IncompatibleDeviceException(e); } } - //Only used to encrypt AES key + /** + * Encrypts the given input using a generated RSA Public Key. + * Used to encrypt the AES key for later storage. + * + * @param decryptedInput the input bytes to encrypt + * @return the encrypted bytes output + * @throws IncompatibleDeviceException in the event the device can't understand the cryptographic settings required + */ @VisibleForTesting - byte[] RSAEncrypt(byte[] decryptedInput) throws CryptoException { + byte[] RSAEncrypt(byte[] decryptedInput) throws IncompatibleDeviceException { try { Certificate certificate = getRSAKeyEntry().getCertificate(); Cipher cipher = Cipher.getInstance(RSA_TRANSFORMATION); cipher.init(Cipher.ENCRYPT_MODE, certificate); return cipher.doFinal(decryptedInput); - } catch (NoSuchAlgorithmException | NoSuchPaddingException | KeyException e) { - throw new CryptoException("Couldn't encrypt the input using the RSA Key.", e); - } catch (BadPaddingException | IllegalBlockSizeException | UnrecoverableEntryException e) { - deleteKeys(); - Log.e(TAG, "The input contained unexpected content and it was deemed unrecoverable." + - " The existing keys have been deleted and a new pair will be created next time.", e); - throw new CryptoException("Couldn't encrypt the input using the RSA Key.", new UnrecoverableContentException(e)); + } catch (NoSuchAlgorithmException | IllegalBlockSizeException | BadPaddingException | NoSuchPaddingException | InvalidKeyException e) { + /* + * This exceptions are safe to be ignored: + * + * - 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. + * - IllegalBlockSizeException: + * Thrown if no padding has been requested and the length is not multiple of block size. + * - BadPaddingException: + * Thrown only on decrypt mode. + * + * Read more in https://developer.android.com/reference/javax/crypto/Cipher + */ + Log.e(TAG, "The device can't encrypt input using a RSA Key.", e); + throw new IncompatibleDeviceException(e); } } + /** + * Attempts to recover the existing AES Key or generates a new one if none is found. + * + * @return a valid AES Key bytes + * @throws IncompatibleDeviceException in the event the device can't understand the cryptographic settings required + */ @VisibleForTesting - byte[] getAESKey() throws KeyException { + byte[] getAESKey() throws IncompatibleDeviceException { final String encodedEncryptedAES = storage.retrieveString(KEY_ALIAS); if (encodedEncryptedAES != null) { //Return existing key @@ -225,31 +326,79 @@ byte[] getAESKey() throws KeyException { storage.store(KEY_ALIAS, encodedEncryptedAESText); return aes; } 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 + * + * However if any of this exceptions happens to be thrown (OEMs often change their Android distribution source code), + * all the checks performed in this class wouldn't matter and the device would not be compatible at all with it. + * + * Read more in https://developer.android.com/reference/javax/crypto/KeyGenerator + */ Log.e(TAG, "Error while creating the AES key.", e); - throw new KeyException("Error while creating the AES key.", e); + throw new IncompatibleDeviceException(e); } } - //Only used to decrypt final DATA - public byte[] decrypt(byte[] encryptedInput) throws CryptoException { + + /** + * Encrypts 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. + * @return the decrypted output bytes + * @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 + */ + public byte[] decrypt(byte[] encryptedInput) throws CryptoException, IncompatibleDeviceException { 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)) { - throw new InvalidAlgorithmParameterException("The AES Key exists but an IV was never stored. Try to encrypt something first."); + //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); } byte[] iv = Base64.decode(encodedIV, Base64.DEFAULT); cipher.init(Cipher.DECRYPT_MODE, key, new IvParameterSpec(iv)); return cipher.doFinal(encryptedInput); - } catch (KeyException | NoSuchAlgorithmException | NoSuchPaddingException | InvalidAlgorithmParameterException | BadPaddingException | IllegalBlockSizeException e) { + } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | InvalidAlgorithmParameterException | BadPaddingException | IllegalBlockSizeException e) { + /* + * This exceptions are safe to be ignored: + * + * - NoSuchPaddingException: + * Thrown if NOPADDING 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: + * If the IV parameter is null. + * - BadPaddingException: + * Thrown if the input doesn't contain the proper padding bytes. In this case, if the input contains padding. + * - IllegalBlockSizeException: + * Thrown only on encrypt mode. + * + * Read more in https://developer.android.com/reference/javax/crypto/Cipher + */ Log.e(TAG, "Error while decrypting the input.", e); - throw new CryptoException("Error while decrypting the input.", e); + throw new IncompatibleDeviceException(e); } } - //Only used to encrypt final DATA - public byte[] encrypt(byte[] decryptedInput) throws CryptoException { + /** + * Encrypts the given input bytes using a symmetric key (AES). + * The AES key is stored protected by an asymmetric key pair (RSA). + * + * @param decryptedInput the input bytes to encrypt. There's no limit in size. + * @return the encrypted output bytes + * @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 + */ + public byte[] encrypt(byte[] decryptedInput) throws CryptoException, IncompatibleDeviceException { try { SecretKey key = new SecretKeySpec(getAESKey(), ALGORITHM_AES); Cipher cipher = Cipher.getInstance(AES_TRANSFORMATION); @@ -259,9 +408,28 @@ public byte[] encrypt(byte[] decryptedInput) throws CryptoException { //Save IV for Decrypt stage storage.store(KEY_IV_ALIAS, new String(encodedIV)); return encrypted; - } catch (KeyException | NoSuchAlgorithmException | NoSuchPaddingException | BadPaddingException | IllegalBlockSizeException e) { + } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | BadPaddingException | IllegalBlockSizeException e) { + /* + * This exceptions are safe to be ignored: + * + * - NoSuchPaddingException: + * Thrown if NOPADDING 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: + * If the IV parameter is null. + * - BadPaddingException: + * Thrown only on decrypt mode. + * - IllegalBlockSizeException: + * Thrown if no padding has been requested and the length is not multiple of block size. + * + * Read more in https://developer.android.com/reference/javax/crypto/Cipher + */ Log.e(TAG, "Error while encrypting the input.", e); - throw new CryptoException("Error while encrypting the input.", e); + throw new IncompatibleDeviceException(e); } } diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/IncompatibleDeviceException.java b/auth0/src/main/java/com/auth0/android/authentication/storage/IncompatibleDeviceException.java new file mode 100644 index 000000000..66199659b --- /dev/null +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/IncompatibleDeviceException.java @@ -0,0 +1,11 @@ +package com.auth0.android.authentication.storage; + +/** + * Exception thrown by the {@link CryptoUtil} class whenever the Keys are deemed invalid + * and so the content encrypted with them unrecoverable. + */ +public class IncompatibleDeviceException extends RuntimeException { + IncompatibleDeviceException(Throwable cause) { + super(String.format("The device is not compatible with the %s class.", CryptoUtil.class.getSimpleName()), cause); + } +} diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java index b4d2aaf6b..2ae038788 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java @@ -90,7 +90,7 @@ public SecureCredentialsManager(@NonNull Context context, @NonNull Authenticatio */ public boolean requireAuthentication(@NonNull Activity activity, @IntRange(from = 1, to = 255) int requestCode, @Nullable String title, @Nullable String description) { if (requestCode < 1 || requestCode > 255) { - throw new IllegalArgumentException("Request code must a value between 1 and 255."); + throw new IllegalArgumentException("Request code must be a value between 1 and 255."); } KeyguardManager kManager = (KeyguardManager) activity.getSystemService(Context.KEYGUARD_SERVICE); this.authIntent = Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP ? kManager.createConfirmDeviceCredentialIntent(title, description) : null; @@ -130,7 +130,7 @@ public boolean checkAuthenticationResult(int requestCode, int resultCode) { * Saves the given credentials in the Storage. * * @param credentials the credentials to save. - * @throws CredentialsManagerException if the credentials couldn't be encrypted. + * @throws CredentialsManagerException if the credentials couldn't be encrypted. If the class is not supported by this device the cause will be a {@link IncompatibleDeviceException}. */ public void saveCredentials(@NonNull Credentials credentials) throws CredentialsManagerException { if ((isEmpty(credentials.getAccessToken()) && isEmpty(credentials.getIdToken())) || credentials.getExpiresAt() == null) { @@ -148,18 +148,22 @@ public void saveCredentials(@NonNull Credentials credentials) throws Credentials storage.store(KEY_CREDENTIALS, encryptedEncoded); storage.store(KEY_EXPIRES_AT, expiresAt); storage.store(KEY_CAN_REFRESH, canRefresh); - } catch (UnrecoverableContentException e) { - //If keys were invalidated, a retry will work fine for the "save credentials" use case. - saveCredentials(credentials); } catch (CryptoException e) { - throw new CredentialsManagerException("An error occurred while encrypting the credentials.", e); + /* + * If the keys were invalidated in the call above a good new pair is going to be available + * to use on the next call. Retrying this operation will succeed. + */ + throw new CredentialsManagerException("A change on the Lock Screen security settings have deemed the encryption keys invalid and have been recreated. Please, try saving the credentials again.", e); + } catch (IncompatibleDeviceException e) { + throw new CredentialsManagerException(String.format("This device is not compatible with the %s class.", SecureCredentialsManager.class.getSimpleName()), e); } } /** * Tries to obtain the credentials from the Storage. - * If a LockScreen is setup the user will be asked to authenticate before accessing the credentials. Your activity must override the - * {@link Activity#onActivityResult(int, int, Intent)} method and call {@link #checkAuthenticationResult(int, int)} with the received values. + * If a LockScreen is setup and {@link #requireAuthentication(Activity, int, String, String)} was called, the user will be asked to authenticate before accessing + * the credentials. Your activity must override the {@link Activity#onActivityResult(int, int, Intent)} method and call + * {@link #checkAuthenticationResult(int, int)} with the received values. * * @param callback the callback to receive the result in. */ @@ -210,11 +214,14 @@ private void continueGetCredentials(final BaseCallback Date: Tue, 22 Jan 2019 17:17:52 -0300 Subject: [PATCH 4/6] refactor tests --- .../storage/CredentialsManagerException.java | 10 + .../authentication/storage/CryptoUtil.java | 9 +- .../storage/IncompatibleDeviceException.java | 2 +- .../storage/SecureCredentialsManager.java | 16 +- .../storage/CryptoUtilTest.java | 520 +++++++++++------- .../storage/SecureCredentialsManagerTest.java | 111 ++-- 6 files changed, 427 insertions(+), 241 deletions(-) diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/CredentialsManagerException.java b/auth0/src/main/java/com/auth0/android/authentication/storage/CredentialsManagerException.java index 0f157e538..c4540c30e 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/CredentialsManagerException.java +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/CredentialsManagerException.java @@ -15,4 +15,14 @@ public class CredentialsManagerException extends Auth0Exception { CredentialsManagerException(String message) { super(message); } + + /** + * Returns true when this Android device doesn't support the cryptographic algorithms used + * to handle encryption and decryption, false otherwise. + * + * @return whether this device is compatible with {@link SecureCredentialsManager} or not. + */ + public boolean isDeviceIncompatible() { + return (getCause() instanceof IncompatibleDeviceException); + } } 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 c32c99007..af9226e8b 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 @@ -232,9 +232,10 @@ private void deleteKeys() { * @param encryptedInput the input bytes to decrypt * @return the decrypted bytes output * @throws IncompatibleDeviceException in the event the device can't understand the cryptographic settings required + * @throws CryptoException if the stored RSA keys can't be recovered and should be deemed invalid */ @VisibleForTesting - byte[] RSADecrypt(byte[] encryptedInput) throws IncompatibleDeviceException { + byte[] RSADecrypt(byte[] encryptedInput) throws IncompatibleDeviceException, CryptoException { try { PrivateKey privateKey = getRSAKeyEntry().getPrivateKey(); Cipher cipher = Cipher.getInstance(RSA_TRANSFORMATION); @@ -270,9 +271,10 @@ byte[] RSADecrypt(byte[] encryptedInput) throws IncompatibleDeviceException { * @param decryptedInput the input bytes to encrypt * @return the encrypted bytes output * @throws IncompatibleDeviceException in the event the device can't understand the cryptographic settings required + * @throws CryptoException if the stored RSA keys can't be recovered and should be deemed invalid */ @VisibleForTesting - byte[] RSAEncrypt(byte[] decryptedInput) throws IncompatibleDeviceException { + byte[] RSAEncrypt(byte[] decryptedInput) throws IncompatibleDeviceException, CryptoException { try { Certificate certificate = getRSAKeyEntry().getCertificate(); Cipher cipher = Cipher.getInstance(RSA_TRANSFORMATION); @@ -306,9 +308,10 @@ byte[] RSAEncrypt(byte[] decryptedInput) throws IncompatibleDeviceException { * * @return a valid AES Key bytes * @throws IncompatibleDeviceException in the event the device can't understand the cryptographic settings required + * @throws CryptoException if the stored RSA keys can't be recovered and should be deemed invalid */ @VisibleForTesting - byte[] getAESKey() throws IncompatibleDeviceException { + byte[] getAESKey() throws IncompatibleDeviceException, CryptoException { final String encodedEncryptedAES = storage.retrieveString(KEY_ALIAS); if (encodedEncryptedAES != null) { //Return existing key diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/IncompatibleDeviceException.java b/auth0/src/main/java/com/auth0/android/authentication/storage/IncompatibleDeviceException.java index 66199659b..83375f2f8 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/IncompatibleDeviceException.java +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/IncompatibleDeviceException.java @@ -4,7 +4,7 @@ * Exception thrown by the {@link CryptoUtil} class whenever the Keys are deemed invalid * and so the content encrypted with them unrecoverable. */ -public class IncompatibleDeviceException extends RuntimeException { +class IncompatibleDeviceException extends CryptoException { IncompatibleDeviceException(Throwable cause) { super(String.format("The device is not compatible with the %s class.", CryptoUtil.class.getSimpleName()), cause); } diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java index 2ae038788..e2a007942 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java @@ -148,14 +148,16 @@ public void saveCredentials(@NonNull Credentials credentials) throws Credentials storage.store(KEY_CREDENTIALS, encryptedEncoded); storage.store(KEY_EXPIRES_AT, expiresAt); storage.store(KEY_CAN_REFRESH, canRefresh); + } catch (IncompatibleDeviceException e) { + throw new CredentialsManagerException(String.format("This device is not compatible with the %s class.", SecureCredentialsManager.class.getSimpleName()), e); } catch (CryptoException e) { /* * If the keys were invalidated in the call above a good new pair is going to be available - * to use on the next call. Retrying this operation will succeed. + * to use on the next call. We clear any existing credentials so #hasValidCredentials returns + * a true value. Retrying this operation will succeed. */ + clearCredentials(); throw new CredentialsManagerException("A change on the Lock Screen security settings have deemed the encryption keys invalid and have been recreated. Please, try saving the credentials again.", e); - } catch (IncompatibleDeviceException e) { - throw new CredentialsManagerException(String.format("This device is not compatible with the %s class.", SecureCredentialsManager.class.getSimpleName()), e); } } @@ -213,6 +215,10 @@ private void continueGetCredentials(final BaseCallbackinstanceOf(UnrecoverableContentException.class)); + public void shouldThrowOnNoSuchPaddingExceptionWhenTryingToRSADecrypt() throws Exception { + exception.expect(IncompatibleDeviceException.class); + exception.expectMessage("The device is not compatible with the CryptoUtil class"); PrivateKey privateKey = PowerMockito.mock(PrivateKey.class); KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); doReturn(privateKey).when(privateKeyEntry).getPrivateKey(); doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry(); + PowerMockito.mockStatic(Cipher.class); + PowerMockito.when(Cipher.getInstance(RSA_TRANSFORMATION)).thenThrow(new NoSuchPaddingException()); - doThrow(new BadPaddingException()).when(rsaCipher).doFinal(any(byte[].class)); cryptoUtil.RSADecrypt(new byte[0]); - - Mockito.verify(keyStore).deleteEntry(KEY_ALIAS); - Mockito.verify(storage).remove(KEY_ALIAS); - Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); } @Test - public void shouldDeleteKeysOnIllegalBlockSizeExceptionWhenTryingToRSADecrypt() throws Exception { - exception.expect(CryptoException.class); - exception.expectMessage("Couldn't decrypt the input using the RSA Key."); - exception.expectCause(CoreMatchers.instanceOf(UnrecoverableContentException.class)); + public void shouldThrowOnBadPaddingExceptionWhenTryingToRSADecrypt() throws Exception { + exception.expect(IncompatibleDeviceException.class); + exception.expectMessage("The device is not compatible with the CryptoUtil class"); PrivateKey privateKey = PowerMockito.mock(PrivateKey.class); KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); doReturn(privateKey).when(privateKeyEntry).getPrivateKey(); doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry(); - doThrow(new IllegalBlockSizeException()).when(rsaCipher).doFinal(any(byte[].class)); + doThrow(new BadPaddingException()).when(rsaCipher).doFinal(any(byte[].class)); cryptoUtil.RSADecrypt(new byte[0]); - - Mockito.verify(keyStore).deleteEntry(KEY_ALIAS); - Mockito.verify(storage).remove(KEY_ALIAS); - Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); } @Test - public void shouldDeleteKeysOnUnrecoverableEntryExceptionWhenTryingToRSADecrypt() throws Exception { - exception.expect(CryptoException.class); - exception.expectMessage("Couldn't decrypt the input using the RSA Key."); - exception.expectCause(CoreMatchers.instanceOf(UnrecoverableContentException.class)); + public void shouldThrowOnIllegalBlockSizeExceptionWhenTryingToRSADecrypt() throws Exception { + exception.expect(IncompatibleDeviceException.class); + exception.expectMessage("The device is not compatible with the CryptoUtil class"); PrivateKey privateKey = PowerMockito.mock(PrivateKey.class); KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); doReturn(privateKey).when(privateKeyEntry).getPrivateKey(); - doThrow(new UnrecoverableEntryException()).when(cryptoUtil).getRSAKeyEntry(); - cryptoUtil.RSADecrypt(new byte[0]); - - Mockito.verify(keyStore).deleteEntry(KEY_ALIAS); - Mockito.verify(storage).remove(KEY_ALIAS); - Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); - } - - @Test - public void shouldDeleteKeysOnBadPaddingExceptionWhenTryingToRSAEncrypt() throws Exception { - exception.expect(CryptoException.class); - exception.expectMessage("Couldn't encrypt the input using the RSA Key."); - exception.expectCause(CoreMatchers.instanceOf(UnrecoverableContentException.class)); - - Certificate certificate = PowerMockito.mock(Certificate.class); - KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); - doReturn(certificate).when(privateKeyEntry).getCertificate(); - doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry(); - - doThrow(new BadPaddingException()).when(rsaCipher).doFinal(any(byte[].class)); - cryptoUtil.RSAEncrypt(new byte[0]); - - Mockito.verify(keyStore).deleteEntry(KEY_ALIAS); - Mockito.verify(storage).remove(KEY_ALIAS); - Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); - } - - @Test - public void shouldDeleteKeysOnIllegalBlockSizeExceptionWhenTryingToRSAEncrypt() throws Exception { - exception.expect(CryptoException.class); - exception.expectMessage("Couldn't encrypt the input using the RSA Key."); - exception.expectCause(CoreMatchers.instanceOf(UnrecoverableContentException.class)); - - Certificate certificate = PowerMockito.mock(Certificate.class); - KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); - doReturn(certificate).when(privateKeyEntry).getCertificate(); doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry(); doThrow(new IllegalBlockSizeException()).when(rsaCipher).doFinal(any(byte[].class)); - cryptoUtil.RSAEncrypt(new byte[0]); - - Mockito.verify(keyStore).deleteEntry(KEY_ALIAS); - Mockito.verify(storage).remove(KEY_ALIAS); - Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); + cryptoUtil.RSADecrypt(new byte[0]); } - @Test - public void shouldDeleteKeysOnUnrecoverableEntryExceptionWhenTryingToRSAEncrypt() throws Exception { - exception.expect(CryptoException.class); - exception.expectMessage("Couldn't encrypt the input using the RSA Key."); - exception.expectCause(CoreMatchers.instanceOf(UnrecoverableContentException.class)); - - Certificate certificate = PowerMockito.mock(Certificate.class); - KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); - doReturn(certificate).when(privateKeyEntry).getCertificate(); - doReturn(privateKeyEntry).when(cryptoUtil).getRSAKeyEntry(); - - doThrow(new UnrecoverableEntryException()).when(cryptoUtil).getRSAKeyEntry(); - cryptoUtil.RSAEncrypt(new byte[0]); - - Mockito.verify(keyStore).deleteEntry(KEY_ALIAS); - Mockito.verify(storage).remove(KEY_ALIAS); - Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); - } + /* + * MAIN ENCRYPT (AES) tests + */ @Test public void shouldAESEncryptData() throws Exception { @@ -836,46 +824,95 @@ public void shouldAESEncryptData() throws Exception { } @Test - public void shouldThrowOnKeyErrorWhenTryingToAESEncrypt() throws Exception { + public void shouldThrowOnCryptoExceptionOnRSAKeyReadingWhenTryingToAESEncrypt() throws Exception { exception.expect(CryptoException.class); - exception.expectMessage("Error while encrypting the input."); - doThrow(new KeyException()).when(cryptoUtil).getAESKey(); + PowerMockito.mockStatic(Base64.class); + PowerMockito.when(Base64.decode("encoded-key", Base64.DEFAULT)).thenReturn(new byte[0]); + PowerMockito.when(storage.retrieveString(KEY_ALIAS)).thenReturn("encoded-key"); + doThrow(new CryptoException(null, null)).when(cryptoUtil).getRSAKeyEntry(); cryptoUtil.encrypt(new byte[0]); } @Test - public void shouldThrowOnNoSuchPaddingErrorWhenTryingToAESEncrypt() throws Exception { + public void shouldThrowOnCryptoExceptionOnAESKeyReadingWhenTryingToAESEncrypt() throws Exception { exception.expect(CryptoException.class); - exception.expectMessage("Error while encrypting the input."); + doThrow(new CryptoException(null, null)).when(cryptoUtil).getAESKey(); + cryptoUtil.encrypt(new byte[0]); + } + + @Test + public void shouldThrowOnIncompatibleDeviceExceptionOnRSAKeyReadingWhenTryingToAESEncrypt() throws Exception { + exception.expect(IncompatibleDeviceException.class); + exception.expectMessage("The device is not compatible with the CryptoUtil class"); + + PowerMockito.mockStatic(Base64.class); + PowerMockito.when(Base64.decode("encoded-key", Base64.DEFAULT)).thenReturn(new byte[0]); + PowerMockito.when(storage.retrieveString(KEY_ALIAS)).thenReturn("encoded-key"); + + doThrow(new IncompatibleDeviceException(null)).when(cryptoUtil).getRSAKeyEntry(); + cryptoUtil.encrypt(new byte[0]); + } + + @Test + public void shouldThrowOnIncompatibleDeviceExceptionOnAESKeyReadingWhenTryingToAESEncrypt() throws Exception { + exception.expect(IncompatibleDeviceException.class); + exception.expectMessage("The device is not compatible with the CryptoUtil class"); + + doThrow(new IncompatibleDeviceException(null)).when(cryptoUtil).getAESKey(); + cryptoUtil.encrypt(new byte[0]); + } + + + @Test + public void shouldThrowOnNoSuchPaddingExceptionWhenTryingToAESEncrypt() throws Exception { + exception.expect(IncompatibleDeviceException.class); doReturn(new byte[]{11, 22, 33}).when(cryptoUtil).getAESKey(); PowerMockito.mockStatic(Cipher.class); - PowerMockito.when(Cipher.getInstance(anyString())).thenThrow(new NoSuchPaddingException()); + PowerMockito.when(Cipher.getInstance(AES_TRANSFORMATION)).thenThrow(new NoSuchPaddingException()); cryptoUtil.encrypt(new byte[0]); } @Test - public void shouldThrowOnNoSuchAlgorithmErrorWhenTryingToAESEncrypt() throws Exception { - exception.expect(CryptoException.class); - exception.expectMessage("Error while encrypting the input."); - + public void shouldThrowOnNoSuchAlgorithmExceptionWhenTryingToAESEncrypt() throws Exception { + exception.expect(IncompatibleDeviceException.class); doReturn(new byte[]{11, 22, 33}).when(cryptoUtil).getAESKey(); PowerMockito.mockStatic(Cipher.class); - PowerMockito.when(Cipher.getInstance(anyString())).thenThrow(new NoSuchAlgorithmException()); + PowerMockito.when(Cipher.getInstance(AES_TRANSFORMATION)).thenThrow(new NoSuchAlgorithmException()); cryptoUtil.encrypt(new byte[0]); } @Test - public void shouldThrowOnBadPaddingErrorWhenTryingToAESEncrypt() throws Exception { - exception.expect(CryptoException.class); - exception.expectMessage("Error while encrypting the input."); + public void shouldThrowOnInvalidKeyExceptionWhenTryingToAESEncrypt() throws Exception { + Exception exception = null; + ArgumentCaptor secretKeyArgumentCaptor = ArgumentCaptor.forClass(SecretKey.class); + byte[] aesKeyBytes = new byte[]{11, 22, 33}; + try { + doReturn(aesKeyBytes).when(cryptoUtil).getAESKey(); + + PowerMockito.mockStatic(Cipher.class); + PowerMockito.when(Cipher.getInstance(AES_TRANSFORMATION)).thenReturn(aesCipher); + doThrow(new InvalidKeyException()).when(aesCipher).init(eq(Cipher.ENCRYPT_MODE), secretKeyArgumentCaptor.capture()); + + cryptoUtil.encrypt(new byte[0]); + } catch (IncompatibleDeviceException e) { + exception = e; + } + assertThat(exception, is(notNullValue())); + assertThat(secretKeyArgumentCaptor.getValue().getAlgorithm(), is("AES")); + assertThat(secretKeyArgumentCaptor.getValue().getEncoded(), is(aesKeyBytes)); + } + + @Test + public void shouldThrowOnBadPaddingExceptionWhenTryingToAESEncrypt() throws Exception { + exception.expect(IncompatibleDeviceException.class); doReturn(new byte[]{11, 22, 33}).when(cryptoUtil).getAESKey(); doThrow(new BadPaddingException()).when(aesCipher) .doFinal(any(byte[].class)); @@ -884,10 +921,8 @@ public void shouldThrowOnBadPaddingErrorWhenTryingToAESEncrypt() throws Exceptio } @Test - public void shouldThrowOnIllegalBlockSizeErrorWhenTryingToAESEncrypt() throws Exception { - exception.expect(CryptoException.class); - exception.expectMessage("Error while encrypting the input."); - + public void shouldThrowOnIllegalBlockSizeExceptionWhenTryingToAESEncrypt() throws Exception { + exception.expect(IncompatibleDeviceException.class); doReturn(new byte[]{11, 22, 33}).when(cryptoUtil).getAESKey(); doThrow(new IllegalBlockSizeException()).when(aesCipher) .doFinal(any(byte[].class)); @@ -895,6 +930,11 @@ public void shouldThrowOnIllegalBlockSizeErrorWhenTryingToAESEncrypt() throws Ex cryptoUtil.encrypt(new byte[0]); } + + /* + * MAIN DECRYPT (AES) tests + */ + @Test public void shouldAESDecryptData() throws Exception { ArgumentCaptor secretKeyCaptor = ArgumentCaptor.forClass(SecretKey.class); @@ -925,79 +965,185 @@ public void shouldAESDecryptData() throws Exception { } @Test - public void shouldThrowOnKeyErrorWhenTryingToAESDecrypt() throws Exception { + public void shouldThrowOnCryptoExceptionOnRSAKeyReadingWhenTryingToAESDecrypt() throws Exception { exception.expect(CryptoException.class); - exception.expectMessage("Error while decrypting the input."); - doThrow(new KeyException()).when(cryptoUtil).getAESKey(); + PowerMockito.mockStatic(Base64.class); + PowerMockito.when(Base64.decode("encoded-key", Base64.DEFAULT)).thenReturn(new byte[0]); + PowerMockito.when(storage.retrieveString(KEY_ALIAS)).thenReturn("encoded-key"); + doThrow(new CryptoException(null, null)).when(cryptoUtil).getRSAKeyEntry(); cryptoUtil.decrypt(new byte[0]); } @Test - public void shouldThrowOnNoSuchPaddingErrorWhenTryingToAESDecrypt() throws Exception { + public void shouldThrowOnCryptoExceptionOnAESKeyReadingWhenTryingToAESDecrypt() throws Exception { exception.expect(CryptoException.class); - exception.expectMessage("Error while decrypting the input."); + doThrow(new CryptoException(null, null)).when(cryptoUtil).getAESKey(); + cryptoUtil.decrypt(new byte[0]); + } + + @Test + public void shouldThrowOnIncompatibleDeviceExceptionOnRSAKeyReadingWhenTryingToAESDecrypt() throws Exception { + exception.expect(IncompatibleDeviceException.class); + exception.expectMessage("The device is not compatible with the CryptoUtil class"); + + PowerMockito.mockStatic(Base64.class); + PowerMockito.when(Base64.decode("encoded-key", Base64.DEFAULT)).thenReturn(new byte[0]); + PowerMockito.when(storage.retrieveString(KEY_ALIAS)).thenReturn("encoded-key"); + + doThrow(new IncompatibleDeviceException(null)).when(cryptoUtil).getRSAKeyEntry(); + cryptoUtil.decrypt(new byte[0]); + } + + @Test + public void shouldThrowOnIncompatibleDeviceExceptionOnAESKeyReadingWhenTryingToAESDecrypt() throws Exception { + exception.expect(IncompatibleDeviceException.class); + exception.expectMessage("The device is not compatible with the CryptoUtil class"); + + doThrow(new IncompatibleDeviceException(null)).when(cryptoUtil).getAESKey(); + cryptoUtil.decrypt(new byte[0]); + } + + @Test + public void shouldThrowOnNoSuchPaddingExceptionWhenTryingToAESDecrypt() throws Exception { + exception.expect(IncompatibleDeviceException.class); doReturn(new byte[]{11, 22, 33}).when(cryptoUtil).getAESKey(); PowerMockito.mockStatic(Cipher.class); - PowerMockito.when(Cipher.getInstance(anyString())).thenThrow(new NoSuchPaddingException()); + PowerMockito.when(Cipher.getInstance(AES_TRANSFORMATION)).thenThrow(new NoSuchPaddingException()); cryptoUtil.decrypt(new byte[0]); } @Test - public void shouldThrowOnNoSuchAlgorithmErrorWhenTryingToAESDecrypt() throws Exception { - exception.expect(CryptoException.class); - exception.expectMessage("Error while decrypting the input."); - + public void shouldThrowOnNoSuchAlgorithmExceptionWhenTryingToAESDecrypt() throws Exception { + exception.expect(IncompatibleDeviceException.class); doReturn(new byte[]{11, 22, 33}).when(cryptoUtil).getAESKey(); PowerMockito.mockStatic(Cipher.class); - PowerMockito.when(Cipher.getInstance(anyString())).thenThrow(new NoSuchAlgorithmException()); + PowerMockito.when(Cipher.getInstance(AES_TRANSFORMATION)).thenThrow(new NoSuchAlgorithmException()); cryptoUtil.decrypt(new byte[0]); } @Test - public void shouldThrowOnInvalidAlgorithmParameterTryingToAESDecrypt() throws Exception { + public void shouldThrowOnEmptyInitializationVectorWhenTryingToAESDecrypt() throws Exception { exception.expect(CryptoException.class); - exception.expectMessage("Error while decrypting the input."); + exception.expectMessage("The encryption keys changed recently. You need to re-encrypt something first."); doReturn(new byte[]{11, 22, 33}).when(cryptoUtil).getAESKey(); - doThrow(new InvalidAlgorithmParameterException()).when(aesCipher) - .init(any(int.class), any(Key.class), any(AlgorithmParameterSpec.class)); + PowerMockito.mockStatic(Cipher.class); + PowerMockito.when(Cipher.getInstance(AES_TRANSFORMATION)).thenReturn(aesCipher); + PowerMockito.when(storage.retrieveString(KEY_ALIAS + "_iv")).thenReturn(""); + cryptoUtil.decrypt(new byte[0]); } @Test - public void shouldThrowOnBadPaddingErrorWhenTryingToAESDecrypt() throws Exception { - exception.expect(CryptoException.class); - exception.expectMessage("Error while decrypting the input."); + public void shouldThrowOnInvalidKeyExceptionWhenTryingToAESDecrypt() throws Exception { + Exception exception = null; + byte[] aesKeyBytes = new byte[]{11, 22, 33}; + byte[] ivBytes = new byte[]{99, 22}; + ArgumentCaptor secretKeyArgumentCaptor = ArgumentCaptor.forClass(SecretKey.class); + ArgumentCaptor ivParameterSpecArgumentCaptor = ArgumentCaptor.forClass(IvParameterSpec.class); + + try { + doReturn(aesKeyBytes).when(cryptoUtil).getAESKey(); + PowerMockito.mockStatic(Cipher.class); + PowerMockito.when(Cipher.getInstance(AES_TRANSFORMATION)).thenReturn(aesCipher); + PowerMockito.when(storage.retrieveString(KEY_ALIAS + "_iv")).thenReturn("a_valid_iv"); + + PowerMockito.mockStatic(Base64.class); + PowerMockito.when(Base64.decode("a_valid_iv", Base64.DEFAULT)).thenReturn(ivBytes); + + doThrow(new InvalidKeyException()).when(aesCipher).init(eq(Cipher.DECRYPT_MODE), secretKeyArgumentCaptor.capture(), ivParameterSpecArgumentCaptor.capture()); + + cryptoUtil.decrypt(new byte[0]); + } catch (IncompatibleDeviceException e) { + exception = e; + } + + assertThat(exception, is(notNullValue())); + assertThat(secretKeyArgumentCaptor.getValue().getAlgorithm(), is("AES")); + assertThat(secretKeyArgumentCaptor.getValue().getEncoded(), is(aesKeyBytes)); + assertThat(ivParameterSpecArgumentCaptor.getValue().getIV(), is(ivBytes)); + } - doReturn(new byte[]{11, 22, 33}).when(cryptoUtil).getAESKey(); - doThrow(new BadPaddingException()).when(aesCipher) - .doFinal(any(byte[].class)); + @Test + public void shouldThrowOnInvalidAlgorithmParameterExceptionWhenTryingToAESDecrypt() throws Exception { + Exception exception = null; + byte[] aesKeyBytes = new byte[]{11, 22, 33}; + byte[] ivBytes = new byte[]{99, 22}; + ArgumentCaptor secretKeyArgumentCaptor = ArgumentCaptor.forClass(SecretKey.class); + ArgumentCaptor ivParameterSpecArgumentCaptor = ArgumentCaptor.forClass(IvParameterSpec.class); + + try { + doReturn(aesKeyBytes).when(cryptoUtil).getAESKey(); + PowerMockito.mockStatic(Cipher.class); + PowerMockito.when(Cipher.getInstance(AES_TRANSFORMATION)).thenReturn(aesCipher); + PowerMockito.when(storage.retrieveString(KEY_ALIAS + "_iv")).thenReturn("a_valid_iv"); + + PowerMockito.mockStatic(Base64.class); + 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]); + } catch (IncompatibleDeviceException e) { + exception = e; + } + + assertThat(exception, is(notNullValue())); + assertThat(secretKeyArgumentCaptor.getValue().getAlgorithm(), is("AES")); + assertThat(secretKeyArgumentCaptor.getValue().getEncoded(), is(aesKeyBytes)); + assertThat(ivParameterSpecArgumentCaptor.getValue().getIV(), is(ivBytes)); + } + + @Test + public void shouldThrowOnBadPaddingExceptionWhenTryingToAESDecrypt() throws Exception { + exception.expect(IncompatibleDeviceException.class); + + byte[] aesKeyBytes = new byte[]{11, 22, 33}; + byte[] ivBytes = new byte[]{99, 22}; + + doReturn(aesKeyBytes).when(cryptoUtil).getAESKey(); + PowerMockito.mockStatic(Cipher.class); + PowerMockito.when(Cipher.getInstance(AES_TRANSFORMATION)).thenReturn(aesCipher); + PowerMockito.when(storage.retrieveString(KEY_ALIAS + "_iv")).thenReturn("a_valid_iv"); + + PowerMockito.mockStatic(Base64.class); + PowerMockito.when(Base64.decode("a_valid_iv", Base64.DEFAULT)).thenReturn(ivBytes); + + doThrow(new BadPaddingException()).when(aesCipher).doFinal(any(byte[].class)); cryptoUtil.decrypt(new byte[0]); } @Test - public void shouldThrowOnIllegalBlockSizeErrorWhenTryingToAESDecrypt() throws Exception { - exception.expect(CryptoException.class); - exception.expectMessage("Error while decrypting the input."); + public void shouldThrowOnIllegalBlockSizeExceptionWhenTryingToAESDecrypt() throws Exception { + exception.expect(IncompatibleDeviceException.class); - doReturn(new byte[]{11, 22, 33}).when(cryptoUtil).getAESKey(); - doThrow(new IllegalBlockSizeException()).when(aesCipher) - .doFinal(any(byte[].class)); + byte[] aesKeyBytes = new byte[]{11, 22, 33}; + doReturn(aesKeyBytes).when(cryptoUtil).getAESKey(); + PowerMockito.mockStatic(Cipher.class); + PowerMockito.when(Cipher.getInstance(AES_TRANSFORMATION)).thenReturn(aesCipher); + PowerMockito.when(storage.retrieveString(KEY_ALIAS + "_iv")).thenReturn("a_valid_iv"); + + byte[] ivBytes = new byte[]{99, 22}; + PowerMockito.mockStatic(Base64.class); + PowerMockito.when(Base64.decode("a_valid_iv", Base64.DEFAULT)).thenReturn(ivBytes); + + doThrow(new IllegalBlockSizeException()).when(aesCipher).doFinal(any(byte[].class)); cryptoUtil.decrypt(new byte[0]); } - //Helper methods + /* + * Helper methods + */ private KeyPairGeneratorSpec.Builder newKeyPairGeneratorSpecBuilder(KeyPairGeneratorSpec expectedBuilderOutput) { KeyPairGeneratorSpec.Builder builder = PowerMockito.mock(KeyPairGeneratorSpec.Builder.class); PowerMockito.when(builder.setAlias(anyString())).thenReturn(builder); diff --git a/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.java b/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.java index 77dfb3b01..74842ecd1 100644 --- a/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.java +++ b/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.java @@ -51,7 +51,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; @@ -108,6 +107,10 @@ public void shouldCreateAManagerInstance() throws Exception { assertThat(manager, is(notNullValue())); } + /* + * SAVE Credentials tests + */ + @Test public void shouldSaveRefreshableCredentialsInStorage() throws Exception { long expirationTime = CredentialsMock.CURRENT_TIME_MS + 123456 * 1000; @@ -161,41 +164,41 @@ public void shouldSaveNonRefreshableCredentialsInStorage() throws Exception { } @Test - public void shouldRetryOnceOnSaveIfUnrecoverableContentException() throws Exception { + public void shouldClearStoredCredentialsAndThrowOnSaveOnCryptoException() throws Exception { long expirationTime = CredentialsMock.CURRENT_TIME_MS + 123456 * 1000; - Credentials credentials = new CredentialsMock("idToken", "accessToken", "type", null, new Date(expirationTime), "scope"); - String json = gson.toJson(credentials); - when(crypto.encrypt(json.getBytes())).thenThrow(new UnrecoverableContentException(null)).thenReturn(json.getBytes()); - - manager.saveCredentials(credentials); + Credentials credentials = new CredentialsMock("idToken", "accessToken", "type", "refreshToken", new Date(expirationTime), "scope"); + when(crypto.encrypt(any(byte[].class))).thenThrow(new CryptoException(null, null)); + + CredentialsManagerException exception = null; + try { + manager.saveCredentials(credentials); + } catch (CredentialsManagerException e) { + exception = e; + } + assertThat(exception, is(notNullValue())); + assertThat(exception.isDeviceIncompatible(), is(false)); + assertThat(exception.getMessage(), is("A change on the Lock Screen security settings have deemed the encryption keys invalid and have been recreated. Please, try saving the credentials again.")); - verify(crypto, times(2)).encrypt(any(byte[].class)); - verify(storage).store(eq("com.auth0.credentials"), stringCaptor.capture()); - verify(storage).store("com.auth0.credentials_expires_at", expirationTime); - verify(storage).store("com.auth0.credentials_can_refresh", false); - verifyNoMoreInteractions(storage); - final String encodedJson = stringCaptor.getValue(); - assertThat(encodedJson, is(notNullValue())); - final byte[] decoded = Base64.decode(encodedJson, Base64.DEFAULT); - Credentials storedCredentials = gson.fromJson(new String(decoded), Credentials.class); - assertThat(storedCredentials.getAccessToken(), is("accessToken")); - assertThat(storedCredentials.getIdToken(), is("idToken")); - assertThat(storedCredentials.getRefreshToken(), is(nullValue())); - assertThat(storedCredentials.getType(), is("type")); - assertThat(storedCredentials.getExpiresAt(), is(notNullValue())); - assertThat(storedCredentials.getExpiresAt().getTime(), is(expirationTime)); - assertThat(storedCredentials.getScope(), is("scope")); + verify(storage).remove("com.auth0.credentials"); + verify(storage).remove("com.auth0.credentials_expires_at"); + verify(storage).remove("com.auth0.credentials_can_refresh"); } @Test - public void shouldThrowOnSaveIfCryptoError() throws Exception { - exception.expect(CredentialsManagerException.class); - exception.expectMessage("An error occurred while encrypting the credentials."); - + public void shouldThrowOnSaveOnIncompatibleDeviceException() throws Exception { long expirationTime = CredentialsMock.CURRENT_TIME_MS + 123456 * 1000; Credentials credentials = new CredentialsMock("idToken", "accessToken", "type", "refreshToken", new Date(expirationTime), "scope"); - when(crypto.encrypt(any(byte[].class))).thenThrow(new CryptoException("something", new Throwable("happened"))); - manager.saveCredentials(credentials); + when(crypto.encrypt(any(byte[].class))).thenThrow(new IncompatibleDeviceException(null)); + + CredentialsManagerException exception = null; + try { + manager.saveCredentials(credentials); + } catch (CredentialsManagerException e) { + exception = e; + } + assertThat(exception, is(notNullValue())); + assertThat(exception.isDeviceIncompatible(), is(true)); + assertThat(exception.getMessage(), is("This device is not compatible with the SecureCredentialsManager class.")); } @Test @@ -232,44 +235,50 @@ public void shouldNotThrowOnSaveIfCredentialsHaveIdTokenAndExpiresIn() throws Ex manager.saveCredentials(credentials); } + /* + * GET Credentials tests + */ + @Test - public void shouldFailOnGetCredentialsWhenCryptoExceptionIsThrown() throws Exception { + public void shouldClearStoredCredentialsAndFailOnGetCredentialsWhenCryptoExceptionIsThrown() throws Exception { verifyNoMoreInteractions(client); Date expiresAt = new Date(CredentialsMock.CURRENT_TIME_MS + 123456L * 1000); String storedJson = insertTestCredentials(true, true, true, expiresAt); - when(crypto.decrypt(storedJson.getBytes())).thenThrow(new CryptoException("This just happened", null)); + when(crypto.decrypt(storedJson.getBytes())).thenThrow(new CryptoException(null, null)); manager.getCredentials(callback); verify(callback).onFailure(exceptionCaptor.capture()); CredentialsManagerException exception = exceptionCaptor.getValue(); assertThat(exception, is(notNullValue())); assertThat(exception.getCause(), IsInstanceOf.instanceOf(CryptoException.class)); - assertThat(exception.getMessage(), is("An error occurred while decrypting the existing credentials.")); + assertThat(exception.getMessage(), is("A change on the Lock Screen security settings have deemed the encryption keys invalid and have been recreated. " + + "Any previously stored content is now lost. Please, try saving the credentials again.")); - verify(storage, never()).remove("com.auth0.credentials"); - verify(storage, never()).remove("com.auth0.credentials_expires_at"); - verify(storage, never()).remove("com.auth0.credentials_can_refresh"); + + verify(storage).remove("com.auth0.credentials"); + verify(storage).remove("com.auth0.credentials_expires_at"); + verify(storage).remove("com.auth0.credentials_can_refresh"); } @Test - public void shouldFailOnGetCredentialsAndClearStoredCredentialsWhenUnrecoverableContentExceptionIsThrown() throws Exception { + public void shouldFailOnGetCredentialsWhenIncompatibleDeviceExceptionIsThrown() throws Exception { verifyNoMoreInteractions(client); Date expiresAt = new Date(CredentialsMock.CURRENT_TIME_MS + 123456L * 1000); String storedJson = insertTestCredentials(true, true, true, expiresAt); - when(crypto.decrypt(storedJson.getBytes())).thenThrow(new UnrecoverableContentException(null)); + when(crypto.decrypt(storedJson.getBytes())).thenThrow(new IncompatibleDeviceException(null)); manager.getCredentials(callback); verify(callback).onFailure(exceptionCaptor.capture()); CredentialsManagerException exception = exceptionCaptor.getValue(); assertThat(exception, is(notNullValue())); - assertThat(exception.getCause(), IsInstanceOf.instanceOf(UnrecoverableContentException.class)); - assertThat(exception.getMessage(), is("An error occurred while decrypting the existing credentials.")); + assertThat(exception.getCause(), IsInstanceOf.instanceOf(IncompatibleDeviceException.class)); + assertThat(exception.getMessage(), is("This device is not compatible with the SecureCredentialsManager class.")); - verify(storage).remove("com.auth0.credentials"); - verify(storage).remove("com.auth0.credentials_expires_at"); - verify(storage).remove("com.auth0.credentials_can_refresh"); + verify(storage, never()).remove("com.auth0.credentials"); + verify(storage, never()).remove("com.auth0.credentials_expires_at"); + verify(storage, never()).remove("com.auth0.credentials_can_refresh"); } @Test @@ -451,6 +460,10 @@ public void shouldGetAndFailToRenewExpiredCredentials() throws Exception { assertThat(exception.getMessage(), is("An error occurred while trying to use the Refresh Token to renew the Credentials.")); } + /* + * CLEAR Credentials tests + */ + @Test public void shouldClearCredentials() throws Exception { manager.clearCredentials(); @@ -461,6 +474,10 @@ public void shouldClearCredentials() throws Exception { verifyNoMoreInteractions(storage); } + /* + * HAS Credentials tests + */ + @Test public void shouldHaveCredentialsWhenTokenHasNotExpired() throws Exception { long expirationTime = CredentialsMock.CURRENT_TIME_MS + 123456L * 1000; @@ -504,10 +521,14 @@ public void shouldNotHaveCredentialsWhenAccessTokenAndIdTokenAreMissing() throws assertFalse(manager.hasValidCredentials()); } + /* + * Authentication tests + */ + @Test public void shouldThrowOnInvalidAuthenticationRequestCode() throws Exception { exception.expect(IllegalArgumentException.class); - exception.expectMessage("Request code must a value between 1 and 255."); + exception.expectMessage("Request code must be a value between 1 and 255."); Activity activity = Robolectric.buildActivity(Activity.class).create().start().resume().get(); manager.requireAuthentication(activity, 256, null, null); @@ -690,6 +711,10 @@ public void shouldNotGetCredentialsOnDifferentAuthenticationRequestCode() throws } + /* + * Helper methods + */ + /** * Used to simplify the tests length */ From d878ec02d32e1bd3f652bcde5bb26281b05a7271 Mon Sep 17 00:00:00 2001 From: Luciano Balmaceda Date: Tue, 22 Jan 2019 17:44:32 -0300 Subject: [PATCH 5/6] update README.md and javadoc with exception explanations --- README.md | 17 +++++++++++++---- .../storage/SecureCredentialsManager.java | 9 +++++++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 1d353b1b9..a098e17cf 100644 --- a/README.md +++ b/README.md @@ -589,11 +589,11 @@ manager.clearCredentials(); ### Encryption enforced (Min API 21) -This version expands the minimum version and adds encryption to the data storage. In those devices where a Secure LockScreen has been configured it can require the user authentication before letting them obtain the stored credentials. The class is called `SecureCredentialsManager` and requires at minimum Android API 21. +This version expands the minimum version and adds encryption to the data storage. Additionally, in those devices where a Secure Lock Screen has been configured it can require the user authentication before letting them obtain the stored credentials. The class is called `SecureCredentialsManager` and requires at minimum Android API 21. #### Usage -The usage is similar to the previous version, with the slight difference that the manager now requires a valid `Context` as shown below: +The usage is similar to the previous version, with the slight difference that the manager now requires a valid android `Context` as shown below: ```java AuthenticationAPIClient authentication = new AuthenticationAPIClient(account); @@ -603,7 +603,7 @@ SecureCredentialsManager manager = new SecureCredentialsManager(this, authentica #### Requiring Authentication -You can require the user authentication to obtain credentials. This will make the manager prompt the user with the device's configured LockScreen, which they must pass correctly in order to obtain the credentials. **This feature is only available on devices where the user has setup a secured LockScreen** (PIN, Pattern, Password or Fingerprint). +You can require the user authentication to obtain credentials. This will make the manager prompt the user with the device's configured Lock Screen, which they must pass correctly in order to obtain the credentials. **This feature is only available on devices where the user has setup a secured Lock Screen** (PIN, Pattern, Password or Fingerprint). To enable authentication you must call the `requireAuthentication` method passing a valid _Activity_ context, a Request Code that represents the authentication call, and the title and description to display in the LockScreen. As seen in the snippet below, you can leave these last two parameters with `null` to use the system default resources. @@ -614,7 +614,7 @@ private static final int AUTH_REQ_CODE = 11; manager.requireAuthentication(this, AUTH_REQ_CODE, null, null); ``` -When the above conditions are met and the manager requires the user authentication, it will use the activity context to launch a new activity for the result. The outcome of getting approved or rejected by the LockScreen is given back to the activity in the `onActivityResult` method, which your activity must override to redirect the data to the manager using the `checkAuthenticationResult` method. +When the above conditions are met and the manager requires the user authentication, it will use the activity context to launch a new activity for the result. The outcome of getting approved or rejected by the Lock Screen is given back to the activity in the `onActivityResult` method, which your activity must override to redirect the data to the manager using the `checkAuthenticationResult` method. ```java @Override @@ -629,6 +629,15 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) { The `checkAuthenticationResult` method will continue the retrieval of credentials on a successful authentication, and the decrypted credentials will be delivered to the callback passed on the `getCredentials` call. +#### Handling exceptions + +In the event that something happened while trying to save or retrieve the credentials, a `CredentialsManagerException` will be thrown. These are some of the expected failure scenarios: + +- Invalid Credentials format or values. e.g. when it's missing the `access_token`, the `id_token` or the `expires_at` values. +- Tokens have expired but no `refresh_token` is available to perform a refresh credentials request. +- Device's Lock Screen security settings have changed (e.g. the PIN code was changed). Even when `hasCredentials` returns true, the encryption keys will be deemed invalid and until `saveCredentials` is called again it won't be possible to decrypt any previously existing content, since they keys used back then are not the same as the new ones. +- Device is not compatible with some of the algorithms required by the `SecureCredentialsManager` class. This is considered a catastrophic event and might happen when the OEM has modified the Android ROM removing some of the officially included algorithms. Nevertheless, it can be checked in the exception instance itself by calling `isDeviceIncompatible`. By doing so you can decide the fallback for storing the credentials, such as using the regular `CredentialsManager`. + ## FAQ * Why is the Android Lint _error_ `'InvalidPackage'` considered a _warning_? diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java index e2a007942..ab6bb691c 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.java @@ -13,6 +13,7 @@ import android.util.Base64; import android.util.Log; +import com.auth0.android.Auth0Exception; import com.auth0.android.authentication.AuthenticationAPIClient; import com.auth0.android.authentication.AuthenticationException; import com.auth0.android.callback.AuthenticationCallback; @@ -130,7 +131,8 @@ public boolean checkAuthenticationResult(int requestCode, int resultCode) { * Saves the given credentials in the Storage. * * @param credentials the credentials to save. - * @throws CredentialsManagerException if the credentials couldn't be encrypted. If the class is not supported by this device the cause will be a {@link IncompatibleDeviceException}. + * @throws CredentialsManagerException if the credentials couldn't be encrypted. Some devices are not compatible at all with the cryptographic + * implementation and will have {@link CredentialsManagerException#isDeviceIncompatible()} return true. */ public void saveCredentials(@NonNull Credentials credentials) throws CredentialsManagerException { if ((isEmpty(credentials.getAccessToken()) && isEmpty(credentials.getIdToken())) || credentials.getExpiresAt() == null) { @@ -162,7 +164,10 @@ public void saveCredentials(@NonNull Credentials credentials) throws Credentials } /** - * Tries to obtain the credentials from the Storage. + * Tries to obtain the credentials from the Storage. The callback's {@link BaseCallback#onSuccess(Object)} method will be called with the result. + * If something unexpected happens, the {@link BaseCallback#onFailure(Auth0Exception)} method will be called with the error. Some devices are not compatible + * at all with the cryptographic implementation and will have {@link CredentialsManagerException#isDeviceIncompatible()} return true. + *

* If a LockScreen is setup and {@link #requireAuthentication(Activity, int, String, String)} was called, the user will be asked to authenticate before accessing * the credentials. Your activity must override the {@link Activity#onActivityResult(int, int, Intent)} method and call * {@link #checkAuthenticationResult(int, int)} with the received values. From af1fbe57731b5097937f88810ed5cba99c5a61f5 Mon Sep 17 00:00:00 2001 From: Luciano Balmaceda Date: Thu, 24 Jan 2019 15:44:11 -0300 Subject: [PATCH 6/6] separate AES/RSA keys deletion when something goes wrong --- .../authentication/storage/CryptoUtil.java | 86 +++++++++++++------ .../storage/CryptoUtilTest.java | 85 +++++++++++++----- 2 files changed, 122 insertions(+), 49 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 af9226e8b..44dbae200 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 @@ -168,14 +168,15 @@ KeyStore.PrivateKeyEntry getRSAKeyEntry() throws CryptoException, IncompatibleDe } catch (IOException | UnrecoverableEntryException e) { /* * Any of this exceptions mean the old key pair is somehow corrupted. - * We can delete it and let the user retry the operation. + * We can delete both the RSA and the AES keys and let the user retry the operation. * * - IOException: * Thrown when there is an I/O or format problem with the keystore data. * - UnrecoverableEntryException: * Thrown when the key cannot be recovered. Probably because it was invalidated by a Lock Screen change. */ - deleteKeys(); + deleteRSAKeys(); + deleteAESKeys(); throw new CryptoException("The existing RSA key pair could not be recovered and has been deleted. " + "This occasionally happens when the Lock Screen settings are changed. You can safely retry this operation.", e); } @@ -208,13 +209,11 @@ private KeyStore.PrivateKeyEntry getKeyEntryCompat(KeyStore keyStore) throws Key } /** - * Removes the AES and RSA keys generated in a previous execution. + * Removes the RSA keys generated in a previous execution. * Used when we want the next call to {@link #encrypt(byte[])} or {@link #decrypt(byte[])} * to recreate the keys. */ - private void deleteKeys() { - storage.remove(KEY_ALIAS); - storage.remove(KEY_IV_ALIAS); + private void deleteRSAKeys() { try { KeyStore keyStore = KeyStore.getInstance(ANDROID_KEY_STORE); keyStore.load(null); @@ -225,6 +224,16 @@ private void deleteKeys() { } } + /** + * Removes the AES keys generated in a previous execution. + * Used when we want the next call to {@link #encrypt(byte[])} or {@link #decrypt(byte[])} + * to recreate the keys. + */ + private void deleteAESKeys() { + storage.remove(KEY_ALIAS); + storage.remove(KEY_IV_ALIAS); + } + /** * Decrypts the given input using a generated RSA Private Key. * Used to decrypt the AES key for later usage. @@ -241,7 +250,7 @@ byte[] RSADecrypt(byte[] encryptedInput) throws IncompatibleDeviceException, Cry Cipher cipher = Cipher.getInstance(RSA_TRANSFORMATION); cipher.init(Cipher.DECRYPT_MODE, privateKey); return cipher.doFinal(encryptedInput); - } catch (NoSuchAlgorithmException | IllegalBlockSizeException | BadPaddingException | NoSuchPaddingException | InvalidKeyException e) { + } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException e) { /* * This exceptions are safe to be ignored: * @@ -252,15 +261,24 @@ 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. + * + * Read more in https://developer.android.com/reference/javax/crypto/Cipher + */ + Log.e(TAG, "The device can't decrypt input using a RSA Key.", e); + throw new IncompatibleDeviceException(e); + } catch (IllegalBlockSizeException | BadPaddingException e) { + /* + * Any of this exceptions mean the encrypted input is somehow corrupted and cannot be recovered. + * Delete the AES keys since those originated the input. + * * - IllegalBlockSizeException: * Thrown only on encrypt mode. * - BadPaddingException: * Thrown if the input doesn't contain the proper padding bytes. * - * Read more in https://developer.android.com/reference/javax/crypto/Cipher */ - Log.e(TAG, "The device can't decrypt input using a RSA Key.", e); - throw new IncompatibleDeviceException(e); + deleteAESKeys(); + throw new CryptoException("The RSA encrypted input is corrupted and cannot be recovered. Please discard it.", e); } } @@ -280,7 +298,7 @@ byte[] RSAEncrypt(byte[] decryptedInput) throws IncompatibleDeviceException, Cry Cipher cipher = Cipher.getInstance(RSA_TRANSFORMATION); cipher.init(Cipher.ENCRYPT_MODE, certificate); return cipher.doFinal(decryptedInput); - } catch (NoSuchAlgorithmException | IllegalBlockSizeException | BadPaddingException | NoSuchPaddingException | InvalidKeyException e) { + } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException e) { /* * This exceptions are safe to be ignored: * @@ -291,15 +309,23 @@ 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. - * - IllegalBlockSizeException: - * Thrown if no padding has been requested and the length is not multiple of block size. - * - BadPaddingException: - * Thrown only on decrypt mode. * * Read more in https://developer.android.com/reference/javax/crypto/Cipher */ Log.e(TAG, "The device can't encrypt input using a RSA Key.", e); throw new IncompatibleDeviceException(e); + } catch (IllegalBlockSizeException | BadPaddingException e) { + /* + * They really should not be thrown at all since padding is requested in the transformation. + * Delete the AES keys since those originated the input. + * + * - IllegalBlockSizeException: + * Thrown if no padding has been requested and the length is not multiple of block size. + * - BadPaddingException: + * Thrown only on decrypt mode. + */ + deleteAESKeys(); + throw new CryptoException("The RSA decrypted input is invalid.", e); } } @@ -335,9 +361,6 @@ byte[] getAESKey() throws IncompatibleDeviceException, CryptoException { * - NoSuchAlgorithmException: * Thrown if the Algorithm implementation is not available. AES was introduced in API 1 * - * However if any of this exceptions happens to be thrown (OEMs often change their Android distribution source code), - * all the checks performed in this class wouldn't matter and the device would not be compatible at all with it. - * * Read more in https://developer.android.com/reference/javax/crypto/KeyGenerator */ Log.e(TAG, "Error while creating the AES key.", e); @@ -367,7 +390,7 @@ public byte[] decrypt(byte[] encryptedInput) throws CryptoException, Incompatibl 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 | BadPaddingException | IllegalBlockSizeException e) { + } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | InvalidAlgorithmParameterException e) { /* * This exceptions are safe to be ignored: * @@ -380,15 +403,20 @@ public byte[] decrypt(byte[] encryptedInput) throws CryptoException, Incompatibl * Thrown if the given key is inappropriate for initializing this cipher. * - InvalidAlgorithmParameterException: * If the IV parameter is null. - * - BadPaddingException: - * Thrown if the input doesn't contain the proper padding bytes. In this case, if the input contains padding. - * - IllegalBlockSizeException: - * Thrown only on encrypt mode. * * Read more in https://developer.android.com/reference/javax/crypto/Cipher */ Log.e(TAG, "Error while decrypting the input.", e); throw new IncompatibleDeviceException(e); + } catch (BadPaddingException | IllegalBlockSizeException e) { + /* + * Any of this exceptions mean the encrypted input is somehow corrupted and cannot be recovered. + * - BadPaddingException: + * Thrown if the input doesn't contain the proper padding bytes. In this case, if the input contains padding. + * - IllegalBlockSizeException: + * Thrown only on encrypt mode. + */ + throw new CryptoException("The AES encrypted input is corrupted and cannot be recovered. Please discard it.", e); } } @@ -411,7 +439,7 @@ public byte[] encrypt(byte[] decryptedInput) throws CryptoException, Incompatibl //Save IV for Decrypt stage storage.store(KEY_IV_ALIAS, new String(encodedIV)); return encrypted; - } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | BadPaddingException | IllegalBlockSizeException e) { + } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException e) { /* * This exceptions are safe to be ignored: * @@ -426,13 +454,19 @@ public byte[] encrypt(byte[] decryptedInput) throws CryptoException, Incompatibl * If the IV parameter is null. * - BadPaddingException: * Thrown only on decrypt mode. - * - IllegalBlockSizeException: - * Thrown if no padding has been requested and the length is not multiple of block size. * * Read more in https://developer.android.com/reference/javax/crypto/Cipher */ Log.e(TAG, "Error while encrypting the input.", e); throw new IncompatibleDeviceException(e); + } catch (IllegalBlockSizeException | BadPaddingException e) { + /* + * - IllegalBlockSizeException: + * Thrown if no padding has been requested and the length is not multiple of block size. + * - BadPaddingException: + * Thrown only on decrypt mode. + */ + throw new CryptoException("The AES decrypted input is invalid.", 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 f8c8dc5e8..a91aedfe5 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 @@ -63,6 +63,7 @@ import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.never; import static org.powermock.api.mockito.PowerMockito.doReturn; import static org.powermock.api.mockito.PowerMockito.doThrow; import static org.powermock.api.mockito.PowerMockito.mock; @@ -161,7 +162,7 @@ public void shouldNotCreateProtectedRSAKeyPairIfMissingAndLockScreenEnabledOnAPI Mockito.verify(builder).setSerialNumber(BigInteger.ONE); Mockito.verify(builder).setStartDate(startDateCaptor.capture()); Mockito.verify(builder).setEndDate(endDateCaptor.capture()); - Mockito.verify(builder, Mockito.never()).setEncryptionRequired(); + Mockito.verify(builder, never()).setEncryptionRequired(); Mockito.verify(keyPairGenerator).initialize(spec); Mockito.verify(keyPairGenerator).generateKeyPair(); @@ -213,7 +214,7 @@ public void shouldCreateUnprotectedRSAKeyPairIfMissingAndLockScreenDisabledOnAPI Mockito.verify(builder).setSerialNumber(BigInteger.ONE); Mockito.verify(builder).setStartDate(startDateCaptor.capture()); Mockito.verify(builder).setEndDate(endDateCaptor.capture()); - Mockito.verify(builder, Mockito.never()).setEncryptionRequired(); + Mockito.verify(builder, never()).setEncryptionRequired(); Mockito.verify(keyPairGenerator).initialize(spec); Mockito.verify(keyPairGenerator).generateKeyPair(); @@ -453,7 +454,7 @@ public void shouldThrowOnUnrecoverableEntryExceptionWhenTryingToObtainRSAKeys() } @Test - public void shouldDeleteKeysAndThrowOnIOExceptionWhenTryingToObtainRSAKeys() throws Exception { + public void shouldDeleteRSAAndAESKeysAndThrowOnIOExceptionWhenTryingToObtainRSAKeys() throws Exception { exception.expect(CryptoException.class); exception.expectMessage("The existing RSA key pair could not be recovered and has been deleted. " + "This occasionally happens when the Lock Screen settings are changed. You can safely retry this operation."); @@ -633,9 +634,9 @@ public void shouldThrowOnInvalidKeyExceptionWhenTryingToRSAEncrypt() throws Exce } @Test - public void shouldThrowOnBadPaddingExceptionWhenTryingToRSAEncrypt() throws Exception { - exception.expect(IncompatibleDeviceException.class); - exception.expectMessage("The device is not compatible with the CryptoUtil class"); + public void shouldDeleteAESKeysAndThrowOnBadPaddingExceptionWhenTryingToRSAEncrypt() throws Exception { + exception.expect(CryptoException.class); + exception.expectMessage("The RSA decrypted input is invalid."); byte[] sampleBytes = new byte[0]; Certificate certificate = PowerMockito.mock(Certificate.class); @@ -647,12 +648,16 @@ public void shouldThrowOnBadPaddingExceptionWhenTryingToRSAEncrypt() throws Exce PowerMockito.when(rsaCipher.doFinal(sampleBytes)).thenThrow(new BadPaddingException()); cryptoUtil.RSAEncrypt(sampleBytes); + + Mockito.verify(keyStore, never()).deleteEntry(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); } @Test - public void shouldThrowOnIllegalBlockSizeExceptionWhenTryingToRSAEncrypt() throws Exception { - exception.expect(IncompatibleDeviceException.class); - exception.expectMessage("The device is not compatible with the CryptoUtil class"); + public void shouldDeleteAESKeysAndThrowOnIllegalBlockSizeExceptionWhenTryingToRSAEncrypt() throws Exception { + exception.expect(CryptoException.class); + exception.expectMessage("The RSA decrypted input is invalid."); Certificate certificate = PowerMockito.mock(Certificate.class); KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); @@ -663,6 +668,10 @@ public void shouldThrowOnIllegalBlockSizeExceptionWhenTryingToRSAEncrypt() throw PowerMockito.when(rsaCipher.doFinal(any(byte[].class))).thenThrow(new IllegalBlockSizeException()); cryptoUtil.RSAEncrypt(new byte[0]); + + Mockito.verify(keyStore, never()).deleteEntry(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); } @Test @@ -764,9 +773,9 @@ public void shouldThrowOnNoSuchPaddingExceptionWhenTryingToRSADecrypt() throws E } @Test - public void shouldThrowOnBadPaddingExceptionWhenTryingToRSADecrypt() throws Exception { - exception.expect(IncompatibleDeviceException.class); - exception.expectMessage("The device is not compatible with the CryptoUtil class"); + public void shouldDeleteAESKeysAndThrowOnBadPaddingExceptionWhenTryingToRSADecrypt() throws Exception { + exception.expect(CryptoException.class); + exception.expectMessage("The RSA encrypted input is corrupted and cannot be recovered. Please discard it."); PrivateKey privateKey = PowerMockito.mock(PrivateKey.class); KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); @@ -775,12 +784,16 @@ public void shouldThrowOnBadPaddingExceptionWhenTryingToRSADecrypt() throws Exce doThrow(new BadPaddingException()).when(rsaCipher).doFinal(any(byte[].class)); cryptoUtil.RSADecrypt(new byte[0]); + + Mockito.verify(keyStore, never()).deleteEntry(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); } @Test - public void shouldThrowOnIllegalBlockSizeExceptionWhenTryingToRSADecrypt() throws Exception { - exception.expect(IncompatibleDeviceException.class); - exception.expectMessage("The device is not compatible with the CryptoUtil class"); + public void shouldDeleteAESKeysAndThrowOnIllegalBlockSizeExceptionWhenTryingToRSADecrypt() throws Exception { + exception.expect(CryptoException.class); + exception.expectMessage("The RSA encrypted input is corrupted and cannot be recovered. Please discard it."); PrivateKey privateKey = PowerMockito.mock(PrivateKey.class); KeyStore.PrivateKeyEntry privateKeyEntry = PowerMockito.mock(KeyStore.PrivateKeyEntry.class); @@ -789,6 +802,10 @@ public void shouldThrowOnIllegalBlockSizeExceptionWhenTryingToRSADecrypt() throw doThrow(new IllegalBlockSizeException()).when(rsaCipher).doFinal(any(byte[].class)); cryptoUtil.RSADecrypt(new byte[0]); + + Mockito.verify(keyStore, never()).deleteEntry(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS); + Mockito.verify(storage).remove(KEY_ALIAS + "_iv"); } /* @@ -911,23 +928,35 @@ public void shouldThrowOnInvalidKeyExceptionWhenTryingToAESEncrypt() throws Exce @Test - public void shouldThrowOnBadPaddingExceptionWhenTryingToAESEncrypt() throws Exception { - exception.expect(IncompatibleDeviceException.class); + public void shouldDeleteAESKeysAndThrowOnBadPaddingExceptionWhenTryingToAESEncrypt() throws Exception { + exception.expect(CryptoException.class); + exception.expectMessage("The AES decrypted input is invalid."); + doReturn(new byte[]{11, 22, 33}).when(cryptoUtil).getAESKey(); doThrow(new BadPaddingException()).when(aesCipher) .doFinal(any(byte[].class)); cryptoUtil.encrypt(new byte[0]); + + Mockito.verify(keyStore, never()).deleteEntry(KEY_ALIAS); + Mockito.verify(storage, never()).remove(KEY_ALIAS); + Mockito.verify(storage, never()).remove(KEY_ALIAS + "_iv"); } @Test - public void shouldThrowOnIllegalBlockSizeExceptionWhenTryingToAESEncrypt() throws Exception { - exception.expect(IncompatibleDeviceException.class); + public void shouldDeleteAESKeysAndThrowOnIllegalBlockSizeExceptionWhenTryingToAESEncrypt() throws Exception { + exception.expect(CryptoException.class); + exception.expectMessage("The AES decrypted input is invalid."); + doReturn(new byte[]{11, 22, 33}).when(cryptoUtil).getAESKey(); doThrow(new IllegalBlockSizeException()).when(aesCipher) .doFinal(any(byte[].class)); cryptoUtil.encrypt(new byte[0]); + + Mockito.verify(keyStore, never()).deleteEntry(KEY_ALIAS); + Mockito.verify(storage, never()).remove(KEY_ALIAS); + Mockito.verify(storage, never()).remove(KEY_ALIAS + "_iv"); } @@ -1102,8 +1131,9 @@ public void shouldThrowOnInvalidAlgorithmParameterExceptionWhenTryingToAESDecryp } @Test - public void shouldThrowOnBadPaddingExceptionWhenTryingToAESDecrypt() throws Exception { - exception.expect(IncompatibleDeviceException.class); + public void shouldDeleteAESKeysAndThrowOnBadPaddingExceptionWhenTryingToAESDecrypt() throws Exception { + exception.expect(CryptoException.class); + exception.expectMessage("The AES encrypted input is corrupted and cannot be recovered. Please discard it."); byte[] aesKeyBytes = new byte[]{11, 22, 33}; byte[] ivBytes = new byte[]{99, 22}; @@ -1119,11 +1149,16 @@ public void shouldThrowOnBadPaddingExceptionWhenTryingToAESDecrypt() throws Exce doThrow(new BadPaddingException()).when(aesCipher).doFinal(any(byte[].class)); cryptoUtil.decrypt(new byte[0]); + + Mockito.verify(keyStore, never()).deleteEntry(KEY_ALIAS); + Mockito.verify(storage, never()).remove(KEY_ALIAS); + Mockito.verify(storage, never()).remove(KEY_ALIAS + "_iv"); } @Test - public void shouldThrowOnIllegalBlockSizeExceptionWhenTryingToAESDecrypt() throws Exception { - exception.expect(IncompatibleDeviceException.class); + public void shouldDeleteAESKeysAndThrowOnIllegalBlockSizeExceptionWhenTryingToAESDecrypt() throws Exception { + exception.expect(CryptoException.class); + exception.expectMessage("The AES encrypted input is corrupted and cannot be recovered. Please discard it."); byte[] aesKeyBytes = new byte[]{11, 22, 33}; doReturn(aesKeyBytes).when(cryptoUtil).getAESKey(); @@ -1138,6 +1173,10 @@ public void shouldThrowOnIllegalBlockSizeExceptionWhenTryingToAESDecrypt() throw doThrow(new IllegalBlockSizeException()).when(aesCipher).doFinal(any(byte[].class)); cryptoUtil.decrypt(new byte[0]); + + Mockito.verify(keyStore, never()).deleteEntry(KEY_ALIAS); + Mockito.verify(storage, never()).remove(KEY_ALIAS); + Mockito.verify(storage, never()).remove(KEY_ALIAS + "_iv"); }