ARTEMIS-4101 caching failed authn result on LDAP cxn failures

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<Throwable> 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));
+   }
+}

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<Throwable> 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;
    }

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) {

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 {

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<String> 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<String> 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<String> 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,

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);
+   }
+}

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`

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<Role> 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<Role> 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");