You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by cl...@apache.org on 2018/05/24 20:29:34 UTC

[19/33] activemq-artemis git commit: ARTEMIS-1872 Improving Security Checks on AMQP Protocol

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/2.6.x
Commit: 3a5971ec81984a091123c21fd1b9ac6e777fc7cf
Parents: 88b2399
Author: Clebert Suconic <cl...@apache.org>
Authored: Wed May 23 14:52:07 2018 -0400
Committer: Clebert Suconic <cl...@apache.org>
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>