This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 11f3036036b64162ac7cf7eb1d7a740978633a7d Author: opencode <[email protected]> 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 0089077d8f..a0dd7e092d 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; @@ -88,4 +91,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: [email protected] For additional commands, e-mail: [email protected]
