From 6ee27224a57d3c5ad7b293497d3bfc892a72a8b0 Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Fri, 7 Jul 2023 12:27:06 -0400 Subject: [PATCH] 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. --- .../core/impl/ActiveMQPacketHandler.java | 9 ++ .../security/RecursiveNettySecurityTest.java | 116 ++++++++++++++++++ 2 files changed, 125 insertions(+) create mode 100644 tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/RecursiveNettySecurityTest.java 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 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 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 roles, CheckType checkType, String address) { + return true; + } + } +} \ No newline at end of file