This is an automated email from the ASF dual-hosted git repository.
clebertsuconic pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git
The following commit(s) were added to refs/heads/main by this push:
new 44d57d6f43 ARTEMIS-5310 check class of Principals in Subject
44d57d6f43 is described below
commit 44d57d6f43d23833e904e0be2ddfd1613374f7cc
Author: Justin Bertram <[email protected]>
AuthorDate: Fri Feb 21 13:43:06 2025 -0600
ARTEMIS-5310 check class of Principals in Subject
---
.../core/security/impl/SecurityStoreImpl.java | 30 +++++++++++-
.../artemis/core/server/ActiveMQServerLogger.java | 3 ++
.../core/security/impl/SecurityStoreImplTest.java | 55 ++++++++++++++++++++--
3 files changed, 82 insertions(+), 6 deletions(-)
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 8587683426..d331a224d5 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
@@ -26,6 +26,7 @@ import java.security.NoSuchAlgorithmException;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLongFieldUpdater;
+import java.util.stream.Collectors;
import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.Caffeine;
@@ -47,12 +48,14 @@ import
org.apache.activemq.artemis.core.settings.HierarchicalRepository;
import
org.apache.activemq.artemis.core.settings.HierarchicalRepositoryChangeListener;
import org.apache.activemq.artemis.logs.AuditLogger;
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.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.NoCacheLoginException;
+import org.apache.activemq.artemis.spi.core.security.jaas.UserPrincipal;
import org.apache.activemq.artemis.utils.ByteUtil;
import org.apache.activemq.artemis.utils.CompositeAddress;
import org.apache.activemq.artemis.utils.collections.ConcurrentHashSet;
@@ -219,8 +222,10 @@ public class SecurityStoreImpl implements SecurityStore,
HierarchicalRepositoryC
if (securityManager instanceof ActiveMQSecurityManager5 manager5) {
try {
subject = manager5.authenticate(user, password, connection,
securityDomain);
- putAuthenticationCacheEntry(authnCacheKey, subject);
- validatedUser = getUserFromSubject(subject);
+ if (validateExpectedUserPrincipal(subject)) {
+ validatedUser = getUserFromSubject(subject);
+ putAuthenticationCacheEntry(authnCacheKey, subject);
+ }
} catch (NoCacheLoginException e) {
handleNoCacheLoginException(e);
}
@@ -258,6 +263,27 @@ public class SecurityStoreImpl implements SecurityStore,
HierarchicalRepositoryC
return null;
}
+ /*
+ * Verify that the Subject (if not null) contains at least one instance of
the expected java.security.Principal
+ * implementation. This check is done before any caching because a failure
here is considered an infrastructure
+ * failure and not something which should be cached as opposed to a
"normal" authentication failure (e.g. wrong
+ * password) which should be cached.
+ */
+ private boolean validateExpectedUserPrincipal(Subject subject) throws
ClassNotFoundException {
+ if (subject != null) {
+ Class expectedPrincipal = UserPrincipal.class;
+ if (securityManager instanceof ActiveMQJAASSecurityManager
jaasManager) {
+ expectedPrincipal =
Class.forName(jaasManager.getUserPrincipalClass());
+ }
+
+ if (subject.getPrincipals(expectedPrincipal).size() == 0) {
+
ActiveMQServerLogger.LOGGER.illegalPrincipal(subject.getPrincipals().stream().map(p
-> p.getClass().getName() + " (" + p.getName() +
")").collect(Collectors.joining(", ")));
+ return false;
+ }
+ }
+ return true;
+ }
+
@Override
public void check(final SimpleString address,
final CheckType checkType,
diff --git
a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java
b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java
index e43aac1a9c..b40fcbb95b 100644
---
a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java
+++
b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java
@@ -1486,4 +1486,7 @@ public interface ActiveMQServerLogger {
@LogMessage(id = 224140, value = "Clearing bindings on cluster-connection
{} failed to remove binding {}: {}", level = LogMessage.Level.WARN)
void clusterConnectionFailedToRemoveBindingOnClear(String
clusterConnection, String binding, String exceptionMessage);
+
+ @LogMessage(id = 224141, value = "Illegal implementation(s) of
java.security.Principal returned from LoginModule: {}", level =
LogMessage.Level.WARN)
+ void illegalPrincipal(String principals);
}
diff --git
a/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImplTest.java
b/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImplTest.java
index 681ba885c1..ca9517d1a1 100644
---
a/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImplTest.java
+++
b/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImplTest.java
@@ -16,11 +16,8 @@
*/
package org.apache.activemq.artemis.core.security.impl;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNull;
-import static org.junit.jupiter.api.Assertions.fail;
-
import javax.security.auth.Subject;
+import java.security.Principal;
import java.security.PrivilegedExceptionAction;
import java.util.Set;
@@ -40,6 +37,10 @@ import org.junit.jupiter.api.Test;
import org.mockito.ArgumentMatchers;
import org.mockito.Mockito;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.fail;
+
public class SecurityStoreImplTest {
final ActiveMQSecurityManager5 securityManager = new
ActiveMQSecurityManager5() {
@@ -69,6 +70,38 @@ public class SecurityStoreImplTest {
}
};
+ final ActiveMQSecurityManager5 wrongPrincipalSecurityManager = new
ActiveMQSecurityManager5() {
+ @Override
+ public Subject authenticate(String user,
+ String password,
+ RemotingConnection remotingConnection,
+ String securityDomain) {
+ Subject subject = new Subject();
+ subject.getPrincipals().add(new Principal() {
+ @Override
+ public String getName() {
+ return "wrong";
+ }
+ });
+ return subject;
+ }
+
+ @Override
+ public boolean authorize(Subject subject, Set<Role> roles, CheckType
checkType, String address) {
+ return true;
+ }
+
+ @Override
+ public boolean validateUser(String user, String password) {
+ return false;
+ }
+
+ @Override
+ public boolean validateUserAndRole(String user, String password,
Set<Role> roles, CheckType checkType) {
+ return false;
+ }
+ };
+
@Test
public void zeroCacheSizeTest() throws Exception {
final String user = RandomUtil.randomUUIDString();
@@ -165,4 +198,18 @@ public class SecurityStoreImplTest {
assertEquals(true, checkResult);
}
+
+ @Test
+ public void testWrongPrincipal() throws Exception {
+ SecurityStoreImpl securityStore = new SecurityStoreImpl(new
HierarchicalObjectRepository<>(), wrongPrincipalSecurityManager, 999, true, "",
null, null, 10, 0);
+ try {
+ securityStore.authenticate(null, null,
Mockito.mock(RemotingConnection.class), null);
+ fail();
+ } catch (ActiveMQSecurityException ignored) {
+ // ignored
+ }
+ assertEquals(0, securityStore.getAuthenticationSuccessCount());
+ assertEquals(1, securityStore.getAuthenticationFailureCount());
+ assertEquals(0, securityStore.getAuthenticationCacheSize());
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
