Merge "No runtime exceptions during normal use of AndroidKeyStore crypto."

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.