diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/CompositeAddress.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/CompositeAddress.java index b646ad38a4..0a6ea6359c 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/CompositeAddress.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/CompositeAddress.java @@ -23,11 +23,22 @@ public class CompositeAddress { public static String SEPARATOR = "::"; public static String toFullyQualified(String address, String qName) { - return new StringBuilder().append(address).append(SEPARATOR).append(qName).toString(); + return toFullyQualified(SimpleString.toSimpleString(address), SimpleString.toSimpleString(qName)).toString(); } public static SimpleString toFullyQualified(SimpleString address, SimpleString qName) { - return address.concat(SEPARATOR).concat(qName); + SimpleString result; + if (address == null && qName == null) { + result = null; + } else if (address != null && qName == null) { + result = address; + } else if (address == null && qName != null) { + result = qName; + } else { + result = address.concat(SEPARATOR).concat(qName); + } + + return result; } public static boolean isFullyQualified(String address) { 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 06d702aa13..a1699b5555 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 @@ -41,6 +41,7 @@ import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager; import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager2; import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager3; import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager4; +import org.apache.activemq.artemis.utils.CompositeAddress; import org.apache.activemq.artemis.utils.collections.ConcurrentHashSet; import org.apache.activemq.artemis.utils.collections.TypedProperties; import org.jboss.logging.Logger; @@ -201,17 +202,28 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC } String user = session.getUsername(); - if (checkCached(address, user, checkType)) { - // OK + // bypass permission checks for management cluster user + if (managementClusterUser.equals(user) && session.getPassword().equals(managementClusterPassword)) { return; } String saddress = address.toString(); - Set roles = securityRepository.getMatch(saddress); - // bypass permission checks for management cluster user - if (managementClusterUser.equals(user) && session.getPassword().equals(managementClusterPassword)) { + /* + * 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 (securityRepository.containsExactMatch(fqqn.toString())) { + roles = securityRepository.getMatch(fqqn.toString()); + isFullyQualified = true; + } + } + + if (checkCached(isFullyQualified ? fqqn : address, user, checkType)) { return; } @@ -257,8 +269,7 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC if (act != null) { set = act; } - set.add(address); - + set.add(isFullyQualified ? fqqn : address); } } 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 4fe0ff4f2e..d6c606fbec 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 @@ -2195,13 +2195,8 @@ public class ActiveMQServerImpl implements ActiveMQServer { } if (session != null) { - - if (queue.isDurable()) { - // make sure the user has privileges to delete this queue - securityStore.check(address, queueName, CheckType.DELETE_DURABLE_QUEUE, session); - } else { - securityStore.check(address, queueName, CheckType.DELETE_NON_DURABLE_QUEUE, session); - } + // make sure the user has privileges to delete this queue + securityStore.check(address, queueName, queue.isDurable() ? CheckType.DELETE_DURABLE_QUEUE : CheckType.DELETE_NON_DURABLE_QUEUE, session); } // This check is only valid if checkConsumerCount == true diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java index 524b2ba7ef..9da5344bb7 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java @@ -538,18 +538,11 @@ public class ServerSessionImpl implements ServerSession, FailureListener { } SimpleString address = removePrefix(binding.getAddress()); - if (browseOnly) { - try { - securityCheck(address, queueName, CheckType.BROWSE, this); - } catch (Exception e) { - securityCheck(address.concat(".").concat(unPrefixedQueueName), queueName, CheckType.BROWSE, this); - } - } else { - try { - securityCheck(address, queueName, CheckType.CONSUME, this); - } catch (Exception e) { - securityCheck(address.concat(".").concat(unPrefixedQueueName), queueName, CheckType.CONSUME, this); - } + try { + securityCheck(address, unPrefixedQueueName, browseOnly ? CheckType.BROWSE : CheckType.CONSUME, this); + } catch (Exception e) { + // this is here for backwards compatibility with the pre-FQQN syntax from ARTEMIS-592 + securityCheck(address.concat(".").concat(unPrefixedQueueName), queueName, browseOnly ? CheckType.BROWSE : CheckType.CONSUME, this); } Filter filter = FilterImpl.createFilter(filterString); @@ -723,12 +716,8 @@ public class ServerSessionImpl implements ServerSession, FailureListener { .setAddress(removePrefix(queueConfiguration.getAddress())) .setName(removePrefix(queueConfiguration.getName())); - if (queueConfiguration.isDurable()) { - // make sure the user has privileges to create this queue - securityCheck(queueConfiguration.getAddress(), queueConfiguration.getName(), CheckType.CREATE_DURABLE_QUEUE, this); - } else { - securityCheck(queueConfiguration.getAddress(), queueConfiguration.getName(), CheckType.CREATE_NON_DURABLE_QUEUE, this); - } + // make sure the user has privileges to create this queue + securityCheck(queueConfiguration.getAddress(), queueConfiguration.getName(), queueConfiguration.isDurable() ? CheckType.CREATE_DURABLE_QUEUE : CheckType.CREATE_NON_DURABLE_QUEUE, this); AddressSettings as = server.getAddressSettingsRepository().getMatch(queueConfiguration.getAddress().toString()); @@ -1043,7 +1032,7 @@ public class ServerSessionImpl implements ServerSession, FailureListener { } queueConfiguration.setAddress(removePrefix(queueConfiguration.getAddress())); - securityCheck(queueConfiguration.getAddress(), queueConfiguration.getName(), queueConfiguration.isDurable() == null || queueConfiguration.isDurable() ? CheckType.CREATE_DURABLE_QUEUE : CheckType.CREATE_NON_DURABLE_QUEUE, this); + securityCheck(queueConfiguration.getAddress(), queueConfiguration.getName(), queueConfiguration.isDurable() ? CheckType.CREATE_DURABLE_QUEUE : CheckType.CREATE_NON_DURABLE_QUEUE, this); server.checkQueueCreationLimit(getUsername()); @@ -2140,10 +2129,9 @@ public class ServerSessionImpl implements ServerSession, FailureListener { AddressInfo art = getAddressAndRoutingType(new AddressInfo(msg.getAddressSimpleString(), routingType)); - // Consumer // check the user has write access to this address. try { - securityCheck(art.getName(), CheckType.SEND, this); + securityCheck(CompositeAddress.extractAddressName(art.getName()), CompositeAddress.isFullyQualified(art.getName()) ? CompositeAddress.extractQueueName(art.getName()) : null, CheckType.SEND, this); } catch (ActiveMQException e) { if (!autoCommitSends && tx != null) { tx.markAsRollbackOnly(e); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/HierarchicalRepository.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/HierarchicalRepository.java index 71379ea88e..80b9f8ee02 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/HierarchicalRepository.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/HierarchicalRepository.java @@ -107,4 +107,6 @@ public interface HierarchicalRepository { void clearCache(); int getCacheSize(); + + boolean containsExactMatch(String match); } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/HierarchicalObjectRepository.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/HierarchicalObjectRepository.java index d6b751df3a..c8eff0113d 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/HierarchicalObjectRepository.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/HierarchicalObjectRepository.java @@ -244,6 +244,11 @@ public class HierarchicalObjectRepository implements HierarchicalRepository +
+ + + + +
+ +``` + +You may want to limit consumption from `q1` to one role and consumption from +`q2` to another role. You can do this using the fully qualified queue name (i.e. +fqqn") in the `match` of the `security-setting`, e.g.: + +```xml + + + + + + +``` + ## Security Setting Plugin Aside from configuring sets of permissions via XML these permissions can 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 2a2c1a377b..87cf781508 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 @@ -65,6 +65,7 @@ import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager3; import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager4; import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; import org.apache.activemq.artemis.tests.util.CreateMessage; +import org.apache.activemq.artemis.utils.CompositeAddress; import org.apache.activemq.command.ActiveMQQueue; import org.junit.Assert; import org.junit.Before; @@ -509,7 +510,17 @@ public class SecurityTest extends ActiveMQTestBase { } @Test - public void testJAASSecurityManagerAuthorizationSameAddressDifferentQueues() throws Exception { + // this is for backwards compatibility with the pre-FQQN syntax from ARTEMIS-592 + public void testJAASSecurityManagerAuthorizationSameAddressDifferentQueuesDotSyntax() throws Exception { + internalJAASSecurityManagerAuthorizationSameAddressDifferentQueues(false); + } + + @Test + public void testJAASSecurityManagerAuthorizationSameAddressDifferentQueuesFqqnSyntax() throws Exception { + internalJAASSecurityManagerAuthorizationSameAddressDifferentQueues(true); + } + + private void internalJAASSecurityManagerAuthorizationSameAddressDifferentQueues(boolean fqqnSyntax) throws Exception { final SimpleString ADDRESS = new SimpleString("address"); final SimpleString QUEUE_A = new SimpleString("a"); final SimpleString QUEUE_B = new SimpleString("b"); @@ -518,10 +529,18 @@ public class SecurityTest extends ActiveMQTestBase { 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, false, false, false, false, false, false, false, false)); - server.getConfiguration().putSecurityRoles(ADDRESS.concat(".").concat(QUEUE_A).toString(), aRoles); + if (fqqnSyntax) { + server.getConfiguration().putSecurityRoles(CompositeAddress.toFullyQualified(ADDRESS, QUEUE_A).toString(), aRoles); + } else { + server.getConfiguration().putSecurityRoles(ADDRESS.concat(".").concat(QUEUE_A).toString(), aRoles); + } Set bRoles = new HashSet<>(); bRoles.add(new Role(QUEUE_B.toString(), false, true, false, false, false, false, false, false, false, false)); - server.getConfiguration().putSecurityRoles(ADDRESS.concat(".").concat(QUEUE_B).toString(), bRoles); + if (fqqnSyntax) { + server.getConfiguration().putSecurityRoles(CompositeAddress.toFullyQualified(ADDRESS, QUEUE_B).toString(), bRoles); + } else { + server.getConfiguration().putSecurityRoles(ADDRESS.concat(".").concat(QUEUE_B).toString(), bRoles); + } server.start(); server.addAddressInfo(new AddressInfo(ADDRESS, RoutingType.ANYCAST)); server.createQueue(new QueueConfiguration(QUEUE_A).setAddress(ADDRESS).setRoutingType(RoutingType.ANYCAST));