Merging Somalaya/release/19.0.2 to release/19.0.2 (#2593)

- Hotfix for Keypair generation errors observed in https://portal.microsofticm.com/imp/v5/incidents/details/592800555/summary and https://portal.microsofticm.com/imp/v5/incidents/details/596010888/summary - PR for original fix made in dev https://github.com/AzureAD/microsoft-authentication-library-common-for-android/pull/2586/files
AzureAD · Feb 13, 2025 · d2e9122 · d2e9122
1 parent f94be27
commit d2e9122
Show file tree

Hide file tree

Showing 8 changed files with 124 additions and 29 deletions.
diff --git a/changelog.txt b/changelog.txt
@@ -1,3 +1,8 @@
+Version 19.0.2
+----------
+(common4j 16.0.2)
+- [PATCH] Add fallback mechanisms for keypair generation (#2586)
+
 Version 19.0.1
 ----------
 (common4j 16.0.1)

diff --git a/common/build.gradle b/common/build.gradle
@@ -31,7 +31,7 @@ codeCoverageReport {
 
 // In dev, we want to keep the dependencies(common4j, broker4j, common) to 1.0.+ to be able to be consumed by daily dev pipeline.
 // In release/*, we change these to specific versions being consumed.
-def common4jVersion = "16.0.1"
+def common4jVersion = "16.0.2"
 if (project.hasProperty("distCommon4jVersion") && project.distCommon4jVersion != '') {
     common4jVersion = project.distCommon4jVersion
 }

diff --git a/common/src/main/java/com/microsoft/identity/common/crypto/AndroidWrappedKeyLoader.java b/common/src/main/java/com/microsoft/identity/common/crypto/AndroidWrappedKeyLoader.java
@@ -35,6 +35,12 @@
 import com.microsoft.identity.common.java.crypto.key.AES256KeyLoader;
 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;
+import com.microsoft.identity.common.java.flighting.CommonFlightsManager;
+import com.microsoft.identity.common.java.opentelemetry.AttributeName;
+import com.microsoft.identity.common.java.opentelemetry.OTelUtility;
+import com.microsoft.identity.common.java.opentelemetry.SpanExtension;
+import com.microsoft.identity.common.java.opentelemetry.SpanName;
 import com.microsoft.identity.common.java.util.CachedData;
 import com.microsoft.identity.common.java.util.FileUtil;
 import com.microsoft.identity.common.logging.Logger;
@@ -45,6 +51,7 @@
 import java.security.KeyPairGenerator;
 import java.security.KeyStore;
 import java.security.spec.AlgorithmParameterSpec;
+import java.security.ProviderException;
 import java.util.Calendar;
 import java.util.Date;
 import java.util.Locale;
@@ -55,6 +62,9 @@
 
 import edu.umd.cs.findbugs.annotations.Nullable;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.api.trace.StatusCode;
+import io.opentelemetry.context.Scope;
 import lombok.NonNull;
 
 /**
@@ -249,17 +259,83 @@ private void saveSecretKeyToStorage(@NonNull final SecretKey unencryptedKey) thr
          * stomping/overwriting one another's keypair.
          */
         KeyPair keyPair = AndroidKeyStoreUtil.readKey(mAlias);
-        if(keyPair == null){
+        if (keyPair == null) {
             Logger.info(methodTag, "No existing keypair. Generating a new one.");
-            keyPair = AndroidKeyStoreUtil.generateKeyPair(
-                    WRAP_KEY_ALGORITHM,
-                    getSpecForKeyStoreKey(mContext, mAlias));
+            final Span span = OTelUtility.createSpan(SpanName.KeyPairGeneration.name());
+            final long keypairGenStartTime = System.currentTimeMillis();
+            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P && CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP)) {
+                try (final Scope scope = SpanExtension.makeCurrentSpan(span)) {
+                    keyPair = attemptKeyPairGeneration(mAlias, true, keypairGenStartTime);
+                    Logger.info(methodTag, "Successfully generated keypair with new KeyPairGeneratorSpec with wrap purpose.");
+                    span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "new_key_gen_spec_with_wrap");
+                    span.setStatus(StatusCode.OK);
+                } catch (final ProviderException e) {
+                    if ("SecureKeyImportUnavailableException".equals(e.getClass().getSimpleName())) {
+                        Logger.warn(methodTag, "Wrap purpose may not be supported. Retrying without wrap.");
+                        try {
+                            keyPair = attemptKeyPairGeneration(mAlias, false, keypairGenStartTime);
+                            Logger.info(methodTag, "Successfully generated keypair with new KeyPairGeneratorSpec without wrap purpose.");
+                            span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "new_key_gen_spec_without_wrap");
+                            span.setStatus(StatusCode.OK);
+                        } catch (final Exception ex) {
+                            // 2nd fallback to legacy keygen spec
+                            Logger.error(methodTag, "Second attempt without wrap also failed. Falling back to legacy spec.", ex);
+                            keyPair = generateKeyPairWithLegacySpec(mAlias, keypairGenStartTime);
+                            if (e.getMessage() != null) {
+                                span.setAttribute(AttributeName.keypair_gen_exception.name(), e.getMessage());
+                            }
+                            span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "legacy_key_gen_spec");
+                            span.setStatus(StatusCode.OK);
+                        }
+                    } else {
+                        Logger.info(methodTag, "Some unknown exception occurred. Running legacy keygen spec logic.");
+                        keyPair = generateKeyPairWithLegacySpec(mAlias, keypairGenStartTime);
+                        span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "legacy_key_gen_spec");
+                        span.setStatus(StatusCode.OK);
+                    }
+                } catch (final Exception e) {
+                    Logger.warn(methodTag, "Unexpected error with new KeyPairGeneratorSpec. Falling back to legacy spec. "+ e);
+                    keyPair = generateKeyPairWithLegacySpec(mAlias, keypairGenStartTime);
+                    if (e.getMessage() != null) {
+                        span.setAttribute(AttributeName.keypair_gen_exception.name(), e.getMessage());
+                    }
+                    span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "legacy_key_gen_spec");
+                    span.setStatus(StatusCode.OK);
+                } finally {
+                    span.end();
+                }
+            }
+            else {
+                // If flight for using new keygen spec is not enabled, use the legacy spec.
+                Logger.info(methodTag, "Using legacy spec for keypair generation directly.");
+                keyPair = generateKeyPairWithLegacySpec(mAlias, keypairGenStartTime);
+            }
         }
 
         final byte[] keyWrapped = AndroidKeyStoreUtil.wrap(unencryptedKey, keyPair, WRAP_ALGORITHM);
         FileUtil.writeDataToFile(keyWrapped, getKeyFile());
     }
 
+    @RequiresApi(api = Build.VERSION_CODES.P)
+    private KeyPair attemptKeyPairGeneration(@NonNull final String alias, boolean useWrapPurpose, long keypairGenStartTime) throws ClientException{
+        KeyPair keyPair = AndroidKeyStoreUtil.generateKeyPair(
+                WRAP_KEY_ALGORITHM, getSpecForKeyStoreKey(alias, useWrapPurpose));
+        recordKeyGenerationTime(keypairGenStartTime);
+        return keyPair;
+    }
+
+    private KeyPair generateKeyPairWithLegacySpec(@NonNull final String alias, long keypairGenStartTime) throws ClientException{
+        KeyPair keyPair = AndroidKeyStoreUtil.generateKeyPair(
+                WRAP_KEY_ALGORITHM, getLegacySpecForKeyStoreKey(mContext, alias));
+        recordKeyGenerationTime(keypairGenStartTime);
+        return keyPair;
+    }
+
+    private void recordKeyGenerationTime(long keypairGenStartTime) {
+        long elapsedTime = System.currentTimeMillis() - keypairGenStartTime;
+        SpanExtension.current().setAttribute(AttributeName.elapsed_time_keypair_generation.name(), elapsedTime);
+    }
+
     /**
      * Wipe all the data associated from this key.
      */
@@ -304,28 +380,21 @@ private static AlgorithmParameterSpec getLegacySpecForKeyStoreKey(@NonNull final
      * Generate a self-signed cert and derive an AlgorithmParameterSpec from that.
      * This is for the key to be generated in {@link KeyStore} via {@link KeyPairGenerator}
      *
-     * @param context an Android {@link Context} object.
+     * @param alias   the alias for the key.
+     * @param tryPurposeWrap whether to try to use the wrap purpose in the key generation spec.
      * @return a {@link AlgorithmParameterSpec} for the keystore key (that we'll use to wrap the secret key).
      */
-    private static AlgorithmParameterSpec getSpecForKeyStoreKey(@NonNull final Context context, @NonNull final String alias) {
-        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) {
-            return getLegacySpecForKeyStoreKey(context, alias);
-        } else {
-            final String certInfo = String.format(Locale.ROOT, "CN=%s, OU=%s",
-                    alias,
-                    context.getPackageName());
-            final int certValidYears = 100;
-            int purposes = KeyProperties.PURPOSE_WRAP_KEY | KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT;
-            return new KeyGenParameterSpec.Builder(alias, purposes)
-                    .setCertificateSubject(new X500Principal(certInfo))
-                    .setCertificateSerialNumber(BigInteger.ONE)
-                    .setCertificateNotBefore(new Date())
-                    .setCertificateNotAfter(new Date(System.currentTimeMillis() + TimeUnit.DAYS.toMillis(365 * certValidYears)))
-                    .setKeySize(2048)
-                    .setDigests(KeyProperties.DIGEST_SHA256, KeyProperties.DIGEST_SHA512)
-                    .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_PKCS1)
-                    .build();
+    @RequiresApi(api = Build.VERSION_CODES.P)
+    private static AlgorithmParameterSpec getSpecForKeyStoreKey(@NonNull final String alias, boolean tryPurposeWrap) {
+        int purposes = KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT;
+        if (tryPurposeWrap) {
+            purposes |= KeyProperties.PURPOSE_WRAP_KEY;
         }
+        return new KeyGenParameterSpec.Builder(alias, purposes)
+                .setKeySize(2048)
+                .setDigests(KeyProperties.DIGEST_SHA256, KeyProperties.DIGEST_SHA512)
+                .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_PKCS1)
+                .build();
     }
 
     /**

diff --git a/common4j/src/main/com/microsoft/identity/common/java/flighting/CommonFlight.java b/common4j/src/main/com/microsoft/identity/common/java/flighting/CommonFlight.java
@@ -88,7 +88,12 @@ public enum CommonFlight implements IFlightConfig {
     /**
      * Flight to enable the re-attachment of new PRT header logic. Default is true.
      */
-    ENABLE_ATTACH_NEW_PRT_HEADER_WHEN_NONCE_EXPIRED("EnableAttachNewPrtHeaderWhenNonceExpired", true);
+    ENABLE_ATTACH_NEW_PRT_HEADER_WHEN_NONCE_EXPIRED("EnableAttachNewPrtHeaderWhenNonceExpired", true),
+
+    /**
+     * Flight to enable the new key generation spec for wrap key. Default is true.
+     */
+    ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP("EnableNewKeyGenSpecForWrap", true);
     private String key;
     private Object defaultValue;
     CommonFlight(@NonNull String key, @NonNull Object defaultValue) {

diff --git a/common4j/src/main/com/microsoft/identity/common/java/opentelemetry/AttributeName.java b/common4j/src/main/com/microsoft/identity/common/java/opentelemetry/AttributeName.java
@@ -312,5 +312,20 @@ public enum AttributeName {
     /**
      * Indicates the new refresh token credential header attached in the eSTS request.
      */
-    is_new_refresh_token_cred_header_attached
+    is_new_refresh_token_cred_header_attached,
+
+    /**
+     * The time (in milliseconds) spent on generating a keypair.
+     */
+    elapsed_time_keypair_generation,
+
+    /**
+     * Indicates the successful method used to generate a keypair.
+     */
+    key_pair_gen_successful_method,
+
+    /**
+     * Indicates the exception in generating a keypair.
+     */
+    keypair_gen_exception
 }
diff --git a/common4j/src/main/com/microsoft/identity/common/java/opentelemetry/SpanName.java b/common4j/src/main/com/microsoft/identity/common/java/opentelemetry/SpanName.java
@@ -59,5 +59,6 @@ public enum SpanName {
     UpgradeDeviceRegistration,
     RemoveBrokerAccount,
     ProcessNonceFromEstsRedirect,
-    DataStoreCorruptionException
+    DataStoreCorruptionException,
+    KeyPairGeneration
 }
diff --git a/common4j/versioning/version.properties b/common4j/versioning/version.properties
@@ -1,4 +1,4 @@
 #Wed May 12 20:08:39 UTC 2021
-versionName=16.0.1
+versionName=16.0.2
 versionCode=1
 latestPatchVersion=227
diff --git a/versioning/version.properties b/versioning/version.properties
@@ -1,4 +1,4 @@
 #Tue Apr 06 22:55:08 UTC 2021
-versionName=19.0.1
+versionName=19.0.2
 versionCode=1
 latestPatchVersion=234