diff options
author | Alex Klyubin <klyubin@google.com> | 2015-07-06 10:31:07 -0700 |
---|---|---|
committer | Alex Klyubin <klyubin@google.com> | 2015-07-06 11:27:16 -0700 |
commit | b6e628644a981b8077b3755b9def4550ff4a46a0 (patch) | |
tree | ab842adc8ff4b36e6372f73d9627f4a6254eb7c9 | |
parent | 7fe86c4753e88058a7f1a1bf8d0302df9a64bd2e (diff) | |
download | frameworks_base-b6e628644a981b8077b3755b9def4550ff4a46a0.zip frameworks_base-b6e628644a981b8077b3755b9def4550ff4a46a0.tar.gz frameworks_base-b6e628644a981b8077b3755b9def4550ff4a46a0.tar.bz2 |
Avoid IllegalStateException when generating/importing keys.
This avoids IllegalStateException when generating/importing keys which
require user authentication when the system is not configured to
generate/import such keys (e.g., secure lock screen not set up).
The documentation states that before generating/importing such keys
apps should check (using public API) whether the system is in a
suitable state. However, some apps are not doing that and instead
catching the IllegalStateException thrown during key
generation/import. This is a bad practice because this exception is an
undocumented implementation detail and should thus not be depended
upon.
This CL addresses this issue as follows:
1. Key(Pair)Generator.init now throws a checked
InvalidAlgorithmParameterException when the system is in a wrong
state. Because in most uses of Key(Pair)Generator .init is
immediately followed by .generate, this prevents .generate from
encountering this state and does so using a checked exception
which is part of public API.
2. Key import rethrows the IllegalStateException as a checked
KeyStoreException which is meant to be thrown if the key cannot be
imported for any reason. Key(Pair)Generator.generate unfortunately
cannot throw any checked exceptions and thus has to continue
throwing unchecked exceptions.
Bug: 22262809
Change-Id: Ic0f7b7a90e0ba63df9139c79b80a8649d2645d2a
4 files changed, 105 insertions, 89 deletions
diff --git a/keystore/java/android/security/keystore/AndroidKeyStoreKeyGeneratorSpi.java b/keystore/java/android/security/keystore/AndroidKeyStoreKeyGeneratorSpi.java index 258133d..6a7930a 100644 --- a/keystore/java/android/security/keystore/AndroidKeyStoreKeyGeneratorSpi.java +++ b/keystore/java/android/security/keystore/AndroidKeyStoreKeyGeneratorSpi.java @@ -239,6 +239,13 @@ public abstract class AndroidKeyStoreKeyGeneratorSpi extends KeyGeneratorSpi { "At least one digest algorithm must be specified"); } } + + // Check that user authentication related parameters are acceptable. This method + // will throw an IllegalStateException if there are issues (e.g., secure lock screen + // not set up). + KeymasterUtils.addUserAuthArgs(new KeymasterArguments(), + spec.isUserAuthenticationRequired(), + spec.getUserAuthenticationValidityDurationSeconds()); } catch (IllegalStateException | IllegalArgumentException e) { throw new InvalidAlgorithmParameterException(e); } diff --git a/keystore/java/android/security/keystore/AndroidKeyStoreKeyPairGeneratorSpi.java b/keystore/java/android/security/keystore/AndroidKeyStoreKeyPairGeneratorSpi.java index 459514d..6b36a58 100644 --- a/keystore/java/android/security/keystore/AndroidKeyStoreKeyPairGeneratorSpi.java +++ b/keystore/java/android/security/keystore/AndroidKeyStoreKeyPairGeneratorSpi.java @@ -310,7 +310,14 @@ public abstract class AndroidKeyStoreKeyPairGeneratorSpi extends KeyPairGenerato } else { mKeymasterDigests = EmptyArray.INT; } - } catch (IllegalArgumentException e) { + + // Check that user authentication related parameters are acceptable. This method + // will throw an IllegalStateException if there are issues (e.g., secure lock screen + // not set up). + KeymasterUtils.addUserAuthArgs(new KeymasterArguments(), + mSpec.isUserAuthenticationRequired(), + mSpec.getUserAuthenticationValidityDurationSeconds()); + } catch (IllegalArgumentException | IllegalStateException e) { throw new InvalidAlgorithmParameterException(e); } diff --git a/keystore/java/android/security/keystore/AndroidKeyStoreSpi.java b/keystore/java/android/security/keystore/AndroidKeyStoreSpi.java index dc8f1e3..e9f19cd 100644 --- a/keystore/java/android/security/keystore/AndroidKeyStoreSpi.java +++ b/keystore/java/android/security/keystore/AndroidKeyStoreSpi.java @@ -484,8 +484,8 @@ public class AndroidKeyStoreSpi extends KeyStoreSpi { spec.getKeyValidityForOriginationEnd()); importArgs.addDateIfNotNull(KeymasterDefs.KM_TAG_USAGE_EXPIRE_DATETIME, spec.getKeyValidityForConsumptionEnd()); - } catch (IllegalArgumentException e) { - throw new KeyStoreException("Invalid parameter", e); + } catch (IllegalArgumentException | IllegalStateException e) { + throw new KeyStoreException(e); } } @@ -598,102 +598,100 @@ public class AndroidKeyStoreSpi extends KeyStoreSpi { + " RAW format export"); } - String keyAlgorithmString = key.getAlgorithm(); - int keymasterAlgorithm; - int keymasterDigest; - try { - keymasterAlgorithm = - KeyProperties.KeyAlgorithm.toKeymasterSecretKeyAlgorithm(keyAlgorithmString); - keymasterDigest = KeyProperties.KeyAlgorithm.toKeymasterDigest(keyAlgorithmString); - } catch (IllegalArgumentException e) { - throw new KeyStoreException("Unsupported secret key algorithm: " + keyAlgorithmString); - } - KeymasterArguments args = new KeymasterArguments(); - args.addEnum(KeymasterDefs.KM_TAG_ALGORITHM, keymasterAlgorithm); - - int[] keymasterDigests; - if (params.isDigestsSpecified()) { - // Digest(s) specified in parameters - keymasterDigests = KeyProperties.Digest.allToKeymaster(params.getDigests()); - if (keymasterDigest != -1) { - // Digest also specified in the JCA key algorithm name. - if (!com.android.internal.util.ArrayUtils.contains( - keymasterDigests, keymasterDigest)) { - throw new KeyStoreException("Key digest mismatch" - + ". Key: " + keyAlgorithmString - + ", parameter spec: " + Arrays.asList(params.getDigests())); - } - // When the key is read back from keystore we reconstruct the JCA key algorithm - // name from the KM_TAG_ALGORITHM and the first KM_TAG_DIGEST. Thus we need to - // ensure that the digest reflected in the JCA key algorithm name is the first - // KM_TAG_DIGEST tag. - if (keymasterDigests[0] != keymasterDigest) { - // The first digest is not the one implied by the JCA key algorithm name. - // Swap the implied digest with the first one. - for (int i = 0; i < keymasterDigests.length; i++) { - if (keymasterDigests[i] == keymasterDigest) { - keymasterDigests[i] = keymasterDigests[0]; - keymasterDigests[0] = keymasterDigest; - break; + try { + int keymasterAlgorithm = + KeyProperties.KeyAlgorithm.toKeymasterSecretKeyAlgorithm(key.getAlgorithm()); + args.addEnum(KeymasterDefs.KM_TAG_ALGORITHM, keymasterAlgorithm); + + int[] keymasterDigests; + int keymasterDigest = KeyProperties.KeyAlgorithm.toKeymasterDigest(key.getAlgorithm()); + if (params.isDigestsSpecified()) { + // Digest(s) specified in parameters + keymasterDigests = KeyProperties.Digest.allToKeymaster(params.getDigests()); + if (keymasterDigest != -1) { + // Digest also specified in the JCA key algorithm name. + if (!com.android.internal.util.ArrayUtils.contains( + keymasterDigests, keymasterDigest)) { + throw new KeyStoreException("Digest specified in key algorithm " + + key.getAlgorithm() + " not specified in protection parameters: " + + Arrays.asList(params.getDigests())); + } + // When the key is read back from keystore we reconstruct the JCA key algorithm + // name from the KM_TAG_ALGORITHM and the first KM_TAG_DIGEST. Thus we need to + // ensure that the digest reflected in the JCA key algorithm name is the first + // KM_TAG_DIGEST tag. + if (keymasterDigests[0] != keymasterDigest) { + // The first digest is not the one implied by the JCA key algorithm name. + // Swap the implied digest with the first one. + for (int i = 0; i < keymasterDigests.length; i++) { + if (keymasterDigests[i] == keymasterDigest) { + keymasterDigests[i] = keymasterDigests[0]; + keymasterDigests[0] = keymasterDigest; + break; + } } } } - } - } else { - // No digest specified in parameters - if (keymasterDigest != -1) { - // Digest specified in the JCA key algorithm name. - keymasterDigests = new int[] {keymasterDigest}; } else { - keymasterDigests = EmptyArray.INT; + // No digest specified in parameters + if (keymasterDigest != -1) { + // Digest specified in the JCA key algorithm name. + keymasterDigests = new int[] {keymasterDigest}; + } else { + keymasterDigests = EmptyArray.INT; + } } - } - args.addEnums(KeymasterDefs.KM_TAG_DIGEST, keymasterDigests); - if (keymasterAlgorithm == KeymasterDefs.KM_ALGORITHM_HMAC) { - if (keymasterDigests.length == 0) { - throw new KeyStoreException("At least one digest algorithm must be specified" - + " for key algorithm " + keyAlgorithmString); + args.addEnums(KeymasterDefs.KM_TAG_DIGEST, keymasterDigests); + if (keymasterAlgorithm == KeymasterDefs.KM_ALGORITHM_HMAC) { + if (keymasterDigests.length == 0) { + throw new KeyStoreException("At least one digest algorithm must be specified" + + " for key algorithm " + key.getAlgorithm()); + } } - } - @KeyProperties.PurposeEnum int purposes = params.getPurposes(); - int[] keymasterBlockModes = - KeyProperties.BlockMode.allToKeymaster(params.getBlockModes()); - if (((purposes & KeyProperties.PURPOSE_ENCRYPT) != 0) - && (params.isRandomizedEncryptionRequired())) { - for (int keymasterBlockMode : keymasterBlockModes) { - if (!KeymasterUtils.isKeymasterBlockModeIndCpaCompatibleWithSymmetricCrypto( - keymasterBlockMode)) { - throw new KeyStoreException( - "Randomized encryption (IND-CPA) required but may be violated by block" - + " mode: " - + KeyProperties.BlockMode.fromKeymaster(keymasterBlockMode) - + ". See KeyProtection documentation."); + @KeyProperties.PurposeEnum int purposes = params.getPurposes(); + int[] keymasterBlockModes = + KeyProperties.BlockMode.allToKeymaster(params.getBlockModes()); + if (((purposes & KeyProperties.PURPOSE_ENCRYPT) != 0) + && (params.isRandomizedEncryptionRequired())) { + for (int keymasterBlockMode : keymasterBlockModes) { + if (!KeymasterUtils.isKeymasterBlockModeIndCpaCompatibleWithSymmetricCrypto( + keymasterBlockMode)) { + throw new KeyStoreException( + "Randomized encryption (IND-CPA) required but may be violated by" + + " block mode: " + + KeyProperties.BlockMode.fromKeymaster(keymasterBlockMode) + + ". See KeyProtection documentation."); + } } } - } - args.addEnums(KeymasterDefs.KM_TAG_PURPOSE, KeyProperties.Purpose.allToKeymaster(purposes)); - args.addEnums(KeymasterDefs.KM_TAG_BLOCK_MODE, keymasterBlockModes); - if (params.getSignaturePaddings().length > 0) { - throw new KeyStoreException("Signature paddings not supported for symmetric keys"); - } - int[] keymasterPaddings = KeyProperties.EncryptionPadding.allToKeymaster( - params.getEncryptionPaddings()); - args.addEnums(KeymasterDefs.KM_TAG_PADDING, keymasterPaddings); - KeymasterUtils.addUserAuthArgs(args, - params.isUserAuthenticationRequired(), - params.getUserAuthenticationValidityDurationSeconds()); - args.addDateIfNotNull(KeymasterDefs.KM_TAG_ACTIVE_DATETIME, params.getKeyValidityStart()); - args.addDateIfNotNull(KeymasterDefs.KM_TAG_ORIGINATION_EXPIRE_DATETIME, - params.getKeyValidityForOriginationEnd()); - args.addDateIfNotNull(KeymasterDefs.KM_TAG_USAGE_EXPIRE_DATETIME, - params.getKeyValidityForConsumptionEnd()); - - if (((purposes & KeyProperties.PURPOSE_ENCRYPT) != 0) - && (!params.isRandomizedEncryptionRequired())) { - // Permit caller-provided IV when encrypting with this key - args.addBoolean(KeymasterDefs.KM_TAG_CALLER_NONCE); + args.addEnums(KeymasterDefs.KM_TAG_PURPOSE, + KeyProperties.Purpose.allToKeymaster(purposes)); + args.addEnums(KeymasterDefs.KM_TAG_BLOCK_MODE, keymasterBlockModes); + if (params.getSignaturePaddings().length > 0) { + throw new KeyStoreException("Signature paddings not supported for symmetric keys"); + } + int[] keymasterPaddings = KeyProperties.EncryptionPadding.allToKeymaster( + params.getEncryptionPaddings()); + args.addEnums(KeymasterDefs.KM_TAG_PADDING, keymasterPaddings); + KeymasterUtils.addUserAuthArgs(args, + params.isUserAuthenticationRequired(), + params.getUserAuthenticationValidityDurationSeconds()); + args.addDateIfNotNull(KeymasterDefs.KM_TAG_ACTIVE_DATETIME, + params.getKeyValidityStart()); + args.addDateIfNotNull(KeymasterDefs.KM_TAG_ORIGINATION_EXPIRE_DATETIME, + params.getKeyValidityForOriginationEnd()); + args.addDateIfNotNull(KeymasterDefs.KM_TAG_USAGE_EXPIRE_DATETIME, + params.getKeyValidityForConsumptionEnd()); + + if (((purposes & KeyProperties.PURPOSE_ENCRYPT) != 0) + && (!params.isRandomizedEncryptionRequired())) { + // Permit caller-provided IV when encrypting with this key + args.addBoolean(KeymasterDefs.KM_TAG_CALLER_NONCE); + } + } catch (IllegalArgumentException | IllegalStateException e) { + throw new KeyStoreException(e); } Credentials.deleteAllTypesForAlias(mKeyStore, entryAlias); diff --git a/keystore/java/android/security/keystore/KeymasterUtils.java b/keystore/java/android/security/keystore/KeymasterUtils.java index 3cd3f2a..92d636c 100644 --- a/keystore/java/android/security/keystore/KeymasterUtils.java +++ b/keystore/java/android/security/keystore/KeymasterUtils.java @@ -87,6 +87,10 @@ public abstract class KeymasterUtils { * @param userAuthenticationValidityDurationSeconds duration of time (seconds) for which user * authentication is valid as authorization for using the key or {@code -1} if every * use of the key needs authorization. + * + * @throws IllegalStateException if user authentication is required but the system is in a wrong + * state (e.g., secure lock screen not set up) for generating or importing keys that + * require user authentication. */ public static void addUserAuthArgs(KeymasterArguments args, boolean userAuthenticationRequired, |