[
https://issues.apache.org/jira/browse/ARTEMIS-5163?focusedWorklogId=986986&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-986986
]
ASF GitHub Bot logged work on ARTEMIS-5163:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 13/Oct/25 15:28
Start Date: 13/Oct/25 15:28
Worklog Time Spent: 10m
Work Description: gemmellr commented on code in PR #5956:
URL: https://github.com/apache/activemq-artemis/pull/5956#discussion_r2426633478
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnection.java:
##########
@@ -481,4 +490,48 @@ private void closeChannel(final Channel channel, boolean
inEventLoop) {
}
}
+ public X509Certificate[] getCertificates() {
+ return Objects.requireNonNullElse(certificates, getCertsFromChannel());
Review Comment:
This seems wrong as it will a) Always have to call getCertsFromChannel
first, even if it already has 'certificates', which is both inefficient and
leads to other problems (see later) and b) Throws if the latter returns null,
which it wouldnt in the existing bits and shouldnt since there is absolutely no
guarantee the connection has peer certificates. Disposing of the
requireNonNullElse would allow avoiding both of those things.
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnection.java:
##########
@@ -481,4 +490,48 @@ private void closeChannel(final Channel channel, boolean
inEventLoop) {
}
}
+ public X509Certificate[] getCertificates() {
+ return Objects.requireNonNullElse(certificates, getCertsFromChannel());
+ }
+
+ private X509Certificate[] getCertsFromChannel() {
+ Certificate[] plainCerts = null;
+ ChannelHandler channelHandler = channel.pipeline().get("ssl");
+ if (channelHandler instanceof SslHandler sslHandler) {
+ try {
+ plainCerts =
sslHandler.engine().getSession().getPeerCertificates();
+ } catch (SSLPeerUnverifiedException e) {
+ // ignore
+ }
+ }
+
+ /*
+ * When using the OpenSSL provider on the broker the
getPeerCertificates() method does *not* return a
+ * X509Certificate[] so we need to convert the Certificate[] that is
returned. This code is inspired by Tomcat's
+ * org.apache.tomcat.util.net.jsse.JSSESupport class.
+ */
+ certificates = null;
Review Comment:
I think having a local value as the existing code did (x509Certs) that is
assigned to the field only at the end would be nicer and safer than this. It is
setting the field null here (even if already non-null), then setting it
non-null below, and again is doing this every time the calling
getCertificates() method is called currently as it must always call
this...making it totally unsafe if ever called concurrently as the field would
flip flop to/from null as they operated. At least with the local value, worst
case is they both safely assign the same thing separately at the end.
Issue Time Tracking
-------------------
Worklog Id: (was: 986986)
Time Spent: 4h 10m (was: 4h)
> Artemis fails to send mqtt will message using mutual TLS
> --------------------------------------------------------
>
> Key: ARTEMIS-5163
> URL: https://issues.apache.org/jira/browse/ARTEMIS-5163
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Components: MQTT
> Affects Versions: 2.31.2, 2.33.0, 2.38.0, 2.39.0, 2.42.0
> Reporter: Olaf Gustav
> Priority: Major
> Labels: pull-request-available
> Time Spent: 4h 10m
> Remaining Estimate: 0h
>
> As discussed in the user mailing list, the MQTT broker fails to sent the
> provided will message when using mutual TLS.
> +set-up for testing:+
> * ActiveMQ Artemis 2.33 as MQTT broker
> * Artemis runs on jdk-21
> * clients are authenticated using mutual TLS
> * certificate DN is used to map to a user and eventually to the configured
> roles
> +issue:+
> During testing we discovered, that the provided will message is not sent as
> expected. We got the following error messages:
> {code:none}
> WARN [org.apache.activemq.artemis.core.server] AMQ222216: Security problem
> while authenticating: AMQ229031: Unable to validate user from /
> 127.0.0.1:51770. Username: null; SSL certificate subject DN: unavailable
> ERROR [org.apache.activemq.artemis.core.protocol.mqtt] AMQ834007:
> Authorization failure sending will message: AMQ229031: Unable to validate
> user from / 127.0.0.1:51770. Username: null; SSL certificate subject DN:
> unavailable
> {code}
> I did some research in the code base. The class
> *org.apache.activemq.artemis.core.remoting.CertificateUtil* retrieves the
> certificate subject DN based on the actual client certificate provided by an
> existing connection. When trying to send a mqtt will message, there is no
> connection to the client anymore. Consequently, the broker fails to get the
> DN. Since the subject DN serves as the key in the authentication cache
> ({*}org.apache.activemq.artemis.core.security.impl. SecurityStoreImpl{*}),
> the will message fails to be checked against access permissions.
> As a workaround, I used the RemotingConnection.clientID as authentication
> cache key instead of the DN. That works as long as the parameter
> *security-invalidation-interval* is properly defined, that means
> {{{}security-invalidation-interval >> sessionExpiryInterval{}}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
