AMBARI-9637. Kerberos: Escape special characters in Distinguished Names used for queries in Active Directory (rlevas)

ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandler.java

@@ -35,13 +35,19 @@ import javax.naming.directory.*;
 import javax.naming.ldap.Control;
 import javax.naming.ldap.InitialLdapContext;
 import javax.naming.ldap.LdapContext;
+import javax.naming.ldap.LdapName;
+import javax.naming.ldap.Rdn;
 import java.io.StringWriter;
 import java.io.UnsupportedEncodingException;
 import java.lang.reflect.Type;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.Set;
 
 /**
  * Implementation of <code>KerberosOperationHandler</code> to created principal in Active Directory
@@ -52,6 +58,34 @@ public class ADKerberosOperationHandler extends KerberosOperationHandler {
 
   private static final String LDAP_CONTEXT_FACTORY_CLASS = "com.sun.jndi.ldap.LdapCtxFactory";
 
+  /**
+   * A Set of special characters that need to be escaped if they exist within a value in a
+   * Distinguished Name.
+   *
+   * See http://social.technet.microsoft.com/wiki/contents/articles/5312.active-directory-characters-to-escape.aspx
+   */
+  private static final Set<Character> SPECIAL_DN_CHARACTERS = Collections.unmodifiableSet(
+      new HashSet<Character>() {
+        {
+          add('/');
+          add(',');
+          add('\\');
+          add('#');
+          add('+');
+          add('<');
+          add('>');
+          add(';');
+          add('"');
+          add('=');
+          add(' ');
+        }
+      });
+
+  /**
+   * The character to use to escape a special character within a value in a Distinguished Name
+   */
+  private static final Character DN_ESCAPE_CHARACTER = '\\';
+
   /**
    * A String containing the URL for the LDAP interface for the relevant Active Directory
    */
@@ -329,7 +363,8 @@ public class ADKerberosOperationHandler extends KerberosOperationHandler {
       String dn = findPrincipalDN(deconstructPrincipal.getNormalizedPrincipal());
 
       if (dn != null) {
-        ldapContext.modifyAttributes(dn,
+        ldapContext.modifyAttributes(
+            escapeDNCharacters(dn),
             new ModificationItem[]{
                 new ModificationItem(DirContext.REPLACE_ATTRIBUTE, new BasicAttribute("unicodePwd", String.format("\"%s\"", password).getBytes("UTF-16LE")))
             }
@@ -559,4 +594,34 @@ public class ADKerberosOperationHandler extends KerberosOperationHandler {
 
     return dn;
   }
+
+  /**
+   * Iterates through the characters of the given distinguished name to escape special characters
+   *
+   * @param dn the distinguished name to process
+   * @return the distinguished name with escaped characters
+   * @see #escapeCharacters(String, java.util.Set, Character)
+   */
+  protected String escapeDNCharacters(String dn) throws InvalidNameException {
+    if ((dn == null) || dn.isEmpty()) {
+      return dn;
+    } else {
+      LdapName name = new LdapName(dn);
+      List<Rdn> rdns = name.getRdns();
+
+      if ((rdns == null) || rdns.isEmpty()) {
+        throw new InvalidNameException(String.format("One or more RDNs are expected for a DN of %s", dn));
+      }
+
+      StringBuilder builder = new StringBuilder();
+      for (Rdn rdn : rdns) {
+        builder.insert(0,
+            String.format(",%s=%s",
+                rdn.getType(),
+                escapeCharacters((String) rdn.getValue(), SPECIAL_DN_CHARACTERS, DN_ESCAPE_CHARACTER)));
+      }
+
+      return builder.substring(1);
+    }
+  }
 }

ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandler.java

@@ -613,4 +613,28 @@ public abstract class KerberosOperationHandler {
     return encryptionTypes;
   }
 
+  /**
+   * Iterates through the characters in a string to escape special characters
+   *
+   * @param string             the String to process
+   * @param charactersToEscape a Set of characters declaring the special characters to escape
+   * @param escapeCharacter    a character to use for escaping
+   * @return the string with escaped characters
+   */
+  protected String escapeCharacters(String string, Set<Character> charactersToEscape, Character escapeCharacter) {
+    if ((string == null) || string.isEmpty() || (charactersToEscape == null) || charactersToEscape.isEmpty()) {
+      return string;
+    } else {
+      StringBuilder builder = new StringBuilder();
+
+      for (char character : string.toCharArray()) {
+        if (charactersToEscape.contains(character)) {
+          builder.append(escapeCharacter);
+        }
+        builder.append(character);
+      }
+
+      return builder.toString();
+    }
+  }
 }

ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandlerTest.java

@@ -28,6 +28,7 @@ import org.junit.Test;
 
 import javax.naming.AuthenticationException;
 import javax.naming.CommunicationException;
+import javax.naming.InvalidNameException;
 import javax.naming.Name;
 import javax.naming.NamingEnumeration;
 import javax.naming.directory.Attributes;
@@ -441,6 +442,21 @@ public class ADKerberosOperationHandlerTest extends KerberosOperationHandlerTest
 
   }
 
+  @Test
+  public void testEscapeDistinguishedName() throws NoSuchMethodException, InvalidNameException {
+    ADKerberosOperationHandler handler = new ADKerberosOperationHandler();
+
+    try {
+      handler.escapeDNCharacters("nn/c6501.ambari.apache.org");
+      Assert.fail("Expected InvalidNameException");
+    } catch (InvalidNameException e) {
+      // This is expected
+    }
+
+    Assert.assertEquals("CN=nn\\/c6501.ambari.apache.org,OU=HDP,DC=HDP01,DC=LOCAL",
+        handler.escapeDNCharacters("CN=nn/c6501.ambari.apache.org,OU=HDP,DC=HDP01,DC=LOCAL"));
+  }
+
   /**
    * Implementation to illustrate the use of operations on this class
    *
@@ -490,6 +506,24 @@ public class ADKerberosOperationHandlerTest extends KerberosOperationHandlerTest
 
     handler.close();
 
+    handler.open(credentials, realm, kerberosEnvMap);
+
+    String evaluatedPrincipal;
+
+    evaluatedPrincipal = "nn/c6501.ambari.apache.org@" + DEFAULT_REALM;
+    if (handler.principalExists(evaluatedPrincipal)) {
+      handler.setPrincipalPassword(evaluatedPrincipal, handler.createSecurePassword());
+    } else {
+      handler.createPrincipal(evaluatedPrincipal, handler.createSecurePassword(), true);
+    }
+
+    evaluatedPrincipal = "hdfs@" + DEFAULT_REALM;
+    if (handler.principalExists(evaluatedPrincipal)) {
+      handler.setPrincipalPassword(evaluatedPrincipal, handler.createSecurePassword());
+    } else {
+      handler.createPrincipal(evaluatedPrincipal, handler.createSecurePassword(), true);
+    }
+
     kerberosEnvMap.put(ADKerberosOperationHandler.KERBEROS_ENV_CREATE_ATTRIBUTES_TEMPLATE,
         "#set( $user = \"${principal_primary}-${principal_digest}\" )" +
             "{" +
@@ -511,6 +545,8 @@ public class ADKerberosOperationHandlerTest extends KerberosOperationHandlerTest
             "}"
     );
 
+    handler.close();
+
     handler.open(credentials, realm, kerberosEnvMap);
 
     // remove the principal

ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerTest.java

@@ -28,8 +28,10 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
+import javax.naming.InvalidNameException;
 import java.io.File;
 import java.io.FileInputStream;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -244,6 +246,39 @@ public abstract class KerberosOperationHandlerTest extends EasyMockSupport {
     );
   }
 
+  @Test
+  public void testEscapeCharacters() throws KerberosOperationException {
+    KerberosOperationHandler handler = createHandler();
+
+    HashSet<Character> specialCharacters = new HashSet<Character>() {
+      {
+        add('/');
+        add(',');
+        add('\\');
+        add('#');
+        add('+');
+        add('<');
+        add('>');
+        add(';');
+        add('"');
+        add('=');
+        add(' ');
+      }
+    };
+
+    Assert.assertEquals("\\/\\,\\\\\\#\\+\\<\\>\\;\\\"\\=\\ ", handler.escapeCharacters("/,\\#+<>;\"= ", specialCharacters, '\\'));
+    Assert.assertNull(handler.escapeCharacters(null, specialCharacters, '\\'));
+    Assert.assertEquals("", handler.escapeCharacters("", specialCharacters, '\\'));
+    Assert.assertEquals("nothing_special_here", handler.escapeCharacters("nothing_special_here", specialCharacters, '\\'));
+    Assert.assertEquals("\\/\\,\\\\\\#\\+\\<\\>\\;\\\"\\=\\ ", handler.escapeCharacters("/,\\#+<>;\"= ", specialCharacters, '\\'));
+
+    Assert.assertEquals("nothing<>special#here!", handler.escapeCharacters("nothing<>special#here!", null, '\\'));
+    Assert.assertEquals("nothing<>special#here!", handler.escapeCharacters("nothing<>special#here!", Collections.<Character>emptySet(), '\\'));
+    Assert.assertEquals("nothing<>special#here!", handler.escapeCharacters("nothing<>special#here!", Collections.singleton('?'), '\\'));
+    Assert.assertEquals("\\A's are special!", handler.escapeCharacters("A's are special!", Collections.singleton('A'), '\\'));
+  }
+
+
   private KerberosOperationHandler createHandler() throws KerberosOperationException {
     KerberosOperationHandler handler = new KerberosOperationHandler() {