https://issues.apache.org/jira/browse/AMQ-3198 - Allow JAAS GuestLoginModule to fail if users specify a password

new option to GuestLoginModule, credentialsInvalidate, when true, presence of a password will cause module to fail login
allowing the next module to validate the credential. Will only guest users who don't specify a password

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

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);
+            synchronized (brokerConnectionStates) {
-            LOG.warn("Failed to add Connection, reason: " +  e.toString());
+                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) {

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")});
+    }
+
+}

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")});
+    }
+
 }

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

activemq-core/src/test/resources/org/apache/activemq/security/jaas-broker-guest-no-creds-only.xml

@@ -0,0 +1,56 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  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.
+-->
+
+<beans
+  xmlns="http://www.springframework.org/schema/beans"
+  xmlns:amq="http://activemq.apache.org/schema/core"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-2.0.xsd
+  http://activemq.apache.org/schema/core http://activemq.apache.org/schema/core/activemq-core.xsd">
+
+  <bean class="org.springframework.beans.factory.config.PropertyPlaceholderConfigurer"/>
+
+  <broker useJmx="false" persistent="false" xmlns="http://activemq.apache.org/schema/core"
+          populateJMSXUserID="true"
+          useAuthenticatedPrincipalForJMXUserID="true">
+
+    <plugins>
+      <!--  use JAAS to authenticate using the login.config file on the classpath to configure JAAS -->
+      <jaasDualAuthenticationPlugin configuration="activemq-guest-when-no-creds-only-domain" sslConfiguration="cert-login" />
+
+      <!--  lets configure a destination based authorization mechanism -->
+      <authorizationPlugin>
+        <map>
+          <authorizationMap>
+            <authorizationEntries>
+             <authorizationEntry queue="&gt;" read="admins" write="admins" admin="admins"/>
+             <authorizationEntry topic="&gt;" read="admins" write="admins" admin="admins"/>
+             <authorizationEntry queue="GuestQueue" read="admins" write="admins, guests" admin="admins"/>
+             <authorizationEntry topic="ActiveMQ.Advisory.&gt;" read="guests" write="guests" admin="guests"/>
+            </authorizationEntries>    
+          </authorizationMap>
+        </map>
+      </authorizationPlugin>
+    </plugins>
+    
+    <transportConnectors>
+		<transportConnector name="stomp"   uri="stomp://localhost:61613"/>
+	</transportConnectors>
+  </broker>
+
+</beans>

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 java.io.IOException;
-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.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<Principal> principals = new HashSet<Principal>();
+    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);
+            LOG.debug("Guest login " + loginSucceeded);
-        }return true;
+        }
+        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);