# HG changeset patch # User jnimeh # Date 1566048049 25200 # Node ID 34bbd91b15223d5115e2436ef4f547d009547a11 # Parent 094ef5a91b68261b7fb7f424f2b91cbdaaff2851 8224997: ChaCha20-Poly1305 TLS cipher suite decryption throws ShortBufferException Reviewed-by: xuelei diff -r 094ef5a91b68 -r 34bbd91b1522 src/java.base/share/classes/com/sun/crypto/provider/ChaCha20Cipher.java --- 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; diff -r 094ef5a91b68 -r 34bbd91b1522 test/jdk/com/sun/crypto/provider/Cipher/ChaCha20/OutputSizeTest.java --- /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; + } +} +