This is an automated email from the ASF dual-hosted git repository.
clebertsuconic pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git
The following commit(s) were added to refs/heads/main by this push:
new 6ee27224a5 ARTEMIS-4352 Avod Deadlock in case the SecurityManager is
reusing the Netty executor.
6ee27224a5 is described below
commit 6ee27224a57d3c5ad7b293497d3bfc892a72a8b0
Author: Clebert Suconic <[email protected]>
AuthorDate: Fri Jul 7 12:27:06 2023 -0400
ARTEMIS-4352 Avod Deadlock in case the SecurityManager is reusing the Netty
executor.
If the Security Manager is using Netty, and in particular the same Netty
connection,
you could run into a deadlock / starvation.
This is particularly true in the Wildfly case where they reuse the same
connection for everything via XNIO.
---
.../protocol/core/impl/ActiveMQPacketHandler.java | 9 ++
.../security/RecursiveNettySecurityTest.java | 116 +++++++++++++++++++++
2 files changed, 125 insertions(+)
diff --git
a/artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQPacketHandler.java
b/artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQPacketHandler.java
index 28c53c4602..b3077f8cd7 100644
---
a/artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQPacketHandler.java
+++
b/artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQPacketHandler.java
@@ -48,6 +48,7 @@ import
org.apache.activemq.artemis.core.server.ActiveMQServerLogger;
import org.apache.activemq.artemis.core.server.ServerSession;
import org.apache.activemq.artemis.core.version.Version;
import org.apache.activemq.artemis.logs.AuditLogger;
+import org.apache.activemq.artemis.utils.actors.Actor;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.lang.invoke.MethodHandles;
@@ -67,6 +68,8 @@ public class ActiveMQPacketHandler implements ChannelHandler {
private final CoreProtocolManager protocolManager;
+ private final Actor<Packet> packetActor;
+
public ActiveMQPacketHandler(final CoreProtocolManager protocolManager,
final ActiveMQServer server,
final Channel channel1,
@@ -78,10 +81,16 @@ public class ActiveMQPacketHandler implements
ChannelHandler {
this.channel1 = channel1;
this.connection = connection;
+
+ packetActor = new Actor<>(server.getExecutorFactory().getExecutor(),
this::internalHandler);
}
@Override
public void handlePacket(final Packet packet) {
+ packetActor.act(packet);
+ }
+
+ private void internalHandler(final Packet packet) {
byte type = packet.getType();
if (AuditLogger.isAnyLoggingEnabled()) {
diff --git
a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/RecursiveNettySecurityTest.java
b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/RecursiveNettySecurityTest.java
new file mode 100644
index 0000000000..aa925e177f
--- /dev/null
+++
b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/RecursiveNettySecurityTest.java
@@ -0,0 +1,116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.security;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.JMSException;
+import javax.security.auth.Subject;
+import java.lang.invoke.MethodHandles;
+import java.lang.management.ManagementFactory;
+import java.util.Set;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.activemq.artemis.core.remoting.impl.netty.NettyConnection;
+import org.apache.activemq.artemis.core.security.CheckType;
+import org.apache.activemq.artemis.core.security.Role;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.ActiveMQServers;
+import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection;
+import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager5;
+import
org.apache.activemq.artemis.spi.core.security.jaas.NoCacheLoginException;
+import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
+import org.apache.activemq.artemis.tests.util.CFUtil;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class RecursiveNettySecurityTest extends ActiveMQTestBase {
+
+ private static final Logger logger =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ /*
+ * create session tests
+ */
+ private static final String addressA = "addressA";
+
+ private static final String queueA = "queueA";
+
+ @Override
+ @Before
+ public void setUp() throws Exception {
+ super.setUp();
+ }
+
+ @Test
+ public void testRecursiveSecurity() throws Exception {
+ RecursiveNettySecurityManager securityManager = new
RecursiveNettySecurityManager();
+ ActiveMQServer server =
addServer(ActiveMQServers.newActiveMQServer(createDefaultNettyConfig().setSecurityEnabled(true),
ManagementFactory.getPlatformMBeanServer(), securityManager, false));
+ server.start();
+
+ ConnectionFactory connectionFactory =
CFUtil.createConnectionFactory("CORE", "tcp://localhost:61616");
+
+ try {
+ Connection connection = connectionFactory.createConnection("first",
"secret");
+ connection.close();
+ } catch (JMSException e) {
+ e.printStackTrace();
+ Assert.fail("should not throw exception");
+ }
+ }
+
+ class RecursiveNettySecurityManager implements ActiveMQSecurityManager5 {
+
+ @Override
+ public boolean validateUser(String user, String password) {
+ return false;
+ }
+
+ @Override
+ public boolean validateUserAndRole(String user, String password,
Set<Role> roles, CheckType checkType) {
+ return false;
+ }
+
+ @Override
+ public Subject authenticate(String user,
+ String password,
+ RemotingConnection remotingConnection,
+ String securityDomain) throws
NoCacheLoginException {
+ NettyConnection nettyConnection = (NettyConnection)
remotingConnection.getTransportConnection();
+ CountDownLatch latch = new CountDownLatch(1);
+ nettyConnection.getChannel().eventLoop().execute(latch::countDown);
+ try {
+ if (!latch.await(10, TimeUnit.SECONDS)) {
+ logger.warn("Cannot complete oepration in time", new
Exception("timeout"));
+ throw new NoCacheLoginException("Can't complete operation in
time");
+ }
+ } catch (InterruptedException e) {
+ logger.warn(e.getMessage(), e);
+ throw new NoCacheLoginException(e.getMessage());
+ }
+ return new Subject();
+ }
+
+ @Override
+ public boolean authorize(Subject subject, Set<Role> roles, CheckType
checkType, String address) {
+ return true;
+ }
+ }
+}
\ No newline at end of file
