Skip to content

Commit 8b47497

Browse files
author
Artur Barashev
committedNov 4, 2024
8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server
Reviewed-by: wetmore, djelinski, xuelei
·
jdk-25+17jdk-24-ga
1 parent 0668e18 commit 8b47497

File tree

4 files changed

+470
-5
lines changed

4 files changed

+470
-5
lines changed
 

‎src/java.base/share/classes/sun/security/ssl/SSLCipher.java

+15-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -1859,10 +1859,20 @@ public Plaintext decrypt(byte contentType, ByteBuffer bb,
18591859
}
18601860

18611861
if (bb.remaining() <= tagSize) {
1862-
throw new BadPaddingException(
1863-
"Insufficient buffer remaining for AEAD cipher " +
1864-
"fragment (" + bb.remaining() + "). Needs to be " +
1865-
"more than tag size (" + tagSize + ")");
1862+
// Check for unexpected plaintext alert.
1863+
if (contentType == ContentType.ALERT.id
1864+
&& bb.remaining() == 2) {
1865+
throw new GeneralSecurityException(String.format(
1866+
"Unexpected plaintext alert received: " +
1867+
"Level: %s; Alert: %s",
1868+
Alert.Level.nameOf(bb.get(bb.position())),
1869+
Alert.nameOf(bb.get(bb.position() + 1))));
1870+
} else {
1871+
throw new BadPaddingException(
1872+
"Insufficient buffer remaining for AEAD cipher " +
1873+
"fragment (" + bb.remaining() + "). Needs to be " +
1874+
"more than tag size (" + tagSize + ")");
1875+
}
18661876
}
18671877

18681878
byte[] sn = sequence;

‎test/jdk/sun/security/ssl/SSLCipher/SSLEngineNoServerHelloClientShutdown.java

+248
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,248 @@
1+
/*
2+
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
// SunJSSE does not support dynamic system properties, no way to re-use
25+
// system properties in samevm/agentvm mode.
26+
27+
/*
28+
* @test
29+
* @bug 8331682
30+
* @summary Slow networks/Impatient clients can potentially send
31+
* unencrypted TLSv1.3 alerts that won't parse on the server.
32+
* @library /javax/net/ssl/templates /test/lib
33+
* @run main/othervm SSLEngineNoServerHelloClientShutdown
34+
*/
35+
36+
import static jdk.test.lib.Asserts.assertEquals;
37+
import static jdk.test.lib.Asserts.fail;
38+
import static jdk.test.lib.security.SecurityUtils.inspectTlsBuffer;
39+
40+
import java.nio.ByteBuffer;
41+
import java.security.GeneralSecurityException;
42+
43+
import javax.net.ssl.SSLEngine;
44+
import javax.net.ssl.SSLEngineResult;
45+
import javax.net.ssl.SSLEngineResult.HandshakeStatus;
46+
import javax.net.ssl.SSLProtocolException;
47+
import javax.net.ssl.SSLSession;
48+
49+
/**
50+
* A SSLEngine usage example which simplifies the presentation
51+
* by removing the I/O and multi-threading concerns.
52+
* <p>
53+
* The test creates two SSLEngines, simulating a client and server.
54+
* The "transport" layer consists two byte buffers: think of them
55+
* as directly connected pipes.
56+
* <p>
57+
* When this application runs, notice that several messages
58+
* (wrap/unwrap) pass before any application data is consumed or
59+
* produced.
60+
*/
61+
public class SSLEngineNoServerHelloClientShutdown extends SSLContextTemplate {
62+
63+
protected static final String EXCEPTION_MSG =
64+
"Unexpected plaintext alert received: " +
65+
"Level: warning; Alert: user_canceled";
66+
67+
protected SSLEngine clientEngine; // client Engine
68+
protected ByteBuffer clientOut; // write side of clientEngine
69+
protected final ByteBuffer clientIn; // read side of clientEngine
70+
71+
protected final SSLEngine serverEngine; // server Engine
72+
protected final ByteBuffer serverOut; // write side of serverEngine
73+
protected final ByteBuffer serverIn; // read side of serverEngine
74+
75+
protected ByteBuffer cTOs; // "reliable" transport client->server
76+
protected final ByteBuffer sTOc; // "reliable" transport server->client
77+
78+
protected SSLEngineNoServerHelloClientShutdown() throws Exception {
79+
serverEngine = configureServerEngine(
80+
createServerSSLContext().createSSLEngine());
81+
82+
clientEngine = configureClientEngine(
83+
createClientSSLContext().createSSLEngine());
84+
85+
// We'll assume the buffer sizes are the same between client and server.
86+
SSLSession session = clientEngine.getSession();
87+
int appBufferMax = session.getApplicationBufferSize();
88+
int netBufferMax = session.getPacketBufferSize();
89+
90+
// We'll make the input buffers a bit bigger than the max needed
91+
// size, so that unwrap()s following a successful data transfer
92+
// won't generate BUFFER_OVERFLOWS.
93+
clientIn = ByteBuffer.allocate(appBufferMax + 50);
94+
serverIn = ByteBuffer.allocate(appBufferMax + 50);
95+
96+
cTOs = ByteBuffer.allocateDirect(netBufferMax * 2);
97+
// Make it larger so subsequent server wraps won't generate
98+
// BUFFER_OVERFLOWS
99+
sTOc = ByteBuffer.allocateDirect(netBufferMax * 2);
100+
101+
clientOut = createClientOutputBuffer();
102+
serverOut = createServerOutputBuffer();
103+
}
104+
105+
protected ByteBuffer createServerOutputBuffer() {
106+
return ByteBuffer.wrap("Hello Client, I'm Server".getBytes());
107+
}
108+
109+
protected ByteBuffer createClientOutputBuffer() {
110+
return ByteBuffer.wrap("Hi Server, I'm Client".getBytes());
111+
}
112+
113+
/*
114+
* Configure the client side engine.
115+
*/
116+
protected SSLEngine configureClientEngine(SSLEngine clientEngine) {
117+
clientEngine.setUseClientMode(true);
118+
clientEngine.setEnabledProtocols(new String[] {"TLSv1.3"});
119+
return clientEngine;
120+
}
121+
122+
/*
123+
* Configure the server side engine.
124+
*/
125+
protected SSLEngine configureServerEngine(SSLEngine serverEngine) {
126+
serverEngine.setUseClientMode(false);
127+
serverEngine.setNeedClientAuth(true);
128+
return serverEngine;
129+
}
130+
131+
public static void main(String[] args) throws Exception {
132+
new SSLEngineNoServerHelloClientShutdown().runTestUserCancelled();
133+
}
134+
135+
//
136+
// Private methods that used to build the common part of the test.
137+
//
138+
139+
private void runTestUserCancelled() throws Exception {
140+
SSLEngineResult clientResult;
141+
SSLEngineResult serverResult;
142+
143+
log("=================");
144+
145+
// Client: produce client_hello
146+
log("---Client Wrap client_hello---");
147+
clientResult = clientEngine.wrap(clientOut, cTOs);
148+
logEngineStatus(clientEngine, clientResult);
149+
runDelegatedTasks(clientEngine);
150+
151+
cTOs.flip();
152+
153+
// Server: consume client_hello
154+
log("---Server Unwrap client_hello---");
155+
serverResult = serverEngine.unwrap(cTOs, serverIn);
156+
logEngineStatus(serverEngine, serverResult);
157+
runDelegatedTasks(serverEngine);
158+
159+
cTOs.compact();
160+
161+
// Server: produce server_hello
162+
log("---Server Wrap server_hello---");
163+
serverResult = serverEngine.wrap(serverOut, sTOc);
164+
logEngineStatus(serverEngine, serverResult);
165+
runDelegatedTasks(serverEngine);
166+
// SH packet went missing. Timeout on Client.
167+
168+
// Server: produce other outbound messages
169+
log("---Server Wrap---");
170+
serverResult = serverEngine.wrap(serverOut, sTOc);
171+
logEngineStatus(serverEngine, serverResult);
172+
runDelegatedTasks(serverEngine);
173+
// CCS packet went missing. Timeout on Client.
174+
175+
// Server: produce other outbound messages
176+
log("---Server Wrap---");
177+
serverResult = serverEngine.wrap(serverOut, sTOc);
178+
logEngineStatus(serverEngine, serverResult);
179+
runDelegatedTasks(serverEngine);
180+
// EE/etc. packet went missing. Timeout on Client.
181+
182+
// Shutdown client
183+
log("---Client closeOutbound---");
184+
clientEngine.closeOutbound();
185+
186+
// Client: produce an unencrypted user_canceled
187+
log("---Client Wrap user_canceled---");
188+
clientResult = clientEngine.wrap(clientOut, cTOs);
189+
logEngineStatus(clientEngine, clientResult);
190+
runDelegatedTasks(clientEngine);
191+
192+
cTOs.flip();
193+
inspectTlsBuffer(cTOs);
194+
195+
// Server unwrap should throw a proper exception when receiving an
196+
// unencrypted 2 byte packet user_canceled alert.
197+
log("---Server Unwrap user_canceled alert---");
198+
try {
199+
serverEngine.unwrap(cTOs, serverIn);
200+
} catch (SSLProtocolException e) {
201+
assertEquals(
202+
GeneralSecurityException.class, e.getCause().getClass());
203+
assertEquals(EXCEPTION_MSG, e.getCause().getMessage());
204+
return;
205+
}
206+
fail("Server should have thrown SSLProtocolException");
207+
}
208+
209+
protected static void logEngineStatus(SSLEngine engine) {
210+
log("\tCurrent HS State: " + engine.getHandshakeStatus());
211+
log("\tisInboundDone() : " + engine.isInboundDone());
212+
log("\tisOutboundDone(): " + engine.isOutboundDone());
213+
}
214+
215+
protected static void logEngineStatus(
216+
SSLEngine engine, SSLEngineResult result) {
217+
log("\tResult Status : " + result.getStatus());
218+
log("\tResult HS Status : " + result.getHandshakeStatus());
219+
log("\tEngine HS Status : " + engine.getHandshakeStatus());
220+
log("\tisInboundDone() : " + engine.isInboundDone());
221+
log("\tisOutboundDone() : " + engine.isOutboundDone());
222+
log("\tMore Result : " + result);
223+
}
224+
225+
protected static void log(String message) {
226+
System.err.println(message);
227+
}
228+
229+
// If the result indicates that we have outstanding tasks to do,
230+
// go ahead and run them in this thread.
231+
protected static void runDelegatedTasks(SSLEngine engine)
232+
throws Exception {
233+
234+
if (engine.getHandshakeStatus() == HandshakeStatus.NEED_TASK) {
235+
Runnable runnable;
236+
while ((runnable = engine.getDelegatedTask()) != null) {
237+
log(" running delegated task...");
238+
runnable.run();
239+
}
240+
HandshakeStatus hsStatus = engine.getHandshakeStatus();
241+
if (hsStatus == HandshakeStatus.NEED_TASK) {
242+
throw new Exception(
243+
"handshake shouldn't need additional tasks");
244+
}
245+
logEngineStatus(engine);
246+
}
247+
}
248+
}

‎test/jdk/sun/security/ssl/SSLCipher/SSLSocketNoServerHelloClientShutdown.java

+177
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
/*
2+
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/*
25+
* @test
26+
* @bug 8331682
27+
* @summary Slow networks/Impatient clients can potentially send
28+
* unencrypted TLSv1.3 alerts that won't parse on the server.
29+
* @library /javax/net/ssl/templates /test/lib
30+
* @run main/othervm SSLSocketNoServerHelloClientShutdown
31+
*/
32+
33+
import static jdk.test.lib.Asserts.assertEquals;
34+
import static jdk.test.lib.Asserts.assertTrue;
35+
import static jdk.test.lib.Asserts.fail;
36+
import static jdk.test.lib.security.SecurityUtils.inspectTlsBuffer;
37+
38+
import java.io.InputStream;
39+
import java.net.InetSocketAddress;
40+
import java.nio.channels.SocketChannel;
41+
import java.security.GeneralSecurityException;
42+
43+
import javax.net.ssl.SSLContext;
44+
import javax.net.ssl.SSLEngineResult;
45+
import javax.net.ssl.SSLEngineResult.Status;
46+
import javax.net.ssl.SSLProtocolException;
47+
import javax.net.ssl.SSLServerSocket;
48+
import javax.net.ssl.SSLServerSocketFactory;
49+
import javax.net.ssl.SSLSocket;
50+
51+
/**
52+
* To reproduce @bug 8331682 (client sends an unencrypted TLS alert during
53+
* TLSv1.3 handshake) with SSLSockets we use an SSLSocket on the server side
54+
* and a plain TCP socket backed by SSLEngine on the client side.
55+
*/
56+
public class SSLSocketNoServerHelloClientShutdown
57+
extends SSLEngineNoServerHelloClientShutdown {
58+
59+
private volatile Exception clientException;
60+
private volatile Exception serverException;
61+
62+
public static void main(String[] args) throws Exception {
63+
new SSLSocketNoServerHelloClientShutdown().runTest();
64+
}
65+
66+
public SSLSocketNoServerHelloClientShutdown() throws Exception {
67+
super();
68+
}
69+
70+
private void runTest() throws Exception {
71+
// Set up SSL server
72+
SSLContext context = createServerSSLContext();
73+
SSLServerSocketFactory sslssf = context.getServerSocketFactory();
74+
75+
try (SSLServerSocket serverSocket =
76+
(SSLServerSocket) sslssf.createServerSocket()) {
77+
78+
serverSocket.setReuseAddress(false);
79+
serverSocket.bind(null);
80+
int port = serverSocket.getLocalPort();
81+
log("Port: " + port);
82+
Thread thread = createClientThread(port);
83+
84+
try {
85+
// Server-side SSL socket that will read.
86+
SSLSocket socket = (SSLSocket) serverSocket.accept();
87+
socket.setSoTimeout(2000);
88+
InputStream is = socket.getInputStream();
89+
byte[] inbound = new byte[512];
90+
91+
log("===Server is ready and reading===");
92+
if (is.read(inbound) > 0) {
93+
throw new Exception("Server returned data");
94+
}
95+
} catch (Exception e) {
96+
serverException = e;
97+
log(e.toString());
98+
} finally {
99+
thread.join();
100+
}
101+
} finally {
102+
if (serverException != null) {
103+
assertEquals(
104+
SSLProtocolException.class, serverException.getClass());
105+
assertEquals(GeneralSecurityException.class,
106+
serverException.getCause().getClass());
107+
assertEquals(
108+
EXCEPTION_MSG, serverException.getCause().getMessage());
109+
} else {
110+
fail("Server should have thrown SSLProtocolException");
111+
}
112+
if (clientException != null) {
113+
throw clientException;
114+
}
115+
}
116+
}
117+
118+
private Thread createClientThread(final int port) {
119+
120+
Thread t = new Thread("ClientThread") {
121+
@Override
122+
public void run() {
123+
// Client-side plain TCP socket.
124+
try (SocketChannel clientSocketChannel = SocketChannel.open(
125+
new InetSocketAddress("localhost", port))) {
126+
127+
SSLEngineResult clientResult;
128+
clientSocketChannel.socket().setSoTimeout(500);
129+
130+
log("=================");
131+
132+
// Produce client_hello
133+
log("---Client Wrap client_hello---");
134+
clientResult = clientEngine.wrap(clientOut, cTOs);
135+
logEngineStatus(clientEngine, clientResult);
136+
runDelegatedTasks(clientEngine);
137+
138+
// Shutdown client
139+
log("---Client closeOutbound---");
140+
clientEngine.closeOutbound();
141+
142+
// Produce an unencrypted user_canceled
143+
log("---Client Wrap user_canceled---");
144+
clientResult = clientEngine.wrap(clientOut, cTOs);
145+
logEngineStatus(clientEngine, clientResult);
146+
runDelegatedTasks(clientEngine);
147+
148+
// Produce an unencrypted close_notify
149+
log("---Client Wrap close_notify---");
150+
clientResult = clientEngine.wrap(clientOut, cTOs);
151+
logEngineStatus(clientEngine, clientResult);
152+
runDelegatedTasks(clientEngine);
153+
assertTrue(clientEngine.isOutboundDone());
154+
assertEquals(clientResult.getStatus(), Status.CLOSED);
155+
156+
// Send client_hello, user_canceled alert and close_notify
157+
// alert to server. Server should throw a proper exception
158+
// when receiving an unencrypted 2 byte packet user_canceled
159+
// alert.
160+
cTOs.flip();
161+
inspectTlsBuffer(cTOs);
162+
log("---Client sends unencrypted alerts---");
163+
int len = clientSocketChannel.write(cTOs);
164+
165+
// Give server a chance to read before we shutdown via
166+
// the try-with-resources block.
167+
Thread.sleep(2000);
168+
} catch (Exception e) {
169+
clientException = e;
170+
}
171+
}
172+
};
173+
174+
t.start();
175+
return t;
176+
}
177+
}

‎test/lib/jdk/test/lib/security/SecurityUtils.java

+30
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
package jdk.test.lib.security;
2525

2626
import java.io.File;
27+
import java.io.IOException;
28+
import java.nio.ByteBuffer;
2729
import java.security.KeyStore;
2830
import java.security.Security;
2931
import java.util.Arrays;
@@ -185,5 +187,33 @@ private static boolean anyMatch(String value, List<String> algs) {
185187
return false;
186188
}
187189

190+
public static void inspectTlsBuffer(ByteBuffer buffer) throws IOException {
191+
if (buffer == null || !buffer.hasRemaining()) {
192+
return;
193+
}
194+
195+
ByteBuffer packet = buffer.slice();
196+
System.err.printf("---TLS Buffer Inspection. Bytes Remaining: %d---\n",
197+
packet.remaining());
198+
199+
for (int i = 1; packet.position() < packet.limit(); i++) {
200+
byte contentType = packet.get(); // pos: 0
201+
byte majorVersion = packet.get(); // pos: 1
202+
byte minorVersion = packet.get(); // pos: 2
203+
int contentLen = getInt16(packet); // pos: 3, 4
204+
205+
System.err.printf(
206+
"Flight %d: contentType: %d; majorVersion: %d; "
207+
+ "minorVersion: %d; contentLen: %d\n", i, (int) contentType,
208+
(int) majorVersion, (int) minorVersion, contentLen);
209+
210+
packet.position(packet.position() + contentLen);
211+
}
212+
}
213+
214+
public static int getInt16(ByteBuffer m) throws IOException {
215+
return ((m.get() & 0xFF) << 8) | (m.get() & 0xFF);
216+
}
217+
188218
private SecurityUtils() {}
189219
}

0 commit comments

Comments
 (0)
Please sign in to comment.