summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Alex Klyubin <klyubin@google.com> 2015-04-21 15:17:24 -0700
committer Alex Klyubin <klyubin@google.com> 2015-04-24 10:54:45 -0700
commitad9ba10ecda10c14e46d00f40fc3e431cc2d9bc2 (patch)
tree2c1234ccb7344f0a70d62e3e6b9066158c508e85
parent71223ebe1b2264b7463a02c8dafd779eb3b8c210 (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
-rw-r--r--keystore/java/android/security/CryptoOperationException.java61
-rw-r--r--keystore/java/android/security/KeyExpiredException.java4
-rw-r--r--keystore/java/android/security/KeyNotYetValidException.java4
-rw-r--r--keystore/java/android/security/KeyStore.java23
-rw-r--r--keystore/java/android/security/KeyStoreCipherSpi.java60
-rw-r--r--keystore/java/android/security/KeyStoreConnectException.java2
-rw-r--r--keystore/java/android/security/KeyStoreCryptoOperationChunkedStreamer.java12
-rw-r--r--keystore/java/android/security/KeyStoreHmacSpi.java24
-rw-r--r--keystore/java/android/security/KeyStoreKeyGeneratorSpi.java3
-rw-r--r--keystore/java/android/security/UserNotAuthenticatedException.java4
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.