ARTEMIS 1369: Include queue name in security errors

Add new error in message bundle to include queue
update security check to support taking optional queue
update code that is operating on queues to pass the queue name during check so queue name could be in the error log if security issue.
This commit is contained in:
Michael Andre Pearce 2017-08-23 08:24:05 +01:00 committed by Clebert Suconic
parent fccf146400
commit 45ea110f62
6 changed files with 34 additions and 11 deletions

View File

@ -744,7 +744,7 @@ public class QueueControlImpl extends AbstractControl implements QueueControl {
final String user,
final String password) throws Exception {
try {
securityStore.check(queue.getAddress(), CheckType.SEND, new SecurityAuth() {
securityStore.check(queue.getAddress(), queue.getName(), CheckType.SEND, new SecurityAuth() {
@Override
public String getUsername() {
return user;

View File

@ -25,6 +25,8 @@ public interface SecurityStore {
void check(SimpleString address, CheckType checkType, SecurityAuth session) throws Exception;
void check(SimpleString address, SimpleString queue, CheckType checkType, SecurityAuth session) throws Exception;
boolean isSecurityEnabled();
void stop();

View File

@ -162,6 +162,14 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC
public void check(final SimpleString address,
final CheckType checkType,
final SecurityAuth session) throws Exception {
check(address, null, checkType, session);
}
@Override
public void check(final SimpleString address,
final SimpleString queue,
final CheckType checkType,
final SecurityAuth session) throws Exception {
if (securityEnabled) {
if (logger.isTraceEnabled()) {
logger.trace("checking access permissions to " + address);
@ -206,7 +214,11 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC
notificationService.sendNotification(notification);
}
throw ActiveMQMessageBundle.BUNDLE.userNoPermissions(session.getUsername(), checkType, saddress);
if (queue == null) {
throw ActiveMQMessageBundle.BUNDLE.userNoPermissions(session.getUsername(), checkType, saddress);
} else {
throw ActiveMQMessageBundle.BUNDLE.userNoPermissionsQueue(session.getUsername(), checkType, queue.toString(), saddress);
}
}
// if we get here we're granted, add to the cache
ConcurrentHashSet<SimpleString> set = new ConcurrentHashSet<>();

View File

@ -433,4 +433,7 @@ public interface ActiveMQMessageBundle {
@Message(id = 119212, value = "Invalid deletion policy type {0}", format = Message.Format.MESSAGE_FORMAT)
IllegalArgumentException invalidDeletionPolicyType(String val);
@Message(id = 119213, value = "User: {0} does not have permission=''{1}'' for queue {2} on address {3}", format = Message.Format.MESSAGE_FORMAT)
ActiveMQSecurityException userNoPermissionsQueue(String username, CheckType checkType, String squeue, String saddress);
}

View File

@ -1861,9 +1861,9 @@ public class ActiveMQServerImpl implements ActiveMQServer {
if (queue.isDurable()) {
// make sure the user has privileges to delete this queue
securityStore.check(address, CheckType.DELETE_DURABLE_QUEUE, session);
securityStore.check(address, queueName, CheckType.DELETE_DURABLE_QUEUE, session);
} else {
securityStore.check(address, CheckType.DELETE_NON_DURABLE_QUEUE, session);
securityStore.check(address, queueName, CheckType.DELETE_NON_DURABLE_QUEUE, session);
}
}

View File

@ -415,6 +415,12 @@ public class ServerSessionImpl implements ServerSession, FailureListener {
}
}
private void securityCheck(SimpleString address, SimpleString queue, CheckType checkType, SecurityAuth auth) throws Exception {
if (securityEnabled) {
securityStore.check(address, queue, checkType, auth);
}
}
@Override
public ServerConsumer createConsumer(final long consumerID,
final SimpleString queueName,
@ -441,15 +447,15 @@ public class ServerSessionImpl implements ServerSession, FailureListener {
SimpleString address = removePrefix(binding.getAddress());
if (browseOnly) {
try {
securityCheck(address, CheckType.BROWSE, this);
securityCheck(address, queueName, CheckType.BROWSE, this);
} catch (Exception e) {
securityCheck(address.concat(".").concat(unPrefixedQueueName), CheckType.BROWSE, this);
securityCheck(address.concat(".").concat(unPrefixedQueueName), queueName, CheckType.BROWSE, this);
}
} else {
try {
securityCheck(address, CheckType.CONSUME, this);
securityCheck(address, queueName, CheckType.CONSUME, this);
} catch (Exception e) {
securityCheck(address.concat(".").concat(unPrefixedQueueName), CheckType.CONSUME, this);
securityCheck(address.concat(".").concat(unPrefixedQueueName), queueName, CheckType.CONSUME, this);
}
}
@ -553,9 +559,9 @@ public class ServerSessionImpl implements ServerSession, FailureListener {
if (durable) {
// make sure the user has privileges to create this queue
securityCheck(address, CheckType.CREATE_DURABLE_QUEUE, this);
securityCheck(address, name, CheckType.CREATE_DURABLE_QUEUE, this);
} else {
securityCheck(address, CheckType.CREATE_NON_DURABLE_QUEUE, this);
securityCheck(address, name, CheckType.CREATE_NON_DURABLE_QUEUE, this);
}
server.checkQueueCreationLimit(getUsername());
@ -631,7 +637,7 @@ public class ServerSessionImpl implements ServerSession, FailureListener {
address = removePrefix(address);
securityCheck(address, CheckType.CREATE_NON_DURABLE_QUEUE, this);
securityCheck(address, name, CheckType.CREATE_NON_DURABLE_QUEUE, this);
server.checkQueueCreationLimit(getUsername());