QPID-4813: Protect operations to update queue and exchange attributes with ACL

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1479200 13f79535-47bb-0310-9956-ffa450edef68

5 files changed, 138 insertions, 4 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BindingAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BindingAdapter.java
index eb2d0dd7e2..39e979174a 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BindingAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BindingAdapter.java
@@ -222,4 +222,19 @@ final class BindingAdapter extends AbstractAdapter implements Binding
         }
         return false;
     }
+
+    @Override
+    public Object setAttribute(final String name, final Object expected, final Object desired) throws IllegalStateException,
+            AccessControlException, IllegalArgumentException
+    {
+        throw new UnsupportedOperationException("Changing attributes on binding is not supported.");
+    }
+
+    @Override
+    public void setAttributes(final Map<String, Object> attributes) throws IllegalStateException, AccessControlException,
+            IllegalArgumentException
+    {
+        throw new UnsupportedOperationException("Changing attributes on binding is not supported.");
+    }
+
 }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/ExchangeAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/ExchangeAdapter.java
index 5d5f3f7378..a081f03f09 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/ExchangeAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/ExchangeAdapter.java
@@ -384,6 +384,30 @@ final class ExchangeAdapter extends AbstractAdapter implements Exchange, org.apa
         return false;
     }
 
+    @Override
+    protected void changeAttributes(Map<String, Object> attributes)
+    {
+        throw new UnsupportedOperationException("Changing attributes on exchange is not supported.");
+    }
+
+    @Override
+    protected void authoriseSetAttribute(String name, Object expected, Object desired) throws AccessControlException
+    {
+        if (!_vhost.getSecurityManager().authoriseUpdate(_exchange))
+        {
+            throw new AccessControlException("Setting of exchange attribute is denied");
+        }
+    }
+
+    @Override
+    protected void authoriseSetAttributes(Map<String, Object> attributes) throws AccessControlException
+    {
+        if (!_vhost.getSecurityManager().authoriseUpdate(_exchange))
+        {
+            throw new AccessControlException("Setting of exchange attributes is denied");
+        }
+    }
+
     private class ExchangeStatistics implements Statistics
     {
 
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/QueueAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/QueueAdapter.java
index 8ac869900c..0916c4e730 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/QueueAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/QueueAdapter.java
@@ -20,6 +20,7 @@
  */
 package org.apache.qpid.server.model.adapter;
 
+import java.lang.reflect.Type;
 import java.security.AccessControlException;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -31,16 +32,15 @@ import java.util.Map;
 import org.apache.qpid.AMQException;
 import org.apache.qpid.AMQStoreException;
 import org.apache.qpid.server.binding.Binding;
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.ConfiguredObjectFinder;
 import org.apache.qpid.server.model.Consumer;
-import org.apache.qpid.server.model.Connection;
 import org.apache.qpid.server.model.Exchange;
 import org.apache.qpid.server.model.IllegalStateTransitionException;
 import org.apache.qpid.server.model.LifetimePolicy;
 import org.apache.qpid.server.model.Queue;
 import org.apache.qpid.server.model.QueueNotificationListener;
-import org.apache.qpid.server.model.Session;
 import org.apache.qpid.server.model.State;
 import org.apache.qpid.server.model.Statistics;
 import org.apache.qpid.server.protocol.AMQConnectionModel;
@@ -51,6 +51,19 @@ import org.apache.qpid.server.util.MapValueConverter;
 
 final class QueueAdapter extends AbstractAdapter implements Queue, AMQQueue.SubscriptionRegistrationListener, AMQQueue.NotificationListener
 {
+    @SuppressWarnings("serial")
+    static final Map<String, Type> ATTRIBUTE_TYPES = Collections.unmodifiableMap(new HashMap<String, Type>(){{
+        put(ALERT_REPEAT_GAP, Long.class);
+        put(ALERT_THRESHOLD_MESSAGE_AGE, Long.class);
+        put(ALERT_THRESHOLD_MESSAGE_SIZE, Long.class);
+        put(ALERT_THRESHOLD_QUEUE_DEPTH_MESSAGES, Long.class);
+        put(ALERT_THRESHOLD_QUEUE_DEPTH_BYTES, Long.class);
+        put(QUEUE_FLOW_CONTROL_SIZE_BYTES, Long.class);
+        put(QUEUE_FLOW_RESUME_SIZE_BYTES, Long.class);
+        put(MAXIMUM_DELIVERY_ATTEMPTS, Integer.class);
+        put(EXCLUSIVE, Boolean.class);
+        put(DESCRIPTION, String.class);
+    }});
 
     static final Map<String, String> ATTRIBUTE_MAPPINGS = new HashMap<String, String>();
     static
@@ -775,4 +788,61 @@ final class QueueAdapter extends AbstractAdapter implements Queue, AMQQueue.Subs
         return false;
     }
 
+    @Override
+    protected void authoriseSetAttribute(String name, Object expected, Object desired) throws AccessControlException
+    {
+        if (!_vhost.getSecurityManager().authoriseUpdate(_queue))
+        {
+            throw new AccessControlException("Setting of queue attribute is denied");
+        }
+    }
+
+    @Override
+    protected void authoriseSetAttributes(Map<String, Object> attributes) throws AccessControlException
+    {
+        if (!_vhost.getSecurityManager().authoriseUpdate(_queue))
+        {
+            throw new AccessControlException("Setting of queue attributes is denied");
+        }
+    }
+
+    @Override
+    protected void changeAttributes(final Map<String, Object> attributes)
+    {
+        Map<String, Object> convertedAttributes = MapValueConverter.convert(attributes, ATTRIBUTE_TYPES);
+        validateAttributes(convertedAttributes);
+
+        super.changeAttributes(convertedAttributes);
+    }
+
+    private void validateAttributes(Map<String, Object> convertedAttributes)
+    {
+        Long queueFlowControlSize = (Long) convertedAttributes.get(QUEUE_FLOW_CONTROL_SIZE_BYTES);
+        Long queueFlowControlResumeSize = (Long) convertedAttributes.get(QUEUE_FLOW_RESUME_SIZE_BYTES);
+        if (queueFlowControlSize != null || queueFlowControlResumeSize != null )
+        {
+            if (queueFlowControlSize == null)
+            {
+                queueFlowControlSize = (Long)getAttribute(QUEUE_FLOW_CONTROL_SIZE_BYTES);
+            }
+            if (queueFlowControlResumeSize == null)
+            {
+                queueFlowControlResumeSize = (Long)getAttribute(QUEUE_FLOW_RESUME_SIZE_BYTES);
+            }
+            if (queueFlowControlResumeSize > queueFlowControlSize)
+            {
+                throw new IllegalConfigurationException("Flow resume size can't be greater than flow control size");
+            }
+        }
+        for (Map.Entry<String, Object> entry: convertedAttributes.entrySet())
+        {
+            Object value = entry.getValue();
+            if (value instanceof Number && ((Number)value).longValue() < 0)
+            {
+                throw new IllegalConfigurationException("Only positive integer value can be specified for the attribute "
+                        + entry.getKey());
+            }
+        }
+    }
+
 }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/SecurityManager.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/SecurityManager.java
index f0570943cf..09e79d3ae9 100755
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/SecurityManager.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/SecurityManager.java
@@ -52,6 +52,7 @@ import static org.apache.qpid.server.security.access.Operation.DELETE;
 import static org.apache.qpid.server.security.access.Operation.PUBLISH;
 import static org.apache.qpid.server.security.access.Operation.PURGE;
 import static org.apache.qpid.server.security.access.Operation.UNBIND;
+import static org.apache.qpid.server.security.access.Operation.UPDATE;
 
 import javax.security.auth.Subject;
 import java.net.SocketAddress;
@@ -386,6 +387,30 @@ public class SecurityManager implements ConfigurationChangeListener
         });
     }
 
+
+    public boolean authoriseUpdate(final AMQQueue queue)
+    {
+        return checkAllPlugins(new AccessCheck()
+        {
+            Result allowed(AccessControl plugin)
+            {
+                return plugin.authorise(UPDATE, QUEUE, new ObjectProperties(queue));
+            }
+        });
+    }
+
+
+    public boolean authoriseUpdate(final Exchange exchange)
+    {
+        return checkAllPlugins(new AccessCheck()
+        {
+            Result allowed(AccessControl plugin)
+            {
+                return plugin.authorise(UPDATE, EXCHANGE, new ObjectProperties(exchange.getName()));
+            }
+        });
+    }
+
     public boolean authoriseDelete(final Exchange exchange)
     {
         return checkAllPlugins(new AccessCheck()
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/ObjectType.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/ObjectType.java
index e93f487bdb..048d9a8fc9 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/ObjectType.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/access/ObjectType.java
@@ -43,8 +43,8 @@ public enum ObjectType
     ALL(Operation.ALL),
     VIRTUALHOST(Operation.ALL, ACCESS),
     MANAGEMENT(Operation.ALL, ACCESS),
-    QUEUE(Operation.ALL, CREATE, DELETE, PURGE, CONSUME),
-    EXCHANGE(Operation.ALL, ACCESS, CREATE, DELETE, BIND, UNBIND, PUBLISH),
+    QUEUE(Operation.ALL, CREATE, DELETE, PURGE, CONSUME, UPDATE),
+    EXCHANGE(Operation.ALL, ACCESS, CREATE, DELETE, BIND, UNBIND, PUBLISH, UPDATE),
     LINK, // Not allowed in the Java broker
     ROUTE, // Not allowed in the Java broker
     METHOD(Operation.ALL, ACCESS, UPDATE),