From da3dd29101c8ab14abfde9dbdac6261c87ba3efa Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon (cshannon)" Date: Wed, 28 Feb 2018 16:28:55 -0500 Subject: [PATCH] ARTEMIS-1726 - check proper permissions when using OpenWire Ensure that on queue creation and deletion that the proper permissions are checked --- .../protocol/openwire/OpenWireConnection.java | 5 +- .../protocol/openwire/amq/AMQSession.java | 2 +- .../integration/security/SecurityTest.java | 125 +++++++++++++++++- 3 files changed, 124 insertions(+), 8 deletions(-) diff --git a/artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java b/artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java index 803ed22f3f..90dfe97b42 100644 --- a/artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java +++ b/artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java @@ -80,7 +80,6 @@ import org.apache.activemq.artemis.core.transaction.TransactionOperationAbstract import org.apache.activemq.artemis.core.transaction.TransactionPropertyIndexes; import org.apache.activemq.artemis.spi.core.protocol.AbstractRemotingConnection; import org.apache.activemq.artemis.spi.core.protocol.ConnectionEntry; -import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection; import org.apache.activemq.artemis.spi.core.remoting.Connection; import org.apache.activemq.artemis.utils.UUIDGenerator; import org.apache.activemq.artemis.utils.collections.ConcurrentHashSet; @@ -227,7 +226,7 @@ public class OpenWireConnection extends AbstractRemotingConnection implements Se // SecurityAuth implementation @Override - public RemotingConnection getRemotingConnection() { + public OpenWireConnection getRemotingConnection() { return this; } @@ -989,7 +988,7 @@ public class OpenWireConnection extends AbstractRemotingConnection implements Se public void removeDestination(ActiveMQDestination dest) throws Exception { if (dest.isQueue()) { try { - server.destroyQueue(new SimpleString(dest.getPhysicalName())); + server.destroyQueue(new SimpleString(dest.getPhysicalName()), getRemotingConnection()); } catch (ActiveMQNonExistentQueueException neq) { //this is ok, ActiveMQ 5 allows this and will actually do it quite often ActiveMQServerLogger.LOGGER.debug("queue never existed"); diff --git a/artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQSession.java b/artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQSession.java index fecb5a19ff..c607ca469b 100644 --- a/artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQSession.java +++ b/artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQSession.java @@ -249,7 +249,7 @@ public class AMQSession implements SessionCallback { routingTypeToUse = as.getDefaultAddressRoutingType(); } } - server.createQueue(addressToUse, routingTypeToUse, queueNameToUse, null, true, isTemporary); + coreSession.createQueue(addressToUse, queueNameToUse, routingTypeToUse, null, isTemporary, true); connection.addKnownDestination(queueName); } else { hasQueue = false; diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java index cb59471799..2bced4783f 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java @@ -16,10 +16,6 @@ */ package org.apache.activemq.artemis.tests.integration.security; -import javax.jms.Session; -import javax.security.cert.X509Certificate; -import javax.transaction.xa.XAResource; -import javax.transaction.xa.Xid; import java.lang.management.ManagementFactory; import java.net.URL; import java.util.HashMap; @@ -27,6 +23,13 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; +import javax.jms.MessageProducer; +import javax.jms.QueueBrowser; +import javax.jms.Session; +import javax.security.cert.X509Certificate; +import javax.transaction.xa.XAResource; +import javax.transaction.xa.Xid; + import org.apache.activemq.ActiveMQConnection; import org.apache.activemq.ActiveMQSslConnectionFactory; import org.apache.activemq.artemis.api.core.ActiveMQException; @@ -60,6 +63,7 @@ import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager2; import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager3; import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; import org.apache.activemq.artemis.tests.util.CreateMessage; +import org.apache.activemq.command.ActiveMQQueue; import org.junit.Assert; import org.junit.Before; import org.junit.Ignore; @@ -220,6 +224,119 @@ public class SecurityTest extends ActiveMQTestBase { } } + /** + * Verify role permissions are applied properly when using OpenWire + * + * @throws Exception + */ + @Test + public void testJAASSecurityManagerOpenWireNegative() throws Exception { + ActiveMQJAASSecurityManager securityManager = new ActiveMQJAASSecurityManager("CertLogin"); + ActiveMQServer server = addServer(ActiveMQServers.newActiveMQServer(createDefaultInVMConfig().setSecurityEnabled(true), ManagementFactory.getPlatformMBeanServer(), securityManager, false)); + + Set roles = new HashSet<>(); + roles.add(new Role("programmers", false, false, false, false, false, false, false, false, false, false)); + server.getConfiguration().putSecurityRoles("#", roles); + + Map params = new HashMap<>(); + params.put(TransportConstants.SSL_ENABLED_PROP_NAME, true); + params.put(TransportConstants.KEYSTORE_PATH_PROP_NAME, "server-side-keystore.jks"); + params.put(TransportConstants.KEYSTORE_PASSWORD_PROP_NAME, "secureexample"); + params.put(TransportConstants.TRUSTSTORE_PATH_PROP_NAME, "server-side-truststore.jks"); + params.put(TransportConstants.TRUSTSTORE_PASSWORD_PROP_NAME, "secureexample"); + params.put(TransportConstants.NEED_CLIENT_AUTH_PROP_NAME, true); + + server.getConfiguration().addAcceptorConfiguration(new TransportConfiguration(NETTY_ACCEPTOR_FACTORY, params)); + server.start(); + + ActiveMQSslConnectionFactory factory = new ActiveMQSslConnectionFactory("ssl://localhost:61616"); + factory.setUserName("test-user"); + factory.setTrustStore("client-side-truststore.jks"); + factory.setTrustStorePassword("secureexample"); + factory.setKeyStore("client-side-keystore.jks"); + factory.setKeyStorePassword("secureexample"); + + try (ActiveMQConnection connection = (ActiveMQConnection) factory.createConnection()) { + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + + //Test queue creation permission + try { + session.createConsumer(session.createQueue("test.queue")); + Assert.fail("should throw exception here"); + } catch (Exception e) { + assertTrue(e.getMessage().contains("User: test-user does not have permission='CREATE_DURABLE_QUEUE' for queue test.queue on address test.queue")); + } + + //Test non durable create permission + try { + session.createConsumer(session.createTopic("test.topic")); + Assert.fail("should throw exception here"); + } catch (Exception e) { + assertTrue(e.getMessage().contains("User: test-user does not have permission='CREATE_NON_DURABLE_QUEUE'")); + } + + //Add a test queue and topic to the server + SimpleString address = SimpleString.toSimpleString("test.queue"); + server.addAddressInfo(new AddressInfo(address, RoutingType.ANYCAST)); + server.createQueue(address, RoutingType.ANYCAST, address, null, true, false); + + SimpleString address2 = SimpleString.toSimpleString("test.topic"); + server.addAddressInfo(new AddressInfo(address2, RoutingType.MULTICAST)); + + //Test queue produce permission + try { + MessageProducer producer = session.createProducer(session.createQueue("test.queue")); + producer.send(session.createMessage()); + Assert.fail("should throw exception here"); + } catch (Exception e) { + assertTrue(e.getMessage().contains("User: test-user does not have permission='SEND'")); + } + + //Test queue consume permission + try { + session.createConsumer(session.createQueue("test.queue")); + Assert.fail("should throw exception here"); + } catch (Exception e) { + assertTrue(e.getMessage().contains("User: test-user does not have permission='CONSUME' for queue test.queue on address test.queue")); + } + + //Test queue browse permission + try { + QueueBrowser browser = session.createBrowser(session.createQueue("test.queue")); + browser.getEnumeration(); + Assert.fail("should throw exception here"); + } catch (Exception e) { + assertTrue(e.getMessage().contains("User: test-user does not have permission='BROWSE' for queue test.queue on address test.queue")); + } + + //Test queue deletion permission + try { + connection.destroyDestination(new ActiveMQQueue("test.queue")); + Assert.fail("should throw exception here"); + } catch (Exception e) { + assertTrue(e.getMessage().contains("User: test-user does not have permission='DELETE_DURABLE_QUEUE' for queue test.queue on address test.queue")); + } + + //Test temp queue + try { + session.createTemporaryQueue(); + Assert.fail("should throw exception here"); + } catch (Exception e) { + assertTrue(e.getMessage().contains("User: test-user does not have permission='CREATE_NON_DURABLE_QUEUE'")); + } + + //Test temp topic + try { + session.createTemporaryTopic(); + Assert.fail("should throw exception here"); + } catch (Exception e) { + assertTrue(e.getMessage().contains("User: test-user does not have permission='CREATE_ADDRESS'")); + } + + session.close(); + } + } + @Test public void testJAASSecurityManagerAuthenticationBadPassword() throws Exception { ActiveMQJAASSecurityManager securityManager = new ActiveMQJAASSecurityManager("PropertiesLogin");