8277881: Missing SessionID in TLS1.3 resumption in compatibility mode

Reviewed-by: ascarpino
openjdk · Dec 24, 2021 · 9d99a37 · 9d99a37 · openjdk-notifier · Dec 24, 2021
1 parent 4669bcd
commit 9d99a37
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 11 deletions.
diff --git a/src/java.base/share/classes/sun/security/ssl/ClientHello.java b/src/java.base/share/classes/sun/security/ssl/ClientHello.java
@@ -568,15 +568,15 @@ public byte[] produce(ConnectionContext context) throws IOException {
                             "No new session is allowed and " +
                             "no existing session can be resumed");
                 }
-
-                if (chc.maximumActiveProtocol.useTLS13PlusSpec() &&
-                        SSLConfiguration.useCompatibilityMode) {
-                    // In compatibility mode, the TLS 1.3 legacy_session_id
-                    // field MUST be non-empty, so a client not offering a
-                    // pre-TLS 1.3 session MUST generate a new 32-byte value.
-                    sessionId =
+            }
+            if (sessionId.length() == 0 &&
+                    chc.maximumActiveProtocol.useTLS13PlusSpec() &&
+                    SSLConfiguration.useCompatibilityMode) {
+                // In compatibility mode, the TLS 1.3 legacy_session_id
+                // field MUST be non-empty, so a client not offering a
+                // pre-TLS 1.3 session MUST generate a new 32-byte value.
+                sessionId =
                         new SessionId(true, chc.sslContext.getSecureRandom());
-                }
             }
 
             ProtocolVersion minimumVersion = ProtocolVersion.NONE;

diff --git a/src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java b/src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java
@@ -97,7 +97,7 @@ final class SSLConfiguration implements Cloneable {
     static final boolean allowLegacyMasterSecret =
         Utilities.getBooleanProperty("jdk.tls.allowLegacyMasterSecret", true);
 
-    // Allow full handshake without Extended Master Secret extension.
+    // Use TLS1.3 middlebox compatibility mode.
     static final boolean useCompatibilityMode = Utilities.getBooleanProperty(
             "jdk.tls.client.useCompatibilityMode", true);
 

diff --git a/test/jdk/javax/net/ssl/SSLSession/ResumeTLS13withSNI.java b/test/jdk/javax/net/ssl/SSLSession/ResumeTLS13withSNI.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -26,7 +26,7 @@
 
 /*
  * @test
- * @bug 8211806
+ * @bug 8211806 8277881
  * @summary TLS 1.3 handshake server name indication is missing on a session resume
  * @run main/othervm ResumeTLS13withSNI
  */
@@ -338,6 +338,9 @@ private static void checkResumedClientHelloSNI(ByteBuffer resCliHello)
 
         // Get the legacy session length and skip that many bytes
         int sessIdLen = Byte.toUnsignedInt(resCliHello.get());
+        if (sessIdLen == 0) {
+            throw new Exception("SessionID field empty");
+        }
         resCliHello.position(resCliHello.position() + sessIdLen);
 
         // Skip over all the cipher suites