Skip to content

Commit f9a7f3f

Browse files
fadidurahfadidurah
authored
Merge branch 'dev' into fadi/fluentMSA
2 parents 1511c79 + 346bae9 commit f9a7f3f

3 files changed

Lines changed: 155 additions & 92 deletions

File tree

‎changelog.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
vNext
22
----------
3+
- [PATCH] Allow generating wrapping key without PURPOSE_WRAP_KEY with Flight (#2633))
34
- [MINOR] Adding a state parameter to prevent phish attacks, in “switch_browser” flow. (#2631)
45
- [MINOR] Move camera logic to CameraPermissionRequestHandler and add SdmQrPinManager (#2630)
56
- [MINOR] Pass Work Profile existence, OS Version, and Manufacturer to ESTS (#2627)

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

Lines changed: 145 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
// THE SOFTWARE.
2323
package com.microsoft.identity.common.crypto;
2424

25-
import android.annotation.TargetApi;
2625
import android.content.Context;
2726
import android.os.Build;
2827
import android.security.KeyPairGeneratorSpec;
@@ -32,6 +31,7 @@
3231
import androidx.annotation.RequiresApi;
3332

3433
import com.microsoft.identity.common.internal.util.AndroidKeyStoreUtil;
34+
import com.microsoft.identity.common.java.controllers.ExceptionAdapter;
3535
import com.microsoft.identity.common.java.crypto.key.AES256KeyLoader;
3636
import com.microsoft.identity.common.java.crypto.key.KeyUtil;
3737
import com.microsoft.identity.common.java.exception.ClientException;
@@ -43,14 +43,14 @@
4343
import com.microsoft.identity.common.java.opentelemetry.SpanName;
4444
import com.microsoft.identity.common.java.util.CachedData;
4545
import com.microsoft.identity.common.java.util.FileUtil;
46+
import com.microsoft.identity.common.java.util.StringUtil;
4647
import com.microsoft.identity.common.logging.Logger;
4748

4849
import java.io.File;
4950
import java.math.BigInteger;
5051
import java.security.KeyPair;
5152
import java.security.KeyPairGenerator;
5253
import java.security.KeyStore;
53-
import java.security.ProviderException;
5454
import java.security.spec.AlgorithmParameterSpec;
5555
import java.util.Calendar;
5656
import java.util.Locale;
@@ -258,83 +258,153 @@ private void saveSecretKeyToStorage(@NonNull final SecretKey unencryptedKey) thr
258258
KeyPair keyPair = AndroidKeyStoreUtil.readKey(mAlias);
259259
if (keyPair == null) {
260260
Logger.info(methodTag, "No existing keypair. Generating a new one.");
261-
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P && CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP)) {
262-
final long keypairGenStartTime = System.currentTimeMillis();
263-
final Span span = OTelUtility.createSpanFromParent(SpanName.KeyPairGeneration.name(), SpanExtension.current().getSpanContext());
264-
try (final Scope scope = SpanExtension.makeCurrentSpan(span)) {
265-
keyPair = attemptKeyPairGeneration(mAlias, true, keypairGenStartTime);
266-
Logger.info(methodTag, "Successfully generated keypair with new KeyPairGeneratorSpec with wrap purpose.");
267-
span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "new_key_gen_spec_with_wrap");
268-
span.setStatus(StatusCode.OK);
269-
} catch (final ProviderException e) {
270-
if ("SecureKeyImportUnavailableException".equals(e.getClass().getSimpleName())) {
271-
Logger.warn(methodTag, "Wrap purpose may not be supported. Retrying without wrap.");
272-
try {
273-
keyPair = attemptKeyPairGeneration(mAlias, false, keypairGenStartTime);
274-
Logger.info(methodTag, "Successfully generated keypair with new KeyPairGeneratorSpec without wrap purpose.");
275-
span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "new_key_gen_spec_without_wrap");
276-
span.setStatus(StatusCode.OK);
277-
} catch (final Exception ex) {
278-
// 2nd fallback to legacy keygen spec
279-
Logger.warn(methodTag, "Second attempt without wrap also failed. Falling back to legacy spec."+ ex);
280-
keyPair = generateKeyPairWithLegacySpec(mAlias, keypairGenStartTime);
281-
if (e.getMessage() != null) {
282-
span.setAttribute(AttributeName.keypair_gen_exception.name(), e.getMessage());
283-
}
284-
span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "legacy_key_gen_spec");
285-
span.setStatus(StatusCode.OK);
286-
}
287-
} else {
288-
Logger.warn(methodTag, "Some unknown exception occurred. Running legacy keygen spec logic."+ e);
289-
keyPair = generateKeyPairWithLegacySpec(mAlias, keypairGenStartTime);
290-
span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "legacy_key_gen_spec");
291-
span.setStatus(StatusCode.OK);
292-
}
293-
} catch (final Throwable throwable) {
294-
Logger.warn(methodTag, "Unexpected error with new KeyPairGeneratorSpec. Falling back to legacy spec. "+ throwable);
295-
keyPair = generateKeyPairWithLegacySpec(mAlias, keypairGenStartTime);
296-
if (throwable.getMessage() != null) {
297-
span.setAttribute(AttributeName.keypair_gen_exception.name(), throwable.getMessage());
298-
}
299-
span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "legacy_key_gen_spec");
300-
span.setStatus(StatusCode.OK);
301-
} finally {
302-
span.end();
303-
}
304-
}
305-
else {
306-
// If flight for using new keygen spec is not enabled, use the legacy spec.
307-
Logger.info(methodTag, "Using legacy spec for keypair generation directly.");
308-
keyPair = AndroidKeyStoreUtil.generateKeyPair(
309-
WRAP_KEY_ALGORITHM, getLegacySpecForKeyStoreKey(mContext, mAlias));
261+
final Span span = OTelUtility.createSpanFromParent(SpanName.KeyPairGeneration.name(), SpanExtension.current().getSpanContext());
262+
try (final Scope scope = SpanExtension.makeCurrentSpan(span)) {
263+
keyPair = generateNewKeyPair();
264+
span.setStatus(StatusCode.OK);
265+
} catch (final ClientException e) {
266+
span.setStatus(StatusCode.ERROR);
267+
span.recordException(e);
268+
throw e;
269+
} finally {
270+
span.end();
310271
}
311272
}
312273
final byte[] keyWrapped = AndroidKeyStoreUtil.wrap(unencryptedKey, keyPair, WRAP_ALGORITHM);
313274
FileUtil.writeDataToFile(keyWrapped, getKeyFile());
314275
}
315276

316-
@RequiresApi(api = Build.VERSION_CODES.P)
317-
private KeyPair attemptKeyPairGeneration(@NonNull final String alias, boolean useWrapPurpose, long keypairGenStartTime) throws ClientException{
318-
KeyPair keyPair = AndroidKeyStoreUtil.generateKeyPair(
319-
WRAP_KEY_ALGORITHM, getSpecForKeyStoreKey(alias, useWrapPurpose));
320-
recordKeyGenerationTime(keypairGenStartTime);
321-
return keyPair;
277+
/**
278+
* Generate a new key pair wrapping key, based on API level uses different spec to generate
279+
* the key pair.
280+
* @return a key pair
281+
*/
282+
@NonNull
283+
private KeyPair generateNewKeyPair() throws ClientException {
284+
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
285+
return generateNewKeyPairAPI28AndAbove();
286+
} else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
287+
return generateNewKeyPairAPI23AndAbove();
288+
} else {
289+
return generateKeyPairWithLegacySpec();
290+
}
322291
}
323292

324-
private KeyPair generateKeyPairWithLegacySpec(@NonNull final String alias, long keypairGenStartTime) throws ClientException{
293+
/**
294+
* Call this for API level >= 28. Starting level API 28 PURPOSE_WRAP_KEY is added. Based on flights
295+
* this method may or may not use the PURPOSE_WRAP_KEY along with PURPOSE_ENCRYPT and PURPOSE_DECRYPT. The logic
296+
* if (wrap key flight enabled) use all three purposes
297+
* else if (new key gen flight enabled) use only encrypt and decrypt purposes
298+
* else use legacy spec.
299+
* @return key pair
300+
*/
301+
@RequiresApi(Build.VERSION_CODES.P)
302+
@NonNull
303+
private KeyPair generateNewKeyPairAPI28AndAbove() throws ClientException {
304+
if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP_WITH_PURPOSE_WRAP_KEY)) {
305+
return generateWrappingKeyPair_WithPurposeWrapKey();
306+
} else if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP_WITHOUT_PURPOSE_WRAP_KEY)) {
307+
return generateWrappingKeyPair();
308+
} else {
309+
return generateKeyPairWithLegacySpec();
310+
}
311+
}
312+
313+
/**
314+
* Call this for API level >= 23. Based on flight new key gen spec is used else legacy which
315+
* is deprecated starting API 23.
316+
* @return key pair
317+
*/
318+
@RequiresApi(Build.VERSION_CODES.M)
319+
@NonNull
320+
private KeyPair generateNewKeyPairAPI23AndAbove() throws ClientException {
321+
if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP_WITHOUT_PURPOSE_WRAP_KEY)) {
322+
return generateWrappingKeyPair();
323+
} else {
324+
return generateKeyPairWithLegacySpec();
325+
}
326+
}
327+
328+
/**
329+
* Generate a new key pair wrapping key based on legacy logic. Call this for API < 23 or as fallback
330+
* until new key gen specs are stable.
331+
* @return key pair generated with legacy spec
332+
* @throws ClientException
333+
*/
334+
@NonNull
335+
private KeyPair generateKeyPairWithLegacySpec() throws ClientException{
336+
final Span span = SpanExtension.current();
325337
try {
326-
final KeyPair keyPair = AndroidKeyStoreUtil.generateKeyPair(
327-
WRAP_KEY_ALGORITHM, getLegacySpecForKeyStoreKey(mContext, alias));
328-
recordKeyGenerationTime(keypairGenStartTime);
338+
final AlgorithmParameterSpec keyPairGenSpec = getLegacySpecForKeyStoreKey();
339+
final KeyPair keyPair = attemptKeyPairGeneration(keyPairGenSpec);
340+
span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "legacy_key_gen_spec");
329341
return keyPair;
330-
} catch (final ClientException e) {
331-
SpanExtension.current().recordException(e);
332-
SpanExtension.current().setStatus(StatusCode.ERROR);
342+
} catch (final Throwable e) {
333343
Logger.error(TAG + ":generateKeyPairWithLegacySpec", "Error generating keypair with legacy spec.", e);
334-
throw e;
344+
throw ExceptionAdapter.clientExceptionFromException(e);
335345
}
336346
}
337347

348+
/**
349+
* Generate a new key pair wrapping key, based on API level >= 28. This method uses new key gen spec
350+
* with PURPOSE_WRAP_KEY. If this fails, it will fallback to generateWrappingKeyPair() which does not use
351+
* PURPOSE_WRAP_KEY (still uses new key gen spec).
352+
*/
353+
@RequiresApi(Build.VERSION_CODES.P)
354+
private KeyPair generateWrappingKeyPair_WithPurposeWrapKey() throws ClientException {
355+
final String methodTag = TAG + ":generateWrappingKeyPair_WithPurposeWrapKey";
356+
final Span span = SpanExtension.current();
357+
try {
358+
Logger.info(methodTag, "Generating new keypair with new spec with purpose_wrap_key");
359+
int purposes = KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT | KeyProperties.PURPOSE_WRAP_KEY;
360+
final AlgorithmParameterSpec keyPairGenSpec = getSpecForWrappingKey(purposes);
361+
final KeyPair keyPair = attemptKeyPairGeneration(keyPairGenSpec);
362+
span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "new_key_gen_spec_with_wrap");
363+
return keyPair;
364+
} catch (final Throwable e) {
365+
Logger.error(methodTag, "Error generating keypair with new spec with purpose_wrap_key." +
366+
"Attempting without purpose_wrap_key." , e);
367+
if (!StringUtil.isNullOrEmpty(e.getMessage())) {
368+
span.setAttribute(AttributeName.keypair_gen_exception.name(), e.getMessage());
369+
}
370+
return generateWrappingKeyPair();
371+
}
372+
}
373+
374+
/**
375+
* Generate a new key pair wrapping key, based on API level >= 23. This method uses new key gen spec
376+
* with purposes PURPOSE_ENCRYPT and PURPOSE_DECRYPT. If this fails, it will fallback to generateKeyPairWithLegacySpec()
377+
* which uses olg key gen spec.
378+
*/
379+
@RequiresApi(Build.VERSION_CODES.M)
380+
private KeyPair generateWrappingKeyPair() throws ClientException {
381+
final String methodTag = TAG + ":generateWrappingKeyPair";
382+
final Span span = SpanExtension.current();
383+
try {
384+
Logger.info(methodTag, "Generating new keypair with new spec without wrap key");
385+
int purposes = KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT;
386+
final AlgorithmParameterSpec keyPairGenSpec = getSpecForWrappingKey(purposes);
387+
final KeyPair keyPair = attemptKeyPairGeneration(keyPairGenSpec);
388+
span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "new_key_gen_spec_without_wrap");
389+
return keyPair;
390+
} catch (final Throwable e) {
391+
Logger.error(methodTag, "Error generating keypair with new spec." +
392+
"Attempting with legacy spec.", e);
393+
if (!StringUtil.isNullOrEmpty(e.getMessage())) {
394+
span.setAttribute(AttributeName.keypair_gen_exception.name(), e.getMessage());
395+
}
396+
return generateKeyPairWithLegacySpec();
397+
}
398+
}
399+
400+
private KeyPair attemptKeyPairGeneration(@NonNull final AlgorithmParameterSpec keyPairGenSpec) throws ClientException{
401+
final long keypairGenStartTime = System.currentTimeMillis();
402+
final KeyPair keyPair = AndroidKeyStoreUtil.generateKeyPair(
403+
WRAP_KEY_ALGORITHM, keyPairGenSpec);
404+
recordKeyGenerationTime(keypairGenStartTime);
405+
return keyPair;
406+
}
407+
338408
private void recordKeyGenerationTime(long keypairGenStartTime) {
339409
long elapsedTime = System.currentTimeMillis() - keypairGenStartTime;
340410
SpanExtension.current().setAttribute(AttributeName.elapsed_time_keypair_generation.name(), elapsedTime);
@@ -353,47 +423,33 @@ public void deleteSecretKeyFromStorage() throws ClientException {
353423
/**
354424
* Generate a self-signed cert and derive an AlgorithmParameterSpec from that.
355425
* This is for the key to be generated in {@link KeyStore} via {@link KeyPairGenerator}
356-
* Note : This is now only for API level < 28
357-
*
358-
* @param context an Android {@link Context} object.
426+
* Note : This is now only for API level < 23 or as fallback.
427+
359428
* @return a {@link AlgorithmParameterSpec} for the keystore key (that we'll use to wrap the secret key).
360429
*/
361-
private static AlgorithmParameterSpec getLegacySpecForKeyStoreKey(@NonNull final Context context,
362-
@NonNull final String alias) {
430+
private AlgorithmParameterSpec getLegacySpecForKeyStoreKey() {
363431
// Generate a self-signed cert.
364432
final String certInfo = String.format(Locale.ROOT, "CN=%s, OU=%s",
365-
alias,
366-
context.getPackageName());
433+
mAlias,
434+
mContext.getPackageName());
367435

368436
final Calendar start = Calendar.getInstance();
369437
final Calendar end = Calendar.getInstance();
370438
final int certValidYears = 100;
371439
end.add(Calendar.YEAR, certValidYears);
372440

373-
return new KeyPairGeneratorSpec.Builder(context)
374-
.setAlias(alias)
441+
return new KeyPairGeneratorSpec.Builder(mContext)
442+
.setAlias(mAlias)
375443
.setSubject(new X500Principal(certInfo))
376444
.setSerialNumber(BigInteger.ONE)
377445
.setStartDate(start.getTime())
378446
.setEndDate(end.getTime())
379447
.build();
380448
}
381449

382-
/**
383-
* Generate a self-signed cert and derive an AlgorithmParameterSpec from that.
384-
* This is for the key to be generated in {@link KeyStore} via {@link KeyPairGenerator}
385-
*
386-
* @param alias the alias for the key.
387-
* @param tryPurposeWrap whether to try to use the wrap purpose in the key generation spec.
388-
* @return a {@link AlgorithmParameterSpec} for the keystore key (that we'll use to wrap the secret key).
389-
*/
390-
@RequiresApi(api = Build.VERSION_CODES.P)
391-
private static AlgorithmParameterSpec getSpecForKeyStoreKey(@NonNull final String alias, boolean tryPurposeWrap) {
392-
int purposes = KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT;
393-
if (tryPurposeWrap) {
394-
purposes |= KeyProperties.PURPOSE_WRAP_KEY;
395-
}
396-
return new KeyGenParameterSpec.Builder(alias, purposes)
450+
@RequiresApi(api = Build.VERSION_CODES.M)
451+
private AlgorithmParameterSpec getSpecForWrappingKey(int purposes) {
452+
return new KeyGenParameterSpec.Builder(mAlias, purposes)
397453
.setKeySize(2048)
398454
.setDigests(KeyProperties.DIGEST_SHA256, KeyProperties.DIGEST_SHA512)
399455
.setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_PKCS1)

‎common4j/src/main/com/microsoft/identity/common/java/flighting/CommonFlight.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,10 @@ public enum CommonFlight implements IFlightConfig {
9191
ENABLE_ATTACH_NEW_PRT_HEADER_WHEN_NONCE_EXPIRED("EnableAttachNewPrtHeaderWhenNonceExpired", true),
9292

9393
/**
94-
* Flight to enable the new key generation spec for wrap key. Default is true.
94+
* Flight to enable the new key generation spec for wrap key using PURPOSE_WRAP_KEY in key gen spec. Default is true.
95+
* This is applicable for API >= 28
9596
*/
96-
ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP("EnableNewKeyGenSpecForWrap", true),
97+
ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP_WITH_PURPOSE_WRAP_KEY("EnableNewKeyGenSpecForWrapWithPurposeWrapKey", true),
9798

9899
/**
99100
* Flight to enable the attachment of PRT header in cross cloud requests. Default is true.
@@ -108,7 +109,12 @@ public enum CommonFlight implements IFlightConfig {
108109
/**
109110
* Flight to enable adding x-client-MN and x-client-WPAvailable extra query parameters
110111
*/
111-
ENABLE_AM_API_WORKPROFILE_EXTRA_QUERY_PARAMETERS("EnableAmApiWorkProfileExtraQueryParameters", false);
112+
ENABLE_AM_API_WORKPROFILE_EXTRA_QUERY_PARAMETERS("EnableAmApiWorkProfileExtraQueryParameters", false),
113+
114+
/** Flight to enable the new key generation without PURPOSE_WRAP_KEY. Default is true.
115+
* This is applicable for API >= 23
116+
*/
117+
ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP_WITHOUT_PURPOSE_WRAP_KEY("EnableNewKeyGenSpecForWrapWithoutPurposeWrapKey", true);
112118

113119
private String key;
114120
private Object defaultValue;

0 commit comments

Comments
 (0)