- Type:
Bug
- Resolution: Fixed
- Priority:
P3 - Affects Version/s:11
- Component/s:security-libs
- b23
- Verified
| Issue | Fix Version | Assignee | Priority | Status | Resolution | Resolved In Build |
|---|---|---|---|---|---|---|
| JDK-8277709 | 17.0.3-oracle | Prajwal Kumaraswamy | P1 | Resolved | Fixed | b01 |
| JDK-8277633 | 17.0.2 | Prajwal Kumaraswamy | P1 | Closed | Fixed | b06 |
| JDK-8281530 | 15.0.7 | Yuri Nesterenko | P3 | Resolved | Fixed | b02 |
| JDK-8281534 | 13.0.11 | Yuri Nesterenko | P3 | Resolved | Fixed | b02 |
| JDK-8277636 | 11.0.15-oracle | Prajwal Kumaraswamy | P1 | Closed | Fixed | b01 |
| JDK-8279012 | 11.0.15 | Zhengyu Gu | P3 | Resolved | Fixed | b01 |
| JDK-8278070 | 8u331 | Prajwal Kumaraswamy | P1 | Closed | Fixed | b01 |
from SSLSockets causes SSLSessions to be invalidated unnecessarily. The
problem happens when one thread calls SSLSocketImpl.close() while another
thread is in SSLSocketImpl.AppInputStream.read().
Before it attempts to read from the socket, the implementation of
SSLSocketImpl.AppInputStream.read() checks whether the underlying socket is
closed and if it is, a SocketException will be thrown without invalidating
the SSLSession. If the socket is open, the code continues and tries to read
from the socket.
The problem happens when another thread closes the socket after the closed
check in SSLSocketImpl.AppInputStream.read(). When this happens, the attempt
to read from the Socket will trigger a SocketException, which is caught and
handled as a fatal condition, and the SSLSession is invalidated. This seems
to be incorrect - as described above, if the socket is closed just a moment
before, it is handled gracefully and the SSLSession is not invalidated.
The code in the testcase below is very contrived to try and force the issue
to occur, but we have seen this in the wild with LDAP connections, where the
reads are handled on an LDAP connection thread and another thread can close
the socket by closing the LDAP context. We have even seen this when the LDAP
context is not closed explicitly, with the finalizer thread doing the close.
If the SSLSession is invalidated, the next SSLSocket opened against the same
server requires a new handshake instead of resuming the previous SSLSession.
These extra handshakes lead to increased CPU usage and worse performance.
SUGGESTED FIX
-------------
The suggested fix is to catch any SocketExceptions thrown when reading from the socket and simply rethrow them, instead of handling them as fatal conditions and invalidating the SSLSessions.
There is lots of synchronization in the SSLSocketImpl implementation, but none of it seems to prevent closing and reading from the underlying socket at the same time. It *might* be possible to address this with some additional synchronization, but this will be difficult because the socket could also be closed by the peer at just the wrong moment. This would trigger a "socket reset" SocketException instead of "socket closed", but the end result would still be the same.
REPRODUCTION INSTRUCTIONS
-------------------------
> javac ReadCloseSSLSockets.java
> java ReadCloseSSLSockets
In the output, note that SSLSessions are invalidated instead of being re-used:
==========================
436765411266600: Main Thread: OpenedSSLSocket@00d94a8b
436765411505400: Main Thread: Started handshake onSSLSocket@00d94a8b
javax.net.ssl|ALL|01| Main Thread|2021-09-28 10:28:19.576 BST|SSLSessionImpl.java:220|Session initialized: Session(1632821299576|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)
436765585578700: Main Thread: Finished handshake onSSLSocket@00d94a8b
436765585993900: Main Thread: *** OPENED NEW SESSION ***: Session(1632821299576|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)
436765586474400: Reader Thread: Started reading fromSSLSocket@00d94a8b
436765605629400: Main Thread: ClosingSSLSocket@00d94a8b
436765609877400: Main Thread: ClosedSSLSocket@00d94a8b
javax.net.ssl|ALL|08|Reader Thread|2021-09-28 10:28:19.718 BST|SSLSessionImpl.java:839|Invalidated session: Session(1632821299576|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)
436765613350000: Reader Thread: Exception reading fromSSLSocket@00d94a8b: javax.net.ssl.SSLException: java.net.SocketException: Socket closed
436766119398800: Main Thread: *** Session(1632821299576|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256) IS INVALID ***
436767146294700: Main Thread: OpenedSSLSocket@00322d26
436767146728500: Main Thread: Started handshake onSSLSocket@00322d26
javax.net.ssl|ALL|01| Main Thread|2021-09-28 10:28:21.289 BST|SSLSessionImpl.java:220|Session initialized: Session(1632821301289|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)
436767233906900: Main Thread: Finished handshake onSSLSocket@00322d26
436767234081700: Main Thread: *** OPENED NEW SESSION ***: Session(1632821301289|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)
436767235490600: Reader Thread: Started reading fromSSLSocket@00322d26
436767248896500: Main Thread: ClosingSSLSocket@00322d26
436767250400100: Main Thread: ClosedSSLSocket@00322d26
javax.net.ssl|ALL|08|Reader Thread|2021-09-28 10:28:21.358 BST|SSLSessionImpl.java:839|Invalidated session: Session(1632821301289|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)
436767252442500: Reader Thread: Exception reading fromSSLSocket@00322d26: javax.net.ssl.SSLException: java.net.SocketException: Socket closed
436767761053200: Main Thread: *** Session(1632821301289|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256) IS INVALID ***
==========================
The testcase can also be run in single threaded mode like this:
> java ReadCloseSSLSockets single
The output shows that the closed socket is not handled as a fatal condition if the close doesn't happen at the "wrong" moment:
==========================
435390504657400: Main Thread: OpenedSSLSocket@00d94a8b
435390505082600: Main Thread: Started handshake onSSLSocket@00d94a8b
javax.net.ssl|ALL|01| Main Thread|2021-09-28 10:05:24.789 BST|SSLSessionImpl.java:220|Session initialized: Session(1632819924787|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)
435390784052800: Main Thread: Finished handshake onSSLSocket@00d94a8b
435390784427000: Main Thread: *** OPENED NEW SESSION ***: Session(1632819924787|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)
435390784647000: Main Thread: ClosingSSLSocket@00d94a8b
435390785825700: Main Thread: ClosedSSLSocket@00d94a8b
435390786017500: Main Thread: Started reading fromSSLSocket@00d94a8b
435390786250000: Main Thread: Finished reading fromSSLSocket@00d94a8b
435390786468100: Main Thread: *** Session(1632819924787|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256) IS VALID ***
435391823632700: Main Thread: OpenedSSLSocket@00322d26
435391824545900: Main Thread: Started handshake onSSLSocket@00322d26
435391869696700: Main Thread: Finished handshake onSSLSocket@00322d26
435391870646900: Main Thread: *** RE-USED PREVIOUS SESSION ***: Session(1632819924787|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256))
435391871107900: Main Thread: ClosingSSLSocket@00322d26
435391872033500: Main Thread: ClosedSSLSocket@00322d26
435391872464600: Main Thread: Started reading fromSSLSocket@00322d26
435391872899100: Main Thread: Finished reading fromSSLSocket@00322d26
435391873318400: Main Thread: *** Session(1632819924787|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256) IS VALID ***
==========================
Note that the CLOSE_DELAY value may need to be tweaked to get the timing right in different environments. Setting it to 0 makes
the problem more intermittent, and the results vary by release. For example, when I test in my environment on 8u261 with
CLOSE_DELAY = 0, I see what happens when the check in SSLSocketImpl.AppInputStream.read() detects that the socket is closed
and handles it gracefully by throwing a SocketException without invalidating the session. For example:
==========================
438032838812500: Main Thread: OpenedSSLSocket@566776ad
438032839276500: Main Thread: Started handshake onSSLSocket@566776ad
438032976327800: Main Thread: Finished handshake onSSLSocket@566776ad
438032976736400: Main Thread: *** OPENED NEW SESSION ***: [Session-2, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256]
438032977193900: Main Thread: ClosingSSLSocket@566776ad
438032977246200: Reader Thread: Started reading fromSSLSocket@566776ad
438032978115500: Main Thread: ClosedSSLSocket@566776ad
438032978296400: Reader Thread: Exception reading fromSSLSocket@566776ad: java.net.SocketException: Socket is closed
438033491791200: Main Thread: *** [Session-2, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256] IS VALID ***
438034523456500: Main Thread: OpenedSSLSocket@14acaea5
438034523784700: Main Thread: Started handshake onSSLSocket@14acaea5
438034547768100: Main Thread: Finished handshake onSSLSocket@14acaea5
438034548001500: Main Thread: *** RE-USING PREVIOUS SESSION ***: [Session-2, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256])
438034548481500: Main Thread: ClosingSSLSocket@14acaea5
438034548488400: Reader Thread: Started reading fromSSLSocket@14acaea5
438034550469400: Main Thread: ClosedSSLSocket@14acaea5
438034550799800: Reader Thread: Exception reading fromSSLSocket@14acaea5: java.net.SocketException: Socket is closed
438035053576100: Main Thread: *** [Session-2, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256] IS VALID ***
==========================
TESTCASE
--------
import java.io.IOException;
import java.io.InputStream;
import javax.net.ssl.SSLSocket;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLSession;
import java.security.NoSuchAlgorithmException;
public class ReadCloseSSLSockets {
private static final String HOSTNAME = "www.google.com";
private static final int PORT = 443;
private static final int ITERATIONS = 5;
// This controls how long the main thread waits before closing the socket.
// This may need tweaking for different environments to get the timing right.
private static final int CLOSE_DELAY = 10;
private static SSLSocket theSSLSocket;
private static SSLSession theSSLSession;
private static InputStream theInputStream;
private static String theSSLSocketHashCode;
private static SSLSession lastSSLSession;
private static volatile boolean readFromSocket = false;
private static volatile boolean finished = false;
private static boolean multiThreaded = true;
public static void main(String[] args) {
if (args.length != 0) {
if (args[0].equals("single")) {
multiThreaded = false;
}
}
if (System.getProperty("javax.net.debug") == null) {
System.setProperty("javax.net.debug", "session");
}
Thread.currentThread().setName(" Main Thread");
if (multiThreaded) {
// Create the reader thread
ReaderThread readerThread = new ReaderThread();
readerThread.setName("Reader Thread");
readerThread.start();
}
try {
for (int i = 0; i < ITERATIONS; i++) {
openSSLSocket();
doHandshake();
getInputStream();
getAndCompareSession();
if (multiThreaded) {
readCloseMultiThreaded();
} else {
readCloseSingleThreaded();
}
isSessionValid();
lastSSLSession = theSSLSession;
// Insert a short gap between iterations
Thread.sleep(1000);
System.out.println();
}
} catch (Exception e) {
logToConsole("Unexpected Exception: " + e);
} finally {
if (multiThreaded) {
// Tell the reader thread to finish
finished = true;
}
}
}
private static void readCloseMultiThreaded() throws IOException, InterruptedException {
// Tell the reader thread to start trying to read from this socket
readFromSocket = true;
// Short pause to give the reader thread time to start reading.
if (CLOSE_DELAY > 0) {
Thread.sleep(CLOSE_DELAY);
}
// The problem happens when the reader thread tries to read
// from the socket while this thread is in the close() call
closeSSLSocket();
// Pause to give the reader thread time to discover that the
// socket is closed and throw a SocketException
Thread.sleep(500);
}
private static void readCloseSingleThreaded() throws IOException, InterruptedException {
closeSSLSocket();
// Try to read from the socket now that it's closed. This will throw
// a SocketException, but the SSLSession won't be invalidated.
try {
readFromSSLSocket();
} catch (Exception e) {
logToConsole("Exception reading from SSLSocket@" + theSSLSocketHashCode + ": " + e);
}
}
private static class ReaderThread extends Thread {
public void run() {
// This thread runs in a tight loop until readFromSocket == true
while (!finished) {
if (readFromSocket) {
int result = 0;
try {
// If the timing is just right, this will throw a SocketException
// and the SSLSession will be invalidated.
result = readFromSSLSocket();
} catch (Exception e) {
logToConsole("Exception reading from SSLSocket@" + theSSLSocketHashCode + ": " + e);
// Stop trying to read from the socket now
readFromSocket = false;
}
if (result == -1) {
logToConsole("Reached end of stream reading from SSLSocket@" + theSSLSocketHashCode);
// Stop trying to read from the socket now
readFromSocket = false;
}
}
}
}
}
private static void openSSLSocket() throws IOException, NoSuchAlgorithmException {
theSSLSocket = (SSLSocket) SSLContext.getDefault().getSocketFactory().createSocket(HOSTNAME, PORT);
theSSLSocketHashCode = String.format("%08x", theSSLSocket.hashCode());
logToConsole("Opened SSLSocket@" + theSSLSocketHashCode);
}
private static void doHandshake() throws IOException {
logToConsole("Started handshake on SSLSocket@" + theSSLSocketHashCode);
theSSLSocket.startHandshake();
logToConsole("Finished handshake on SSLSocket@" + theSSLSocketHashCode);
}
private static void getInputStream() throws IOException {
theInputStream = theSSLSocket.getInputStream();
}
private static void getAndCompareSession() {
theSSLSession = theSSLSocket.getSession();
// Have we opened a new session or re-used the last one?
if (lastSSLSession == null || !theSSLSession.equals(lastSSLSession)) {
logToConsole("*** OPENED NEW SESSION ***: " + theSSLSession);
} else {
logToConsole("*** RE-USING PREVIOUS SESSION ***: " + theSSLSession + ")");
}
}
private static void closeSSLSocket() throws IOException {
logToConsole("Closing SSLSocket@" + theSSLSocketHashCode);
theSSLSocket.close();
logToConsole("Closed SSLSocket@" + theSSLSocketHashCode);
}
private static int readFromSSLSocket() throws Exception {
logToConsole("Started reading from SSLSocket@" + theSSLSocketHashCode);
int result = theInputStream.read();
logToConsole("Finished reading from SSLSocket@" + theSSLSocketHashCode + ": result = " + result);
return result;
}
private static void isSessionValid() {
// Is the session still valid?
if (theSSLSession.isValid()) {
logToConsole("*** " + theSSLSession + " IS VALID ***");
} else {
logToConsole("*** " + theSSLSession + " IS INVALID ***");
}
}
private static void logToConsole(String s) {
System.out.println(System.nanoTime() + ": " + Thread.currentThread().getName() + ": " + s);
}
}
- backported by
JDK-8277709Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily

- Resolved
JDK-8279012Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily

- Resolved
JDK-8281530Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily

- Resolved
JDK-8281534Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily

- Resolved
JDK-8277633Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily

- Closed
JDK-8277636Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily

- Closed
JDK-8278070Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily

- Closed
- relates to
JDK-8287739[11u] ProblemList sun/security/ssl/SSLSessionImpl/NoInvalidateSocketException.java

- Resolved
JDK-8280158New test from JDK-8274736 failed with/without patch in JDK11u

- Resolved
JDK-8274524SSLSocket.close() hangs if it is called during the ssl handshake

- Closed
JDK-8277970Test jdk/sun/security/ssl/SSLSessionImpl/NoInvalidateSocketException.java fails with "tag mismatch"

- Closed
- links to
Commitopenjdk/jdk11u-dev/ec89f1b6
Commitopenjdk/jdk13u-dev/d959f811
Commitopenjdk/jdk15u-dev/c9a3110b
Commitopenjdk/jdk17u/6f2bdc01
Commitopenjdk/jdk/8822d41f
Reviewopenjdk/jdk11u-dev/711
Reviewopenjdk/jdk13u-dev/324
Reviewopenjdk/jdk15u-dev/172
Reviewopenjdk/jdk17u/294
Reviewopenjdk/jdk/6197
- Assignee:
Jamil Nimeh
- Reporter:
Shadow Bug
- Votes:
- 0Vote for this issue
- Watchers:
- 14Start watching this issue
- Created:
- Updated:
- Resolved: