OSDN Git Service

No runtime exceptions during normal use of AndroidKeyStore crypto.
authorAlex Klyubin <klyubin@google.com>
Tue, 21 Apr 2015 22:17:24 +0000 (15:17 -0700)
committerAlex Klyubin <klyubin@google.com>
Fri, 24 Apr 2015 17:54:45 +0000 (10:54 -0700)
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

keystore/java/android/security/CryptoOperationException.java [deleted file]
keystore/java/android/security/KeyExpiredException.java
keystore/java/android/security/KeyNotYetValidException.java
keystore/java/android/security/KeyStore.java
keystore/java/android/security/KeyStoreCipherSpi.java
keystore/java/android/security/KeyStoreConnectException.java
keystore/java/android/security/KeyStoreCryptoOperationChunkedStreamer.java
keystore/java/android/security/KeyStoreHmacSpi.java
keystore/java/android/security/KeyStoreKeyGeneratorSpi.java
keystore/java/android/security/UserNotAuthenticatedException.java

diff --git a/keystore/java/android/security/CryptoOperationException.java b/keystore/java/android/security/CryptoOperationException.java
deleted file mode 100644 (file)
index 00c142f..0000000
+++ /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);
-    }
-}
index 35a5acc..e64bffa 100644 (file)
 
 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.
index f1c2cac..d36d80c 100644 (file)
 
 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.
index 84a664e..ff8534d 100644 (file)
@@ -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));
     }
 }
index 1f8d8ec..3b13e83 100644 (file)
@@ -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");
             }
         }
index 8ed6e04..1aa3aec 100644 (file)
@@ -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");
     }
index aafd2fa..0619199 100644 (file)
@@ -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);
         }
 
index f8b6fef..175369c 100644 (file)
@@ -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();
index 87e7ee6..dde3b8f 100644 (file)
@@ -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);
index e6342ef..d0410b8 100644 (file)
 
 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.