[
https://issues.apache.org/jira/browse/ARTEMIS-4744?focusedWorklogId=916436&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-916436
]
ASF GitHub Bot logged work on ARTEMIS-4744:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 25/Apr/24 15:59
Start Date: 25/Apr/24 15:59
Worklog Time Spent: 10m
Work Description: gemmellr commented on code in PR #4906:
URL: https://github.com/apache/activemq-artemis/pull/4906#discussion_r1579751853
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/connect/AMQPConnectSaslTest.java:
##########
@@ -214,4 +216,186 @@ private void doConnectWithExternalTestImpl(boolean
requireClientCert) throws Exc
peer.waitForScriptToComplete(5, TimeUnit.SECONDS);
}
}
+
+ @Test(timeout = 20_000)
+ public void testReconnectConnectsWithVerifyHostOffOnSecondURI() throws
Exception {
+ final String keyStorePath =
this.getClass().getClassLoader().getResource(UNKNOWN_SERVER_KEYSTORE_NAME).getFile();
+
+ ProtonTestServerOptions server1Options = new ProtonTestServerOptions();
+ server1Options.setSecure(true);
+ server1Options.setKeyStoreLocation(keyStorePath);
+ server1Options.setKeyStorePassword(SERVER_KEYSTORE_PASSWORD);
+ server1Options.setVerifyHost(false);
+
+ ProtonTestServerOptions server2Options = new ProtonTestServerOptions();
+ server2Options.setSecure(true);
+ server2Options.setKeyStoreLocation(keyStorePath);
+ server2Options.setKeyStorePassword(SERVER_KEYSTORE_PASSWORD);
+ server2Options.setVerifyHost(false);
+
+ try (ProtonTestServer firstPeer = new ProtonTestServer(server1Options);
+ ProtonTestServer secondPeer = new ProtonTestServer(server2Options))
{
+
+ firstPeer.expectConnectionToDrop();
+ firstPeer.start();
+
+ secondPeer.expectSASLHeader().respondWithSASLHeader();
+ secondPeer.remoteSaslMechanisms().withMechanisms(EXTERNAL,
PLAIN).queue();
+
secondPeer.expectSaslInit().withMechanism(PLAIN).withInitialResponse(secondPeer.saslPlainInitialResponse(USER,
PASSWD));
+ secondPeer.remoteSaslOutcome().withCode(SaslCode.OK).queue();
+ secondPeer.expectAMQPHeader().respondWithAMQPHeader();
+ secondPeer.expectOpen().respond();
+ secondPeer.expectBegin().respond();
+ secondPeer.start();
+
+ final URI firstPeerURI = firstPeer.getServerURI();
+ logger.debug("Connect test started, first peer listening on: {}",
firstPeerURI);
+
+ final URI secondPeerURI = secondPeer.getServerURI();
+ logger.debug("Connect test started, second peer listening on: {}",
secondPeerURI);
+
+ // First connection fails because we use a server certificate with
whose common name
+ // doesn't match the host, second connection should work as we
disable host verification
+ String amqpServerConnectionURI =
+ "(tcp://localhost:" + firstPeerURI.getPort() + "?verifyHost=true" +
+ ",tcp://localhost:" + secondPeerURI.getPort() +
"?verifyHost=false)" +
+ "?sslEnabled=true;trustStorePath=" + SERVER_TRUSTSTORE_NAME +
+ ";trustStorePassword=" + SERVER_TRUSTSTORE_PASSWORD;
+
+ AMQPBrokerConnectConfiguration amqpConnection =
+ new AMQPBrokerConnectConfiguration(getTestName(),
amqpServerConnectionURI);
+ amqpConnection.setRetryInterval(100); // Allow reconnects
Review Comment:
Maybe put a limit on attempts, just in case?
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/connect/AMQPConnectSaslTest.java:
##########
@@ -214,4 +216,186 @@ private void doConnectWithExternalTestImpl(boolean
requireClientCert) throws Exc
peer.waitForScriptToComplete(5, TimeUnit.SECONDS);
}
}
+
+ @Test(timeout = 20_000)
+ public void testReconnectConnectsWithVerifyHostOffOnSecondURI() throws
Exception {
+ final String keyStorePath =
this.getClass().getClassLoader().getResource(UNKNOWN_SERVER_KEYSTORE_NAME).getFile();
+
+ ProtonTestServerOptions server1Options = new ProtonTestServerOptions();
+ server1Options.setSecure(true);
+ server1Options.setKeyStoreLocation(keyStorePath);
+ server1Options.setKeyStorePassword(SERVER_KEYSTORE_PASSWORD);
+ server1Options.setVerifyHost(false);
+
+ ProtonTestServerOptions server2Options = new ProtonTestServerOptions();
+ server2Options.setSecure(true);
+ server2Options.setKeyStoreLocation(keyStorePath);
+ server2Options.setKeyStorePassword(SERVER_KEYSTORE_PASSWORD);
+ server2Options.setVerifyHost(false);
+
+ try (ProtonTestServer firstPeer = new ProtonTestServer(server1Options);
+ ProtonTestServer secondPeer = new ProtonTestServer(server2Options))
{
+
+ firstPeer.expectConnectionToDrop();
+ firstPeer.start();
+
+ secondPeer.expectSASLHeader().respondWithSASLHeader();
+ secondPeer.remoteSaslMechanisms().withMechanisms(EXTERNAL,
PLAIN).queue();
+
secondPeer.expectSaslInit().withMechanism(PLAIN).withInitialResponse(secondPeer.saslPlainInitialResponse(USER,
PASSWD));
+ secondPeer.remoteSaslOutcome().withCode(SaslCode.OK).queue();
+ secondPeer.expectAMQPHeader().respondWithAMQPHeader();
+ secondPeer.expectOpen().respond();
+ secondPeer.expectBegin().respond();
+ secondPeer.start();
+
+ final URI firstPeerURI = firstPeer.getServerURI();
+ logger.debug("Connect test started, first peer listening on: {}",
firstPeerURI);
+
+ final URI secondPeerURI = secondPeer.getServerURI();
+ logger.debug("Connect test started, second peer listening on: {}",
secondPeerURI);
+
+ // First connection fails because we use a server certificate with
whose common name
+ // doesn't match the host, second connection should work as we
disable host verification
+ String amqpServerConnectionURI =
+ "(tcp://localhost:" + firstPeerURI.getPort() + "?verifyHost=true" +
+ ",tcp://localhost:" + secondPeerURI.getPort() +
"?verifyHost=false)" +
+ "?sslEnabled=true;trustStorePath=" + SERVER_TRUSTSTORE_NAME +
+ ";trustStorePassword=" + SERVER_TRUSTSTORE_PASSWORD;
Review Comment:
The docs around this indicate a different syntax, not having "(...)", and
saying you are meant to use # for the first separator:
https://github.com/apache/activemq-artemis/blob/2.33.0/docs/user-manual/amqp-broker-connections.adoc?plain=1#L87-L97
Though I dont really understand why given it says what is after is to be
comma separated. Its not clear if you were meant to be able to specify
different options for the CSV elements, or just host:port values as it
indicates, since it doesnt say either way.
The existing test I found around this (that doesnt use TLS and so didnt
notice it didnt work so much with that) uses the same basic config from the
doc:
https://github.com/apache/activemq-artemis/blob/2.33.0/tests/e2e-tests/src/main/resources/servers/brokerConnect/replicaMainServerA/broker.xml#L59
We should probably be testing whatever the doc says...and/or updating the
docs to clarify what is actually expected to work.
Issue Time Tracking
-------------------
Worklog Id: (was: 916436)
Time Spent: 40m (was: 0.5h)
> AMQP broker connections don't fully support multi host URIs
> -----------------------------------------------------------
>
> Key: ARTEMIS-4744
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4744
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Components: AMQP
> Affects Versions: 2.33.0
> Reporter: Timothy A. Bish
> Assignee: Timothy A. Bish
> Priority: Major
> Fix For: 2.34.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> When configuring a multi host connection URI for an AMQP broker connection
> the connection will utilize some but not all of the configuration. The
> broker will attempt connection to each host and port part specific on the URI
> but does not apply configuration specific to a given host. This can lead to
> failure on connect due to using the TLS configuration from the first host
> when attempting to connect to the following N hosts. Users need to be able
> to configure TLS specific options per host as values such as host
> verification, SNI and trust stores can vary amongst hosts.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
