diff --git a/activemq-core/src/main/java/org/apache/activemq/broker/TransportConnection.java b/activemq-core/src/main/java/org/apache/activemq/broker/TransportConnection.java index 1eeb398c06..0d257daafe 100755 --- a/activemq-core/src/main/java/org/apache/activemq/broker/TransportConnection.java +++ b/activemq-core/src/main/java/org/apache/activemq/broker/TransportConnection.java @@ -694,8 +694,11 @@ public class TransportConnection implements Connection, Task, CommandVisitor { try { broker.addConnection(context, info); } catch (Exception e) { - brokerConnectionStates.remove(info); - LOG.warn("Failed to add Connection, reason: " + e.toString()); + synchronized (brokerConnectionStates) { + brokerConnectionStates.remove(info.getConnectionId()); + } + unregisterConnectionState(info.getConnectionId()); + LOG.warn("Failed to add Connection " + info.getConnectionId() + ", reason: " + e.toString()); if (LOG.isDebugEnabled()) { LOG.debug("Exception detail:", e); } @@ -741,7 +744,10 @@ public class TransportConnection implements Connection, Task, CommandVisitor { try { broker.removeConnection(cs.getContext(), cs.getInfo(), null); } catch (Throwable e) { - SERVICELOG.warn("Failed to remove connection " + cs.getInfo(), e); + SERVICELOG.warn("Failed to remove connection " + cs.getInfo() + ", reason: " + e.toString()); + if (LOG.isDebugEnabled()) { + SERVICELOG.debug("Exception detail:", e); + } } TransportConnectionState state = unregisterConnectionState(id); if (state != null) { diff --git a/activemq-core/src/test/java/org/apache/activemq/security/XBeanSecurityWithGuestNoCredentialsOnlyTest.java b/activemq-core/src/test/java/org/apache/activemq/security/XBeanSecurityWithGuestNoCredentialsOnlyTest.java new file mode 100644 index 0000000000..14b6dccccf --- /dev/null +++ b/activemq-core/src/test/java/org/apache/activemq/security/XBeanSecurityWithGuestNoCredentialsOnlyTest.java @@ -0,0 +1,139 @@ +/** + * 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.activemq.security; + +import java.net.URI; +import javax.jms.Connection; +import javax.jms.JMSException; +import javax.jms.Message; +import javax.jms.MessageConsumer; +import javax.jms.Session; +import javax.jms.TextMessage; +import junit.framework.Test; +import org.apache.activemq.ActiveMQConnection; +import org.apache.activemq.CombinationTestSupport; +import org.apache.activemq.JmsTestSupport; +import org.apache.activemq.broker.BrokerFactory; +import org.apache.activemq.broker.BrokerService; +import org.apache.activemq.command.ActiveMQDestination; +import org.apache.activemq.command.ActiveMQMessage; +import org.apache.activemq.command.ActiveMQQueue; +import org.apache.activemq.command.ActiveMQTopic; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class XBeanSecurityWithGuestNoCredentialsOnlyTest extends JmsTestSupport { + + private static final Logger LOG = LoggerFactory.getLogger(XBeanSecurityWithGuestNoCredentialsOnlyTest.class); + public ActiveMQDestination destination; + + public static Test suite() { + return suite(XBeanSecurityWithGuestNoCredentialsOnlyTest.class); + } + + public void testUserSendGoodPassword() throws JMSException { + Message m = doSend(false); + assertEquals("system", ((ActiveMQMessage)m).getUserID()); + assertEquals("system", m.getStringProperty("JMSXUserID")); + } + + public void testUserSendWrongPassword() throws JMSException { + try { + doSend(true); + fail("expect exception on connect"); + } catch (JMSException expected) { + assertTrue("cause as expected", expected.getCause() instanceof SecurityException); + } + } + + public void testUserSendNoCredentials() throws JMSException { + Message m = doSend(false); + // note brokerService.useAuthenticatedPrincipalForJMXUserID=true for this + assertEquals("guest", ((ActiveMQMessage)m).getUserID()); + assertEquals("guest", m.getStringProperty("JMSXUserID")); + } + + protected BrokerService createBroker() throws Exception { + return createBroker("org/apache/activemq/security/jaas-broker-guest-no-creds-only.xml"); + } + + protected BrokerService createBroker(String uri) throws Exception { + LOG.info("Loading broker configuration from the classpath with URI: " + uri); + return BrokerFactory.createBroker(new URI("xbean:" + uri)); + } + + public Message doSend(boolean fail) throws JMSException { + + Connection adminConnection = factory.createConnection("system", "manager"); + connections.add(adminConnection); + + adminConnection.start(); + Session adminSession = adminConnection.createSession(false, Session.AUTO_ACKNOWLEDGE); + MessageConsumer consumer = adminSession.createConsumer(destination); + + connections.remove(connection); + connection = (ActiveMQConnection)factory.createConnection(userName, password); + connections.add(connection); + + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + try { + sendMessages(session, destination, 1); + } catch (JMSException e) { + // If test is expected to fail, the cause must only be a + // SecurityException + // otherwise rethrow the exception + if (!fail || !(e.getCause() instanceof SecurityException)) { + throw e; + } + } + + Message m = consumer.receive(1000); + if (fail) { + assertNull(m); + } else { + assertNotNull(m); + assertEquals("0", ((TextMessage)m).getText()); + assertNull(consumer.receiveNoWait()); + } + return m; + } + + /** + * @see {@link CombinationTestSupport} + */ + public void initCombosForTestUserSendGoodPassword() { + addCombinationValues("userName", new Object[] {"system"}); + addCombinationValues("password", new Object[] {"manager"}); + addCombinationValues("destination", new Object[] {new ActiveMQQueue("test"), new ActiveMQTopic("test")}); + } + + /** + * @see {@link CombinationTestSupport} + */ + public void initCombosForTestUserSendWrongPassword() { + addCombinationValues("userName", new Object[] {"system"}); + addCombinationValues("password", new Object[] {"wrongpassword"}); + addCombinationValues("destination", new Object[] {new ActiveMQQueue("GuestQueue")}); + } + + public void initCombosForTestUserSendNoCredentials() { + addCombinationValues("userName", new Object[] {null, "system"}); + addCombinationValues("password", new Object[] {null}); + addCombinationValues("destination", new Object[] {new ActiveMQQueue("GuestQueue")}); + } + +} diff --git a/activemq-core/src/test/java/org/apache/activemq/security/XBeanSecurityWithGuestTest.java b/activemq-core/src/test/java/org/apache/activemq/security/XBeanSecurityWithGuestTest.java index 114517bc07..16f95b49a8 100644 --- a/activemq-core/src/test/java/org/apache/activemq/security/XBeanSecurityWithGuestTest.java +++ b/activemq-core/src/test/java/org/apache/activemq/security/XBeanSecurityWithGuestTest.java @@ -18,7 +18,6 @@ package org.apache.activemq.security; import java.net.URI; import javax.jms.Connection; -import javax.jms.ConnectionFactory; import javax.jms.JMSException; import javax.jms.Message; import javax.jms.MessageConsumer; @@ -26,7 +25,6 @@ import javax.jms.Session; import javax.jms.TextMessage; import junit.framework.Test; import org.apache.activemq.ActiveMQConnection; -import org.apache.activemq.ActiveMQConnectionFactory; import org.apache.activemq.CombinationTestSupport; import org.apache.activemq.JmsTestSupport; import org.apache.activemq.broker.BrokerFactory; @@ -60,6 +58,13 @@ public class XBeanSecurityWithGuestTest extends JmsTestSupport { assertEquals("guest", m.getStringProperty("JMSXUserID")); } + public void testUserSendNoCredentials() throws JMSException { + Message m = doSend(false); + // note brokerService.useAuthenticatedPrincipalForJMXUserID=true for this + assertEquals("guest", ((ActiveMQMessage)m).getUserID()); + assertEquals("guest", m.getStringProperty("JMSXUserID")); + } + protected BrokerService createBroker() throws Exception { return createBroker("org/apache/activemq/security/jaas-broker-guest.xml"); } @@ -122,4 +127,11 @@ public class XBeanSecurityWithGuestTest extends JmsTestSupport { addCombinationValues("password", new Object[] {"wrongpassword"}); addCombinationValues("destination", new Object[] {new ActiveMQQueue("GuestQueue")}); } + + public void initCombosForTestUserSendNoCredentials() { + addCombinationValues("userName", new Object[] {"", null}); + addCombinationValues("password", new Object[] {"", null}); + addCombinationValues("destination", new Object[] {new ActiveMQQueue("GuestQueue")}); + } + } diff --git a/activemq-core/src/test/resources/login.config b/activemq-core/src/test/resources/login.config index b8a008e7ea..6f669f738b 100644 --- a/activemq-core/src/test/resources/login.config +++ b/activemq-core/src/test/resources/login.config @@ -32,6 +32,19 @@ activemq-guest-domain { org.apache.activemq.jaas.guest.group="guests"; }; +activemq-guest-when-no-creds-only-domain { + org.apache.activemq.jaas.GuestLoginModule sufficient + debug=true + credentialsInvalidate=true + org.apache.activemq.jaas.guest.user="guest" + org.apache.activemq.jaas.guest.group="guests"; + + org.apache.activemq.jaas.PropertiesLoginModule requisite + debug=true + org.apache.activemq.jaas.properties.user="org/apache/activemq/security/users.properties" + org.apache.activemq.jaas.properties.group="org/apache/activemq/security/groups.properties"; +}; + cert-login { org.apache.activemq.jaas.TextFileCertificateLoginModule required debug=true diff --git a/activemq-core/src/test/resources/org/apache/activemq/security/jaas-broker-guest-no-creds-only.xml b/activemq-core/src/test/resources/org/apache/activemq/security/jaas-broker-guest-no-creds-only.xml new file mode 100644 index 0000000000..eeb568d9c4 --- /dev/null +++ b/activemq-core/src/test/resources/org/apache/activemq/security/jaas-broker-guest-no-creds-only.xml @@ -0,0 +1,56 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/activemq-jaas/src/main/java/org/apache/activemq/jaas/GuestLoginModule.java b/activemq-jaas/src/main/java/org/apache/activemq/jaas/GuestLoginModule.java index 394f91b422..c3eaed6561 100644 --- a/activemq-jaas/src/main/java/org/apache/activemq/jaas/GuestLoginModule.java +++ b/activemq-jaas/src/main/java/org/apache/activemq/jaas/GuestLoginModule.java @@ -17,17 +17,20 @@ package org.apache.activemq.jaas; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import javax.security.auth.Subject; -import javax.security.auth.callback.CallbackHandler; -import javax.security.auth.login.LoginException; -import javax.security.auth.spi.LoginModule; +import java.io.IOException; import java.security.Principal; import java.util.HashSet; import java.util.Map; import java.util.Set; +import javax.security.auth.Subject; +import javax.security.auth.callback.Callback; +import javax.security.auth.callback.CallbackHandler; +import javax.security.auth.callback.PasswordCallback; +import javax.security.auth.callback.UnsupportedCallbackException; +import javax.security.auth.login.LoginException; +import javax.security.auth.spi.LoginModule; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Always login the user with a default 'guest' identity. @@ -48,13 +51,17 @@ public class GuestLoginModule implements LoginModule { private String groupName = "guests"; private Subject subject; private boolean debug; + private boolean credentialsInvalidate; private Set principals = new HashSet(); + private CallbackHandler callbackHandler; + private boolean loginSucceeded; public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) { this.subject = subject; - + this.callbackHandler = callbackHandler; debug = "true".equalsIgnoreCase((String)options.get("debug")); + credentialsInvalidate = "true".equalsIgnoreCase((String)options.get("credentialsInvalidate")); if (options.get(GUEST_USER) != null) { userName = (String)options.get(GUEST_USER); } @@ -71,19 +78,37 @@ public class GuestLoginModule implements LoginModule { } public boolean login() throws LoginException { - + loginSucceeded = true; + if (credentialsInvalidate) { + PasswordCallback passwordCallback = new PasswordCallback("Password: ", false); + try { + callbackHandler.handle(new Callback[]{passwordCallback}); + if (passwordCallback.getPassword() != null) { + if (debug) { + LOG.debug("Guest login failing (credentialsInvalidate=true) on presence of a password"); + } + loginSucceeded = false; + passwordCallback.clearPassword(); + }; + } catch (IOException ioe) { + } catch (UnsupportedCallbackException uce) { + } + } if (debug) { - LOG.debug("login " + userName); - }return true; + LOG.debug("Guest login " + loginSucceeded); + } + return loginSucceeded; } public boolean commit() throws LoginException { - subject.getPrincipals().addAll(principals); + if (loginSucceeded) { + subject.getPrincipals().addAll(principals); + } if (debug) { LOG.debug("commit"); } - return true; + return loginSucceeded; } public boolean abort() throws LoginException { @@ -91,7 +116,8 @@ public class GuestLoginModule implements LoginModule { if (debug) { LOG.debug("abort"); } - return true; } + return true; + } public boolean logout() throws LoginException { subject.getPrincipals().removeAll(principals);