From d23dc502b0a1952887d4453cba98aa2e3d2f5009 Mon Sep 17 00:00:00 2001 From: Alex Klyubin Date: Wed, 24 Jun 2015 12:25:52 -0700 Subject: 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 --- .../AndroidKeyStoreAuthenticatedAESCipherSpi.java | 7 +- .../keystore/AndroidKeyStoreCipherSpiBase.java | 6 +- .../keystore/AndroidKeyStoreECDSASignatureSpi.java | 89 ++++++++++++++++++++-- .../security/keystore/AndroidKeyStoreHmacSpi.java | 1 + .../keystore/AndroidKeyStoreRSACipherSpi.java | 6 +- .../keystore/AndroidKeyStoreSignatureSpiBase.java | 58 +++++++++----- .../KeyStoreCryptoOperationChunkedStreamer.java | 16 ++-- .../keystore/KeyStoreCryptoOperationStreamer.java | 8 +- 8 files changed, 147 insertions(+), 44 deletions(-) (limited to 'keystore') 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 + * + *

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. * *

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(); } -- cgit v1.1