From b84a5ef1bb70787ccf34ca3a7471f1ee61dc2948 Mon Sep 17 00:00:00 2001 From: Elijah Zupancic Date: Mon, 12 Jun 2017 15:57:08 -0700 Subject: [PATCH] Fixes #250 and adds unit tests. --- .../MantaEncryptedObjectInputStream.java | 68 ++++++++++---- .../crypto/IncompleteByteReadInputStream.java | 46 +++++++++ .../MantaEncryptedObjectInputStreamTest.java | 93 +++++++++++++++++++ 3 files changed, 188 insertions(+), 19 deletions(-) create mode 100644 java-manta-client/src/test/java/com/joyent/manta/client/crypto/IncompleteByteReadInputStream.java diff --git a/java-manta-client/src/main/java/com/joyent/manta/client/crypto/MantaEncryptedObjectInputStream.java b/java-manta-client/src/main/java/com/joyent/manta/client/crypto/MantaEncryptedObjectInputStream.java index be120153..898a5059 100644 --- a/java-manta-client/src/main/java/com/joyent/manta/client/crypto/MantaEncryptedObjectInputStream.java +++ b/java-manta-client/src/main/java/com/joyent/manta/client/crypto/MantaEncryptedObjectInputStream.java @@ -652,6 +652,49 @@ private int calculateBufferSize() { return bufferSize; } + /** + * Reads the HMAC from the end of the underlying stream and returns it as + * a byte array. + * + * @return HMAC as byte array + * @throws MantaIOException thrown when stream is unavailable or invalid + */ + private byte[] readHmacFromEndOfStream() throws MantaIOException { + final InputStream stream = super.getBackingStream(); + final int hmacSize = this.hmac.getMacSize(); + final byte[] hmacBytes = new byte[hmacSize]; + + int totalBytesRead = 0; + int bytesRead; + + while (totalBytesRead < hmacSize) { + final int lengthToRead = hmacSize - totalBytesRead; + + try { + bytesRead = stream.read(hmacBytes, totalBytesRead, lengthToRead); + } catch (IOException e) { + String msg = "Unable to read HMAC from the end of stream"; + MantaIOException mioe = new MantaIOException(msg); + annotateException(mioe); + mioe.setContextValue("backing_stream_class", stream.getClass()); + mioe.setContextValue("total_hmac_bytes_read", totalBytesRead); + + throw mioe; + } + + if (totalBytesRead < hmacSize && bytesRead == EOF) { + String msg = "No HMAC was stored at the end of the stream"; + MantaIOException e = new MantaIOException(msg); + annotateException(e); + throw e; + } + + totalBytesRead += bytesRead; + } + + return hmacBytes; + } + @Override public void close() throws IOException { synchronized (this.closeLock) { @@ -664,34 +707,21 @@ public void close() throws IOException { readRemainingBytes(); - IOUtils.closeQuietly(cipherInputStream); + try { + IOUtils.closeQuietly(cipherInputStream); + } catch (Exception e) { + LOGGER.warn("Error closing CipherInputStream", e); + } if (hmac != null && authenticateCiphertext) { byte[] checksum = new byte[hmac.getMacSize()]; hmac.doFinal(checksum, 0); - byte[] expected = new byte[this.hmac.getMacSize()]; - int readHmacBytes = super.getBackingStream().read(expected); - - if (super.getBackingStream().read() != EOF) { - String msg = "Expecting the end of the stream. However, more " - + "bytes were available."; - MantaIOException e = new MantaIOException(msg); - annotateException(e); - throw e; - } + byte[] expected = readHmacFromEndOfStream(); if (LOGGER.isDebugEnabled()) { LOGGER.debug("Calculated HMAC is: {}", Hex.encodeHexString(checksum)); } - if (readHmacBytes != this.hmac.getMacSize()) { - MantaIOException e = new MantaIOException("The HMAC stored was in the incorrect size"); - annotateException(e); - e.setContextValue("expectedHmacSize", this.hmac.getMacSize()); - e.setContextValue("actualHmacSize", readHmacBytes); - throw e; - } - if (super.getBackingStream().read() >= 0) { MantaIOException e = new MantaIOException("More bytes were available than the " + "expected HMAC length"); diff --git a/java-manta-client/src/test/java/com/joyent/manta/client/crypto/IncompleteByteReadInputStream.java b/java-manta-client/src/test/java/com/joyent/manta/client/crypto/IncompleteByteReadInputStream.java new file mode 100644 index 00000000..0e1bcdb9 --- /dev/null +++ b/java-manta-client/src/test/java/com/joyent/manta/client/crypto/IncompleteByteReadInputStream.java @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2017, Joyent, Inc. All rights reserved. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +package com.joyent.manta.client.crypto; + +import java.io.IOException; +import java.io.InputStream; + +/** + * {@link java.io.InputStream} implementation that will always read one byte + * less than the length specified in a {@link InputStream#read(byte[], int, int)} + * method invocation. + * + * @author Elijah Zupancic + * @since 3.1.1 + */ +public class IncompleteByteReadInputStream extends InputStream { + private final InputStream wrapped; + + public IncompleteByteReadInputStream(final InputStream wrapped) { + this.wrapped = wrapped; + } + + @Override + public int read() throws IOException { + return wrapped.read(); + } + + @Override + public int read(final byte[] b) throws IOException { + return read(b, 0, b.length); + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + if (len > 1 && len - off - 1 > 0) { + return wrapped.read(b, off, len - 1); + } else { + return wrapped.read(b, off, len); + } + } +} diff --git a/java-manta-client/src/test/java/com/joyent/manta/client/crypto/MantaEncryptedObjectInputStreamTest.java b/java-manta-client/src/test/java/com/joyent/manta/client/crypto/MantaEncryptedObjectInputStreamTest.java index 399c6883..fadb2ee0 100644 --- a/java-manta-client/src/test/java/com/joyent/manta/client/crypto/MantaEncryptedObjectInputStreamTest.java +++ b/java-manta-client/src/test/java/com/joyent/manta/client/crypto/MantaEncryptedObjectInputStreamTest.java @@ -11,10 +11,13 @@ import com.joyent.manta.client.MantaObjectInputStream; import com.joyent.manta.client.MantaObjectResponse; import com.joyent.manta.exception.MantaClientEncryptionCiphertextAuthenticationException; +import com.joyent.manta.exception.MantaIOException; import com.joyent.manta.http.MantaHttpHeaders; import com.joyent.manta.http.entity.MantaInputStreamEntity; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; +import org.apache.commons.io.input.BoundedInputStream; +import org.apache.commons.io.output.NullOutputStream; import org.apache.commons.lang3.RandomUtils; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.conn.EofSensorInputStream; @@ -32,6 +35,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; import java.io.RandomAccessFile; import java.io.UncheckedIOException; import java.net.URISyntaxException; @@ -570,6 +574,50 @@ public void canCopyStreamWithLargeBufferBufferUnauthenticatedAesCtr256() throws } + + public void willErrorIfCiphertextIsMissingHmacAesCbc128() throws IOException { + willErrorWhenMissingHMAC(AesCbcCipherDetails.INSTANCE_128_BIT); + } + + @Test(groups = {"unlimited-crypto"}) + public void willErrorIfCiphertextIsMissingHmacAesCbc192() throws IOException { + willErrorWhenMissingHMAC(AesCbcCipherDetails.INSTANCE_192_BIT); + } + + @Test(groups = {"unlimited-crypto"}) + public void willErrorIfCiphertextIsMissingHmacAesCbc256() throws IOException { + willErrorWhenMissingHMAC(AesCbcCipherDetails.INSTANCE_256_BIT); + } + + public void willErrorIfCiphertextIsMissingHmacAesCtr128() throws IOException { + willErrorWhenMissingHMAC(AesCtrCipherDetails.INSTANCE_128_BIT); + } + + @Test(groups = {"unlimited-crypto"}) + public void willErrorIfCiphertextIsMissingHmacAesCtr192() throws IOException { + willErrorWhenMissingHMAC(AesCtrCipherDetails.INSTANCE_192_BIT); + } + + @Test(groups = {"unlimited-crypto"}) + public void willErrorIfCiphertextIsMissingHmacAesCtr256() throws IOException { + willErrorWhenMissingHMAC(AesCtrCipherDetails.INSTANCE_256_BIT); + } + + + public void willValidateIfHmacIsReadInMultipleReadsAesCtr128() throws IOException { + willValidateIfHmacIsReadInMultipleReads(AesCtrCipherDetails.INSTANCE_128_BIT); + } + + @Test(groups = {"unlimited-crypto"}) + public void willValidateIfHmacIsReadInMultipleReadsAesCtr192() throws IOException { + willValidateIfHmacIsReadInMultipleReads(AesCtrCipherDetails.INSTANCE_192_BIT); + } + + @Test(groups = {"unlimited-crypto"}) + public void willValidateIfHmacIsReadInMultipleReadsAesCtr256() throws IOException { + willValidateIfHmacIsReadInMultipleReads(AesCtrCipherDetails.INSTANCE_256_BIT); + } + /* TEST UTILITY CLASSES */ private static class EncryptedFile { @@ -1016,6 +1064,51 @@ private void willThrowExceptionWhenCiphertextIsAltered(SupportedCipherDetails ci Assert.assertTrue(thrown, "Ciphertext authentication exception wasn't thrown"); } + private void willErrorWhenMissingHMAC(final SupportedCipherDetails cipherDetails) throws IOException { + SecretKey key = SecretKeyUtils.generate(cipherDetails); + EncryptedFile encryptedFile = encryptedFile(key, cipherDetails, this.plaintextSize); + long ciphertextSize = encryptedFile.file.length(); + int hmacSize = cipherDetails.getAuthenticationTagOrHmacLengthInBytes(); + long ciphertextSizeWithoutHmac = ciphertextSize - hmacSize; + + boolean thrown = false; + + try (FileInputStream fin = new FileInputStream(encryptedFile.file); + BoundedInputStream in = new BoundedInputStream(fin, ciphertextSizeWithoutHmac); + MantaEncryptedObjectInputStream min = createEncryptedObjectInputStream(key, in, + ciphertextSize, cipherDetails, encryptedFile.cipher.getIV(), + true, (long)this.plaintextSize)) { + // Do a single read to make sure that everything is working + Assert.assertNotEquals(min.read(), -1, + "The encrypted stream should not be empty"); + } catch (MantaIOException e) { + if (e.getMessage().startsWith("No HMAC was stored at the end of the stream")) { + thrown = true; + } else { + throw e; + } + } + + Assert.assertTrue(thrown, "Expected MantaIOException was not thrown"); + } + + private void willValidateIfHmacIsReadInMultipleReads(final SupportedCipherDetails cipherDetails) + throws IOException { + SecretKey key = SecretKeyUtils.generate(cipherDetails); + EncryptedFile encryptedFile = encryptedFile(key, cipherDetails, this.plaintextSize); + long ciphertextSize = encryptedFile.file.length(); + + try (FileInputStream fin = new FileInputStream(encryptedFile.file); + IncompleteByteReadInputStream ibrin = new IncompleteByteReadInputStream(fin); + MantaEncryptedObjectInputStream min = createEncryptedObjectInputStream(key, ibrin, + ciphertextSize, cipherDetails, encryptedFile.cipher.getIV(), + true, (long)this.plaintextSize); + OutputStream out = new NullOutputStream()) { + + IOUtils.copy(min, out); + } + } + private EncryptedFile encryptedFile( SecretKey key, SupportedCipherDetails cipherDetails, long plaintextSize) throws IOException {