QPID-4334: removed the firewall plugin and moved its functionality into the Access Control plugin.

Applied patch from Philip Harvey <phil@philharveyonline.com>.

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

30 files changed, 1464 insertions, 1185 deletions

diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AbstractConfiguration.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AbstractConfiguration.java
index f04dd38aca..44c48523e2 100644
--- a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AbstractConfiguration.java
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AbstractConfiguration.java
@@ -23,12 +23,9 @@ package org.apache.qpid.server.security.access.config;
 import java.io.File;
 
 import org.apache.commons.configuration.ConfigurationException;
-import org.apache.log4j.Logger;
 
 public abstract class AbstractConfiguration implements ConfigurationFile
 {
-    private static final Logger _logger = Logger.getLogger(ConfigurationFile.class);
-    
     private File _file;
     private RuleSet _config;
     
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclAction.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclAction.java
new file mode 100644
index 0000000000..e4bf21a082
--- /dev/null
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclAction.java
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.access.config;
+
+import org.apache.commons.lang.builder.EqualsBuilder;
+import org.apache.commons.lang.builder.HashCodeBuilder;
+import org.apache.commons.lang.builder.ToStringBuilder;
+import org.apache.commons.lang.builder.ToStringStyle;
+import org.apache.qpid.server.security.access.ObjectProperties;
+import org.apache.qpid.server.security.access.ObjectType;
+import org.apache.qpid.server.security.access.Operation;
+import org.apache.qpid.server.security.access.firewall.FirewallRule;
+
+public class AclAction
+{
+    private Action _action;
+    private FirewallRule _firewallRule;
+
+    public AclAction(Operation operation, ObjectType object, AclRulePredicates predicates)
+    {
+        _action = new Action(operation, object, predicates.getObjectProperties());
+        _firewallRule = predicates.getFirewallRule();
+    }
+
+    public AclAction(Operation operation)
+    {
+        _action = new Action(operation);
+    }
+
+    public AclAction(Operation operation, ObjectType object, ObjectProperties properties)
+    {
+        _action = new Action(operation, object, properties);
+    }
+
+    public FirewallRule getFirewallRule()
+    {
+        return _firewallRule;
+    }
+
+    public Action getAction()
+    {
+        return _action;
+    }
+
+    public boolean isAllowed()
+    {
+        return _action.isAllowed();
+    }
+
+    @Override
+    public int hashCode()
+    {
+        return new HashCodeBuilder()
+            .append(_action)
+            .append(_firewallRule).toHashCode();
+    }
+
+    @Override
+    public boolean equals(Object obj)
+    {
+        if (obj == null)
+        {
+            return false;
+        }
+        if (obj == this)
+        {
+            return true;
+        }
+        if (obj.getClass() != getClass())
+        {
+            return false;
+        }
+        AclAction rhs = (AclAction) obj;
+        return new EqualsBuilder()
+            .append(_action, rhs._action)
+            .append(_firewallRule, rhs._firewallRule).isEquals();
+    }
+
+    @Override
+    public String toString()
+    {
+        return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
+            .append(_action)
+            .append(_firewallRule).toString();
+    }
+}
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclRulePredicates.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclRulePredicates.java
new file mode 100644
index 0000000000..0ea6b2fc9c
--- /dev/null
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclRulePredicates.java
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.access.config;
+
+import org.apache.commons.lang.builder.ToStringBuilder;
+import org.apache.commons.lang.builder.ToStringStyle;
+import org.apache.log4j.Logger;
+import org.apache.qpid.server.security.access.ObjectProperties;
+import org.apache.qpid.server.security.access.ObjectProperties.Property;
+import org.apache.qpid.server.security.access.firewall.FirewallRule;
+import org.apache.qpid.server.security.access.firewall.FirewallRuleFactory;
+
+/**
+ * Represents the predicates on an ACL rule by combining predicates relating to the object being operated on
+ * (e.g. name=foo) with firewall rules.
+ */
+public class AclRulePredicates
+{
+    private static final Logger _logger = Logger.getLogger(AclRulePredicates.class);
+
+    private static final String SEPARATOR = ",";
+
+    private ObjectProperties _properties = new ObjectProperties();
+
+    private FirewallRule _firewallRule;
+
+    private FirewallRuleFactory _firewallRuleFactory = new FirewallRuleFactory();
+
+    public void parse(String key, String value)
+    {
+        ObjectProperties.Property property = ObjectProperties.Property.parse(key);
+
+        if(property == Property.FROM_HOSTNAME)
+        {
+            checkFirewallRuleNotAlreadyDefined(key, value);
+            _firewallRule = _firewallRuleFactory.createForHostname(value.split(SEPARATOR));
+        }
+        else if(property == Property.FROM_NETWORK)
+        {
+            checkFirewallRuleNotAlreadyDefined(key, value);
+            _firewallRule = _firewallRuleFactory.createForNetwork(value.split(SEPARATOR));
+        }
+        else
+        {
+            _properties.put(property, value);
+        }
+
+        _logger.debug("Parsed " + property + " with value " + value);
+    }
+
+    private void checkFirewallRuleNotAlreadyDefined(String key, String value)
+    {
+        if(_firewallRule != null)
+        {
+            throw new IllegalStateException(
+                    "Cannot parse " + key + "=" + value
+                    + " because firewall rule " + _firewallRule + " has already been defined");
+        }
+    }
+
+    @Override
+    public String toString()
+    {
+        return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
+            .append(_properties)
+            .append(_firewallRule).toString();
+    }
+
+    public FirewallRule getFirewallRule()
+    {
+        return _firewallRule;
+    }
+
+    public ObjectProperties getObjectProperties()
+    {
+        return _properties;
+    }
+
+    void setFirewallRuleFactory(FirewallRuleFactory firewallRuleFactory)
+    {
+        _firewallRuleFactory = firewallRuleFactory;
+    }
+}
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Action.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Action.java
index b887d1e079..d244af480a 100644
--- a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Action.java
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Action.java
@@ -20,8 +20,6 @@
  */
 package org.apache.qpid.server.security.access.config;
 
-import java.util.Comparator;
-
 import org.apache.commons.lang.builder.EqualsBuilder;
 import org.apache.commons.lang.builder.HashCodeBuilder;
 import org.apache.commons.lang.builder.ToStringBuilder;
@@ -32,7 +30,7 @@ import org.apache.qpid.server.security.access.Operation;
 
 /**
  * An access control v2 rule action.
- * 
+ *
  * An action consists of an {@link Operation} on an {@link ObjectType} with certain properties, stored in a {@link java.util.Map}.
  * The operation and object should be an allowable combination, based on the {@link ObjectType#isAllowed(Operation)}
  * method of the object, which is exposed as the {@link #isAllowed()} method here. The internal {@link #propertiesMatch(Map)}
@@ -48,29 +46,29 @@ public class Action
     private Operation _operation;
     private ObjectType _object;
     private ObjectProperties _properties;
-    
+
     public Action(Operation operation)
     {
         this(operation, ObjectType.ALL);
     }
-    
+
     public Action(Operation operation, ObjectType object, String name)
     {
         this(operation, object, new ObjectProperties(name));
     }
-    
+
     public Action(Operation operation, ObjectType object)
     {
         this(operation, object, ObjectProperties.EMPTY);
     }
-    
+
     public Action(Operation operation, ObjectType object, ObjectProperties properties)
     {
         setOperation(operation);
         setObjectType(object);
         setProperties(properties);
     }
-    
+
     public Operation getOperation()
     {
         return _operation;
@@ -95,12 +93,12 @@ public class Action
     {
         return _properties;
     }
-    
+
     public void setProperties(ObjectProperties properties)
     {
         _properties = properties;
     }
-    
+
     public boolean isAllowed()
     {
         return _object.isAllowed(_operation);
@@ -109,40 +107,13 @@ public class Action
     /** @see Comparable#compareTo(Object) */
     public boolean matches(Action a)
     {
-        return ((Operation.ALL == a.getOperation() || getOperation() == a.getOperation())
-                    && (ObjectType.ALL == a.getObjectType() || getObjectType() == a.getObjectType())
-                    && _properties.matches(a.getProperties()));
-    }
+        boolean operationMatches = Operation.ALL == a.getOperation() || getOperation() == a.getOperation();
+        boolean objectTypeMatches = ObjectType.ALL == a.getObjectType() || getObjectType() == a.getObjectType();
+        boolean propertiesMatch = _properties.matches(a.getProperties());
 
-    /**
-     * An ordering based on specificity
-     * 
-     * @see Comparator#compare(Object, Object)
-     */
-    public class Specificity implements Comparator<Action>
-    {
-        public int compare(Action a, Action b)
-        {
-            if (a.getOperation() == Operation.ALL && b.getOperation() != Operation.ALL)
-            {
-                return 1; // B is more specific
-            }
-            else if (b.getOperation() == Operation.ALL && a.getOperation() != Operation.ALL)
-            {
-                return 1; // A is more specific
-            }
-            else if (a.getOperation() == b.getOperation())
-            {
-                return 1; // b is more specific
-            }
-            else // Different operations
-            {
-                return a.getOperation().compareTo(b.getOperation()); // Arbitrary
-            }
-        }
+        return (operationMatches && objectTypeMatches && propertiesMatch);
     }
 
-    /** @see Object#equals(Object) */
     @Override
     public boolean equals(Object o)
     {
@@ -151,26 +122,24 @@ public class Action
             return false;
         }
         Action a = (Action) o;
-        
+
         return new EqualsBuilder()
                 .append(_operation, a.getOperation())
                 .append(_object, a.getObjectType())
-                .appendSuper(_properties.equals(a.getProperties()))
+                .append(_properties, a.getProperties())
                 .isEquals();
     }
 
-    /** @see Object#hashCode() */
     @Override
     public int hashCode()
     {
         return new HashCodeBuilder()
                 .append(_operation)
-                .append(_operation)
+                .append(_object)
                 .append(_properties)
                 .toHashCode();
     }
 
-    /** @see Object#toString() */
     @Override
     public String toString()
     {
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/ClientAction.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/ClientAction.java
new file mode 100644
index 0000000000..fed20a56c8
--- /dev/null
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/ClientAction.java
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.access.config;
+
+import java.net.InetAddress;
+
+import org.apache.commons.lang.builder.ToStringBuilder;
+import org.apache.commons.lang.builder.ToStringStyle;
+import org.apache.qpid.server.security.access.ObjectProperties;
+import org.apache.qpid.server.security.access.ObjectType;
+import org.apache.qpid.server.security.access.Operation;
+import org.apache.qpid.server.security.access.firewall.FirewallRule;
+
+/**
+ * I represent an {@link Action} taken by a client from a known address. The address is used to
+ * determine if I match an {@link AclAction}, which may contain firewall rules.
+ */
+public class ClientAction
+{
+    private Action _clientAction;
+
+    public ClientAction(Action clientAction)
+    {
+        _clientAction = clientAction;
+    }
+
+    public ClientAction(Operation operation, ObjectType objectType, ObjectProperties properties)
+    {
+        _clientAction = new Action(operation, objectType, properties);
+    }
+
+    public boolean matches(AclAction ruleAction, InetAddress addressOfClient)
+    {
+        return _clientAction.matches(ruleAction.getAction())
+                && addressOfClientMatches(ruleAction, addressOfClient);
+    }
+
+    private boolean addressOfClientMatches(AclAction ruleAction, InetAddress addressOfClient)
+    {
+        FirewallRule firewallRule = ruleAction.getFirewallRule();
+        if(firewallRule == null || addressOfClient == null)
+        {
+            return true;
+        }
+        else
+        {
+            return firewallRule.matches(addressOfClient);
+        }
+    }
+
+    public Operation getOperation()
+    {
+        return _clientAction.getOperation();
+    }
+
+    public ObjectType getObjectType()
+    {
+        return _clientAction.getObjectType();
+    }
+
+    public ObjectProperties getProperties()
+    {
+        return _clientAction.getProperties();
+    }
+
+    @Override
+    public String toString()
+    {
+        return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
+            .append(_clientAction).toString();
+    }
+}
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/PlainConfiguration.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/PlainConfiguration.java
index afaece6138..9f56b05e0f 100644
--- a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/PlainConfiguration.java
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/PlainConfiguration.java
@@ -34,13 +34,15 @@ import java.util.Stack;
 
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.commons.lang.StringUtils;
-import org.apache.qpid.server.security.access.ObjectProperties;
+import org.apache.log4j.Logger;
 import org.apache.qpid.server.security.access.ObjectType;
 import org.apache.qpid.server.security.access.Operation;
 import org.apache.qpid.server.security.access.Permission;
 
 public class PlainConfiguration extends AbstractConfiguration
 {
+    private static final Logger _logger = Logger.getLogger(PlainConfiguration.class);
+
     public static final Character COMMENT = '#';
     public static final Character CONTINUATION = '\\';
 
@@ -74,9 +76,16 @@ public class PlainConfiguration extends AbstractConfiguration
     {
         RuleSet ruleSet = super.load();
 
+        File file = getFile();
+
         try
         {
-            _st = new StreamTokenizer(new BufferedReader(new FileReader(getFile())));
+            if(_logger.isDebugEnabled())
+            {
+                _logger.debug("About to load ACL file " + file);
+            }
+
+            _st = new StreamTokenizer(new BufferedReader(new FileReader(file)));
             _st.resetSyntax(); // setup the tokenizer
 
             _st.commentChar(COMMENT); // single line comments
@@ -195,11 +204,11 @@ public class PlainConfiguration extends AbstractConfiguration
         }
         catch (FileNotFoundException fnfe)
         {
-            throw new ConfigurationException(String.format(CONFIG_NOT_FOUND_MSG, getFile().getName()), fnfe);
+            throw new ConfigurationException(String.format(CONFIG_NOT_FOUND_MSG, file.getName()), fnfe);
         }
         catch (IOException ioe)
         {
-            throw new ConfigurationException(String.format(CANNOT_LOAD_MSG, getFile().getName()), ioe);
+            throw new ConfigurationException(String.format(CANNOT_LOAD_MSG, file.getName()), ioe);
         }
 
         return ruleSet;
@@ -228,9 +237,9 @@ public class PlainConfiguration extends AbstractConfiguration
         else
         {
             ObjectType object = ObjectType.parse(args.get(3));
-            ObjectProperties properties = toObjectProperties(args.subList(4, args.size()));
+            AclRulePredicates predicates = toRulePredicates(args.subList(4, args.size()));
 
-            getConfiguration().grant(number, identity, permission, operation, object, properties);
+            getConfiguration().grant(number, identity, permission, operation, object, predicates);
         }
     }
 
@@ -246,10 +255,9 @@ public class PlainConfiguration extends AbstractConfiguration
         getConfiguration().configure(properties);
     }
 
-    /** Converts a {@link List} of "name", "=", "value" tokens into a {@link Map}. */
-    protected ObjectProperties toObjectProperties(List<String> args) throws ConfigurationException
+    private AclRulePredicates toRulePredicates(List<String> args) throws ConfigurationException
     {
-        ObjectProperties properties = new ObjectProperties();
+        AclRulePredicates predicates = new AclRulePredicates();
         Iterator<String> i = args.iterator();
         while (i.hasNext())
         {
@@ -268,11 +276,9 @@ public class PlainConfiguration extends AbstractConfiguration
             }
             String value = i.next();
 
-            // parse property key
-            ObjectProperties.Property property = ObjectProperties.Property.parse(key);
-            properties.put(property, value);
+            predicates.parse(key, value);
         }
-        return properties;
+        return predicates;
     }
 
     /** Converts a {@link List} of "name", "=", "value" tokens into a {@link Map}. */
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Rule.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Rule.java
index 5e98e0bd1b..cef9a8696b 100644
--- a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Rule.java
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Rule.java
@@ -41,10 +41,10 @@ public class Rule implements Comparable<Rule>
     private Integer _number;
     private Boolean _enabled = Boolean.TRUE;
     private String _identity;
-    private Action _action;
+    private AclAction _action;
     private Permission _permission;
 
-    public Rule(Integer number, String identity, Action action, Permission permission)
+    public Rule(Integer number, String identity, AclAction action, Permission permission)
     {
         setNumber(number);
         setIdentity(identity);
@@ -52,7 +52,7 @@ public class Rule implements Comparable<Rule>
         setPermission(permission);
     }
 
-    public Rule(String identity, Action action, Permission permission)
+    public Rule(String identity, AclAction action, Permission permission)
     {
         this(null, identity, action, permission);
     }
@@ -99,10 +99,15 @@ public class Rule implements Comparable<Rule>
 
     public Action getAction()
     {
+        return _action.getAction();
+    }
+
+    public AclAction getAclAction()
+    {
         return _action;
     }
 
-    public void setAction(Action action)
+    public void setAction(AclAction action)
     {
         _action = action;
     }
@@ -117,7 +122,7 @@ public class Rule implements Comparable<Rule>
         _permission = permission;
     }
 
-    /** @see Comparable#compareTo(Object) */
+    @Override
     public int compareTo(Rule r)
     {
         return new CompareToBuilder()
@@ -127,7 +132,6 @@ public class Rule implements Comparable<Rule>
                 .toComparison();
     }
 
-    /** @see Object#equals(Object) */
     @Override
     public boolean equals(Object o)
     {
@@ -139,30 +143,28 @@ public class Rule implements Comparable<Rule>
 
         return new EqualsBuilder()
                 .append(getIdentity(), r.getIdentity())
-                .append(getAction(), r.getAction())
+                .append(getAclAction(), r.getAclAction())
                 .append(getPermission(), r.getPermission())
                 .isEquals();
     }
 
-    /** @see Object#hashCode() */
     @Override
     public int hashCode()
     {
         return new HashCodeBuilder()
                 .append(getIdentity())
-                .append(getAction())
+                .append(getAclAction())
                 .append(getPermission())
                 .toHashCode();
     }
 
-    /** @see Object#toString() */
     @Override
     public String toString()
     {
         return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
                 .append("#", getNumber())
                 .append("identity", getIdentity())
-                .append("action", getAction())
+                .append("action", getAclAction())
                 .append("permission", getPermission())
                 .append("enabled", isEnabled())
                 .toString();
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSet.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSet.java
index 2477455de4..e61370fced 100644
--- a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSet.java
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSet.java
@@ -18,6 +18,7 @@
  */
 package org.apache.qpid.server.security.access.config;
 
+import java.net.InetAddress;
 import java.security.Principal;
 import java.util.Arrays;
 import java.util.Collections;
@@ -53,7 +54,7 @@ import org.apache.qpid.server.security.access.logging.AccessControlMessages;
  */
 public class RuleSet
 {
-    public static final Logger _logger = Logger.getLogger(RuleSet.class);
+    private static final Logger _logger = Logger.getLogger(RuleSet.class);
 
     private static final String AT = "@";
     private static final String SLASH = "/";
@@ -154,21 +155,27 @@ public class RuleSet
 
     public void grant(Integer number, String identity, Permission permission, Operation operation)
     {
-        Action action = new Action(operation);
+        AclAction action = new AclAction(operation);
         addRule(number, identity, permission, action);
     }
 
     public void grant(Integer number, String identity, Permission permission, Operation operation, ObjectType object, ObjectProperties properties)
     {
-        Action action = new Action(operation, object, properties);
+        AclAction action = new AclAction(operation, object, properties);
         addRule(number, identity, permission, action);
     }
 
-    public boolean ruleExists(String identity, Action action)
+    public void grant(Integer number, String identity, Permission permission, Operation operation, ObjectType object, AclRulePredicates predicates)
+    {
+        AclAction aclAction = new AclAction(operation, object, predicates);
+        addRule(number, identity, permission, aclAction);
+    }
+
+    public boolean ruleExists(String identity, AclAction action)
     {
         for (Rule rule : _rules.values())
         {
-            if (rule.getIdentity().equals(identity) && rule.getAction().equals(action))
+            if (rule.getIdentity().equals(identity) && rule.getAclAction().equals(action))
             {
                 return true;
             }
@@ -176,8 +183,7 @@ public class RuleSet
         return false;
     }
 
-    // TODO make this work when group membership is not known at file parse time
-    public void addRule(Integer number, String identity, Permission permission, Action action)
+    public void addRule(Integer number, String identity, Permission permission, AclAction action)
     {
         _cache.clear();
 
@@ -263,6 +269,16 @@ public class RuleSet
     }
 
     /**
+     * Checks for the case when the client's address is not known.
+     *
+     * @see #check(Subject, Operation, ObjectType, ObjectProperties, InetAddress)
+     */
+    public Result check(Subject subject, Operation operation, ObjectType objectType, ObjectProperties properties)
+    {
+        return check(subject, operation, objectType, properties, null);
+    }
+
+    /**
      * Check the authorisation granted to a particular identity for an operation on an object type with
      * specific properties.
      *
@@ -271,10 +287,9 @@ public class RuleSet
      * the first match found, or denies access if there are no matching rules. Normally, it would be expected
      * to have a default deny or allow rule at the end of an access configuration however.
      */
-    public Result check(Subject subject, Operation operation, ObjectType objectType, ObjectProperties properties)
+    public Result check(Subject subject, Operation operation, ObjectType objectType, ObjectProperties properties, InetAddress addressOfClient)
     {
-        // Create the action to check
-        Action action = new Action(operation, objectType, properties);
+        ClientAction action = new ClientAction(operation, objectType, properties);
 
         if(_logger.isDebugEnabled())
         {
@@ -293,27 +308,31 @@ public class RuleSet
         }
 
         // Iterate through a filtered set of rules dealing with this identity and operation
-        for (Rule current : rules)
+        for (Rule rule : rules)
         {
             if(_logger.isDebugEnabled())
             {
-                _logger.debug("Checking against rule: " + current);
+                _logger.debug("Checking against rule: " + rule);
             }
-            // Check if action matches
-            if (action.matches(current.getAction()))
+
+            if (action.matches(rule.getAclAction(), addressOfClient))
             {
-                Permission permission = current.getPermission();
+                Permission permission = rule.getPermission();
 
                 switch (permission)
                 {
                     case ALLOW_LOG:
                         CurrentActor.get().message(AccessControlMessages.ALLOWED(
-                                action.getOperation().toString(), action.getObjectType().toString(), action.getProperties().toString()));
+                                action.getOperation().toString(),
+                                action.getObjectType().toString(),
+                                action.getProperties().toString()));
                     case ALLOW:
                         return Result.ALLOWED;
                     case DENY_LOG:
                         CurrentActor.get().message(AccessControlMessages.DENIED(
-                                action.getOperation().toString(), action.getObjectType().toString(), action.getProperties().toString()));
+                                action.getOperation().toString(),
+                                action.getObjectType().toString(),
+                                action.getProperties().toString()));
                     case DENY:
                         return Result.DENIED;
                 }
@@ -419,5 +438,4 @@ public class RuleSet
         }
         return objects;
     }
-
 }
diff --git a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/config/FirewallException.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/AccessControlFirewallException.java
index a9e3fdc242..efae7f5653 100644
--- a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/config/FirewallException.java
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/AccessControlFirewallException.java
@@ -18,29 +18,26 @@
  * under the License.
  *
  */
-package org.apache.qpid.server.security.access.config;
+package org.apache.qpid.server.security.access.firewall;
 
-/**
- * Firewall plugin exception.
- */
-public class FirewallException extends Exception
+public class AccessControlFirewallException extends RuntimeException
 {
     /** serialVersionUID */
     private static final long serialVersionUID = 4526157149690917805L;
-    
-    public FirewallException() {
+
+    public AccessControlFirewallException() {
 	    super();
     }
 
-    public FirewallException(String message) {
+    public AccessControlFirewallException(String message) {
 	    super(message);
     }
 
-    public FirewallException(String message, Throwable cause) {
+    public AccessControlFirewallException(String message, Throwable cause) {
         super(message, cause);
     }
 
-    public FirewallException(Throwable cause) {
+    public AccessControlFirewallException(Throwable cause) {
         super(cause);
     }
 }
\ No newline at end of file
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/FirewallRule.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/FirewallRule.java
new file mode 100644
index 0000000000..482a795693
--- /dev/null
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/FirewallRule.java
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.access.firewall;
+
+import java.net.InetAddress;
+
+public interface FirewallRule
+{
+    boolean matches(InetAddress addressOfClient);
+}
diff --git a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/FirewallActivator.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/FirewallRuleFactory.java
index 1669352085..64be26c209 100644
--- a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/FirewallActivator.java
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/FirewallRuleFactory.java
@@ -1,5 +1,4 @@
 /*
- * 
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
  * distributed with this work for additional information
@@ -7,35 +6,28 @@
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
  * KIND, either express or implied.  See the License for the
  * specific language governing permissions and limitations
  * under the License.
- * 
  */
-package org.apache.qpid.server.security.access.plugins;
+package org.apache.qpid.server.security.access.firewall;
 
-import org.apache.qpid.server.configuration.plugins.ConfigurationPluginFactory;
-import org.apache.qpid.server.security.SecurityPluginActivator;
-import org.apache.qpid.server.security.SecurityPluginFactory;
-
-/**
- * The OSGi {@link org.osgi.framework.BundleActivator} for {@link Firewall}.
- */
-public class FirewallActivator extends SecurityPluginActivator
+public class FirewallRuleFactory
 {
-    public SecurityPluginFactory getFactory()
+    public FirewallRule createForHostname(String[] hostnames)
     {
-        return Firewall.FACTORY;
+        return new HostnameFirewallRule(hostnames);
     }
 
-    public ConfigurationPluginFactory getConfigurationFactory()
+    public FirewallRule createForNetwork(String[] networks)
     {
-        return FirewallConfiguration.FACTORY;
+        return new NetworkFirewallRule(networks);
     }
+
 }
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/HostnameFirewallRule.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/HostnameFirewallRule.java
new file mode 100644
index 0000000000..9d60c6d745
--- /dev/null
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/HostnameFirewallRule.java
@@ -0,0 +1,156 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.access.firewall;
+
+import java.net.InetAddress;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.FutureTask;
+import java.util.concurrent.TimeUnit;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang.builder.EqualsBuilder;
+import org.apache.commons.lang.builder.HashCodeBuilder;
+import org.apache.commons.lang.builder.ToStringBuilder;
+import org.apache.commons.lang.builder.ToStringStyle;
+import org.apache.log4j.Logger;
+
+public class HostnameFirewallRule implements FirewallRule
+{
+    private static final Logger _logger = Logger.getLogger(HostnameFirewallRule.class);
+
+    private static final long DNS_TIMEOUT = 30000;
+    private static final ExecutorService DNS_LOOKUP = Executors.newCachedThreadPool();
+
+    private Pattern[] _hostnamePatterns;
+    private String[] _hostnames;
+
+    public HostnameFirewallRule(String... hostnames)
+    {
+        _hostnames = hostnames;
+
+        int i = 0;
+        _hostnamePatterns = new Pattern[hostnames.length];
+        for (String hostname : hostnames)
+        {
+            _hostnamePatterns[i++] = Pattern.compile(hostname);
+        }
+
+        if(_logger.isDebugEnabled())
+        {
+            _logger.debug("Created " + this);
+        }
+    }
+
+    @Override
+    public boolean matches(InetAddress remote)
+    {
+        String hostname = getHostname(remote);
+        if (hostname == null)
+        {
+            throw new AccessControlFirewallException("DNS lookup failed");
+        }
+        for (Pattern pattern : _hostnamePatterns)
+        {
+            boolean hostnameMatches = pattern.matcher(hostname).matches();
+
+
+            if (hostnameMatches)
+            {
+                if(_logger.isDebugEnabled())
+                {
+                    _logger.debug("Hostname " + hostname + " matches rule " + pattern.toString());
+                }
+                return true;
+            }
+        }
+
+        if(_logger.isDebugEnabled())
+        {
+            _logger.debug("Hostname " + hostname + " matches no configured hostname patterns");
+        }
+
+        return false;
+    }
+
+
+    /**
+     * @param remote
+     *            the InetAddress to look up
+     * @return the hostname, null if not found, takes longer than
+     *         {@value #DNS_LOOKUP} to find or otherwise fails
+     */
+    private String getHostname(final InetAddress remote) throws AccessControlFirewallException
+    {
+        FutureTask<String> lookup = new FutureTask<String>(new Callable<String>()
+        {
+            public String call()
+            {
+                return remote.getCanonicalHostName();
+            }
+        });
+        DNS_LOOKUP.execute(lookup);
+
+        try
+        {
+            return lookup.get(DNS_TIMEOUT, TimeUnit.MILLISECONDS);
+        }
+        catch (Exception e)
+        {
+            return null;
+        }
+        finally
+        {
+            lookup.cancel(true);
+        }
+    }
+
+    @Override
+    public int hashCode()
+    {
+        return new HashCodeBuilder().append(_hostnames).toHashCode();
+    }
+
+    @Override
+    public boolean equals(Object obj)
+    {
+        if (obj == null)
+        {
+            return false;
+        }
+        if (obj == this)
+        {
+            return true;
+        }
+        if (obj.getClass() != getClass())
+        {
+            return false;
+        }
+        HostnameFirewallRule rhs = (HostnameFirewallRule) obj;
+        return new EqualsBuilder().append(_hostnames, rhs._hostnames).isEquals();
+    }
+
+    @Override
+    public String toString()
+    {
+        return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
+            .append(_hostnames).toString();
+    }
+}
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/InetNetwork.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/InetNetwork.java
new file mode 100644
index 0000000000..52516af84c
--- /dev/null
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/InetNetwork.java
@@ -0,0 +1,193 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.access.firewall;
+
+import java.net.InetAddress;
+
+class InetNetwork
+{
+    /*
+     * Implements network masking, and is compatible with RFC 1518 and
+     * RFC 1519, which describe CIDR: Classless Inter-Domain Routing.
+     */
+
+    private InetAddress network;
+    private InetAddress netmask;
+
+    public InetNetwork(InetAddress ip, InetAddress netmask)
+    {
+        network = maskIP(ip, netmask);
+        this.netmask = netmask;
+    }
+
+    public boolean contains(final String name) throws java.net.UnknownHostException
+    {
+        return network.equals(maskIP(InetAddress.getByName(name), netmask));
+    }
+
+    public boolean contains(final InetAddress ip)
+    {
+        return network.equals(maskIP(ip, netmask));
+    }
+
+    public String toString()
+    {
+        return network.getHostAddress() + "/" + netmask.getHostAddress();
+    }
+
+    public int hashCode()
+    {
+        return maskIP(network, netmask).hashCode();
+    }
+
+    public boolean equals(Object obj)
+    {
+        return (obj != null) && (obj instanceof InetNetwork) &&
+                ((((InetNetwork)obj).network.equals(network)) && (((InetNetwork)obj).netmask.equals(netmask)));
+    }
+
+    public static InetNetwork getFromString(String netspec) throws java.net.UnknownHostException
+    {
+        if (netspec.endsWith("*"))
+        {
+            netspec = normalizeFromAsterisk(netspec);
+        }
+        else
+        {
+            int iSlash = netspec.indexOf('/');
+            if (iSlash == -1)
+            {
+                netspec += "/255.255.255.255";
+            }
+            else if (netspec.indexOf('.', iSlash) == -1)
+            {
+                netspec = normalizeFromCIDR(netspec);
+            }
+        }
+
+        return new InetNetwork(InetAddress.getByName(netspec.substring(0, netspec.indexOf('/'))),
+                InetAddress.getByName(netspec.substring(netspec.indexOf('/') + 1)));
+    }
+
+    public static InetAddress maskIP(final byte[] ip, final byte[] mask)
+    {
+        try
+        {
+            return getByAddress(new byte[]
+            {
+                (byte) (mask[0] & ip[0]),
+                (byte) (mask[1] & ip[1]),
+                (byte) (mask[2] & ip[2]),
+                (byte) (mask[3] & ip[3])
+            });
+        }
+        catch(Exception _) {}
+        {
+            return null;
+        }
+    }
+
+    public static InetAddress maskIP(final InetAddress ip, final InetAddress mask)
+    {
+        return maskIP(ip.getAddress(), mask.getAddress());
+    }
+
+    /*
+     * This converts from an uncommon "wildcard" CIDR format
+     * to "address + mask" format:
+     *
+     *   *               =>  000.000.000.0/000.000.000.0
+     *   xxx.*           =>  xxx.000.000.0/255.000.000.0
+     *   xxx.xxx.*       =>  xxx.xxx.000.0/255.255.000.0
+     *   xxx.xxx.xxx.*   =>  xxx.xxx.xxx.0/255.255.255.0
+     */
+    static private String normalizeFromAsterisk(final String netspec)
+    {
+        String[] masks = {  "0.0.0.0/0.0.0.0", "0.0.0/255.0.0.0", "0.0/255.255.0.0", "0/255.255.255.0" };
+        char[] srcb = netspec.toCharArray();
+        int octets = 0;
+        for (int i = 1; i < netspec.length(); i++)
+        {
+            if (srcb[i] == '.')
+            {
+                octets++;
+            }
+        }
+        return (octets == 0) ? masks[0] : netspec.substring(0, netspec.length() -1 ).concat(masks[octets]);
+    }
+
+    /*
+     * RFC 1518, 1519 - Classless Inter-Domain Routing (CIDR)
+     * This converts from "prefix + prefix-length" format to
+     * "address + mask" format, e.g. from xxx.xxx.xxx.xxx/yy
+     * to xxx.xxx.xxx.xxx/yyy.yyy.yyy.yyy.
+     */
+    static private String normalizeFromCIDR(final String netspec)
+    {
+        final int bits = 32 - Integer.parseInt(netspec.substring(netspec.indexOf('/')+1));
+        final int mask = (bits == 32) ? 0 : 0xFFFFFFFF - ((1 << bits)-1);
+
+        return netspec.substring(0, netspec.indexOf('/') + 1) +
+                Integer.toString(mask >> 24 & 0xFF, 10) + "." +
+                Integer.toString(mask >> 16 & 0xFF, 10) + "." +
+                Integer.toString(mask >>  8 & 0xFF, 10) + "." +
+                Integer.toString(mask >>  0 & 0xFF, 10);
+    }
+
+    private static java.lang.reflect.Method getByAddress = null;
+
+    static {
+        try {
+            Class<?> inetAddressClass = Class.forName("java.net.InetAddress");
+            Class<?>[] parameterTypes = { byte[].class };
+            getByAddress = inetAddressClass.getMethod("getByAddress", parameterTypes);
+        } catch (Exception e) {
+            getByAddress = null;
+        }
+    }
+
+    private static InetAddress getByAddress(byte[] ip) throws java.net.UnknownHostException
+    {
+        InetAddress addr = null;
+        if (getByAddress != null)
+        {
+            try
+            {
+                addr = (InetAddress) getByAddress.invoke(null, new Object[] { ip });
+            }
+            catch (IllegalAccessException e)
+            {
+            }
+            catch (java.lang.reflect.InvocationTargetException e)
+            {
+            }
+        }
+
+        if (addr == null) {
+            addr = InetAddress.getByName
+                   (
+                    Integer.toString(ip[0] & 0xFF, 10) + "." +
+                    Integer.toString(ip[1] & 0xFF, 10) + "." +
+                    Integer.toString(ip[2] & 0xFF, 10) + "." +
+                    Integer.toString(ip[3] & 0xFF, 10)
+                   );
+        }
+        return addr;
+    }
+}
\ No newline at end of file
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/NetworkFirewallRule.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/NetworkFirewallRule.java
new file mode 100644
index 0000000000..ad619a0e0b
--- /dev/null
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/NetworkFirewallRule.java
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.access.firewall;
+
+import java.net.InetAddress;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.commons.lang.builder.EqualsBuilder;
+import org.apache.commons.lang.builder.HashCodeBuilder;
+import org.apache.commons.lang.builder.ToStringBuilder;
+import org.apache.commons.lang.builder.ToStringStyle;
+import org.apache.log4j.Logger;
+
+public class NetworkFirewallRule implements FirewallRule
+{
+    private static final Logger _logger = Logger.getLogger(NetworkFirewallRule.class);
+
+    private List<InetNetwork> _networks;
+
+    public NetworkFirewallRule(String... networks)
+    {
+        _networks = new ArrayList<InetNetwork>();
+        for (int i = 0; i < networks.length; i++)
+        {
+            String network = networks[i];
+            try
+            {
+                InetNetwork inetNetwork = InetNetwork.getFromString(network);
+                if (!_networks.contains(inetNetwork))
+                {
+                    _networks.add(inetNetwork);
+                }
+            }
+            catch (java.net.UnknownHostException uhe)
+            {
+                _logger.error("Cannot resolve address: " + network, uhe);
+            }
+        }
+
+        if(_logger.isDebugEnabled())
+        {
+            _logger.debug("Created " + this);
+        }
+    }
+
+    @Override
+    public boolean matches(InetAddress ip)
+    {
+        for (InetNetwork network : _networks)
+        {
+            if (network.contains(ip))
+            {
+                if(_logger.isDebugEnabled())
+                {
+                    _logger.debug("Client address " + ip + " matches configured network " + network);
+                }
+                return true;
+            }
+        }
+
+        if(_logger.isDebugEnabled())
+        {
+            _logger.debug("Client address " + ip + " does not match any configured networks");
+        }
+
+        return false;
+    }
+
+    @Override
+    public int hashCode()
+    {
+        return new HashCodeBuilder().append(_networks).toHashCode();
+    }
+
+    @Override
+    public boolean equals(Object obj)
+    {
+        if (obj == null)
+        {
+            return false;
+        }
+        if (obj == this)
+        {
+            return true;
+        }
+        if (obj.getClass() != getClass())
+        {
+            return false;
+        }
+        NetworkFirewallRule rhs = (NetworkFirewallRule) obj;
+        return new EqualsBuilder().append(_networks, rhs._networks).isEquals();
+    }
+
+    @Override
+    public String toString()
+    {
+        return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
+            .append(_networks).toString();
+    }
+}
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/logging/AccessControl_logmessages.properties b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/logging/AccessControl_logmessages.properties
index bf80df3722..2a5eb7b3be 100644
--- a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/logging/AccessControl_logmessages.properties
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/logging/AccessControl_logmessages.properties
@@ -25,4 +25,4 @@
 ALLOWED = ACL-1001 : Allowed : {0} {1} {2}
 
 # 'deny-log' rule message
-DENIED = ACL-1002 : Denied : {0} {1} {2}
\ No newline at end of file
+DENIED = ACL-1002 : Denied : {0} {1} {2}
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java
index d36ae810c6..d94948237f 100644
--- a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java
@@ -20,9 +20,13 @@
  */
 package org.apache.qpid.server.security.access.plugins;
 
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+
 import javax.security.auth.Subject;
 
 import org.apache.commons.configuration.ConfigurationException;
+import org.apache.commons.lang.ObjectUtils;
 import org.apache.log4j.Logger;
 import org.apache.qpid.server.configuration.plugins.ConfigurationPlugin;
 import org.apache.qpid.server.security.AbstractPlugin;
@@ -39,7 +43,7 @@ import org.apache.qpid.server.security.access.config.RuleSet;
  */
 public class AccessControl extends AbstractPlugin
 {
-    public static final Logger _logger = Logger.getLogger(AccessControl.class);
+    private static final Logger _logger = Logger.getLogger(AccessControl.class);
 
     private RuleSet _ruleSet;
 
@@ -82,9 +86,16 @@ public class AccessControl extends AbstractPlugin
 	 * Delegate to the {@link #authorise(Operation, ObjectType, ObjectProperties)} method, with
      * the operation set to ACCESS and no object properties.
 	 */
-    public Result access(ObjectType objectType, Object instance)
+    public Result access(ObjectType objectType, Object inetSocketAddress)
     {
-        return authorise(Operation.ACCESS, objectType, ObjectProperties.EMPTY);
+        InetAddress addressOfClient = null;
+
+        if(inetSocketAddress != null)
+        {
+            addressOfClient = ((InetSocketAddress) inetSocketAddress).getAddress();
+        }
+
+        return authoriseFromAddress(Operation.ACCESS, objectType, ObjectProperties.EMPTY, addressOfClient);
     }
 
     /**
@@ -94,6 +105,11 @@ public class AccessControl extends AbstractPlugin
      */
     public Result authorise(Operation operation, ObjectType objectType, ObjectProperties properties)
     {
+        return authoriseFromAddress(operation, objectType, properties, null);
+    }
+
+    public Result authoriseFromAddress(Operation operation, ObjectType objectType, ObjectProperties properties, InetAddress addressOfClient)
+    {
         final Subject subject = SecurityManager.getThreadSubject();
         // Abstain if there is no subject/principal associated with this thread
         if (subject == null  || subject.getPrincipals().size() == 0)
@@ -101,8 +117,20 @@ public class AccessControl extends AbstractPlugin
             return Result.ABSTAIN;
         }
 
-        _logger.debug("Checking " + operation + " " + objectType);
-        return  _ruleSet.check(subject, operation, objectType, properties);
+        if(_logger.isDebugEnabled())
+        {
+            _logger.debug("Checking " + operation + " " + objectType + " " + ObjectUtils.defaultIfNull(addressOfClient, ""));
+        }
+
+        try
+        {
+            return  _ruleSet.check(subject, operation, objectType, properties, addressOfClient);
+        }
+        catch(Exception e)
+        {
+            _logger.error("Unable to check " + operation + " " + objectType + " " + ObjectUtils.defaultIfNull(addressOfClient, ""), e);
+            return Result.DENIED;
+        }
     }
 
     public void configure(ConfigurationPlugin config)
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclActionTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclActionTest.java
new file mode 100644
index 0000000000..14620cff70
--- /dev/null
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclActionTest.java
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.access.config;
+
+import static org.mockito.Mockito.*;
+
+import org.apache.qpid.server.security.access.ObjectProperties;
+import org.apache.qpid.server.security.access.ObjectType;
+import org.apache.qpid.server.security.access.Operation;
+import org.apache.qpid.server.security.access.firewall.FirewallRule;
+
+import junit.framework.TestCase;
+
+public class AclActionTest extends TestCase
+{
+    public void testEqualsAndHashCode()
+    {
+        AclRulePredicates predicates = createAclRulePredicates();
+        ObjectType objectType = ObjectType.EXCHANGE;
+        Operation operation = Operation.ACCESS;
+
+        AclAction aclAction = new AclAction(operation, objectType, predicates);
+        AclAction equalAclAction = new AclAction(operation, objectType, predicates);
+
+        assertTrue(aclAction.equals(aclAction));
+        assertTrue(aclAction.equals(equalAclAction));
+        assertTrue(equalAclAction.equals(aclAction));
+
+        assertTrue(aclAction.hashCode() == equalAclAction.hashCode());
+
+        assertFalse("Different operation should cause aclActions to be unequal",
+                aclAction.equals(new AclAction(Operation.BIND, objectType, predicates)));
+
+        assertFalse("Different operation type should cause aclActions to be unequal",
+                aclAction.equals(new AclAction(operation, ObjectType.GROUP, predicates)));
+
+        assertFalse("Different predicates should cause aclActions to be unequal",
+                aclAction.equals(new AclAction(operation, objectType, createAclRulePredicates())));
+
+    }
+
+    private AclRulePredicates createAclRulePredicates()
+    {
+        AclRulePredicates predicates = mock(AclRulePredicates.class);
+        when(predicates.getFirewallRule()).thenReturn(mock(FirewallRule.class));
+        when(predicates.getObjectProperties()).thenReturn(mock(ObjectProperties.class));
+        return predicates;
+    }
+
+}
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclRulePredicatesTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclRulePredicatesTest.java
new file mode 100644
index 0000000000..93b765d0fb
--- /dev/null
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclRulePredicatesTest.java
@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.access.config;
+
+import static org.apache.qpid.server.security.access.ObjectProperties.Property.*;
+
+import org.apache.qpid.server.security.access.firewall.FirewallRule;
+import org.apache.qpid.server.security.access.firewall.FirewallRuleFactory;
+
+import static org.mockito.Mockito.*;
+
+import junit.framework.TestCase;
+
+public class AclRulePredicatesTest extends TestCase
+{
+    private AclRulePredicates _aclRulePredicates = new AclRulePredicates();
+    private FirewallRuleFactory _firewallRuleFactory = mock(FirewallRuleFactory.class);
+
+    @Override
+    protected void setUp() throws Exception
+    {
+        _aclRulePredicates.setFirewallRuleFactory(_firewallRuleFactory);
+
+        when(_firewallRuleFactory.createForHostname((String[]) any())).thenReturn(mock(FirewallRule.class));
+        when(_firewallRuleFactory.createForNetwork((String[]) any())).thenReturn(mock(FirewallRule.class));
+    }
+
+    public void testParse()
+    {
+        String name = "name";
+        String className = "class";
+
+        _aclRulePredicates.parse(NAME.name(), name);
+        _aclRulePredicates.parse(CLASS.name(), className);
+
+        assertEquals(name, _aclRulePredicates.getObjectProperties().get(NAME));
+        assertEquals(className, _aclRulePredicates.getObjectProperties().get(CLASS));
+    }
+
+    public void testParseHostnameFirewallRule()
+    {
+        String hostname = "hostname1,hostname2";
+        _aclRulePredicates.parse(FROM_HOSTNAME.name(), hostname);
+
+        verify(_firewallRuleFactory).createForHostname(new String[] {"hostname1", "hostname2"});
+    }
+
+    public void testParseNetworkFirewallRule()
+    {
+        _aclRulePredicates.setFirewallRuleFactory(_firewallRuleFactory);
+
+        String networks = "network1,network2";
+        _aclRulePredicates.parse(FROM_NETWORK.name(), networks);
+
+        verify(_firewallRuleFactory).createForNetwork(new String[] {"network1", "network2"});
+    }
+
+    public void testParseThrowsExceptionIfBothHostnameAndNetworkSpecified()
+    {
+        _aclRulePredicates.parse(FROM_NETWORK.name(), "network1,network2");
+        try
+        {
+            _aclRulePredicates.parse(FROM_HOSTNAME.name(), "hostname1,hostname2");
+            fail("Exception not thrown");
+        }
+        catch(IllegalStateException e)
+        {
+            // pass
+        }
+    }
+}
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/ClientActionTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/ClientActionTest.java
new file mode 100644
index 0000000000..ae5d3fda74
--- /dev/null
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/ClientActionTest.java
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.access.config;
+
+import static org.mockito.Mockito.*;
+
+import java.net.InetAddress;
+
+import org.apache.qpid.server.security.access.firewall.FirewallRule;
+
+import junit.framework.TestCase;
+
+public class ClientActionTest extends TestCase
+{
+    private Action _action = mock(Action.class);
+    private AclAction _ruleAction = mock(AclAction.class);
+    private InetAddress _addressOfClient = mock(InetAddress.class);
+
+    private ClientAction _clientAction = new ClientAction(_action);
+
+    public void testMatches_returnsTrueWhenActionsMatchAndNoFirewallRule()
+    {
+        when(_action.matches(any(Action.class))).thenReturn(true);
+        when(_ruleAction.getFirewallRule()).thenReturn(null);
+
+        assertTrue(_clientAction.matches(_ruleAction, _addressOfClient));
+    }
+
+    public void testMatches_returnsFalseWhenActionsDontMatch()
+    {
+        FirewallRule firewallRule = mock(FirewallRule.class);
+        when(firewallRule.matches(_addressOfClient)).thenReturn(true);
+
+        when(_action.matches(any(Action.class))).thenReturn(false);
+        when(_ruleAction.getFirewallRule()).thenReturn(firewallRule);
+
+        assertFalse(_clientAction.matches(_ruleAction, _addressOfClient));
+    }
+
+    public void testMatches_returnsTrueWhenActionsAndFirewallRuleMatch()
+    {
+        FirewallRule firewallRule = mock(FirewallRule.class);
+        when(firewallRule.matches(_addressOfClient)).thenReturn(true);
+
+        when(_action.matches(any(Action.class))).thenReturn(true);
+        when(_ruleAction.getFirewallRule()).thenReturn(firewallRule);
+
+        assertTrue(_clientAction.matches(_ruleAction, _addressOfClient));
+    }
+
+    public void testMatches_ignoresFirewallRuleIfClientAddressIsNull()
+    {
+        FirewallRule firewallRule = mock(FirewallRule.class);
+
+        when(_action.matches(any(Action.class))).thenReturn(true);
+        when(_ruleAction.getFirewallRule()).thenReturn(firewallRule);
+
+        assertTrue(_clientAction.matches(_ruleAction, null));
+
+        verifyZeroInteractions(firewallRule);
+    }
+
+}
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleTest.java
new file mode 100644
index 0000000000..2ae7759679
--- /dev/null
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleTest.java
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.access.config;
+
+import static org.mockito.Mockito.*;
+
+import org.apache.qpid.server.security.access.Permission;
+
+import junit.framework.TestCase;
+
+public class RuleTest extends TestCase
+{
+    public void testEqualsAndHashCode()
+    {
+        AclAction aclAction = mock(AclAction.class);
+        String identity = "identity";
+        Permission allow = Permission.ALLOW;
+
+        Rule rule = new Rule(identity, aclAction, allow);
+        Rule equalRule = new Rule(identity, aclAction, allow);
+
+        assertTrue(rule.equals(rule));
+        assertTrue(rule.equals(equalRule));
+        assertTrue(equalRule.equals(rule));
+
+        assertTrue(rule.hashCode() == equalRule.hashCode());
+
+        assertFalse("Different identity should cause rules to be unequal",
+                rule.equals(new Rule("identity2", aclAction, allow)));
+
+        assertFalse("Different action should cause rules to be unequal",
+                rule.equals(new Rule(identity, mock(AclAction.class), allow)));
+
+        assertFalse("Different permission should cause rules to be unequal",
+                rule.equals(new Rule(identity, aclAction, Permission.DENY)));
+    }
+}
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/firewall/HostnameFirewallRuleTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/firewall/HostnameFirewallRuleTest.java
new file mode 100644
index 0000000000..be82cb294a
--- /dev/null
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/firewall/HostnameFirewallRuleTest.java
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.access.firewall;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.net.InetAddress;
+
+import org.apache.qpid.server.security.access.firewall.HostnameFirewallRule;
+
+import junit.framework.TestCase;
+
+public class HostnameFirewallRuleTest extends TestCase
+{
+    private InetAddress _addressNotInRule;
+
+    private HostnameFirewallRule _HostnameFirewallRule;
+
+    @Override
+    protected void setUp() throws Exception
+    {
+        _addressNotInRule = InetAddress.getByName("127.0.0.1");
+    }
+
+    public void testSingleHostname() throws Exception
+    {
+        String hostnameInRule = "hostnameInRule";
+        InetAddress addressWithMatchingHostname = mock(InetAddress.class);
+        when(addressWithMatchingHostname.getCanonicalHostName()).thenReturn(hostnameInRule);
+
+        _HostnameFirewallRule = new HostnameFirewallRule(hostnameInRule);
+
+        assertFalse(_HostnameFirewallRule.matches(_addressNotInRule));
+        assertTrue(_HostnameFirewallRule.matches(addressWithMatchingHostname));
+    }
+
+    public void testSingleHostnameWilcard() throws Exception
+    {
+        String hostnameInRule = ".*FOO.*";
+        InetAddress addressWithMatchingHostname = mock(InetAddress.class);
+        when(addressWithMatchingHostname.getCanonicalHostName()).thenReturn("xxFOOxx");
+
+        _HostnameFirewallRule = new HostnameFirewallRule(hostnameInRule);
+
+        assertFalse(_HostnameFirewallRule.matches(_addressNotInRule));
+        assertTrue(_HostnameFirewallRule.matches(addressWithMatchingHostname));
+    }
+
+    public void testMultipleHostnames() throws Exception
+    {
+        String[] hostnamesInRule = new String[] {"hostnameInRule1", "hostnameInRule2"};
+
+        _HostnameFirewallRule = new HostnameFirewallRule(hostnamesInRule);
+
+        assertFalse(_HostnameFirewallRule.matches(_addressNotInRule));
+        for (String hostnameInRule : hostnamesInRule)
+        {
+            InetAddress addressWithMatchingHostname = mock(InetAddress.class);
+            when(addressWithMatchingHostname.getCanonicalHostName()).thenReturn(hostnameInRule);
+
+            assertTrue(_HostnameFirewallRule.matches(addressWithMatchingHostname));
+        }
+    }
+
+    public void testEqualsAndHashCode()
+    {
+        String hostname1 = "hostname1";
+        String hostname2 = "hostname2";
+
+        HostnameFirewallRule rule = new HostnameFirewallRule(hostname1, hostname2);
+        HostnameFirewallRule equalRule = new HostnameFirewallRule(hostname1, hostname2);
+
+        assertTrue(rule.equals(rule));
+        assertTrue(rule.equals(equalRule));
+        assertTrue(equalRule.equals(rule));
+
+        assertTrue(rule.hashCode() == equalRule.hashCode());
+
+        assertFalse("Different hostnames should cause rules to be unequal",
+                rule.equals(new HostnameFirewallRule(hostname1, "different-hostname")));
+    }
+}
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/firewall/NetworkFirewallRuleTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/firewall/NetworkFirewallRuleTest.java
new file mode 100644
index 0000000000..e521039db2
--- /dev/null
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/firewall/NetworkFirewallRuleTest.java
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.access.firewall;
+
+import java.net.InetAddress;
+
+import org.apache.qpid.server.security.access.firewall.NetworkFirewallRule;
+
+import junit.framework.TestCase;
+
+public class NetworkFirewallRuleTest extends TestCase
+{
+    private static final String LOCALHOST_IP = "127.0.0.1";
+    private static final String OTHER_IP_1 = "192.168.23.1";
+    private static final String OTHER_IP_2 = "192.168.23.2";
+
+    private InetAddress _addressNotInRule;
+
+    private NetworkFirewallRule _networkFirewallRule;
+
+    @Override
+    protected void setUp() throws Exception
+    {
+        _addressNotInRule = InetAddress.getByName(LOCALHOST_IP);
+    }
+
+    public void testIpRule() throws Exception
+    {
+        String ipAddressInRule = OTHER_IP_1;
+
+        _networkFirewallRule = new NetworkFirewallRule(ipAddressInRule);
+
+        assertFalse(_networkFirewallRule.matches(_addressNotInRule));
+        assertTrue(_networkFirewallRule.matches(InetAddress.getByName(ipAddressInRule)));
+    }
+
+    public void testNetMask() throws Exception
+    {
+        String ipAddressInRule = "192.168.23.0/24";
+        _networkFirewallRule = new NetworkFirewallRule(ipAddressInRule);
+
+        assertFalse(_networkFirewallRule.matches(InetAddress.getByName("192.168.24.1")));
+        assertTrue(_networkFirewallRule.matches(InetAddress.getByName("192.168.23.0")));
+        assertTrue(_networkFirewallRule.matches(InetAddress.getByName("192.168.23.255")));
+    }
+
+    public void testWildcard() throws Exception
+    {
+        // Test xxx.xxx.*
+
+        assertFalse(new NetworkFirewallRule("192.168.*")
+             .matches(InetAddress.getByName("192.169.1.0")));
+
+        assertTrue(new NetworkFirewallRule("192.168.*")
+            .matches(InetAddress.getByName("192.168.1.0")));
+
+        assertTrue(new NetworkFirewallRule("192.168.*")
+            .matches(InetAddress.getByName("192.168.255.255")));
+
+        // Test xxx.xxx.xxx.*
+
+        assertFalse(new NetworkFirewallRule("192.168.1.*")
+             .matches(InetAddress.getByName("192.169.2.0")));
+
+        assertTrue(new NetworkFirewallRule("192.168.1.*")
+            .matches(InetAddress.getByName("192.168.1.0")));
+
+        assertTrue(new NetworkFirewallRule("192.168.1.*")
+            .matches(InetAddress.getByName("192.168.1.255")));
+    }
+
+    public void testMultipleNetworks() throws Exception
+    {
+        String[] ipAddressesInRule = new String[] {OTHER_IP_1, OTHER_IP_2};
+
+        _networkFirewallRule = new NetworkFirewallRule(ipAddressesInRule);
+
+        assertFalse(_networkFirewallRule.matches(_addressNotInRule));
+        for (String ipAddressInRule : ipAddressesInRule)
+        {
+            assertTrue(_networkFirewallRule.matches(InetAddress.getByName(ipAddressInRule)));
+        }
+    }
+
+    public void testEqualsAndHashCode()
+    {
+        NetworkFirewallRule rule = new NetworkFirewallRule(LOCALHOST_IP, OTHER_IP_1);
+        NetworkFirewallRule equalRule = new NetworkFirewallRule(LOCALHOST_IP, OTHER_IP_1);
+
+        assertTrue(rule.equals(rule));
+        assertTrue(rule.equals(equalRule));
+        assertTrue(equalRule.equals(rule));
+
+        assertTrue(rule.hashCode() == equalRule.hashCode());
+
+        assertFalse("Different networks should cause rules to be unequal",
+                rule.equals(new NetworkFirewallRule(LOCALHOST_IP, OTHER_IP_2)));
+    }
+}
diff --git a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AccessControlTest.java b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AccessControlTest.java
index 2385bcc3dd..a65c442bcf 100644
--- a/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AccessControlTest.java
+++ b/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AccessControlTest.java
@@ -20,6 +20,13 @@
  */
 package org.apache.qpid.server.security.access.plugins;
 
+import static org.mockito.Mockito.*;
+
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+
+import javax.security.auth.Subject;
+
 import junit.framework.TestCase;
 
 import org.apache.commons.configuration.ConfigurationException;
@@ -194,6 +201,49 @@ public class AccessControlTest extends TestCase
         assertEquals(Result.DEFER, result);
     }
 
+    public void testAccess() throws Exception
+    {
+        Subject subject = TestPrincipalUtils.createTestSubject("user1");
+        SecurityManager.setThreadSubject(subject);
+
+        RuleSet mockRuleSet = mock(RuleSet.class);
+        ConfigurationPlugin accessControlConfiguration = createConfiguration(mockRuleSet);
+
+        InetAddress inetAddress = InetAddress.getLocalHost();
+        InetSocketAddress inetSocketAddress = new InetSocketAddress(inetAddress, 1);
+
+        AccessControl accessControl = AccessControl.FACTORY.newInstance(accessControlConfiguration);
+
+        accessControl.access(ObjectType.VIRTUALHOST, inetSocketAddress);
+
+        verify(mockRuleSet).check(subject, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY, inetAddress);
+    }
+
+    public void testAccessIsDeniedIfRuleThrowsException() throws Exception
+    {
+        Subject subject = TestPrincipalUtils.createTestSubject("user1");
+        SecurityManager.setThreadSubject(subject);
+
+        InetAddress inetAddress = InetAddress.getLocalHost();
+        InetSocketAddress inetSocketAddress = new InetSocketAddress(inetAddress, 1);
+
+        RuleSet mockRuleSet = mock(RuleSet.class);
+        when(mockRuleSet.check(
+                subject,
+                Operation.ACCESS,
+                ObjectType.VIRTUALHOST,
+                ObjectProperties.EMPTY,
+                inetAddress)).thenThrow(new RuntimeException());
+
+        ConfigurationPlugin accessControlConfiguration = createConfiguration(mockRuleSet);
+
+        AccessControl accessControl = AccessControl.FACTORY.newInstance(accessControlConfiguration);
+        Result result = accessControl.access(ObjectType.VIRTUALHOST, inetSocketAddress);
+
+        assertEquals(Result.DENIED, result);
+    }
+
+
     /**
      * Tests that a grant access method rule allows any access operation to be performed on a specified component
      */
@@ -332,7 +382,7 @@ public class AccessControlTest extends TestCase
         final ConfigurationPlugin cp = new ConfigurationPlugin()
         {
             @SuppressWarnings("unchecked")
-            public AccessControlConfiguration  getConfiguration(final String plugin)
+            public AccessControlConfiguration getConfiguration(final String plugin)
             {
                 return new AccessControlConfiguration()
                 {
diff --git a/java/broker-plugins/firewall/MANIFEST.MF b/java/broker-plugins/firewall/MANIFEST.MF
deleted file mode 100644
index a302921d03..0000000000
--- a/java/broker-plugins/firewall/MANIFEST.MF
+++ /dev/null
@@ -1,34 +0,0 @@
-Manifest-Version: 1.0
-Bundle-ManifestVersion: 2
-Bundle-Name: Qpid Broker-Plugins Firewall
-Bundle-SymbolicName: broker-plugins-firewall
-Bundle-Description: Firewall plugin for Qpid.
-Bundle-License: http://www.apache.org/licenses/LICENSE-2.0.txt
-Bundle-DocURL: http://www.apache.org/
-Bundle-Version: 1.0.0
-Bundle-Activator: org.apache.qpid.server.security.access.plugins.FirewallActivator
-Bundle-RequiredExecutionEnvironment: JavaSE-1.6
-Bundle-ClassPath: .
-Bundle-ActivationPolicy: lazy
-Import-Package:  org.apache.qpid,
- org.apache.qpid.framing,
- org.apache.qpid.protocol,
- org.apache.qpid.server.configuration,
- org.apache.qpid.server.configuration.plugins,
- org.apache.qpid.server.exchange,
- org.apache.qpid.server.plugins,
- org.apache.qpid.server.queue,
- org.apache.qpid.server.security,
- org.apache.qpid.server.security.access,
- org.apache.qpid.server.virtualhost,
- org.apache.qpid.util,
- org.apache.commons.configuration;version=1.0.0,
- org.apache.commons.lang;version=1.0.0,
- org.apache.commons.lang.builder;version=1.0.0,
- org.apache.log4j;version=1.0.0,
- javax.management;version=1.0.0,
- javax.management.openmbean;version=1.0.0,
- org.osgi.util.tracker;version=1.0.0,
- org.osgi.framework;version=1.3
-Private-Package: org.apache.qpid.server.security.access.config
-Export-Package: org.apache.qpid.server.security.access.plugins;uses:="org.osgi.framework"
diff --git a/java/broker-plugins/firewall/build.xml b/java/broker-plugins/firewall/build.xml
deleted file mode 100644
index 6ae6a35b89..0000000000
--- a/java/broker-plugins/firewall/build.xml
+++ /dev/null
@@ -1,34 +0,0 @@
-<!--
- - Licensed to the Apache Software Foundation (ASF) under one
- - or more contributor license agreements.  See the NOTICE file
- - distributed with this work for additional information
- - regarding copyright ownership.  The ASF licenses this file
- - to you under the Apache License, Version 2.0 (the
- - "License"); you may not use this file except in compliance
- - with the License.  You may obtain a copy of the License at
- - 
- -   http://www.apache.org/licenses/LICENSE-2.0
- - 
- - Unless required by applicable law or agreed to in writing,
- - software distributed under the License is distributed on an
- - "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- - KIND, either express or implied.  See the License for the
- - specific language governing permissions and limitations
- - under the License.
- -->
-<project name="Qpid Broker-Plugins Firewall" default="build">
-    <property name="module.depends" value="common broker" />
-    <property name="module.test.depends" value="test broker/test common/test management/common" />
-    
-    <property name="module.manifest" value="MANIFEST.MF" />
-    <property name="module.plugin" value="true" />
-    <property name="module.genpom" value="true"/>
-    <property name="module.genpom.args" value="-Sqpid-common=provided -Sqpid-broker=provided"/>
-
-    <property name="broker-plugins-firewall.libs" value=""/>
-
-    <import file="../../module.xml" />
-
-    <target name="bundle" depends="bundle-tasks" />
-
-</project>
diff --git a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/config/FirewallRule.java b/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/config/FirewallRule.java
deleted file mode 100644
index ecec4b0cec..0000000000
--- a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/config/FirewallRule.java
+++ /dev/null
@@ -1,136 +0,0 @@
-/*
- *
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- *
- */
-package org.apache.qpid.server.security.access.config;
-
-import org.apache.qpid.server.security.Result;
-import org.apache.qpid.util.NetMatcher;
-
-import java.net.InetAddress;
-import java.util.List;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.FutureTask;
-import java.util.concurrent.TimeUnit;
-import java.util.regex.Pattern;
-
-public class FirewallRule 
-{
-	public static final String ALLOW = "ALLOW";
-	public static final String DENY = "DENY";
-	
-    private static final long DNS_TIMEOUT = 30000;
-    private static final ExecutorService DNS_LOOKUP = Executors.newCachedThreadPool();
-    
-    private Result _access;
-    private NetMatcher _network;
-    private Pattern[] _hostnamePatterns;
-
-    public FirewallRule(String access, List networks, List hostnames)
-    {
-        _access = (access.equalsIgnoreCase(ALLOW)) ? Result.ALLOWED : Result.DENIED;
-
-        if (networks != null && networks.size() > 0)
-        {
-            String[] networkStrings = objListToStringArray(networks);
-            _network = new NetMatcher(networkStrings);
-        }
-
-        if (hostnames != null && hostnames.size() > 0)
-        {
-            int i = 0;
-            _hostnamePatterns = new Pattern[hostnames.size()];
-            for (String hostname : objListToStringArray(hostnames))
-            {
-                _hostnamePatterns[i++] = Pattern.compile(hostname);
-            }
-        }
-    }
-
-    private String[] objListToStringArray(List objList)
-    {
-        String[] networkStrings = new String[objList.size()];
-        int i = 0;
-        for (Object network : objList)
-        {
-            networkStrings[i++] = (String) network;
-        }
-        return networkStrings;
-    }
-
-    public boolean match(InetAddress remote) throws FirewallException
-    {
-        if (_hostnamePatterns != null)
-        {
-            String hostname = getHostname(remote);
-            if (hostname == null)
-            {
-                throw new FirewallException("DNS lookup failed");
-            }
-            for (Pattern pattern : _hostnamePatterns)
-            {
-                if (pattern.matcher(hostname).matches())
-                {
-                    return true;
-                }
-            }
-            return false;
-        }
-        else
-        {
-            return _network.matchInetNetwork(remote);
-        }
-    }
-
-    /**
-     * @param remote the InetAddress to look up
-     * @return the hostname, null if not found, takes longer than 30s to find or otherwise fails
-     */
-    private String getHostname(final InetAddress remote) throws FirewallException
-    {
-        FutureTask<String> lookup = new FutureTask<String>(new Callable<String>()
-        {
-           public String call()
-           {
-               return remote.getCanonicalHostName();
-           }
-        });
-        DNS_LOOKUP.execute(lookup);
-        
-        try
-        {
-            return lookup.get(DNS_TIMEOUT, TimeUnit.MILLISECONDS);
-        }
-        catch (Exception e)
-        {
-            return null; 
-        }
-        finally
-        {
-            lookup.cancel(true);
-        }
-    }
-
-    public Result getAccess()
-    {
-        return _access;
-    }
-}
\ No newline at end of file
diff --git a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/Firewall.java b/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/Firewall.java
deleted file mode 100644
index 40a65fddba..0000000000
--- a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/Firewall.java
+++ /dev/null
@@ -1,137 +0,0 @@
-/*
- *
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- *
- */
-package org.apache.qpid.server.security.access.plugins;
-
-import org.apache.commons.configuration.Configuration;
-import org.apache.commons.configuration.ConfigurationException;
-
-import org.apache.qpid.server.configuration.plugins.ConfigurationPlugin;
-import org.apache.qpid.server.security.AbstractPlugin;
-import org.apache.qpid.server.security.Result;
-import org.apache.qpid.server.security.SecurityPluginFactory;
-import org.apache.qpid.server.security.access.ObjectProperties;
-import org.apache.qpid.server.security.access.ObjectType;
-import org.apache.qpid.server.security.access.Operation;
-import org.apache.qpid.server.security.access.config.FirewallException;
-import org.apache.qpid.server.security.access.config.FirewallRule;
-
-import java.net.InetAddress;
-import java.net.InetSocketAddress;
-
-public class Firewall extends AbstractPlugin
-{
-    public static final SecurityPluginFactory<Firewall> FACTORY = new SecurityPluginFactory<Firewall>()
-    {
-        public Firewall newInstance(ConfigurationPlugin config) throws ConfigurationException
-        {
-            FirewallConfiguration configuration = config.getConfiguration(FirewallConfiguration.class.getName());
-
-            // If there is no configuration for this plugin then don't load it.
-            if (configuration == null)
-            {
-                return null;
-            }
-            
-            Firewall plugin = new Firewall();
-            plugin.configure(configuration);
-            return plugin;
-        }
-        
-        public Class<Firewall> getPluginClass()
-        {
-            return Firewall.class;
-        }
-
-        public String getPluginName()
-        {
-            return Firewall.class.getName();
-        }
-    };
-	
-    private Result _default = Result.ABSTAIN;
-    private FirewallRule[] _rules;
-    
-	public Result getDefault()
-	{
-		return _default;
-	}
-
-    public Result authorise(Operation operation, ObjectType objectType, ObjectProperties properties)
-    {
-        return Result.ABSTAIN; // We only deal with access requests
-    }
-
-    public Result access(ObjectType objectType, Object instance)
-    {
-        if (objectType != ObjectType.VIRTUALHOST)
-        {
-            return Result.ABSTAIN; // We are only interested in access to virtualhosts
-        }
-
-        if (!(instance instanceof InetSocketAddress))
-        {
-            return Result.ABSTAIN; // We need an internet address
-        }
-
-        InetAddress address = ((InetSocketAddress) instance).getAddress();
-        
-        try
-        {
-            for (FirewallRule rule : _rules)
-            {
-                boolean match = rule.match(address);
-                if (match)
-                {
-                    return rule.getAccess();
-                }
-            }
-            return getDefault();
-        }
-        catch (FirewallException fe)
-        {
-            return Result.DENIED;
-        }
-    }
-    
-
-    public void configure(ConfigurationPlugin config)
-    {
-        super.configure(config);
-        FirewallConfiguration firewallConfiguration = (FirewallConfiguration) getConfig();
-
-        // Get default action
-        _default = firewallConfiguration.getDefaultAction();
-
-        Configuration finalConfig = firewallConfiguration.getConfiguration();
-
-        // all rules must have an access attribute
-        int numRules = finalConfig.getList("rule[@access]").size();
-        _rules = new FirewallRule[numRules];
-        for (int i = 0; i < numRules; i++)
-        {
-            FirewallRule rule = new FirewallRule(finalConfig.getString("rule(" + i + ")[@access]"),
-                                                 finalConfig.getList("rule(" + i + ")[@network]"),
-                                                 finalConfig.getList("rule(" + i + ")[@hostname]"));
-            _rules[i] = rule;
-        }
-
-    }
-}
diff --git a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/FirewallConfiguration.java b/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/FirewallConfiguration.java
deleted file mode 100644
index 010d1652f0..0000000000
--- a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/FirewallConfiguration.java
+++ /dev/null
@@ -1,103 +0,0 @@
-/*
- *
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- *
- */
-package org.apache.qpid.server.security.access.plugins;
-
-import org.apache.commons.configuration.CompositeConfiguration;
-import org.apache.commons.configuration.Configuration;
-import org.apache.commons.configuration.ConfigurationException;
-import org.apache.commons.configuration.XMLConfiguration;
-
-import org.apache.qpid.server.configuration.plugins.ConfigurationPlugin;
-import org.apache.qpid.server.configuration.plugins.ConfigurationPluginFactory;
-import org.apache.qpid.server.security.Result;
-import org.apache.qpid.server.security.access.config.FirewallRule;
-
-import java.util.Arrays;
-import java.util.List;
-
-public class FirewallConfiguration extends ConfigurationPlugin
-{
-    private CompositeConfiguration _finalConfig;
-
-    public static final ConfigurationPluginFactory FACTORY = new ConfigurationPluginFactory()
-    {
-        public ConfigurationPlugin newInstance(String path, Configuration config) throws ConfigurationException
-        {
-            ConfigurationPlugin instance = new FirewallConfiguration();
-            instance.setConfiguration(path, config);
-            return instance;
-        }
-
-        public List<String> getParentPaths()
-        {
-            return Arrays.asList("security.firewall", "virtualhosts.virtualhost.security.firewall");
-        }
-    };
-
-    public String[] getElementsProcessed()
-    {
-        return new String[] { "" };
-    }
-
-    public Configuration getConfiguration()
-    {
-        return _finalConfig;
-    }
-
-    public Result getDefaultAction()
-    {
-        String defaultAction = getConfig().getString("[@default-action]");
-        if (defaultAction == null)
-        {
-            return Result.ABSTAIN;
-        }
-        else if (defaultAction.equalsIgnoreCase(FirewallRule.ALLOW))
-        {
-            return Result.ALLOWED;
-        }
-        else
-        {
-            return Result.DENIED;
-        }
-    }
-
-
-
-    @Override
-    public void validateConfiguration() throws ConfigurationException
-    {
-        // Valid Configuration either has xml links to new files
-        _finalConfig = new CompositeConfiguration(getConfig());
-        List subFiles = getConfig().getList("xml[@fileName]");
-        for (Object subFile : subFiles)
-        {
-            _finalConfig.addConfiguration(new XMLConfiguration((String) subFile));
-        }
-
-        // all rules must have an access attribute or a default value
-        if (_finalConfig.getList("rule[@access]").size() == 0 &&
-            getConfig().getString("[@default-action]") == null)
-        {
-            throw new ConfigurationException("No rules or default-action found in firewall configuration.");
-        }
-    }
-
-}
diff --git a/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallConfigurationTest.java b/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallConfigurationTest.java
deleted file mode 100644
index 8969363979..0000000000
--- a/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallConfigurationTest.java
+++ /dev/null
@@ -1,322 +0,0 @@
-/*
- *
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- *
- */
-package org.apache.qpid.server.security.access;
-
-import org.apache.qpid.server.registry.ApplicationRegistry;
-import org.apache.qpid.server.registry.ConfigurationFileApplicationRegistry;
-import org.apache.qpid.server.virtualhost.VirtualHost;
-import org.apache.qpid.server.virtualhost.VirtualHostRegistry;
-import org.apache.qpid.test.utils.QpidTestCase;
-
-import java.io.File;
-import java.io.FileWriter;
-import java.io.IOException;
-import java.io.RandomAccessFile;
-import java.net.InetSocketAddress;
-
-public class FirewallConfigurationTest extends QpidTestCase
-{
-    @Override
-    protected void tearDown() throws Exception
-    {
-        super.tearDown();
-        ApplicationRegistry.remove();
-    }
-
-    public void testFirewallConfiguration() throws Exception
-    {
-        // Write out config
-        File mainFile = File.createTempFile(getClass().getName(), null);
-        mainFile.deleteOnExit();
-        writeConfigFile(mainFile, false);
-
-        // Load config
-        ApplicationRegistry reg = new ConfigurationFileApplicationRegistry(mainFile);
-        ApplicationRegistry.initialise(reg);
-
-        // Test config
-        assertFalse(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.0.0.1", 65535)));
-        assertTrue(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.1.2.3", 65535)));
-    }
-
-    public void testCombinedConfigurationFirewall() throws Exception
-    {
-        // Write out config
-        File mainFile = File.createTempFile(getClass().getName(), null);
-        File fileA = File.createTempFile(getClass().getName(), null);
-        File fileB = File.createTempFile(getClass().getName(), null);
-
-        mainFile.deleteOnExit();
-        fileA.deleteOnExit();
-        fileB.deleteOnExit();
-
-        FileWriter out = new FileWriter(mainFile);
-        out.write("<configuration><system/>");
-        out.write("<xml fileName=\"" + fileA.getAbsolutePath() + "\"/>");
-        out.write("</configuration>");
-        out.close();
-
-        out = new FileWriter(fileA);
-        out.write("<broker>\n");
-        out.write("\t<plugin-directory>${QPID_HOME}/lib/plugins</plugin-directory>\n");
-        out.write("\t<cache-directory>${QPID_WORK}/cache</cache-directory>\n");
-        out.write("\t<management><enabled>false</enabled></management>\n");
-        out.write("\t<security>\n");
-        out.write("\t\t<pd-auth-manager>\n");
-        out.write("\t\t\t<principal-database>\n");
-        out.write("\t\t\t\t<class>org.apache.qpid.server.security.auth.database.PlainPasswordFilePrincipalDatabase</class>\n");
-        out.write("\t\t\t\t<attributes>\n");
-        out.write("\t\t\t\t\t<attribute>\n");
-        out.write("\t\t\t\t\t\t<name>passwordFile</name>\n");
-        out.write("\t\t\t\t\t\t<value>/dev/null</value>\n");
-        out.write("\t\t\t\t\t</attribute>\n");
-        out.write("\t\t\t\t</attributes>\n");
-        out.write("\t\t\t</principal-database>\n");
-        out.write("\t\t</pd-auth-manager>\n");
-        out.write("\t\t<firewall>\n");
-        out.write("\t\t\t<xml fileName=\"" + fileB.getAbsolutePath() + "\"/>");
-        out.write("\t\t</firewall>\n");
-        out.write("\t</security>\n");
-        out.write("\t<virtualhosts>\n");
-        out.write("\t\t<virtualhost>\n");
-        out.write("\t\t\t<name>test</name>\n");
-        out.write("\t\t</virtualhost>\n");
-        out.write("\t</virtualhosts>\n");
-        out.write("</broker>\n");
-        out.close();
-
-        out = new FileWriter(fileB);
-        out.write("<firewall>\n");
-        out.write("\t<rule access=\"deny\" network=\"127.0.0.1\"/>");
-        out.write("</firewall>\n");
-        out.close();
-
-        // Load config
-        ApplicationRegistry reg = new ConfigurationFileApplicationRegistry(mainFile);
-        ApplicationRegistry.initialise(reg);
-
-        // Test config
-        assertFalse(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.0.0.1", 65535)));
-    }
-
-    public void testConfigurationFirewallReload() throws Exception
-    {
-        // Write out config
-        File mainFile = File.createTempFile(getClass().getName(), null);
-
-        mainFile.deleteOnExit();
-        writeConfigFile(mainFile, false);
-
-        // Load config
-        ApplicationRegistry reg = new ConfigurationFileApplicationRegistry(mainFile);
-        ApplicationRegistry.initialise(reg);
-
-        // Test config
-        assertFalse(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.0.0.1", 65535)));
-
-        // Switch to deny the connection
-        writeConfigFile(mainFile, true);
-
-        reg.getConfiguration().reparseConfigFileSecuritySections();
-
-        assertTrue(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.0.0.1", 65535)));
-    }
-
-    public void testCombinedConfigurationFirewallReload() throws Exception
-    {
-        // Write out config
-        File mainFile = File.createTempFile(getClass().getName(), null);
-        File fileA = File.createTempFile(getClass().getName(), null);
-        File fileB = File.createTempFile(getClass().getName(), null);
-
-        mainFile.deleteOnExit();
-        fileA.deleteOnExit();
-        fileB.deleteOnExit();
-
-        FileWriter out = new FileWriter(mainFile);
-        out.write("<configuration><system/>");
-        out.write("<xml fileName=\"" + fileA.getAbsolutePath() + "\"/>");
-        out.write("</configuration>");
-        out.close();
-
-        out = new FileWriter(fileA);
-        out.write("<broker>\n");
-        out.write("\t<plugin-directory>${QPID_HOME}/lib/plugins</plugin-directory>\n");
-        out.write("\t<management><enabled>false</enabled></management>\n");
-        out.write("\t<security>\n");
-        out.write("\t\t<pd-auth-manager>\n");
-        out.write("\t\t\t<principal-database>\n");
-        out.write("\t\t\t\t<class>org.apache.qpid.server.security.auth.database.PlainPasswordFilePrincipalDatabase</class>\n");
-        out.write("\t\t\t\t<attributes>\n");
-        out.write("\t\t\t\t\t<attribute>\n");
-        out.write("\t\t\t\t\t\t<name>passwordFile</name>\n");
-        out.write("\t\t\t\t\t\t<value>/dev/null</value>\n");
-        out.write("\t\t\t\t\t</attribute>\n");
-        out.write("\t\t\t\t</attributes>\n");
-        out.write("\t\t\t</principal-database>\n");
-        out.write("\t\t</pd-auth-manager>\n");
-        out.write("\t\t<firewall>\n");
-        out.write("\t\t\t<xml fileName=\"" + fileB.getAbsolutePath() + "\"/>");
-        out.write("\t\t</firewall>\n");
-        out.write("\t</security>\n");
-        out.write("\t<virtualhosts>\n");
-        out.write("\t\t<virtualhost>\n");
-        out.write("\t\t\t<name>test</name>\n");
-        out.write("\t\t</virtualhost>\n");
-        out.write("\t</virtualhosts>\n");
-        out.write("</broker>\n");
-        out.close();
-
-        out = new FileWriter(fileB);
-        out.write("<firewall>\n");
-        out.write("\t<rule access=\"deny\" network=\"127.0.0.1\"/>");
-        out.write("</firewall>\n");
-        out.close();
-
-        // Load config
-        ApplicationRegistry reg = new ConfigurationFileApplicationRegistry(mainFile);
-        ApplicationRegistry.initialise(reg);
-
-        // Test config
-        assertFalse(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.0.0.1", 65535)));
-
-        RandomAccessFile fileBRandom = new RandomAccessFile(fileB, "rw");
-        fileBRandom.setLength(0);
-        fileBRandom.seek(0);
-        fileBRandom.close();
-
-        out = new FileWriter(fileB);
-        out.write("<firewall>\n");
-        out.write("\t<rule access=\"allow\" network=\"127.0.0.1\"/>");
-        out.write("</firewall>\n");
-        out.close();
-
-        reg.getConfiguration().reparseConfigFileSecuritySections();
-
-        assertTrue(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.0.0.1", 65535)));
-
-        fileBRandom = new RandomAccessFile(fileB, "rw");
-        fileBRandom.setLength(0);
-        fileBRandom.seek(0);
-        fileBRandom.close();
-
-        out = new FileWriter(fileB);
-        out.write("<firewall>\n");
-        out.write("\t<rule access=\"deny\" network=\"127.0.0.1\"/>");
-        out.write("</firewall>\n");
-        out.close();
-
-        reg.getConfiguration().reparseConfigFileSecuritySections();
-
-        assertFalse(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.0.0.1", 65535)));
-    }
-
-    private void writeFirewallVhostsFile(File vhostsFile, boolean allow) throws IOException
-    {
-        FileWriter out = new FileWriter(vhostsFile);
-        String ipAddr = "127.0.0.1"; // FIXME: get this from InetAddress.getLocalHost().getAddress() ?
-        out.write("<virtualhosts><virtualhost>");
-        out.write("<name>test</name>");
-        out.write("<test>");
-        out.write("<security><firewall>");
-        out.write("<rule access=\""+((allow) ? "allow" : "deny")+"\" network=\""+ipAddr +"\"/>");
-        out.write("</firewall></security>");
-        out.write("</test>");
-        out.write("</virtualhost></virtualhosts>");
-        out.close();
-    }
-
-    private void writeConfigFile(File mainFile, boolean allow) throws IOException {
-        writeConfigFile(mainFile, allow, true, null, "test");
-    }
-
-    /*
-        XMLConfiguration config = new XMLConfiguration(mainFile);
-        PluginManager pluginManager = new MockPluginManager("");
-        SecurityManager manager = new SecurityManager(config, pluginManager, Firewall.FACTORY);
-        
-     */
-    private void writeConfigFile(File mainFile, boolean allow, boolean includeVhosts, File vhostsFile, String name) throws IOException {
-        FileWriter out = new FileWriter(mainFile);
-        out.write("<broker>\n");
-        out.write("\t<plugin-directory>${QPID_HOME}/lib/plugins</plugin-directory>\n");
-        out.write("\t<management><enabled>false</enabled></management>\n");
-        out.write("\t<security>\n");
-        out.write("\t\t<pd-auth-manager>\n");
-        out.write("\t\t\t<principal-database>\n");
-        out.write("\t\t\t\t<class>org.apache.qpid.server.security.auth.database.PlainPasswordFilePrincipalDatabase</class>\n");
-        out.write("\t\t\t\t<attributes>\n");
-        out.write("\t\t\t\t\t<attribute>\n");
-        out.write("\t\t\t\t\t\t<name>passwordFile</name>\n");
-        out.write("\t\t\t\t\t\t<value>/dev/null</value>\n");
-        out.write("\t\t\t\t\t</attribute>\n");
-        out.write("\t\t\t\t</attributes>\n");
-        out.write("\t\t\t</principal-database>\n");
-        out.write("\t\t</pd-auth-manager>\n");
-        out.write("\t\t<firewall>\n");
-        out.write("\t\t\t<rule access=\""+ ((allow) ? "allow" : "deny") +"\" network=\"127.0.0.1\"/>");
-        out.write("\t\t</firewall>\n");
-        out.write("\t</security>\n");
-        if (includeVhosts)
-        {
-	        out.write("\t<virtualhosts>\n");
-            out.write("\t\t<default>test</default>\n");
-	        out.write("\t\t<virtualhost>\n");
-	        out.write(String.format("\t\t\t<name>%s</name>\n", name));
-	        out.write("\t\t</virtualhost>\n");
-	        out.write("\t</virtualhosts>\n");
-        }
-        if (vhostsFile != null)
-        {
-        	out.write("\t<virtualhosts>"+vhostsFile.getAbsolutePath()+"</virtualhosts>\n");	
-        }
-        out.write("</broker>\n");
-        out.close();
-    }
-    
-    /**
-     * Test that configuration loads correctly when virtual hosts are specified in an external
-     * configuration file only.
-     * <p>
-     * Test for QPID-2360
-     */
-    public void testExternalFirewallVirtualhostXMLFile() throws Exception
-    {
-        // Write out config
-        File mainFile = File.createTempFile(getClass().getName(), "config");
-        mainFile.deleteOnExit();
-        File vhostsFile = File.createTempFile(getClass().getName(), "vhosts");
-        vhostsFile.deleteOnExit();
-        writeConfigFile(mainFile, false, false, vhostsFile, null);    
-        writeFirewallVhostsFile(vhostsFile, false);
-
-        // Load config
-        ApplicationRegistry reg = new ConfigurationFileApplicationRegistry(mainFile);
-        ApplicationRegistry.initialise(reg);
-    
-        // Test config
-        VirtualHostRegistry virtualHostRegistry = reg.getVirtualHostRegistry();
-        VirtualHost virtualHost = virtualHostRegistry.getVirtualHost("test");
-
-        assertEquals("Incorrect virtualhost count", 1, virtualHostRegistry.getVirtualHosts().size());
-        assertEquals("Incorrect virtualhost name", "test", virtualHost.getName());
-    }
-}
diff --git a/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallPluginTest.java b/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallPluginTest.java
deleted file mode 100644
index 2004852c48..0000000000
--- a/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallPluginTest.java
+++ /dev/null
@@ -1,294 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.qpid.server.security.access;
-
-import org.apache.commons.configuration.ConfigurationException;
-import org.apache.commons.configuration.XMLConfiguration;
-
-import org.apache.qpid.server.configuration.ServerConfiguration;
-import org.apache.qpid.server.registry.ApplicationRegistry;
-import org.apache.qpid.server.security.Result;
-import org.apache.qpid.server.security.access.plugins.Firewall;
-import org.apache.qpid.server.security.access.plugins.FirewallConfiguration;
-import org.apache.qpid.server.util.TestApplicationRegistry;
-import org.apache.qpid.test.utils.QpidTestCase;
-
-import java.io.BufferedWriter;
-import java.io.File;
-import java.io.FileWriter;
-import java.io.IOException;
-import java.net.InetSocketAddress;
-import java.net.SocketAddress;
-
-public class FirewallPluginTest extends QpidTestCase
-{
-    public class RuleInfo
-    {
-        private String _access;
-        private String _network;
-        private String _hostname;
-        
-        public void setAccess(String _access)
-        {
-            this._access = _access;
-        }
-        
-        public String getAccess()
-        {
-            return _access;
-        }
-        
-        public void setNetwork(String _network)
-        {
-            this._network = _network;
-        }
-        
-        public String getNetwork()
-        {
-            return _network;
-        }
-        
-        public void setHostname(String _hostname)
-        {
-            this._hostname = _hostname;
-        }
-        
-        public String getHostname()
-        {
-            return _hostname;
-        }
-    }
-
-    // IP address
-    private SocketAddress _address;
-    private ServerConfiguration _serverConfig;
-
-    @Override
-    protected void setUp() throws Exception
-    {
-        super.setUp();
-        _serverConfig = new ServerConfiguration(new XMLConfiguration());
-        ApplicationRegistry.initialise(new TestApplicationRegistry(_serverConfig));
-        _address = new InetSocketAddress("127.0.0.1", 65535);
-    }
-
-    @Override
-    protected void tearDown() throws Exception
-    {
-        super.tearDown();
-        ApplicationRegistry.remove();
-    }
-    private Firewall initialisePlugin(String defaultAction, RuleInfo[] rules) throws IOException, ConfigurationException
-    {
-        // Create sample config file
-        File confFile = File.createTempFile(getClass().getSimpleName()+"conffile", null);
-        confFile.deleteOnExit();
-        BufferedWriter buf = new BufferedWriter(new FileWriter(confFile));
-        buf.write("<firewall default-action=\""+defaultAction+"\">\n");
-        if (rules != null)
-        {
-            for (RuleInfo rule : rules)
-            {
-                buf.write("<rule");
-                buf.write(" access=\""+rule.getAccess()+"\"");
-                if (rule.getHostname() != null)
-                {
-                    buf.write(" hostname=\""+rule.getHostname()+"\"");
-                }
-                if (rule.getNetwork() != null)
-                {
-                    buf.write(" network=\""+rule.getNetwork()+"\"");
-                }
-                buf.write("/>\n");
-            }
-        }
-        buf.write("</firewall>");
-        buf.close();
-
-        // Configure plugin
-        FirewallConfiguration config = new FirewallConfiguration();
-        config.setConfiguration("", new XMLConfiguration(confFile));
-        Firewall plugin = new Firewall();
-        plugin.configure(config);
-        return plugin;
-    }
-
-    private Firewall initialisePlugin(String string) throws ConfigurationException, IOException
-    {
-        return initialisePlugin(string, null);
-    }
-    
-    public void testDefaultAction() throws Exception
-    {
-        // Test simple deny
-        Firewall plugin = initialisePlugin("deny");
-        assertEquals(Result.DENIED, plugin.access(ObjectType.VIRTUALHOST, _address));
-
-        // Test simple allow
-        plugin = initialisePlugin("allow");
-        assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
-    }
-    
-
-    public void testSingleIPRule() throws Exception
-    {
-        RuleInfo rule = new RuleInfo();
-        rule.setAccess("allow");
-        rule.setNetwork("192.168.23.23");
-        
-        Firewall plugin = initialisePlugin("deny", new RuleInfo[]{rule});
-
-        assertEquals(Result.DENIED, plugin.access(ObjectType.VIRTUALHOST, _address));
-        
-        // Set IP so that we're connected from the right address
-        _address = new InetSocketAddress("192.168.23.23", 65535);
-        assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
-    }
-    
-    public void testSingleNetworkRule() throws Exception
-    {
-        RuleInfo rule = new RuleInfo();
-        rule.setAccess("allow");
-        rule.setNetwork("192.168.23.0/24");
-        
-        Firewall plugin = initialisePlugin("deny", new RuleInfo[]{rule});
-
-        assertEquals(Result.DENIED, plugin.access(ObjectType.VIRTUALHOST, _address));
-        
-        // Set IP so that we're connected from the right address
-        _address = new InetSocketAddress("192.168.23.23", 65535);
-        assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
-    }
-
-    public void testSingleHostRule() throws Exception
-    {
-        RuleInfo rule = new RuleInfo();
-        rule.setAccess("allow");
-        rule.setHostname(new InetSocketAddress("127.0.0.1", 5672).getHostName());
-        
-        Firewall plugin = initialisePlugin("deny", new RuleInfo[]{rule});
-
-        // Set IP so that we're connected from the right address
-        _address = new InetSocketAddress("127.0.0.1", 65535);
-        assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
-    }
-
-    public void testSingleHostWilcardRule() throws Exception
-    {
-        RuleInfo rule = new RuleInfo();
-        rule.setAccess("allow");
-        String hostname = new InetSocketAddress("127.0.0.1", 0).getHostName();
-        rule.setHostname(".*"+hostname.subSequence(hostname.length() - 1, hostname.length())+"*");
-        Firewall plugin = initialisePlugin("deny", new RuleInfo[]{rule});
-
-        // Set IP so that we're connected from the right address
-        _address = new InetSocketAddress("127.0.0.1", 65535);
-        assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
-    }
-    
-    public void testSeveralFirstAllowsAccess() throws Exception
-    {
-        RuleInfo firstRule = new RuleInfo();
-        firstRule.setAccess("allow");
-        firstRule.setNetwork("192.168.23.23");
-        
-        RuleInfo secondRule = new RuleInfo();
-        secondRule.setAccess("deny");
-        secondRule.setNetwork("192.168.42.42");
-
-        RuleInfo thirdRule = new RuleInfo();
-        thirdRule.setAccess("deny");
-        thirdRule.setHostname("localhost");
-        
-        Firewall plugin = initialisePlugin("deny", new RuleInfo[]{firstRule, secondRule, thirdRule});
-
-        assertEquals(Result.DENIED, plugin.access(ObjectType.VIRTUALHOST, _address));
-        
-        // Set IP so that we're connected from the right address
-        _address = new InetSocketAddress("192.168.23.23", 65535);
-        assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
-    }
-    
-    public void testSeveralLastAllowsAccess() throws Exception
-    {
-        RuleInfo firstRule = new RuleInfo();
-        firstRule.setAccess("deny");
-        firstRule.setHostname("localhost");
-        
-        RuleInfo secondRule = new RuleInfo();
-        secondRule.setAccess("deny");
-        secondRule.setNetwork("192.168.42.42");
-
-        RuleInfo thirdRule = new RuleInfo();
-        thirdRule.setAccess("allow");
-        thirdRule.setNetwork("192.168.23.23");
-        
-        Firewall plugin = initialisePlugin("deny", new RuleInfo[]{firstRule, secondRule, thirdRule});
-
-        assertEquals(Result.DENIED, plugin.access(ObjectType.VIRTUALHOST, _address));
-        
-        // Set IP so that we're connected from the right address
-        _address = new InetSocketAddress("192.168.23.23", 65535);
-        assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
-    }
-
-    public void testNetmask() throws Exception
-    {
-        RuleInfo firstRule = new RuleInfo();
-        firstRule.setAccess("allow");
-        firstRule.setNetwork("192.168.23.0/24");
-        Firewall plugin = initialisePlugin("deny", new RuleInfo[]{firstRule});
-
-        assertEquals(Result.DENIED, plugin.access(ObjectType.VIRTUALHOST, _address));
-        
-        // Set IP so that we're connected from the right address
-        _address = new InetSocketAddress("192.168.23.23", 65535);
-        assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
-    }
-    
-    public void testCommaSeperatedNetmask() throws Exception
-    {
-        RuleInfo firstRule = new RuleInfo();
-        firstRule.setAccess("allow");
-        firstRule.setNetwork("10.1.1.1/8, 192.168.23.0/24");
-        Firewall plugin = initialisePlugin("deny", new RuleInfo[]{firstRule});
-
-        assertEquals(Result.DENIED, plugin.access(ObjectType.VIRTUALHOST, _address));
-        
-        // Set IP so that we're connected from the right address
-        _address = new InetSocketAddress("192.168.23.23", 65535);
-        assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
-    }
-    
-    public void testCommaSeperatedHostnames() throws Exception
-    {
-        RuleInfo firstRule = new RuleInfo();
-        firstRule.setAccess("allow");
-        firstRule.setHostname("foo, bar, "+new InetSocketAddress("127.0.0.1", 5672).getHostName());
-        Firewall plugin = initialisePlugin("deny", new RuleInfo[]{firstRule});
-        
-        // Set IP so that we're connected from the right address
-        _address = new InetSocketAddress("10.0.0.1", 65535);
-        assertEquals(Result.DENIED, plugin.access(ObjectType.VIRTUALHOST, _address));
-        
-        // Set IP so that we're connected from the right address
-        _address = new InetSocketAddress("127.0.0.1", 65535);
-        assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
-    }
-}