QPID-4292: add ACL rule to authorise access to the web management UI

* added object name MANAGEMENT to represent both JMX and Web Management layers
* Change both JMX/Web entry points to permission access with an access management check
* Updated examples and docbook
* Made Principals serialised to avoid container warnings when Qpid principals are placed within a HttpSession.

Work of Robbie Gemmell <robbie@apache.org> and myself.

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

8 files changed, 156 insertions, 88 deletions

diff --git a/java/broker/etc/broker_example.acl b/java/broker/etc/broker_example.acl
index a5e01fb895..fee1192371 100644
--- a/java/broker/etc/broker_example.acl
+++ b/java/broker/etc/broker_example.acl
@@ -23,9 +23,12 @@
 
 ### JMX MANAGEMENT ####
 
-# Allow everyone to perform read operations on the ServerInformation mbean
-# This is used for items such as querying the management API and broker release versions.
-ACL ALLOW ALL ACCESS METHOD component="ServerInformation"
+# To use JMX management, first give the user/group ACCESS MANAGEMENT permission
+ACL ALLOW administrators ACCESS MANAGEMENT
+ACL ALLOW guest ACCESS MANAGEMENT
+
+# Allow guest to perform read operations on the ServerInformation mbean
+ACL ALLOW guest ACCESS METHOD component="ServerInformation"
 
 # Allow 'administrators' all management operations. To reduce log file noise, only non-read-only operations are logged.
 ACL ALLOW administrators ACCESS METHOD
@@ -42,33 +45,13 @@ ACL DENY-LOG ALL ACCESS METHOD component="UserManagement"
 ACL DENY-LOG ALL ACCESS METHOD component="ConfigurationManagement"
 ACL DENY-LOG ALL ACCESS METHOD component="LoggingManagement"
 
-# Allow everyone to perform all read operations (using ALLOW rather than ALLOW-LOG to reduce log file noise)
-# on the mbeans not listed in the DENY rules above
+# Allow everyone to perform all read operations on the mbeans not listed in the DENY rules above
 ACL ALLOW ALL ACCESS METHOD
 
-### MESSAGING ###
-
-#Example permissions for request-response based messaging.
-
-#Allow 'messaging-users' group to connect to the virtualhost
-ACL ALLOW-LOG messaging-users ACCESS VIRTUALHOST
-
-# Client side
-# Allow the 'client' user to publish requests to the request queue and create, consume from, and delete temporary reply queues.
-ACL ALLOW-LOG client CREATE QUEUE temporary="true"
-ACL ALLOW-LOG client CONSUME QUEUE temporary="true"
-ACL ALLOW-LOG client DELETE QUEUE temporary="true"
-ACL ALLOW-LOG client BIND EXCHANGE name="amq.direct" temporary="true"
-ACL ALLOW-LOG client UNBIND EXCHANGE name="amq.direct" temporary="true"
-ACL ALLOW-LOG client PUBLISH EXCHANGE name="amq.direct" routingKey="example.RequestQueue"
+### WEB MANAGEMENT ####
 
-# Server side
-# Allow the 'server' user to create and consume from the request queue and publish a response to the temporary response queue created by
-# client.
-ACL ALLOW-LOG server CREATE QUEUE name="example.RequestQueue"
-ACL ALLOW-LOG server CONSUME QUEUE name="example.RequestQueue"
-ACL ALLOW-LOG server BIND EXCHANGE
-ACL ALLOW-LOG server PUBLISH EXCHANGE name="amq.direct" routingKey="TempQueue*"
+# To use web management, first give the user/group ACCESS MANAGEMENT permission
+ACL ALLOW webadmins ACCESS MANAGEMENT
 
 # ACL for web management console admins
 # All rules below are required for console admin users
@@ -94,6 +77,34 @@ ACL ALLOW-LOG webadmins UPDATE METHOD
 #ACL ALLOW-LOG webadmins UPDATE METHOD component="VirtualHost.Queue" name="copyMessages"
 #ACL ALLOW-LOG webadmins UPDATE METHOD component="VirtualHost.Queue" name="deleteMessages"
 
+### MESSAGING ###
+
+#Example permissions for request-response based messaging.
+
+#Allow 'messaging-users' group to connect to all virtualhosts
+ACL ALLOW-LOG messaging-users ACCESS VIRTUALHOST
+# Deny messaging-users management
+ACL DENY-LOG messaging-users ACCESS MANAGEMENT
+
+
+# Client side
+# Allow the 'client' user to publish requests to the request queue and create, consume from, and delete temporary reply queues.
+ACL ALLOW-LOG client CREATE QUEUE temporary="true"
+ACL ALLOW-LOG client CONSUME QUEUE temporary="true"
+ACL ALLOW-LOG client DELETE QUEUE temporary="true"
+ACL ALLOW-LOG client BIND EXCHANGE name="amq.direct" temporary="true"
+ACL ALLOW-LOG client UNBIND EXCHANGE name="amq.direct" temporary="true"
+ACL ALLOW-LOG client PUBLISH EXCHANGE name="amq.direct" routingKey="example.RequestQueue"
+
+# Server side
+# Allow the 'server' user to create and consume from the request queue and publish a response to the temporary response queue created by
+# client.
+ACL ALLOW-LOG server CREATE QUEUE name="example.RequestQueue"
+ACL ALLOW-LOG server CONSUME QUEUE name="example.RequestQueue"
+ACL ALLOW-LOG server BIND EXCHANGE
+ACL ALLOW-LOG server PUBLISH EXCHANGE name="amq.direct" routingKey="TempQueue*"
+
+
 ### DEFAULT ###
 
 # Deny all users from performing all operations
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/SecurityManager.java b/java/broker/src/main/java/org/apache/qpid/server/security/SecurityManager.java
index ce0ea2faea..1e377be1d2 100755
--- a/java/broker/src/main/java/org/apache/qpid/server/security/SecurityManager.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/SecurityManager.java
@@ -29,6 +29,7 @@ import org.apache.qpid.server.exchange.Exchange;
 import org.apache.qpid.server.plugins.PluginManager;
 import org.apache.qpid.server.queue.AMQQueue;
 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 static org.apache.qpid.server.security.access.ObjectType.EXCHANGE;
@@ -359,6 +360,17 @@ public class SecurityManager
         });
     }
 
+    public boolean accessManagement()
+    {
+        return checkAllPlugins(new AccessCheck()
+        {
+            Result allowed(SecurityPlugin plugin)
+            {
+                return plugin.access(ObjectType.MANAGEMENT, null);
+            }
+        });
+    }
+
     public boolean accessVirtualhost(final String vhostname, final SocketAddress remoteAddress)
     {
         return checkAllPlugins(new AccessCheck()
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/access/ObjectType.java b/java/broker/src/main/java/org/apache/qpid/server/security/access/ObjectType.java
index 043d4909d5..8bc4b9d278 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/access/ObjectType.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/access/ObjectType.java
@@ -41,6 +41,7 @@ public enum ObjectType
 {
     ALL(Operation.ALL),
     VIRTUALHOST(Operation.ALL, ACCESS),
+    MANAGEMENT(Operation.ALL, ACCESS),
     QUEUE(Operation.ALL, CREATE, DELETE, PURGE, CONSUME),
     EXCHANGE(Operation.ALL, ACCESS, CREATE, DELETE, BIND, UNBIND, PUBLISH),
     LINK, // Not allowed in the Java broker
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/auth/AuthenticatedPrincipal.java b/java/broker/src/main/java/org/apache/qpid/server/security/auth/AuthenticatedPrincipal.java
index 96360e83e4..fb31132514 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/auth/AuthenticatedPrincipal.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/auth/AuthenticatedPrincipal.java
@@ -18,6 +18,7 @@
  */
 package org.apache.qpid.server.security.auth;
 
+import java.io.Serializable;
 import java.security.Principal;
 import java.util.Set;
 
@@ -30,7 +31,7 @@ import org.apache.qpid.server.security.auth.UsernamePrincipal;
  * by calling {@link Subject#getPrincipals(Class)}, passing in {@link AuthenticatedPrincipal}.class,
  * e.g. when logging.
  */
-public final class AuthenticatedPrincipal implements Principal
+public final class AuthenticatedPrincipal implements Principal, Serializable
 {
     private final Principal _wrappedPrincipal;
 
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/auth/UsernamePrincipal.java b/java/broker/src/main/java/org/apache/qpid/server/security/auth/UsernamePrincipal.java
index cc414f801a..5b3c1d59cf 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/auth/UsernamePrincipal.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/auth/UsernamePrincipal.java
@@ -20,10 +20,11 @@
  */
 package org.apache.qpid.server.security.auth;
 
+import java.io.Serializable;
 import java.security.Principal;
 
 /** A principal that is just a wrapper for a simple username. */
-public class UsernamePrincipal implements Principal
+public class UsernamePrincipal implements Principal, Serializable
 {
     private final String _name;
 
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticator.java b/java/broker/src/main/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticator.java
index 808447b7ff..9f5d87bf62 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticator.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticator.java
@@ -22,7 +22,8 @@ package org.apache.qpid.server.security.auth.rmi;
 
 import java.net.SocketAddress;
 
-import org.apache.qpid.server.registry.ApplicationRegistry;
+import org.apache.qpid.server.registry.IApplicationRegistry;
+import org.apache.qpid.server.security.SecurityManager;
 import org.apache.qpid.server.security.SubjectCreator;
 import org.apache.qpid.server.security.auth.AuthenticationResult.AuthenticationStatus;
 import org.apache.qpid.server.security.auth.SubjectAuthenticationResult;
@@ -37,23 +38,33 @@ public class RMIPasswordAuthenticator implements JMXAuthenticator
     static final String SHOULD_HAVE_2_ELEMENTS = "User details should have 2 elements, username, password";
     static final String SHOULD_BE_NON_NULL = "Supplied username and password should be non-null";
     static final String INVALID_CREDENTIALS = "Invalid user details supplied";
+    static final String USER_NOT_AUTHORISED_FOR_MANAGEMENT = "User not authorised for management";
     static final String CREDENTIALS_REQUIRED = "User details are required. " +
-    		            "Please ensure you are using an up to date management console to connect.";
+                        "Please ensure you are using an up to date management console to connect.";
 
-    private SubjectCreator _subjectCreator = null;
-    private SocketAddress _socketAddress;
+    private final IApplicationRegistry _appRegistry;
+    private final SocketAddress _socketAddress;
 
-    public RMIPasswordAuthenticator(SocketAddress socketAddress)
+    public RMIPasswordAuthenticator(IApplicationRegistry appRegistry, SocketAddress socketAddress)
     {
+        _appRegistry = appRegistry;
         _socketAddress = socketAddress;
     }
 
-    public void setSubjectCreator(final SubjectCreator subjectCreator)
+    public Subject authenticate(Object credentials) throws SecurityException
     {
-        _subjectCreator = subjectCreator;
+        validateCredentials(credentials);
+
+        final String[] userCredentials = (String[]) credentials;
+        final String username = (String) userCredentials[0];
+        final String password = (String) userCredentials[1];
+
+        final Subject authenticatedSubject = doAuthentication(username, password);
+        doManagementAuthorisation(authenticatedSubject);
+        return authenticatedSubject;
     }
 
-    public Subject authenticate(Object credentials) throws SecurityException
+    private void validateCredentials(Object credentials)
     {
         // Verify that credential's are of type String[].
         if (!(credentials instanceof String[]))
@@ -69,41 +80,27 @@ public class RMIPasswordAuthenticator implements JMXAuthenticator
         }
 
         // Verify that required number of credentials.
-        final String[] userCredentials = (String[]) credentials;
-        if (userCredentials.length != 2)
+        if (((String[])credentials).length != 2)
         {
             throw new SecurityException(SHOULD_HAVE_2_ELEMENTS);
         }
+    }
 
-        final String username = (String) userCredentials[0];
-        final String password = (String) userCredentials[1];
-
+    private Subject doAuthentication(final String username, final String password)
+    {
         // Verify that all required credentials are actually present.
         if (username == null || password == null)
         {
             throw new SecurityException(SHOULD_BE_NON_NULL);
         }
 
-        // Verify that an SubjectCreator has been set.
-        if (_subjectCreator == null)
+        SubjectCreator subjectCreator = _appRegistry.getSubjectCreator(_socketAddress);
+        if (subjectCreator == null)
         {
-            try
-            {
-                if(ApplicationRegistry.getInstance().getSubjectCreator(_socketAddress) != null)
-                {
-                    _subjectCreator = ApplicationRegistry.getInstance().getSubjectCreator(_socketAddress);
-                }
-                else
-                {
-                    throw new SecurityException(UNABLE_TO_LOOKUP);
-                }
-            }
-            catch(IllegalStateException e)
-            {
-                throw new SecurityException(UNABLE_TO_LOOKUP);
-            }
+            throw new SecurityException("Can't get subject creator for " + _socketAddress);
         }
-        final SubjectAuthenticationResult result = _subjectCreator.authenticate(username, password);
+
+        final SubjectAuthenticationResult result = subjectCreator.authenticate(username, password);
 
         if (AuthenticationStatus.ERROR.equals(result.getStatus()))
         {
@@ -119,4 +116,21 @@ public class RMIPasswordAuthenticator implements JMXAuthenticator
         }
     }
 
+    private void doManagementAuthorisation(Subject authenticatedSubject)
+    {
+        SecurityManager.setThreadSubject(authenticatedSubject);
+        try
+        {
+            if (!_appRegistry.getSecurityManager().accessManagement())
+            {
+                throw new SecurityException(USER_NOT_AUTHORISED_FOR_MANAGEMENT);
+            }
+        }
+        finally
+        {
+            SecurityManager.setThreadSubject(null);
+        }
+    }
+
+
 }
\ No newline at end of file
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/group/GroupPrincipal.java b/java/broker/src/main/java/org/apache/qpid/server/security/group/GroupPrincipal.java
index ccb446b719..a9590bb964 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/group/GroupPrincipal.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/group/GroupPrincipal.java
@@ -20,6 +20,7 @@
  */
 package org.apache.qpid.server.security.group;
 
+import java.io.Serializable;
 import java.security.Principal;
 import java.security.acl.Group;
 import java.util.Enumeration;
@@ -30,7 +31,7 @@ import java.util.Enumeration;
  * methods etc throw {@link UnsupportedOperationException}.
  *
  */
-public class GroupPrincipal implements Group
+public class GroupPrincipal implements Group, Serializable
 {
     /** Name of the group */
     private final String _groupName;
diff --git a/java/broker/src/test/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticatorTest.java b/java/broker/src/test/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticatorTest.java
index efdb286866..89580dc392 100644
--- a/java/broker/src/test/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticatorTest.java
+++ b/java/broker/src/test/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticatorTest.java
@@ -31,10 +31,12 @@ import javax.security.auth.Subject;
 
 import junit.framework.TestCase;
 
+import org.apache.qpid.server.registry.ApplicationRegistry;
 import org.apache.qpid.server.security.SubjectCreator;
 import org.apache.qpid.server.security.auth.AuthenticationResult;
 import org.apache.qpid.server.security.auth.AuthenticationResult.AuthenticationStatus;
 import org.apache.qpid.server.security.auth.SubjectAuthenticationResult;
+import org.apache.qpid.server.security.SecurityManager;
 
 /**
  * Tests the RMIPasswordAuthenticator and its collaboration with the AuthenticationManager.
@@ -42,17 +44,25 @@ import org.apache.qpid.server.security.auth.SubjectAuthenticationResult;
  */
 public class RMIPasswordAuthenticatorTest extends TestCase
 {
-    private static final Subject SUBJECT = new Subject();
-    private final String USERNAME = "guest";
-    private final String PASSWORD = "guest";
+    private static final String USERNAME = "guest";
+    private static final String PASSWORD = "password";
+
+    private final ApplicationRegistry _applicationRegistry = mock(ApplicationRegistry.class);
+    private final SecurityManager _securityManager = mock(SecurityManager.class);
+    private final InetSocketAddress _jmxSocketAddress = new InetSocketAddress(8999);
+    private final Subject _loginSubject = new Subject();
+    private final String[] _credentials = new String[] {USERNAME, PASSWORD};
+
     private RMIPasswordAuthenticator _rmipa;
-    private String[] _credentials;
+
+    private SubjectCreator _usernamePasswordOkaySuvjectCreator = createMockSubjectCreator(true, null);
+    private SubjectCreator _badPasswordSubjectCreator = createMockSubjectCreator(false, null);
 
     protected void setUp() throws Exception
     {
-        _rmipa = new RMIPasswordAuthenticator(new InetSocketAddress(5672));
+        _rmipa = new RMIPasswordAuthenticator(_applicationRegistry, _jmxSocketAddress);
 
-        _credentials = new String[] {USERNAME, PASSWORD};
+        when(_applicationRegistry.getSecurityManager()).thenReturn(_securityManager);
     }
 
     /**
@@ -60,10 +70,11 @@ public class RMIPasswordAuthenticatorTest extends TestCase
      */
     public void testAuthenticationSuccess()
     {
-        _rmipa.setSubjectCreator(createMockSubjectCreator(true, null));
+        when(_applicationRegistry.getSubjectCreator(_jmxSocketAddress)).thenReturn(_usernamePasswordOkaySuvjectCreator);
+        when(_securityManager.accessManagement()).thenReturn(true);
 
         Subject newSubject = _rmipa.authenticate(_credentials);
-        assertSame("Subject must be unchanged", SUBJECT, newSubject);
+        assertSame("Subject must be unchanged", _loginSubject, newSubject);
     }
 
     /**
@@ -71,7 +82,7 @@ public class RMIPasswordAuthenticatorTest extends TestCase
      */
     public void testUsernameOrPasswordInvalid()
     {
-        _rmipa.setSubjectCreator(createMockSubjectCreator(false, null));
+        when(_applicationRegistry.getSubjectCreator(_jmxSocketAddress)).thenReturn(_badPasswordSubjectCreator);
 
         try
         {
@@ -82,17 +93,31 @@ public class RMIPasswordAuthenticatorTest extends TestCase
         {
             assertEquals("Unexpected exception message",
                     RMIPasswordAuthenticator.INVALID_CREDENTIALS, se.getMessage());
+        }
+    }
+
+    public void testAuthorisationFailure()
+    {
+        when(_applicationRegistry.getSubjectCreator(_jmxSocketAddress)).thenReturn(_usernamePasswordOkaySuvjectCreator);
+        when(_securityManager.accessManagement()).thenReturn(false);
 
+        try
+        {
+            _rmipa.authenticate(_credentials);
+            fail("Exception not thrown");
+        }
+        catch (SecurityException se)
+        {
+            assertEquals("Unexpected exception message",
+                    RMIPasswordAuthenticator.USER_NOT_AUTHORISED_FOR_MANAGEMENT, se.getMessage());
         }
     }
 
-    /**
-     * Tests case where authentication system itself fails.
-     */
-    public void testAuthenticationFailure()
+    public void testSubjectCreatorInternalFailure()
     {
         final Exception mockAuthException = new Exception("Mock Auth system failure");
-        _rmipa.setSubjectCreator(createMockSubjectCreator(false, mockAuthException));
+        SubjectCreator subjectCreator = createMockSubjectCreator(false, mockAuthException);
+        when(_applicationRegistry.getSubjectCreator(_jmxSocketAddress)).thenReturn(subjectCreator);
 
         try
         {
@@ -105,13 +130,13 @@ public class RMIPasswordAuthenticatorTest extends TestCase
         }
     }
 
-
     /**
      * Tests case where authentication manager is not set.
      */
-    public void testNullAuthenticationManager() throws Exception
+    public void testNullSubjectCreator() throws Exception
     {
-        _rmipa.setSubjectCreator(null);
+        when(_applicationRegistry.getSubjectCreator(_jmxSocketAddress)).thenReturn(null);
+
         try
         {
             _rmipa.authenticate(_credentials);
@@ -120,7 +145,7 @@ public class RMIPasswordAuthenticatorTest extends TestCase
         catch (SecurityException se)
         {
             assertEquals("Unexpected exception message",
-                    RMIPasswordAuthenticator.UNABLE_TO_LOOKUP, se.getMessage());
+                    "Can't get subject creator for 0.0.0.0/0.0.0.0:8999", se.getMessage());
         }
     }
 
@@ -148,11 +173,13 @@ public class RMIPasswordAuthenticatorTest extends TestCase
      */
     public void testWithIllegalNumberOfArguments()
     {
+        String[] credentials;
+
         // Test handling of incorrect number of credentials
         try
         {
-            _credentials = new String[]{USERNAME, PASSWORD, PASSWORD};
-            _rmipa.authenticate(_credentials);
+            credentials = new String[]{USERNAME, PASSWORD, PASSWORD};
+            _rmipa.authenticate(credentials);
             fail("SecurityException expected due to supplying wrong number of credentials");
         }
         catch (SecurityException se)
@@ -165,8 +192,8 @@ public class RMIPasswordAuthenticatorTest extends TestCase
         try
         {
             //send a null array
-            _credentials = null;
-            _rmipa.authenticate(_credentials);
+            credentials = null;
+            _rmipa.authenticate(credentials);
             fail("SecurityException expected due to not supplying an array of credentials");
         }
         catch (SecurityException se)
@@ -178,8 +205,8 @@ public class RMIPasswordAuthenticatorTest extends TestCase
         try
         {
             //send a null password
-            _credentials = new String[]{USERNAME, null};
-            _rmipa.authenticate(_credentials);
+            credentials = new String[]{USERNAME, null};
+            _rmipa.authenticate(credentials);
             fail("SecurityException expected due to sending a null password");
         }
         catch (SecurityException se)
@@ -191,8 +218,8 @@ public class RMIPasswordAuthenticatorTest extends TestCase
         try
         {
             //send a null username
-            _credentials = new String[]{null, PASSWORD};
-            _rmipa.authenticate(_credentials);
+            credentials = new String[]{null, PASSWORD};
+            _rmipa.authenticate(credentials);
             fail("SecurityException expected due to sending a null username");
         }
         catch (SecurityException se)
@@ -217,7 +244,7 @@ public class RMIPasswordAuthenticatorTest extends TestCase
         {
 
             subjectAuthenticationResult = new SubjectAuthenticationResult(
-                    new AuthenticationResult(mock(Principal.class)), SUBJECT);
+                    new AuthenticationResult(mock(Principal.class)), _loginSubject);
         }
         else
         {