From 90853409a04bed9f64787447528defe869213b52 Mon Sep 17 00:00:00 2001 From: Justin Bertram Date: Mon, 24 Aug 2020 16:24:25 +0200 Subject: [PATCH] ARTEMIS-2886 optimize security auth Both authentication and authorization will hit the underlying security repository (e.g. files, LDAP, etc.). For example, creating a JMS connection and a consumer will result in 2 hits with the *same* authentication request. This can cause unwanted (and unnecessary) resource utilization, especially in the case of networked configuration like LDAP. There is already a rudimentary cache for authorization, but it is cleared *totally* every 10 seconds by default (controlled via the security-invalidation-interval setting), and it must be populated initially which still results in duplicate auth requests. This commit optimizes authentication and authorization via the following changes: - Replace our home-grown cache with Google Guava's cache. This provides simple caching with both time-based and size-based LRU eviction. See more at https://github.com/google/guava/wiki/CachesExplained. I also thought about using Caffeine, but we already have a dependency on Guava and the cache implementions look to be negligibly different for this use-case. - Add caching for authentication. Both successful and unsuccessful authentication attempts will be cached to spare the underlying security repository as much as possible. Authenticated Subjects will be cached and re-used whenever possible. - Authorization will used Subjects cached during authentication. If the required Subject is not in the cache it will be fetched from the underlying security repo. - Caching can be disabled by setting the security-invalidation-interval to 0. - Cache sizes are configurable. - Management operations exist to inspect cache sizes at runtime. --- .../apache/activemq/cli/test/ArtemisTest.java | 2 + .../activemq/artemis/logs/AuditLogger.java | 16 ++ .../config/ActiveMQDefaultConfiguration.java | 20 ++ .../management/ActiveMQServerControl.java | 12 ++ .../src/main/resources/features.xml | 1 + artemis-server/pom.xml | 8 +- .../artemis/core/config/Configuration.java | 22 ++ .../core/config/impl/ConfigurationImpl.java | 26 +++ .../impl/FileConfigurationParser.java | 6 +- .../impl/ActiveMQServerControlImpl.java | 17 ++ .../core/security/impl/SecurityStoreImpl.java | 188 +++++++++++++----- .../core/server/impl/ActiveMQServerImpl.java | 2 +- .../security/ActiveMQJAASSecurityManager.java | 48 +---- .../security/ActiveMQSecurityManager5.java | 61 ++++++ .../schema/artemis-configuration.xsd | 18 +- .../config/impl/FileConfigurationTest.java | 2 + .../jaas/JAASSecurityManagerTest.java | 13 +- .../ConfigurationTest-full-config.xml | 2 + .../ConfigurationTest-xinclude-config.xml | 2 + docs/user-manual/en/security.md | 14 +- .../example/JAASSecurityManagerWrapper.java | 26 +-- .../management/ActiveMQServerControlTest.java | 26 +++ .../ActiveMQServerControlUsingCoreTest.java | 10 + .../integration/security/SecurityTest.java | 16 +- .../StompWithClientIdValidationTest.java | 8 +- 25 files changed, 441 insertions(+), 125 deletions(-) create mode 100644 artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/ActiveMQSecurityManager5.java diff --git a/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java b/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java index 92d4503674..c050d2e379 100644 --- a/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java +++ b/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java @@ -69,6 +69,7 @@ import org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl; import org.apache.activemq.artemis.core.config.FileDeploymentManager; import org.apache.activemq.artemis.core.config.impl.FileConfiguration; import org.apache.activemq.artemis.core.security.CheckType; +import org.apache.activemq.artemis.core.security.impl.SecurityStoreImpl; import org.apache.activemq.artemis.core.server.ActiveMQServer; import org.apache.activemq.artemis.core.server.JournalType; import org.apache.activemq.artemis.core.server.management.ManagementContext; @@ -621,6 +622,7 @@ public class ArtemisTest extends CliTestBase { activeMQServerControl.addSecuritySettings("myAddress", "myRole", "myRole", "myRole", "myRole", "myRole", "myRole", "myRole", "myRole", "myRole", "myRole"); // change properties files which should cause another "reload" event activeMQServerControl.addUser("foo", "bar", "myRole", true); + ((SecurityStoreImpl)activeMQServer.getSecurityStore()).invalidateAuthenticationCache(); ClientSession session = sessionFactory.createSession("foo", "bar", false, false, false, false, 0); session.createQueue("myAddress", RoutingType.ANYCAST, "myQueue", true); ClientProducer producer = session.createProducer("myAddress"); diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/logs/AuditLogger.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/logs/AuditLogger.java index 7fcc0ed007..11fd9d497a 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/logs/AuditLogger.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/logs/AuditLogger.java @@ -2737,4 +2737,20 @@ public interface AuditLogger extends BasicLogger { @LogMessage(level = Logger.Level.INFO) @Message(id = 601735, value = "User {0} is getting group rebalance pause dispatch property on target resource: {1} {2}", format = Message.Format.MESSAGE_FORMAT) void isGroupRebalancePauseDispatch(String user, Object source, Object... args); + + static void getAuthenticationCacheSize(Object source) { + LOGGER.getAuthenticationCacheSize(getCaller(), source); + } + + @LogMessage(level = Logger.Level.INFO) + @Message(id = 601736, value = "User {0} is getting authentication cache size on target resource: {1} {2}", format = Message.Format.MESSAGE_FORMAT) + void getAuthenticationCacheSize(String user, Object source, Object... args); + + static void getAuthorizationCacheSize(Object source) { + LOGGER.getAuthorizationCacheSize(getCaller(), source); + } + + @LogMessage(level = Logger.Level.INFO) + @Message(id = 601737, value = "User {0} is getting authorization cache size on target resource: {1} {2}", format = Message.Format.MESSAGE_FORMAT) + void getAuthorizationCacheSize(String user, Object source, Object... args); } diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/config/ActiveMQDefaultConfiguration.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/config/ActiveMQDefaultConfiguration.java index f7d33ffb68..cffd2639c9 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/config/ActiveMQDefaultConfiguration.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/config/ActiveMQDefaultConfiguration.java @@ -160,6 +160,12 @@ public final class ActiveMQDefaultConfiguration { // how long (in ms) to wait before invalidating the security cache private static long DEFAULT_SECURITY_INVALIDATION_INTERVAL = 10000; + // how large to make the authentication cache + private static long DEFAULT_AUTHENTICATION_CACHE_SIZE = 1000; + + // how large to make the authorization cache + private static long DEFAULT_AUTHORIZATION_CACHE_SIZE = 1000; + // how long (in ms) to wait to acquire a file lock on the journal private static long DEFAULT_JOURNAL_LOCK_ACQUISITION_TIMEOUT = -1; @@ -680,6 +686,20 @@ public final class ActiveMQDefaultConfiguration { return DEFAULT_SECURITY_INVALIDATION_INTERVAL; } + /** + * how large to make the authentication cache + */ + public static long getDefaultAuthenticationCacheSize() { + return DEFAULT_AUTHENTICATION_CACHE_SIZE; + } + + /** + * how large to make the authorization cache + */ + public static long getDefaultAuthorizationCacheSize() { + return DEFAULT_AUTHORIZATION_CACHE_SIZE; + } + /** * how long (in ms) to wait to acquire a file lock on the journal */ diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/ActiveMQServerControl.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/ActiveMQServerControl.java index 84a70ad409..ea739cd38e 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/ActiveMQServerControl.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/ActiveMQServerControl.java @@ -460,6 +460,18 @@ public interface ActiveMQServerControl { @Attribute(desc = ADDRESS_MEMORY_USAGE_PERCENTAGE_DESCRIPTION) int getAddressMemoryUsagePercentage(); + /** + * Returns the runtime size of the authentication cache + */ + @Attribute(desc = "The runtime size of the authentication cache") + long getAuthenticationCacheSize(); + + /** + * Returns the runtime size of the authorization cache + */ + @Attribute(desc = "The runtime size of the authorization cache") + long getAuthorizationCacheSize(); + // Operations ---------------------------------------------------- @Operation(desc = "Isolate the broker", impact = MBeanOperationInfo.ACTION) boolean freezeReplication(); diff --git a/artemis-features/src/main/resources/features.xml b/artemis-features/src/main/resources/features.xml index 5d3c6a2980..96aaa1a19e 100644 --- a/artemis-features/src/main/resources/features.xml +++ b/artemis-features/src/main/resources/features.xml @@ -71,6 +71,7 @@ mvn:org.apache.commons/commons-text/${commons.text.version} mvn:org.apache.commons/commons-lang3/${commons.lang.version} mvn:org.jctools/jctools-core/${jctools.version} + mvn:com.google.guava/guava/${guava.version} diff --git a/artemis-server/pom.xml b/artemis-server/pom.xml index aa41cf256c..378b30e57f 100644 --- a/artemis-server/pom.xml +++ b/artemis-server/pom.xml @@ -45,8 +45,12 @@ true - com.google.errorprone - error_prone_core + com.google.errorprone + error_prone_core + + + com.google.guava + guava org.jboss.logging diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java index 67d72de3a7..0a50775005 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java @@ -211,6 +211,28 @@ public interface Configuration { */ Configuration setSecurityInvalidationInterval(long interval); + /** + * Sets the size of the authentication cache. + */ + Configuration setAuthenticationCacheSize(long size); + + /** + * Returns the configured size of the authentication cache.
+ * Default value is {@link org.apache.activemq.artemis.api.config.ActiveMQDefaultConfiguration#DEFAULT_AUTHENTICATION_CACHE_SIZE}. + */ + long getAuthenticationCacheSize(); + + /** + * Sets the size of the authorization cache. + */ + Configuration setAuthorizationCacheSize(long size); + + /** + * Returns the configured size of the authorization cache.
+ * Default value is {@link org.apache.activemq.artemis.api.config.ActiveMQDefaultConfiguration#DEFAULT_AUTHORIZATION_CACHE_SIZE}. + */ + long getAuthorizationCacheSize(); + /** * Returns whether security is enabled for this server.
* Default value is {@link org.apache.activemq.artemis.api.config.ActiveMQDefaultConfiguration#DEFAULT_SECURITY_ENABLED}. diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java index ca59995a96..10ab2ce2c0 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java @@ -122,6 +122,10 @@ public class ConfigurationImpl implements Configuration, Serializable { private long securityInvalidationInterval = ActiveMQDefaultConfiguration.getDefaultSecurityInvalidationInterval(); + private long authenticationCacheSize = ActiveMQDefaultConfiguration.getDefaultAuthenticationCacheSize(); + + private long authorizationCacheSize = ActiveMQDefaultConfiguration.getDefaultAuthorizationCacheSize(); + private boolean securityEnabled = ActiveMQDefaultConfiguration.isDefaultSecurityEnabled(); private boolean gracefulShutdownEnabled = ActiveMQDefaultConfiguration.isDefaultGracefulShutdownEnabled(); @@ -506,6 +510,28 @@ public class ConfigurationImpl implements Configuration, Serializable { return this; } + @Override + public long getAuthenticationCacheSize() { + return authenticationCacheSize; + } + + @Override + public ConfigurationImpl setAuthenticationCacheSize(final long size) { + authenticationCacheSize = size; + return this; + } + + @Override + public long getAuthorizationCacheSize() { + return authorizationCacheSize; + } + + @Override + public ConfigurationImpl setAuthorizationCacheSize(final long size) { + authorizationCacheSize = size; + return this; + } + @Override public long getConnectionTTLOverride() { return connectionTTLOverride; diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java index 95cae11e01..0952ff560d 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java @@ -370,7 +370,11 @@ public final class FileConfigurationParser extends XMLConfigurationUtil { config.setJMXUseBrokerName(getBoolean(e, "jmx-use-broker-name", config.isJMXUseBrokerName())); - config.setSecurityInvalidationInterval(getLong(e, "security-invalidation-interval", config.getSecurityInvalidationInterval(), Validators.GT_ZERO)); + config.setSecurityInvalidationInterval(getLong(e, "security-invalidation-interval", config.getSecurityInvalidationInterval(), Validators.GE_ZERO)); + + config.setAuthenticationCacheSize(getLong(e, "authentication-cache-size", config.getAuthenticationCacheSize(), Validators.GE_ZERO)); + + config.setAuthorizationCacheSize(getLong(e, "authorization-cache-size", config.getAuthorizationCacheSize(), Validators.GE_ZERO)); config.setConnectionTTLOverride(getLong(e, "connection-ttl-override", config.getConnectionTTLOverride(), Validators.MINUS_ONE_OR_GT_ZERO)); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java index e6493af522..1dd405299c 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java @@ -91,6 +91,7 @@ import org.apache.activemq.artemis.core.postoffice.impl.LocalQueueBinding; import org.apache.activemq.artemis.core.remoting.server.RemotingService; import org.apache.activemq.artemis.core.security.CheckType; import org.apache.activemq.artemis.core.security.Role; +import org.apache.activemq.artemis.core.security.impl.SecurityStoreImpl; import org.apache.activemq.artemis.core.server.ActiveMQMessageBundle; import org.apache.activemq.artemis.core.server.ActiveMQServer; import org.apache.activemq.artemis.core.server.ActiveMQServerLogger; @@ -757,6 +758,22 @@ public class ActiveMQServerControlImpl extends AbstractControl implements Active return (int) result; } + @Override + public long getAuthenticationCacheSize() { + if (AuditLogger.isEnabled()) { + AuditLogger.getAuthenticationCacheSize(this.server); + } + return ((SecurityStoreImpl)server.getSecurityStore()).getAuthenticationCacheSize(); + } + + @Override + public long getAuthorizationCacheSize() { + if (AuditLogger.isEnabled()) { + AuditLogger.getAuthorizationCacheSize(this.server); + } + return ((SecurityStoreImpl)server.getSecurityStore()).getAuthorizationCacheSize(); + } + @Override public boolean freezeReplication() { if (AuditLogger.isEnabled()) { diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java index a1699b5555..95e3b626b1 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java @@ -16,11 +16,14 @@ */ package org.apache.activemq.artemis.core.security.impl; +import javax.security.auth.Subject; import javax.security.cert.X509Certificate; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; +import org.apache.activemq.artemis.api.core.Pair; import org.apache.activemq.artemis.api.core.SimpleString; import org.apache.activemq.artemis.api.core.management.CoreNotificationType; import org.apache.activemq.artemis.api.core.management.ManagementHelper; @@ -41,6 +44,8 @@ import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager; import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager2; import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager3; import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager4; +import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager5; +import org.apache.activemq.artemis.spi.core.security.jaas.UserPrincipal; import org.apache.activemq.artemis.utils.CompositeAddress; import org.apache.activemq.artemis.utils.collections.ConcurrentHashSet; import org.apache.activemq.artemis.utils.collections.TypedProperties; @@ -57,11 +62,9 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC private final ActiveMQSecurityManager securityManager; - private final ConcurrentMap> cache = new ConcurrentHashMap<>(); + private final Cache> authorizationCache; - private final long invalidationInterval; - - private volatile long lastCheck; + private final Cache> authenticationCache; private boolean securityEnabled; @@ -82,14 +85,23 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC final boolean securityEnabled, final String managementClusterUser, final String managementClusterPassword, - final NotificationService notificationService) { + final NotificationService notificationService, + final long authenticationCacheSize, + final long authorizationCacheSize) { this.securityRepository = securityRepository; this.securityManager = securityManager; - this.invalidationInterval = invalidationInterval; this.securityEnabled = securityEnabled; this.managementClusterUser = managementClusterUser; this.managementClusterPassword = managementClusterPassword; this.notificationService = notificationService; + authenticationCache = CacheBuilder.newBuilder() + .maximumSize(authenticationCacheSize) + .expireAfterWrite(invalidationInterval, TimeUnit.MILLISECONDS) + .build(); + authorizationCache = CacheBuilder.newBuilder() + .maximumSize(authorizationCacheSize) + .expireAfterWrite(invalidationInterval, TimeUnit.MILLISECONDS) + .build(); this.securityRepository.registerListener(this); } @@ -142,23 +154,40 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC String validatedUser = null; boolean userIsValid = false; + boolean check = true; - if (securityManager instanceof ActiveMQSecurityManager4) { - validatedUser = ((ActiveMQSecurityManager4) securityManager).validateUser(user, password, connection, securityDomain); - } else if (securityManager instanceof ActiveMQSecurityManager3) { - validatedUser = ((ActiveMQSecurityManager3) securityManager).validateUser(user, password, connection); - } else if (securityManager instanceof ActiveMQSecurityManager2) { - userIsValid = ((ActiveMQSecurityManager2) securityManager).validateUser(user, password, CertificateUtil.getCertsFromConnection(connection)); - } else { - userIsValid = securityManager.validateUser(user, password); + Pair cacheEntry = authenticationCache.getIfPresent(createAuthenticationCacheKey(user, password, connection)); + if (cacheEntry != null) { + if (!cacheEntry.getA()) { + // cached authentication failed previously so don't check again + check = false; + } else { + // cached authentication succeeded previously so don't check again + check = false; + userIsValid = true; + validatedUser = getUserFromSubject(cacheEntry.getB()); + } } - if (!userIsValid && validatedUser == null) { - String certSubjectDN = "unavailable"; - X509Certificate[] certs = CertificateUtil.getCertsFromConnection(connection); - if (certs != null && certs.length > 0 && certs[0] != null) { - certSubjectDN = certs[0].getSubjectDN().getName(); + if (check) { + if (securityManager instanceof ActiveMQSecurityManager5) { + Subject subject = ((ActiveMQSecurityManager5) securityManager).authenticate(user, password, connection, securityDomain); + authenticationCache.put(createAuthenticationCacheKey(user, password, connection), new Pair<>(subject != null, subject)); + validatedUser = getUserFromSubject(subject); + } else if (securityManager instanceof ActiveMQSecurityManager4) { + validatedUser = ((ActiveMQSecurityManager4) securityManager).validateUser(user, password, connection, securityDomain); + } else if (securityManager instanceof ActiveMQSecurityManager3) { + validatedUser = ((ActiveMQSecurityManager3) securityManager).validateUser(user, password, connection); + } else if (securityManager instanceof ActiveMQSecurityManager2) { + userIsValid = ((ActiveMQSecurityManager2) securityManager).validateUser(user, password, CertificateUtil.getCertsFromConnection(connection)); + } else { + userIsValid = securityManager.validateUser(user, password); } + } + + // authentication failed, send a notification & throw an exception + if (!userIsValid && validatedUser == null) { + String certSubjectDN = getCertSubjectDN(connection); if (notificationService != null) { TypedProperties props = new TypedProperties(); @@ -184,6 +213,15 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC return null; } + public String getCertSubjectDN(RemotingConnection connection) { + String certSubjectDN = "unavailable"; + X509Certificate[] certs = CertificateUtil.getCertsFromConnection(connection); + if (certs != null && certs.length > 0 && certs[0] != null) { + certSubjectDN = certs[0].getSubjectDN().getName(); + } + return certSubjectDN; + } + @Override public void check(final SimpleString address, final CheckType checkType, @@ -201,8 +239,8 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC logger.trace("checking access permissions to " + address); } - String user = session.getUsername(); // bypass permission checks for management cluster user + String user = session.getUsername(); if (managementClusterUser.equals(user) && session.getPassword().equals(managementClusterPassword)) { return; } @@ -223,20 +261,20 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC } } - if (checkCached(isFullyQualified ? fqqn : address, user, checkType)) { + if (checkAuthorizationCache(isFullyQualified ? fqqn : address, user, checkType)) { return; } - final boolean validated; - if (securityManager instanceof ActiveMQSecurityManager4) { - final ActiveMQSecurityManager4 securityManager4 = (ActiveMQSecurityManager4) securityManager; - validated = securityManager4.validateUserAndRole(user, session.getPassword(), roles, checkType, saddress, session.getRemotingConnection(), session.getSecurityDomain()) != null; + final Boolean validated; + if (securityManager instanceof ActiveMQSecurityManager5) { + Subject subject = getSubjectForAuthorization(session, ((ActiveMQSecurityManager5) securityManager)); + validated = ((ActiveMQSecurityManager5) securityManager).authorize(subject, roles, checkType); + } else if (securityManager instanceof ActiveMQSecurityManager4) { + validated = ((ActiveMQSecurityManager4) securityManager).validateUserAndRole(user, session.getPassword(), roles, checkType, saddress, session.getRemotingConnection(), session.getSecurityDomain()) != null; } else if (securityManager instanceof ActiveMQSecurityManager3) { - final ActiveMQSecurityManager3 securityManager3 = (ActiveMQSecurityManager3) securityManager; - validated = securityManager3.validateUserAndRole(user, session.getPassword(), roles, checkType, saddress, session.getRemotingConnection()) != null; + validated = ((ActiveMQSecurityManager3) securityManager).validateUserAndRole(user, session.getPassword(), roles, checkType, saddress, session.getRemotingConnection()) != null; } else if (securityManager instanceof ActiveMQSecurityManager2) { - final ActiveMQSecurityManager2 securityManager2 = (ActiveMQSecurityManager2) securityManager; - validated = securityManager2.validateUserAndRole(user, session.getPassword(), roles, checkType, saddress, session.getRemotingConnection()); + validated = ((ActiveMQSecurityManager2) securityManager).validateUserAndRole(user, session.getPassword(), roles, checkType, saddress, session.getRemotingConnection()); } else { validated = securityManager.validateUserAndRole(user, session.getPassword(), roles, checkType); } @@ -263,11 +301,16 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC AuditLogger.securityFailure(ex); throw ex; } + // if we get here we're granted, add to the cache - ConcurrentHashSet set = new ConcurrentHashSet<>(); - ConcurrentHashSet act = cache.putIfAbsent(user + "." + checkType.name(), set); + ConcurrentHashSet set; + String key = createAuthorizationCacheKey(user, checkType); + ConcurrentHashSet act = authorizationCache.getIfPresent(key); if (act != null) { set = act; + } else { + set = new ConcurrentHashSet<>(); + authorizationCache.put(key, set); } set.add(isFullyQualified ? fqqn : address); } @@ -275,39 +318,78 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC @Override public void onChange() { - invalidateCache(); + invalidateAuthorizationCache(); + // we don't invalidate the authentication cache here because it's not necessary } - // Public -------------------------------------------------------- + public static String getUserFromSubject(Subject subject) { + if (subject == null) { + return null; + } - // Protected ----------------------------------------------------- + String validatedUser = ""; + Set users = subject.getPrincipals(UserPrincipal.class); - // Package Private ----------------------------------------------- - - // Private ------------------------------------------------------- - private void invalidateCache() { - cache.clear(); + // should only ever be 1 UserPrincipal + for (UserPrincipal userPrincipal : users) { + validatedUser = userPrincipal.getName(); + } + return validatedUser; } - private boolean checkCached(final SimpleString dest, final String user, final CheckType checkType) { - long now = System.currentTimeMillis(); + /** + * Get the cached Subject. If the Subject is not in the cache then authenticate again to retrieve it. + * + * @param auth contains the authentication data + * @param securityManager used to authenticate the user if the Subject is not in the cache + * @return the authenticated Subject with all associated role principals + */ + private Subject getSubjectForAuthorization(SecurityAuth auth, ActiveMQSecurityManager5 securityManager) { + Pair cached = authenticationCache.getIfPresent(createAuthenticationCacheKey(auth.getUsername(), auth.getPassword(), auth.getRemotingConnection())); + /* + * We don't need to worry about the cached boolean being false as users always have to + * successfully authenticate before requesting authorization for anything. + */ + if (cached == null) { + return securityManager.authenticate(auth.getUsername(), auth.getPassword(), auth.getRemotingConnection(), auth.getSecurityDomain()); + } + return cached.getB(); + } + // public for testing purposes + public void invalidateAuthorizationCache() { + authorizationCache.invalidateAll(); + } + + // public for testing purposes + public void invalidateAuthenticationCache() { + authenticationCache.invalidateAll(); + } + + public long getAuthenticationCacheSize() { + return authenticationCache.size(); + } + + public long getAuthorizationCacheSize() { + return authorizationCache.size(); + } + + private boolean checkAuthorizationCache(final SimpleString dest, final String user, final CheckType checkType) { boolean granted = false; - if (now - lastCheck > invalidationInterval) { - invalidateCache(); - - lastCheck = now; - } else { - ConcurrentHashSet act = cache.get(user + "." + checkType.name()); - if (act != null) { - granted = act.contains(dest); - } + ConcurrentHashSet act = authorizationCache.getIfPresent(createAuthorizationCacheKey(user, checkType)); + if (act != null) { + granted = act.contains(dest); } return granted; } - // Inner class --------------------------------------------------- + private String createAuthenticationCacheKey(String username, String password, RemotingConnection connection) { + return username + password + getCertSubjectDN(connection); + } + private String createAuthorizationCacheKey(String user, CheckType checkType) { + return user + "." + checkType.name(); + } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java index d6c606fbec..43a42e62bf 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java @@ -2884,7 +2884,7 @@ public class ActiveMQServerImpl implements ActiveMQServer { ActiveMQServerLogger.LOGGER.clusterSecurityRisk(); } - securityStore = new SecurityStoreImpl(securityRepository, securityManager, configuration.getSecurityInvalidationInterval(), configuration.isSecurityEnabled(), configuration.getClusterUser(), configuration.getClusterPassword(), managementService); + securityStore = new SecurityStoreImpl(securityRepository, securityManager, configuration.getSecurityInvalidationInterval(), configuration.isSecurityEnabled(), configuration.getClusterUser(), configuration.getClusterPassword(), managementService, configuration.getAuthenticationCacheSize(), configuration.getAuthorizationCacheSize()); queueFactory = new QueueFactoryImpl(executorFactory, scheduledPool, addressSettingsRepository, storageManager, this); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/ActiveMQJAASSecurityManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/ActiveMQJAASSecurityManager.java index 1c22412ad0..f90451d76f 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/ActiveMQJAASSecurityManager.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/ActiveMQJAASSecurityManager.java @@ -34,7 +34,6 @@ import org.apache.activemq.artemis.logs.AuditLogger; import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection; import org.apache.activemq.artemis.spi.core.security.jaas.JaasCallbackHandler; import org.apache.activemq.artemis.spi.core.security.jaas.RolePrincipal; -import org.apache.activemq.artemis.spi.core.security.jaas.UserPrincipal; import org.jboss.logging.Logger; import static org.apache.activemq.artemis.core.remoting.CertificateUtil.getCertsFromConnection; @@ -45,7 +44,7 @@ import static org.apache.activemq.artemis.core.remoting.CertificateUtil.getCerts * The {@link Subject} returned by the login context is expecting to have a set of {@link RolePrincipal} for each * role of the user. */ -public class ActiveMQJAASSecurityManager implements ActiveMQSecurityManager4 { +public class ActiveMQJAASSecurityManager implements ActiveMQSecurityManager5 { private static final Logger logger = Logger.getLogger(ActiveMQJAASSecurityManager.class); @@ -95,9 +94,9 @@ public class ActiveMQJAASSecurityManager implements ActiveMQSecurityManager4 { } @Override - public String validateUser(final String user, final String password, RemotingConnection remotingConnection, final String securityDomain) { + public Subject authenticate(final String user, final String password, RemotingConnection remotingConnection, final String securityDomain) { try { - return getUserFromSubject(getAuthenticatedSubject(user, password, remotingConnection, securityDomain)); + return getAuthenticatedSubject(user, password, remotingConnection, securityDomain); } catch (LoginException e) { if (logger.isDebugEnabled()) { logger.debug("Couldn't validate user", e); @@ -106,49 +105,24 @@ public class ActiveMQJAASSecurityManager implements ActiveMQSecurityManager4 { } } - public String getUserFromSubject(Subject subject) { - String validatedUser = ""; - Set users = subject.getPrincipals(UserPrincipal.class); - - // should only ever be 1 UserPrincipal - for (UserPrincipal userPrincipal : users) { - validatedUser = userPrincipal.getName(); - } - return validatedUser; - } - @Override public boolean validateUserAndRole(String user, String password, Set roles, CheckType checkType) { throw new UnsupportedOperationException("Invoke validateUserAndRole(String, String, Set, CheckType, String, RemotingConnection, String) instead"); } @Override - public String validateUserAndRole(final String user, - final String password, - final Set roles, - final CheckType checkType, - final String address, - final RemotingConnection remotingConnection, - final String securityDomain) { - Subject localSubject; - try { - localSubject = getAuthenticatedSubject(user, password, remotingConnection, securityDomain); - } catch (LoginException e) { - if (logger.isDebugEnabled()) { - logger.debug("Couldn't validate user", e); - } - return null; - } - + public boolean authorize(final Subject subject, + final Set roles, + final CheckType checkType) { boolean authorized = false; - if (localSubject != null) { + if (subject != null) { Set rolesWithPermission = getPrincipalsInRole(checkType, roles); // Check the caller's roles Set rolesForSubject = new HashSet<>(); try { - rolesForSubject.addAll(localSubject.getPrincipals(Class.forName(rolePrincipalClass).asSubclass(Principal.class))); + rolesForSubject.addAll(subject.getPrincipals(Class.forName(rolePrincipalClass).asSubclass(Principal.class))); } catch (Exception e) { ActiveMQServerLogger.LOGGER.failedToFindRolesForTheSubject(e); } @@ -169,11 +143,7 @@ public class ActiveMQJAASSecurityManager implements ActiveMQSecurityManager4 { } } - if (authorized) { - return getUserFromSubject(localSubject); - } else { - return null; - } + return authorized; } private Subject getAuthenticatedSubject(final String user, diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/ActiveMQSecurityManager5.java b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/ActiveMQSecurityManager5.java new file mode 100644 index 0000000000..e0438663bd --- /dev/null +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/ActiveMQSecurityManager5.java @@ -0,0 +1,61 @@ +/* + * 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.spi.core.security; + +import javax.security.auth.Subject; +import java.util.Set; + +import org.apache.activemq.artemis.core.security.CheckType; +import org.apache.activemq.artemis.core.security.Role; +import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection; + +/** + * Used to validate whether a user is authorized to connect to the + * server and perform certain functions on certain addresses + * + * This is an evolution of {@link ActiveMQSecurityManager4} + * that integrates with the new Subject caching functionality. + */ +public interface ActiveMQSecurityManager5 extends ActiveMQSecurityManager { + + /** + * is this a valid user. + * + * This method is called instead of + * {@link ActiveMQSecurityManager#validateUser(String, String)}. + * + * @param user the user + * @param password the user's password + * @param remotingConnection the user's connection which contains any corresponding SSL certs + * @param securityDomain the name of the JAAS security domain to use (can be null) + * @return the Subject of the authenticated user or null if the user isn't authenticated + */ + Subject authenticate(String user, String password, RemotingConnection remotingConnection, String securityDomain); + + /** + * Determine whether the given user has the correct role for the given check type. + * + * This method is called instead of + * {@link ActiveMQSecurityManager#validateUserAndRole(String, String, Set, CheckType)}. + * + * @param subject the Subject to authorize + * @param roles the roles configured in the security-settings + * @param checkType which permission to validate + * @return true if the user is authorized, else false + */ + boolean authorize(Subject subject, Set roles, CheckType checkType); +} diff --git a/artemis-server/src/main/resources/schema/artemis-configuration.xsd b/artemis-server/src/main/resources/schema/artemis-configuration.xsd index 6387a69185..492b47cb18 100644 --- a/artemis-server/src/main/resources/schema/artemis-configuration.xsd +++ b/artemis-server/src/main/resources/schema/artemis-configuration.xsd @@ -134,7 +134,23 @@ - how long (in ms) to wait before invalidating the security cache + how long (in ms) to wait before invalidating an entry in the authentication or authorization cache + + + + + + + + how large to make the authentication cache + + + + + + + + how large to make the authorization cache diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationTest.java index c43618c81b..d34d40d26a 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationTest.java +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationTest.java @@ -102,6 +102,8 @@ public class FileConfigurationTest extends ConfigurationImplTest { Assert.assertEquals(54321, conf.getThreadPoolMaxSize()); Assert.assertEquals(false, conf.isSecurityEnabled()); Assert.assertEquals(5423, conf.getSecurityInvalidationInterval()); + Assert.assertEquals(333, conf.getAuthenticationCacheSize()); + Assert.assertEquals(444, conf.getAuthorizationCacheSize()); Assert.assertEquals(true, conf.isWildcardRoutingEnabled()); Assert.assertEquals(new SimpleString("Giraffe"), conf.getManagementAddress()); Assert.assertEquals(new SimpleString("Whatever"), conf.getManagementNotificationAddress()); diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/jaas/JAASSecurityManagerTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/jaas/JAASSecurityManagerTest.java index d9e535b74e..b2f73b1266 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/jaas/JAASSecurityManagerTest.java +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/jaas/JAASSecurityManagerTest.java @@ -16,8 +16,11 @@ */ package org.apache.activemq.artemis.core.security.jaas; +import javax.security.auth.Subject; + import org.apache.activemq.artemis.core.security.CheckType; import org.apache.activemq.artemis.core.security.Role; +import org.apache.activemq.artemis.core.security.impl.SecurityStoreImpl; import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager; import org.jboss.logging.Logger; import org.junit.Rule; @@ -38,6 +41,7 @@ import java.util.Set; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; @RunWith(Parameterized.class) public class JAASSecurityManagerTest { @@ -80,18 +84,17 @@ public class JAASSecurityManagerTest { } ActiveMQJAASSecurityManager securityManager = new ActiveMQJAASSecurityManager("PropertiesLogin"); - String result = securityManager.validateUser("first", "secret", null, null); + Subject result = securityManager.authenticate("first", "secret", null, null); assertNotNull(result); - assertEquals("first", result); + assertEquals("first", SecurityStoreImpl.getUserFromSubject(result)); Role role = new Role("programmers", true, true, true, true, true, true, true, true, true, true); Set roles = new HashSet<>(); roles.add(role); - result = securityManager.validateUserAndRole("first", "secret", roles, CheckType.SEND, "someaddress", null, null); + boolean authorizationResult = securityManager.authorize(result, roles, CheckType.SEND); - assertNotNull(result); - assertEquals("first", result); + assertTrue(authorizationResult); } finally { Thread.currentThread().setContextClassLoader(existingLoader); diff --git a/artemis-server/src/test/resources/ConfigurationTest-full-config.xml b/artemis-server/src/test/resources/ConfigurationTest-full-config.xml index 820d39dd90..2d1e1907e6 100644 --- a/artemis-server/src/test/resources/ConfigurationTest-full-config.xml +++ b/artemis-server/src/test/resources/ConfigurationTest-full-config.xml @@ -28,6 +28,8 @@ true 12345 5423 + 333 + 444 123 true Giraffe diff --git a/artemis-server/src/test/resources/ConfigurationTest-xinclude-config.xml b/artemis-server/src/test/resources/ConfigurationTest-xinclude-config.xml index d4bdc8be14..46408cf2be 100644 --- a/artemis-server/src/test/resources/ConfigurationTest-xinclude-config.xml +++ b/artemis-server/src/test/resources/ConfigurationTest-xinclude-config.xml @@ -29,6 +29,8 @@ true 12345 5423 + 333 + 444 123 true Giraffe diff --git a/docs/user-manual/en/security.md b/docs/user-manual/en/security.md index 5bf08bf1a1..7ba7da8da5 100644 --- a/docs/user-manual/en/security.md +++ b/docs/user-manual/en/security.md @@ -6,9 +6,17 @@ you can configure it. To disable security completely simply set the `security-enabled` property to `false` in the `broker.xml` file. -For performance reasons security is cached and invalidated every so long. To -change this period set the property `security-invalidation-interval`, which is -in milliseconds. The default is `10000` ms. +For performance reasons both **authentication and authorization is cached** +independently. Entries are removed from the caches (i.e. invalidated) either +when the cache reaches its maximum size in which case the least-recently used +entry is removed or when an entry has been in the cache "too long". + +The size of the caches are controlled by the `authentication-cache-size` and +`authorization-cache-size` configuration parameters. Both deafult to `1000`. + +How long cache entries are valid is controlled by +`security-invalidation-interval`, which is in milliseconds. Using `0` will +disable caching. The default is `10000` ms. ## Tracking the Validated User diff --git a/examples/features/standard/security-manager/src/main/java/org/apache/activemq/artemis/jms/example/JAASSecurityManagerWrapper.java b/examples/features/standard/security-manager/src/main/java/org/apache/activemq/artemis/jms/example/JAASSecurityManagerWrapper.java index 053fc02c27..251e4672a2 100644 --- a/examples/features/standard/security-manager/src/main/java/org/apache/activemq/artemis/jms/example/JAASSecurityManagerWrapper.java +++ b/examples/features/standard/security-manager/src/main/java/org/apache/activemq/artemis/jms/example/JAASSecurityManagerWrapper.java @@ -17,6 +17,7 @@ package org.apache.activemq.artemis.jms.example; +import javax.security.auth.Subject; import java.util.Map; import java.util.Set; @@ -25,28 +26,23 @@ import org.apache.activemq.artemis.core.security.Role; import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection; import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager; import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager; -import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager4; +import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager5; -public class JAASSecurityManagerWrapper implements ActiveMQSecurityManager4 { +public class JAASSecurityManagerWrapper implements ActiveMQSecurityManager5 { ActiveMQJAASSecurityManager activeMQJAASSecurityManager; @Override - public String validateUser(String user, String password, RemotingConnection remotingConnection, String securityDomain) { - System.out.println("validateUser(" + user + ", " + password + ", " + remotingConnection.getRemoteAddress() + ")"); - return activeMQJAASSecurityManager.validateUser(user, password, remotingConnection, securityDomain); + public Subject authenticate(String user, String password, RemotingConnection remotingConnection, String securityDomain) { + System.out.println("authenticate(" + user + ", " + password + ", " + remotingConnection.getRemoteAddress() + ")"); + return activeMQJAASSecurityManager.authenticate(user, password, remotingConnection, securityDomain); } - @Override - public String validateUserAndRole(String user, - String password, - Set roles, - CheckType checkType, - String address, - RemotingConnection remotingConnection, - String securityDomain) { - System.out.println("validateUserAndRole(" + user + ", " + password + ", " + roles + ", " + checkType + ", " + address + ", " + remotingConnection.getRemoteAddress() + ")"); - return activeMQJAASSecurityManager.validateUserAndRole(user, password, roles, checkType, address, remotingConnection, securityDomain); + public boolean authorize(Subject subject, + Set roles, + CheckType checkType) { + System.out.println("authorize(" + subject + ", " + roles + ", " + checkType + ")"); + return activeMQJAASSecurityManager.authorize(subject, roles, checkType); } @Override diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java index 9347cc2e74..05faa6ac8a 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java @@ -197,6 +197,32 @@ public class ActiveMQServerControlTest extends ManagementTestBase { Assert.assertTrue(serverControl.isActive()); } + @Test + public void testSecurityCacheSizes() throws Exception { + ActiveMQServerControl serverControl = createManagementControl(); + + Assert.assertEquals(usingCore() ? 1 : 0, serverControl.getAuthenticationCacheSize()); + Assert.assertEquals(usingCore() ? 7 : 0, serverControl.getAuthorizationCacheSize()); + + ServerLocator loc = createInVMNonHALocator(); + ClientSessionFactory csf = createSessionFactory(loc); + ClientSession session = csf.createSession("myUser", "myPass", false, true, false, false, 0); + session.start(); + + final String address = "ADDRESS"; + + serverControl.createAddress(address, "MULTICAST"); + + ClientProducer producer = session.createProducer(address); + + ClientMessage m = session.createMessage(true); + m.putStringProperty("hello", "world"); + producer.send(m); + + Assert.assertEquals(usingCore() ? 2 : 1, serverControl.getAuthenticationCacheSize()); + Assert.assertEquals(usingCore() ? 8 : 1, serverControl.getAuthorizationCacheSize()); + } + @Test public void testGetConnectors() throws Exception { ActiveMQServerControl serverControl = createManagementControl(); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlUsingCoreTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlUsingCoreTest.java index 95f051341a..313084c24a 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlUsingCoreTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlUsingCoreTest.java @@ -768,6 +768,16 @@ public class ActiveMQServerControlUsingCoreTest extends ActiveMQServerControlTes return 0; } + @Override + public long getAuthenticationCacheSize() { + return (Long) proxy.retrieveAttributeValue("AuthenticationCacheSize", Long.class); + } + + @Override + public long getAuthorizationCacheSize() { + return (Long) proxy.retrieveAttributeValue("AuthorizationCacheSize", Long.class); + } + @Override public double getDiskStoreUsage() { try { diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java index 87cf781508..8535cb0385 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java @@ -51,6 +51,7 @@ import org.apache.activemq.artemis.core.remoting.impl.invm.InVMConnection; import org.apache.activemq.artemis.core.remoting.impl.netty.TransportConstants; import org.apache.activemq.artemis.core.security.CheckType; import org.apache.activemq.artemis.core.security.Role; +import org.apache.activemq.artemis.core.security.impl.SecurityStoreImpl; import org.apache.activemq.artemis.core.server.ActiveMQServer; import org.apache.activemq.artemis.core.server.ActiveMQServers; import org.apache.activemq.artemis.core.server.Queue; @@ -66,6 +67,7 @@ import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager4; import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; import org.apache.activemq.artemis.tests.util.CreateMessage; import org.apache.activemq.artemis.utils.CompositeAddress; +import org.apache.activemq.artemis.utils.Wait; import org.apache.activemq.command.ActiveMQQueue; import org.junit.Assert; import org.junit.Before; @@ -1412,6 +1414,9 @@ public class SecurityTest extends ActiveMQTestBase { securityManager.getConfiguration().addRole("auser", "receiver"); + // invalidate the authentication cache so the new role will be picked up + ((SecurityStoreImpl)server.getSecurityStore()).invalidateAuthenticationCache(); + session.createConsumer(SecurityTest.queueA); // Removing the Role... the check should be cached, so the next createConsumer shouldn't fail @@ -1483,7 +1488,7 @@ public class SecurityTest extends ActiveMQTestBase { @Test public void testSendMessageUpdateSender() throws Exception { - Configuration configuration = createDefaultInVMConfig().setSecurityEnabled(true).setSecurityInvalidationInterval(-1); + Configuration configuration = createDefaultInVMConfig().setSecurityEnabled(true).setSecurityInvalidationInterval(1000); ActiveMQServer server = createServer(false, configuration); server.start(); HierarchicalRepository> securityRepository = server.getSecurityRepository(); @@ -1518,7 +1523,14 @@ public class SecurityTest extends ActiveMQTestBase { securityManager.getConfiguration().addRole("auser", "receiver"); - session.createConsumer(SecurityTest.queueA); + Wait.assertTrue(() -> { + try { + session.createConsumer(SecurityTest.queueA); + return true; + } catch (Exception e) { + return false; + } + }, 2000, 100); // Removing the Role... the check should be cached... but we used // setSecurityInvalidationInterval(0), so the diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompWithClientIdValidationTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompWithClientIdValidationTest.java index 22f123ab72..070d87cecd 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompWithClientIdValidationTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompWithClientIdValidationTest.java @@ -17,6 +17,7 @@ package org.apache.activemq.artemis.tests.integration.stomp; +import javax.security.auth.Subject; import java.lang.management.ManagementFactory; import org.apache.activemq.artemis.api.core.TransportConfiguration; @@ -46,13 +47,14 @@ public class StompWithClientIdValidationTest extends StompTestBase { .setSecurityEnabled(isSecurityEnabled()) .setPersistenceEnabled(isPersistenceEnabled()) .addAcceptorConfiguration("stomp", "tcp://localhost:61613?enabledProtocols=STOMP") - .addAcceptorConfiguration(new TransportConfiguration(InVMAcceptorFactory.class.getName())); + .addAcceptorConfiguration(new TransportConfiguration(InVMAcceptorFactory.class.getName())) + .setSecurityInvalidationInterval(0); // disable caching ActiveMQJAASSecurityManager securityManager = new ActiveMQJAASSecurityManager(InVMLoginModule.class.getName(), new SecurityConfiguration()) { @Override - public String validateUser(String user, String password, RemotingConnection remotingConnection, String securityDomain) { + public Subject authenticate(String user, String password, RemotingConnection remotingConnection, String securityDomain) { - String validatedUser = super.validateUser(user, password, remotingConnection, securityDomain); + Subject validatedUser = super.authenticate(user, password, remotingConnection, securityDomain); if (validatedUser == null) { return null; }