From 53f8bc3daff9da95aba406c72e706177c28a9012 Mon Sep 17 00:00:00 2001
From: Justin Bertram <jbertram@apache.org>
Date: Fri, 3 Aug 2018 16:39:09 -0500
Subject: [PATCH] ARTEMIS-2010 actively detect unauthenticated LDAP Bind
 requests

---
 .../core/security/jaas/LDAPLoginModule.java   | 15 ++++--
 .../security/jaas/LDAPLoginModuleTest.java    | 51 +++++++++++++++++++
 2 files changed, 63 insertions(+), 3 deletions(-)

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 7d58a0b97b..1470040584 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
@@ -167,7 +167,7 @@ public class LDAPLoginModule implements LoginModule {
          throw (LoginException) new LoginException().initCause(e);
       }
 
-      String password;
+      String password = null;
 
       username = ((NameCallback) callbacks[0]).getName();
       if (username == null)
@@ -175,8 +175,17 @@ public class LDAPLoginModule implements LoginModule {
 
       if (((PasswordCallback) callbacks[1]).getPassword() != null)
          password = new String(((PasswordCallback) callbacks[1]).getPassword());
-      else
-         password = "";
+
+      /**
+       * https://tools.ietf.org/html/rfc4513#section-6.3.1
+       *
+       * Clients that use the results from a simple Bind operation to make
+       * authorization decisions should actively detect unauthenticated Bind
+       * requests (by verifying that the supplied password is not empty) and
+       * react appropriately.
+       */
+      if (password == null || (password != null && password.length() == 0))
+         throw new FailedLoginException("Password cannot be null or empty");
 
       // authenticate will throw LoginException
       // in case of failed authentication
diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/jaas/LDAPLoginModuleTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/jaas/LDAPLoginModuleTest.java
index 47bedd9b07..b52a717ba9 100644
--- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/jaas/LDAPLoginModuleTest.java
+++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/jaas/LDAPLoginModuleTest.java
@@ -27,6 +27,7 @@ import javax.security.auth.callback.CallbackHandler;
 import javax.security.auth.callback.NameCallback;
 import javax.security.auth.callback.PasswordCallback;
 import javax.security.auth.callback.UnsupportedCallbackException;
+import javax.security.auth.login.FailedLoginException;
 import javax.security.auth.login.LoginContext;
 import javax.security.auth.login.LoginException;
 import javax.security.auth.spi.LoginModule;
@@ -268,6 +269,56 @@ public class LDAPLoginModuleTest extends AbstractLdapTestUnit {
       }
    }
 
+   @Test
+   public void testEmptyPassword() throws Exception {
+      LoginContext context = new LoginContext("LDAPLogin", new CallbackHandler() {
+         @Override
+         public void handle(Callback[] callbacks) throws IOException, UnsupportedCallbackException {
+            for (int i = 0; i < callbacks.length; i++) {
+               if (callbacks[i] instanceof NameCallback) {
+                  ((NameCallback) callbacks[i]).setName("first");
+               } else if (callbacks[i] instanceof PasswordCallback) {
+                  ((PasswordCallback) callbacks[i]).setPassword("".toCharArray());
+               } else {
+                  throw new UnsupportedCallbackException(callbacks[i]);
+               }
+            }
+         }
+      });
+      try {
+         context.login();
+         fail("Should have thrown a FailedLoginException");
+      } catch (FailedLoginException fle) {
+         assertEquals("Password cannot be null or empty", fle.getMessage());
+      }
+      context.logout();
+   }
+
+   @Test
+   public void testNullPassword() throws Exception {
+      LoginContext context = new LoginContext("LDAPLogin", new CallbackHandler() {
+         @Override
+         public void handle(Callback[] callbacks) throws IOException, UnsupportedCallbackException {
+            for (int i = 0; i < callbacks.length; i++) {
+               if (callbacks[i] instanceof NameCallback) {
+                  ((NameCallback) callbacks[i]).setName("first");
+               } else if (callbacks[i] instanceof PasswordCallback) {
+                  ((PasswordCallback) callbacks[i]).setPassword(null);
+               } else {
+                  throw new UnsupportedCallbackException(callbacks[i]);
+               }
+            }
+         }
+      });
+      try {
+         context.login();
+         fail("Should have thrown a FailedLoginException");
+      } catch (FailedLoginException fle) {
+         assertEquals("Password cannot be null or empty", fle.getMessage());
+      }
+      context.logout();
+   }
+
    private boolean presentInArray(LDAPLoginProperty[] ldapProps, String propertyName) {
       for (LDAPLoginProperty conf : ldapProps) {
          if (conf.getPropertyName().equals(propertyName) && (conf.getPropertyValue() != null && !"".equals(conf.getPropertyValue())))