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

    IssueFix VersionAssigneePriorityStatusResolutionResolved In Build
    JDK-827770917.0.3-oracle Prajwal KumaraswamyP1ResolvedFixedb01
    JDK-827763317.0.2 Prajwal KumaraswamyP1ClosedFixedb06
    JDK-828153015.0.7 Yuri NesterenkoP3ResolvedFixedb02
    JDK-828153413.0.11 Yuri NesterenkoP3ResolvedFixedb02
    JDK-827763611.0.15-oracle Prajwal KumaraswamyP1ClosedFixedb01
    JDK-827901211.0.15 Zhengyu GuP3ResolvedFixedb01
    JDK-82780708u331 Prajwal KumaraswamyP1ClosedFixedb01

      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: 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

        Backport - A issue that is required to port a Bug or Feature into another product release. This issue type is generally associated with the main Bug/Feature to represent each individual release of the port.JDK-8277709Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily

        • P1 - Blocks development and/or testing work, production could not run.
        • Resolved

        Backport - A issue that is required to port a Bug or Feature into another product release. This issue type is generally associated with the main Bug/Feature to represent each individual release of the port.JDK-8279012Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily

        • P3 - Major loss of function.
        • Resolved

        Backport - A issue that is required to port a Bug or Feature into another product release. This issue type is generally associated with the main Bug/Feature to represent each individual release of the port.JDK-8281530Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily

        • P3 - Major loss of function.
        • Resolved

        Backport - A issue that is required to port a Bug or Feature into another product release. This issue type is generally associated with the main Bug/Feature to represent each individual release of the port.JDK-8281534Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily

        • P3 - Major loss of function.
        • Resolved

        Backport - A issue that is required to port a Bug or Feature into another product release. This issue type is generally associated with the main Bug/Feature to represent each individual release of the port.JDK-8277633Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily

        • P1 - Blocks development and/or testing work, production could not run.
        • Closed

        Backport - A issue that is required to port a Bug or Feature into another product release. This issue type is generally associated with the main Bug/Feature to represent each individual release of the port.JDK-8277636Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily

        • P1 - Blocks development and/or testing work, production could not run.
        • Closed

        Backport - A issue that is required to port a Bug or Feature into another product release. This issue type is generally associated with the main Bug/Feature to represent each individual release of the port.JDK-8278070Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily

        • P1 - Blocks development and/or testing work, production could not run.
        • Closed
        relates to

        Sub-task - The sub-task of the issueJDK-8287739[11u] ProblemList sun/security/ssl/SSLSessionImpl/NoInvalidateSocketException.java

        • P3 - Major loss of function.
        • Resolved

        Bug - A problem which impairs or prevents the functions of the product.JDK-8280158New test from JDK-8274736 failed with/without patch in JDK11u

        • P3 - Major loss of function.
        • Resolved

        Bug - A problem which impairs or prevents the functions of the product.JDK-8274524SSLSocket.close() hangs if it is called during the ssl handshake

        • P2 - Crashes, loss of data, severe memory leak.
        • Closed

        Bug - A problem which impairs or prevents the functions of the product.JDK-8277970Test jdk/sun/security/ssl/SSLSessionImpl/NoInvalidateSocketException.java fails with "tag mismatch"

        • P3 - Major loss of function.
        • Closed
        links to

        CommitCommitopenjdk/jdk11u-dev/ec89f1b6

          CommitCommitopenjdk/jdk13u-dev/d959f811

            CommitCommitopenjdk/jdk15u-dev/c9a3110b

              CommitCommitopenjdk/jdk17u/6f2bdc01

                CommitCommitopenjdk/jdk/8822d41f

                  ReviewReviewopenjdk/jdk11u-dev/711

                    ReviewReviewopenjdk/jdk13u-dev/324

                      ReviewReviewopenjdk/jdk15u-dev/172

                        ReviewReviewopenjdk/jdk17u/294

                          ReviewReviewopenjdk/jdk/6197

                            (2 backported by, 4 relates to, 10 links to)