Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8274736

Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily

XMLWordPrintable

    • b23
    • Verified

        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);
                }
        }

              jnimeh Jamil Nimeh
              shadowbug Shadow Bug
              Votes:
              0 Vote for this issue
              Watchers:
              14 Start watching this issue

                Created:
                Updated:
                Resolved: