8213202: Possible race condition in TLS 1.3 session resumption jdk-12+21
authorapetcher
Wed, 21 Nov 2018 15:06:13 -0500
changeset 52643 f8fb0c86f2b3
parent 52642 9cfc8b0c45fd
child 52644 43efb4ca6d6c
8213202: Possible race condition in TLS 1.3 session resumption Reviewed-by: jnimeh
src/java.base/share/classes/sun/security/ssl/PreSharedKeyExtension.java
src/java.base/share/classes/sun/security/ssl/SSLSessionImpl.java
--- a/src/java.base/share/classes/sun/security/ssl/PreSharedKeyExtension.java	Wed Nov 21 10:46:45 2018 -0800
+++ b/src/java.base/share/classes/sun/security/ssl/PreSharedKeyExtension.java	Wed Nov 21 15:06:13 2018 -0500
@@ -656,7 +656,7 @@
                 return null;
             }
             SecretKey psk = pskOpt.get();
-            Optional<byte[]> pskIdOpt = chc.resumingSession.getPskIdentity();
+            Optional<byte[]> pskIdOpt = chc.resumingSession.consumePskIdentity();
             if (!pskIdOpt.isPresent()) {
                 if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
                     SSLLogger.fine(
@@ -666,6 +666,11 @@
             }
             byte[] pskId = pskIdOpt.get();
 
+            //The session cannot be used again. Remove it from the cache.
+            SSLSessionContextImpl sessionCache = (SSLSessionContextImpl)
+                chc.sslContext.engineGetClientSessionContext();
+            sessionCache.remove(chc.resumingSession.getSessionId());
+
             if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
                 SSLLogger.fine(
                     "Found resumable session. Preparing PSK message.");
@@ -828,10 +833,6 @@
                     "Received pre_shared_key extension: ", shPsk);
             }
 
-            // The PSK identity should not be reused, even if it is
-            // not selected.
-            chc.resumingSession.consumePskIdentity();
-
             if (shPsk.selectedIdentity != 0) {
                 chc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
                     "Selected identity index is not in correct range.");
@@ -841,11 +842,6 @@
                 SSLLogger.fine(
                 "Resuming session: ", chc.resumingSession);
             }
-
-            // remove the session from the cache
-            SSLSessionContextImpl sessionCache = (SSLSessionContextImpl)
-                    chc.sslContext.engineGetClientSessionContext();
-            sessionCache.remove(chc.resumingSession.getSessionId());
         }
     }
 
@@ -860,13 +856,6 @@
                 SSLLogger.fine("Handling pre_shared_key absence.");
             }
 
-            if (chc.handshakeExtensions.containsKey(
-                    SSLExtension.CH_PRE_SHARED_KEY)) {
-                // The PSK identity should not be reused, even if it is
-                // not selected.
-                chc.resumingSession.consumePskIdentity();
-            }
-
             // The server refused to resume, or the client did not
             // request 1.3 resumption.
             chc.resumingSession = null;
--- a/src/java.base/share/classes/sun/security/ssl/SSLSessionImpl.java	Wed Nov 21 10:46:45 2018 -0800
+++ b/src/java.base/share/classes/sun/security/ssl/SSLSessionImpl.java	Wed Nov 21 15:06:13 2018 -0500
@@ -305,13 +305,6 @@
         return this.identificationProtocol;
     }
 
-    /*
-     * Get the PSK identity. Take care not to use it in multiple connections.
-     */
-    synchronized Optional<byte[]> getPskIdentity() {
-        return Optional.ofNullable(pskIdentity);
-    }
-
     /* PSK identities created from new_session_ticket messages should only
      * be used once. This method will return the identity and then clear it
      * so it cannot be used again.