Skip to content

Commit d2e9122

Browse files
somalayasomalaya
authored
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
1 parent f94be27 commit d2e9122

8 files changed

Lines changed: 124 additions & 29 deletions

File tree

‎changelog.txt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
Version 19.0.2
2+
----------
3+
(common4j 16.0.2)
4+
- [PATCH] Add fallback mechanisms for keypair generation (#2586)
5+
16
Version 19.0.1
27
----------
38
(common4j 16.0.1)

‎common/build.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ codeCoverageReport {
3131

3232
// In dev, we want to keep the dependencies(common4j, broker4j, common) to 1.0.+ to be able to be consumed by daily dev pipeline.
3333
// In release/*, we change these to specific versions being consumed.
34-
def common4jVersion = "16.0.1"
34+
def common4jVersion = "16.0.2"
3535
if (project.hasProperty("distCommon4jVersion") && project.distCommon4jVersion != '') {
3636
common4jVersion = project.distCommon4jVersion
3737
}

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

Lines changed: 92 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,12 @@
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;
38+
import com.microsoft.identity.common.java.flighting.CommonFlight;
39+
import com.microsoft.identity.common.java.flighting.CommonFlightsManager;
40+
import com.microsoft.identity.common.java.opentelemetry.AttributeName;
41+
import com.microsoft.identity.common.java.opentelemetry.OTelUtility;
42+
import com.microsoft.identity.common.java.opentelemetry.SpanExtension;
43+
import com.microsoft.identity.common.java.opentelemetry.SpanName;
3844
import com.microsoft.identity.common.java.util.CachedData;
3945
import com.microsoft.identity.common.java.util.FileUtil;
4046
import com.microsoft.identity.common.logging.Logger;
@@ -45,6 +51,7 @@
4551
import java.security.KeyPairGenerator;
4652
import java.security.KeyStore;
4753
import java.security.spec.AlgorithmParameterSpec;
54+
import java.security.ProviderException;
4855
import java.util.Calendar;
4956
import java.util.Date;
5057
import java.util.Locale;
@@ -55,6 +62,9 @@
5562

5663
import edu.umd.cs.findbugs.annotations.Nullable;
5764
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
65+
import io.opentelemetry.api.trace.Span;
66+
import io.opentelemetry.api.trace.StatusCode;
67+
import io.opentelemetry.context.Scope;
5868
import lombok.NonNull;
5969

6070
/**
@@ -249,17 +259,83 @@ private void saveSecretKeyToStorage(@NonNull final SecretKey unencryptedKey) thr
249259
* stomping/overwriting one another's keypair.
250260
*/
251261
KeyPair keyPair = AndroidKeyStoreUtil.readKey(mAlias);
252-
if(keyPair == null){
262+
if (keyPair == null) {
253263
Logger.info(methodTag, "No existing keypair. Generating a new one.");
254-
keyPair = AndroidKeyStoreUtil.generateKeyPair(
255-
WRAP_KEY_ALGORITHM,
256-
getSpecForKeyStoreKey(mContext, mAlias));
264+
final Span span = OTelUtility.createSpan(SpanName.KeyPairGeneration.name());
265+
final long keypairGenStartTime = System.currentTimeMillis();
266+
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P && CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP)) {
267+
try (final Scope scope = SpanExtension.makeCurrentSpan(span)) {
268+
keyPair = attemptKeyPairGeneration(mAlias, true, keypairGenStartTime);
269+
Logger.info(methodTag, "Successfully generated keypair with new KeyPairGeneratorSpec with wrap purpose.");
270+
span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "new_key_gen_spec_with_wrap");
271+
span.setStatus(StatusCode.OK);
272+
} catch (final ProviderException e) {
273+
if ("SecureKeyImportUnavailableException".equals(e.getClass().getSimpleName())) {
274+
Logger.warn(methodTag, "Wrap purpose may not be supported. Retrying without wrap.");
275+
try {
276+
keyPair = attemptKeyPairGeneration(mAlias, false, keypairGenStartTime);
277+
Logger.info(methodTag, "Successfully generated keypair with new KeyPairGeneratorSpec without wrap purpose.");
278+
span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "new_key_gen_spec_without_wrap");
279+
span.setStatus(StatusCode.OK);
280+
} catch (final Exception ex) {
281+
// 2nd fallback to legacy keygen spec
282+
Logger.error(methodTag, "Second attempt without wrap also failed. Falling back to legacy spec.", ex);
283+
keyPair = generateKeyPairWithLegacySpec(mAlias, keypairGenStartTime);
284+
if (e.getMessage() != null) {
285+
span.setAttribute(AttributeName.keypair_gen_exception.name(), e.getMessage());
286+
}
287+
span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "legacy_key_gen_spec");
288+
span.setStatus(StatusCode.OK);
289+
}
290+
} else {
291+
Logger.info(methodTag, "Some unknown exception occurred. Running legacy keygen spec logic.");
292+
keyPair = generateKeyPairWithLegacySpec(mAlias, keypairGenStartTime);
293+
span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "legacy_key_gen_spec");
294+
span.setStatus(StatusCode.OK);
295+
}
296+
} catch (final Exception e) {
297+
Logger.warn(methodTag, "Unexpected error with new KeyPairGeneratorSpec. Falling back to legacy spec. "+ e);
298+
keyPair = generateKeyPairWithLegacySpec(mAlias, keypairGenStartTime);
299+
if (e.getMessage() != null) {
300+
span.setAttribute(AttributeName.keypair_gen_exception.name(), e.getMessage());
301+
}
302+
span.setAttribute(AttributeName.key_pair_gen_successful_method.name(), "legacy_key_gen_spec");
303+
span.setStatus(StatusCode.OK);
304+
} finally {
305+
span.end();
306+
}
307+
}
308+
else {
309+
// If flight for using new keygen spec is not enabled, use the legacy spec.
310+
Logger.info(methodTag, "Using legacy spec for keypair generation directly.");
311+
keyPair = generateKeyPairWithLegacySpec(mAlias, keypairGenStartTime);
312+
}
257313
}
258314

259315
final byte[] keyWrapped = AndroidKeyStoreUtil.wrap(unencryptedKey, keyPair, WRAP_ALGORITHM);
260316
FileUtil.writeDataToFile(keyWrapped, getKeyFile());
261317
}
262318

319+
@RequiresApi(api = Build.VERSION_CODES.P)
320+
private KeyPair attemptKeyPairGeneration(@NonNull final String alias, boolean useWrapPurpose, long keypairGenStartTime) throws ClientException{
321+
KeyPair keyPair = AndroidKeyStoreUtil.generateKeyPair(
322+
WRAP_KEY_ALGORITHM, getSpecForKeyStoreKey(alias, useWrapPurpose));
323+
recordKeyGenerationTime(keypairGenStartTime);
324+
return keyPair;
325+
}
326+
327+
private KeyPair generateKeyPairWithLegacySpec(@NonNull final String alias, long keypairGenStartTime) throws ClientException{
328+
KeyPair keyPair = AndroidKeyStoreUtil.generateKeyPair(
329+
WRAP_KEY_ALGORITHM, getLegacySpecForKeyStoreKey(mContext, alias));
330+
recordKeyGenerationTime(keypairGenStartTime);
331+
return keyPair;
332+
}
333+
334+
private void recordKeyGenerationTime(long keypairGenStartTime) {
335+
long elapsedTime = System.currentTimeMillis() - keypairGenStartTime;
336+
SpanExtension.current().setAttribute(AttributeName.elapsed_time_keypair_generation.name(), elapsedTime);
337+
}
338+
263339
/**
264340
* Wipe all the data associated from this key.
265341
*/
@@ -304,28 +380,21 @@ private static AlgorithmParameterSpec getLegacySpecForKeyStoreKey(@NonNull final
304380
* Generate a self-signed cert and derive an AlgorithmParameterSpec from that.
305381
* This is for the key to be generated in {@link KeyStore} via {@link KeyPairGenerator}
306382
*
307-
* @param context an Android {@link Context} object.
383+
* @param alias the alias for the key.
384+
* @param tryPurposeWrap whether to try to use the wrap purpose in the key generation spec.
308385
* @return a {@link AlgorithmParameterSpec} for the keystore key (that we'll use to wrap the secret key).
309386
*/
310-
private static AlgorithmParameterSpec getSpecForKeyStoreKey(@NonNull final Context context, @NonNull final String alias) {
311-
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) {
312-
return getLegacySpecForKeyStoreKey(context, alias);
313-
} else {
314-
final String certInfo = String.format(Locale.ROOT, "CN=%s, OU=%s",
315-
alias,
316-
context.getPackageName());
317-
final int certValidYears = 100;
318-
int purposes = KeyProperties.PURPOSE_WRAP_KEY | KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT;
319-
return new KeyGenParameterSpec.Builder(alias, purposes)
320-
.setCertificateSubject(new X500Principal(certInfo))
321-
.setCertificateSerialNumber(BigInteger.ONE)
322-
.setCertificateNotBefore(new Date())
323-
.setCertificateNotAfter(new Date(System.currentTimeMillis() + TimeUnit.DAYS.toMillis(365 * certValidYears)))
324-
.setKeySize(2048)
325-
.setDigests(KeyProperties.DIGEST_SHA256, KeyProperties.DIGEST_SHA512)
326-
.setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_PKCS1)
327-
.build();
387+
@RequiresApi(api = Build.VERSION_CODES.P)
388+
private static AlgorithmParameterSpec getSpecForKeyStoreKey(@NonNull final String alias, boolean tryPurposeWrap) {
389+
int purposes = KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT;
390+
if (tryPurposeWrap) {
391+
purposes |= KeyProperties.PURPOSE_WRAP_KEY;
328392
}
393+
return new KeyGenParameterSpec.Builder(alias, purposes)
394+
.setKeySize(2048)
395+
.setDigests(KeyProperties.DIGEST_SHA256, KeyProperties.DIGEST_SHA512)
396+
.setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_PKCS1)
397+
.build();
329398
}
330399

331400
/**

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,12 @@ public enum CommonFlight implements IFlightConfig {
8888
/**
8989
* Flight to enable the re-attachment of new PRT header logic. Default is true.
9090
*/
91-
ENABLE_ATTACH_NEW_PRT_HEADER_WHEN_NONCE_EXPIRED("EnableAttachNewPrtHeaderWhenNonceExpired", true);
91+
ENABLE_ATTACH_NEW_PRT_HEADER_WHEN_NONCE_EXPIRED("EnableAttachNewPrtHeaderWhenNonceExpired", true),
92+
93+
/**
94+
* Flight to enable the new key generation spec for wrap key. Default is true.
95+
*/
96+
ENABLE_NEW_KEY_GEN_SPEC_FOR_WRAP("EnableNewKeyGenSpecForWrap", true);
9297
private String key;
9398
private Object defaultValue;
9499
CommonFlight(@NonNull String key, @NonNull Object defaultValue) {

‎common4j/src/main/com/microsoft/identity/common/java/opentelemetry/AttributeName.java‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,5 +312,20 @@ public enum AttributeName {
312312
/**
313313
* Indicates the new refresh token credential header attached in the eSTS request.
314314
*/
315-
is_new_refresh_token_cred_header_attached
315+
is_new_refresh_token_cred_header_attached,
316+
317+
/**
318+
* The time (in milliseconds) spent on generating a keypair.
319+
*/
320+
elapsed_time_keypair_generation,
321+
322+
/**
323+
* Indicates the successful method used to generate a keypair.
324+
*/
325+
key_pair_gen_successful_method,
326+
327+
/**
328+
* Indicates the exception in generating a keypair.
329+
*/
330+
keypair_gen_exception
316331
}

‎common4j/src/main/com/microsoft/identity/common/java/opentelemetry/SpanName.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,5 +59,6 @@ public enum SpanName {
5959
UpgradeDeviceRegistration,
6060
RemoveBrokerAccount,
6161
ProcessNonceFromEstsRedirect,
62-
DataStoreCorruptionException
62+
DataStoreCorruptionException,
63+
KeyPairGeneration
6364
}

‎common4j/versioning/version.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
#Wed May 12 20:08:39 UTC 2021
2-
versionName=16.0.1
2+
versionName=16.0.2
33
versionCode=1
44
latestPatchVersion=227

‎versioning/version.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
#Tue Apr 06 22:55:08 UTC 2021
2-
versionName=19.0.1
2+
versionName=19.0.2
33
versionCode=1
44
latestPatchVersion=234

0 commit comments

Comments
 (0)