diff options
Diffstat (limited to 'java/broker-core/src')
7 files changed, 284 insertions, 24 deletions
diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java b/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java index 77eda0dc67..2aba33104a 100644 --- a/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java @@ -895,9 +895,19 @@ public class BrokerAdapter extends AbstractConfiguredObject<BrokerAdapter> imple protected <C extends ConfiguredObject> void authoriseCreateChild(Class<C> childClass, Map<String, Object> attributes, ConfiguredObject... otherParents) throws AccessControlException { - if (!_securityManager.authoriseConfiguringBroker(String.valueOf(attributes.get(NAME)), childClass, Operation.CREATE)) + if (childClass == VirtualHostNode.class) { - throw new AccessControlException("Creation of new broker level entity is denied"); + _securityManager.authoriseVirtualHostNode(String.valueOf(attributes.get(NAME)), Operation.CREATE); + + } + else + { + if (!_securityManager.authoriseConfiguringBroker(String.valueOf(attributes.get(NAME)), + childClass, + Operation.CREATE)) + { + throw new AccessControlException("Creation of new broker level entity is denied"); + } } } diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java b/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java index a8a59a317c..2f7acba91d 100755 --- a/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java @@ -25,6 +25,7 @@ import static org.apache.qpid.server.security.access.ObjectType.METHOD; import static org.apache.qpid.server.security.access.ObjectType.QUEUE; import static org.apache.qpid.server.security.access.ObjectType.USER; import static org.apache.qpid.server.security.access.ObjectType.VIRTUALHOST; +import static org.apache.qpid.server.security.access.ObjectType.VIRTUALHOSTNODE; import static org.apache.qpid.server.security.access.Operation.*; import java.security.AccessControlException; @@ -242,9 +243,24 @@ public class SecurityManager implements ConfigurationChangeListener } } - public void authoriseCreateConnection(final AMQConnectionModel connection) + public void authoriseVirtualHostNode(final String virtualHostNodeName, final Operation operation) + { + if(!checkAllPlugins(new AccessCheck() + { + Result allowed(AccessControl plugin) + { + ObjectProperties properties = new ObjectProperties(virtualHostNodeName); + return plugin.authorise(operation, VIRTUALHOSTNODE, properties); + } + })) + { + throw new AccessControlException(operation + " permission denied for " + VIRTUALHOSTNODE + + " : " + virtualHostNodeName); + } + } + + public void authoriseVirtualHost(final String virtualHostName, final Operation operation) { - final String virtualHostName = connection.getVirtualHostName(); if(!checkAllPlugins(new AccessCheck() { Result allowed(AccessControl plugin) @@ -252,10 +268,24 @@ public class SecurityManager implements ConfigurationChangeListener // We put the name into the properties under both name and virtualhost_name so the user may express predicates using either. ObjectProperties properties = new ObjectProperties(virtualHostName); properties.put(Property.VIRTUALHOST_NAME, virtualHostName); - return plugin.authorise(Operation.ACCESS, VIRTUALHOST, properties); + return plugin.authorise(operation, VIRTUALHOST, properties); } })) { + throw new AccessControlException(operation + " permission denied for " + VIRTUALHOST + + " : " + virtualHostName); + } + } + + public void authoriseCreateConnection(final AMQConnectionModel connection) + { + String virtualHostName = connection.getVirtualHostName(); + try + { + authoriseVirtualHost(virtualHostName, Operation.ACCESS); + } + catch (AccessControlException ace) + { throw new AccessControlException("Permission denied: " + virtualHostName); } } diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/security/access/ObjectType.java b/java/broker-core/src/main/java/org/apache/qpid/server/security/access/ObjectType.java index 9016205d1c..88b1a6b727 100644 --- a/java/broker-core/src/main/java/org/apache/qpid/server/security/access/ObjectType.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/security/access/ObjectType.java @@ -42,7 +42,8 @@ import java.util.Set; public enum ObjectType { ALL(Operation.ALL), - VIRTUALHOST(Operation.ALL, ACCESS), + VIRTUALHOSTNODE(Operation.ALL, CREATE, DELETE, UPDATE), + VIRTUALHOST(Operation.ALL, ACCESS, CREATE, DELETE, UPDATE), MANAGEMENT(Operation.ALL, ACCESS), QUEUE(Operation.ALL, CREATE, DELETE, PURGE, CONSUME, UPDATE), EXCHANGE(Operation.ALL, ACCESS, CREATE, DELETE, BIND, UNBIND, PUBLISH, UPDATE), diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java b/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java index 3c63c5b869..fa6da37d45 100644 --- a/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java @@ -318,20 +318,18 @@ public abstract class AbstractVirtualHost<X extends AbstractVirtualHost<X>> exte { if(desiredState == State.DELETED) { - if (!_broker.getSecurityManager().authoriseConfiguringBroker(getName(), org.apache.qpid.server.model.VirtualHost.class, Operation.DELETE)) - { - throw new AccessControlException("Deletion of virtual host is denied"); - } + _broker.getSecurityManager().authoriseVirtualHost(getName(), Operation.DELETE); + } + else + { + _broker.getSecurityManager().authoriseVirtualHost(getName(), Operation.UPDATE); } } @Override protected void authoriseSetAttributes(ConfiguredObject<?> modified, Set<String> attributes) throws AccessControlException { - if (!_broker.getSecurityManager().authoriseConfiguringBroker(getName(), org.apache.qpid.server.model.VirtualHost.class, Operation.UPDATE)) - { - throw new AccessControlException("Setting of virtual host attributes is denied"); - } + _broker.getSecurityManager().authoriseVirtualHost(getName(), Operation.UPDATE); } public Collection<Connection> getConnections() diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java b/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java index 0cb69e4cd8..b21fcd704f 100644 --- a/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java @@ -197,20 +197,36 @@ public abstract class AbstractVirtualHostNode<X extends AbstractVirtualHostNode< { if(desiredState == State.DELETED) { - if (!_broker.getSecurityManager().authoriseConfiguringBroker(getName(), VirtualHostNode.class, Operation.DELETE)) - { - throw new AccessControlException("Deletion of virtual host node is denied"); - } + _broker.getSecurityManager().authoriseVirtualHostNode(getName(), Operation.DELETE); + } + else + { + _broker.getSecurityManager().authoriseVirtualHostNode(getName(), Operation.UPDATE); } } @Override - protected void authoriseSetAttributes(ConfiguredObject<?> modified, Set<String> attributes) throws AccessControlException + protected <C extends ConfiguredObject> void authoriseCreateChild(final Class<C> childClass, + final Map<String, Object> attributes, + final ConfiguredObject... otherParents) + throws AccessControlException { - if (!_broker.getSecurityManager().authoriseConfiguringBroker(getName(), VirtualHostNode.class, Operation.UPDATE)) + if (childClass == VirtualHost.class) { - throw new AccessControlException("Setting of virtual host node attributes is denied"); + _broker.getSecurityManager().authoriseVirtualHost(String.valueOf(attributes.get(VirtualHost.NAME)), + Operation.CREATE); + } + else + { + super.authoriseCreateChild(childClass, attributes, otherParents); + } + } + + @Override + protected void authoriseSetAttributes(ConfiguredObject<?> modified, Set<String> attributes) throws AccessControlException + { + _broker.getSecurityManager().authoriseVirtualHostNode(getName(), Operation.UPDATE); } private void closeConfigurationStore() diff --git a/java/broker-core/src/test/java/org/apache/qpid/server/model/VirtualHostTest.java b/java/broker-core/src/test/java/org/apache/qpid/server/model/VirtualHostTest.java index b69af13c13..d12f0bc5f9 100644 --- a/java/broker-core/src/test/java/org/apache/qpid/server/model/VirtualHostTest.java +++ b/java/broker-core/src/test/java/org/apache/qpid/server/model/VirtualHostTest.java @@ -22,12 +22,15 @@ package org.apache.qpid.server.model; import static org.mockito.Matchers.argThat; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import java.security.AccessControlException; import java.util.HashMap; import java.util.Map; import java.util.UUID; @@ -36,6 +39,8 @@ import org.mockito.ArgumentMatcher; import org.apache.qpid.server.configuration.updater.CurrentThreadTaskExecutor; import org.apache.qpid.server.configuration.updater.TaskExecutor; +import org.apache.qpid.server.security.SecurityManager; +import org.apache.qpid.server.security.access.Operation; import org.apache.qpid.server.store.ConfiguredObjectRecord; import org.apache.qpid.server.store.DurableConfigurationStore; import org.apache.qpid.server.util.BrokerTestHelper; @@ -44,6 +49,7 @@ import org.apache.qpid.test.utils.QpidTestCase; public class VirtualHostTest extends QpidTestCase { + private final SecurityManager _mockSecurityManager = mock(SecurityManager.class); private Broker _broker; private TaskExecutor _taskExecutor; private VirtualHostNode<?> _virtualHostNode; @@ -180,6 +186,82 @@ public class VirtualHostTest extends QpidTestCase verify(_configStore, never()).create(matchesRecord(queue.getId(), queue.getType())); } + // *************** VH Access Control Tests *************** + + public void testUpdateDeniedByACL() + { + when(_broker.getSecurityManager()).thenReturn(_mockSecurityManager); + + String virtualHostName = getName(); + VirtualHost<?,?,?> virtualHost = createVirtualHost(virtualHostName); + + doThrow(new AccessControlException("mocked ACL exception")).when(_mockSecurityManager).authoriseVirtualHost( + virtualHostName, + Operation.UPDATE); + + assertNull(virtualHost.getDescription()); + + try + { + virtualHost.setAttribute(VirtualHost.DESCRIPTION, null, "My description"); + fail("Exception not thrown"); + } + catch (AccessControlException ace) + { + // PASS + } + + verify(_configStore, never()).update(eq(false), matchesRecord(virtualHost.getId(), virtualHost.getType())); + } + + public void testStopDeniedByACL() + { + when(_broker.getSecurityManager()).thenReturn(_mockSecurityManager); + + String virtualHostName = getName(); + VirtualHost<?,?,?> virtualHost = createVirtualHost(virtualHostName); + + doThrow(new AccessControlException("mocked ACL exception")).when(_mockSecurityManager).authoriseVirtualHost( + virtualHostName, + Operation.UPDATE); + + try + { + virtualHost.stop(); + fail("Exception not thrown"); + } + catch (AccessControlException ace) + { + // PASS + } + + verify(_configStore, never()).update(eq(false), matchesRecord(virtualHost.getId(), virtualHost.getType())); + } + + public void testDeleteDeniedByACL() + { + when(_broker.getSecurityManager()).thenReturn(_mockSecurityManager); + + String virtualHostName = getName(); + VirtualHost<?,?,?> virtualHost = createVirtualHost(virtualHostName); + + doThrow(new AccessControlException("mocked ACL exception")).when(_mockSecurityManager).authoriseVirtualHost( + virtualHostName, + Operation.DELETE); + + try + { + virtualHost.delete(); + fail("Exception not thrown"); + } + catch (AccessControlException ace) + { + // PASS + } + + verify(_configStore, never()).remove(matchesRecord(virtualHost.getId(), virtualHost.getType())); + } + private VirtualHost<?,?,?> createVirtualHost(final String virtualHostName) { Map<String, Object> attributes = new HashMap<>(); diff --git a/java/broker-core/src/test/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNodeTest.java b/java/broker-core/src/test/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNodeTest.java index ea66911a89..d7c35ba6a8 100644 --- a/java/broker-core/src/test/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNodeTest.java +++ b/java/broker-core/src/test/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNodeTest.java @@ -21,9 +21,11 @@ package org.apache.qpid.server.virtualhostnode; import static org.apache.qpid.server.virtualhostnode.AbstractStandardVirtualHostNode.*; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.security.AccessControlException; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -39,6 +41,8 @@ import org.apache.qpid.server.model.State; import org.apache.qpid.server.model.SystemContext; import org.apache.qpid.server.model.VirtualHost; import org.apache.qpid.server.model.VirtualHostNode; +import org.apache.qpid.server.security.SecurityManager; +import org.apache.qpid.server.security.access.Operation; import org.apache.qpid.server.store.ConfiguredObjectRecord; import org.apache.qpid.server.store.DurableConfigurationStore; import org.apache.qpid.server.store.NullMessageStore; @@ -53,7 +57,8 @@ public class AbstractStandardVirtualHostNodeTest extends QpidTestCase private static final String TEST_VIRTUAL_HOST_NODE_NAME = "testNode"; private static final String TEST_VIRTUAL_HOST_NAME = "testVirtualHost"; - private UUID _nodeId = UUID.randomUUID(); + private final UUID _nodeId = UUID.randomUUID(); + private final SecurityManager _mockSecurityManager = mock(SecurityManager.class); private Broker<?> _broker; private TaskExecutor _taskExecutor; @@ -109,7 +114,6 @@ public class AbstractStandardVirtualHostNodeTest extends QpidTestCase assertEquals("Unexpected virtual host id", virtualHostId, virtualHost.getId()); } - /** * Tests activating a virtualhostnode with a config store which does not specify * a virtualhost. Checks no virtualhost is created. @@ -128,7 +132,6 @@ public class AbstractStandardVirtualHostNodeTest extends QpidTestCase VirtualHost<?, ?, ?> virtualHost = node.getVirtualHost(); assertNull("Virtual host should not be automatically created", virtualHost); - } /** @@ -233,6 +236,125 @@ public class AbstractStandardVirtualHostNodeTest extends QpidTestCase assertEquals("Unexpected virtual host id", virtualHostId, virtualHost.getId()); } + public void testStopStartVHN() throws Exception + { + DurableConfigurationStore configStore = configStoreThatProducesNoRecords(); + + Map<String, Object> nodeAttributes = new HashMap<>(); + nodeAttributes.put(VirtualHostNode.NAME, TEST_VIRTUAL_HOST_NODE_NAME); + nodeAttributes.put(VirtualHostNode.ID, _nodeId); + + VirtualHostNode<?> node = new TestVirtualHostNode(_broker, nodeAttributes, configStore); + node.open(); + node.start(); + + assertEquals("Unexpected virtual host node state", State.ACTIVE, node.getState()); + + node.stop(); + assertEquals("Unexpected virtual host node state after stop", State.STOPPED, node.getState()); + + node.start(); + assertEquals("Unexpected virtual host node state after start", State.ACTIVE, node.getState()); + } + + + // *************** VHN Access Control Tests *************** + + public void testUpdateVHNDeniedByACL() throws Exception + { + when(_broker.getSecurityManager()).thenReturn(_mockSecurityManager); + + DurableConfigurationStore configStore = configStoreThatProducesNoRecords(); + + Map<String, Object> nodeAttributes = new HashMap<>(); + nodeAttributes.put(VirtualHostNode.NAME, TEST_VIRTUAL_HOST_NODE_NAME); + nodeAttributes.put(VirtualHostNode.ID, _nodeId); + + VirtualHostNode<?> node = new TestVirtualHostNode(_broker, nodeAttributes, configStore); + node.open(); + node.start(); + + doThrow(new AccessControlException("mocked ACL exception")).when(_mockSecurityManager).authoriseVirtualHostNode( + TEST_VIRTUAL_HOST_NODE_NAME, + Operation.UPDATE); + + assertNull(node.getDescription()); + try + { + node.setAttribute(VirtualHostNode.DESCRIPTION, null, "My virtualhost node"); + fail("Exception not throws"); + } + catch (AccessControlException ace) + { + // PASS + } + assertNull("Description unexpected updated", node.getDescription()); + } + + public void testDeleteVHNDeniedByACL() throws Exception + { + SecurityManager mockSecurityManager = mock(SecurityManager.class); + when(_broker.getSecurityManager()).thenReturn(mockSecurityManager); + + DurableConfigurationStore configStore = configStoreThatProducesNoRecords(); + + Map<String, Object> nodeAttributes = new HashMap<>(); + nodeAttributes.put(VirtualHostNode.NAME, TEST_VIRTUAL_HOST_NODE_NAME); + nodeAttributes.put(VirtualHostNode.ID, _nodeId); + + VirtualHostNode<?> node = new TestVirtualHostNode(_broker, nodeAttributes, configStore); + node.open(); + node.start(); + + doThrow(new AccessControlException("mocked ACL exception")).when(mockSecurityManager).authoriseVirtualHostNode( + TEST_VIRTUAL_HOST_NODE_NAME, + Operation.DELETE); + + try + { + node.delete(); + fail("Exception not throws"); + } + catch (AccessControlException ace) + { + // PASS + } + + assertEquals("Virtual host node state changed unexpectedly", State.ACTIVE, node.getState()); + } + + public void testStopVHNDeniedByACL() throws Exception + { + SecurityManager mockSecurityManager = mock(SecurityManager.class); + when(_broker.getSecurityManager()).thenReturn(mockSecurityManager); + + DurableConfigurationStore configStore = configStoreThatProducesNoRecords(); + + Map<String, Object> nodeAttributes = new HashMap<>(); + nodeAttributes.put(VirtualHostNode.NAME, TEST_VIRTUAL_HOST_NODE_NAME); + nodeAttributes.put(VirtualHostNode.ID, _nodeId); + + VirtualHostNode<?> node = new TestVirtualHostNode(_broker, nodeAttributes, configStore); + node.open(); + node.start(); + + doThrow(new AccessControlException("mocked ACL exception")).when(mockSecurityManager).authoriseVirtualHostNode( + TEST_VIRTUAL_HOST_NODE_NAME, + Operation.UPDATE); + + try + { + node.stop(); + fail("Exception not throws"); + } + catch (AccessControlException ace) + { + // PASS + } + + assertEquals("Virtual host node state changed unexpectedly", State.ACTIVE, node.getState()); + } + private ConfiguredObjectRecord createVirtualHostConfiguredObjectRecord(UUID virtualHostId) { Map<String, Object> virtualHostAttributes = new HashMap<>(); @@ -263,6 +385,7 @@ public class AbstractStandardVirtualHostNodeTest extends QpidTestCase } }; } + private NullMessageStore configStoreThatProducesNoRecords() { return configStoreThatProduces(null); |