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.
This commit is contained in:
Justin Bertram 2020-03-20 14:22:46 -05:00 committed by Clebert Suconic
parent 1eeb06270d
commit 3107535a32
3 changed files with 146 additions and 70 deletions

View File

@ -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;
}
Set<Role> roles = securityRepository.getMatch(match);
String match = rdns.get(rdns.size() - 2).getValue().toString();
if (logger.isDebugEnabled()) {
logger.debug("Destination name: " + match);
}
List<Role> rolesToRemove = new ArrayList<>();
if (match != null) {
Set<Role> roles = securityRepository.getMatch(match);
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);
List<Role> rolesToRemove = new ArrayList<>();
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 from " + match);
for (Role role : roles) {
if (role.isConsume()) {
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);
}
}
}
} 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(adminPermissionValue)) {
logger.debug("Removing admin permission");
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);
}

View File

@ -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();
}
}

View File

@ -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