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