You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by cl...@apache.org on 2020/03/23 21:02:52 UTC
[activemq-artemis] branch master updated: ARTEMIS-2671 NPE in LDAP
security plugin listener
This is an automated email from the ASF dual-hosted git repository.
clebertsuconic pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git
The following commit(s) were added to refs/heads/master by this push:
new 3107535 ARTEMIS-2671 NPE in LDAP security plugin listener
new b6e24ba This closes #3038
3107535 is described below
commit 3107535a324448909149c245b26ca4085bb6e42b
Author: Justin Bertram <jb...@apache.org>
AuthorDate: Fri Mar 20 14:22:46 2020 -0500
ARTEMIS-2671 NPE in LDAP security plugin listener
To get the name of the destination use the relative Rdn position rather than a
strict match of "uid". Also, improve logging.
---
.../impl/LegacyLDAPSecuritySettingPlugin.java | 92 +++++++++++++---------
...egacyLDAPSecuritySettingPluginListenerTest.java | 84 +++++++++++++++++---
.../src/test/resources/AMQauth.ldif | 36 ++++-----
3 files changed, 144 insertions(+), 68 deletions(-)
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 812b0dc..183d794 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
@@ -317,10 +317,10 @@ public class LegacyLDAPSecuritySettingPlugin implements SecuritySettingPlugin {
try {
if (logger.isDebugEnabled()) {
logger.debug(new StringBuilder().append("Performing LDAP search: ").append(destinationBase)
- .append("\tfilter: ").append(filter)
- .append("\tcontrols:")
- .append("\t\treturningAttributes: ").append(roleAttribute)
- .append("\t\tsearchScope: SUBTREE_SCOPE"));
+ .append("\n\tfilter: ").append(filter)
+ .append("\n\tcontrols:")
+ .append("\n\t\treturningAttributes: ").append(roleAttribute)
+ .append("\n\t\tsearchScope: SUBTREE_SCOPE"));
}
NamingEnumeration<SearchResult> searchResults = context.search(destinationBase, filter, searchControls);
while (searchResults.hasMore()) {
@@ -359,7 +359,7 @@ public class LegacyLDAPSecuritySettingPlugin implements SecuritySettingPlugin {
if (logger.isDebugEnabled()) {
logMessage.append("LDAP search result: ").append(searchResultLdapName);
}
- // we can count on the RNDs being in order from right to left
+ // we can count on the RDNs being in order from right to left
Rdn rdn = rdns.get(rdns.size() - 3);
String rawDestinationType = rdn.getValue().toString();
String destinationType = "unknown";
@@ -369,23 +369,23 @@ public class LegacyLDAPSecuritySettingPlugin implements SecuritySettingPlugin {
destinationType = "topic";
}
if (logger.isDebugEnabled()) {
- logMessage.append("\tDestination type: ").append(destinationType);
+ logMessage.append("\n\tDestination type: ").append(destinationType);
}
rdn = rdns.get(rdns.size() - 2);
if (logger.isDebugEnabled()) {
- logMessage.append("\tDestination name: ").append(rdn.getValue());
+ logMessage.append("\n\tDestination name: ").append(rdn.getValue());
}
String destination = rdn.getValue().toString();
rdn = rdns.get(rdns.size() - 1);
if (logger.isDebugEnabled()) {
- logMessage.append("\tPermission type: ").append(rdn.getValue());
+ logMessage.append("\n\tPermission type: ").append(rdn.getValue());
}
String permissionType = rdn.getValue().toString();
if (logger.isDebugEnabled()) {
- logMessage.append("\tAttributes: ").append(attrs);
+ logMessage.append("\n\tAttributes: ").append(attrs);
}
Attribute attr = attrs.get(roleAttribute);
NamingEnumeration<?> e = attr.getAll();
@@ -403,7 +403,7 @@ public class LegacyLDAPSecuritySettingPlugin implements SecuritySettingPlugin {
rdn = ldapname.getRdn(ldapname.size() - 1);
String roleName = rdn.getValue().toString();
if (logger.isDebugEnabled()) {
- logMessage.append("\tRole name: ").append(roleName);
+ logMessage.append("\n\tRole name: ").append(roleName);
}
Role role = new Role(roleName,
permissionType.equalsIgnoreCase(writePermissionValue), // send
@@ -454,6 +454,9 @@ public class LegacyLDAPSecuritySettingPlugin implements SecuritySettingPlugin {
* @param namingEvent the new entry event that occurred
*/
public void objectAdded(NamingEvent namingEvent) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("objectAdded:\n\told binding: " + namingEvent.getOldBinding() + "\n\tnew binding: " + namingEvent.getNewBinding());
+ }
Map<String, Set<Role>> newRoles = new HashMap<>();
try {
@@ -475,45 +478,57 @@ public class LegacyLDAPSecuritySettingPlugin implements SecuritySettingPlugin {
* @param namingEvent the removed entry event that occurred
*/
public void objectRemoved(NamingEvent namingEvent) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("objectRemoved:\n\told binding: " + namingEvent.getOldBinding() + "\n\tnew binding: " + namingEvent.getNewBinding());
+ }
+
try {
LdapName ldapName = new LdapName(namingEvent.getOldBinding().getName());
- String match = null;
- for (Rdn rdn : ldapName.getRdns()) {
- if (rdn.getType().equals("uid")) {
- match = rdn.getValue().toString();
+ List<Rdn> rdns = ldapName.getRdns();
+ if (rdns.size() < 3) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("Skipping old binding name \"" + namingEvent.getOldBinding().getName() + "\" with " + rdns.size() + " RDNs.");
}
+ return;
+ }
+
+ String match = rdns.get(rdns.size() - 2).getValue().toString();
+ if (logger.isDebugEnabled()) {
+ logger.debug("Destination name: " + match);
}
- Set<Role> roles = securityRepository.getMatch(match);
+ if (match != null) {
+ Set<Role> roles = securityRepository.getMatch(match);
- List<Role> rolesToRemove = new ArrayList<>();
+ List<Role> rolesToRemove = new ArrayList<>();
- for (Rdn rdn : ldapName.getRdns()) {
- if (rdn.getValue().equals(writePermissionValue)) {
- logger.debug("Removing write permission");
- for (Role role : roles) {
- if (role.isSend()) {
- rolesToRemove.add(role);
+ for (Rdn rdn : ldapName.getRdns()) {
+ if (rdn.getValue().equals(writePermissionValue)) {
+ logger.debug("Removing write permission from " + match);
+ for (Role role : roles) {
+ if (role.isSend()) {
+ rolesToRemove.add(role);
+ }
}
- }
- } else if (rdn.getValue().equals(readPermissionValue)) {
- logger.debug("Removing read permission");
- for (Role role : roles) {
- if (role.isConsume()) {
- rolesToRemove.add(role);
+ } else if (rdn.getValue().equals(readPermissionValue)) {
+ logger.debug("Removing read permission from " + match);
+ for (Role role : roles) {
+ if (role.isConsume()) {
+ rolesToRemove.add(role);
+ }
}
- }
- } else if (rdn.getValue().equals(adminPermissionValue)) {
- logger.debug("Removing admin permission");
- for (Role role : roles) {
- if (role.isCreateDurableQueue() || role.isCreateNonDurableQueue() || role.isDeleteDurableQueue() || role.isDeleteNonDurableQueue()) {
- rolesToRemove.add(role);
+ } else if (rdn.getValue().equals(adminPermissionValue)) {
+ logger.debug("Removing admin permission from " + match);
+ for (Role role : roles) {
+ if (role.isCreateDurableQueue() || role.isCreateNonDurableQueue() || role.isDeleteDurableQueue() || role.isDeleteNonDurableQueue()) {
+ rolesToRemove.add(role);
+ }
}
}
- }
- for (Role roleToRemove : rolesToRemove) {
- roles.remove(roleToRemove);
+ for (Role roleToRemove : rolesToRemove) {
+ roles.remove(roleToRemove);
+ }
}
}
} catch (NamingException e) {
@@ -534,6 +549,9 @@ public class LegacyLDAPSecuritySettingPlugin implements SecuritySettingPlugin {
* @param namingEvent the changed entry event that occurred
*/
public void objectChanged(NamingEvent namingEvent) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("objectChanged:\n\told binding: " + namingEvent.getOldBinding() + "\n\tnew binding: " + namingEvent.getNewBinding());
+ }
objectRemoved(namingEvent);
objectAdded(namingEvent);
}
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/LegacyLDAPSecuritySettingPluginListenerTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/LegacyLDAPSecuritySettingPluginListenerTest.java
index dd7327b..74ef8a3 100644
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/LegacyLDAPSecuritySettingPluginListenerTest.java
+++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/LegacyLDAPSecuritySettingPluginListenerTest.java
@@ -178,8 +178,8 @@ public class LegacyLDAPSecuritySettingPluginListenerTest extends AbstractLdapTes
DirContext ctx = getContext();
BasicAttributes basicAttributes = new BasicAttributes();
- basicAttributes.put("uniquemember", "uid=role2");
- ctx.modifyAttributes("cn=write,uid=queue1,ou=queues,ou=destinations,o=ActiveMQ,ou=system", DirContext.REPLACE_ATTRIBUTE, basicAttributes);
+ basicAttributes.put("uniquemember", "cn=role2");
+ ctx.modifyAttributes("cn=write,cn=queue1,ou=queues,ou=destinations,o=ActiveMQ,ou=system", DirContext.REPLACE_ATTRIBUTE, basicAttributes);
ctx.close();
producer2.send(name, session.createMessage(true));
@@ -217,8 +217,8 @@ public class LegacyLDAPSecuritySettingPluginListenerTest extends AbstractLdapTes
DirContext ctx = getContext();
BasicAttributes basicAttributes = new BasicAttributes();
- basicAttributes.put("uniquemember", "uid=role2");
- ctx.modifyAttributes("cn=read,uid=queue1,ou=queues,ou=destinations,o=ActiveMQ,ou=system", DirContext.REPLACE_ATTRIBUTE, basicAttributes);
+ basicAttributes.put("uniquemember", "cn=role2");
+ ctx.modifyAttributes("cn=read,cn=queue1,ou=queues,ou=destinations,o=ActiveMQ,ou=system", DirContext.REPLACE_ATTRIBUTE, basicAttributes);
ctx.close();
consumer2 = session2.createConsumer(queue);
@@ -254,17 +254,17 @@ public class LegacyLDAPSecuritySettingPluginListenerTest extends AbstractLdapTes
DirContext ctx = getContext();
BasicAttributes basicAttributes = new BasicAttributes();
- basicAttributes.put("uniquemember", "uid=role1");
+ basicAttributes.put("uniquemember", "cn=role1");
Attribute objclass = new BasicAttribute("objectclass");
objclass.add("top");
objclass.add("groupOfUniqueNames");
basicAttributes.put(objclass);
- ctx.bind("cn=read,uid=" + queue + ",ou=queues,ou=destinations,o=ActiveMQ,ou=system", null, basicAttributes);
+ ctx.bind("cn=read,cn=" + queue + ",ou=queues,ou=destinations,o=ActiveMQ,ou=system", null, basicAttributes);
consumer = session.createConsumer(queue);
consumer.receiveImmediate();
- ctx.unbind("cn=read,uid=" + queue + ",ou=queues,ou=destinations,o=ActiveMQ,ou=system");
+ ctx.unbind("cn=read,cn=" + queue + ",ou=queues,ou=destinations,o=ActiveMQ,ou=system");
ctx.close();
try {
@@ -296,16 +296,16 @@ public class LegacyLDAPSecuritySettingPluginListenerTest extends AbstractLdapTes
DirContext ctx = getContext();
BasicAttributes basicAttributes = new BasicAttributes();
- basicAttributes.put("uniquemember", "uid=role1");
+ basicAttributes.put("uniquemember", "cn=role1");
Attribute objclass = new BasicAttribute("objectclass");
objclass.add("top");
objclass.add("groupOfUniqueNames");
basicAttributes.put(objclass);
- ctx.bind("cn=write,uid=" + queue + ",ou=queues,ou=destinations,o=ActiveMQ,ou=system", null, basicAttributes);
+ ctx.bind("cn=write,cn=" + queue + ",ou=queues,ou=destinations,o=ActiveMQ,ou=system", null, basicAttributes);
producer.send(session.createMessage(true));
- ctx.unbind("cn=write,uid=" + queue + ",ou=queues,ou=destinations,o=ActiveMQ,ou=system");
+ ctx.unbind("cn=write,cn=" + queue + ",ou=queues,ou=destinations,o=ActiveMQ,ou=system");
ctx.close();
try {
@@ -317,4 +317,68 @@ public class LegacyLDAPSecuritySettingPluginListenerTest extends AbstractLdapTes
cf.close();
}
+
+ @Test
+ public void testNewUserAndRole() throws Exception {
+ server.getConfiguration().setSecurityInvalidationInterval(0);
+ server.start();
+ String queue = "queue1";
+ server.createQueue(SimpleString.toSimpleString(queue), RoutingType.ANYCAST, SimpleString.toSimpleString(queue), null, false, false);
+ ClientSessionFactory cf = locator.createSessionFactory();
+
+ // authentication should fail
+ try {
+ cf.createSession("third", "secret", false, true, true, false, 0);
+ Assert.fail("Creating a session here should fail due to the original security data.");
+ } catch (ActiveMQException e) {
+ Assert.assertTrue(e.getMessage().contains("229031")); // authentication exception
+ }
+
+ { // add new user
+ DirContext ctx = getContext();
+ BasicAttributes basicAttributes = new BasicAttributes();
+ basicAttributes.put("userPassword", "secret");
+ Attribute objclass = new BasicAttribute("objectclass");
+ objclass.add("top");
+ objclass.add("simpleSecurityObject");
+ objclass.add("account");
+ basicAttributes.put(objclass);
+ ctx.bind("uid=third,ou=system", null, basicAttributes);
+ }
+
+ { // add new role
+ DirContext ctx = getContext();
+ BasicAttributes basicAttributes = new BasicAttributes();
+ basicAttributes.put("member", "uid=third,ou=system");
+ Attribute objclass = new BasicAttribute("objectclass");
+ objclass.add("top");
+ objclass.add("groupOfNames");
+ basicAttributes.put(objclass);
+ ctx.bind("cn=role3,ou=system", null, basicAttributes);
+ }
+
+ // authentication should succeed now, but authorization for sending should still fail
+ try {
+ ClientSession session = cf.createSession("third", "secret", false, true, true, false, 0);
+ ClientProducer producer = session.createProducer(SimpleString.toSimpleString(queue));
+ producer.send(session.createMessage(true));
+ Assert.fail("Producing here should fail due to the original security data.");
+ } catch (ActiveMQException e) {
+ Assert.assertTrue(e.getMessage().contains("229032")); // authorization exception
+ }
+
+ { // add write/send permission for new role to existing "queue1"
+ DirContext ctx = getContext();
+ BasicAttributes basicAttributes = new BasicAttributes();
+ basicAttributes.put("uniquemember", "cn=role3");
+ ctx.modifyAttributes("cn=write,cn=queue1,ou=queues,ou=destinations,o=ActiveMQ,ou=system", DirContext.ADD_ATTRIBUTE, basicAttributes);
+ ctx.close();
+ }
+
+ ClientSession session = cf.createSession("third", "secret", false, true, true, false, 0);
+ ClientProducer producer = session.createProducer(SimpleString.toSimpleString(queue));
+ producer.send(session.createMessage(true));
+
+ cf.close();
+ }
}
diff --git a/tests/integration-tests/src/test/resources/AMQauth.ldif b/tests/integration-tests/src/test/resources/AMQauth.ldif
index 83959d6..12ba199 100755
--- a/tests/integration-tests/src/test/resources/AMQauth.ldif
+++ b/tests/integration-tests/src/test/resources/AMQauth.ldif
@@ -61,62 +61,56 @@ objectclass: organizationalUnit
objectclass: top
ou: queues
-dn: uid=queue1,ou=queues,ou=destinations,o=ActiveMQ,ou=system
+dn: cn=queue1,ou=queues,ou=destinations,o=ActiveMQ,ou=system
objectclass: applicationProcess
-objectclass: uidObject
objectclass: top
-uid: queue1
cn: queue1
-dn: uid=queue2,ou=queues,ou=destinations,o=ActiveMQ,ou=system
+dn: cn=queue2,ou=queues,ou=destinations,o=ActiveMQ,ou=system
objectclass: applicationProcess
-objectclass: uidObject
objectclass: top
-uid: queue2
cn: queue2
-dn: uid=activemq.management,ou=queues,ou=destinations,o=ActiveMQ,ou=system
+dn: cn=activemq.management,ou=queues,ou=destinations,o=ActiveMQ,ou=system
objectclass: applicationProcess
-objectclass: uidObject
objectclass: top
-uid: activemq.management
cn: activemq.management
-dn: cn=read,uid=queue1,ou=queues,ou=destinations,o=ActiveMQ,ou=system
+dn: cn=read,cn=queue1,ou=queues,ou=destinations,o=ActiveMQ,ou=system
objectclass: groupOfUniqueNames
objectclass: top
cn: read
-uniquemember: uid=role1
+uniquemember: cn=role1
-dn: cn=write,uid=queue1,ou=queues,ou=destinations,o=ActiveMQ,ou=system
+dn: cn=write,cn=queue1,ou=queues,ou=destinations,o=ActiveMQ,ou=system
objectclass: groupOfUniqueNames
objectclass: top
cn: write
-uniquemember: uid=role1
+uniquemember: cn=role1
-dn: cn=admin,uid=queue1,ou=queues,ou=destinations,o=ActiveMQ,ou=system
+dn: cn=admin,cn=queue1,ou=queues,ou=destinations,o=ActiveMQ,ou=system
objectclass: groupOfUniqueNames
objectclass: top
cn: admin
-uniquemember: uid=role1
+uniquemember: cn=role1
-dn: cn=read,uid=activemq.management,ou=queues,ou=destinations,o=ActiveMQ,ou=system
+dn: cn=read,cn=activemq.management,ou=queues,ou=destinations,o=ActiveMQ,ou=system
objectclass: groupOfUniqueNames
objectclass: top
cn: read
-uniquemember: uid=role1
+uniquemember: cn=role1
-dn: cn=write,uid=activemq.management,ou=queues,ou=destinations,o=ActiveMQ,ou=system
+dn: cn=write,cn=activemq.management,ou=queues,ou=destinations,o=ActiveMQ,ou=system
objectclass: groupOfUniqueNames
objectclass: top
cn: write
-uniquemember: uid=role1
+uniquemember: cn=role1
-dn: cn=admin,uid=activemq.management,ou=queues,ou=destinations,o=ActiveMQ,ou=system
+dn: cn=admin,cn=activemq.management,ou=queues,ou=destinations,o=ActiveMQ,ou=system
objectclass: groupOfUniqueNames
objectclass: top
cn: admin
-uniquemember: uid=role1
+uniquemember: cn=role1
## group with member identified just by DN from SASL external tls certificate subject DN
dn: cn=widgets,ou=system