activemq-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tab...@apache.org
Subject [08/14] activemq git commit: [AMQ-6787] release securty context on failure to addConnection subsequent to auth, resolve leak. fix and test
Date Thu, 07 Sep 2017 16:17:44 GMT
[AMQ-6787] release securty context on failure to addConnection subsequent to auth, resolve
leak. fix and test

(cherry picked from commit a15626193c8d3c099b0bcf1e605b892b533a4d3e)


Project: http://git-wip-us.apache.org/repos/asf/activemq/repo
Commit: http://git-wip-us.apache.org/repos/asf/activemq/commit/035baf17
Tree: http://git-wip-us.apache.org/repos/asf/activemq/tree/035baf17
Diff: http://git-wip-us.apache.org/repos/asf/activemq/diff/035baf17

Branch: refs/heads/activemq-5.15.x
Commit: 035baf1722f79022c8af0a17158b2c418fa8c577
Parents: 199f25e
Author: gtully <gary.tully@gmail.com>
Authored: Fri Aug 4 13:46:16 2017 +0100
Committer: Timothy Bish <tabish121@gmail.com>
Committed: Thu Sep 7 12:13:29 2017 -0400

----------------------------------------------------------------------
 .../security/JaasAuthenticationBroker.java      | 18 +++-
 .../org/apache/activemq/broker/StubBroker.java  |  6 ++
 .../security/JaasAuthenticationBrokerTest.java  | 97 ++++++++++++++++++++
 3 files changed, 116 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/activemq/blob/035baf17/activemq-broker/src/main/java/org/apache/activemq/security/JaasAuthenticationBroker.java
----------------------------------------------------------------------
diff --git a/activemq-broker/src/main/java/org/apache/activemq/security/JaasAuthenticationBroker.java
b/activemq-broker/src/main/java/org/apache/activemq/security/JaasAuthenticationBroker.java
index 148c2e2..6756027 100644
--- a/activemq-broker/src/main/java/org/apache/activemq/security/JaasAuthenticationBroker.java
+++ b/activemq-broker/src/main/java/org/apache/activemq/security/JaasAuthenticationBroker.java
@@ -63,16 +63,24 @@ public class JaasAuthenticationBroker extends AbstractAuthenticationBroker
{
             // Set the TCCL since it seems JAAS needs it to find the login module classes.
             ClassLoader original = Thread.currentThread().getContextClassLoader();
             Thread.currentThread().setContextClassLoader(JaasAuthenticationBroker.class.getClassLoader());
-
+            SecurityContext securityContext = null;
             try {
-                SecurityContext s = authenticate(info.getUserName(), info.getPassword(),
null);
-                context.setSecurityContext(s);
-                securityContexts.add(s);
+                securityContext = authenticate(info.getUserName(), info.getPassword(), null);
+                context.setSecurityContext(securityContext);
+                securityContexts.add(securityContext);
+                super.addConnection(context, info);
+            } catch (Exception error) {
+                if (securityContext != null) {
+                    securityContexts.remove(securityContext);
+                }
+                context.setSecurityContext(null);
+                throw error;
             } finally {
                 Thread.currentThread().setContextClassLoader(original);
             }
+        } else {
+            super.addConnection(context, info);
         }
-        super.addConnection(context, info);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/activemq/blob/035baf17/activemq-unit-tests/src/test/java/org/apache/activemq/broker/StubBroker.java
----------------------------------------------------------------------
diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/StubBroker.java
b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/StubBroker.java
index 7b4fa1b..af42eb0 100644
--- a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/StubBroker.java
+++ b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/StubBroker.java
@@ -17,6 +17,7 @@
 
 package org.apache.activemq.broker;
 
+import javax.jms.InvalidClientIDException;
 import java.util.LinkedList;
 import org.apache.activemq.command.ConnectionInfo;
 
@@ -47,6 +48,11 @@ public class StubBroker extends EmptyBroker {
     }
 
     public void addConnection(ConnectionContext context, ConnectionInfo info) throws Exception
{
+        for (AddConnectionData data : addConnectionData) {
+            if (data.connectionInfo.getClientId() != null && data.connectionInfo.getClientId().equals(info.getClientId()))
{
+                throw new InvalidClientIDException("ClientID already exists");
+            }
+        }
         addConnectionData.add(new AddConnectionData(context, info));
     }
 

http://git-wip-us.apache.org/repos/asf/activemq/blob/035baf17/activemq-unit-tests/src/test/java/org/apache/activemq/security/JaasAuthenticationBrokerTest.java
----------------------------------------------------------------------
diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/security/JaasAuthenticationBrokerTest.java
b/activemq-unit-tests/src/test/java/org/apache/activemq/security/JaasAuthenticationBrokerTest.java
new file mode 100644
index 0000000..75f567e
--- /dev/null
+++ b/activemq-unit-tests/src/test/java/org/apache/activemq/security/JaasAuthenticationBrokerTest.java
@@ -0,0 +1,97 @@
+/**
+ * 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 javax.jms.InvalidClientIDException;
+import javax.security.auth.login.AppConfigurationEntry;
+import javax.security.auth.login.Configuration;
+import java.util.HashMap;
+import java.util.concurrent.CopyOnWriteArrayList;
+
+import junit.framework.TestCase;
+
+import org.apache.activemq.broker.Broker;
+import org.apache.activemq.broker.ConnectionContext;
+import org.apache.activemq.broker.StubBroker;
+import org.apache.activemq.command.ConnectionInfo;
+
+public class JaasAuthenticationBrokerTest extends TestCase {
+    StubBroker receiveBroker;
+
+    JaasAuthenticationBroker authBroker;
+
+    ConnectionContext connectionContext;
+    ConnectionInfo connectionInfo;
+
+    CopyOnWriteArrayList<SecurityContext> visibleSecurityContexts;
+
+    class JaasAuthenticationBrokerTester extends JaasAuthenticationBroker {
+        public JaasAuthenticationBrokerTester(Broker next, String jassConfiguration) {
+            super(next, jassConfiguration);
+            visibleSecurityContexts = securityContexts;
+        }
+    }
+
+    @Override
+    protected void setUp() throws Exception {
+        receiveBroker = new StubBroker();
+
+        authBroker = new JaasAuthenticationBrokerTester(receiveBroker, "");
+
+        connectionContext = new ConnectionContext();
+        connectionInfo = new ConnectionInfo();
+    }
+
+    @Override
+    protected void tearDown() throws Exception {
+        super.tearDown();
+    }
+
+    private void setConfiguration(boolean loginShouldSucceed) {
+        HashMap<String, String> configOptions = new HashMap<String, String>();
+
+        configOptions.put(StubLoginModule.ALLOW_LOGIN_PROPERTY, loginShouldSucceed ? "true"
: "false");
+        configOptions.put(StubLoginModule.USERS_PROPERTY, "");
+        configOptions.put(StubLoginModule.GROUPS_PROPERTY, "");
+        AppConfigurationEntry configEntry = new AppConfigurationEntry("org.apache.activemq.security.StubLoginModule",
AppConfigurationEntry.LoginModuleControlFlag.REQUIRED, configOptions);
+
+        StubJaasConfiguration jaasConfig = new StubJaasConfiguration(configEntry);
+
+        Configuration.setConfiguration(jaasConfig);
+    }
+
+
+    public void testAddConnectionFailureOnDuplicateClientId() throws Exception {
+        setConfiguration(true);
+
+        connectionInfo.setClientId("CliIdX");
+        authBroker.addConnection(connectionContext, connectionInfo);
+        ConnectionContext secondContext = connectionContext.copy();
+        secondContext.setSecurityContext(null);
+        ConnectionInfo secondInfo = connectionInfo.copy();
+        try {
+            authBroker.addConnection(secondContext, secondInfo);
+            fail("Expect duplicate id");
+        } catch (InvalidClientIDException expected) {
+        }
+
+        assertEquals("one connection allowed.", 1, receiveBroker.addConnectionData.size());
+        assertEquals("one context .", 1, visibleSecurityContexts.size());
+
+    }
+}


Mime
View raw message