JDK-8274736 : Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily
  • Type: Bug
  • Component: security-libs
  • Sub-Component: java.security
  • Affected Version: 11
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2021-10-04
  • Updated: 2022-06-02
  • Resolved: 2021-11-10
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 11 JDK 13 JDK 15 JDK 17 JDK 18 JDK 8
11.0.15-oracleFixed 13.0.11Fixed 15.0.7Fixed 17.0.2Fixed 18 b23Fixed 8u331Fixed
Related Reports
Relates :  
Relates :  
Relates :  
Relates :  
Description
A race in the code in sun.security.ssl.SSLSocketImpl for closing and reading
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: Opened SSLSocket@00d94a8b
436765411505400:   Main Thread: Started handshake on SSLSocket@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 on SSLSocket@00d94a8b
436765585993900:   Main Thread: *** OPENED NEW SESSION ***: Session(1632821299576|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)
436765586474400: Reader Thread: Started reading from SSLSocket@00d94a8b
436765605629400:   Main Thread: Closing SSLSocket@00d94a8b
436765609877400:   Main Thread: Closed SSLSocket@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 from SSLSocket@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: Opened SSLSocket@00322d26
436767146728500:   Main Thread: Started handshake on SSLSocket@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 on SSLSocket@00322d26
436767234081700:   Main Thread: *** OPENED NEW SESSION ***: Session(1632821301289|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)
436767235490600: Reader Thread: Started reading from SSLSocket@00322d26
436767248896500:   Main Thread: Closing SSLSocket@00322d26
436767250400100:   Main Thread: Closed SSLSocket@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 from SSLSocket@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: Opened SSLSocket@00d94a8b
435390505082600:   Main Thread: Started handshake on SSLSocket@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 on SSLSocket@00d94a8b
435390784427000:   Main Thread: *** OPENED NEW SESSION ***: Session(1632819924787|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)
435390784647000:   Main Thread: Closing SSLSocket@00d94a8b
435390785825700:   Main Thread: Closed SSLSocket@00d94a8b
435390786017500:   Main Thread: Started reading from SSLSocket@00d94a8b
435390786250000:   Main Thread: Finished reading from SSLSocket@00d94a8b
435390786468100:   Main Thread: *** Session(1632819924787|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256) IS VALID ***

435391823632700:   Main Thread: Opened SSLSocket@00322d26
435391824545900:   Main Thread: Started handshake on SSLSocket@00322d26
435391869696700:   Main Thread: Finished handshake on SSLSocket@00322d26
435391870646900:   Main Thread: *** RE-USED PREVIOUS SESSION ***: Session(1632819924787|TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256))
435391871107900:   Main Thread: Closing SSLSocket@00322d26
435391872033500:   Main Thread: Closed SSLSocket@00322d26
435391872464600:   Main Thread: Started reading from SSLSocket@00322d26
435391872899100:   Main Thread: Finished reading from SSLSocket@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: Opened SSLSocket@566776ad
438032839276500:   Main Thread: Started handshake on SSLSocket@566776ad
438032976327800:   Main Thread: Finished handshake on SSLSocket@566776ad
438032976736400:   Main Thread: *** OPENED NEW SESSION ***: [Session-2, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256]
438032977193900:   Main Thread: Closing SSLSocket@566776ad
438032977246200: Reader Thread: Started reading from SSLSocket@566776ad
438032978115500:   Main Thread: Closed SSLSocket@566776ad
438032978296400: Reader Thread: Exception reading from SSLSocket@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: Opened SSLSocket@14acaea5
438034523784700:   Main Thread: Started handshake on SSLSocket@14acaea5
438034547768100:   Main Thread: Finished handshake on SSLSocket@14acaea5
438034548001500:   Main Thread: *** RE-USING PREVIOUS SESSION ***: [Session-2, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256])
438034548481500:   Main Thread: Closing SSLSocket@14acaea5
438034548488400: Reader Thread: Started reading from SSLSocket@14acaea5
438034550469400:   Main Thread: Closed SSLSocket@14acaea5
438034550799800: Reader Thread: Exception reading from SSLSocket@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);
        }
}

Comments
verified
08-04-2022

Fix request (13u): the fix applies cleanly but one of the tests does require an additional line with a declaration. With that, all test/jdk/javax/net/ssl, test/jdk/sun/security/ssl do pass.
09-02-2022

A pull request was submitted for review. URL: https://git.openjdk.java.net/jdk15u-dev/pull/172 Date: 2022-02-09 11:10:09 +0000
09-02-2022

Fix request (15u): for parity with major releases. The patch applies cleanly, all (non-ignored) jdk/javax/net/ssl tests pass in 15u.
09-02-2022

A pull request was submitted for review. URL: https://git.openjdk.java.net/jdk13u-dev/pull/324 Date: 2022-02-09 11:56:50 +0000
09-02-2022

not for BPR inclusion on the advice of Kevin B
15-12-2021

Fix Request (11u) Backport for parity with Oracle 11.0.15. Original patch applies cleanly, but new test does not build. 11u patch has been reviewed and new test passes.
15-12-2021

Changeset: 8822d41f Author: Jamil Nimeh <jnimeh@openjdk.org> Date: 2021-11-10 01:24:33 +0000 URL: https://git.openjdk.java.net/jdk/commit/8822d41fdcc2c2d568badd72635dc587d21dbd63
10-11-2021