From 53d544a4b9964166f90f34b46f3866cafefb84e7 Mon Sep 17 00:00:00 2001 From: Alex Klyubin Date: Wed, 15 Jul 2015 17:15:08 -0700 Subject: KM module may consume less input than provided by finish time. Keymaster1 HAL permits the implementation of "update" operation to leave some input unconsumed by the time "finish" operation neeeds to be invoked. This needs to be treated as "invalid input" error rather than a "can't happen" exception. This CL was confirmed to fix the issue by the vendor who encountered the issue. Bug: 22512100 Change-Id: Ibb1a37d58f650d03605612559a154ce2416d147c --- .../KeyStoreCryptoOperationChunkedStreamer.java | 94 ++++++++++++++++------ 1 file changed, 71 insertions(+), 23 deletions(-) (limited to 'keystore') diff --git a/keystore/java/android/security/keystore/KeyStoreCryptoOperationChunkedStreamer.java b/keystore/java/android/security/keystore/KeyStoreCryptoOperationChunkedStreamer.java index ea0f4b9..dbb79bc 100644 --- a/keystore/java/android/security/keystore/KeyStoreCryptoOperationChunkedStreamer.java +++ b/keystore/java/android/security/keystore/KeyStoreCryptoOperationChunkedStreamer.java @@ -19,12 +19,14 @@ package android.security.keystore; import android.os.IBinder; import android.security.KeyStore; import android.security.KeyStoreException; +import android.security.keymaster.KeymasterDefs; import android.security.keymaster.OperationResult; import libcore.util.EmptyArray; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.security.ProviderException; /** * Helper for streaming a crypto operation's input and output via {@link KeyStore} service's @@ -135,14 +137,15 @@ class KeyStoreCryptoOperationChunkedStreamer implements KeyStoreCryptoOperationS mBuffered = EmptyArray.BYTE; mBufferedOffset = 0; mBufferedLength = 0; - } else if (opResult.inputConsumed == 0) { + } else if (opResult.inputConsumed <= 0) { // Nothing was consumed. More input needed. if (inputLength > 0) { // 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 IllegalStateException("Nothing consumed from max-sized chunk: " - + chunk.length + " bytes"); + throw new KeyStoreException(KeymasterDefs.KM_ERROR_UNKNOWN_ERROR, + "Keystore consumed nothing from max-sized chunk: " + chunk.length + + " bytes"); } mBuffered = chunk; mBufferedOffset = 0; @@ -153,8 +156,9 @@ class KeyStoreCryptoOperationChunkedStreamer implements KeyStoreCryptoOperationS mBufferedOffset = opResult.inputConsumed; mBufferedLength = chunk.length - opResult.inputConsumed; } else { - throw new IllegalStateException("Consumed more than provided: " - + opResult.inputConsumed + ", provided: " + chunk.length); + throw new KeyStoreException(KeymasterDefs.KM_ERROR_UNKNOWN_ERROR, + "Keystore consumed more input than provided. Provided: " + chunk.length + + ", consumed: " + opResult.inputConsumed); } if ((opResult.output != null) && (opResult.output.length > 0)) { @@ -165,7 +169,7 @@ class KeyStoreCryptoOperationChunkedStreamer implements KeyStoreCryptoOperationS try { bufferedOutput.write(opResult.output); } catch (IOException e) { - throw new IllegalStateException("Failed to buffer output", e); + throw new ProviderException("Failed to buffer output", e); } } } else { @@ -179,7 +183,7 @@ class KeyStoreCryptoOperationChunkedStreamer implements KeyStoreCryptoOperationS try { bufferedOutput.write(opResult.output); } catch (IOException e) { - throw new IllegalStateException("Failed to buffer output", e); + throw new ProviderException("Failed to buffer output", e); } result = bufferedOutput.toByteArray(); } @@ -229,27 +233,71 @@ class KeyStoreCryptoOperationChunkedStreamer implements KeyStoreCryptoOperationS return EmptyArray.BYTE; } - byte[] chunk = ArrayUtils.subarray(mBuffered, mBufferedOffset, mBufferedLength); - mBuffered = EmptyArray.BYTE; - mBufferedLength = 0; - mBufferedOffset = 0; + // Keep invoking the update operation with remaining buffered data until either all of the + // buffered data is consumed or until update fails to consume anything. + ByteArrayOutputStream bufferedOutput = null; + while (mBufferedLength > 0) { + byte[] chunk = ArrayUtils.subarray(mBuffered, mBufferedOffset, mBufferedLength); + OperationResult opResult = mKeyStoreStream.update(chunk); + if (opResult == null) { + throw new KeyStoreConnectException(); + } else if (opResult.resultCode != KeyStore.NO_ERROR) { + throw KeyStore.getKeyStoreException(opResult.resultCode); + } - OperationResult opResult = mKeyStoreStream.update(chunk); - if (opResult == null) { - throw new KeyStoreConnectException(); - } else if (opResult.resultCode != KeyStore.NO_ERROR) { - throw KeyStore.getKeyStoreException(opResult.resultCode); + if (opResult.inputConsumed <= 0) { + // Nothing was consumed. Break out of the loop to avoid an infinite loop. + break; + } + + if (opResult.inputConsumed >= chunk.length) { + // All of the input was consumed + mBuffered = EmptyArray.BYTE; + mBufferedOffset = 0; + mBufferedLength = 0; + } else { + // Some of the input was not consumed + mBuffered = chunk; + mBufferedOffset = opResult.inputConsumed; + mBufferedLength = chunk.length - opResult.inputConsumed; + } + + if (opResult.inputConsumed > chunk.length) { + throw new KeyStoreException(KeymasterDefs.KM_ERROR_UNKNOWN_ERROR, + "Keystore consumed more input than provided. Provided: " + + chunk.length + ", consumed: " + opResult.inputConsumed); + } + + if ((opResult.output != null) && (opResult.output.length > 0)) { + // Some output was produced by this update operation + if (bufferedOutput == null) { + // No output buffered yet. + if (mBufferedLength == 0) { + // No more output will be produced by this flush operation + mProducedOutputSizeBytes += opResult.output.length; + return opResult.output; + } else { + // More output might be produced by this flush operation -- buffer output. + bufferedOutput = new ByteArrayOutputStream(); + } + } + // Buffer the output from this update operation + try { + bufferedOutput.write(opResult.output); + } catch (IOException e) { + throw new ProviderException("Failed to buffer output", e); + } + } } - if (opResult.inputConsumed < chunk.length) { - throw new IllegalStateException("Keystore failed to consume all input. Provided: " - + chunk.length + ", consumed: " + opResult.inputConsumed); - } else if (opResult.inputConsumed > chunk.length) { - throw new IllegalStateException("Keystore consumed more input than provided" - + " . Provided: " + chunk.length + ", consumed: " + opResult.inputConsumed); + if (mBufferedLength > 0) { + throw new KeyStoreException(KeymasterDefs.KM_ERROR_INVALID_INPUT_LENGTH, + "Keystore failed to consume last " + + ((mBufferedLength != 1) ? (mBufferedLength + " bytes") : "byte") + + " of input"); } - byte[] result = (opResult.output != null) ? opResult.output : EmptyArray.BYTE; + byte[] result = (bufferedOutput != null) ? bufferedOutput.toByteArray() : EmptyArray.BYTE; mProducedOutputSizeBytes += result.length; return result; } -- cgit v1.1