Skip to content

ARTEMIS-5958 refactor security store to expose hasPermission bool, to… #6309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gtully wants to merge 1 commit into
base: main
Choose a base branch
from
+102 −74
Open

ARTEMIS-5958 refactor security store to expose hasPermission bool, to… #6309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions artemis-server/src/main/java/org/apache/activemq/artemis/core/security/SecurityStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ public interface SecurityStore {

void check(SimpleString address, SimpleString queue, CheckType checkType, SecurityAuth session) throws Exception;

boolean hasPermission(SimpleString address, SimpleString queue, CheckType checkType, SecurityAuth session) throws Exception;

boolean isSecurityEnabled();

void setSecurityEnabled(boolean securityEnabled);
Expand Down
152 changes: 87 additions & 65 deletions ...erver/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,22 +297,42 @@ public void check(final SimpleString address,
}

@Override
public void check(final SimpleString address,
final SimpleString queue,
final CheckType checkType,
final SecurityAuth session) throws Exception {
if (securityEnabled) {
public boolean hasPermission(final SimpleString address,
final SimpleString queue,
final CheckType checkType,
final SecurityAuth session) throws Exception {
if (!securityEnabled) {
return true;
}

// bypass permission checks for management cluster user
String user = session.getUsername();
if (managementClusterUser.equals(user) && session.getPassword().equals(managementClusterPassword)) {
return true;
}

// Special case: detect authentication failure for ActiveMQSecurityManager5
// This must throw an authentication exception, not an authorization exception
Subject subjectForManager5 = null;
if (securityManager instanceof ActiveMQSecurityManager5 manager5) {
subjectForManager5 = getSubjectForAuthorization(session, manager5);
/*
* A user may authenticate successfully at first, but then later when their Subject is evicted from the
* local cache re-authentication may fail. This could happen, for example, if the user was removed from LDAP
* or the user's token expired.
*
* If the subject is null then authorization will *always* fail.
*/
if (subjectForManager5 == null) {
authenticationFailed(session.getUsername(), session.getRemotingConnection());
}
}
try {
SimpleString bareAddress = CompositeAddress.extractAddressName(address);
SimpleString bareQueue = CompositeAddress.extractQueueName(queue);

logger.trace("checking access permissions to {}", bareAddress);

// bypass permission checks for management cluster user
String user = session.getUsername();
if (managementClusterUser.equals(user) && session.getPassword().equals(managementClusterPassword)) {
AUTHORIZATION_SUCCESS_COUNT_UPDATER.incrementAndGet(this);
return;
}

Set<Role> roles = securityRepository.getMatch(bareAddress.toString());

Expand All @@ -325,27 +345,14 @@ public void check(final SimpleString address,
}
}

if (checkAuthorizationCache(fqqn != null ? fqqn : bareAddress, user, checkType)) {
AUTHORIZATION_SUCCESS_COUNT_UPDATER.incrementAndGet(this);
return;
if (checkAuthorizationCache(fqqn != null ? fqqn : bareAddress, user, checkType)) {
return true;
}

final Boolean validated;
if (securityManager instanceof ActiveMQSecurityManager5 manager5) {
Subject subject = getSubjectForAuthorization(session, manager5);

/*
* A user may authenticate successfully at first, but then later when their Subject is evicted from the
* local cache re-authentication may fail. This could happen, for example, if the user was removed from LDAP
* or the user's token expired.
*
* If the subject is null then authorization will *always* fail.
*/
if (subject == null) {
authenticationFailed(user, session.getRemotingConnection());
}

validated = manager5.authorize(subject, roles, checkType, fqqn != null ? fqqn.toString() : bareAddress.toString());
// Reuse subject from earlier authentication check
validated = manager5.authorize(subjectForManager5, roles, checkType, fqqn != null ? fqqn.toString() : bareAddress.toString());
} else if (securityManager instanceof ActiveMQSecurityManager4 manager4) {
validated = manager4.validateUserAndRole(user, session.getPassword(), roles, checkType, bareAddress.toString(), session.getRemotingConnection(), session.getSecurityDomain()) != null;
} else if (securityManager instanceof ActiveMQSecurityManager3 manager3) {
Expand All @@ -356,52 +363,67 @@ public void check(final SimpleString address,
validated = securityManager.validateUserAndRole(user, session.getPassword(), roles, checkType);
}

if (!validated) {
if (notificationService != null) {
TypedProperties props = new TypedProperties();

props.putSimpleStringProperty(ManagementHelper.HDR_ADDRESS, bareAddress);
props.putSimpleStringProperty(ManagementHelper.HDR_CHECK_TYPE, SimpleString.of(checkType.toString()));
props.putSimpleStringProperty(ManagementHelper.HDR_USER, SimpleString.of(getCaller(user, session.getRemotingConnection().getSubject())));

Notification notification = new Notification(null, CoreNotificationType.SECURITY_PERMISSION_VIOLATION, props);

notificationService.sendNotification(notification);
}

Exception ex;
if (bareQueue == null) {
ex = ActiveMQMessageBundle.BUNDLE.userNoPermissions(getCaller(user, session.getRemotingConnection().getSubject()), checkType, bareAddress);
if (validated && user != null) {
// if we get here we're granted, add to the cache
ConcurrentHashSet<SimpleString> set;
String key = createAuthorizationCacheKey(user, checkType);
ConcurrentHashSet<SimpleString> act = getAuthorizationCacheEntry(key);
if (act != null) {
set = act;
} else {
ex = ActiveMQMessageBundle.BUNDLE.userNoPermissionsQueue(getCaller(user, session.getRemotingConnection().getSubject()), checkType, bareQueue, bareAddress);
set = new ConcurrentHashSet<>();
putAuthorizationCacheEntry(set, key);
}
AuditLogger.securityFailure(session.getRemotingConnection().getSubject(), session.getRemotingConnection().getRemoteAddress(), ex.getMessage(), ex);
AUTHORIZATION_FAILURE_COUNT_UPDATER.incrementAndGet(this);
throw ex;
set.add(Objects.requireNonNullElse(fqqn, bareAddress));
}

// if we get here we're granted, add to the cache
return validated;
} catch (Exception e) {
logger.debug("Permission check failed", e);
return false;
}
}

@Override
public void check(final SimpleString address,
final SimpleString queue,
final CheckType checkType,
final SecurityAuth session) throws Exception {
if (!securityEnabled) {
return;
}

if (hasPermission(address, queue, checkType, session)) {
AUTHORIZATION_SUCCESS_COUNT_UPDATER.incrementAndGet(this);
return;
}

if (user == null) {
// should get all user/pass into a subject and only cache subjects
// till then when subject is in play, the user may be null and
// we cannot cache as we don't have a unique key
return;
}
// Permission denied - handle side effects
SimpleString bareAddress = CompositeAddress.extractAddressName(address);
SimpleString bareQueue = CompositeAddress.extractQueueName(queue);
String user = session.getUsername();

ConcurrentHashSet<SimpleString> set;
String key = createAuthorizationCacheKey(user, checkType);
ConcurrentHashSet<SimpleString> act = getAuthorizationCacheEntry(key);
if (act != null) {
set = act;
} else {
set = new ConcurrentHashSet<>();
putAuthorizationCacheEntry(set, key);
}
set.add(Objects.requireNonNullElse(fqqn, bareAddress));
if (notificationService != null) {
TypedProperties props = new TypedProperties();

props.putSimpleStringProperty(ManagementHelper.HDR_ADDRESS, bareAddress);
props.putSimpleStringProperty(ManagementHelper.HDR_CHECK_TYPE, SimpleString.of(checkType.toString()));
props.putSimpleStringProperty(ManagementHelper.HDR_USER, SimpleString.of(getCaller(user, session.getRemotingConnection().getSubject())));

Notification notification = new Notification(null, CoreNotificationType.SECURITY_PERMISSION_VIOLATION, props);

notificationService.sendNotification(notification);
}

Exception ex;
if (bareQueue == null) {
ex = ActiveMQMessageBundle.BUNDLE.userNoPermissions(getCaller(user, session.getRemotingConnection().getSubject()), checkType, bareAddress);
} else {
ex = ActiveMQMessageBundle.BUNDLE.userNoPermissionsQueue(getCaller(user, session.getRemotingConnection().getSubject()), checkType, bareQueue, bareAddress);
}
AuditLogger.securityFailure(session.getRemotingConnection().getSubject(), session.getRemotingConnection().getRemoteAddress(), ex.getMessage(), ex);
AUTHORIZATION_FAILURE_COUNT_UPDATER.incrementAndGet(this);
throw ex;
}

@Override
Expand Down
22 changes: 13 additions & 9 deletions ...java/org/apache/activemq/artemis/core/server/management/ArtemisRbacInvocationHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,24 +329,28 @@ SimpleString addressFrom(ObjectName objectName, String methodName) {
}

private boolean viewPermissionCheckFails(Object candidate) {
boolean failed = false;
ObjectName objectName = candidate instanceof ObjectInstance oi ? oi.getObjectName() : (ObjectName) candidate;
if (!isUncheckedDomain(objectName)) {
try {
final SimpleString rbacAddress = addressFrom(objectName);
securityStoreCheck(rbacAddress, CheckType.VIEW);
} catch (Exception checkFailed) {
failed = true;
}
if (isUncheckedDomain(objectName)) {
return false;
}
return failed;
final SimpleString rbacAddress = addressFrom(objectName);
return !hasPermission(rbacAddress, CheckType.VIEW);
}

private void securityStoreCheck(SimpleString rbacAddress, CheckType checkType) throws Exception {
// use accessor as security store can be updated on config reload
activeMQServer.getSecurityStore().check(rbacAddress, checkType, delegateToAccessController);
}

private boolean hasPermission(SimpleString rbacAddress, CheckType checkType) {
// use accessor as security store can be updated on config reload
try {
return activeMQServer.getSecurityStore().hasPermission(rbacAddress, null, checkType, delegateToAccessController);
} catch (Exception notAuthenticated) {
return false;
}
}

// sufficiently empty to delegate to use of AccessController
// ideally AccessController should be the source of truth
private final SecurityAuth delegateToAccessController = new SecurityAuth() {
Expand Down