Make NONEwithECDSA truncate input when necessary.

Keymaster's implementation of ECDSA with digest NONE rejects input
longer than group size in bytes. RI's NONEwithECDSA accepts inputs
of arbitrary length by truncating them to the above size. This CL
makes Android Keystore's NONEwithECDSA do the truncation to keep
the JCA and Keymaster happy.

The change is inside AndroidKeyStoreECDSASignatureSpi$NONE. All other
small modifications are for supporting that change by making it
possible for AndroidKeyStoreSignatureSpiBase to pass in the signature
being verified into KeyStoreCryptoOperationStreamer. This in turn is
needed to make it possible for NONEwithECDSA implementation to provide
a wrapper streamer which truncates input.

Bug: 22030217
Change-Id: I26064f6df37ef8c631d70a36a356aa0b76a9ad29

8 files changed, 147 insertions, 44 deletions

diff --git a/keystore/java/android/security/keystore/AndroidKeyStoreAuthenticatedAESCipherSpi.java b/keystore/java/android/security/keystore/AndroidKeyStoreAuthenticatedAESCipherSpi.java
index 83dad0e..6411066 100644
--- a/keystore/java/android/security/keystore/AndroidKeyStoreAuthenticatedAESCipherSpi.java
+++ b/keystore/java/android/security/keystore/AndroidKeyStoreAuthenticatedAESCipherSpi.java
@@ -363,8 +363,9 @@ abstract class AndroidKeyStoreAuthenticatedAESCipherSpi extends AndroidKeyStoreC
 
         @Override
         public byte[] doFinal(byte[] input, int inputOffset, int inputLength,
-                byte[] additionalEntropy) throws KeyStoreException {
-            byte[] output = mDelegate.doFinal(input, inputOffset, inputLength, additionalEntropy);
+                byte[] signature, byte[] additionalEntropy) throws KeyStoreException {
+            byte[] output = mDelegate.doFinal(input, inputOffset, inputLength, signature,
+                    additionalEntropy);
             if (output != null) {
                 try {
                     mBufferedOutput.write(output);
@@ -425,7 +426,7 @@ abstract class AndroidKeyStoreAuthenticatedAESCipherSpi extends AndroidKeyStoreC
         }
 
         @Override
-        public OperationResult finish(byte[] additionalEntropy) {
+        public OperationResult finish(byte[] signature, byte[] additionalEntropy) {
             if ((additionalEntropy != null) && (additionalEntropy.length > 0)) {
                 throw new ProviderException("AAD stream does not support additional entropy");
             }
diff --git a/keystore/java/android/security/keystore/AndroidKeyStoreCipherSpiBase.java b/keystore/java/android/security/keystore/AndroidKeyStoreCipherSpiBase.java
index 83131ed..50f3ed4 100644
--- a/keystore/java/android/security/keystore/AndroidKeyStoreCipherSpiBase.java
+++ b/keystore/java/android/security/keystore/AndroidKeyStoreCipherSpiBase.java
@@ -353,6 +353,7 @@ abstract class AndroidKeyStoreCipherSpiBase extends CipherSpi implements KeyStor
             try {
                 output = mAdditionalAuthenticationDataStreamer.doFinal(
                         EmptyArray.BYTE, 0, 0,
+                        null, // no signature
                         null // no additional entropy needed flushing AAD
                         );
             } finally {
@@ -469,7 +470,10 @@ abstract class AndroidKeyStoreCipherSpiBase extends CipherSpi implements KeyStor
             byte[] additionalEntropy =
                     KeyStoreCryptoOperationUtils.getRandomBytesToMixIntoKeystoreRng(
                             mRng, getAdditionalEntropyAmountForFinish());
-            output = mMainDataStreamer.doFinal(input, inputOffset, inputLen, additionalEntropy);
+            output = mMainDataStreamer.doFinal(
+                    input, inputOffset, inputLen,
+                    null, // no signature involved
+                    additionalEntropy);
         } catch (KeyStoreException e) {
             switch (e.getErrorCode()) {
                 case KeymasterDefs.KM_ERROR_INVALID_INPUT_LENGTH:
diff --git a/keystore/java/android/security/keystore/AndroidKeyStoreECDSASignatureSpi.java b/keystore/java/android/security/keystore/AndroidKeyStoreECDSASignatureSpi.java
index f80feef..10aab7e 100644
--- a/keystore/java/android/security/keystore/AndroidKeyStoreECDSASignatureSpi.java
+++ b/keystore/java/android/security/keystore/AndroidKeyStoreECDSASignatureSpi.java
@@ -17,11 +17,16 @@
 package android.security.keystore;
 
 import android.annotation.NonNull;
+import android.os.IBinder;
 import android.security.KeyStore;
+import android.security.KeyStoreException;
 import android.security.keymaster.KeyCharacteristics;
 import android.security.keymaster.KeymasterArguments;
 import android.security.keymaster.KeymasterDefs;
 
+import libcore.util.EmptyArray;
+
+import java.io.ByteArrayOutputStream;
 import java.security.InvalidKeyException;
 import java.security.SignatureSpi;
 
@@ -36,6 +41,71 @@ abstract class AndroidKeyStoreECDSASignatureSpi extends AndroidKeyStoreSignature
         public NONE() {
             super(KeymasterDefs.KM_DIGEST_NONE);
         }
+
+        @Override
+        protected KeyStoreCryptoOperationStreamer createMainDataStreamer(KeyStore keyStore,
+                IBinder operationToken) {
+            return new TruncateToFieldSizeMessageStreamer(
+                    super.createMainDataStreamer(keyStore, operationToken),
+                    getGroupSizeBits());
+        }
+
+        /**
+         * Streamer which buffers all input, then truncates it to field size, and then sends it into
+         * KeyStore via the provided delegate streamer.
+         */
+        private static class TruncateToFieldSizeMessageStreamer
+                implements KeyStoreCryptoOperationStreamer {
+
+            private final KeyStoreCryptoOperationStreamer mDelegate;
+            private final int mGroupSizeBits;
+            private final ByteArrayOutputStream mInputBuffer = new ByteArrayOutputStream();
+            private long mConsumedInputSizeBytes;
+
+            private TruncateToFieldSizeMessageStreamer(
+                    KeyStoreCryptoOperationStreamer delegate,
+                    int groupSizeBits) {
+                mDelegate = delegate;
+                mGroupSizeBits = groupSizeBits;
+            }
+
+            @Override
+            public byte[] update(byte[] input, int inputOffset, int inputLength)
+                    throws KeyStoreException {
+                if (inputLength > 0) {
+                    mInputBuffer.write(input, inputOffset, inputLength);
+                    mConsumedInputSizeBytes += inputLength;
+                }
+                return EmptyArray.BYTE;
+            }
+
+            @Override
+            public byte[] doFinal(byte[] input, int inputOffset, int inputLength, byte[] signature,
+                    byte[] additionalEntropy) throws KeyStoreException {
+                if (inputLength > 0) {
+                    mConsumedInputSizeBytes += inputLength;
+                    mInputBuffer.write(input, inputOffset, inputLength);
+                }
+
+                byte[] bufferedInput = mInputBuffer.toByteArray();
+                mInputBuffer.reset();
+                // Truncate input at field size (bytes)
+                return mDelegate.doFinal(bufferedInput,
+                        0,
+                        Math.min(bufferedInput.length, ((mGroupSizeBits + 7) / 8)),
+                        signature, additionalEntropy);
+            }
+
+            @Override
+            public long getConsumedInputSizeBytes() {
+                return mConsumedInputSizeBytes;
+            }
+
+            @Override
+            public long getProducedOutputSizeBytes() {
+                return mDelegate.getProducedOutputSizeBytes();
+            }
+        }
     }
 
     public final static class SHA1 extends AndroidKeyStoreECDSASignatureSpi {
@@ -70,7 +140,7 @@ abstract class AndroidKeyStoreECDSASignatureSpi extends AndroidKeyStoreSignature
 
     private final int mKeymasterDigest;
 
-    private int mGroupSizeBytes = -1;
+    private int mGroupSizeBits = -1;
 
     AndroidKeyStoreECDSASignatureSpi(int keymasterDigest) {
         mKeymasterDigest = keymasterDigest;
@@ -95,14 +165,14 @@ abstract class AndroidKeyStoreECDSASignatureSpi extends AndroidKeyStoreSignature
         } else if (keySizeBits > Integer.MAX_VALUE) {
             throw new InvalidKeyException("Key too large: " + keySizeBits + " bits");
         }
-        mGroupSizeBytes = (int) ((keySizeBits + 7) / 8);
+        mGroupSizeBits = (int) keySizeBits;
 
         super.initKey(key);
     }
 
     @Override
     protected final void resetAll() {
-        mGroupSizeBytes = -1;
+        mGroupSizeBits = -1;
         super.resetAll();
     }
 
@@ -112,14 +182,21 @@ abstract class AndroidKeyStoreECDSASignatureSpi extends AndroidKeyStoreSignature
     }
 
     @Override
-    protected void addAlgorithmSpecificParametersToBegin(
+    protected final void addAlgorithmSpecificParametersToBegin(
             @NonNull KeymasterArguments keymasterArgs) {
         keymasterArgs.addEnum(KeymasterDefs.KM_TAG_ALGORITHM, KeymasterDefs.KM_ALGORITHM_EC);
         keymasterArgs.addEnum(KeymasterDefs.KM_TAG_DIGEST, mKeymasterDigest);
     }
 
     @Override
-    protected int getAdditionalEntropyAmountForSign() {
-        return mGroupSizeBytes;
+    protected final int getAdditionalEntropyAmountForSign() {
+        return (mGroupSizeBits + 7) / 8;
+    }
+
+    protected final int getGroupSizeBits() {
+        if (mGroupSizeBits == -1) {
+            throw new IllegalStateException("Not initialized");
+        }
+        return mGroupSizeBits;
     }
 }
diff --git a/keystore/java/android/security/keystore/AndroidKeyStoreHmacSpi.java b/keystore/java/android/security/keystore/AndroidKeyStoreHmacSpi.java
index cc858d3..d20e3af 100644
--- a/keystore/java/android/security/keystore/AndroidKeyStoreHmacSpi.java
+++ b/keystore/java/android/security/keystore/AndroidKeyStoreHmacSpi.java
@@ -234,6 +234,7 @@ public abstract class AndroidKeyStoreHmacSpi extends MacSpi implements KeyStoreC
         try {
             result = mChunkedStreamer.doFinal(
                     null, 0, 0,
+                    null, // no signature provided -- this invocation will generate one
                     null // no additional entropy needed -- HMAC is deterministic
                     );
         } catch (KeyStoreException e) {
diff --git a/keystore/java/android/security/keystore/AndroidKeyStoreRSACipherSpi.java b/keystore/java/android/security/keystore/AndroidKeyStoreRSACipherSpi.java
index 1d4ca40..8e58195 100644
--- a/keystore/java/android/security/keystore/AndroidKeyStoreRSACipherSpi.java
+++ b/keystore/java/android/security/keystore/AndroidKeyStoreRSACipherSpi.java
@@ -150,8 +150,7 @@ abstract class AndroidKeyStoreRSACipherSpi extends AndroidKeyStoreCipherSpiBase
 
             @Override
             public byte[] doFinal(byte[] input, int inputOffset, int inputLength,
-                    byte[] additionalEntropy)
-                    throws KeyStoreException {
+                    byte[] signature, byte[] additionalEntropy) throws KeyStoreException {
                 if (inputLength > 0) {
                     mConsumedInputSizeBytes += inputLength;
                     mInputBuffer.write(input, inputOffset, inputLength);
@@ -174,7 +173,8 @@ abstract class AndroidKeyStoreRSACipherSpi extends AndroidKeyStoreCipherSpiBase
                             "Message size (" + bufferedInput.length + " bytes) must be smaller than"
                             + " modulus (" + mModulusSizeBytes + " bytes)");
                 }
-                return mDelegate.doFinal(paddedInput, 0, paddedInput.length, additionalEntropy);
+                return mDelegate.doFinal(paddedInput, 0, paddedInput.length, signature,
+                        additionalEntropy);
             }
 
             @Override
diff --git a/keystore/java/android/security/keystore/AndroidKeyStoreSignatureSpiBase.java b/keystore/java/android/security/keystore/AndroidKeyStoreSignatureSpiBase.java
index 5cdcc41..76240dd 100644
--- a/keystore/java/android/security/keystore/AndroidKeyStoreSignatureSpiBase.java
+++ b/keystore/java/android/security/keystore/AndroidKeyStoreSignatureSpiBase.java
@@ -58,7 +58,7 @@ abstract class AndroidKeyStoreSignatureSpiBase extends SignatureSpi
      */
     private IBinder mOperationToken;
     private long mOperationHandle;
-    private KeyStoreCryptoOperationChunkedStreamer mMessageStreamer;
+    private KeyStoreCryptoOperationStreamer mMessageStreamer;
 
     /**
      * Encountered exception which could not be immediately thrown because it was encountered inside
@@ -229,9 +229,20 @@ abstract class AndroidKeyStoreSignatureSpiBase extends SignatureSpi
             throw new ProviderException("Keystore returned invalid operation handle");
         }
 
-        mMessageStreamer = new KeyStoreCryptoOperationChunkedStreamer(
+        mMessageStreamer = createMainDataStreamer(mKeyStore, opResult.token);
+    }
+
+    /**
+     * Creates a streamer which sends the message to be signed/verified into the provided KeyStore
+     *
+     * <p>This implementation returns a working streamer.
+     */
+    @NonNull
+    protected KeyStoreCryptoOperationStreamer createMainDataStreamer(
+            KeyStore keyStore, IBinder operationToken) {
+        return new KeyStoreCryptoOperationChunkedStreamer(
                 new KeyStoreCryptoOperationChunkedStreamer.MainDataStream(
-                        mKeyStore, opResult.token));
+                        keyStore, operationToken));
     }
 
     @Override
@@ -314,7 +325,10 @@ abstract class AndroidKeyStoreSignatureSpiBase extends SignatureSpi
             byte[] additionalEntropy =
                     KeyStoreCryptoOperationUtils.getRandomBytesToMixIntoKeystoreRng(
                             appRandom, getAdditionalEntropyAmountForSign());
-            signature = mMessageStreamer.doFinal(EmptyArray.BYTE, 0, 0, additionalEntropy);
+            signature = mMessageStreamer.doFinal(
+                    EmptyArray.BYTE, 0, 0,
+                    null, // no signature provided -- it'll be generated by this invocation
+                    additionalEntropy);
         } catch (InvalidKeyException | KeyStoreException e) {
             throw new SignatureException(e);
         }
@@ -329,31 +343,37 @@ abstract class AndroidKeyStoreSignatureSpiBase extends SignatureSpi
             throw new SignatureException(mCachedException);
         }
 
-        boolean result;
         try {
             ensureKeystoreOperationInitialized();
-            mMessageStreamer.flush();
-            OperationResult opResult = mKeyStore.finish(mOperationToken, null, signature);
-            if (opResult == null) {
-                throw new KeyStoreConnectException();
+        } catch (InvalidKeyException e) {
+            throw new SignatureException(e);
+        }
+
+        boolean verified;
+        try {
+            byte[] output = mMessageStreamer.doFinal(
+                    EmptyArray.BYTE, 0, 0,
+                    signature,
+                    null // no additional entropy needed -- verification is deterministic
+                    );
+            if (output.length != 0) {
+                throw new ProviderException(
+                        "Signature verification unexpected produced output: " + output.length
+                        + " bytes");
             }
-            switch (opResult.resultCode) {
-                case KeyStore.NO_ERROR:
-                    result = true;
-                    break;
+            verified = true;
+        } catch (KeyStoreException e) {
+            switch (e.getErrorCode()) {
                 case KeymasterDefs.KM_ERROR_VERIFICATION_FAILED:
-                    result = false;
+                    verified = false;
                     break;
                 default:
-                    throw new SignatureException(
-                            KeyStore.getKeyStoreException(opResult.resultCode));
+                    throw new SignatureException(e);
             }
-        } catch (InvalidKeyException | KeyStoreException e) {
-            throw new SignatureException(e);
         }
 
         resetWhilePreservingInitState();
-        return result;
+        return verified;
     }
 
     @Override
diff --git a/keystore/java/android/security/keystore/KeyStoreCryptoOperationChunkedStreamer.java b/keystore/java/android/security/keystore/KeyStoreCryptoOperationChunkedStreamer.java
index 894d52a..ea0f4b9 100644
--- a/keystore/java/android/security/keystore/KeyStoreCryptoOperationChunkedStreamer.java
+++ b/keystore/java/android/security/keystore/KeyStoreCryptoOperationChunkedStreamer.java
@@ -35,8 +35,8 @@ import java.io.IOException;
  * amount of data in one go because the operations are marshalled via Binder. Secondly, the update
  * operation may consume less data than provided, in which case the caller has to buffer the
  * remainder for next time. The helper exposes {@link #update(byte[], int, int) update} and
- * {@link #doFinal(byte[], int, int, byte[]) doFinal} operations which can be used to conveniently
- * implement various JCA crypto primitives.
+ * {@link #doFinal(byte[], int, int, byte[], byte[]) doFinal} operations which can be used to
+ * conveniently implement various JCA crypto primitives.
  *
  * <p>Bidirectional chunked streaming of data via a KeyStore crypto operation is abstracted away as
  * a {@link Stream} to avoid having this class deal with operation tokens and occasional additional
@@ -60,7 +60,7 @@ class KeyStoreCryptoOperationChunkedStreamer implements KeyStoreCryptoOperationS
          * Returns the result of the KeyStore {@code finish} operation or null if keystore couldn't
          * be reached.
          */
-        OperationResult finish(byte[] additionalEntropy);
+        OperationResult finish(byte[] siganture, byte[] additionalEntropy);
     }
 
     // Binder buffer is about 1MB, but it's shared between all active transactions of the process.
@@ -201,8 +201,8 @@ class KeyStoreCryptoOperationChunkedStreamer implements KeyStoreCryptoOperationS
     }
 
     @Override
-    public byte[] doFinal(byte[] input, int inputOffset, int inputLength, byte[] additionalEntropy)
-            throws KeyStoreException {
+    public byte[] doFinal(byte[] input, int inputOffset, int inputLength,
+            byte[] signature, byte[] additionalEntropy) throws KeyStoreException {
         if (inputLength == 0) {
             // No input provided -- simplify the rest of the code
             input = EmptyArray.BYTE;
@@ -213,7 +213,7 @@ class KeyStoreCryptoOperationChunkedStreamer implements KeyStoreCryptoOperationS
         byte[] output = update(input, inputOffset, inputLength);
         output = ArrayUtils.concat(output, flush());
 
-        OperationResult opResult = mKeyStoreStream.finish(additionalEntropy);
+        OperationResult opResult = mKeyStoreStream.finish(signature, additionalEntropy);
         if (opResult == null) {
             throw new KeyStoreConnectException();
         } else if (opResult.resultCode != KeyStore.NO_ERROR) {
@@ -286,8 +286,8 @@ class KeyStoreCryptoOperationChunkedStreamer implements KeyStoreCryptoOperationS
         }
 
         @Override
-        public OperationResult finish(byte[] additionalEntropy) {
-            return mKeyStore.finish(mOperationToken, null, null, additionalEntropy);
+        public OperationResult finish(byte[] signature, byte[] additionalEntropy) {
+            return mKeyStore.finish(mOperationToken, null, signature, additionalEntropy);
         }
     }
 }
diff --git a/keystore/java/android/security/keystore/KeyStoreCryptoOperationStreamer.java b/keystore/java/android/security/keystore/KeyStoreCryptoOperationStreamer.java
index 897bd71..062c2d4 100644
--- a/keystore/java/android/security/keystore/KeyStoreCryptoOperationStreamer.java
+++ b/keystore/java/android/security/keystore/KeyStoreCryptoOperationStreamer.java
@@ -28,15 +28,15 @@ import android.security.KeyStoreException;
  * amount of data in one go because the operations are marshalled via Binder. Secondly, the update
  * operation may consume less data than provided, in which case the caller has to buffer the
  * remainder for next time. The helper exposes {@link #update(byte[], int, int) update} and
- * {@link #doFinal(byte[], int, int, byte[]) doFinal} operations which can be used to conveniently
- * implement various JCA crypto primitives.
+ * {@link #doFinal(byte[], int, int, byte[], byte[]) doFinal} operations which can be used to
+ * conveniently implement various JCA crypto primitives.
  *
  * @hide
  */
 interface KeyStoreCryptoOperationStreamer {
     byte[] update(byte[] input, int inputOffset, int inputLength) throws KeyStoreException;
-    byte[] doFinal(byte[] input, int inputOffset, int inputLength, byte[] additionalEntropy)
-            throws KeyStoreException;
+    byte[] doFinal(byte[] input, int inputOffset, int inputLength, byte[] signature,
+            byte[] additionalEntropy) throws KeyStoreException;
     long getConsumedInputSizeBytes();
     long getProducedOutputSizeBytes();
 }