ARTEMIS-6037 refactor handling of cluster credentials

Author: jbertram

artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java

@@ -29,6 +29,7 @@
 
 import com.github.benmanes.caffeine.cache.Cache;
 import com.github.benmanes.caffeine.cache.Caffeine;
+import org.apache.activemq.artemis.api.core.ActiveMQSecurityException;
 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;
@@ -63,6 +64,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static org.apache.activemq.artemis.api.config.ActiveMQDefaultConfiguration.getDefaultClusterPassword;
+import static org.apache.activemq.artemis.api.config.ActiveMQDefaultConfiguration.getDefaultClusterUser;
 import static org.apache.activemq.artemis.utils.CertificateUtil.CERT_SUBJECT_DN_UNAVAILABLE;
 
 /**
@@ -113,7 +116,7 @@ public SecurityStoreImpl(final HierarchicalRepository<Set<Role>> securityReposit
                             final String managementClusterPassword,
                             final NotificationService notificationService,
                             final long authenticationCacheSize,
-                            final long authorizationCacheSize) throws NoSuchAlgorithmException {
+                            final long authorizationCacheSize) {
       this.securityRepository = securityRepository;
       this.securityManager = securityManager;
       this.securityEnabled = securityEnabled;
@@ -178,24 +181,10 @@ public String authenticate(final String user,
                               RemotingConnection connection,
                               String securityDomain) throws Exception {
       if (securityEnabled) {
-
-         if (managementClusterUser.equals(user)) {
-            logger.trace("Authenticating cluster admin user");
-
-            /*
-             * The special user cluster user is used for creating sessions that replicate management
-             * operation between nodes
-             */
-            if (!managementClusterPassword.equals(password)) {
-               AUTHENTICATION_FAILURE_COUNT_UPDATER.incrementAndGet(this);
-               throw ActiveMQMessageBundle.BUNDLE.unableToValidateClusterUser(user);
-            } else {
-               AUTHENTICATION_SUCCESS_COUNT_UPDATER.incrementAndGet(this);
-               return managementClusterUser;
-            }
+         String validatedUser = handleClusterAuthentication(user, password, connection);
+         if (validatedUser != null) {
+            return validatedUser;
          }
-
-         String validatedUser = null;
          boolean userIsValid = false;
          boolean check = true;
 
@@ -305,10 +294,12 @@ public boolean hasPermission(final SimpleString address,
          return true;
       }
 
-      // bypass permission checks for management cluster user
       String user = session.getUsername();
-      if (managementClusterUser.equals(user) && session.getPassword().equals(managementClusterPassword)) {
+      ClusterCredentialsCheckResult checkResult = checkClusterCredentials(user, session.getPassword());
+      if (checkResult == ClusterCredentialsCheckResult.VALID) {
          return true;
+      } else if (checkResult == ClusterCredentialsCheckResult.INVALID) {
+         return false;
       }
 
       // Special case: detect authentication failure for ActiveMQSecurityManager5
@@ -384,6 +375,39 @@ public boolean hasPermission(final SimpleString address,
       }
    }
 
+   private String handleClusterAuthentication(String user, String password, RemotingConnection connection) throws ActiveMQSecurityException {
+      ClusterCredentialsCheckResult checkResult = checkClusterCredentials(user, password);
+
+      if (checkResult == ClusterCredentialsCheckResult.VALID) {
+         AUTHENTICATION_SUCCESS_COUNT_UPDATER.incrementAndGet(this);
+         return user;
+      } else if (checkResult == ClusterCredentialsCheckResult.INVALID) {
+         AUTHENTICATION_FAILURE_COUNT_UPDATER.incrementAndGet(this);
+         throw ActiveMQMessageBundle.BUNDLE.unableToValidateUser(connection == null ? "null" : connection.getRemoteAddress(), user, null);
+      } else {
+         return null;
+      }
+   }
+
+   private ClusterCredentialsCheckResult checkClusterCredentials(String user, String password) {
+      if ((getDefaultClusterUser().equals(user) && getDefaultClusterPassword().equals(password))) {
+         // reject default cluster credentials
+         return ClusterCredentialsCheckResult.INVALID;
+      } else if (managementClusterUser.equals(user) && !managementClusterPassword.equals(password)) {
+         // reject if username is right, but password is wrong
+         return ClusterCredentialsCheckResult.INVALID;
+      } else if (managementClusterUser.equals(user) && managementClusterPassword.equals(password)) {
+         // accept if both user & password are right
+         return ClusterCredentialsCheckResult.VALID;
+      } else {
+         return ClusterCredentialsCheckResult.IGNORE;
+      }
+   }
+
+   enum ClusterCredentialsCheckResult {
+      VALID, INVALID, IGNORE
+   }
+
    @Override
    public void check(final SimpleString address,
                      final SimpleString queue,

artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java

@@ -341,8 +341,8 @@ void slowConsumerDetected(String sessionID,
    @LogMessage(id = 222006, value = "Binding already exists with name {}, divert will not be deployed", level = LogMessage.Level.WARN)
    void divertBindingAlreadyExists(SimpleString bindingName);
 
-   @LogMessage(id = 222007, value = "Security risk! Apache Artemis is running with the default cluster admin user and default password. Please see the cluster chapter in the Artemis User Guide for instructions on how to change this.", level = LogMessage.Level.WARN)
-   void clusterSecurityRisk();
+   @LogMessage(id = 222007, value = "Apache Artemis is running with the default cluster user and password. Any connection using these credentials will be rejected. Please see the cluster chapter in the Artemis User Guide for instructions on how to change this.", level = LogMessage.Level.WARN)
+   void defaultClusterCredentialsInUse();
 
    @LogMessage(id = 222008, value = "unable to restart server, please kill and restart manually", level = LogMessage.Level.WARN)
    void serverRestartWarning(Exception e);

artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/ClusterConnection.java

@@ -74,13 +74,6 @@ void nodeAnnounced(long eventUID,
 
    boolean isNodeActive(String id);
 
-   /**
-    * Verifies whether user and password match the ones configured for this ClusterConnection.
-    *
-    * @return {@code true} if username and password match, {@code false} otherwise
-    */
-   boolean verify(String clusterUser, String clusterPassword);
-
    void removeRecord(String targetNodeID);
 
    void disconnectRecord(String targetNodeID);

artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/ClusterController.java

@@ -407,11 +407,19 @@ public void handlePacket(Packet packet) {
 
                ClusterConnectMessage msg = (ClusterConnectMessage) packet;
 
-               if (server.getConfiguration().isSecurityEnabled() && !clusterConnection.verify(msg.getClusterUser(), msg.getClusterPassword())) {
-                  clusterChannel.send(new ClusterConnectReplyMessage(false));
-               } else {
+               boolean userIsValid = false;
+               try {
+                  server.validateUser(msg.getClusterUser(), msg.getClusterPassword(), null, null);
+                  userIsValid = true;
+               } catch (Exception e) {
+                  // cluster user isn't valid
+               }
+
+               if (userIsValid) {
                   authorized = true;
                   clusterChannel.send(new ClusterConnectReplyMessage(true));
+               } else {
+                  clusterChannel.send(new ClusterConnectReplyMessage(false));
                }
             }
          } else {

artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/ClusterManager.java

@@ -506,7 +506,7 @@ public boolean intercept(Packet packet, RemotingConnection connection) throws Ac
          if (packet.getType() == PacketImpl.EXCEPTION) {
             ActiveMQExceptionMessage msg = (ActiveMQExceptionMessage) packet;
             final ActiveMQException exception = msg.getException();
-            if (exception.getType() == ActiveMQExceptionType.CLUSTER_SECURITY_EXCEPTION) {
+            if (exception.getType() == ActiveMQExceptionType.CLUSTER_SECURITY_EXCEPTION || exception.getType() == ActiveMQExceptionType.SECURITY_EXCEPTION) {
                ActiveMQServerLogger.LOGGER.clusterManagerAuthenticationError(exception.getMessage());
                executor.execute(() -> {
                   try {

artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionImpl.java

@@ -1769,11 +1769,6 @@ public ServerLocatorInternal createServerLocator() {
       }
    }
 
-   @Override
-   public boolean verify(String clusterUser0, String clusterPassword0) {
-      return clusterUser.equals(clusterUser0) && clusterPassword.equals(clusterPassword0);
-   }
-
    @Override
    public void removeRecord(String targetNodeID) {
       logger.debug("Removing record for: {}", targetNodeID);

artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java

@@ -3380,7 +3380,7 @@ synchronized boolean initialisePart1(boolean scalingDown) throws Exception {
       storageManager = createStorageManager();
 
       if (!configuration.getClusterConfigurations().isEmpty() && ActiveMQDefaultConfiguration.getDefaultClusterUser().equals(configuration.getClusterUser()) && ActiveMQDefaultConfiguration.getDefaultClusterPassword().equals(configuration.getClusterPassword())) {
-         ActiveMQServerLogger.LOGGER.clusterSecurityRisk();
+         ActiveMQServerLogger.LOGGER.defaultClusterCredentialsInUse();
       }
 
       securityStore = new SecurityStoreImpl(securityRepository, securityManager, configuration.getSecurityInvalidationInterval(), configuration.isSecurityEnabled(), configuration.getClusterUser(), configuration.getClusterPassword(), managementService, configuration.getAuthenticationCacheSize(), configuration.getAuthorizationCacheSize());