diff --git a/changelog.txt b/changelog.txt index f22258463f..77c4c4f5ca 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,5 +1,6 @@ vNext ---------- +- [PATCH] Allow generating wrapping key without PURPOSE_WRAP_KEY with Flight (#2633)) - [MINOR] Adding a state parameter to prevent phish attacks, in “switch_browser” flow. (#2631) - [MINOR] Move camera logic to CameraPermissionRequestHandler and add SdmQrPinManager (#2630) - [MINOR] Pass Work Profile existence, OS Version, and Manufacturer to ESTS (#2627) 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 index 63fa6fe2aa..6e4495fdbd 100644 --- a/common/src/main/java/com/microsoft/identity/common/crypto/AndroidWrappedKeyLoader.java +++ b/common/src/main/java/com/microsoft/identity/common/crypto/AndroidWrappedKeyLoader.java @@ -22,7 +22,6 @@ // THE SOFTWARE. package com.microsoft.identity.common.crypto; -import android.annotation.TargetApi; import android.content.Context; import android.os.Build; import android.security.KeyPairGeneratorSpec; @@ -32,6 +31,7 @@ import androidx.annotation.RequiresApi; 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.KeyUtil; import com.microsoft.identity.common.java.exception.ClientException; @@ -43,6 +43,7 @@ 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.java.util.StringUtil; import com.microsoft.identity.common.logging.Logger; import java.io.File; @@ -50,7 +51,6 @@ import java.security.KeyPair; import java.security.KeyPairGenerator; import java.security.KeyStore; -import java.security.ProviderException; import java.security.spec.AlgorithmParameterSpec; import java.util.Calendar; import java.util.Locale; @@ -258,83 +258,153 @@ private void saveSecretKeyToStorage(@NonNull final SecretKey unencryptedKey) thr KeyPair keyPair = AndroidKeyStoreUtil.readKey(mAlias); if (keyPair == null) { Logger.info(methodTag, "No existing keypair. Generating a new one."); - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P && CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP)) { - final long keypairGenStartTime = System.currentTimeMillis(); - final Span span = OTelUtility.createSpanFromParent(SpanName.KeyPairGeneration.name(), SpanExtension.current().getSpanContext()); - 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.warn(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.warn(methodTag, "Some unknown exception occurred. Running legacy keygen spec logic."+ e); - keyPair = generateKeyPairWithLegacySpec(mAlias, keypairGenStartTime); - span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "legacy_key_gen_spec"); - span.setStatus(StatusCode.OK); - } - } catch (final Throwable throwable) { - Logger.warn(methodTag, "Unexpected error with new KeyPairGeneratorSpec. Falling back to legacy spec. "+ throwable); - keyPair = generateKeyPairWithLegacySpec(mAlias, keypairGenStartTime); - if (throwable.getMessage() != null) { - span.setAttribute(AttributeName.keypair_gen_exception.name(), throwable.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 = AndroidKeyStoreUtil.generateKeyPair( - WRAP_KEY_ALGORITHM, getLegacySpecForKeyStoreKey(mContext, mAlias)); + final Span span = OTelUtility.createSpanFromParent(SpanName.KeyPairGeneration.name(), SpanExtension.current().getSpanContext()); + try (final Scope scope = SpanExtension.makeCurrentSpan(span)) { + keyPair = generateNewKeyPair(); + span.setStatus(StatusCode.OK); + } catch (final ClientException e) { + span.setStatus(StatusCode.ERROR); + span.recordException(e); + throw e; + } finally { + span.end(); } } 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; + /** + * Generate a new key pair wrapping key, based on API level uses different spec to generate + * the key pair. + * @return a key pair + */ + @NonNull + private KeyPair generateNewKeyPair() throws ClientException { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { + return generateNewKeyPairAPI28AndAbove(); + } else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + return generateNewKeyPairAPI23AndAbove(); + } else { + return generateKeyPairWithLegacySpec(); + } } - private KeyPair generateKeyPairWithLegacySpec(@NonNull final String alias, long keypairGenStartTime) throws ClientException{ + /** + * Call this for API level >= 28. Starting level API 28 PURPOSE_WRAP_KEY is added. Based on flights + * this method may or may not use the PURPOSE_WRAP_KEY along with PURPOSE_ENCRYPT and PURPOSE_DECRYPT. The logic + * if (wrap key flight enabled) use all three purposes + * else if (new key gen flight enabled) use only encrypt and decrypt purposes + * else use legacy spec. + * @return key pair + */ + @RequiresApi(Build.VERSION_CODES.P) + @NonNull + private KeyPair generateNewKeyPairAPI28AndAbove() throws ClientException { + if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP_WITH_PURPOSE_WRAP_KEY)) { + return generateWrappingKeyPair_WithPurposeWrapKey(); + } else if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP_WITHOUT_PURPOSE_WRAP_KEY)) { + return generateWrappingKeyPair(); + } else { + return generateKeyPairWithLegacySpec(); + } + } + + /** + * Call this for API level >= 23. Based on flight new key gen spec is used else legacy which + * is deprecated starting API 23. + * @return key pair + */ + @RequiresApi(Build.VERSION_CODES.M) + @NonNull + private KeyPair generateNewKeyPairAPI23AndAbove() throws ClientException { + if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP_WITHOUT_PURPOSE_WRAP_KEY)) { + return generateWrappingKeyPair(); + } else { + return generateKeyPairWithLegacySpec(); + } + } + + /** + * 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 + */ + @NonNull + private KeyPair generateKeyPairWithLegacySpec() throws ClientException{ + final Span span = SpanExtension.current(); try { - final KeyPair keyPair = AndroidKeyStoreUtil.generateKeyPair( - WRAP_KEY_ALGORITHM, getLegacySpecForKeyStoreKey(mContext, alias)); - recordKeyGenerationTime(keypairGenStartTime); + final AlgorithmParameterSpec keyPairGenSpec = getLegacySpecForKeyStoreKey(); + final KeyPair keyPair = attemptKeyPairGeneration(keyPairGenSpec); + span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "legacy_key_gen_spec"); return keyPair; - } catch (final ClientException e) { - SpanExtension.current().recordException(e); - SpanExtension.current().setStatus(StatusCode.ERROR); + } catch (final Throwable e) { Logger.error(TAG + ":generateKeyPairWithLegacySpec", "Error generating keypair with legacy spec.", e); - throw e; + throw ExceptionAdapter.clientExceptionFromException(e); } } + /** + * Generate a new key pair wrapping key, based on API level >= 28. This method uses new key gen spec + * with PURPOSE_WRAP_KEY. If this fails, it will fallback to generateWrappingKeyPair() which does not use + * PURPOSE_WRAP_KEY (still uses new key gen spec). + */ + @RequiresApi(Build.VERSION_CODES.P) + private KeyPair generateWrappingKeyPair_WithPurposeWrapKey() throws ClientException { + final String methodTag = TAG + ":generateWrappingKeyPair_WithPurposeWrapKey"; + final Span span = SpanExtension.current(); + try { + Logger.info(methodTag, "Generating new keypair with new spec with purpose_wrap_key"); + int purposes = KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT | KeyProperties.PURPOSE_WRAP_KEY; + final AlgorithmParameterSpec keyPairGenSpec = getSpecForWrappingKey(purposes); + final KeyPair keyPair = attemptKeyPairGeneration(keyPairGenSpec); + span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "new_key_gen_spec_with_wrap"); + return keyPair; + } catch (final Throwable e) { + Logger.error(methodTag, "Error generating keypair with new spec with purpose_wrap_key." + + "Attempting without purpose_wrap_key." , e); + if (!StringUtil.isNullOrEmpty(e.getMessage())) { + span.setAttribute(AttributeName.keypair_gen_exception.name(), e.getMessage()); + } + return generateWrappingKeyPair(); + } + } + + /** + * Generate a new key pair wrapping key, based on API level >= 23. This method uses new key gen spec + * with purposes PURPOSE_ENCRYPT and PURPOSE_DECRYPT. If this fails, it will fallback to generateKeyPairWithLegacySpec() + * which uses olg key gen spec. + */ + @RequiresApi(Build.VERSION_CODES.M) + private KeyPair generateWrappingKeyPair() throws ClientException { + final String methodTag = TAG + ":generateWrappingKeyPair"; + final Span span = SpanExtension.current(); + try { + Logger.info(methodTag, "Generating new keypair with new spec without wrap key"); + int purposes = KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT; + final AlgorithmParameterSpec keyPairGenSpec = getSpecForWrappingKey(purposes); + final KeyPair keyPair = attemptKeyPairGeneration(keyPairGenSpec); + span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "new_key_gen_spec_without_wrap"); + return keyPair; + } catch (final Throwable e) { + Logger.error(methodTag, "Error generating keypair with new spec." + + "Attempting with legacy spec.", e); + if (!StringUtil.isNullOrEmpty(e.getMessage())) { + span.setAttribute(AttributeName.keypair_gen_exception.name(), e.getMessage()); + } + return generateKeyPairWithLegacySpec(); + } + } + + private KeyPair attemptKeyPairGeneration(@NonNull final AlgorithmParameterSpec keyPairGenSpec) throws ClientException{ + final long keypairGenStartTime = System.currentTimeMillis(); + final KeyPair keyPair = AndroidKeyStoreUtil.generateKeyPair( + WRAP_KEY_ALGORITHM, keyPairGenSpec); + recordKeyGenerationTime(keypairGenStartTime); + return keyPair; + } + private void recordKeyGenerationTime(long keypairGenStartTime) { long elapsedTime = System.currentTimeMillis() - keypairGenStartTime; SpanExtension.current().setAttribute(AttributeName.elapsed_time_keypair_generation.name(), elapsedTime); @@ -353,25 +423,23 @@ public void deleteSecretKeyFromStorage() throws ClientException { /** * 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} - * Note : This is now only for API level < 28 - * - * @param context an Android {@link Context} object. + * Note : This is now only for API level < 23 or as fallback. + * @return a {@link AlgorithmParameterSpec} for the keystore key (that we'll use to wrap the secret key). */ - private static AlgorithmParameterSpec getLegacySpecForKeyStoreKey(@NonNull final Context context, - @NonNull final String alias) { + private AlgorithmParameterSpec getLegacySpecForKeyStoreKey() { // Generate a self-signed cert. final String certInfo = String.format(Locale.ROOT, "CN=%s, OU=%s", - alias, - context.getPackageName()); + mAlias, + mContext.getPackageName()); final Calendar start = Calendar.getInstance(); final Calendar end = Calendar.getInstance(); final int certValidYears = 100; end.add(Calendar.YEAR, certValidYears); - return new KeyPairGeneratorSpec.Builder(context) - .setAlias(alias) + return new KeyPairGeneratorSpec.Builder(mContext) + .setAlias(mAlias) .setSubject(new X500Principal(certInfo)) .setSerialNumber(BigInteger.ONE) .setStartDate(start.getTime()) @@ -379,21 +447,9 @@ private static AlgorithmParameterSpec getLegacySpecForKeyStoreKey(@NonNull final .build(); } - /** - * 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 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). - */ - @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) + @RequiresApi(api = Build.VERSION_CODES.M) + private AlgorithmParameterSpec getSpecForWrappingKey(int purposes) { + return new KeyGenParameterSpec.Builder(mAlias, purposes) .setKeySize(2048) .setDigests(KeyProperties.DIGEST_SHA256, KeyProperties.DIGEST_SHA512) .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_PKCS1) 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 index 5d730778a7..9239d653c5 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/flighting/CommonFlight.java +++ b/common4j/src/main/com/microsoft/identity/common/java/flighting/CommonFlight.java @@ -91,9 +91,10 @@ public enum CommonFlight implements IFlightConfig { ENABLE_ATTACH_NEW_PRT_HEADER_WHEN_NONCE_EXPIRED("EnableAttachNewPrtHeaderWhenNonceExpired", true), /** - * Flight to enable the new key generation spec for wrap key. Default is true. + * Flight to enable the new key generation spec for wrap key using PURPOSE_WRAP_KEY in key gen spec. Default is true. + * This is applicable for API >= 28 */ - ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP("EnableNewKeyGenSpecForWrap", true), + ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP_WITH_PURPOSE_WRAP_KEY("EnableNewKeyGenSpecForWrapWithPurposeWrapKey", true), /** * Flight to enable the attachment of PRT header in cross cloud requests. Default is true. @@ -108,7 +109,12 @@ public enum CommonFlight implements IFlightConfig { /** * Flight to enable adding x-client-MN and x-client-WPAvailable extra query parameters */ - ENABLE_AM_API_WORKPROFILE_EXTRA_QUERY_PARAMETERS("EnableAmApiWorkProfileExtraQueryParameters", false); + ENABLE_AM_API_WORKPROFILE_EXTRA_QUERY_PARAMETERS("EnableAmApiWorkProfileExtraQueryParameters", false), + + /** Flight to enable the new key generation without PURPOSE_WRAP_KEY. Default is true. + * This is applicable for API >= 23 + */ + ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP_WITHOUT_PURPOSE_WRAP_KEY("EnableNewKeyGenSpecForWrapWithoutPurposeWrapKey", true); private String key; private Object defaultValue;