diff options
| author | 2015-04-21 15:17:24 -0700 | |
|---|---|---|
| committer | 2015-04-24 10:54:45 -0700 | |
| commit | ad9ba10ecda10c14e46d00f40fc3e431cc2d9bc2 (patch) | |
| tree | 2c1234ccb7344f0a70d62e3e6b9066158c508e85 | |
| parent | 71223ebe1b2264b7463a02c8dafd779eb3b8c210 (diff) | |
No runtime exceptions during normal use of AndroidKeyStore crypto.
This changes the implementation of AndroidKeyStore-backed Cipher and
Mac to avoid throwing runtime exceptions during normal use. Runtime
exceptions will now be thrown only due to truly exceptional and
unrecoverable errors (e.g., keystore unreachable, or crypto primitive
not initialized).
This also changes the implementation of Cipher to cache any errors
encountered in Cipher.update until Cipher.doFinal which then throws
them as checked exceptions.
Bug: 20525947
Change-Id: I3c4ad57fe70abfbb817a79402f722a0208660727
10 files changed, 102 insertions, 95 deletions
diff --git a/keystore/java/android/security/CryptoOperationException.java b/keystore/java/android/security/CryptoOperationException.java deleted file mode 100644 index 00c142fbec9d..000000000000 --- a/keystore/java/android/security/CryptoOperationException.java +++ /dev/null @@ -1,61 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package android.security; - -/** - * Base class for exceptions during cryptographic operations which cannot throw a suitable checked - * exception. - * - * <p>The contract of the majority of crypto primitives/operations (e.g. {@code Cipher} or - * {@code Signature}) is that they can throw a checked exception during initialization, but are not - * permitted to throw a checked exception during operation. Because crypto operations can fail - * for a variety of reasons after initialization, this base class provides type-safety for unchecked - * exceptions that may be thrown in those cases. - * - * @hide - */ -public class CryptoOperationException extends RuntimeException { - - /** - * Constructs a new {@code CryptoOperationException} without detail message and cause. - */ - public CryptoOperationException() { - super(); - } - - /** - * Constructs a new {@code CryptoOperationException} with the provided detail message and no - * cause. - */ - public CryptoOperationException(String message) { - super(message); - } - - /** - * Constructs a new {@code CryptoOperationException} with the provided detail message and cause. - */ - public CryptoOperationException(String message, Throwable cause) { - super(message, cause); - } - - /** - * Constructs a new {@code CryptoOperationException} with the provided cause. - */ - public CryptoOperationException(Throwable cause) { - super(cause); - } -} diff --git a/keystore/java/android/security/KeyExpiredException.java b/keystore/java/android/security/KeyExpiredException.java index 35a5accbc94c..e64bffa481db 100644 --- a/keystore/java/android/security/KeyExpiredException.java +++ b/keystore/java/android/security/KeyExpiredException.java @@ -16,13 +16,15 @@ package android.security; +import java.security.InvalidKeyException; + /** * Indicates that a cryptographic operation failed because the employed key's validity end date * is in the past. * * @hide */ -public class KeyExpiredException extends CryptoOperationException { +public class KeyExpiredException extends InvalidKeyException { /** * Constructs a new {@code KeyExpiredException} without detail message and cause. diff --git a/keystore/java/android/security/KeyNotYetValidException.java b/keystore/java/android/security/KeyNotYetValidException.java index f1c2cac672b9..d36d80c80ca6 100644 --- a/keystore/java/android/security/KeyNotYetValidException.java +++ b/keystore/java/android/security/KeyNotYetValidException.java @@ -16,13 +16,15 @@ package android.security; +import java.security.InvalidKeyException; + /** * Indicates that a cryptographic operation failed because the employed key's validity start date * is in the future. * * @hide */ -public class KeyNotYetValidException extends CryptoOperationException { +public class KeyNotYetValidException extends InvalidKeyException { /** * Constructs a new {@code KeyNotYetValidException} without detail message and cause. diff --git a/keystore/java/android/security/KeyStore.java b/keystore/java/android/security/KeyStore.java index 84a664e30163..ff8534df3f4a 100644 --- a/keystore/java/android/security/KeyStore.java +++ b/keystore/java/android/security/KeyStore.java @@ -30,6 +30,7 @@ import android.security.keymaster.KeymasterDefs; import android.security.keymaster.OperationResult; import android.util.Log; +import java.security.InvalidKeyException; import java.util.Locale; /** @@ -508,7 +509,11 @@ public class KeyStore { } } - public static KeyStoreException getKeyStoreException(int errorCode) { + /** + * Returns a {@link KeyStoreException} corresponding to the provided keystore/keymaster error + * code. + */ + static KeyStoreException getKeyStoreException(int errorCode) { if (errorCode > 0) { // KeyStore layer error switch (errorCode) { @@ -544,7 +549,11 @@ public class KeyStore { } } - public static CryptoOperationException getCryptoOperationException(KeyStoreException e) { + /** + * Returns an {@link InvalidKeyException} corresponding to the provided + * {@link KeyStoreException}. + */ + static InvalidKeyException getInvalidKeyException(KeyStoreException e) { switch (e.getErrorCode()) { case KeymasterDefs.KM_ERROR_KEY_EXPIRED: return new KeyExpiredException(); @@ -553,11 +562,15 @@ public class KeyStore { case KeymasterDefs.KM_ERROR_KEY_USER_NOT_AUTHENTICATED: return new UserNotAuthenticatedException(); default: - return new CryptoOperationException("Crypto operation failed", e); + return new InvalidKeyException("Keystore operation failed", e); } } - public static CryptoOperationException getCryptoOperationException(int errorCode) { - return getCryptoOperationException(getKeyStoreException(errorCode)); + /** + * Returns an {@link InvalidKeyException} corresponding to the provided keystore/keymaster error + * code. + */ + static InvalidKeyException getInvalidKeyException(int errorCode) { + return getInvalidKeyException(getKeyStoreException(errorCode)); } } diff --git a/keystore/java/android/security/KeyStoreCipherSpi.java b/keystore/java/android/security/KeyStoreCipherSpi.java index 1f8d8ec4ff43..3b13e83520bb 100644 --- a/keystore/java/android/security/KeyStoreCipherSpi.java +++ b/keystore/java/android/security/KeyStoreCipherSpi.java @@ -136,6 +136,14 @@ public abstract class KeyStoreCipherSpi extends CipherSpi implements KeyStoreCry private Long mOperationHandle; private KeyStoreCryptoOperationChunkedStreamer mMainDataStreamer; + /** + * Encountered exception which could not be immediately thrown because it was encountered inside + * a method that does not throw checked exception. This exception will be thrown from + * {@code engineDoFinal}. Once such an exception is encountered, {@code engineUpdate} and + * {@code engineDoFinal} start ignoring input data. + */ + private Exception mCachedException; + protected KeyStoreCipherSpi( int keymasterAlgorithm, int keymasterBlockMode, @@ -158,7 +166,11 @@ public abstract class KeyStoreCipherSpi extends CipherSpi implements KeyStoreCry try { init(opmode, key, random); initAlgorithmSpecificParameters(); - ensureKeystoreOperationInitialized(); + try { + ensureKeystoreOperationInitialized(); + } catch (InvalidAlgorithmParameterException e) { + throw new InvalidKeyException(e); + } success = true; } finally { if (!success) { @@ -236,6 +248,7 @@ public abstract class KeyStoreCipherSpi extends CipherSpi implements KeyStoreCry mOperationToken = null; mOperationHandle = null; mMainDataStreamer = null; + mCachedException = null; } private void resetWhilePreservingInitState() { @@ -247,12 +260,17 @@ public abstract class KeyStoreCipherSpi extends CipherSpi implements KeyStoreCry mOperationHandle = null; mMainDataStreamer = null; mAdditionalEntropyForBegin = null; + mCachedException = null; } - private void ensureKeystoreOperationInitialized() { + private void ensureKeystoreOperationInitialized() throws InvalidKeyException, + InvalidAlgorithmParameterException { if (mMainDataStreamer != null) { return; } + if (mCachedException != null) { + return; + } if (mKey == null) { throw new IllegalStateException("Not initialized"); } @@ -281,11 +299,15 @@ public abstract class KeyStoreCipherSpi extends CipherSpi implements KeyStoreCry if (opResult == null) { throw new KeyStoreConnectException(); } else if (opResult.resultCode != KeyStore.NO_ERROR) { - throw KeyStore.getCryptoOperationException(opResult.resultCode); + switch (opResult.resultCode) { + case KeymasterDefs.KM_ERROR_INVALID_NONCE: + throw new InvalidAlgorithmParameterException("Invalid IV"); + } + throw KeyStore.getInvalidKeyException(opResult.resultCode); } if (opResult.token == null) { - throw new CryptoOperationException("Keystore returned null operation token"); + throw new IllegalStateException("Keystore returned null operation token"); } mOperationToken = opResult.token; mOperationHandle = opResult.operationHandle; @@ -299,7 +321,15 @@ public abstract class KeyStoreCipherSpi extends CipherSpi implements KeyStoreCry @Override protected byte[] engineUpdate(byte[] input, int inputOffset, int inputLen) { - ensureKeystoreOperationInitialized(); + if (mCachedException != null) { + return null; + } + try { + ensureKeystoreOperationInitialized(); + } catch (InvalidKeyException | InvalidAlgorithmParameterException e) { + mCachedException = e; + return null; + } if (inputLen == 0) { return null; @@ -309,7 +339,8 @@ public abstract class KeyStoreCipherSpi extends CipherSpi implements KeyStoreCry try { output = mMainDataStreamer.update(input, inputOffset, inputLen); } catch (KeyStoreException e) { - throw KeyStore.getCryptoOperationException(e); + mCachedException = e; + return null; } if (output.length == 0) { @@ -338,7 +369,16 @@ public abstract class KeyStoreCipherSpi extends CipherSpi implements KeyStoreCry @Override protected byte[] engineDoFinal(byte[] input, int inputOffset, int inputLen) throws IllegalBlockSizeException, BadPaddingException { - ensureKeystoreOperationInitialized(); + if (mCachedException != null) { + throw (IllegalBlockSizeException) + new IllegalBlockSizeException().initCause(mCachedException); + } + + try { + ensureKeystoreOperationInitialized(); + } catch (InvalidKeyException | InvalidAlgorithmParameterException e) { + throw (IllegalBlockSizeException) new IllegalBlockSizeException().initCause(e); + } byte[] output; try { @@ -352,7 +392,7 @@ public abstract class KeyStoreCipherSpi extends CipherSpi implements KeyStoreCry case KeymasterDefs.KM_ERROR_VERIFICATION_FAILED: throw new AEADBadTagException(); default: - throw KeyStore.getCryptoOperationException(e); + throw (IllegalBlockSizeException) new IllegalBlockSizeException().initCause(e); } } @@ -613,11 +653,11 @@ public abstract class KeyStoreCipherSpi extends CipherSpi implements KeyStoreCry if (mIv == null) { mIv = returnedIv; } else if ((returnedIv != null) && (!Arrays.equals(returnedIv, mIv))) { - throw new CryptoOperationException("IV in use differs from provided IV"); + throw new IllegalStateException("IV in use differs from provided IV"); } } else { if (returnedIv != null) { - throw new CryptoOperationException( + throw new IllegalStateException( "IV in use despite IV not being used by this transformation"); } } diff --git a/keystore/java/android/security/KeyStoreConnectException.java b/keystore/java/android/security/KeyStoreConnectException.java index 8ed6e04ddfe8..1aa3aecc0b1e 100644 --- a/keystore/java/android/security/KeyStoreConnectException.java +++ b/keystore/java/android/security/KeyStoreConnectException.java @@ -21,7 +21,7 @@ package android.security; * * @hide */ -public class KeyStoreConnectException extends CryptoOperationException { +public class KeyStoreConnectException extends IllegalStateException { public KeyStoreConnectException() { super("Failed to communicate with keystore service"); } diff --git a/keystore/java/android/security/KeyStoreCryptoOperationChunkedStreamer.java b/keystore/java/android/security/KeyStoreCryptoOperationChunkedStreamer.java index aafd2fa97aff..06191994af62 100644 --- a/keystore/java/android/security/KeyStoreCryptoOperationChunkedStreamer.java +++ b/keystore/java/android/security/KeyStoreCryptoOperationChunkedStreamer.java @@ -136,7 +136,7 @@ public class KeyStoreCryptoOperationChunkedStreamer { // More input is available, but it wasn't included into the previous chunk // because the chunk reached its maximum permitted size. // Shouldn't have happened. - throw new CryptoOperationException("Nothing consumed from max-sized chunk: " + throw new IllegalStateException("Nothing consumed from max-sized chunk: " + chunk.length + " bytes"); } mBuffered = chunk; @@ -148,7 +148,7 @@ public class KeyStoreCryptoOperationChunkedStreamer { mBufferedOffset = opResult.inputConsumed; mBufferedLength = chunk.length - opResult.inputConsumed; } else { - throw new CryptoOperationException("Consumed more than provided: " + throw new IllegalStateException("Consumed more than provided: " + opResult.inputConsumed + ", provided: " + chunk.length); } @@ -160,7 +160,7 @@ public class KeyStoreCryptoOperationChunkedStreamer { try { bufferedOutput.write(opResult.output); } catch (IOException e) { - throw new CryptoOperationException("Failed to buffer output", e); + throw new IllegalStateException("Failed to buffer output", e); } } } else { @@ -173,7 +173,7 @@ public class KeyStoreCryptoOperationChunkedStreamer { try { bufferedOutput.write(opResult.output); } catch (IOException e) { - throw new CryptoOperationException("Failed to buffer output", e); + throw new IllegalStateException("Failed to buffer output", e); } return bufferedOutput.toByteArray(); } @@ -233,10 +233,10 @@ public class KeyStoreCryptoOperationChunkedStreamer { } if (opResult.inputConsumed < chunk.length) { - throw new CryptoOperationException("Keystore failed to consume all input. Provided: " + throw new IllegalStateException("Keystore failed to consume all input. Provided: " + chunk.length + ", consumed: " + opResult.inputConsumed); } else if (opResult.inputConsumed > chunk.length) { - throw new CryptoOperationException("Keystore consumed more input than provided" + throw new IllegalStateException("Keystore consumed more input than provided" + " . Provided: " + chunk.length + ", consumed: " + opResult.inputConsumed); } diff --git a/keystore/java/android/security/KeyStoreHmacSpi.java b/keystore/java/android/security/KeyStoreHmacSpi.java index f8b6fef2a90f..175369ce91d0 100644 --- a/keystore/java/android/security/KeyStoreHmacSpi.java +++ b/keystore/java/android/security/KeyStoreHmacSpi.java @@ -147,7 +147,7 @@ public abstract class KeyStoreHmacSpi extends MacSpi implements KeyStoreCryptoOp resetWhilePreservingInitState(); } - private void ensureKeystoreOperationInitialized() { + private void ensureKeystoreOperationInitialized() throws InvalidKeyException { if (mChunkedStreamer != null) { return; } @@ -169,10 +169,10 @@ public abstract class KeyStoreHmacSpi extends MacSpi implements KeyStoreCryptoOp if (opResult == null) { throw new KeyStoreConnectException(); } else if (opResult.resultCode != KeyStore.NO_ERROR) { - throw KeyStore.getCryptoOperationException(opResult.resultCode); + throw KeyStore.getInvalidKeyException(opResult.resultCode); } if (opResult.token == null) { - throw new CryptoOperationException("Keystore returned null operation token"); + throw new IllegalStateException("Keystore returned null operation token"); } mOperationToken = opResult.token; mOperationHandle = opResult.operationHandle; @@ -188,28 +188,36 @@ public abstract class KeyStoreHmacSpi extends MacSpi implements KeyStoreCryptoOp @Override protected void engineUpdate(byte[] input, int offset, int len) { - ensureKeystoreOperationInitialized(); + try { + ensureKeystoreOperationInitialized(); + } catch (InvalidKeyException e) { + throw new IllegalStateException("Failed to reinitialize MAC", e); + } byte[] output; try { output = mChunkedStreamer.update(input, offset, len); } catch (KeyStoreException e) { - throw KeyStore.getCryptoOperationException(e); + throw new IllegalStateException("Keystore operation failed", e); } if ((output != null) && (output.length != 0)) { - throw new CryptoOperationException("Update operation unexpectedly produced output"); + throw new IllegalStateException("Update operation unexpectedly produced output"); } } @Override protected byte[] engineDoFinal() { - ensureKeystoreOperationInitialized(); + try { + ensureKeystoreOperationInitialized(); + } catch (InvalidKeyException e) { + throw new IllegalStateException("Failed to reinitialize MAC", e); + } byte[] result; try { result = mChunkedStreamer.doFinal(null, 0, 0); } catch (KeyStoreException e) { - throw KeyStore.getCryptoOperationException(e); + throw new IllegalStateException("Keystore operation failed", e); } resetWhilePreservingInitState(); diff --git a/keystore/java/android/security/KeyStoreKeyGeneratorSpi.java b/keystore/java/android/security/KeyStoreKeyGeneratorSpi.java index 87e7ee644444..dde3b8fe3771 100644 --- a/keystore/java/android/security/KeyStoreKeyGeneratorSpi.java +++ b/keystore/java/android/security/KeyStoreKeyGeneratorSpi.java @@ -200,7 +200,8 @@ public abstract class KeyStoreKeyGeneratorSpi extends KeyGeneratorSpi { int errorCode = mKeyStore.generateKey( keyAliasInKeystore, args, additionalEntropy, flags, new KeyCharacteristics()); if (errorCode != KeyStore.NO_ERROR) { - throw KeyStore.getCryptoOperationException(errorCode); + throw new IllegalStateException( + "Keystore operation failed", KeyStore.getKeyStoreException(errorCode)); } String keyAlgorithmJCA = KeymasterUtils.getJcaSecretKeyAlgorithm(mKeymasterAlgorithm, mKeymasterDigest); diff --git a/keystore/java/android/security/UserNotAuthenticatedException.java b/keystore/java/android/security/UserNotAuthenticatedException.java index e6342ef79673..d0410b8f0171 100644 --- a/keystore/java/android/security/UserNotAuthenticatedException.java +++ b/keystore/java/android/security/UserNotAuthenticatedException.java @@ -16,13 +16,15 @@ package android.security; +import java.security.InvalidKeyException; + /** * Indicates that a cryptographic operation could not be performed because the user has not been * authenticated recently enough. * * @hide */ -public class UserNotAuthenticatedException extends CryptoOperationException { +public class UserNotAuthenticatedException extends InvalidKeyException { /** * Constructs a new {@code UserNotAuthenticatedException} without detail message and cause. |