You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/10/15 18:56:15 UTC
(tomcat) 18/24: Fix broken SNI fallback for ClientHello values larger than the parse limit
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 5d50161f0f6b46166c9d06d4d9ca1e06dec04ae6
Author: opencode <re...@apache.org>
AuthorDate: Thu Oct 15 13:58:52 2026 +0200
Fix broken SNI fallback for ClientHello values larger than the parse limit
When a ClientHello could not be buffered within sniParseLimit,
processSNI() selected the default SSLHostConfig but then still called
the TLSClientHelloExtractor getters, which throw IllegalStateException
for an incomplete extraction. The documented fallback therefore never
happened: the exception escaped to the socket processor's error
handler, logging an error and aborting the connection. Pass empty
lists for all extractor-derived values (requested ciphers, application
protocols, protocol versions, cipher names, supported groups and
signature schemes) through local variables on the UNDERFLOW path
instead of consulting the extractor, which also prevents NPEs on the
unguarded isEmpty()/contains() usages in createSSLEngine(). Add a test
that sends an oversized ClientHello record and asserts the default-host
fallback is logged without an IllegalStateException.
---
.../apache/tomcat/util/net/SecureNioChannel.java | 16 +++++--
.../tomcat/util/net/TestLargeClientHello.java | 56 ++++++++++++++++++++++
2 files changed, 68 insertions(+), 4 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/SecureNioChannel.java b/java/org/apache/tomcat/util/net/SecureNioChannel.java
index 4fe0ac58a7..b746f54d14 100644
--- a/java/org/apache/tomcat/util/net/SecureNioChannel.java
+++ b/java/org/apache/tomcat/util/net/SecureNioChannel.java
@@ -293,6 +293,8 @@ public class SecureNioChannel extends NioChannel {
String hostName = null;
List<Cipher> clientRequestedCiphers = null;
List<String> clientRequestedApplicationProtocols = null;
+ List<String> clientRequestedProtocols = null;
+ List<String> clientRequestedCipherNames = null;
List<Group> clientSupportedGroups = null;
List<SignatureScheme> clientSignatureSchemes = null;
switch (extractor.getResult()) {
@@ -303,6 +305,8 @@ public class SecureNioChannel extends NioChannel {
//$FALL-THROUGH$ to set the client requested ciphers
case NOT_PRESENT:
clientRequestedCiphers = extractor.getClientRequestedCiphers();
+ clientRequestedProtocols = extractor.getClientRequestedProtocols();
+ clientRequestedCipherNames = extractor.getClientRequestedCipherNames();
clientSupportedGroups = extractor.getClientSupportedGroups();
clientSignatureSchemes = extractor.getClientSignatureSchemes();
break;
@@ -315,6 +319,11 @@ public class SecureNioChannel extends NioChannel {
}
hostName = endpoint.getDefaultSSLHostConfigName();
clientRequestedCiphers = Collections.emptyList();
+ clientRequestedApplicationProtocols = Collections.emptyList();
+ clientRequestedProtocols = Collections.emptyList();
+ clientRequestedCipherNames = Collections.emptyList();
+ clientSupportedGroups = Collections.emptyList();
+ clientSignatureSchemes = Collections.emptyList();
break;
case NON_SECURE:
netOutBuffer.clear();
@@ -329,13 +338,12 @@ public class SecureNioChannel extends NioChannel {
}
createSSLEngine(hostName, clientRequestedCiphers, clientRequestedApplicationProtocols,
- extractor.getClientRequestedProtocols(), clientSupportedGroups, clientSignatureSchemes);
+ clientRequestedProtocols, clientSupportedGroups, clientSignatureSchemes);
// Populate additional TLS attributes obtained from the handshake that
// aren't available from the session
- additionalTlsAttributes.put(SSLSupport.REQUESTED_PROTOCOL_VERSIONS_KEY,
- extractor.getClientRequestedProtocols());
- additionalTlsAttributes.put(SSLSupport.REQUESTED_CIPHERS_KEY, extractor.getClientRequestedCipherNames());
+ additionalTlsAttributes.put(SSLSupport.REQUESTED_PROTOCOL_VERSIONS_KEY, clientRequestedProtocols);
+ additionalTlsAttributes.put(SSLSupport.REQUESTED_CIPHERS_KEY, clientRequestedCipherNames);
// Ensure the application buffers (which have to be created earlier) are
// big enough.
diff --git a/test/org/apache/tomcat/util/net/TestLargeClientHello.java b/test/org/apache/tomcat/util/net/TestLargeClientHello.java
index ebdab27de0..d700fc122a 100644
--- a/test/org/apache/tomcat/util/net/TestLargeClientHello.java
+++ b/test/org/apache/tomcat/util/net/TestLargeClientHello.java
@@ -18,6 +18,9 @@
package org.apache.tomcat.util.net;
import java.io.File;
+import java.io.OutputStream;
+import java.net.InetSocketAddress;
+import java.net.Socket;
import java.util.logging.Level;
import javax.net.ssl.SSLContext;
@@ -83,4 +86,57 @@ public class TestLargeClientHello extends TomcatBaseTest {
}
}
+
+ @Test
+ public void testClientHelloLargerThanParseLimit() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+
+ Context ctx = getProgrammaticRootContext();
+ Tomcat.addServlet(ctx, "hello", new HelloWorldServlet());
+ ctx.addServletMapping("/", "hello");
+
+ TesterSupport.initSsl(tomcat);
+
+ // Limit SNI parsing to less than SecureNioChannel.DEFAULT_NET_BUFFER_SIZE (16921) so that the
+ // buffer cannot be expanded to hold the record below and SNI extraction returns UNDERFLOW. The
+ // endpoint must then fall back to the default SSLHostConfig rather than failing with an
+ // IllegalStateException from the extractor getters.
+ tomcat.getConnector().setProperty("sniParseLimit", "1024");
+
+ // A handshake record with a declared length far larger than sniParseLimit
+ byte[] record = new byte[65537];
+ record[0] = 22; // Handshake content type
+ record[1] = 3;
+ record[2] = 3;
+ record[3] = (byte) 0xFF; // Record length of 65535
+ record[4] = (byte) 0xFF;
+ record[5] = 1; // ClientHello handshake type
+
+ try (LogCapture logCapture = attachLogCapture(Level.FINE, "org.apache.tomcat.util.net");
+ Socket socket = new Socket()) {
+
+ tomcat.start();
+
+ socket.connect(new InetSocketAddress("localhost", getPort()));
+ OutputStream out = socket.getOutputStream();
+ out.write(record);
+ out.flush();
+
+ String sniDefault = TomcatBaseTest.getKeyFromPropertiesFile("org.apache.tomcat.util.net",
+ "channel.nio.ssl.sniDefault");
+
+ // The fallback to the default SSLHostConfig is logged once the extraction returns UNDERFLOW
+ long start = System.currentTimeMillis();
+ while (!logCapture.containsText(sniDefault) && System.currentTimeMillis() - start < 10000) {
+ Thread.sleep(100);
+ }
+ Assert.assertTrue("Fallback to the default SSLHostConfig was not logged",
+ logCapture.containsText(sniDefault));
+
+ // Give any error triggered after the fallback a chance to be logged, then confirm that the
+ // extractor getters were not called on the incomplete extraction result
+ Thread.sleep(500);
+ Assert.assertFalse(logCapture.hasException(IllegalStateException.class));
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org