8224997: ChaCha20-Poly1305 TLS cipher suite decryption throws ShortBufferException
Reviewed-by: xuelei
--- a/src/java.base/share/classes/com/sun/crypto/provider/ChaCha20Cipher.java Fri Aug 16 18:27:36 2019 -0400
+++ b/src/java.base/share/classes/com/sun/crypto/provider/ChaCha20Cipher.java Sat Aug 17 06:20:49 2019 -0700
@@ -33,12 +33,11 @@
import java.nio.ByteOrder;
import java.security.*;
import java.security.spec.AlgorithmParameterSpec;
-import java.util.Arrays;
import java.util.Objects;
+import javax.crypto.*;
import javax.crypto.spec.ChaCha20ParameterSpec;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.SecretKeySpec;
-import javax.crypto.*;
import sun.security.util.DerValue;
/**
@@ -134,8 +133,7 @@
/**
* Default constructor.
*/
- protected ChaCha20Cipher() {
- }
+ protected ChaCha20Cipher() { }
/**
* Set the mode of operation. Since this is a stream cipher, there
@@ -185,11 +183,13 @@
}
/**
- * Get the output size based on an input length. In simple stream-cipher
+ * Get the output size required to hold the result of the next update or
+ * doFinal operation. In simple stream-cipher
* mode, the output size will equal the input size. For ChaCha20-Poly1305
* for encryption the output size will be the sum of the input length
- * and tag length. For decryption, the output size will be the input
- * length less the tag length or zero, whichever is larger.
+ * and tag length. For decryption, the output size will be the input
+ * length plus any previously unprocessed data minus the tag
+ * length, minimum zero.
*
* @param inputLen the length in bytes of the input
*
@@ -197,17 +197,7 @@
*/
@Override
protected int engineGetOutputSize(int inputLen) {
- int outLen = 0;
-
- if (mode == MODE_NONE) {
- outLen = inputLen;
- } else if (mode == MODE_AEAD) {
- outLen = (direction == Cipher.ENCRYPT_MODE) ?
- Math.addExact(inputLen, TAG_LENGTH) :
- Integer.max(inputLen - TAG_LENGTH, 0);
- }
-
- return outLen;
+ return engine.getOutputSize(inputLen, true);
}
/**
@@ -237,13 +227,10 @@
AlgorithmParameters params = null;
if (mode == MODE_AEAD) {
try {
- // Force the 12-byte nonce into a DER-encoded OCTET_STRING
- byte[] derNonce = new byte[nonce.length + 2];
- derNonce[0] = 0x04; // OCTET_STRING tag
- derNonce[1] = (byte)nonce.length; // 12-byte length;
- System.arraycopy(nonce, 0, derNonce, 2, nonce.length);
+ // Place the 12-byte nonce into a DER-encoded OCTET_STRING
params = AlgorithmParameters.getInstance("ChaCha20-Poly1305");
- params.init(derNonce);
+ params.init((new DerValue(
+ DerValue.tag_OctetString, nonce).toByteArray()));
} catch (NoSuchAlgorithmException | IOException exc) {
throw new RuntimeException(exc);
}
@@ -638,7 +625,7 @@
*/
@Override
protected byte[] engineUpdate(byte[] in, int inOfs, int inLen) {
- byte[] out = new byte[inLen];
+ byte[] out = new byte[engine.getOutputSize(inLen, false)];
try {
engine.doUpdate(in, inOfs, inLen, out, 0);
} catch (ShortBufferException | KeyException exc) {
@@ -696,7 +683,7 @@
@Override
protected byte[] engineDoFinal(byte[] in, int inOfs, int inLen)
throws AEADBadTagException {
- byte[] output = new byte[engineGetOutputSize(inLen)];
+ byte[] output = new byte[engine.getOutputSize(inLen, true)];
try {
engine.doFinal(in, inOfs, inLen, output, 0);
} catch (ShortBufferException | KeyException exc) {
@@ -1158,6 +1145,17 @@
*/
interface ChaChaEngine {
/**
+ * Size an output buffer based on the input and where applicable
+ * the current state of the engine in a multipart operation.
+ *
+ * @param inLength the input length.
+ * @param isFinal true if this is invoked from a doFinal call.
+ *
+ * @return the recommended size for the output buffer.
+ */
+ int getOutputSize(int inLength, boolean isFinal);
+
+ /**
* Perform a multi-part update for ChaCha20.
*
* @param in the input data.
@@ -1201,6 +1199,12 @@
private EngineStreamOnly () { }
@Override
+ public int getOutputSize(int inLength, boolean isFinal) {
+ // The isFinal parameter is not relevant in this kind of engine
+ return inLength;
+ }
+
+ @Override
public int doUpdate(byte[] in, int inOff, int inLen, byte[] out,
int outOff) throws ShortBufferException, KeyException {
if (initialized) {
@@ -1234,6 +1238,11 @@
private final class EngineAEADEnc implements ChaChaEngine {
+ @Override
+ public int getOutputSize(int inLength, boolean isFinal) {
+ return (isFinal ? Math.addExact(inLength, TAG_LENGTH) : inLength);
+ }
+
private EngineAEADEnc() throws InvalidKeyException {
initAuthenticator();
counter = 1;
@@ -1294,6 +1303,18 @@
private final ByteArrayOutputStream cipherBuf;
private final byte[] tag;
+ @Override
+ public int getOutputSize(int inLen, boolean isFinal) {
+ // If we are performing a decrypt-update we should always return
+ // zero length since we cannot return any data until the tag has
+ // been consumed and verified. CipherSpi.engineGetOutputSize will
+ // always set isFinal to true to get the required output buffer
+ // size.
+ return (isFinal ?
+ Integer.max(Math.addExact((inLen - TAG_LENGTH),
+ cipherBuf.size()), 0) : 0);
+ }
+
private EngineAEADDec() throws InvalidKeyException {
initAuthenticator();
counter = 1;
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/com/sun/crypto/provider/Cipher/ChaCha20/OutputSizeTest.java Sat Aug 17 06:20:49 2019 -0700
@@ -0,0 +1,181 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @bug 8224997
+ * @summary ChaCha20-Poly1305 TLS cipher suite decryption throws ShortBufferException
+ * @library /test/lib
+ * @build jdk.test.lib.Convert
+ * @run main OutputSizeTest
+ */
+
+import java.nio.ByteBuffer;
+import java.security.GeneralSecurityException;
+import java.security.Key;
+import java.security.SecureRandom;
+import javax.crypto.Cipher;
+import javax.crypto.KeyGenerator;
+import javax.crypto.spec.ChaCha20ParameterSpec;
+import javax.crypto.spec.IvParameterSpec;
+
+public class OutputSizeTest {
+
+ private static final SecureRandom SR = new SecureRandom();
+
+ public static void main(String args[]) throws Exception {
+ testCC20GetOutSize();
+ testCC20P1305GetOutSize();
+ testMultiPartAEADDec();
+ }
+
+ private static void testCC20GetOutSize()
+ throws GeneralSecurityException {
+ boolean result = true;
+ KeyGenerator kg = KeyGenerator.getInstance("ChaCha20", "SunJCE");
+ kg.init(256);
+
+ // ChaCha20 encrypt
+ Cipher cc20 = Cipher.getInstance("ChaCha20", "SunJCE");
+ cc20.init(Cipher.ENCRYPT_MODE, kg.generateKey(),
+ new ChaCha20ParameterSpec(getRandBuf(12), 10));
+
+ testOutLen(cc20, 0, 0);
+ testOutLen(cc20, 5, 5);
+ testOutLen(cc20, 5120, 5120);
+ // perform an update, then test with a final block
+ byte[] input = new byte[5120];
+ SR.nextBytes(input);
+ cc20.update(input);
+ testOutLen(cc20, 1024, 1024);
+
+ // Decryption lengths should be calculated the same way as encryption
+ cc20.init(Cipher.DECRYPT_MODE, kg.generateKey(),
+ new ChaCha20ParameterSpec(getRandBuf(12), 10));
+ testOutLen(cc20, 0, 0);
+ testOutLen(cc20, 5, 5);
+ testOutLen(cc20, 5120, 5120);
+ // perform an update, then test with a final block
+ cc20.update(input);
+ testOutLen(cc20, 1024, 1024);
+ }
+
+ private static void testCC20P1305GetOutSize()
+ throws GeneralSecurityException {
+ KeyGenerator kg = KeyGenerator.getInstance("ChaCha20", "SunJCE");
+ kg.init(256);
+
+ // ChaCha20 encrypt
+ Cipher cc20 = Cipher.getInstance("ChaCha20-Poly1305", "SunJCE");
+ cc20.init(Cipher.ENCRYPT_MODE, kg.generateKey(),
+ new IvParameterSpec(getRandBuf(12)));
+
+ // Encryption lengths are calculated as the input length plus the tag
+ // length (16).
+ testOutLen(cc20, 0, 16);
+ testOutLen(cc20, 5, 21);
+ testOutLen(cc20, 5120, 5136);
+ // perform an update, then test with a final block
+ byte[] input = new byte[5120];
+ SR.nextBytes(input);
+ cc20.update(input);
+ testOutLen(cc20, 1024, 1040);
+
+ // Decryption lengths are handled differently for AEAD mode. The length
+ // should be zero for anything up to and including the first 16 bytes
+ // (since that's the tag). Anything above that should be the input
+ // length plus any unprocessed input (via update calls), minus the
+ // 16 byte tag.
+ cc20.init(Cipher.DECRYPT_MODE, kg.generateKey(),
+ new IvParameterSpec(getRandBuf(12)));
+ testOutLen(cc20, 0, 0);
+ testOutLen(cc20, 5, 0);
+ testOutLen(cc20, 16, 0);
+ testOutLen(cc20, 5120, 5104);
+ // Perform an update, then test with a the length of a final chunk
+ // of data.
+ cc20.update(input);
+ testOutLen(cc20, 1024, 6128);
+ }
+
+ private static void testMultiPartAEADDec() throws GeneralSecurityException {
+ KeyGenerator kg = KeyGenerator.getInstance("ChaCha20", "SunJCE");
+ kg.init(256);
+ Key key = kg.generateKey();
+ IvParameterSpec ivps = new IvParameterSpec(getRandBuf(12));
+
+ // Encrypt some data so we can test decryption.
+ byte[] pText = getRandBuf(2048);
+ ByteBuffer pTextBase = ByteBuffer.wrap(pText);
+
+ Cipher enc = Cipher.getInstance("ChaCha20-Poly1305", "SunJCE");
+ enc.init(Cipher.ENCRYPT_MODE, key, ivps);
+ ByteBuffer ctBuf = ByteBuffer.allocateDirect(
+ enc.getOutputSize(pText.length));
+ enc.doFinal(pTextBase, ctBuf);
+
+ // Create a new direct plain text ByteBuffer which will catch the
+ // decrypted data.
+ ByteBuffer ptBuf = ByteBuffer.allocateDirect(pText.length);
+
+ // Set the cipher text buffer limit to roughly half the data so we can
+ // do an update/final sequence.
+ ctBuf.position(0).limit(1024);
+
+ Cipher dec = Cipher.getInstance("ChaCha20-Poly1305", "SunJCE");
+ dec.init(Cipher.DECRYPT_MODE, key, ivps);
+ dec.update(ctBuf, ptBuf);
+ System.out.println("CTBuf: " + ctBuf);
+ System.out.println("PTBuf: " + ptBuf);
+ ctBuf.limit(ctBuf.capacity());
+ dec.doFinal(ctBuf, ptBuf);
+
+ ptBuf.flip();
+ pTextBase.flip();
+ System.out.println("PT Base:" + pTextBase);
+ System.out.println("PT Actual:" + ptBuf);
+
+ if (pTextBase.compareTo(ptBuf) != 0) {
+ StringBuilder sb = new StringBuilder();
+ sb.append("Plaintext mismatch: Original: ").
+ append(pTextBase.toString()).append("\nActual :").
+ append(ptBuf);
+ throw new RuntimeException(sb.toString());
+ }
+ }
+
+ private static void testOutLen(Cipher c, int inLen, int expOut) {
+ int actualOut = c.getOutputSize(inLen);
+ if (actualOut != expOut) {
+ throw new RuntimeException("Cipher " + c + ", in: " + inLen +
+ ", expOut: " + expOut + ", actual: " + actualOut);
+ }
+ }
+
+ private static byte[] getRandBuf(int len) {
+ byte[] buf = new byte[len];
+ SR.nextBytes(buf);
+ return buf;
+ }
+}
+