Do not add the MGF Digest tag for upgraded keys
If a key does not have the MGF_DIGEST tag in its key characteristics,
do not include the MGF_DIGEST tag for it (even if the algorithm string
specifies it).
This fixes an issue with keys that were generated on Android 13, where
the MGF_DIGEST tag was not propagated from the SPI layer. Such keys will
not have the MGF_DIGEST tag and so it will not be added by the SPI layer
even if the algorithm string specifies it. This maintains Android 13's
(incorrect) behaviour of ignoring the MGF Digest specification, but is
necessary to use those keys (otherwise KeyMint will error out
on begin() due to an incompatible MGF digest specification).
Bug: 278157584
Test: atest CtsKeystoreWycheproofTestCases:com.google.security.wycheproof.RsaOaepTest
Change-Id: I0f1fa7983f9c771bec3196c6a617eb7044ac2e79
diff --git a/keystore/java/android/security/keystore2/AndroidKeyStoreCipherSpiBase.java b/keystore/java/android/security/keystore2/AndroidKeyStoreCipherSpiBase.java
index 7571e44..d129891 100644
--- a/keystore/java/android/security/keystore2/AndroidKeyStoreCipherSpiBase.java
+++ b/keystore/java/android/security/keystore2/AndroidKeyStoreCipherSpiBase.java
@@ -24,6 +24,7 @@
import android.security.KeyStoreOperation;
import android.security.keymaster.KeymasterDefs;
import android.security.keystore.KeyStoreCryptoOperation;
+import android.system.keystore2.Authorization;
import libcore.util.EmptyArray;
@@ -119,6 +120,14 @@
mCipher = null;
}
+ private Authorization[] getKeyCharacteristics(Key key) {
+ if (!(key instanceof AndroidKeyStoreKey)) {
+ return new Authorization[] {};
+ }
+
+ return ((AndroidKeyStoreKey) key).getAuthorizations();
+ }
+
@Override
protected final void engineInit(int opmode, Key key, SecureRandom random)
throws InvalidKeyException {
@@ -173,7 +182,7 @@
init(opmode, key, random);
initAlgorithmSpecificParameters();
try {
- ensureKeystoreOperationInitialized();
+ ensureKeystoreOperationInitialized(getKeyCharacteristics(key));
} catch (InvalidAlgorithmParameterException e) {
throw new InvalidKeyException(e);
}
@@ -206,7 +215,7 @@
try {
init(opmode, key, random);
initAlgorithmSpecificParameters(params);
- ensureKeystoreOperationInitialized();
+ ensureKeystoreOperationInitialized(getKeyCharacteristics(key));
success = true;
} finally {
if (!success) {
@@ -236,7 +245,7 @@
try {
init(opmode, key, random);
initAlgorithmSpecificParameters(params);
- ensureKeystoreOperationInitialized();
+ ensureKeystoreOperationInitialized(getKeyCharacteristics(key));
success = true;
} finally {
if (!success) {
@@ -310,7 +319,8 @@
mCachedException = null;
}
- private void ensureKeystoreOperationInitialized() throws InvalidKeyException,
+ private void ensureKeystoreOperationInitialized(Authorization[] keyCharacteristics)
+ throws InvalidKeyException,
InvalidAlgorithmParameterException {
if (mMainDataStreamer != null) {
return;
@@ -323,7 +333,7 @@
}
List<KeyParameter> parameters = new ArrayList<>();
- addAlgorithmSpecificParametersToBegin(parameters);
+ addAlgorithmSpecificParametersToBegin(parameters, keyCharacteristics);
int purpose;
if (mKeymasterPurposeOverride != -1) {
@@ -404,7 +414,7 @@
return null;
}
try {
- ensureKeystoreOperationInitialized();
+ ensureKeystoreOperationInitialized(getKeyCharacteristics(mKey));
} catch (InvalidKeyException | InvalidAlgorithmParameterException e) {
mCachedException = e;
return null;
@@ -520,7 +530,7 @@
}
try {
- ensureKeystoreOperationInitialized();
+ ensureKeystoreOperationInitialized(getKeyCharacteristics(mKey));
} catch (InvalidKeyException | InvalidAlgorithmParameterException e) {
mCachedException = e;
return;
@@ -597,7 +607,7 @@
}
try {
- ensureKeystoreOperationInitialized();
+ ensureKeystoreOperationInitialized(getKeyCharacteristics(mKey));
} catch (InvalidKeyException | InvalidAlgorithmParameterException e) {
throw (IllegalBlockSizeException) new IllegalBlockSizeException().initCause(e);
}
@@ -1012,6 +1022,22 @@
@NonNull List<KeyParameter> parameters);
/**
+ * Invoked to add algorithm-specific parameters for the KeyStore's {@code begin} operation,
+ * including the key characteristics. This is useful in case the parameters to {@code begin}
+ * depend on how the key was generated.
+ * The default implementation provided here simply ignores these key characteristics because
+ * they are not be needed for most engines.
+ *
+ * @param parameters keystore/keymaster arguments to be populated with algorithm-specific
+ * parameters.
+ * @param keyCharacteristics The key's characteristics.
+ */
+ protected void addAlgorithmSpecificParametersToBegin(
+ @NonNull List<KeyParameter> parameters, Authorization[] keyCharacteristics) {
+ addAlgorithmSpecificParametersToBegin(parameters);
+ }
+
+ /**
* Invoked to obtain algorithm-specific parameters from the result of the KeyStore's
* {@code begin} operation.
*
diff --git a/keystore/java/android/security/keystore2/AndroidKeyStoreRSACipherSpi.java b/keystore/java/android/security/keystore2/AndroidKeyStoreRSACipherSpi.java
index e9b66aa..3bb2564 100644
--- a/keystore/java/android/security/keystore2/AndroidKeyStoreRSACipherSpi.java
+++ b/keystore/java/android/security/keystore2/AndroidKeyStoreRSACipherSpi.java
@@ -288,16 +288,34 @@
}
}
+ private static boolean isMgfDigestTagPresentInKeyProperties(
+ Authorization[] keyCharacteristics) {
+ for (Authorization authorization : keyCharacteristics) {
+ if (authorization.keyParameter.tag == KeymasterDefs.KM_TAG_RSA_OAEP_MGF_DIGEST) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
@Override
protected final void addAlgorithmSpecificParametersToBegin(
- @NonNull List<KeyParameter> parameters) {
- super.addAlgorithmSpecificParametersToBegin(parameters);
+ @NonNull List<KeyParameter> parameters, Authorization[] keyCharacteristics) {
+ super.addAlgorithmSpecificParametersToBegin(parameters, keyCharacteristics);
parameters.add(KeyStore2ParameterUtils.makeEnum(
KeymasterDefs.KM_TAG_DIGEST, mKeymasterDigest
));
- parameters.add(KeyStore2ParameterUtils.makeEnum(
- KeymasterDefs.KM_TAG_RSA_OAEP_MGF_DIGEST, mKeymasterMgf1Digest
- ));
+ // Only add the KM_TAG_RSA_OAEP_MGF_DIGEST tag to begin() if the MGF Digest is
+ // present in the key properties. Keys generated prior to Android 14 did not have
+ // this tag (Keystore didn't add it) so specifying any MGF digest tag would cause
+ // a begin() operation (on an Android 14 device) to fail (with a key that was generated
+ // on Android 13 or below).
+ if (isMgfDigestTagPresentInKeyProperties(keyCharacteristics)) {
+ parameters.add(KeyStore2ParameterUtils.makeEnum(
+ KeymasterDefs.KM_TAG_RSA_OAEP_MGF_DIGEST, mKeymasterMgf1Digest
+ ));
+ }
}
@Override
