This is an automated email from the ASF dual-hosted git repository.

vavrtom pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/qpid-broker-j.git


The following commit(s) were added to refs/heads/main by this push:
     new 39b5915f6a QPID-8689: [Broker-J] Remove switch for ignoring SNI host 
errors (#311)
39b5915f6a is described below

commit 39b5915f6a89396f7931fc87afdf3643a2b634df
Author: Daniil Kirilyuk <daniel.kiril...@gmail.com>
AuthorDate: Thu Jul 31 14:15:59 2025 +0200

    QPID-8689: [Broker-J] Remove switch for ignoring SNI host errors (#311)
---
 .../apache/qpid/server/model/port/AmqpPort.java    | 15 ---------
 .../qpid/server/model/port/AmqpPortImpl.java       | 10 ------
 .../NonBlockingConnectionTLSDelegate.java          | 19 ++----------
 .../org/apache/qpid/server/transport/SNITest.java  | 36 ++++++----------------
 4 files changed, 13 insertions(+), 67 deletions(-)

diff --git 
a/broker-core/src/main/java/org/apache/qpid/server/model/port/AmqpPort.java 
b/broker-core/src/main/java/org/apache/qpid/server/model/port/AmqpPort.java
index 20afa83d11..229a6ee7f0 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/model/port/AmqpPort.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/model/port/AmqpPort.java
@@ -81,18 +81,6 @@ public interface AmqpPort<X extends AmqpPort<X>> extends 
Port<X>
     @ManagedContextDefault( name = 
PORT_DIAGNOSIS_OF_SSL_ENGINE_LOOPING_BREAK_THRESHOLD)
     long DEFAULT_PORT_DIAGNOSIS_OF_SSL_ENGINE_LOOPING_BREAK_THRESHOLD = 1005;
 
-    String PORT_IGNORE_INVALID_SNI = "qpid.port.amqp.ignoreInvalidSni";
-
-    /**
-     * In Java 17 logic SNI hostname validation became stricter and this flag 
will not help with the syntax errors
-     * in SNI hostnames provided by client. They will result in 
SSLPeerUnverifiedException thrown by SSLEngine.
-     * Therefore, usage of this flag is discouraged. It may be deleted in one 
of the future broker releases.
-     */
-    @Deprecated
-    @SuppressWarnings("unused")
-    @ManagedContextDefault(name = PORT_IGNORE_INVALID_SNI)
-    boolean DEFAULT_PORT_IGNORE_INVALID_SNI = false;
-
     @SuppressWarnings("unused")
     @ManagedContextDefault( name = PORT_AMQP_THREAD_POOL_SIZE)
     long DEFAULT_PORT_AMQP_THREAD_POOL_SIZE = 8;
@@ -200,9 +188,6 @@ public interface AmqpPort<X extends AmqpPort<X>> extends 
Port<X>
     @ManagedAttribute( defaultValue = "${" + PORT_MAX_OPEN_CONNECTIONS + "}" )
     int getMaxOpenConnections();
 
-    @DerivedAttribute
-    boolean isIgnoreInvalidSni();
-
     @ManagedStatistic(statisticType = StatisticType.POINT_IN_TIME, units = 
StatisticUnit.COUNT,
             label = "Open Connections",
             description = "Current number of connections made through this 
port",
diff --git 
a/broker-core/src/main/java/org/apache/qpid/server/model/port/AmqpPortImpl.java 
b/broker-core/src/main/java/org/apache/qpid/server/model/port/AmqpPortImpl.java
index 20942c5ae8..c519800049 100644
--- 
a/broker-core/src/main/java/org/apache/qpid/server/model/port/AmqpPortImpl.java
+++ 
b/broker-core/src/main/java/org/apache/qpid/server/model/port/AmqpPortImpl.java
@@ -74,10 +74,8 @@ import 
org.apache.qpid.server.util.ServerScopedRuntimeException;
 
 public class AmqpPortImpl extends AbstractPort<AmqpPortImpl> implements 
AmqpPort<AmqpPortImpl>
 {
-
     private static final Logger LOGGER = 
LoggerFactory.getLogger(AmqpPortImpl.class);
 
-
     @ManagedAttributeField
     private boolean _tcpNoDelay;
 
@@ -108,7 +106,6 @@ public class AmqpPortImpl extends 
AbstractPort<AmqpPortImpl> implements AmqpPort
     private volatile int _tlsSessionTimeout;
     private volatile int _tlsSessionCacheSize;
     private volatile List<ConnectionPropertyEnricher> 
_connectionPropertyEnrichers;
-    private volatile boolean _ignoreInvalidSni;
 
 
     @ManagedObjectFactoryConstructor
@@ -149,12 +146,6 @@ public class AmqpPortImpl extends 
AbstractPort<AmqpPortImpl> implements AmqpPort
         return _maxOpenConnections;
     }
 
-    @Override
-    public boolean isIgnoreInvalidSni()
-    {
-        return _ignoreInvalidSni;
-    }
-
     @Override
     protected void onCreate()
     {
@@ -195,7 +186,6 @@ public class AmqpPortImpl extends 
AbstractPort<AmqpPortImpl> implements AmqpPort
         _heartBeatDelay = getContextValue(Integer.class, 
AmqpPort.HEART_BEAT_DELAY);
         _tlsSessionTimeout = getContextValue(Integer.class, 
AmqpPort.TLS_SESSION_TIMEOUT);
         _tlsSessionCacheSize = getContextValue(Integer.class, 
AmqpPort.TLS_SESSION_CACHE_SIZE);
-        _ignoreInvalidSni = getContextValue(Boolean.class, 
AmqpPort.PORT_IGNORE_INVALID_SNI);
 
         @SuppressWarnings("unchecked")
         List<String> configurationPropertyEnrichers = 
getContextValue(List.class, AmqpPort.CONNECTION_PROPERTY_ENRICHERS);
diff --git 
a/broker-core/src/main/java/org/apache/qpid/server/transport/NonBlockingConnectionTLSDelegate.java
 
b/broker-core/src/main/java/org/apache/qpid/server/transport/NonBlockingConnectionTLSDelegate.java
index 925521641c..6a4dab02f5 100644
--- 
a/broker-core/src/main/java/org/apache/qpid/server/transport/NonBlockingConnectionTLSDelegate.java
+++ 
b/broker-core/src/main/java/org/apache/qpid/server/transport/NonBlockingConnectionTLSDelegate.java
@@ -44,7 +44,6 @@ import org.slf4j.LoggerFactory;
 import org.apache.qpid.server.bytebuffer.QpidByteBuffer;
 import org.apache.qpid.server.model.port.AmqpPort;
 import org.apache.qpid.server.transport.network.security.ssl.SSLUtil;
-import org.apache.qpid.server.util.ConnectionScopedRuntimeException;
 import org.apache.qpid.server.util.ServerScopedRuntimeException;
 
 public class NonBlockingConnectionTLSDelegate implements 
NonBlockingConnectionDelegate
@@ -63,7 +62,6 @@ public class NonBlockingConnectionTLSDelegate implements 
NonBlockingConnectionDe
     private QpidByteBuffer _netInputBuffer;
     private QpidByteBuffer _netOutputBuffer;
     private QpidByteBuffer _applicationBuffer;
-    private final boolean _ignoreInvalidSni;
     private final AtomicInteger _loopingCounter = new AtomicInteger(0);
     private final boolean _enableDiagnosisOfSslEngineLooping;
     private final long _diagnosisOfSslEngineLoopingWarnThreshold;
@@ -85,7 +83,6 @@ public class NonBlockingConnectionTLSDelegate implements 
NonBlockingConnectionDe
         _netInputBuffer = QpidByteBuffer.allocateDirect(_networkBufferSize);
         _applicationBuffer = QpidByteBuffer.allocateDirect(_networkBufferSize);
         _netOutputBuffer = QpidByteBuffer.allocateDirect(_networkBufferSize);
-        _ignoreInvalidSni = port.isIgnoreInvalidSni();
         _enableDiagnosisOfSslEngineLooping = 
port.getContextValue(Boolean.class, 
AmqpPort.PORT_DIAGNOSIS_OF_SSL_ENGINE_LOOPING);
         _diagnosisOfSslEngineLoopingWarnThreshold = 
port.getContextValue(Integer.class, 
AmqpPort.PORT_DIAGNOSIS_OF_SSL_ENGINE_LOOPING_WARN_THRESHOLD);
         _diagnosisOfSslEngineLoopingBreakThreshold = 
port.getContextValue(Integer.class, 
AmqpPort.PORT_DIAGNOSIS_OF_SSL_ENGINE_LOOPING_BREAK_THRESHOLD);
@@ -168,20 +165,10 @@ public class NonBlockingConnectionTLSDelegate implements 
NonBlockingConnectionDe
 
     private SNIHostName getSNIHostName(final QpidByteBuffer buffer)
     {
-        try
+        final String name = SSLUtil.getServerNameFromTLSClientHello(buffer);
+        if (name != null)
         {
-            final String name = 
SSLUtil.getServerNameFromTLSClientHello(buffer);
-            if (name != null)
-            {
-                return SSLUtil.createSNIHostName(name);
-            }
-        }
-        catch (ConnectionScopedRuntimeException e)
-        {
-            if (!_ignoreInvalidSni)
-            {
-                throw e;
-            }
+            return SSLUtil.createSNIHostName(name);
         }
         return null;
     }
diff --git 
a/broker-core/src/test/java/org/apache/qpid/server/transport/SNITest.java 
b/broker-core/src/test/java/org/apache/qpid/server/transport/SNITest.java
index 6598e27e57..d3ce9a127d 100644
--- a/broker-core/src/test/java/org/apache/qpid/server/transport/SNITest.java
+++ b/broker-core/src/test/java/org/apache/qpid/server/transport/SNITest.java
@@ -22,8 +22,6 @@ package org.apache.qpid.server.transport;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.condition.JRE.JAVA_16;
-import static org.junit.jupiter.api.condition.JRE.JAVA_17;
 
 import java.io.File;
 import java.net.InetSocketAddress;
@@ -48,12 +46,9 @@ import javax.net.ssl.X509TrustManager;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 
-import org.apache.qpid.server.model.port.AmqpPort;
-
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.condition.EnabledForJreRange;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
 import org.apache.qpid.server.SystemLauncher;
@@ -131,58 +126,49 @@ public class SNITest extends UnitTestBase
     @Test
     public void testValidCertChosen() throws Exception
     {
-        performTest(true, "fooinvalid", "foo", _fooValid, false);
+        performTest(true, "fooinvalid", "foo", _fooValid);
     }
 
     @Test
     public void testMatchCertChosenEvenIfInvalid() throws Exception
     {
-        performTest(true, "fooinvalid", "bar", _barInvalid, false);
+        performTest(true, "fooinvalid", "bar", _barInvalid);
     }
 
     @Test
     public void testDefaultCertChose() throws Exception
     {
-        performTest(true, "fooinvalid", null, _fooInvalid, false);
+        performTest(true, "fooinvalid", null, _fooInvalid);
     }
 
     @Test
     public void testMatchingCanBeDisabled() throws Exception
     {
-        performTest(false, "fooinvalid", "foo", _fooInvalid, false);
+        performTest(false, "fooinvalid", "foo", _fooInvalid);
     }
 
     @Test
     public void testInvalidHostname()
     {
         assertThrows(SSLPeerUnverifiedException.class,
-                () -> performTest(false, "fooinvalid", "_foo", _fooInvalid, 
false),
+                () -> performTest(false, "fooinvalid", "_foo", _fooInvalid),
                 "Expected exception not thrown");
     }
 
     @Test
-    @EnabledForJreRange(max = JAVA_16)
-    public void testBypassInvalidSniHostname() throws Exception
-    {
-        performTest(false, "foovalid", "_foo", _fooValid, true);
-    }
-
-    @Test
-    @EnabledForJreRange(min = JAVA_17)
     public void testBypassInvalidSniHostnameWithJava17()
     {
         assertThrows(SSLPeerUnverifiedException.class,
-                () -> performTest(false, "foovalid", "_foo", _fooValid, true),
+                () -> performTest(false, "foovalid", "_foo", _fooValid),
                 "Expected exception not thrown");
     }
 
     private void performTest(final boolean useMatching,
                              final String defaultAlias,
                              final String sniHostName,
-                             final KeyCertificatePair expectedCert,
-                             final boolean ignoreInvalidSni) throws Exception
+                             final KeyCertificatePair expectedCert) throws 
Exception
     {
-        doBrokerStartup(useMatching, defaultAlias, ignoreInvalidSni);
+        doBrokerStartup(useMatching, defaultAlias);
         final SSLContext context = SSLUtil.tryGetSSLContext();
         context.init(null, new TrustManager[]
         {
@@ -225,8 +211,7 @@ public class SNITest extends UnitTestBase
     }
 
     private void doBrokerStartup(final boolean useMatching,
-                                 final String defaultAlias,
-                                 final boolean ignoreInvalidSni) throws 
Exception
+                                 final String defaultAlias) throws Exception
     {
         final File initialConfiguration = createInitialContext();
         _brokerWork = TestFileUtils.createTestDirectory("qpid-work", true);
@@ -261,8 +246,7 @@ public class SNITest extends UnitTestBase
                 Port.TRANSPORTS, Set.of(Transport.SSL),
                 Port.PORT, 0,
                 Port.AUTHENTICATION_PROVIDER, authProvider,
-                Port.KEY_STORE, keyStore,
-                Port.CONTEXT, Map.of(AmqpPort.PORT_IGNORE_INVALID_SNI, 
String.valueOf(ignoreInvalidSni)));
+                Port.KEY_STORE, keyStore);
         final Port<?> port = _broker.createChild(Port.class, portAttr);
 
         _boundPort = port.getBoundPort();


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org
For additional commands, e-mail: commits-h...@qpid.apache.org

Reply via email to