You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by gt...@apache.org on 2012/07/31 15:54:41 UTC

svn commit: r1367553 - in /activemq/trunk/activemq-core/src: main/java/org/apache/activemq/ main/java/org/apache/activemq/broker/ main/java/org/apache/activemq/transport/ test/java/org/apache/activemq/security/ test/resources/org/apache/activemq/security/

Author: gtully
Date: Tue Jul 31 13:54:41 2012
New Revision: 1367553

URL: http://svn.apache.org/viewvc?rev=1367553&view=rev
Log:
https://issues.apache.org/jira/browse/AMQ-3294 - further refinement of dispose on security exception, now only on connection attempts. A security exception on an authenticated connection is reported back to the client and remains intact. Made use of inactivitymonitor w useKeepAlive=false to abort dos style hogging connections

Modified:
    activemq/trunk/activemq-core/src/main/java/org/apache/activemq/ActiveMQConnection.java
    activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/TransportConnection.java
    activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/AbstractInactivityMonitor.java
    activemq/trunk/activemq-core/src/test/java/org/apache/activemq/security/DoSTest.java
    activemq/trunk/activemq-core/src/test/resources/org/apache/activemq/security/dos-broker.xml

Modified: activemq/trunk/activemq-core/src/main/java/org/apache/activemq/ActiveMQConnection.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/ActiveMQConnection.java?rev=1367553&r1=1367552&r2=1367553&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/main/java/org/apache/activemq/ActiveMQConnection.java (original)
+++ activemq/trunk/activemq-core/src/main/java/org/apache/activemq/ActiveMQConnection.java Tue Jul 31 13:54:41 2012
@@ -1301,7 +1301,7 @@ public class ActiveMQConnection implemen
      * @return
      * @throws JMSException
      */
-    public void syncSendPacket(Command command, final AsyncCallback onComplete) throws JMSException {
+    public void syncSendPacket(final Command command, final AsyncCallback onComplete) throws JMSException {
         if(onComplete==null) {
             syncSendPacket(command);
         } else {
@@ -1336,8 +1336,8 @@ public class ActiveMQConnection implemen
                                 } catch(Throwable e) {
                                     LOG.error("Caught an exception trying to create a JMSException for " +exception,e);
                                 }
-                                //dispose of transport for security exceptions
-                                if (exception instanceof SecurityException){
+                                // dispose of transport for security exceptions on connection initiation
+                                if (exception instanceof SecurityException && command instanceof ConnectionInfo){
                                     Transport t = transport;
                                     if (null != t){
                                         ServiceSupport.dispose(t);
@@ -1380,7 +1380,7 @@ public class ActiveMQConnection implemen
                             LOG.error("Caught an exception trying to create a JMSException for " +er.getException(),e);
                         }
                         //dispose of transport for security exceptions
-                        if (er.getException() instanceof SecurityException){
+                        if (er.getException() instanceof SecurityException && command instanceof ConnectionInfo){
                             Transport t = this.transport;
                             if (null != t){
                                 ServiceSupport.dispose(t);

Modified: activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/TransportConnection.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/TransportConnection.java?rev=1367553&r1=1367552&r2=1367553&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/TransportConnection.java (original)
+++ activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/TransportConnection.java Tue Jul 31 13:54:41 2012
@@ -299,11 +299,6 @@ public class TransportConnection impleme
                         + " command: " + command + ", exception: " + e, e);
             }
 
-            if (e instanceof java.lang.SecurityException) {
-                // still need to close this down - in case the peer of this transport doesn't play nice
-                delayedStop(2000, "Failed with SecurityException: " + e.getLocalizedMessage(), e);
-            }
-
             if (responseRequired) {
                 response = new ExceptionResponse(e);
             } else {
@@ -722,6 +717,10 @@ public class TransportConnection impleme
             if (LOG.isDebugEnabled()) {
                 LOG.debug("Exception detail:", e);
             }
+            if (e instanceof SecurityException) {
+                // close this down - in case the peer of this transport doesn't play nice
+                delayedStop(2000, "Failed with SecurityException: " + e.getLocalizedMessage(), e);
+            }
             throw e;
         }
         if (info.isManageable()) {

Modified: activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/AbstractInactivityMonitor.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/AbstractInactivityMonitor.java?rev=1367553&r1=1367552&r2=1367553&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/AbstractInactivityMonitor.java (original)
+++ activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/AbstractInactivityMonitor.java Tue Jul 31 13:54:41 2012
@@ -180,7 +180,7 @@ public abstract class AbstractInactivity
         }
         if (!commandReceived.get() && monitorStarted.get() && !ASYNC_TASKS.isTerminating()) {
             if (LOG.isDebugEnabled()) {
-                LOG.debug("No message received since last read check for " + toString() + "! Throwing InactivityIOException.");
+                LOG.debug("No message received since last read check for " + toString() + ". Throwing InactivityIOException.");
             }
             ASYNC_TASKS.execute(new Runnable() {
                 public void run() {

Modified: activemq/trunk/activemq-core/src/test/java/org/apache/activemq/security/DoSTest.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/test/java/org/apache/activemq/security/DoSTest.java?rev=1367553&r1=1367552&r2=1367553&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/test/java/org/apache/activemq/security/DoSTest.java (original)
+++ activemq/trunk/activemq-core/src/test/java/org/apache/activemq/security/DoSTest.java Tue Jul 31 13:54:41 2012
@@ -17,6 +17,8 @@
 package org.apache.activemq.security;
 
 import java.net.URI;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import javax.jms.Connection;
 import javax.jms.JMSException;
@@ -29,7 +31,7 @@ import org.slf4j.LoggerFactory;
 
 /**
  * The configuration is set to except a maximum of 2 concurrent connections
- * As the exception is delibrately ignored, the ActiveMQConnection would continue to
+ * As the exception is deliberately ignored, the ActiveMQConnection would continue to
  * attempt to connect unless the connection's transport was also stopped on an error.
  * <p/>
  * As the maximum connections allowed is 2, no more connections would be allowed unless
@@ -42,20 +44,48 @@ public class DoSTest extends JmsTestSupp
 
     public void testInvalidAuthentication() throws Throwable {
 
-        for (int i = 0; i < 1000; i++) {
+        // with failover reconnect, we don't expect this thread to complete
+        // but periodically the failure changes from ExceededMaximumConnectionsException on the broker
+        // side to a SecurityException.
+        // A failed to authenticated but idle connection (dos style) is aborted by the inactivity monitor
+        // since useKeepAlive=false
+
+        final AtomicBoolean done = new AtomicBoolean(false);
+        Thread thread = new Thread() {
+            Connection connection = null;
+
+            public void run() {
+                for (int i = 0; i < 1000 && !done.get(); i++) {
+                    ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory();
+                    try {
+                        // Bad password
+                        connection = factory.createConnection("bad", "krap");
+                        connection.start();
+                        fail("Expected exception.");
+                    } catch (JMSException e) {
+                        // ignore exception and don't close
+                        e.printStackTrace();
+                    }
+                }
+            }
+        };
 
-            try {
-                // Bad password
-                ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory();
-                Connection c = factory.createConnection("bad", "krap");
-                c.start();
-                fail("Expected exception.");
-            } catch (JMSException e) {
+        thread.start();
 
-            }
-        }
+        // run dos for a while
+        TimeUnit.SECONDS.sleep(10);
 
+        LOG.info("trying genuine connection ...");
+        // verify a valid connection can work with one of the 2 allowed connections provided it is eager!
+        // it could take a while as it is competing with the three other reconnect threads.
+        // wonder if it makes sense to serialise these reconnect attempts on an executor
+        ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory("failover:(tcp://127.0.0.1:61616)?useExponentialBackOff=false&reconnectDelay=10");
+        Connection goodConnection = factory.createConnection("user", "password");
+        goodConnection.start();
+        goodConnection.close();
 
+        LOG.info("giving up on DOS");
+        done.set(true);
     }
 
     protected BrokerService createBroker() throws Exception {

Modified: activemq/trunk/activemq-core/src/test/resources/org/apache/activemq/security/dos-broker.xml
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/test/resources/org/apache/activemq/security/dos-broker.xml?rev=1367553&r1=1367552&r2=1367553&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/test/resources/org/apache/activemq/security/dos-broker.xml (original)
+++ activemq/trunk/activemq-core/src/test/resources/org/apache/activemq/security/dos-broker.xml Tue Jul 31 13:54:41 2012
@@ -57,7 +57,9 @@
       </authorizationPlugin>
     </plugins>
       <transportConnectors>
-            <transportConnector uri="tcp://localhost:61616?maximumConnections=2"/>
+          <!-- disable keepalive and use low tolerance for inactive connections such that rogue clients that connect
+          and do nothing (dos style) get aborted -->
+            <transportConnector uri="tcp://localhost:61616?maximumConnections=2&amp;wireFormat.maxInactivityDuration=500&amp;transport.useKeepAlive=false"/>
         </transportConnectors>
   </broker>