8221253: TLSv1.3 may generate TLSInnerPlainText longer than 2^14+1 bytes

Reviewed-by: jnimeh
This commit is contained in:
Xue-Lei Andrew Fan 2019-05-10 12:33:40 -07:00
parent 032fff8042
commit 7aec6727ac
5 changed files with 138 additions and 25 deletions

View File

@ -244,9 +244,8 @@ final class DTLSOutputRecord extends OutputRecord implements DTLSRecord {
fragLen = Record.maxDataSize;
}
if (fragmentSize > 0) {
fragLen = Math.min(fragLen, fragmentSize);
}
// Calculate more impact, for example TLS 1.3 padding.
fragLen = calculateFragmentSize(fragLen);
int dstPos = destination.position();
int dstLim = destination.limit();
@ -464,9 +463,8 @@ final class DTLSOutputRecord extends OutputRecord implements DTLSRecord {
fragLen = Record.maxDataSize;
}
if (fragmentSize > 0) {
fragLen = Math.min(fragLen, fragmentSize);
}
// Calculate more impact, for example TLS 1.3 padding.
fragLen = calculateFragmentSize(fragLen);
int dstPos = dstBuf.position();
int dstLim = dstBuf.limit();

View File

@ -64,7 +64,7 @@ abstract class OutputRecord
int packetSize;
// fragment size
int fragmentSize;
private int fragmentSize;
// closed or not?
volatile boolean isClosed;
@ -293,6 +293,24 @@ abstract class OutputRecord
// shared helpers
//
private static final class T13PaddingHolder {
private static final byte[] zeros = new byte[16];
}
int calculateFragmentSize(int fragmentLimit) {
if (fragmentSize > 0) {
fragmentLimit = Math.min(fragmentLimit, fragmentSize);
}
if (protocolVersion.useTLS13PlusSpec()) {
// No negative integer checking as the fragment capacity should
// have been ensured.
return fragmentLimit - T13PaddingHolder.zeros.length - 1;
}
return fragmentLimit;
}
// Encrypt a fragment and wrap up a record.
//
// To be consistent with the spec of SSLEngine.wrap() methods, the
@ -374,8 +392,12 @@ abstract class OutputRecord
if (!encCipher.isNullCipher()) {
// inner plaintext, using zero length padding.
int endOfPt = destination.limit();
destination.limit(endOfPt + 1);
destination.put(endOfPt, contentType);
int startOfPt = destination.position();
destination.position(endOfPt);
destination.limit(endOfPt + 1 + T13PaddingHolder.zeros.length);
destination.put(contentType);
destination.put(T13PaddingHolder.zeros);
destination.position(startOfPt);
}
// use the right TLSCiphertext.opaque_type and legacy_record_version
@ -443,10 +465,6 @@ abstract class OutputRecord
}
}
private static final class T13PaddingHolder {
private static final byte[] zeros = new byte[16];
}
private long t13Encrypt(
SSLWriteCipher encCipher, byte contentType, int headerSize) {
if (!encCipher.isNullCipher()) {

View File

@ -237,9 +237,8 @@ final class SSLEngineOutputRecord extends OutputRecord implements SSLRecord {
fragLen = Record.maxDataSize;
}
if (fragmentSize > 0) {
fragLen = Math.min(fragLen, fragmentSize);
}
// Calculate more impact, for example TLS 1.3 padding.
fragLen = calculateFragmentSize(fragLen);
}
int dstPos = destination.position();
@ -444,9 +443,8 @@ final class SSLEngineOutputRecord extends OutputRecord implements SSLRecord {
fragLen = Record.maxDataSize;
}
if (fragmentSize > 0) {
fragLen = Math.min(fragLen, fragmentSize);
}
// Calculate more impact, for example TLS 1.3 padding.
fragLen = calculateFragmentSize(fragLen);
int dstPos = dstBuf.position();
int dstLim = dstBuf.limit();

View File

@ -312,9 +312,8 @@ final class SSLSocketOutputRecord extends OutputRecord implements SSLRecord {
fragLen = Record.maxDataSize;
}
if (fragmentSize > 0) {
fragLen = Math.min(fragLen, fragmentSize);
}
// Calculate more impact, for example TLS 1.3 padding.
fragLen = calculateFragmentSize(fragLen);
if (isFirstRecordOfThePayload && needToSplitPayload()) {
fragLen = 1;
@ -413,9 +412,8 @@ final class SSLSocketOutputRecord extends OutputRecord implements SSLRecord {
fragLimit = Record.maxDataSize;
}
if (fragmentSize > 0) {
fragLimit = Math.min(fragLimit, fragmentSize);
}
// Calculate more impact, for example TLS 1.3 padding.
fragLimit = calculateFragmentSize(fragLimit);
return fragLimit;
}

View File

@ -0,0 +1,101 @@
/*
* 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.
*/
//
// Please run in othervm mode. SunJSSE does not support dynamic system
// properties, no way to re-use system properties in samevm/agentvm mode.
//
/*
* @test
* @bug 8221253
* @summary TLSv1.3 may generate TLSInnerPlainText longer than 2^14+1 bytes
* @modules jdk.crypto.ec
* @library /javax/net/ssl/templates
* @run main/othervm Tls13PacketSize
*/
import java.io.InputStream;
import java.io.OutputStream;
import javax.net.ssl.SSLSocket;
public class Tls13PacketSize extends SSLSocketTemplate {
private static final byte[] appData = new byte[16385];
static {
for (int i = 0; i < appData.length; i++) {
appData[i] = (byte)('A' + (i % 26));
}
}
// Run the test case.
public static void main(String[] args) throws Exception {
(new Tls13PacketSize()).run();
}
@Override
protected void runServerApplication(SSLSocket socket) throws Exception {
// here comes the test logic
InputStream sslIS = socket.getInputStream();
OutputStream sslOS = socket.getOutputStream();
sslIS.read();
int extra = sslIS.available();
System.out.println("Server input bytes: " + extra);
// Considering the padding impact, the record plaintext is less
// than the TLSPlaintext.fragment length (2^14).
if (extra >= 16383) { // 16383: 2^14 - 1 byte read above
throw new Exception(
"Client record plaintext exceeds 2^14 octets: " + extra);
}
sslOS.write(appData);
sslOS.flush();
}
/*
* Define the client side application of the test for the specified socket.
* This method is used if the returned value of
* isCustomizedClientConnection() is false.
*
* @param socket may be null is no client socket is generated.
*
* @see #isCustomizedClientConnection()
*/
protected void runClientApplication(SSLSocket socket) throws Exception {
socket.setEnabledProtocols(new String[] {"TLSv1.3"});
InputStream sslIS = socket.getInputStream();
OutputStream sslOS = socket.getOutputStream();
sslOS.write(appData);
sslOS.flush();
sslIS.read();
int extra = sslIS.available();
System.out.println("Client input bytes: " + extra);
// Considering the padding impact, the record plaintext is less
// than the TLSPlaintext.fragment length (2^14).
if (extra >= 16383) { // 16383: 2^14 - 1 byte read above
throw new Exception(
"Server record plaintext exceeds 2^14 octets: " + extra);
}
}
}