From 499e3c119f008a662ddbec2eaf1e00c8c6f94185 Mon Sep 17 00:00:00 2001 From: Justin Bertram Date: Tue, 29 Nov 2022 00:24:56 -0600 Subject: [PATCH] ARTEMIS-4101 caching failed authn result on LDAP cxn failures --- .../activemq/artemis/utils/ExceptionUtil.java | 33 +++++++++ .../remoting/impl/netty/NettyAcceptor.java | 18 +---- .../core/security/impl/SecurityStoreImpl.java | 11 ++- .../security/ActiveMQJAASSecurityManager.java | 9 ++- .../core/security/jaas/LDAPLoginModule.java | 33 +++++++-- .../security/jaas/NoCacheLoginException.java | 27 +++++++ docs/user-manual/en/security.md | 9 +++ .../integration/security/SecurityTest.java | 74 +++++++++++++++++++ .../src/test/resources/login.config | 40 ++++++++++ 9 files changed, 228 insertions(+), 26 deletions(-) create mode 100644 artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ExceptionUtil.java create mode 100644 artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/NoCacheLoginException.java diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ExceptionUtil.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ExceptionUtil.java new file mode 100644 index 00000000000..9caff3494eb --- /dev/null +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ExceptionUtil.java @@ -0,0 +1,33 @@ +/** + * 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.utils; + +import java.util.ArrayList; +import java.util.List; + +public class ExceptionUtil { + public static Throwable getRootCause(final Throwable throwable) { + List list = new ArrayList<>(); + Throwable current = throwable; + while (current != null && list.contains(current) == false) { + list.add(current); + current = current.getCause(); + } + return (list.size() < 2 ? throwable : list.get(list.size() - 1)); + } +} diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java index e063cde8bda..6d69e1cbaa3 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java @@ -21,14 +21,13 @@ import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLHandshakeException; import javax.net.ssl.SSLParameters; +import java.lang.invoke.MethodHandles; import java.net.InetSocketAddress; import java.net.SocketAddress; import java.security.AccessController; import java.security.PrivilegedAction; -import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -95,10 +94,10 @@ import org.apache.activemq.artemis.spi.core.remoting.ssl.SSLContextFactoryProvider; import org.apache.activemq.artemis.utils.ActiveMQThreadFactory; import org.apache.activemq.artemis.utils.ConfigurationHelper; +import org.apache.activemq.artemis.utils.ExceptionUtil; import org.apache.activemq.artemis.utils.collections.TypedProperties; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.lang.invoke.MethodHandles; /** * A Netty TCP Acceptor that is embedding Netty. @@ -480,7 +479,7 @@ public void initChannel(Channel channel) throws Exception { pipeline.addLast("ssl", getSslHandler(channel.alloc(), peerInfo.getA(), peerInfo.getB())); pipeline.addLast("sslHandshakeExceptionHandler", new SslHandshakeExceptionHandler()); } catch (Exception e) { - Throwable rootCause = getRootCause(e); + Throwable rootCause = ExceptionUtil.getRootCause(e); ActiveMQServerLogger.LOGGER.gettingSslHandlerFailed(channel.remoteAddress().toString(), rootCause.getClass().getName() + ": " + rootCause.getMessage()); logger.debug("Getting SSL handler failed", e); @@ -1037,7 +1036,7 @@ public void handlerRemoved(ChannelHandlerContext ctx) throws Exception { @Override public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { if (cause.getMessage() != null && cause.getMessage().startsWith(SSLHandshakeException.class.getName())) { - Throwable rootCause = getRootCause(cause); + Throwable rootCause = ExceptionUtil.getRootCause(cause); String errorMessage = rootCause.getClass().getName() + ": " + rootCause.getMessage(); ActiveMQServerLogger.LOGGER.sslHandshakeFailed(ctx.channel().remoteAddress().toString(), errorMessage); @@ -1047,15 +1046,6 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws E } } - private Throwable getRootCause(Throwable throwable) { - List list = new ArrayList<>(); - while (throwable != null && list.contains(throwable) == false) { - list.add(throwable); - throwable = throwable.getCause(); - } - return (list.size() < 2 ? throwable : list.get(list.size() - 1)); - } - public boolean isAutoStart() { return autoStart; } 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 197594b03a7..a834779c1f8 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 @@ -44,6 +44,7 @@ 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.CompositeAddress; import org.apache.activemq.artemis.utils.collections.ConcurrentHashSet; @@ -170,9 +171,13 @@ public String authenticate(final String user, } if (check) { if (securityManager instanceof ActiveMQSecurityManager5) { - subject = ((ActiveMQSecurityManager5) securityManager).authenticate(user, password, connection, securityDomain); - authenticationCache.put(createAuthenticationCacheKey(user, password, connection), new Pair<>(subject != null, subject)); - validatedUser = getUserFromSubject(subject); + try { + subject = ((ActiveMQSecurityManager5) securityManager).authenticate(user, password, connection, securityDomain); + authenticationCache.put(createAuthenticationCacheKey(user, password, connection), new Pair<>(subject != null, subject)); + validatedUser = getUserFromSubject(subject); + } catch (NoCacheLoginException e) { + logger.debug("Skipping authentication cache due to exception", e); + } } else if (securityManager instanceof ActiveMQSecurityManager4) { validatedUser = ((ActiveMQSecurityManager4) securityManager).validateUser(user, password, connection, securityDomain); } else if (securityManager instanceof ActiveMQSecurityManager3) { 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 3c84e9eb7c1..bcfcc923bc3 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 @@ -26,7 +26,9 @@ 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.jaas.JaasCallbackHandler; +import org.apache.activemq.artemis.spi.core.security.jaas.NoCacheLoginException; import org.apache.activemq.artemis.spi.core.security.jaas.RolePrincipal; +import org.apache.activemq.artemis.utils.ExceptionUtil; import org.apache.activemq.artemis.utils.SecurityManagerUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -139,7 +141,12 @@ private Subject getAuthenticatedSubject(final String user, try { lc.login(); } catch (LoginException e) { - throw e; + Throwable rootCause = ExceptionUtil.getRootCause(e); + if (rootCause instanceof NoCacheLoginException) { + throw (NoCacheLoginException) rootCause; + } else { + throw e; + } } return lc.getSubject(); } finally { diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/LDAPLoginModule.java b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/LDAPLoginModule.java index 8eae7a81886..f78e255743d 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/LDAPLoginModule.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/LDAPLoginModule.java @@ -48,6 +48,8 @@ import java.security.PrivilegedExceptionAction; import java.text.MessageFormat; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.Hashtable; import java.util.LinkedList; @@ -57,6 +59,7 @@ import java.util.Set; import org.apache.activemq.artemis.core.server.ActiveMQServerLogger; +import org.apache.activemq.artemis.utils.ExceptionUtil; import org.apache.activemq.artemis.utils.PasswordMaskingUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -92,7 +95,8 @@ enum ConfigKey { PASSWORD_CODEC("passwordCodec"), CONNECTION_POOL("connectionPool"), CONNECTION_TIMEOUT("connectionTimeout"), - READ_TIMEOUT("readTimeout"); + READ_TIMEOUT("readTimeout"), + NO_CACHE_EXCEPTIONS("noCacheExceptions"); private final String name; @@ -126,6 +130,7 @@ static boolean contains(String key) { private Subject brokerGssapiIdentity = null; private boolean isRoleAttributeSet = false; private String roleAttributeName = null; + private List noCacheExceptions; private String codecClass = null; @@ -151,6 +156,12 @@ public void initialize(Subject subject, isRoleAttributeSet = isLoginPropertySet(ConfigKey.ROLE_NAME); roleAttributeName = getLDAPPropertyValue(ConfigKey.ROLE_NAME); codecClass = getLDAPPropertyValue(ConfigKey.PASSWORD_CODEC); + if (isLoginPropertySet(ConfigKey.NO_CACHE_EXCEPTIONS)) { + noCacheExceptions = Arrays.asList(getLDAPPropertyValue(ConfigKey.NO_CACHE_EXCEPTIONS).split(",")); + noCacheExceptions.replaceAll(String::trim); + } else { + noCacheExceptions = Collections.emptyList(); + } } private String getPlainPassword(String password) { @@ -303,10 +314,8 @@ private String resolveDN(String username, List roles) throws FailedLogin logger.debug("Create the LDAP initial context."); try { openContext(); - } catch (Exception ne) { - FailedLoginException ex = new FailedLoginException("Error opening LDAP connection"); - ex.initCause(ne); - throw ex; + } catch (Exception e) { + return handleException(e, "Error opening LDAP connection"); } if (!isLoginPropertySet(ConfigKey.USER_SEARCH_MATCHING)) { @@ -431,14 +440,22 @@ private String resolveDN(String username, List roles) throws FailedLogin } } catch (NamingException e) { closeContext(); - FailedLoginException ex = new FailedLoginException("Error contacting LDAP"); - ex.initCause(e); - throw ex; + handleException(e, "Error contacting LDAP"); } return dn; } + private String handleException(Exception e, String s) throws FailedLoginException { + FailedLoginException ex = new FailedLoginException(s); + if (noCacheExceptions.contains(ExceptionUtil.getRootCause(e).getClass().getName())) { + ex.initCause(new NoCacheLoginException()); + } else { + ex.initCause(e); + } + throw ex; + } + protected void addRoles(DirContext context, String dn, String username, diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/NoCacheLoginException.java b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/NoCacheLoginException.java new file mode 100644 index 00000000000..61ae8318e62 --- /dev/null +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/NoCacheLoginException.java @@ -0,0 +1,27 @@ +/** + * 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.jaas; + +public class NoCacheLoginException extends RuntimeException { + public NoCacheLoginException() { + super(); + } + public NoCacheLoginException(Exception e) { + super(e); + } +} diff --git a/docs/user-manual/en/security.md b/docs/user-manual/en/security.md index 2cdb4692e54..1e7d405c810 100644 --- a/docs/user-manual/en/security.md +++ b/docs/user-manual/en/security.md @@ -821,6 +821,15 @@ system. It is implemented by previous role search. This option must always be set to enable role expansion because it has no default value. Example value: `(member={0})`. +- `noCacheExceptions` - comma separated list of class names of exceptions which + may thrown during communication with the LDAP server; default is empty. + Typically any failure to authenticate will be stored in the authentication cache + so that the underlying security data store (e.g. LDAP) is spared any unnecessary + traffic. However, in cases where the failure is, for example, due to a temporary + network outage and the `security-invalidation-interval` is relatively high this + can be problematic. Users can enumerate any relevant exceptions which the cache + should ignore (e.g. `java.net.ConnectException`) to avoid any such problems. + - `debug` - boolean flag; if `true`, enable debugging; this is used only for testing or debugging; normally, it should be set to `false`, or omitted; default is `false` 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 ba339491842..7923c6e37e2 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 @@ -24,6 +24,7 @@ import javax.jms.QueueBrowser; import javax.jms.Session; import java.security.cert.X509Certificate; +import javax.security.auth.Subject; import javax.transaction.xa.XAResource; import javax.transaction.xa.Xid; import java.lang.management.ManagementFactory; @@ -68,6 +69,8 @@ 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.tests.util.ActiveMQTestBase; import org.apache.activemq.artemis.tests.util.CreateMessage; import org.apache.activemq.artemis.utils.CompositeAddress; @@ -130,6 +133,77 @@ public void testJAASSecurityManagerAuthentication() throws Exception { } } + @Test + public void testNoCacheException() throws Exception { + ActiveMQSecurityManager5 securityManager = new ActiveMQSecurityManager5() { + boolean flipper = false; + + @Override + public Subject authenticate(String user, + String password, + RemotingConnection remotingConnection, + String securityDomain) { + flipper = !flipper; + if (flipper) { + return new Subject(); + } else { + throw new NoCacheLoginException(); + } + } + + @Override + public boolean authorize(Subject subject, Set roles, CheckType checkType, String address) { + return false; + } + + @Override + public boolean validateUser(String user, String password) { + return false; + } + + @Override + public boolean validateUserAndRole(String user, String password, Set roles, CheckType checkType) { + return false; + } + }; + ActiveMQServer server = addServer(ActiveMQServers.newActiveMQServer(createDefaultInVMConfig().setSecurityEnabled(true), ManagementFactory.getPlatformMBeanServer(), securityManager, false)); + server.start(); + ClientSessionFactory cf = createSessionFactory(locator); + + cf.createSession("first", "secret", false, true, true, false, 0).close(); + assertEquals(1, ((SecurityStoreImpl)server.getSecurityStore()).getAuthenticationCacheSize()); + try { + cf.createSession("first", "secret", false, true, true, false, 0); + } catch (ActiveMQException e) { + // expected + } + assertEquals(1, ((SecurityStoreImpl)server.getSecurityStore()).getAuthenticationCacheSize()); + } + + @Test + public void testNoCacheNamingException() throws Exception { + internalTestNoCacheException("BrokenLDAPLoginNamingException"); + } + + @Test + public void testNoCacheConnectException() throws Exception { + internalTestNoCacheException("BrokenLDAPLoginConnectException"); + } + + private void internalTestNoCacheException(String ldapConfigName) throws Exception { + ActiveMQJAASSecurityManager securityManager = new ActiveMQJAASSecurityManager(ldapConfigName); + ActiveMQServer server = addServer(ActiveMQServers.newActiveMQServer(createDefaultInVMConfig().setSecurityEnabled(true), ManagementFactory.getPlatformMBeanServer(), securityManager, false)); + server.start(); + ClientSessionFactory cf = createSessionFactory(locator); + + try { + cf.createSession("first", "secret", false, true, true, false, 0); + } catch (ActiveMQException e) { + // expected + } + assertEquals(0, ((SecurityStoreImpl)server.getSecurityStore()).getAuthenticationCacheSize()); + } + @Test public void testJAASSecurityManagerAuthenticationWithPasswordCodec() throws Exception { ActiveMQJAASSecurityManager securityManager = new ActiveMQJAASSecurityManager("PropertiesLoginWithPasswordCodec"); diff --git a/tests/integration-tests/src/test/resources/login.config b/tests/integration-tests/src/test/resources/login.config index a9eaff49a80..63f5978f8eb 100644 --- a/tests/integration-tests/src/test/resources/login.config +++ b/tests/integration-tests/src/test/resources/login.config @@ -86,6 +86,46 @@ UnAuthenticatedLDAPLogin { ; }; +BrokenLDAPLoginNamingException { + org.apache.activemq.artemis.spi.core.security.jaas.LDAPLoginModule required + debug=true + initialContextFactory=com.sun.jndi.ldap.LdapCtxFactory + connectionURL="ldap://localhost:1024" + connectionUsername="uid=admin,ou=system" + connectionPassword="" + connectionProtocol=s + authentication=simple + userBase="ou=system" + userSearchMatching="(uid={0})" + userSearchSubtree=false + roleBase="ou=system" + roleName=dummyRoleName + roleSearchMatching="(uid={1})" + roleSearchSubtree=false + noCacheExceptions="javax.naming.NamingException" + ; +}; + +BrokenLDAPLoginConnectException { + org.apache.activemq.artemis.spi.core.security.jaas.LDAPLoginModule required + debug=true + initialContextFactory=com.sun.jndi.ldap.LdapCtxFactory + connectionURL="ldap://localhost:1024" + connectionUsername="uid=admin,ou=system" + connectionPassword="123" + connectionProtocol=s + authentication=simple + userBase="ou=system" + userSearchMatching="(uid={0})" + userSearchSubtree=false + roleBase="ou=system" + roleName=dummyRoleName + roleSearchMatching="(uid={1})" + roleSearchSubtree=false + noCacheExceptions="javax.naming.NamingException, java.net.ConnectException" + ; +}; + ExpandedLDAPLogin { org.apache.activemq.artemis.spi.core.security.jaas.LDAPLoginModule required debug=true