From 2e89cda74f33ff7c819c0c0a937d52aef67993ba Mon Sep 17 00:00:00 2001 From: Kess Plasmeier Date: Tue, 27 May 2025 15:56:32 -0700 Subject: [PATCH 1/5] fix: do not truncate encrypted streams when offset is greater than zero --- .../internal/FrameEncryptionHandler.java | 8 +- .../internal/FrameEncryptionHandlerTest.java | 97 +++++++++++++++++++ 2 files changed, 102 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandler.java b/src/main/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandler.java index d9fc7f639..d82526f30 100644 --- a/src/main/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandler.java +++ b/src/main/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandler.java @@ -81,8 +81,8 @@ public FrameEncryptionHandler( * * * @param in the input byte array. - * @param inOff the offset into the in array where the data to be encrypted starts. - * @param inLen the number of bytes to be encrypted. + * @param off the offset into the in array where the data to be encrypted starts. + * @param len the number of bytes to be encrypted. * @param out the output buffer the encrypted bytes go into. * @param outOff the offset into the output byte array the encrypted data starts at. * @return the number of bytes written to out and processed @@ -116,7 +116,9 @@ public ProcessingSummary processBytes( // update offset by the size of bytes being encrypted. offset += size; // update size to the remaining bytes starting at offset. - size = len - offset; + // the original offset MUST NOT be part of the length calculation, + // or else the encryption will be truncated. + size = len - (offset - off); } return new ProcessingSummary(actualOutLen, len); diff --git a/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java b/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java index da6bf08f0..02b77d4d5 100644 --- a/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java +++ b/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java @@ -19,14 +19,30 @@ import com.amazonaws.encryptionsdk.AwsCrypto; import com.amazonaws.encryptionsdk.CryptoAlgorithm; +import com.amazonaws.encryptionsdk.CryptoInputStream; +import com.amazonaws.encryptionsdk.CryptoOutputStream; +import com.amazonaws.encryptionsdk.CryptoResult; import com.amazonaws.encryptionsdk.TestUtils; import com.amazonaws.encryptionsdk.model.CipherFrameHeaders; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.lang.reflect.Field; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.security.SecureRandom; +import java.util.Collections; import javax.crypto.SecretKey; import javax.crypto.spec.SecretKeySpec; import org.bouncycastle.util.encoders.Hex; import org.junit.Before; import org.junit.Test; +import software.amazon.awssdk.utils.StringUtils; +import software.amazon.cryptography.materialproviders.IKeyring; +import software.amazon.cryptography.materialproviders.MaterialProviders; +import software.amazon.cryptography.materialproviders.model.AesWrappingAlg; +import software.amazon.cryptography.materialproviders.model.CreateRawAesKeyringInput; +import software.amazon.cryptography.materialproviders.model.MaterialProvidersConfig; public class FrameEncryptionHandlerTest { private final CryptoAlgorithm cryptoAlgorithm_ = TestUtils.DEFAULT_TEST_CRYPTO_ALG; @@ -117,4 +133,85 @@ private void assertHeaderNonce(byte[] expectedNonce, byte[] buf) { private void generateTestBlock(byte[] buf) { frameEncryptionHandler_.processBytes(new byte[frameSize_], 0, frameSize_, buf, 0); } + + /** + * This isn't a unit test, + * but it reproduces a bug in the FrameEncryptionHandler + * where the stream would be truncated when the offset is >0 + * @throws Exception + */ + @Test + public void testStreamTruncation() throws Exception { + String testDataString = StringUtils.repeat("Hello, World! ", 10_000); + + int startOffset = 100; // The data will start from this offset + byte[] inputData = new byte[10_000]; + System.arraycopy( + testDataString.getBytes(StandardCharsets.UTF_8), + 0, + inputData, + startOffset, + inputData.length - startOffset); + // decryptData doesn't know about the offset + byte[] expectedOutput = new byte[10_000 - startOffset]; + System.arraycopy( + testDataString.getBytes(StandardCharsets.UTF_8), + 0, + expectedOutput, + 0, + inputData.length - startOffset); + int originalLength = inputData.length - startOffset; + + // Generate a random AES key + SecureRandom rnd = new SecureRandom(); + byte[] rawKey = new byte[16]; + rnd.nextBytes(rawKey); + SecretKeySpec cryptoKey = new SecretKeySpec(rawKey, "AES"); + + // Create a keyring using the generated AES key + MaterialProviders materialProviders = MaterialProviders.builder() + .MaterialProvidersConfig(MaterialProvidersConfig.builder().build()) + .build(); + CreateRawAesKeyringInput keyringInput = CreateRawAesKeyringInput.builder() + .wrappingKey(ByteBuffer.wrap(cryptoKey.getEncoded())) + .keyNamespace("Example") + .keyName("RandomKey") + .wrappingAlg(AesWrappingAlg.ALG_AES128_GCM_IV12_TAG16) + .build(); + IKeyring keyring = materialProviders.CreateRawAesKeyring(keyringInput); + AwsCrypto crypto = AwsCrypto.standard(); + + // Encrypt the data + byte[] encryptedData; + try (ByteArrayOutputStream os = new ByteArrayOutputStream()) { + try (CryptoOutputStream cryptoOutput = crypto.createEncryptingStream(keyring, os, Collections.emptyMap())) { + cryptoOutput.write(inputData, startOffset, inputData.length - startOffset); + } + encryptedData = os.toByteArray(); + } + + // Check non-streaming decrypt + CryptoResult nonStreamDecrypt = crypto.decryptData(keyring, encryptedData); + assertEquals(originalLength, nonStreamDecrypt.getResult().length); + assertArrayEquals(expectedOutput, nonStreamDecrypt.getResult()); + + // Check streaming decrypt + int decryptedLength = 0; + byte[] decryptedData = new byte[inputData.length]; + try (ByteArrayInputStream is = new ByteArrayInputStream(encryptedData); + CryptoInputStream cryptoInput = crypto.createDecryptingStream(keyring, is)) { + int offset = startOffset; + do { + int bytesRead = cryptoInput.read(decryptedData, offset, decryptedData.length - offset); + if (bytesRead <= 0) { + break; // End of stream + } + offset += bytesRead; + decryptedLength += bytesRead; + } while (true); + } + assertEquals(originalLength, decryptedLength); + assertArrayEquals(inputData, decryptedData); + + } } From 01fbe7a8bee6663369b3c6b276ed295cc77a53a8 Mon Sep 17 00:00:00 2001 From: Kess Plasmeier Date: Wed, 28 May 2025 14:14:48 -0700 Subject: [PATCH 2/5] format --- .../internal/FrameEncryptionHandlerTest.java | 53 ++++++++++--------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java b/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java index 02b77d4d5..962004126 100644 --- a/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java +++ b/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java @@ -24,7 +24,6 @@ import com.amazonaws.encryptionsdk.CryptoResult; import com.amazonaws.encryptionsdk.TestUtils; import com.amazonaws.encryptionsdk.model.CipherFrameHeaders; - import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.lang.reflect.Field; @@ -135,9 +134,9 @@ private void generateTestBlock(byte[] buf) { } /** - * This isn't a unit test, - * but it reproduces a bug in the FrameEncryptionHandler - * where the stream would be truncated when the offset is >0 + * This isn't a unit test, but it reproduces a bug in the FrameEncryptionHandler where the stream + * would be truncated when the offset is >0 + * * @throws Exception */ @Test @@ -147,19 +146,19 @@ public void testStreamTruncation() throws Exception { int startOffset = 100; // The data will start from this offset byte[] inputData = new byte[10_000]; System.arraycopy( - testDataString.getBytes(StandardCharsets.UTF_8), - 0, - inputData, - startOffset, - inputData.length - startOffset); + testDataString.getBytes(StandardCharsets.UTF_8), + 0, + inputData, + startOffset, + inputData.length - startOffset); // decryptData doesn't know about the offset byte[] expectedOutput = new byte[10_000 - startOffset]; System.arraycopy( - testDataString.getBytes(StandardCharsets.UTF_8), - 0, - expectedOutput, - 0, - inputData.length - startOffset); + testDataString.getBytes(StandardCharsets.UTF_8), + 0, + expectedOutput, + 0, + inputData.length - startOffset); int originalLength = inputData.length - startOffset; // Generate a random AES key @@ -169,22 +168,25 @@ public void testStreamTruncation() throws Exception { SecretKeySpec cryptoKey = new SecretKeySpec(rawKey, "AES"); // Create a keyring using the generated AES key - MaterialProviders materialProviders = MaterialProviders.builder() - .MaterialProvidersConfig(MaterialProvidersConfig.builder().build()) - .build(); - CreateRawAesKeyringInput keyringInput = CreateRawAesKeyringInput.builder() - .wrappingKey(ByteBuffer.wrap(cryptoKey.getEncoded())) - .keyNamespace("Example") - .keyName("RandomKey") - .wrappingAlg(AesWrappingAlg.ALG_AES128_GCM_IV12_TAG16) - .build(); + MaterialProviders materialProviders = + MaterialProviders.builder() + .MaterialProvidersConfig(MaterialProvidersConfig.builder().build()) + .build(); + CreateRawAesKeyringInput keyringInput = + CreateRawAesKeyringInput.builder() + .wrappingKey(ByteBuffer.wrap(cryptoKey.getEncoded())) + .keyNamespace("Example") + .keyName("RandomKey") + .wrappingAlg(AesWrappingAlg.ALG_AES128_GCM_IV12_TAG16) + .build(); IKeyring keyring = materialProviders.CreateRawAesKeyring(keyringInput); AwsCrypto crypto = AwsCrypto.standard(); // Encrypt the data byte[] encryptedData; try (ByteArrayOutputStream os = new ByteArrayOutputStream()) { - try (CryptoOutputStream cryptoOutput = crypto.createEncryptingStream(keyring, os, Collections.emptyMap())) { + try (CryptoOutputStream cryptoOutput = + crypto.createEncryptingStream(keyring, os, Collections.emptyMap())) { cryptoOutput.write(inputData, startOffset, inputData.length - startOffset); } encryptedData = os.toByteArray(); @@ -199,7 +201,7 @@ public void testStreamTruncation() throws Exception { int decryptedLength = 0; byte[] decryptedData = new byte[inputData.length]; try (ByteArrayInputStream is = new ByteArrayInputStream(encryptedData); - CryptoInputStream cryptoInput = crypto.createDecryptingStream(keyring, is)) { + CryptoInputStream cryptoInput = crypto.createDecryptingStream(keyring, is)) { int offset = startOffset; do { int bytesRead = cryptoInput.read(decryptedData, offset, decryptedData.length - offset); @@ -212,6 +214,5 @@ public void testStreamTruncation() throws Exception { } assertEquals(originalLength, decryptedLength); assertArrayEquals(inputData, decryptedData); - } } From f35960113dbda02a8c58df5a2f2cd7b4453cff21 Mon Sep 17 00:00:00 2001 From: Kess Plasmeier Date: Wed, 28 May 2025 17:14:54 -0700 Subject: [PATCH 3/5] make implementation clearer, add comments --- .../internal/FrameEncryptionHandler.java | 14 ++-- .../internal/FrameEncryptionHandlerTest.java | 69 ++++++++++--------- 2 files changed, 42 insertions(+), 41 deletions(-) diff --git a/src/main/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandler.java b/src/main/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandler.java index d82526f30..be2bd1238 100644 --- a/src/main/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandler.java +++ b/src/main/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandler.java @@ -95,13 +95,13 @@ public ProcessingSummary processBytes( int actualOutLen = 0; int size = len; - int offset = off; + int processedBytes = 0; while (size > 0) { final int currentFrameCapacity = frameSize_ - bytesToFrameLen_; // bind size to the capacity of the current frame size = Math.min(currentFrameCapacity, size); - System.arraycopy(in, offset, bytesToFrame_, bytesToFrameLen_, size); + System.arraycopy(in, off + processedBytes, bytesToFrame_, bytesToFrameLen_, size); bytesToFrameLen_ += size; // check if there is enough bytes to create a frame @@ -113,12 +113,10 @@ public ProcessingSummary processBytes( bytesToFrameLen_ = 0; } - // update offset by the size of bytes being encrypted. - offset += size; - // update size to the remaining bytes starting at offset. - // the original offset MUST NOT be part of the length calculation, - // or else the encryption will be truncated. - size = len - (offset - off); + // add the size of this frame to processedBytes + processedBytes += size; + // remaining size is original len minus processedBytes + size = len - processedBytes; } return new ProcessingSummary(actualOutLen, len); diff --git a/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java b/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java index 962004126..f3b83b27b 100644 --- a/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java +++ b/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java @@ -141,65 +141,67 @@ private void generateTestBlock(byte[] buf) { */ @Test public void testStreamTruncation() throws Exception { - String testDataString = StringUtils.repeat("Hello, World! ", 10_000); + // Initialize AES key and keyring + SecureRandom rnd = new SecureRandom(); + byte[] rawKey = new byte[16]; + rnd.nextBytes(rawKey); + SecretKeySpec cryptoKey = new SecretKeySpec(rawKey, "AES"); + MaterialProviders materialProviders = + MaterialProviders.builder() + .MaterialProvidersConfig(MaterialProvidersConfig.builder().build()) + .build(); + CreateRawAesKeyringInput keyringInput = + CreateRawAesKeyringInput.builder() + .wrappingKey(ByteBuffer.wrap(cryptoKey.getEncoded())) + .keyNamespace("Example") + .keyName("RandomKey") + .wrappingAlg(AesWrappingAlg.ALG_AES128_GCM_IV12_TAG16) + .build(); + IKeyring keyring = materialProviders.CreateRawAesKeyring(keyringInput); + AwsCrypto crypto = AwsCrypto.standard(); + + String testDataString = StringUtils.repeat("Hello, World! ", 5_000); int startOffset = 100; // The data will start from this offset - byte[] inputData = new byte[10_000]; + byte[] inputDataWithOffset = new byte[10_000]; + // the length of the actual data + int dataLength = dataLength; + // copy some data, starting at the startOffset + // so the first |startOffset| bytes are 0s System.arraycopy( testDataString.getBytes(StandardCharsets.UTF_8), 0, - inputData, + inputDataWithOffset, startOffset, - inputData.length - startOffset); - // decryptData doesn't know about the offset + dataLength); + // decryptData (non-streaming) doesn't know about the offset + // it will strip out the original 0s byte[] expectedOutput = new byte[10_000 - startOffset]; System.arraycopy( testDataString.getBytes(StandardCharsets.UTF_8), 0, expectedOutput, 0, - inputData.length - startOffset); - int originalLength = inputData.length - startOffset; - - // Generate a random AES key - SecureRandom rnd = new SecureRandom(); - byte[] rawKey = new byte[16]; - rnd.nextBytes(rawKey); - SecretKeySpec cryptoKey = new SecretKeySpec(rawKey, "AES"); - - // Create a keyring using the generated AES key - MaterialProviders materialProviders = - MaterialProviders.builder() - .MaterialProvidersConfig(MaterialProvidersConfig.builder().build()) - .build(); - CreateRawAesKeyringInput keyringInput = - CreateRawAesKeyringInput.builder() - .wrappingKey(ByteBuffer.wrap(cryptoKey.getEncoded())) - .keyNamespace("Example") - .keyName("RandomKey") - .wrappingAlg(AesWrappingAlg.ALG_AES128_GCM_IV12_TAG16) - .build(); - IKeyring keyring = materialProviders.CreateRawAesKeyring(keyringInput); - AwsCrypto crypto = AwsCrypto.standard(); + dataLength); // Encrypt the data byte[] encryptedData; try (ByteArrayOutputStream os = new ByteArrayOutputStream()) { try (CryptoOutputStream cryptoOutput = crypto.createEncryptingStream(keyring, os, Collections.emptyMap())) { - cryptoOutput.write(inputData, startOffset, inputData.length - startOffset); + cryptoOutput.write(inputDataWithOffset, startOffset, dataLength); } encryptedData = os.toByteArray(); } // Check non-streaming decrypt CryptoResult nonStreamDecrypt = crypto.decryptData(keyring, encryptedData); - assertEquals(originalLength, nonStreamDecrypt.getResult().length); + assertEquals(dataLength, nonStreamDecrypt.getResult().length); assertArrayEquals(expectedOutput, nonStreamDecrypt.getResult()); // Check streaming decrypt int decryptedLength = 0; - byte[] decryptedData = new byte[inputData.length]; + byte[] decryptedData = new byte[inputDataWithOffset.length]; try (ByteArrayInputStream is = new ByteArrayInputStream(encryptedData); CryptoInputStream cryptoInput = crypto.createDecryptingStream(keyring, is)) { int offset = startOffset; @@ -212,7 +214,8 @@ public void testStreamTruncation() throws Exception { decryptedLength += bytesRead; } while (true); } - assertEquals(originalLength, decryptedLength); - assertArrayEquals(inputData, decryptedData); + assertEquals(dataLength, decryptedLength); + // These arrays will be offset, i.e. the first |startOffset| bytes are 0s + assertArrayEquals(inputDataWithOffset, decryptedData); } } From e20d3a9380cbdfc2f5b52d795ea51b13ff46747d Mon Sep 17 00:00:00 2001 From: Kess Plasmeier Date: Wed, 28 May 2025 17:15:49 -0700 Subject: [PATCH 4/5] format --- .../internal/FrameEncryptionHandlerTest.java | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java b/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java index f3b83b27b..24c75c0ce 100644 --- a/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java +++ b/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java @@ -147,16 +147,16 @@ public void testStreamTruncation() throws Exception { rnd.nextBytes(rawKey); SecretKeySpec cryptoKey = new SecretKeySpec(rawKey, "AES"); MaterialProviders materialProviders = - MaterialProviders.builder() - .MaterialProvidersConfig(MaterialProvidersConfig.builder().build()) - .build(); + MaterialProviders.builder() + .MaterialProvidersConfig(MaterialProvidersConfig.builder().build()) + .build(); CreateRawAesKeyringInput keyringInput = - CreateRawAesKeyringInput.builder() - .wrappingKey(ByteBuffer.wrap(cryptoKey.getEncoded())) - .keyNamespace("Example") - .keyName("RandomKey") - .wrappingAlg(AesWrappingAlg.ALG_AES128_GCM_IV12_TAG16) - .build(); + CreateRawAesKeyringInput.builder() + .wrappingKey(ByteBuffer.wrap(cryptoKey.getEncoded())) + .keyNamespace("Example") + .keyName("RandomKey") + .wrappingAlg(AesWrappingAlg.ALG_AES128_GCM_IV12_TAG16) + .build(); IKeyring keyring = materialProviders.CreateRawAesKeyring(keyringInput); AwsCrypto crypto = AwsCrypto.standard(); @@ -178,11 +178,7 @@ public void testStreamTruncation() throws Exception { // it will strip out the original 0s byte[] expectedOutput = new byte[10_000 - startOffset]; System.arraycopy( - testDataString.getBytes(StandardCharsets.UTF_8), - 0, - expectedOutput, - 0, - dataLength); + testDataString.getBytes(StandardCharsets.UTF_8), 0, expectedOutput, 0, dataLength); // Encrypt the data byte[] encryptedData; From fb40f77d37e965b690e795b61f91c0591bd35362 Mon Sep 17 00:00:00 2001 From: Kess Plasmeier Date: Wed, 28 May 2025 17:30:31 -0700 Subject: [PATCH 5/5] fix --- .../encryptionsdk/internal/FrameEncryptionHandlerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java b/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java index 24c75c0ce..542ca9951 100644 --- a/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java +++ b/src/test/java/com/amazonaws/encryptionsdk/internal/FrameEncryptionHandlerTest.java @@ -165,7 +165,7 @@ public void testStreamTruncation() throws Exception { int startOffset = 100; // The data will start from this offset byte[] inputDataWithOffset = new byte[10_000]; // the length of the actual data - int dataLength = dataLength; + int dataLength = inputDataWithOffset.length - startOffset; // copy some data, starting at the startOffset // so the first |startOffset| bytes are 0s System.arraycopy(