You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by ta...@apache.org on 2017/09/07 16:17:44 UTC
[08/14] activemq git commit: [AMQ-6787] release securty context on
failure to addConnection subsequent to auth, resolve leak. fix and test
[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 <ga...@gmail.com>
Authored: Fri Aug 4 13:46:16 2017 +0100
Committer: Timothy Bish <ta...@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());
+
+ }
+}