From a9a85f98db4f49d633541069ae997b4d099837e3 Mon Sep 17 00:00:00 2001 From: Justin Bertram Date: Sat, 10 Sep 2022 00:09:37 -0500 Subject: [PATCH] ARTEMIS-3984 LegacyLDAPSecuritySettingPlugin shouldn't modify existing matches When the LegacyLDAPSecuritySettingPlugin has enableListener set to true and a new permission is added it will try to modify the existing match if one exists. This is problematic if there's a more generic wildcard match than the specific one that's modified. This commit fixes that problem so that instead of modifying the existing match(es) it simply adds a new one. The plugin never should have tried modifying the existing match in the first place as two identical matches would be a configuration error. --- .../impl/LegacyLDAPSecuritySettingPlugin.java | 13 +- artemis-website/package-lock.json | 4 +- ...DAPSecuritySettingPluginListenerTest2.java | 241 ++++++++++++++++++ .../src/test/resources/AMQauth3.ldif | 113 ++++++++ .../src/test/resources/login.config | 21 ++ 5 files changed, 379 insertions(+), 13 deletions(-) create mode 100644 tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/LegacyLDAPSecuritySettingPluginListenerTest2.java create mode 100755 tests/integration-tests/src/test/resources/AMQauth3.ldif diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/LegacyLDAPSecuritySettingPlugin.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/LegacyLDAPSecuritySettingPlugin.java index d3b22bc684..9e8f6df7b8 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/LegacyLDAPSecuritySettingPlugin.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/LegacyLDAPSecuritySettingPlugin.java @@ -476,17 +476,8 @@ public class LegacyLDAPSecuritySettingPlugin implements SecuritySettingPlugin { try { processSearchResult(newRoles, (SearchResult) namingEvent.getNewBinding()); for (Map.Entry> entry : newRoles.entrySet()) { - Set existingRoles = securityRepository.getMatch(entry.getKey()); - // see if this the *actual* default object, not just "equals"; we don't want to change the default security match - if (existingRoles != securityRepository.getDefault()) { - for (Role role : entry.getValue()) { - logger.debug("adding role " + role + " to existing roles " + existingRoles + " at " + entry.getKey()); - existingRoles.add(role); - } - } else { - logger.debug("adding new roles " + entry.getValue() + " at " + entry.getKey()); - securityRepository.addMatch(entry.getKey(), entry.getValue()); - } + logger.debug("adding match " + entry.getKey() + ": " + entry.getValue()); + securityRepository.addMatch(entry.getKey(), entry.getValue()); } } catch (NamingException e) { ActiveMQServerLogger.LOGGER.failedToProcessEvent(e); diff --git a/artemis-website/package-lock.json b/artemis-website/package-lock.json index 51ea6670d3..1e5b81dd37 100644 --- a/artemis-website/package-lock.json +++ b/artemis-website/package-lock.json @@ -1,11 +1,11 @@ { - "name": "artemis-websiteb", + "name": "artemis-website", "version": "1.5.1", "lockfileVersion": 2, "requires": true, "packages": { "": { - "name": "artemis-websiteb", + "name": "artemis-website", "version": "1.5.1", "license": "Apache-2.0", "devDependencies": { diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/LegacyLDAPSecuritySettingPluginListenerTest2.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/LegacyLDAPSecuritySettingPluginListenerTest2.java new file mode 100644 index 0000000000..a486403ef9 --- /dev/null +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/LegacyLDAPSecuritySettingPluginListenerTest2.java @@ -0,0 +1,241 @@ +/* + * 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.artemis.tests.integration.security; + +import javax.naming.Context; +import javax.naming.NamingException; +import javax.naming.directory.Attribute; +import javax.naming.directory.BasicAttribute; +import javax.naming.directory.BasicAttributes; +import javax.naming.directory.DirContext; +import javax.naming.directory.InitialDirContext; +import java.io.File; +import java.lang.management.ManagementFactory; +import java.net.URL; +import java.util.HashMap; +import java.util.Hashtable; +import java.util.Map; + +import org.apache.activemq.artemis.api.core.ActiveMQException; +import org.apache.activemq.artemis.api.core.TransportConfiguration; +import org.apache.activemq.artemis.api.core.client.ActiveMQClient; +import org.apache.activemq.artemis.api.core.client.ClientProducer; +import org.apache.activemq.artemis.api.core.client.ClientSession; +import org.apache.activemq.artemis.api.core.client.ClientSessionFactory; +import org.apache.activemq.artemis.api.core.client.ServerLocator; +import org.apache.activemq.artemis.core.config.Configuration; +import org.apache.activemq.artemis.core.config.WildcardConfiguration; +import org.apache.activemq.artemis.core.config.impl.ConfigurationImpl; +import org.apache.activemq.artemis.core.remoting.impl.invm.InVMAcceptorFactory; +import org.apache.activemq.artemis.core.remoting.impl.invm.InVMConnectorFactory; +import org.apache.activemq.artemis.core.server.ActiveMQServer; +import org.apache.activemq.artemis.core.server.ActiveMQServers; +import org.apache.activemq.artemis.core.server.impl.LegacyLDAPSecuritySettingPlugin; +import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager; +import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; +import org.apache.directory.server.annotations.CreateLdapServer; +import org.apache.directory.server.annotations.CreateTransport; +import org.apache.directory.server.core.annotations.ApplyLdifFiles; +import org.apache.directory.server.core.annotations.CreateDS; +import org.apache.directory.server.core.annotations.CreatePartition; +import org.apache.directory.server.core.integ.AbstractLdapTestUnit; +import org.apache.directory.server.core.integ.FrameworkRunner; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; + +@RunWith(FrameworkRunner.class) +@CreateDS(name = "myDS", + partitions = { + @CreatePartition(name = "test", suffix = "dc=example,dc=com") + }) +@CreateLdapServer(transports = {@CreateTransport(protocol = "LDAP", port = 1024)}) +@ApplyLdifFiles("AMQauth3.ldif") +public class LegacyLDAPSecuritySettingPluginListenerTest2 extends AbstractLdapTestUnit { + + static { + String path = System.getProperty("java.security.auth.login.config"); + if (path == null) { + URL resource = LegacyLDAPSecuritySettingPluginListenerTest2.class.getClassLoader().getResource("login.config"); + if (resource != null) { + path = resource.getFile(); + System.setProperty("java.security.auth.login.config", path); + } + } + } + + private ServerLocator locator; + ActiveMQServer server; + + public static final String TARGET_TMP = "./target/tmp"; + private static final String PRINCIPAL = "uid=admin,ou=system"; + private static final String CREDENTIALS = "secret"; + + public LegacyLDAPSecuritySettingPluginListenerTest2() { + File parent = new File(TARGET_TMP); + parent.mkdirs(); + temporaryFolder = new TemporaryFolder(parent); + } + + @Rule + public TemporaryFolder temporaryFolder; + private String testDir; + + @Before + public void setUp() throws Exception { + locator = ActiveMQClient.createServerLocatorWithoutHA(new TransportConfiguration(InVMConnectorFactory.class.getCanonicalName())); + testDir = temporaryFolder.getRoot().getAbsolutePath(); + + LegacyLDAPSecuritySettingPlugin legacyLDAPSecuritySettingPlugin = new LegacyLDAPSecuritySettingPlugin(); + Map map = new HashMap<>(); + map.put(LegacyLDAPSecuritySettingPlugin.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory"); + map.put(LegacyLDAPSecuritySettingPlugin.CONNECTION_URL, "ldap://localhost:1024"); + map.put(LegacyLDAPSecuritySettingPlugin.CONNECTION_USERNAME, "uid=admin,ou=system"); + map.put(LegacyLDAPSecuritySettingPlugin.CONNECTION_PASSWORD, "secret"); + map.put(LegacyLDAPSecuritySettingPlugin.CONNECTION_PROTOCOL, "s"); + map.put(LegacyLDAPSecuritySettingPlugin.AUTHENTICATION, "simple"); + map.put(LegacyLDAPSecuritySettingPlugin.ENABLE_LISTENER, "true"); + map.put(LegacyLDAPSecuritySettingPlugin.DESTINATION_BASE, "ou=destinations,ou=ActiveMQ,dc=example,dc=com"); + map.put(LegacyLDAPSecuritySettingPlugin.MAP_ADMIN_TO_MANAGE, "true"); + legacyLDAPSecuritySettingPlugin.init(map); + + ActiveMQJAASSecurityManager securityManager = new ActiveMQJAASSecurityManager("LDAPLogin3"); + WildcardConfiguration wildcardConfiguration = new WildcardConfiguration(); + wildcardConfiguration.setAnyWords('$'); + Configuration configuration = new ConfigurationImpl().setSecurityEnabled(true).addAcceptorConfiguration(new TransportConfiguration(InVMAcceptorFactory.class.getCanonicalName())).setJournalDirectory(ActiveMQTestBase.getJournalDir(testDir, 0, false)).setBindingsDirectory(ActiveMQTestBase.getBindingsDir(testDir, 0, false)).setPagingDirectory(ActiveMQTestBase.getPageDir(testDir, 0, false)).setLargeMessagesDirectory(ActiveMQTestBase.getLargeMessagesDir(testDir, 0, false)).setPersistenceEnabled(false).addSecuritySettingPlugin(legacyLDAPSecuritySettingPlugin).setWildCardConfiguration(wildcardConfiguration); + + server = ActiveMQServers.newActiveMQServer(configuration, ManagementFactory.getPlatformMBeanServer(), securityManager, false); + } + + @After + public void tearDown() throws Exception { + locator.close(); + server.stop(); + } + + private DirContext getContext() throws NamingException { + Hashtable env = new Hashtable<>(); + env.put(Context.PROVIDER_URL, "ldap://localhost:1024"); + env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory"); + env.put(Context.SECURITY_AUTHENTICATION, "simple"); + env.put(Context.SECURITY_PRINCIPAL, PRINCIPAL); + env.put(Context.SECURITY_CREDENTIALS, CREDENTIALS); + return new InitialDirContext(env); + } + + @Test + public void testNewUserAndRoleWithWildcard() throws Exception { + server.getConfiguration().setSecurityInvalidationInterval(0); + server.start(); + ClientSessionFactory cf = locator.createSessionFactory(); + + // authz should succeed + try { + ClientSession session = cf.createSession("user1", "secret", false, true, true, false, 0); + ClientProducer producer = session.createProducer("project1.test"); + producer.send(session.createMessage(true)); + } catch (ActiveMQException e) { + e.printStackTrace(); + Assert.fail("Should NOT fail"); + } + + // authz should fail + try { + ClientSession session = cf.createSession("user1", "secret", false, true, true, false, 0); + ClientProducer producer = session.createProducer("project2.test"); + producer.send(session.createMessage(true)); + Assert.fail("Sending message here should fail!"); + } catch (ActiveMQException e) { + Assert.assertTrue(e.getMessage().contains("229032")); // authorization exception + } + + { // add new user - user3 + DirContext ctx = getContext(); + BasicAttributes basicAttributes = new BasicAttributes(); + basicAttributes.put("userPassword", "secret"); + Attribute objclass = new BasicAttribute("objectclass"); + objclass.add("top"); + objclass.add("person"); + objclass.add("organizationalPerson"); + objclass.add("inetOrgPerson"); + basicAttributes.put(objclass); + Attribute cn = new BasicAttribute("cn"); + cn.add("user3"); + basicAttributes.put(cn); + Attribute sn = new BasicAttribute("sn"); + sn.add("user3"); + basicAttributes.put(sn); + ctx.bind("uid=user3,ou=users,dc=example,dc=com", null, basicAttributes); + } + + { // add new role - team3 + DirContext ctx = getContext(); + BasicAttributes basicAttributes = new BasicAttributes(); + basicAttributes.put("uniqueMember", "uid=user3,ou=users,dc=example,dc=com"); + Attribute objclass = new BasicAttribute("objectclass"); + objclass.add("top"); + objclass.add("groupOfUniqueNames"); + basicAttributes.put(objclass); + ctx.bind("cn=team3,ou=roles,dc=example,dc=com", null, basicAttributes); + } + + { // add new security match - project3.$ + DirContext ctx = getContext(); + BasicAttributes basicAttributes = new BasicAttributes(); + Attribute objclass = new BasicAttribute("objectclass"); + objclass.add("top"); + objclass.add("applicationProcess"); + basicAttributes.put(objclass); + ctx.bind("cn=project3.$,ou=queues,ou=destinations,ou=ActiveMQ,dc=example,dc=com", null, basicAttributes); + } + + { // add write permission for match + DirContext ctx = getContext(); + BasicAttributes basicAttributes = new BasicAttributes(); + basicAttributes.put("uniquemember", "cn=team3,ou=roles,dc=example,dc=com"); + Attribute objclass = new BasicAttribute("objectclass"); + objclass.add("top"); + objclass.add("groupOfUniqueNames"); + basicAttributes.put(objclass); + ctx.bind("cn=write,cn=project3.$,ou=queues,ou=destinations,ou=ActiveMQ,dc=example,dc=com", null, basicAttributes); + } + + // authz should succeed + try { + ClientSession session = cf.createSession("user3", "secret", false, true, true, false, 0); + ClientProducer producer = session.createProducer("project3.test"); + producer.send(session.createMessage(true)); + } catch (ActiveMQException e) { + e.printStackTrace(); + Assert.fail("Should NOT fail"); + } + + // authz should fail + try { + ClientSession session = cf.createSession("user3", "secret", false, true, true, false, 0); + ClientProducer producer = session.createProducer("project7.test"); + producer.send(session.createMessage(true)); + Assert.fail("Sending message here should fail!"); + } catch (ActiveMQException e) { + Assert.assertTrue(e.getMessage().contains("229032")); // authorization exception + } + } +} diff --git a/tests/integration-tests/src/test/resources/AMQauth3.ldif b/tests/integration-tests/src/test/resources/AMQauth3.ldif new file mode 100755 index 0000000000..d5997ce645 --- /dev/null +++ b/tests/integration-tests/src/test/resources/AMQauth3.ldif @@ -0,0 +1,113 @@ +## --------------------------------------------------------------------------- +## 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. +## --------------------------------------------------------------------------- + +dn: dc=example,dc=com +dc: example +objectclass: domain +objectclass: top + +dn: ou=users,dc=example,dc=com +objectclass: organizationalUnit +objectclass: top +ou: users + +dn: uid=user1,ou=users,dc=example,dc=com +objectClass: inetOrgPerson +objectClass: organizationalPerson +objectClass: person +objectClass: top +cn: user1 +sn: user1 +givenName: user1 +uid: user1 +userPassword: secret + +dn: uid=user2,ou=users,dc=example,dc=com +objectClass: inetOrgPerson +objectClass: organizationalPerson +objectClass: person +objectClass: top +cn: user2 +sn: user2 +givenName: user2 +uid: user2 +userPassword: secret + +dn: ou=roles,dc=example,dc=com +objectclass: organizationalUnit +objectclass: top +ou: roles + +dn: cn=team1,ou=roles,dc=example,dc=com +objectClass: groupOfUniqueNames +objectClass: top +cn: team1 +uniqueMember: uid=user1,ou=users,dc=example,dc=com + +dn: cn=team2,ou=roles,dc=example,dc=com +objectClass: groupOfUniqueNames +objectClass: top +cn: team2 +uniqueMember: uid=user2,ou=users,dc=example,dc=com + +dn: ou=ActiveMQ,dc=example,dc=com +objectClass: organizationalUnit +objectClass: top +ou: ActiveMQ + +dn: ou=destinations,ou=ActiveMQ,dc=example,dc=com +objectClass: organizationalUnit +objectClass: top +ou: destinations + +dn: ou=queues,ou=destinations,ou=ActiveMQ,dc=example,dc=com +objectClass: organizationalUnit +objectClass: top +ou: queues + +dn: cn=project1.$,ou=queues,ou=destinations,ou=ActiveMQ,dc=example,dc=com +objectClass: applicationProcess +objectClass: top +cn: project1.$ + +dn: cn=write,cn=project1.$,ou=queues,ou=destinations,ou=ActiveMQ,dc=example,dc=com +objectClass: top +objectClass: groupOfUniqueNames +cn: write +uniqueMember: cn=team1,ou=roles,dc=example,dc=com + +dn: cn=project2.$,ou=queues,ou=destinations,ou=ActiveMQ,dc=example,dc=com +objectClass: applicationProcess +objectClass: top +cn: project2.$ + +dn: cn=read,cn=project2.$,ou=queues,ou=destinations,ou=ActiveMQ,dc=example,dc=com +objectClass: groupOfUniqueNames +objectClass: top +cn: read +uniqueMember: cn=team2,ou=roles,dc=example,dc=com + +dn: cn=$,ou=queues,ou=destinations,ou=ActiveMQ,dc=example,dc=com +objectClass: top +objectClass: applicationProcess +cn: $ + +dn: cn=write,cn=$,ou=queues,ou=destinations,ou=ActiveMQ,dc=example,dc=com +objectClass: top +objectClass: groupOfUniqueNames +cn: write +uniqueMember: cn=some_role,ou=roles,dc=example,dc=com diff --git a/tests/integration-tests/src/test/resources/login.config b/tests/integration-tests/src/test/resources/login.config index f1fe8dfaba..a9eaff49a8 100644 --- a/tests/integration-tests/src/test/resources/login.config +++ b/tests/integration-tests/src/test/resources/login.config @@ -107,6 +107,27 @@ ExpandedLDAPLogin { ; }; +LDAPLogin3 { + org.apache.activemq.artemis.spi.core.security.jaas.LDAPLoginModule required + debug=true + initialContextFactory=com.sun.jndi.ldap.LdapCtxFactory + connectionURL="ldap://localhost:1024" + connectionUsername="uid=admin,ou=system" + connectionPassword=secret + connectionProtocol=s + authentication=simple + userBase="ou=users,dc=example,dc=com" + userSearchMatching="(uid={0})" + userSearchSubtree=true + roleBase="ou=roles,dc=example,dc=com" + roleName=cn + roleSearchMatching="(uniqueMember={0})" + roleSearchSubtree=true + connectionPool=true + connectionTimeout="5000" + ; +}; + GuestLogin { org.apache.activemq.artemis.spi.core.security.jaas.GuestLoginModule required debug=true