From f5a6189e2dcc24b59cd1fc287d0d4cbc65c20efb Mon Sep 17 00:00:00 2001 From: Justin Bertram Date: Fri, 28 Aug 2020 22:24:47 -0500 Subject: [PATCH] ARTEMIS-2890 FQQN security-settings + JMS not working --- .../core/security/impl/SecurityStoreImpl.java | 30 +++---- .../core/server/ActiveMQMessageBundle.java | 4 +- .../integration/security/SecurityTest.java | 82 +++++++++++++++++-- 3 files changed, 93 insertions(+), 23 deletions(-) diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java index 95e3b626b1..c6707910b0 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java @@ -235,8 +235,11 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC final CheckType checkType, final SecurityAuth session) throws Exception { if (securityEnabled) { + SimpleString bareAddress = CompositeAddress.extractAddressName(address); + SimpleString bareQueue = CompositeAddress.extractQueueName(queue); + if (logger.isTraceEnabled()) { - logger.trace("checking access permissions to " + address); + logger.trace("checking access permissions to " + bareAddress); } // bypass permission checks for management cluster user @@ -245,23 +248,22 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC return; } - String saddress = address.toString(); - Set roles = securityRepository.getMatch(saddress); + Set roles = securityRepository.getMatch(bareAddress.toString()); /* * If a valid queue is passed in and there's an exact match for the FQQN then use the FQQN instead of the address */ boolean isFullyQualified = false; SimpleString fqqn = null; - if (queue != null) { - fqqn = CompositeAddress.toFullyQualified(address, queue); + if (bareQueue != null) { + fqqn = CompositeAddress.toFullyQualified(bareAddress, bareQueue); if (securityRepository.containsExactMatch(fqqn.toString())) { roles = securityRepository.getMatch(fqqn.toString()); isFullyQualified = true; } } - if (checkAuthorizationCache(isFullyQualified ? fqqn : address, user, checkType)) { + if (checkAuthorizationCache(isFullyQualified ? fqqn : bareAddress, user, checkType)) { return; } @@ -270,11 +272,11 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC Subject subject = getSubjectForAuthorization(session, ((ActiveMQSecurityManager5) securityManager)); validated = ((ActiveMQSecurityManager5) securityManager).authorize(subject, roles, checkType); } else if (securityManager instanceof ActiveMQSecurityManager4) { - validated = ((ActiveMQSecurityManager4) securityManager).validateUserAndRole(user, session.getPassword(), roles, checkType, saddress, session.getRemotingConnection(), session.getSecurityDomain()) != null; + validated = ((ActiveMQSecurityManager4) securityManager).validateUserAndRole(user, session.getPassword(), roles, checkType, bareAddress.toString(), session.getRemotingConnection(), session.getSecurityDomain()) != null; } else if (securityManager instanceof ActiveMQSecurityManager3) { - validated = ((ActiveMQSecurityManager3) securityManager).validateUserAndRole(user, session.getPassword(), roles, checkType, saddress, session.getRemotingConnection()) != null; + validated = ((ActiveMQSecurityManager3) securityManager).validateUserAndRole(user, session.getPassword(), roles, checkType, bareAddress.toString(), session.getRemotingConnection()) != null; } else if (securityManager instanceof ActiveMQSecurityManager2) { - validated = ((ActiveMQSecurityManager2) securityManager).validateUserAndRole(user, session.getPassword(), roles, checkType, saddress, session.getRemotingConnection()); + validated = ((ActiveMQSecurityManager2) securityManager).validateUserAndRole(user, session.getPassword(), roles, checkType, bareAddress.toString(), session.getRemotingConnection()); } else { validated = securityManager.validateUserAndRole(user, session.getPassword(), roles, checkType); } @@ -283,7 +285,7 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC if (notificationService != null) { TypedProperties props = new TypedProperties(); - props.putSimpleStringProperty(ManagementHelper.HDR_ADDRESS, address); + props.putSimpleStringProperty(ManagementHelper.HDR_ADDRESS, bareAddress); props.putSimpleStringProperty(ManagementHelper.HDR_CHECK_TYPE, new SimpleString(checkType.toString())); props.putSimpleStringProperty(ManagementHelper.HDR_USER, SimpleString.toSimpleString(user)); @@ -293,10 +295,10 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC } Exception ex; - if (queue == null) { - ex = ActiveMQMessageBundle.BUNDLE.userNoPermissions(session.getUsername(), checkType, saddress); + if (bareQueue == null) { + ex = ActiveMQMessageBundle.BUNDLE.userNoPermissions(session.getUsername(), checkType, bareAddress); } else { - ex = ActiveMQMessageBundle.BUNDLE.userNoPermissionsQueue(session.getUsername(), checkType, queue.toString(), saddress); + ex = ActiveMQMessageBundle.BUNDLE.userNoPermissionsQueue(session.getUsername(), checkType, bareQueue, bareAddress); } AuditLogger.securityFailure(ex); throw ex; @@ -312,7 +314,7 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC set = new ConcurrentHashSet<>(); authorizationCache.put(key, set); } - set.add(isFullyQualified ? fqqn : address); + set.add(isFullyQualified ? fqqn : bareAddress); } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQMessageBundle.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQMessageBundle.java index 5da0521cb9..b25919444c 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQMessageBundle.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQMessageBundle.java @@ -164,7 +164,7 @@ public interface ActiveMQMessageBundle { ActiveMQSecurityException unableToValidateUser(String remoteAddress, String user, String certMessage); @Message(id = 229032, value = "User: {0} does not have permission=''{1}'' on address {2}", format = Message.Format.MESSAGE_FORMAT) - ActiveMQSecurityException userNoPermissions(String username, CheckType checkType, String saddress); + ActiveMQSecurityException userNoPermissions(String username, CheckType checkType, SimpleString address); @Message(id = 229033, value = "Server and client versions incompatible") ActiveMQIncompatibleClientServerException incompatibleClientServer(); @@ -437,7 +437,7 @@ public interface ActiveMQMessageBundle { IllegalArgumentException invalidDeletionPolicyType(String val); @Message(id = 229213, value = "User: {0} does not have permission=''{1}'' for queue {2} on address {3}", format = Message.Format.MESSAGE_FORMAT) - ActiveMQSecurityException userNoPermissionsQueue(String username, CheckType checkType, String squeue, String saddress); + ActiveMQSecurityException userNoPermissionsQueue(String username, CheckType checkType, SimpleString queue, SimpleString address); @Message(id = 229214, value = "{0} must be a valid percentage value between 0 and 100 or -1 (actual value: {1})", format = Message.Format.MESSAGE_FORMAT) IllegalArgumentException notPercentOrMinusOne(String name, Number val); 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 8535cb0385..3a0642f625 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,19 +16,22 @@ */ package org.apache.activemq.artemis.tests.integration.security; -import java.lang.management.ManagementFactory; -import java.net.URL; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; - +import javax.jms.Connection; +import javax.jms.ConnectionFactory; +import javax.jms.JMSException; +import javax.jms.JMSSecurityException; 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 java.lang.management.ManagementFactory; +import java.net.URL; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; import org.apache.activemq.ActiveMQConnection; import org.apache.activemq.ActiveMQSslConnectionFactory; @@ -58,6 +61,7 @@ import org.apache.activemq.artemis.core.server.Queue; import org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl; import org.apache.activemq.artemis.core.server.impl.AddressInfo; import org.apache.activemq.artemis.core.settings.HierarchicalRepository; +import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory; import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection; import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager; import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager; @@ -585,6 +589,70 @@ public class SecurityTest extends ActiveMQTestBase { } } + @Test + public void testJAASSecurityManagerFQQNAuthorizationWithJMS() throws Exception { + final SimpleString ADDRESS = new SimpleString("address"); + final SimpleString QUEUE_A = new SimpleString("a"); + final SimpleString QUEUE_B = new SimpleString("b"); + + ActiveMQJAASSecurityManager securityManager = new ActiveMQJAASSecurityManager("PropertiesLogin"); + ActiveMQServer server = addServer(ActiveMQServers.newActiveMQServer(createDefaultInVMConfig().setSecurityEnabled(true), ManagementFactory.getPlatformMBeanServer(), securityManager, false)); + + Set aRoles = new HashSet<>(); + aRoles.add(new Role(QUEUE_A.toString(), false, true, true, false, false, false, false, false, true, false)); + server.getConfiguration().putSecurityRoles(CompositeAddress.toFullyQualified(ADDRESS, QUEUE_A).toString(), aRoles); + + Set bRoles = new HashSet<>(); + bRoles.add(new Role(QUEUE_B.toString(), false, true, true, false, false, false, false, false, true, false)); + server.getConfiguration().putSecurityRoles(CompositeAddress.toFullyQualified(ADDRESS, QUEUE_B).toString(), bRoles); + + server.start(); + + ConnectionFactory cf = new ActiveMQConnectionFactory("vm://0"); + Connection aConnection = cf.createConnection("a", "a"); + Session aSession = aConnection.createSession(); + Connection bConnection = cf.createConnection("b", "b"); + Session bSession = bConnection.createSession(); + + javax.jms.Queue queueA = aSession.createQueue(CompositeAddress.toFullyQualified(ADDRESS, QUEUE_A).toString()); + javax.jms.Queue queueB = bSession.createQueue(CompositeAddress.toFullyQualified(ADDRESS, QUEUE_B).toString()); + + // client A CONSUME from queue A + try { + aSession.createConsumer(queueA); + } catch (JMSException e) { + e.printStackTrace(); + Assert.fail("should not throw exception here"); + } + + // client B CONSUME from queue A + try { + bSession.createConsumer(queueA); + Assert.fail("should throw exception here"); + } catch (JMSException e) { + assertTrue(e instanceof JMSSecurityException); + } + + // client B CONSUME from queue B + try { + bSession.createConsumer(queueB); + } catch (JMSException e) { + e.printStackTrace(); + Assert.fail("should not throw exception here"); + } + + // client A CONSUME from queue B + try { + aSession.createConsumer(queueB); + Assert.fail("should throw exception here"); + } catch (JMSException e) { + assertTrue(e instanceof JMSSecurityException); + } + + aConnection.close(); + bConnection.close(); + } + @Test public void testJAASSecurityManagerAuthorizationNegativeWithCerts() throws Exception { final SimpleString ADDRESS = new SimpleString("address");