Repository: activemq-artemis Updated Branches: refs/heads/master 88b23994c -> d6d685134
ARTEMIS-1872 Improving Security Checks on AMQP Protocol Also improving test coverage on SecureConfigurationTest This commit will fix JMSConnectionWithSecurityTest. Project: http://git-wip-us.apache.org/repos/asf/activemq-artemis/repo Commit: http://git-wip-us.apache.org/repos/asf/activemq-artemis/commit/3a5971ec Tree: http://git-wip-us.apache.org/repos/asf/activemq-artemis/tree/3a5971ec Diff: http://git-wip-us.apache.org/repos/asf/activemq-artemis/diff/3a5971ec Branch: refs/heads/master Commit: 3a5971ec81984a091123c21fd1b9ac6e777fc7cf Parents: 88b2399 Author: Clebert Suconic <[email protected]> Authored: Wed May 23 14:52:07 2018 -0400 Committer: Clebert Suconic <[email protected]> Committed: Wed May 23 15:01:41 2018 -0400 ---------------------------------------------------------------------- .../amqp/broker/AMQPSessionCallback.java | 2 +- .../proton/ProtonServerReceiverContext.java | 3 + .../server/SecureConfigurationTest.java | 83 +++++++++++--------- .../src/test/resources/multicast_topic.xml | 12 +-- 4 files changed, 55 insertions(+), 45 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/3a5971ec/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.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 1301f0b..df9b61e 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 @@ -248,7 +248,7 @@ public class AMQPSessionCallback implements SessionCallback { try { serverSession.createQueue(address, queueName, routingType, filter, true, false); } catch (ActiveMQSecurityException se) { - throw ActiveMQAMQPProtocolMessageBundle.BUNDLE.securityErrorCreatingConsumer(se.getMessage()); + throw ActiveMQAMQPProtocolMessageBundle.BUNDLE.securityErrorCreatingTempDestination(se.getMessage()); } } http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/3a5971ec/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerReceiverContext.java ---------------------------------------------------------------------- diff --git a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerReceiverContext.java b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerReceiverContext.java index 0036004..aad89a8 100644 --- a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerReceiverContext.java +++ b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerReceiverContext.java @@ -29,6 +29,7 @@ import org.apache.activemq.artemis.protocol.amqp.broker.AMQPSessionCallback; import org.apache.activemq.artemis.protocol.amqp.exceptions.ActiveMQAMQPException; import org.apache.activemq.artemis.protocol.amqp.exceptions.ActiveMQAMQPInternalErrorException; import org.apache.activemq.artemis.protocol.amqp.exceptions.ActiveMQAMQPNotFoundException; +import org.apache.activemq.artemis.protocol.amqp.exceptions.ActiveMQAMQPSecurityException; import org.apache.activemq.artemis.protocol.amqp.logger.ActiveMQAMQPProtocolMessageBundle; import org.apache.activemq.artemis.protocol.amqp.sasl.PlainSASLResult; import org.apache.activemq.artemis.protocol.amqp.sasl.SASLResult; @@ -108,6 +109,8 @@ public class ProtonServerReceiverContext extends ProtonInitializable implements try { sessionSPI.createTemporaryQueue(address, defRoutingType); + } catch (ActiveMQAMQPSecurityException e) { + throw e; } catch (ActiveMQSecurityException e) { throw ActiveMQAMQPProtocolMessageBundle.BUNDLE.securityErrorCreatingTempDestination(e.getMessage()); } catch (Exception e) { http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/3a5971ec/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SecureConfigurationTest.java ---------------------------------------------------------------------- diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SecureConfigurationTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SecureConfigurationTest.java index fd0a12e..615ffcc 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SecureConfigurationTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SecureConfigurationTest.java @@ -25,10 +25,13 @@ import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory; import org.apache.activemq.artemis.jms.server.config.impl.FileJMSConfiguration; import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager; import org.apache.activemq.artemis.spi.core.security.jaas.InVMLoginModule; +import org.apache.activemq.artemis.tests.integration.IntegrationTestLogger; import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; import org.apache.qpid.jms.JmsConnectionFactory; +import org.junit.After; import org.junit.Assert; import org.junit.Assume; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -58,24 +61,30 @@ public class SecureConfigurationTest extends ActiveMQTestBase { @Parameterized.Parameter(0) public String protocol; - @Test - public void testSecureSharedDurableSubscriber() throws Exception { - //This is because OpenWire does not support JMS 2.0 - Assume.assumeFalse(protocol.equals("OPENWIRE")); + ActiveMQServer server; + + @Before + public void startSever() throws Exception { + server = getActiveMQServer("multicast_topic.xml"); + server.start(); + } - ActiveMQServer server = getActiveMQServer("multicast_topic.xml"); + @After + public void stopServer() throws Exception { try { - server.start(); - internal_testSecureSharedDurableSubscriber(getConnectionFactory("b", "b")); - } finally { - try { + if (server != null) { server.stop(); - } catch (Exception e) { } + } catch (Throwable e) { + e.printStackTrace(); } } - private void internal_testSecureSharedDurableSubscriber(ConnectionFactory connectionFactory) throws JMSException { + @Test + public void testSecureSharedDurableSubscriber() throws Exception { + //This is because OpenWire does not support JMS 2.0 + Assume.assumeFalse(protocol.equals("OPENWIRE")); + ConnectionFactory connectionFactory = getConnectionFactory("b", "b"); String message = "blah"; //Expect to be able to create subscriber on pre-defined/existing queue. @@ -101,20 +110,7 @@ public class SecureConfigurationTest extends ActiveMQTestBase { public void testSecureSharedSubscriber() throws Exception { //This is because OpenWire does not support JMS 2.0 Assume.assumeFalse(protocol.equals("OPENWIRE")); - - ActiveMQServer server = getActiveMQServer("multicast_topic.xml"); - try { - server.start(); - internal_testSecureSharedSubscriber(getConnectionFactory("b", "b")); - } finally { - try { - server.stop(); - } catch (Exception e) { - } - } - } - - private void internal_testSecureSharedSubscriber(ConnectionFactory connectionFactory) throws JMSException { + ConnectionFactory connectionFactory = getConnectionFactory("b", "b"); String message = "blah"; //Expect to be able to create subscriber on pre-defined/existing queue. @@ -138,19 +134,7 @@ public class SecureConfigurationTest extends ActiveMQTestBase { @Test public void testSecureDurableSubscriber() throws Exception { - ActiveMQServer server = getActiveMQServer("multicast_topic.xml"); - try { - server.start(); - internal_testSecureDurableSubscriber(getConnectionFactory("b", "b")); - } finally { - try { - server.stop(); - } catch (Exception e) { - } - } - } - - private void internal_testSecureDurableSubscriber(ConnectionFactory connectionFactory) throws JMSException { + ConnectionFactory connectionFactory = getConnectionFactory("b", "b"); String message = "blah"; //Expect to be able to create subscriber on pre-defined/existing queue. @@ -177,8 +161,31 @@ public class SecureConfigurationTest extends ActiveMQTestBase { } catch (JMSSecurityException j) { //Expected exception } + + Connection connection = null; + + try { + connection = connectionFactory.createConnection(); + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + + try { + session.createTemporaryQueue(); + Assert.fail("Security exception expected, but did not occur, excepetion expected as not permissioned to create a temporary queue"); + } catch (JMSSecurityException jmsse) { + IntegrationTestLogger.LOGGER.info("Client should have thrown a JMSSecurityException but only threw JMSException"); + } catch (JMSException e) { + e.printStackTrace(); + Assert.fail("thrown a JMSEXception instead of a JMSSEcurityException"); + } + + // Should not be fatal + assertNotNull(connection.createSession(false, Session.AUTO_ACKNOWLEDGE)); + } finally { + connection.close(); + } } + private ConnectionFactory getConnectionFactory(String user, String password) { switch (protocol) { case "CORE": return getActiveMQConnectionFactory(user, password); http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/3a5971ec/tests/integration-tests/src/test/resources/multicast_topic.xml ---------------------------------------------------------------------- diff --git a/tests/integration-tests/src/test/resources/multicast_topic.xml b/tests/integration-tests/src/test/resources/multicast_topic.xml index cf5430e..2535ad3 100644 --- a/tests/integration-tests/src/test/resources/multicast_topic.xml +++ b/tests/integration-tests/src/test/resources/multicast_topic.xml @@ -85,13 +85,13 @@ under the License. <security-settings> <security-setting match="#"> - <permission type="createNonDurableQueue" roles="a,b"/> - <permission type="deleteNonDurableQueue" roles="a,b"/> - <permission type="createDurableQueue" roles="a,b"/> - <permission type="deleteDurableQueue" roles="a,b"/> + <permission type="createNonDurableQueue" roles="a"/> + <permission type="deleteNonDurableQueue" roles="a"/> + <permission type="createDurableQueue" roles="a"/> + <permission type="deleteDurableQueue" roles="a"/> <permission type="browse" roles="a"/> - <permission type="send" roles="a,b"/> - <permission type="consume" roles="a,b" /> + <permission type="send" roles="a"/> + <permission type="consume" roles="a" /> <!-- we need this otherwise ./artemis data imp wouldn't work --> <permission type="manage" roles="a"/> </security-setting>
