From b4beea1f2c2c2cd3292f19a5ec00e2a27d03e671 Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Mon, 22 Mar 2021 12:22:41 -0400 Subject: [PATCH] ARTEMIS-3150 Broker Connections with restricted security The local connections and sessions created internally were supposed to bypass security just like bridges and other internal components --- .../amqp/broker/AMQPSessionCallback.java | 21 +- .../amqp/proton/AMQPConnectionContext.java | 4 + .../artemis/core/server/ActiveMQServer.java | 15 ++ .../core/server/impl/ActiveMQServerImpl.java | 48 +++- tests/smoke-tests/pom.xml | 40 +++ .../artemis-roles.properties | 19 ++ .../artemis-users.properties | 19 ++ .../brokerConnectBridgeSecurityA/broker.xml | 251 ++++++++++++++++++ .../artemis-roles.properties | 18 ++ .../artemis-users.properties | 18 ++ .../brokerConnectBridgeSecurityB/broker.xml | 243 +++++++++++++++++ .../BrokerConnectionBridgeSecurityTest.java | 96 +++++++ 12 files changed, 774 insertions(+), 18 deletions(-) create mode 100644 tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityA/artemis-roles.properties create mode 100644 tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityA/artemis-users.properties create mode 100644 tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityA/broker.xml create mode 100644 tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityB/artemis-roles.properties create mode 100644 tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityB/artemis-users.properties create mode 100644 tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityB/broker.xml create mode 100644 tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/brokerConnection/BrokerConnectionBridgeSecurityTest.java diff --git a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java index f0fb1efe45..9be3858f04 100644 --- a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java +++ b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java @@ -207,12 +207,21 @@ public class AMQPSessionCallback implements SessionCallback { } } - serverSession = manager.getServer().createSession(name, user, passcode, ActiveMQClient.DEFAULT_MIN_LARGE_MESSAGE_SIZE, protonSPI.getProtonConnectionDelegate(), // RemotingConnection remotingConnection, - false, // boolean autoCommitSends - false, // boolean autoCommitAcks, - false, // boolean preAcknowledge, - true, //boolean xa, - (String) null, this, true, operationContext, manager.getPrefixes(), manager.getSecurityDomain()); + if (connection.isBridgeConnection()) { + serverSession = manager.getServer().createInternalSession(name, ActiveMQClient.DEFAULT_MIN_LARGE_MESSAGE_SIZE, protonSPI.getProtonConnectionDelegate(), // RemotingConnection remotingConnection, + false, // boolean autoCommitSends + false, // boolean autoCommitAcks, + false, // boolean preAcknowledge, + true, //boolean xa, + (String) null, this, true, operationContext, manager.getPrefixes(), manager.getSecurityDomain()); + } else { + serverSession = manager.getServer().createSession(name, user, passcode, ActiveMQClient.DEFAULT_MIN_LARGE_MESSAGE_SIZE, protonSPI.getProtonConnectionDelegate(), // RemotingConnection remotingConnection, + false, // boolean autoCommitSends + false, // boolean autoCommitAcks, + false, // boolean preAcknowledge, + true, //boolean xa, + (String) null, this, true, operationContext, manager.getPrefixes(), manager.getSecurityDomain()); + } } @Override diff --git a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java index 5c490f6d7a..de37eacc56 100644 --- a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java +++ b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java @@ -172,6 +172,10 @@ public class AMQPConnectionContext extends ProtonInitializable implements EventH } } + public boolean isBridgeConnection() { + return bridgeConnection; + } + public void requireInHandler() { handler.requireHandler(); } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java index 291a8f8b1f..2d6eb63422 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java @@ -342,6 +342,21 @@ public interface ActiveMQServer extends ServiceComponent { Map prefixes, String securityDomain) throws Exception; + /** This is to be used in places where security is bypassed, like internal sessions, broker connections, etc... */ + ServerSession createInternalSession(String name, + int minLargeMessageSize, + RemotingConnection remotingConnection, + boolean autoCommitSends, + boolean autoCommitAcks, + boolean preAcknowledge, + boolean xa, + String defaultAddress, + SessionCallback callback, + boolean autoCreateQueues, + OperationContext context, + Map prefixes, + String securityDomain) throws Exception; + SecurityStore getSecurityStore(); void removeSession(String name) throws Exception; diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java index 306b1955d1..cfc95f6068 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java @@ -1681,21 +1681,31 @@ public class ActiveMQServerImpl implements ActiveMQServer { checkSessionLimit(validatedUser); - if (hasBrokerSessionPlugins()) { - callBrokerSessionPlugins(plugin -> plugin.beforeCreateSession(name, username, minLargeMessageSize, connection, - autoCommitSends, autoCommitAcks, preAcknowledge, xa, defaultAddress, callback, autoCreateQueues, context, prefixes)); - } final ServerSessionImpl session = internalCreateSession(name, username, password, validatedUser, minLargeMessageSize, connection, autoCommitSends, autoCommitAcks, preAcknowledge, xa, defaultAddress, callback, context, autoCreateQueues, prefixes, securityDomain); - sessions.put(name, session); - - if (hasBrokerSessionPlugins()) { - callBrokerSessionPlugins(plugin -> plugin.afterCreateSession(session)); - } - return session; } + @Override + public ServerSession createInternalSession(String name, + int minLargeMessageSize, + RemotingConnection connection, + boolean autoCommitSends, + boolean autoCommitAcks, + boolean preAcknowledge, + boolean xa, + String defaultAddress, + SessionCallback callback, + boolean autoCreateQueues, + OperationContext context, + Map prefixes, + String securityDomain) throws Exception { + ServerSessionImpl session = internalCreateSession(name, null, null, null, minLargeMessageSize, connection, autoCommitSends, autoCommitAcks, preAcknowledge, xa, defaultAddress, callback, context, autoCreateQueues, prefixes, securityDomain); + session.disableSecurity(); + return session; + } + + private void checkSessionLimit(String username) throws Exception { if (configuration.getResourceLimitSettings() != null && configuration.getResourceLimitSettings().containsKey(username)) { ResourceLimitSettings limits = configuration.getResourceLimitSettings().get(username); @@ -1759,10 +1769,24 @@ public class ActiveMQServerImpl implements ActiveMQServer { String defaultAddress, SessionCallback callback, OperationContext context, - boolean autoCreateJMSQueues, + boolean autoCreateQueues, Map prefixes, String securityDomain) throws Exception { - return new ServerSessionImpl(name, username, password, validatedUser, minLargeMessageSize, autoCommitSends, autoCommitAcks, preAcknowledge, configuration.isPersistDeliveryCountBeforeDelivery(), xa, connection, storageManager, postOffice, resourceManager, securityStore, managementService, this, configuration.getManagementAddress(), defaultAddress == null ? null : new SimpleString(defaultAddress), callback, context, pagingManager, prefixes, securityDomain); + + if (hasBrokerSessionPlugins()) { + callBrokerSessionPlugins(plugin -> plugin.beforeCreateSession(name, username, minLargeMessageSize, connection, + autoCommitSends, autoCommitAcks, preAcknowledge, xa, defaultAddress, callback, autoCreateQueues, context, prefixes)); + } + + ServerSessionImpl session = new ServerSessionImpl(name, username, password, validatedUser, minLargeMessageSize, autoCommitSends, autoCommitAcks, preAcknowledge, configuration.isPersistDeliveryCountBeforeDelivery(), xa, connection, storageManager, postOffice, resourceManager, securityStore, managementService, this, configuration.getManagementAddress(), defaultAddress == null ? null : new SimpleString(defaultAddress), callback, context, pagingManager, prefixes, securityDomain); + + sessions.put(name, session); + + if (hasBrokerSessionPlugins()) { + callBrokerSessionPlugins(plugin -> plugin.afterCreateSession(session)); + } + + return session; } @Override diff --git a/tests/smoke-tests/pom.xml b/tests/smoke-tests/pom.xml index abd693d997..135b63c777 100644 --- a/tests/smoke-tests/pom.xml +++ b/tests/smoke-tests/pom.xml @@ -143,6 +143,14 @@ 1.15.1 test + + + org.apache.activemq.tests + integration-tests + ${project.version} + test + test-jar + @@ -667,6 +675,38 @@ ${basedir}/target/transfer2 + + + test-compile + createBrokerConnectBridgeSecurityA + + create + + + false + A + A + true + ${basedir}/target/brokerConnectBridgeSecurityA + ${basedir}/target/classes/servers/brokerConnectBridgeSecurityA + + + + test-compile + createBrokerConnectBridgeSecurityB + + create + + + false + B + B + true + 100 + ${basedir}/target/brokerConnectBridgeSecurityB + ${basedir}/target/classes/servers/brokerConnectBridgeSecurityB + + diff --git a/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityA/artemis-roles.properties b/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityA/artemis-roles.properties new file mode 100644 index 0000000000..7d6b609bc6 --- /dev/null +++ b/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityA/artemis-roles.properties @@ -0,0 +1,19 @@ +## --------------------------------------------------------------------------- +## 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. +## --------------------------------------------------------------------------- + +sender = A +amq = admin \ No newline at end of file diff --git a/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityA/artemis-users.properties b/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityA/artemis-users.properties new file mode 100644 index 0000000000..27a853c1d2 --- /dev/null +++ b/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityA/artemis-users.properties @@ -0,0 +1,19 @@ +## --------------------------------------------------------------------------- +## 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. +## --------------------------------------------------------------------------- + +A = A +admin = admin \ No newline at end of file diff --git a/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityA/broker.xml b/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityA/broker.xml new file mode 100644 index 0000000000..72e6541076 --- /dev/null +++ b/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityA/broker.xml @@ -0,0 +1,251 @@ + + + + + + + + 0.0.0.0 + + + false + + + NIO + + data/paging + + data/bindings + + data/journal + + data/large-messages + + true + + 2 + + 10 + + 4096 + + 10M + + + 40000 + + + + 1 + + + + + + + + + + + + + + + + + + + + + 5000 + + + 90 + + + true + + 120000 + + 60000 + + HALT + + + 40000 + + + + tcp://0.0.0.0:61616?tcpSendBufferSize=1048576;tcpReceiveBufferSize=1048576;amqpMinLargeMessageSize=102400;protocols=CORE,AMQP,STOMP,HORNETQ,MQTT,OPENWIRE;useEpoll=true;amqpCredits=1000;amqpLowCredits=300;amqpDuplicateDetection=true + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + DLQ + ExpiryQueue + 0 + + -1 + 10 + PAGE + true + true + true + true + + + + DLQ + ExpiryQueue + 0 + + -1 + 10 + PAGE + true + true + true + true + + + + +
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
+ +
+ + + + +
+
diff --git a/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityB/artemis-roles.properties b/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityB/artemis-roles.properties new file mode 100644 index 0000000000..d5afbe587d --- /dev/null +++ b/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityB/artemis-roles.properties @@ -0,0 +1,18 @@ +## --------------------------------------------------------------------------- +## 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. +## --------------------------------------------------------------------------- + +amq = B \ No newline at end of file diff --git a/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityB/artemis-users.properties b/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityB/artemis-users.properties new file mode 100644 index 0000000000..9842cd6fc7 --- /dev/null +++ b/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityB/artemis-users.properties @@ -0,0 +1,18 @@ +## --------------------------------------------------------------------------- +## 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. +## --------------------------------------------------------------------------- + +B = B \ No newline at end of file diff --git a/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityB/broker.xml b/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityB/broker.xml new file mode 100644 index 0000000000..694d947815 --- /dev/null +++ b/tests/smoke-tests/src/main/resources/servers/brokerConnectBridgeSecurityB/broker.xml @@ -0,0 +1,243 @@ + + + + + + + + 0.0.0.0 + + + false + + + NIO + + data/paging + + data/bindings + + data/journal + + data/large-messages + + true + + 2 + + 10 + + 4096 + + 10M + + + 40000 + + + + 1 + + + + + + + + + + + + + + + + + + + + + 5000 + + + 90 + + + true + + 120000 + + 60000 + + HALT + + + 40000 + + + + tcp://0.0.0.0:61617?tcpSendBufferSize=1048576;tcpReceiveBufferSize=1048576;amqpMinLargeMessageSize=102400;protocols=CORE,AMQP,STOMP,HORNETQ,MQTT,OPENWIRE;useEpoll=true;amqpCredits=1000;amqpLowCredits=300;amqpDuplicateDetection=true + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + DLQ + ExpiryQueue + 0 + + -1 + 10 + PAGE + true + true + true + true + + + + DLQ + ExpiryQueue + 0 + + -1 + 10 + PAGE + true + true + true + true + + + + +
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
+ +
+ + + + +
+
diff --git a/tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/brokerConnection/BrokerConnectionBridgeSecurityTest.java b/tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/brokerConnection/BrokerConnectionBridgeSecurityTest.java new file mode 100644 index 0000000000..1980d12dc4 --- /dev/null +++ b/tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/brokerConnection/BrokerConnectionBridgeSecurityTest.java @@ -0,0 +1,96 @@ +/* + * 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.smoke.brokerConnection; + +import javax.jms.Connection; +import javax.jms.ConnectionFactory; +import javax.jms.MessageConsumer; +import javax.jms.MessageProducer; +import javax.jms.Queue; +import javax.jms.Session; +import javax.jms.TextMessage; + +import org.apache.activemq.artemis.tests.smoke.common.SmokeTestBase; +import org.apache.activemq.artemis.util.ServerUtil; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.apache.activemq.artemis.tests.util.CFUtil; + +public class BrokerConnectionBridgeSecurityTest extends SmokeTestBase { + + public static final String SERVER_NAME_A = "brokerConnectBridgeSecurityA"; + public static final String SERVER_NAME_B = "brokerConnectBridgeSecurityB"; + + @Before + public void before() throws Exception { + // no need to cleanup, these servers don't have persistence + // start serverB first, after all ServerA needs it alive to create connections + startServer(SERVER_NAME_B, 0, 0); + startServer(SERVER_NAME_A, 0, 0); + + ServerUtil.waitForServerToStart(1, "B", "B", 30000); + ServerUtil.waitForServerToStart(0, "A", "A", 30000); + } + + @Test + public void testBridgeOverBokerConnection() throws Throwable { + ConnectionFactory cfA = CFUtil.createConnectionFactory("amqp", "tcp://localhost:61616"); + ConnectionFactory cfB = CFUtil.createConnectionFactory("amqp", "tcp://localhost:61617"); + + + try (Connection connectionA = cfA.createConnection("A", "A"); + Connection connectionB = cfB.createConnection("B", "B")) { + Session sessionA = connectionA.createSession(false, Session.AUTO_ACKNOWLEDGE); + Queue queueToB = sessionA.createQueue("toB"); + Queue queueToA = sessionA.createQueue("toA"); + MessageProducer producerA = sessionA.createProducer(queueToB); + for (int i = 0; i < 10; i++) { + producerA.send(sessionA.createTextMessage("toB")); + } + + Session sessionB = connectionB.createSession(false, Session.AUTO_ACKNOWLEDGE); + MessageConsumer consumerB = sessionB.createConsumer(queueToB); + connectionB.start(); + + for (int i = 0; i < 10; i++) { + TextMessage message = (TextMessage) consumerB.receive(1000); + Assert.assertNotNull(message); + Assert.assertEquals("toB", message.getText()); + } + + MessageProducer producerB = sessionB.createProducer(queueToA); + for (int i = 0; i < 10; i++) { + producerB.send(sessionA.createTextMessage("toA")); + } + Assert.assertNull(consumerB.receiveNoWait()); + + connectionA.start(); + + MessageConsumer consumerA = sessionA.createConsumer(queueToA); + for (int i = 0; i < 10; i++) { + TextMessage message = (TextMessage) consumerA.receive(1000); + Assert.assertNotNull(message); + Assert.assertEquals("toA", message.getText()); + } + Assert.assertNull(consumerA.receiveNoWait()); + + } + } + +}