From 163fd8847760a0f3e3af473530f8f7ccf03ae910 Mon Sep 17 00:00:00 2001 From: Wesley Rosenblum Date: Thu, 26 Mar 2020 16:58:55 -0700 Subject: [PATCH 1/5] Add validation to ensure the length of the final frame in the final frame header does not exceed the frame size specified in the message header. --- .../internal/FrameDecryptionHandler.java | 5 +++++ .../internal/FrameDecryptionHandlerTest.java | 19 ++++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java b/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java index 82d9cab76..b7e3f4c8c 100644 --- a/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java +++ b/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java @@ -133,6 +133,11 @@ public ProcessingSummary processBytes(final byte[] in, final int off, final int int protectedContentLen = -1; if (currentFrameHeaders_.isFinalFrame()) { protectedContentLen = currentFrameHeaders_.getFrameContentLength(); + + // The final frame should not be able to exceed the frameLength + if(frameSize_ > 0 && protectedContentLen > frameSize_) { + throw new BadCiphertextException("Final frame length exceeds frame length."); + } } else { protectedContentLen = frameSize_; } diff --git a/src/test/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandlerTest.java b/src/test/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandlerTest.java index 40554f72d..9f0a637e9 100644 --- a/src/test/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandlerTest.java +++ b/src/test/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandlerTest.java @@ -15,11 +15,14 @@ import static org.junit.Assert.assertTrue; +import java.nio.ByteBuffer; import java.security.SecureRandom; import javax.crypto.SecretKey; import javax.crypto.spec.SecretKeySpec; +import com.amazonaws.encryptionsdk.TestUtils; +import com.amazonaws.encryptionsdk.exception.BadCiphertextException; import org.junit.Before; import org.junit.Test; @@ -72,4 +75,18 @@ public void decryptMaxContentLength() { frameDecryptionHandler_.processBytes(in, 0, in.length, out, 0); frameDecryptionHandler_.processBytes(in, 0, Integer.MAX_VALUE, out, 0); } -} \ No newline at end of file + + @Test(expected = BadCiphertextException.class) + public void finalFrameLengthTooLarge() { + + final ByteBuffer byteBuffer = ByteBuffer.allocate(25); + byteBuffer.put(TestUtils.unsignedBytesToSignedBytes( + new int[] {255, 255, 255, 255, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1})); + byteBuffer.putInt(AwsCrypto.getDefaultFrameSize() + 1); + + final byte[] in = byteBuffer.array(); + final byte[] out = new byte[in.length]; + + frameDecryptionHandler_.processBytes(in, 0, in.length, out, 0); + } +} From b58b5c1419c09eb6625a986af815478f7f6c37ed Mon Sep 17 00:00:00 2001 From: Wesley Rosenblum Date: Fri, 27 Mar 2020 14:18:37 -0700 Subject: [PATCH 2/5] Validate that frame length is positive for framed data --- .../encryptionsdk/internal/FrameDecryptionHandler.java | 7 +------ .../encryptionsdk/model/CiphertextHeaders.java | 10 +++++++++- .../encryptionsdk/model/CiphertextHeadersTest.java | 4 ++-- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java b/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java index b7e3f4c8c..612ee212d 100644 --- a/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java +++ b/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java @@ -119,11 +119,6 @@ public ProcessingSummary processBytes(final byte[] in, final int off, final int if (currentFrameHeaders_ == null) { currentFrameHeaders_ = new CipherFrameHeaders(); currentFrameHeaders_.setNonceLength(nonceLen_); - if (frameSize_ == 0) { - // if frame size in ciphertext headers is 0, the frame size - // will need to be parsed in individual frame headers. - currentFrameHeaders_.includeFrameSize(true); - } } totalParsedBytes += currentFrameHeaders_.deserialize(bytesToParse, totalParsedBytes); @@ -135,7 +130,7 @@ public ProcessingSummary processBytes(final byte[] in, final int off, final int protectedContentLen = currentFrameHeaders_.getFrameContentLength(); // The final frame should not be able to exceed the frameLength - if(frameSize_ > 0 && protectedContentLen > frameSize_) { + if(protectedContentLen > frameSize_) { throw new BadCiphertextException("Final frame length exceeds frame length."); } } else { diff --git a/src/main/java/com/amazonaws/encryptionsdk/model/CiphertextHeaders.java b/src/main/java/com/amazonaws/encryptionsdk/model/CiphertextHeaders.java index 5e3b90886..a8b80574a 100644 --- a/src/main/java/com/amazonaws/encryptionsdk/model/CiphertextHeaders.java +++ b/src/main/java/com/amazonaws/encryptionsdk/model/CiphertextHeaders.java @@ -143,6 +143,10 @@ public CiphertextHeaders(final byte version, final CiphertextType type, final Cr messageId_ = new byte[Constants.MESSAGE_ID_LEN]; RND.nextBytes(messageId_); + if(contentType == ContentType.FRAME && frameSize <= 0) { + throw new BadCiphertextException("Framed data requires a positive frame length"); + } + frameLength_ = frameSize; // Completed by construction @@ -630,6 +634,10 @@ public int deserialize(final byte[] b, final int off) throws ParseException { parsedBytes += parseHeaderTag(b, off + parsedBytes); } + if(frameLength_ <= 0 && ContentType.deserialize(contentTypeVal_) == ContentType.FRAME) { + throw new BadCiphertextException("Framed data requires a positive frame length"); + } + isComplete_ = true; } catch (ParseException e) { // this results when we do partial parsing and there aren't enough @@ -859,4 +867,4 @@ public void setHeaderNonce(final byte[] headerNonce) { public void setHeaderTag(final byte[] headerTag) { headerTag_ = headerTag.clone(); } -} \ No newline at end of file +} diff --git a/src/test/java/com/amazonaws/encryptionsdk/model/CiphertextHeadersTest.java b/src/test/java/com/amazonaws/encryptionsdk/model/CiphertextHeadersTest.java index 47216e59c..ea3645775 100644 --- a/src/test/java/com/amazonaws/encryptionsdk/model/CiphertextHeadersTest.java +++ b/src/test/java/com/amazonaws/encryptionsdk/model/CiphertextHeadersTest.java @@ -431,8 +431,8 @@ public void invalidFrameLength() { readUptoNonceLen(headerBuff); - // set content type to an invalid value - headerBuff.putInt(-1); + // set frame type to an invalid value for framed data + headerBuff.putInt(0); final CiphertextHeaders reconstructedHeaders = new CiphertextHeaders(); reconstructedHeaders.deserialize(headerBuff.array(), 0); From 534630921efe70af1b7aa74fb031f53ad662c131 Mon Sep 17 00:00:00 2001 From: Wesley Rosenblum Date: Thu, 2 Apr 2020 10:40:11 -0700 Subject: [PATCH 3/5] Reverting removal of variable frame length code --- .../encryptionsdk/internal/FrameDecryptionHandler.java | 7 ++++++- .../amazonaws/encryptionsdk/model/CiphertextHeaders.java | 8 -------- .../encryptionsdk/model/CiphertextHeadersTest.java | 4 ++-- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java b/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java index 612ee212d..b7e3f4c8c 100644 --- a/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java +++ b/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java @@ -119,6 +119,11 @@ public ProcessingSummary processBytes(final byte[] in, final int off, final int if (currentFrameHeaders_ == null) { currentFrameHeaders_ = new CipherFrameHeaders(); currentFrameHeaders_.setNonceLength(nonceLen_); + if (frameSize_ == 0) { + // if frame size in ciphertext headers is 0, the frame size + // will need to be parsed in individual frame headers. + currentFrameHeaders_.includeFrameSize(true); + } } totalParsedBytes += currentFrameHeaders_.deserialize(bytesToParse, totalParsedBytes); @@ -130,7 +135,7 @@ public ProcessingSummary processBytes(final byte[] in, final int off, final int protectedContentLen = currentFrameHeaders_.getFrameContentLength(); // The final frame should not be able to exceed the frameLength - if(protectedContentLen > frameSize_) { + if(frameSize_ > 0 && protectedContentLen > frameSize_) { throw new BadCiphertextException("Final frame length exceeds frame length."); } } else { diff --git a/src/main/java/com/amazonaws/encryptionsdk/model/CiphertextHeaders.java b/src/main/java/com/amazonaws/encryptionsdk/model/CiphertextHeaders.java index a8b80574a..aca71665b 100644 --- a/src/main/java/com/amazonaws/encryptionsdk/model/CiphertextHeaders.java +++ b/src/main/java/com/amazonaws/encryptionsdk/model/CiphertextHeaders.java @@ -143,10 +143,6 @@ public CiphertextHeaders(final byte version, final CiphertextType type, final Cr messageId_ = new byte[Constants.MESSAGE_ID_LEN]; RND.nextBytes(messageId_); - if(contentType == ContentType.FRAME && frameSize <= 0) { - throw new BadCiphertextException("Framed data requires a positive frame length"); - } - frameLength_ = frameSize; // Completed by construction @@ -634,10 +630,6 @@ public int deserialize(final byte[] b, final int off) throws ParseException { parsedBytes += parseHeaderTag(b, off + parsedBytes); } - if(frameLength_ <= 0 && ContentType.deserialize(contentTypeVal_) == ContentType.FRAME) { - throw new BadCiphertextException("Framed data requires a positive frame length"); - } - isComplete_ = true; } catch (ParseException e) { // this results when we do partial parsing and there aren't enough diff --git a/src/test/java/com/amazonaws/encryptionsdk/model/CiphertextHeadersTest.java b/src/test/java/com/amazonaws/encryptionsdk/model/CiphertextHeadersTest.java index ea3645775..3ce8d3ad3 100644 --- a/src/test/java/com/amazonaws/encryptionsdk/model/CiphertextHeadersTest.java +++ b/src/test/java/com/amazonaws/encryptionsdk/model/CiphertextHeadersTest.java @@ -431,8 +431,8 @@ public void invalidFrameLength() { readUptoNonceLen(headerBuff); - // set frame type to an invalid value for framed data - headerBuff.putInt(0); + // set frame type to an invalid value + headerBuff.putInt(-1); final CiphertextHeaders reconstructedHeaders = new CiphertextHeaders(); reconstructedHeaders.deserialize(headerBuff.array(), 0); From b8d754e5fef1fb1fe05c2a4160d2eb1491436263 Mon Sep 17 00:00:00 2001 From: Wesley Rosenblum Date: Thu, 2 Apr 2020 10:44:15 -0700 Subject: [PATCH 4/5] Reverting removal of variable frame length code --- .../amazonaws/encryptionsdk/model/CiphertextHeadersTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/amazonaws/encryptionsdk/model/CiphertextHeadersTest.java b/src/test/java/com/amazonaws/encryptionsdk/model/CiphertextHeadersTest.java index 3ce8d3ad3..47216e59c 100644 --- a/src/test/java/com/amazonaws/encryptionsdk/model/CiphertextHeadersTest.java +++ b/src/test/java/com/amazonaws/encryptionsdk/model/CiphertextHeadersTest.java @@ -431,7 +431,7 @@ public void invalidFrameLength() { readUptoNonceLen(headerBuff); - // set frame type to an invalid value + // set content type to an invalid value headerBuff.putInt(-1); final CiphertextHeaders reconstructedHeaders = new CiphertextHeaders(); From 6e38615af4e331ffc35f85c1823b4b9c7ef47b39 Mon Sep 17 00:00:00 2001 From: Wesley Rosenblum Date: Thu, 2 Apr 2020 10:54:38 -0700 Subject: [PATCH 5/5] Fix spacing after if --- .../encryptionsdk/internal/FrameDecryptionHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java b/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java index b7e3f4c8c..2cfd14a1a 100644 --- a/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java +++ b/src/main/java/com/amazonaws/encryptionsdk/internal/FrameDecryptionHandler.java @@ -135,7 +135,7 @@ public ProcessingSummary processBytes(final byte[] in, final int off, final int protectedContentLen = currentFrameHeaders_.getFrameContentLength(); // The final frame should not be able to exceed the frameLength - if(frameSize_ > 0 && protectedContentLen > frameSize_) { + if (frameSize_ > 0 && protectedContentLen > frameSize_) { throw new BadCiphertextException("Final frame length exceeds frame length."); } } else {