Correctly preserve key generation parameters
Due to an oversight, some of the key generation parameters that are set
in KeyGenParameterSpec were not preserved when parceling the object
(they should have been added to ParcelableKeyGenParameterSpec but were
not).
This means these parameters will be ignored when generating keys using
the DevicePolicyManager.generateKeyPair method, leading to an
inconsistent key generation behaviour between the DevicePolicyManager
and KeyStore.
In particular, this would prevent callers from using StrongBox when
generating keys for use in the KeyChain.
Fix the issue by simply persisting these parameters in
ParcelableKeyGenParameterSpec and making sure that the Builder copies
them too from the source KeyGenParameterSpec.
Left to do is put in place an automated measure to find out
discrepancies between the two classes.
Bug: 110915980
Bug: 110882855
Bug: 109953656
Test: atest KeystoreTests
Change-Id: Ic64bd2921b6dfc97ea34ecba55f532312963ffcb
diff --git a/keystore/java/android/security/keystore/KeyGenParameterSpec.java b/keystore/java/android/security/keystore/KeyGenParameterSpec.java
index b2e0f67..553e86e 100644
--- a/keystore/java/android/security/keystore/KeyGenParameterSpec.java
+++ b/keystore/java/android/security/keystore/KeyGenParameterSpec.java
@@ -268,6 +268,11 @@
private final boolean mIsStrongBoxBacked;
private final boolean mUserConfirmationRequired;
private final boolean mUnlockedDeviceRequired;
+ /*
+ * ***NOTE***: All new fields MUST also be added to the following:
+ * ParcelableKeyGenParameterSpec class.
+ * The KeyGenParameterSpec.Builder constructor that takes a KeyGenParameterSpec
+ */
/**
* @hide should be built with Builder
@@ -791,6 +796,9 @@
mUniqueIdIncluded = sourceSpec.isUniqueIdIncluded();
mUserAuthenticationValidWhileOnBody = sourceSpec.isUserAuthenticationValidWhileOnBody();
mInvalidatedByBiometricEnrollment = sourceSpec.isInvalidatedByBiometricEnrollment();
+ mIsStrongBoxBacked = sourceSpec.isStrongBoxBacked();
+ mUserConfirmationRequired = sourceSpec.isUserConfirmationRequired();
+ mUnlockedDeviceRequired = sourceSpec.isUnlockedDeviceRequired();
}
/**
diff --git a/keystore/java/android/security/keystore/ParcelableKeyGenParameterSpec.java b/keystore/java/android/security/keystore/ParcelableKeyGenParameterSpec.java
index 911bbf8..8231dc9 100644
--- a/keystore/java/android/security/keystore/ParcelableKeyGenParameterSpec.java
+++ b/keystore/java/android/security/keystore/ParcelableKeyGenParameterSpec.java
@@ -97,11 +97,14 @@
out.writeBoolean(mSpec.isRandomizedEncryptionRequired());
out.writeBoolean(mSpec.isUserAuthenticationRequired());
out.writeInt(mSpec.getUserAuthenticationValidityDurationSeconds());
+ out.writeBoolean(mSpec.isUserPresenceRequired());
out.writeByteArray(mSpec.getAttestationChallenge());
out.writeBoolean(mSpec.isUniqueIdIncluded());
out.writeBoolean(mSpec.isUserAuthenticationValidWhileOnBody());
out.writeBoolean(mSpec.isInvalidatedByBiometricEnrollment());
- out.writeBoolean(mSpec.isUserPresenceRequired());
+ out.writeBoolean(mSpec.isStrongBoxBacked());
+ out.writeBoolean(mSpec.isUserConfirmationRequired());
+ out.writeBoolean(mSpec.isUnlockedDeviceRequired());
}
private static Date readDateOrNull(Parcel in) {
@@ -114,19 +117,12 @@
}
private ParcelableKeyGenParameterSpec(Parcel in) {
- String keystoreAlias = in.readString();
- int purposes = in.readInt();
- KeyGenParameterSpec.Builder builder = new KeyGenParameterSpec.Builder(
- keystoreAlias, purposes);
- builder.setUid(in.readInt());
- // KeySize is -1 by default, if the KeyGenParameterSpec previously parcelled had the default
- // value, do not set it as this will cause setKeySize to throw.
- int keySize = in.readInt();
- if (keySize >= 0) {
- builder.setKeySize(keySize);
- }
+ final String keystoreAlias = in.readString();
+ final int purposes = in.readInt();
+ final int uid = in.readInt();
+ final int keySize = in.readInt();
- int keySpecType = in.readInt();
+ final int keySpecType = in.readInt();
AlgorithmParameterSpec algorithmSpec = null;
if (keySpecType == ALGORITHM_PARAMETER_SPEC_NONE) {
algorithmSpec = null;
@@ -141,32 +137,60 @@
throw new IllegalArgumentException(
String.format("Unknown algorithm parameter spec: %d", keySpecType));
}
- if (algorithmSpec != null) {
- builder.setAlgorithmParameterSpec(algorithmSpec);
- }
- builder.setCertificateSubject(new X500Principal(in.createByteArray()));
- builder.setCertificateSerialNumber(new BigInteger(in.createByteArray()));
- builder.setCertificateNotBefore(new Date(in.readLong()));
- builder.setCertificateNotAfter(new Date(in.readLong()));
- builder.setKeyValidityStart(readDateOrNull(in));
- builder.setKeyValidityForOriginationEnd(readDateOrNull(in));
- builder.setKeyValidityForConsumptionEnd(readDateOrNull(in));
- String[] digests = in.createStringArray();
- if (digests != null) {
- builder.setDigests(digests);
- }
- builder.setEncryptionPaddings(in.createStringArray());
- builder.setSignaturePaddings(in.createStringArray());
- builder.setBlockModes(in.createStringArray());
- builder.setRandomizedEncryptionRequired(in.readBoolean());
- builder.setUserAuthenticationRequired(in.readBoolean());
- builder.setUserAuthenticationValidityDurationSeconds(in.readInt());
- builder.setAttestationChallenge(in.createByteArray());
- builder.setUniqueIdIncluded(in.readBoolean());
- builder.setUserAuthenticationValidWhileOnBody(in.readBoolean());
- builder.setInvalidatedByBiometricEnrollment(in.readBoolean());
- builder.setUserPresenceRequired(in.readBoolean());
- mSpec = builder.build();
+
+ final X500Principal certificateSubject = new X500Principal(in.createByteArray());
+ final BigInteger certificateSerialNumber = new BigInteger(in.createByteArray());
+ final Date certificateNotBefore = new Date(in.readLong());
+ final Date certificateNotAfter = new Date(in.readLong());
+ final Date keyValidityStartDate = readDateOrNull(in);
+ final Date keyValidityForOriginationEnd = readDateOrNull(in);
+ final Date keyValidityForConsumptionEnd = readDateOrNull(in);
+ final String[] digests = in.createStringArray();
+ final String[] encryptionPaddings = in.createStringArray();
+ final String[] signaturePaddings = in.createStringArray();
+ final String[] blockModes = in.createStringArray();
+ final boolean randomizedEncryptionRequired = in.readBoolean();
+ final boolean userAuthenticationRequired = in.readBoolean();
+ final int userAuthenticationValidityDurationSeconds = in.readInt();
+ final boolean userPresenceRequired = in.readBoolean();
+ final byte[] attestationChallenge = in.createByteArray();
+ final boolean uniqueIdIncluded = in.readBoolean();
+ final boolean userAuthenticationValidWhileOnBody = in.readBoolean();
+ final boolean invalidatedByBiometricEnrollment = in.readBoolean();
+ final boolean isStrongBoxBacked = in.readBoolean();
+ final boolean userConfirmationRequired = in.readBoolean();
+ final boolean unlockedDeviceRequired = in.readBoolean();
+ // The KeyGenParameterSpec is intentionally not constructed using a Builder here:
+ // The intention is for this class to break if new parameters are added to the
+ // KeyGenParameterSpec constructor (whereas using a builder would silently drop them).
+ mSpec = new KeyGenParameterSpec(
+ keystoreAlias,
+ uid,
+ keySize,
+ algorithmSpec,
+ certificateSubject,
+ certificateSerialNumber,
+ certificateNotBefore,
+ certificateNotAfter,
+ keyValidityStartDate,
+ keyValidityForOriginationEnd,
+ keyValidityForConsumptionEnd,
+ purposes,
+ digests,
+ encryptionPaddings,
+ signaturePaddings,
+ blockModes,
+ randomizedEncryptionRequired,
+ userAuthenticationRequired,
+ userAuthenticationValidityDurationSeconds,
+ userPresenceRequired,
+ attestationChallenge,
+ uniqueIdIncluded,
+ userAuthenticationValidWhileOnBody,
+ invalidatedByBiometricEnrollment,
+ isStrongBoxBacked,
+ userConfirmationRequired,
+ unlockedDeviceRequired);
}
public static final Creator<ParcelableKeyGenParameterSpec> CREATOR = new Creator<ParcelableKeyGenParameterSpec>() {
diff --git a/keystore/tests/src/android/security/ParcelableKeyGenParameterSpecTest.java b/keystore/tests/src/android/security/ParcelableKeyGenParameterSpecTest.java
index 254b6be..32f8ec4 100644
--- a/keystore/tests/src/android/security/ParcelableKeyGenParameterSpecTest.java
+++ b/keystore/tests/src/android/security/ParcelableKeyGenParameterSpecTest.java
@@ -77,6 +77,9 @@
.setUniqueIdIncluded(true)
.setUserAuthenticationValidWhileOnBody(true)
.setInvalidatedByBiometricEnrollment(true)
+ .setIsStrongBoxBacked(true)
+ .setUserConfirmationRequired(true)
+ .setUnlockedDeviceRequired(true)
.build();
}
@@ -105,6 +108,9 @@
assertThat(spec.isUniqueIdIncluded(), is(true));
assertThat(spec.isUserAuthenticationValidWhileOnBody(), is(true));
assertThat(spec.isInvalidatedByBiometricEnrollment(), is(true));
+ assertThat(spec.isStrongBoxBacked(), is(true));
+ assertThat(spec.isUserConfirmationRequired(), is(true));
+ assertThat(spec.isUnlockedDeviceRequired(), is(true));
}
private Parcel parcelForReading(ParcelableKeyGenParameterSpec spec) {
