From ff7167be41036edf73fc4725591f8a0ad2a45f29 Mon Sep 17 00:00:00 2001 From: Robert Gemmell Date: Thu, 12 May 2011 12:10:52 +0000 Subject: QPID-3249: Remove incomplete support for authentication at virtualhost level. Applied patch from Keith Wall git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1102258 13f79535-47bb-0310-9956-ffa450edef68 --- .../configuration/VirtualHostConfiguration.java | 8 +- .../qpid/server/registry/ApplicationRegistry.java | 2 +- .../auth/manager/AuthenticationManager.java | 4 + .../PrincipalDatabaseAuthenticationManager.java | 83 ++------ .../qpid/server/virtualhost/VirtualHostImpl.java | 4 +- .../VirtualHostConfigurationTest.java | 26 +++ ...PrincipalDatabaseAuthenticationManagerTest.java | 209 +++++++++++++++++++++ 7 files changed, 264 insertions(+), 72 deletions(-) create mode 100644 qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManagerTest.java diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java index 48f2d776bb..a710230616 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java @@ -306,7 +306,13 @@ public class VirtualHostConfiguration extends ConfigurationPlugin @Override public void validateConfiguration() throws ConfigurationException { - //Currently doesn't do validation + // QPID-3249. Support for specifying authentication name at vhost level is no longer supported. + if (getListValue("security.authentication.name").size() > 0) + { + String message = "Validation error : security/authentication/name is no longer a supported element within the configuration xml." + + " It appears in virtual host definition : " + _name; + throw new ConfigurationException(message); + } } public int getHouseKeepingThreadCount() diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java index 72b2a68450..b6df0cc0a6 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java @@ -287,7 +287,7 @@ public abstract class ApplicationRegistry implements IApplicationRegistry createDatabaseManager(_configuration); - _authenticationManager = new PrincipalDatabaseAuthenticationManager(null, null); + _authenticationManager = new PrincipalDatabaseAuthenticationManager(); _databaseManager.initialiseManagement(_configuration); diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/AuthenticationManager.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/AuthenticationManager.java index bc771162fd..39e1e07c57 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/AuthenticationManager.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/AuthenticationManager.java @@ -26,6 +26,10 @@ import javax.security.sasl.SaslServer; import org.apache.qpid.common.Closeable; import org.apache.qpid.server.security.auth.AuthenticationResult; +/** + * The AuthenticationManager class is the entity responsible for + * determining the authenticity of user credentials. + */ public interface AuthenticationManager extends Closeable { String getMechanisms(); diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java index 2a967f02af..d10ad2c170 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java @@ -21,9 +21,6 @@ package org.apache.qpid.server.security.auth.manager; import org.apache.log4j.Logger; -import org.apache.commons.configuration.Configuration; -import org.apache.commons.configuration.ConfigurationException; -import org.apache.qpid.server.configuration.VirtualHostConfiguration; import org.apache.qpid.server.registry.ApplicationRegistry; import org.apache.qpid.server.security.auth.manager.AuthenticationManager; import org.apache.qpid.server.security.auth.database.PrincipalDatabase; @@ -41,6 +38,11 @@ import java.util.HashMap; import java.util.TreeMap; import java.security.Security; +/** + * Concrete implementation of the AuthenticationManager that determines if supplied + * user credentials match those appearing in a PrincipalDatabase. + * + */ public class PrincipalDatabaseAuthenticationManager implements AuthenticationManager { private static final Logger _logger = Logger.getLogger(PrincipalDatabaseAuthenticationManager.class); @@ -57,47 +59,17 @@ public class PrincipalDatabaseAuthenticationManager implements AuthenticationMan */ private Map> _serverCreationProperties = new HashMap>(); - private AuthenticationManager _default = null; /** The name for the required SASL Server mechanisms */ public static final String PROVIDER_NAME= "AMQSASLProvider-Server"; - public PrincipalDatabaseAuthenticationManager(String name, VirtualHostConfiguration hostConfig) throws Exception + public PrincipalDatabaseAuthenticationManager() { - _logger.info("Initialising " + (name == null ? "Default" : "'" + name + "'") - + " PrincipalDatabase authentication manager."); - - // Fixme This should be done per Vhost but allowing global hack isn't right but ... - // required as authentication is done before Vhost selection + _logger.info("Initialising PrincipalDatabase authentication manager."); Map> providerMap = new TreeMap>(); - if (name == null || hostConfig == null) - { - initialiseAuthenticationMechanisms(providerMap, ApplicationRegistry.getInstance().getDatabaseManager().getDatabases()); - } - else - { - String databaseName = hostConfig.getAuthenticationDatabase(); - - if (databaseName == null) - { - - _default = ApplicationRegistry.getInstance().getAuthenticationManager(); - return; - } - else - { - PrincipalDatabase database = ApplicationRegistry.getInstance().getDatabaseManager().getDatabases().get(databaseName); - - if (database == null) - { - throw new ConfigurationException("Requested database:" + databaseName + " was not found"); - } - - initialiseAuthenticationMechanisms(providerMap, database); - } - } + initialiseAuthenticationMechanisms(providerMap, ApplicationRegistry.getInstance().getDatabaseManager().getDatabases()); if (providerMap.size() > 0) { @@ -116,11 +88,9 @@ public class PrincipalDatabaseAuthenticationManager implements AuthenticationMan { _logger.warn("No additional SASL providers registered."); } - } - - private void initialiseAuthenticationMechanisms(Map> providerMap, Map databases) throws Exception + private void initialiseAuthenticationMechanisms(Map> providerMap, Map databases) { if (databases.size() > 1) { @@ -136,7 +106,7 @@ public class PrincipalDatabaseAuthenticationManager implements AuthenticationMan } } - private void initialiseAuthenticationMechanisms(Map> providerMap, PrincipalDatabase database) throws Exception + private void initialiseAuthenticationMechanisms(Map> providerMap, PrincipalDatabase database) { if (database == null || database.getMechanisms().size() == 0) { @@ -152,7 +122,7 @@ public class PrincipalDatabaseAuthenticationManager implements AuthenticationMan private void initialiseAuthenticationMechanism(String mechanism, AuthenticationProviderInitialiser initialiser, Map> providerMap) - throws Exception + { if (_mechanisms == null) { @@ -175,41 +145,17 @@ public class PrincipalDatabaseAuthenticationManager implements AuthenticationMan public String getMechanisms() { - if (_default != null) - { - // Use the default AuthenticationManager if present - return _default.getMechanisms(); - } - else - { - return _mechanisms; - } + return _mechanisms; } public SaslServer createSaslServer(String mechanism, String localFQDN) throws SaslException { - if (_default != null) - { - // Use the default AuthenticationManager if present - return _default.createSaslServer(mechanism, localFQDN); - } - else - { - return Sasl.createSaslServer(mechanism, "AMQP", localFQDN, _serverCreationProperties.get(mechanism), - _callbackHandlerMap.get(mechanism)); - } - + return Sasl.createSaslServer(mechanism, "AMQP", localFQDN, _serverCreationProperties.get(mechanism), + _callbackHandlerMap.get(mechanism)); } public AuthenticationResult authenticate(SaslServer server, byte[] response) { - // Use the default AuthenticationManager if present - if (_default != null) - { - return _default.authenticate(server, response); - } - - try { // Process response from the client @@ -232,6 +178,7 @@ public class PrincipalDatabaseAuthenticationManager implements AuthenticationMan public void close() { + _mechanisms = null; Security.removeProvider(PROVIDER_NAME); } } diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java index 8ce4121b97..33c713c62a 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java @@ -101,7 +101,7 @@ public class VirtualHostImpl implements VirtualHost private AMQBrokerManagerMBean _brokerMBean; - private AuthenticationManager _authenticationManager; + private final AuthenticationManager _authenticationManager; private SecurityManager _securityManager; @@ -248,7 +248,7 @@ public class VirtualHostImpl implements VirtualHost initialiseMessageStore(hostConfig); } - _authenticationManager = new PrincipalDatabaseAuthenticationManager(_name, _configuration); + _authenticationManager = ApplicationRegistry.getInstance().getAuthenticationManager(); _brokerMBean = new AMQBrokerManagerMBean(_virtualHostMBean); _brokerMBean.register(); diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/VirtualHostConfigurationTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/VirtualHostConfigurationTest.java index 917755e8a5..593119041d 100644 --- a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/VirtualHostConfigurationTest.java +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/VirtualHostConfigurationTest.java @@ -20,6 +20,8 @@ package org.apache.qpid.server.configuration; +import org.apache.commons.configuration.ConfigurationException; +import org.apache.commons.configuration.XMLConfiguration; import org.apache.qpid.framing.AMQShortString; import org.apache.qpid.server.queue.AMQPriorityQueue; import org.apache.qpid.server.queue.AMQQueue; @@ -203,5 +205,29 @@ public class VirtualHostConfigurationTest extends InternalBrokerBaseCase } + /** + * Tests that the old element security.authentication.name is rejected. This element + * was never supported properly as authentication is performed before the virtual host + * is considered. + */ + public void testSecurityAuthenticationNameRejected() throws Exception + { + getConfigXml().addProperty("virtualhosts.virtualhost.testSecurityAuthenticationNameRejected.security.authentication.name", + "testdb"); + + try + { + super.createBroker(); + fail("Exception not thrown"); + } + catch(ConfigurationException ce) + { + assertEquals("Incorrect error message", + "Validation error : security/authentication/name is no longer a supported element within the configuration xml." + + " It appears in virtual host definition : " + getName(), + ce.getMessage()); + } + } + } diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManagerTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManagerTest.java new file mode 100644 index 0000000000..f51ce0b6c6 --- /dev/null +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManagerTest.java @@ -0,0 +1,209 @@ +/* + * + * 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.auth.manager; + +import java.security.Provider; +import java.security.Security; + +import javax.security.sasl.SaslException; +import javax.security.sasl.SaslServer; + +import org.apache.qpid.server.security.auth.AuthenticationResult; +import org.apache.qpid.server.security.auth.AuthenticationResult.AuthenticationStatus; +import org.apache.qpid.server.util.InternalBrokerBaseCase; + +/** + * + * Tests the public methods of PrincipalDatabaseAuthenticationManager. + * + */ +public class PrincipalDatabaseAuthenticationManagerTest extends InternalBrokerBaseCase +{ + private PrincipalDatabaseAuthenticationManager _manager = null; + + /** + * @see org.apache.qpid.server.util.InternalBrokerBaseCase#tearDown() + */ + @Override + public void tearDown() throws Exception + { + super.tearDown(); + if (_manager != null) + { + _manager.close(); + } + } + + /** + * @see org.apache.qpid.server.util.InternalBrokerBaseCase#setUp() + */ + @Override + public void setUp() throws Exception + { + super.setUp(); + + _manager = new PrincipalDatabaseAuthenticationManager(); + } + + /** + * Tests that the PDAM registers SASL mechanisms correctly with the runtime. + */ + public void testRegisteredMechanisms() throws Exception + { + assertNotNull(_manager.getMechanisms()); + // relies on those mechanisms attached to PropertiesPrincipalDatabaseManager + assertEquals("PLAIN CRAM-MD5", _manager.getMechanisms()); + + Provider qpidProvider = Security.getProvider(PrincipalDatabaseAuthenticationManager.PROVIDER_NAME); + assertNotNull(qpidProvider); + } + + /** + * Tests that the SASL factory method createSaslServer correctly + * returns a non-null implementation. + */ + public void testSaslMechanismCreation() throws Exception + { + SaslServer server = _manager.createSaslServer("CRAM-MD5", "localhost"); + assertNotNull(server); + // Merely tests the creation of the mechanism. Mechanisms themselves are tested + // by their own tests. + } + + /** + * + * Tests that the authenticate method correctly interprets an + * authentication success. + * + */ + public void testAuthenticationSuccess() throws Exception + { + SaslServer testServer = createTestSaslServer(true, false); + + AuthenticationResult result = _manager.authenticate(testServer, "12345".getBytes()); + assertEquals(AuthenticationStatus.SUCCESS, result.status); + } + + /** + * + * Tests that the authenticate method correctly interprets an + * authentication not complete. + * + */ + public void testAuthenticationNotCompleted() throws Exception + { + SaslServer testServer = createTestSaslServer(false, false); + + AuthenticationResult result = _manager.authenticate(testServer, "12345".getBytes()); + assertEquals(AuthenticationStatus.CONTINUE, result.status); + } + + /** + * + * Tests that the authenticate method correctly interprets an + * authentication error. + * + */ + public void testAuthenticationError() throws Exception + { + SaslServer testServer = createTestSaslServer(false, true); + + AuthenticationResult result = _manager.authenticate(testServer, "12345".getBytes()); + assertEquals(AuthenticationStatus.ERROR, result.status); + } + + /** + * Tests the ability to de-register the provider. + */ + public void testClose() throws Exception + { + assertEquals("PLAIN CRAM-MD5", _manager.getMechanisms()); + assertNotNull(Security.getProvider(PrincipalDatabaseAuthenticationManager.PROVIDER_NAME)); + + _manager.close(); + + // Check provider has been removed. + assertNull(_manager.getMechanisms()); + assertNull(Security.getProvider(PrincipalDatabaseAuthenticationManager.PROVIDER_NAME)); + _manager = null; + } + + /** + * Test SASL implementation used to test the authenticate() method. + */ + private SaslServer createTestSaslServer(final boolean complete, final boolean throwSaslException) + { + return new SaslServer() + { + + @Override + public String getMechanismName() + { + return null; + } + + @Override + public byte[] evaluateResponse(byte[] response) throws SaslException + { + if (throwSaslException) + { + throw new SaslException("Mocked exception"); + } + return null; + } + + @Override + public boolean isComplete() + { + return complete; + } + + @Override + public String getAuthorizationID() + { + return null; + } + + @Override + public byte[] unwrap(byte[] incoming, int offset, int len) throws SaslException + { + return null; + } + + @Override + public byte[] wrap(byte[] outgoing, int offset, int len) throws SaslException + { + return null; + } + + @Override + public Object getNegotiatedProperty(String propName) + { + return null; + } + + @Override + public void dispose() throws SaslException + { + } + }; + } +} -- cgit v1.2.1