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,38 @@ 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)) ||
+         (managementClusterUser.equals(user) && !managementClusterPassword.equals(password))) {
+         // reject default cluster credentials
+         // reject also 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/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());