Replace AbstractSecretKeyLoader with ISecretKeyProvider, Fixes AB#3297746

changelog.txt

@@ -1,5 +1,6 @@
 vNext
 ----------
+- [MINOR] Replace AbstractSecretKeyLoader with ISecretKeyProvider (#2666)
 - [MINOR] Update IP phone app teams signature constants to use SHA-512 format (#2700)
 - [MINOR] Updating handling of ssl error received in Android WebView's onReceivedSslError callback (#2691)
 - [MINOR] Fixing the sign in screens when edge to edge is enabled (#2665)

common/src/androidTest/java/com/microsoft/identity/common/crypto/AndroidWrappedKeyLoaderTest.java

@@ -29,7 +29,6 @@
 
 import com.microsoft.identity.common.adal.internal.AuthenticationSettings;
 import com.microsoft.identity.common.internal.util.AndroidKeyStoreUtil;
-import com.microsoft.identity.common.java.crypto.key.AES256KeyLoader;
 import com.microsoft.identity.common.java.exception.ClientException;
 import com.microsoft.identity.common.java.util.FileUtil;
 
@@ -57,6 +56,8 @@ public class AndroidWrappedKeyLoaderTest {
     final String MOCK_KEY_FILE_PATH = "MOCK_KEY_FILE_PATH";
     final int TEST_LOOP = 100;
 
+    final String AES_ALGORITHM = "AES";
+
     @Before
     public void setUp() throws Exception {
         // Everything is on clean slate.
@@ -113,7 +114,7 @@ public void testGenerateKey() throws ClientException {
         final AndroidWrappedKeyLoader keyLoader = new AndroidWrappedKeyLoader(MOCK_KEY_ALIAS, MOCK_KEY_FILE_PATH, context);
         final SecretKey secretKey = keyLoader.generateRandomKey();
 
-        Assert.assertEquals(AES256KeyLoader.AES_ALGORITHM, secretKey.getAlgorithm());
+        Assert.assertEquals(AES_ALGORITHM, secretKey.getAlgorithm());
     }
 
     @Test
@@ -125,8 +126,8 @@ public void testReadKeyDirectly() throws ClientException {
         // They're not the same object!
         Assert.assertNotSame(secretKey, storedSecretKey);
 
-        Assert.assertEquals(AES256KeyLoader.AES_ALGORITHM, secretKey.getAlgorithm());
-        Assert.assertEquals(AES256KeyLoader.AES_ALGORITHM, storedSecretKey.getAlgorithm());
+        Assert.assertEquals(AES_ALGORITHM, secretKey.getAlgorithm());
+        Assert.assertEquals(AES_ALGORITHM, storedSecretKey.getAlgorithm());
 
         Assert.assertNotNull(secretKey.getEncoded());
         Assert.assertNotNull(storedSecretKey.getEncoded());
@@ -145,7 +146,7 @@ public void testLoadKey() throws ClientException {
 
         final SecretKey key = keyLoader.getKeyCache().getData();
         Assert.assertNotNull(key);
-        Assert.assertEquals(AES256KeyLoader.AES_ALGORITHM, secretKey.getAlgorithm());
+        Assert.assertEquals(AES_ALGORITHM, secretKey.getAlgorithm());
         Assert.assertArrayEquals(secretKey.getEncoded(), key.getEncoded());
         Assert.assertEquals(secretKey.getFormat(), key.getFormat());
     }

common/src/main/java/com/microsoft/identity/common/crypto/AndroidAuthSdkStorageEncryptionManager.java

@@ -26,9 +26,8 @@
 
 import com.microsoft.identity.common.adal.internal.AuthenticationSettings;
 import com.microsoft.identity.common.java.crypto.StorageEncryptionManager;
-import com.microsoft.identity.common.java.crypto.key.AES256KeyLoader;
-import com.microsoft.identity.common.java.crypto.key.AbstractSecretKeyLoader;
-import com.microsoft.identity.common.java.crypto.key.PredefinedKeyLoader;
+import com.microsoft.identity.common.java.crypto.key.ISecretKeyProvider;
+import com.microsoft.identity.common.java.crypto.key.PredefinedKeyProvider;
 import com.microsoft.identity.common.logging.Logger;
 
 import java.util.Collections;
@@ -54,26 +53,27 @@ public class AndroidAuthSdkStorageEncryptionManager extends StorageEncryptionMan
      */
     public static final String WRAPPED_KEY_FILE_NAME = "adalks";
 
-    private final PredefinedKeyLoader mPredefinedKeyLoader;
-    private final AndroidWrappedKeyLoader mKeyStoreKeyLoader;
+    private final PredefinedKeyProvider mPredefinedKeyLoader;
+    private final ISecretKeyProvider mKeyStoreKeyLoader;
 
     public AndroidAuthSdkStorageEncryptionManager(@NonNull final Context context) {
         if (AuthenticationSettings.INSTANCE.getSecretKeyData() == null) {
             mPredefinedKeyLoader = null;
         } else {
-            mPredefinedKeyLoader = new PredefinedKeyLoader("USER_DEFINED_KEY",
+            mPredefinedKeyLoader = new PredefinedKeyProvider("USER_DEFINED_KEY",
                     AuthenticationSettings.INSTANCE.getSecretKeyData());
         }
 
         mKeyStoreKeyLoader = new AndroidWrappedKeyLoader(
                 WRAPPING_KEY_ALIAS,
                 WRAPPED_KEY_FILE_NAME,
-                context);
+                context
+        );
     }
 
     @Override
     @NonNull
-    public AES256KeyLoader getKeyLoaderForEncryption() {
+    public ISecretKeyProvider getKeyLoaderForEncryption() {
         if (mPredefinedKeyLoader != null) {
             return mPredefinedKeyLoader;
         }
@@ -83,20 +83,20 @@ public AES256KeyLoader getKeyLoaderForEncryption() {
 
     @Override
     @NonNull
-    public List<AbstractSecretKeyLoader> getKeyLoaderForDecryption(byte[] cipherText) {
+    public List<ISecretKeyProvider> getKeyLoaderForDecryption(byte[] cipherText) {
         final String methodTag = TAG + ":getKeyLoaderForDecryption";
 
         final String keyIdentifier = getKeyIdentifierFromCipherText(cipherText);
-        if (PredefinedKeyLoader.USER_PROVIDED_KEY_IDENTIFIER.equalsIgnoreCase(keyIdentifier)) {
+        if (PredefinedKeyProvider.USER_PROVIDED_KEY_IDENTIFIER.equalsIgnoreCase(keyIdentifier)) {
             if (mPredefinedKeyLoader != null) {
-                return Collections.<AbstractSecretKeyLoader>singletonList(mPredefinedKeyLoader);
+                return Collections.singletonList(mPredefinedKeyLoader);
             } else {
                 throw new IllegalStateException(
                         "Cipher Text is encrypted by USER_PROVIDED_KEY_IDENTIFIER, " +
                                 "but mPredefinedKeyLoader is null.");
             }
-        } else if (AndroidWrappedKeyLoader.WRAPPED_KEY_KEY_IDENTIFIER.equalsIgnoreCase(keyIdentifier)) {
-            return Collections.<AbstractSecretKeyLoader>singletonList(mKeyStoreKeyLoader);
+        } else if (mKeyStoreKeyLoader.getKeyTypeIdentifier().equalsIgnoreCase(keyIdentifier)) {
+            return Collections.singletonList(mKeyStoreKeyLoader);
         }
 
         Logger.warn(methodTag,

common/src/main/java/com/microsoft/identity/common/crypto/AndroidWrappedKeyLoader.java

@@ -32,7 +32,7 @@
 
 import com.microsoft.identity.common.internal.util.AndroidKeyStoreUtil;
 import com.microsoft.identity.common.java.controllers.ExceptionAdapter;
-import com.microsoft.identity.common.java.crypto.key.AES256KeyLoader;
+import com.microsoft.identity.common.java.crypto.key.AbstractAES256SecretKeyProvider;
 import com.microsoft.identity.common.java.crypto.key.KeyUtil;
 import com.microsoft.identity.common.java.exception.ClientException;
 import com.microsoft.identity.common.java.flighting.CommonFlight;
@@ -71,7 +71,15 @@
  * Instead, the actual key that we use to encrypt/decrypt data is 'wrapped/encrypted' with the keystore key
  * before it get saved to the file.
  */
-public class AndroidWrappedKeyLoader extends AES256KeyLoader {
+public class AndroidWrappedKeyLoader extends AbstractAES256SecretKeyProvider {
+
+    /**
+     * AES is 16 bytes (128 bits), thus PKCS#5 padding should not work, but in
+     * Java AES/CBC/PKCS5Padding is default(!) algorithm name, thus PKCS5 here
+     * probably doing PKCS7. We decide to go with Java default string.
+     */
+    private static final String CIPHER_TRANSFORMATION = "AES/CBC/PKCS5Padding";
+
     private static final String TAG = AndroidWrappedKeyLoader.class.getSimpleName() + "#";
 
     /**
@@ -177,12 +185,11 @@ public synchronized SecretKey getKey() throws ClientException {
         return key;
     }
 
-    @Override
     @NonNull
     protected SecretKey generateRandomKey() throws ClientException {
         final String methodTag = TAG + ":generateRandomKey";
 
-        final SecretKey key = super.generateRandomKey();
+        final SecretKey key = AES_256_KEY_GENERATOR.generateRandomKey();
         saveSecretKeyToStorage(key);
 
         Logger.info(methodTag, "New key is generated with thumbprint: " +
@@ -217,7 +224,7 @@ protected SecretKey generateRandomKey() throws ClientException {
                 return null;
             }
 
-            final SecretKey key = AndroidKeyStoreUtil.unwrap(wrappedSecretKey, getKeySpecAlgorithm(), keyPair, WRAP_ALGORITHM);
+            final SecretKey key = AndroidKeyStoreUtil.unwrap(wrappedSecretKey, KEY_ALGORITHM, keyPair, WRAP_ALGORITHM);
 
             Logger.info(methodTag, "Key is loaded with thumbprint: " +
                     KeyUtil.getKeyThumbPrint(key));
@@ -259,7 +266,7 @@ private void saveSecretKeyToStorage(@NonNull final SecretKey unencryptedKey) thr
         if (keyPair == null) {
             Logger.info(methodTag, "No existing keypair. Generating a new one.");
             final Span span = OTelUtility.createSpanFromParent(SpanName.KeyPairGeneration.name(), SpanExtension.current().getSpanContext());
-            try (final Scope scope = SpanExtension.makeCurrentSpan(span)) {
+            try (final Scope ignored = SpanExtension.makeCurrentSpan(span)) {
                 keyPair = generateNewKeyPair();
                 span.setStatus(StatusCode.OK);
             } catch (final ClientException e) {
@@ -329,7 +336,7 @@ private KeyPair generateNewKeyPairAPI23AndAbove() throws ClientException {
      * Generate a new key pair wrapping key based on legacy logic. Call this for API < 23 or as fallback
      * until new key gen specs are stable.
      * @return key pair generated with legacy spec
-     * @throws ClientException
+     * @throws ClientException if there is an error generating the key pair.
      */
     @NonNull
     private KeyPair generateKeyPairWithLegacySpec() throws ClientException{
@@ -464,4 +471,10 @@ private File getKeyFile() {
                 mContext.getDir(mContext.getPackageName(), Context.MODE_PRIVATE),
                 mFilePath);
     }
+
+    @NonNull
+    @Override
+    public String getCipherTransformation() {
+        return CIPHER_TRANSFORMATION;
+    }
 }

common/src/test/java/com/microsoft/identity/common/crypto/AndroidAuthSdkStorageEncryptionManagerTest.java

@@ -32,9 +32,9 @@
 import androidx.test.core.app.ApplicationProvider;
 
 import com.microsoft.identity.common.adal.internal.AuthenticationSettings;
-import com.microsoft.identity.common.java.crypto.key.AbstractSecretKeyLoader;
+import com.microsoft.identity.common.java.crypto.key.ISecretKeyProvider;
 import com.microsoft.identity.common.java.crypto.key.KeyUtil;
-import com.microsoft.identity.common.java.crypto.key.PredefinedKeyLoader;
+import com.microsoft.identity.common.java.crypto.key.PredefinedKeyProvider;
 
 import org.junit.Assert;
 import org.junit.Before;
@@ -62,7 +62,7 @@ public void setUp() {
     public void testGetEncryptionKey() {
         final AndroidAuthSdkStorageEncryptionManager manager = new AndroidAuthSdkStorageEncryptionManager(context);
 
-        final AbstractSecretKeyLoader loader = manager.getKeyLoaderForEncryption();
+        final ISecretKeyProvider loader = manager.getKeyLoaderForEncryption();
         Assert.assertTrue(loader instanceof AndroidWrappedKeyLoader);
         Assert.assertNotEquals(KeyUtil.getKeyThumbPrint(secretKeyMock), KeyUtil.getKeyThumbPrint(loader));
     }
@@ -72,8 +72,8 @@ public void testGetEncryptionKey_PreDefinedKeyProvided() {
         AuthenticationSettings.INSTANCE.setSecretKey(PREDEFINED_KEY);
         final AndroidAuthSdkStorageEncryptionManager manager = new AndroidAuthSdkStorageEncryptionManager(context);
 
-        final AbstractSecretKeyLoader loader = manager.getKeyLoaderForEncryption();
-        Assert.assertTrue(loader instanceof PredefinedKeyLoader);
+        final ISecretKeyProvider loader = manager.getKeyLoaderForEncryption();
+        Assert.assertTrue(loader instanceof PredefinedKeyProvider);
         Assert.assertEquals(KeyUtil.getKeyThumbPrint(secretKeyMock), KeyUtil.getKeyThumbPrint(loader));
     }
 
@@ -84,7 +84,7 @@ public void testGetEncryptionKey_PreDefinedKeyProvided() {
     @Test
     public void testGetDecryptionKey_ForDataEncryptedWithKeyStoreKey() {
         final AndroidAuthSdkStorageEncryptionManager manager = new AndroidAuthSdkStorageEncryptionManager(context);
-        final List<AbstractSecretKeyLoader> keyLoaderList = manager.getKeyLoaderForDecryption(TEXT_ENCRYPTED_BY_ANDROID_WRAPPED_KEY);
+        final List<ISecretKeyProvider> keyLoaderList = manager.getKeyLoaderForDecryption(TEXT_ENCRYPTED_BY_ANDROID_WRAPPED_KEY);
 
         Assert.assertEquals(1, keyLoaderList.size());
         Assert.assertTrue(keyLoaderList.get(0) instanceof AndroidWrappedKeyLoader);
@@ -99,7 +99,7 @@ public void testGetDecryptionKey_ForDataEncryptedWithKeyStoreKey() {
     public void testGetDecryptionKey_ForDataEncryptedWithKeyStoreKey_PreDefinedKeyProvided() {
         AuthenticationSettings.INSTANCE.setSecretKey(PREDEFINED_KEY);
         final AndroidAuthSdkStorageEncryptionManager manager = new AndroidAuthSdkStorageEncryptionManager(context);
-        final List<AbstractSecretKeyLoader> keyLoaderList = manager.getKeyLoaderForDecryption(TEXT_ENCRYPTED_BY_ANDROID_WRAPPED_KEY);
+        final List<ISecretKeyProvider> keyLoaderList = manager.getKeyLoaderForDecryption(TEXT_ENCRYPTED_BY_ANDROID_WRAPPED_KEY);
 
         Assert.assertEquals(1, keyLoaderList.size());
         Assert.assertTrue(keyLoaderList.get(0) instanceof AndroidWrappedKeyLoader);
@@ -114,7 +114,7 @@ public void testGetDecryptionKey_ForDataEncryptedWithKeyStoreKey_PreDefinedKeyPr
     public void testGetDecryptionKey_ForDataEncryptedWithPreDefinedKey() {
         final AndroidAuthSdkStorageEncryptionManager manager = new AndroidAuthSdkStorageEncryptionManager(context);
         try {
-            final List<AbstractSecretKeyLoader> keyLoaderList = manager.getKeyLoaderForDecryption(TEXT_ENCRYPTED_BY_PREDEFINED_KEY);
+            final List<ISecretKeyProvider> keyLoaderList = manager.getKeyLoaderForDecryption(TEXT_ENCRYPTED_BY_PREDEFINED_KEY);
         } catch (IllegalStateException ex) {
             Assert.assertEquals(
                     "Cipher Text is encrypted by USER_PROVIDED_KEY_IDENTIFIER, but mPredefinedKeyLoader is null.",
@@ -125,7 +125,7 @@ public void testGetDecryptionKey_ForDataEncryptedWithPreDefinedKey() {
     public void testGetDecryptionKey_ForUnencryptedText_returns_empty_keyloader() {
         AuthenticationSettings.INSTANCE.setIgnoreKeyLoaderNotFoundError(false);
         final AndroidAuthSdkStorageEncryptionManager manager = new AndroidAuthSdkStorageEncryptionManager(context);
-        final List<AbstractSecretKeyLoader> keyLoaderList = manager.getKeyLoaderForDecryption("Unencrypted".getBytes(ENCODING_UTF8));
+        final List<ISecretKeyProvider> keyLoaderList = manager.getKeyLoaderForDecryption("Unencrypted".getBytes(ENCODING_UTF8));
         Assert.assertEquals(0, keyLoaderList.size());
     }
 
@@ -137,10 +137,10 @@ public void testGetDecryptionKey_ForUnencryptedText_returns_empty_keyloader() {
     public void testGetDecryptionKey_ForDataEncryptedWithPreDefinedKey_PreDefinedKeyProvided() {
         AuthenticationSettings.INSTANCE.setSecretKey(PREDEFINED_KEY);
         final AndroidAuthSdkStorageEncryptionManager manager = new AndroidAuthSdkStorageEncryptionManager(context);
-        final List<AbstractSecretKeyLoader> keyLoaderList = manager.getKeyLoaderForDecryption(TEXT_ENCRYPTED_BY_PREDEFINED_KEY);
+        final List<ISecretKeyProvider> keyLoaderList = manager.getKeyLoaderForDecryption(TEXT_ENCRYPTED_BY_PREDEFINED_KEY);
 
         Assert.assertEquals(1, keyLoaderList.size());
-        Assert.assertTrue(keyLoaderList.get(0) instanceof PredefinedKeyLoader);
+        Assert.assertTrue(keyLoaderList.get(0) instanceof PredefinedKeyProvider);
         Assert.assertEquals(KeyUtil.getKeyThumbPrint(secretKeyMock), KeyUtil.getKeyThumbPrint(keyLoaderList.get(0)));
     }
 }

common/src/test/java/com/microsoft/identity/common/shadows/ShadowAndroidSdkStorageEncryptionManager.java

@@ -23,8 +23,8 @@
 package com.microsoft.identity.common.shadows;
 
 import com.microsoft.identity.common.crypto.AndroidAuthSdkStorageEncryptionManager;
-import com.microsoft.identity.common.java.crypto.key.PredefinedKeyLoader;
-import com.microsoft.identity.common.java.crypto.key.AES256KeyLoader;
+import com.microsoft.identity.common.java.crypto.key.PredefinedKeyProvider;
+import com.microsoft.identity.common.java.crypto.key.AbstractAES256SecretKeyProvider;
 
 import org.robolectric.annotation.Implements;
 
@@ -35,14 +35,14 @@
 public class ShadowAndroidSdkStorageEncryptionManager {
 
     final byte[] encryptionKey = new byte[]{22, 78, -69, -66, 84, -65, 119, -9, -34, -80, 60, 67, -12, -117, 86, -47, -84, -24, -18, 121, 70, 32, -110, 51, -93, -10, -93, -110, 124, -68, -42, -119};
-    final AES256KeyLoader mUserDefinedKey = new PredefinedKeyLoader("MOCK_ALIAS", encryptionKey);
+    final AbstractAES256SecretKeyProvider mUserDefinedKey = new PredefinedKeyProvider("MOCK_ALIAS", encryptionKey);
 
-    public  AES256KeyLoader getKeyLoaderForEncryption() {
+    public AbstractAES256SecretKeyProvider getKeyLoaderForEncryption() {
         return mUserDefinedKey;
     }
 
-    public List<AES256KeyLoader> getKeyLoaderForDecryption(byte[] cipherText) {
-        return new ArrayList<AES256KeyLoader>() {{
+    public List<AbstractAES256SecretKeyProvider> getKeyLoaderForDecryption(byte[] cipherText) {
+        return new ArrayList<AbstractAES256SecretKeyProvider>() {{
             add(mUserDefinedKey);
         }};
     }