Fix quoting issues in LDAP KDB module

Modify ldap_filter_correct() to quote special characters for DN
strings as well as filters, since it is already used to quote a DN
string in krb5_ldap_name_to_policydn() and there's no harm in
over-quoting.  In krb5_ldap_put_principal(), quote the unparsed
principal name for use in DNs we choose.  In
krb5_ldap_create_password_policy(), use the policy name for the CN of
the policy entry instead of the (possibly quoted) first element of the
DN.

Adapted from a patch by Jim Shi <hanmao_shi@apple.com>.

ticket: 7296

3 files changed, 30 insertions, 81 deletions

diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
index a7101a738..c386a9ea9 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
@@ -496,6 +496,7 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry,
     LDAPMessage                 *result=NULL, *ent=NULL;
     char                        *user=NULL, *subtree=NULL, *principal_dn=NULL;
     char                        **values=NULL, *strval[10]={NULL}, errbuf[1024];
+    char                        *filtuser=NULL;
     struct berval               **bersecretkey=NULL;
     LDAPMod                     **mods=NULL;
     krb5_boolean                create_standalone_prinicipal=FALSE;
@@ -534,6 +535,11 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry,
         if (((st=krb5_unparse_name(context, entry->princ, &user)) != 0) ||
             ((st=krb5_ldap_unparse_principal_name(user)) != 0))
             goto cleanup;
+        filtuser = ldap_filter_correct(user);
+        if (filtuser == NULL) {
+            st = ENOMEM;
+            goto cleanup;
+        }
     }
 
     /* Identity the type of operation, it can be
@@ -554,7 +560,7 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry,
         goto cleanup;
 
     if (entry->mask & KADM5_LOAD) {
-        int              tree = 0, ntrees = 0, princlen = 0, numlentries = 0;
+        int              tree = 0, ntrees = 0, numlentries = 0;
         char             **subtreelist = NULL, *filter = NULL;
 
         /*  A load operation is special, will do a mix-in (add krbprinc
@@ -572,12 +578,11 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry,
                                      "name not found"));
             goto cleanup;
         }
-        princlen = strlen(FILTER) + strlen(user) + 2 + 1;      /* 2 for closing brackets */
-        if ((filter = malloc(princlen)) == NULL) {
+        if (asprintf(&filter, FILTER"%s))", filtuser) < 0) {
+            filter = NULL;
             st = ENOMEM;
             goto cleanup;
         }
-        snprintf(filter, princlen, FILTER"%s))", user);
 
         /* get the current subtree list */
         if ((st = krb5_get_subtree_info(ldap_context, &subtreelist, &ntrees)) != 0)
@@ -684,7 +689,7 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry,
         CHECK_NULL(subtree);
 
         if (asprintf(&standalone_principal_dn, "krbprincipalname=%s,%s",
-                     user, subtree) < 0)
+                     filtuser, subtree) < 0)
             standalone_principal_dn = NULL;
         CHECK_NULL(standalone_principal_dn);
         /*
@@ -1262,6 +1267,9 @@ cleanup:
     if (user)
         free(user);
 
+    if (filtuser)
+        free(filtuser);
+
     free_xargs(xargs);
 
     if (standalone_principal_dn)
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_pwd_policy.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_pwd_policy.c
index 09cfb8ca0..e955f8e40 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_pwd_policy.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_pwd_policy.c
@@ -140,7 +140,7 @@ krb5_ldap_create_password_policy(krb5_context context, osa_policy_ent_t policy)
     kdb5_dal_handle             *dal_handle=NULL;
     krb5_ldap_context           *ldap_context=NULL;
     krb5_ldap_server_handle     *ldap_server_handle=NULL;
-    char                        **rdns=NULL, *strval[2]={NULL}, *policy_dn;
+    char                        *strval[2]={NULL}, *policy_dn;
 
     /* Clear the global error string */
     krb5_clear_error_message(context);
@@ -156,16 +156,7 @@ krb5_ldap_create_password_policy(krb5_context context, osa_policy_ent_t policy)
     if (st != 0)
         goto cleanup;
 
-    /* get the first component of the dn to set the cn attribute */
-    rdns = ldap_explode_dn(policy_dn, 1);
-    if (rdns == NULL) {
-        st = EINVAL;
-        krb5_set_error_message(context, st,
-                               _("Invalid password policy DN syntax"));
-        goto cleanup;
-    }
-
-    strval[0] = rdns[0];
+    strval[0] = policy->name;
     if ((st=krb5_add_str_mem_ldap_mod(&mods, "cn", LDAP_MOD_ADD, strval)) != 0)
         goto cleanup;
 
@@ -184,9 +175,6 @@ krb5_ldap_create_password_policy(krb5_context context, osa_policy_ent_t policy)
     }
 
 cleanup:
-    if (rdns)
-        ldap_value_free(rdns);
-
     if (policy_dn != NULL)
         free (policy_dn);
     ldap_mods_free(mods, 1);
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_realm.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_realm.c
index 45649da02..7e0d45689 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_realm.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_realm.c
@@ -70,72 +70,25 @@ char  *krbContainerRefclass[] = { "krbContainerRefAux", NULL};
  * list realms from eDirectory
  */
 
-/*
- * Function to remove all special characters from a string (rfc2254).
- * Use whenever exact matching is to be done ...
- */
+/* Return a copy of in, quoting all characters which are special in an LDAP
+ * filter (RFC 4515) or DN string (RFC 4514).  Return NULL on failure. */
 char *
 ldap_filter_correct (char *in)
 {
-    size_t i, count;
-    char *out, *ptr;
-    size_t len = strlen(in);
-
-    for (i = 0, count = 0; i < len; i++)
-        switch (in[i]) {
-        case '*':
-        case '(':
-        case ')':
-        case '\\':
-        case '\0':
-            count ++;
-        }
-
-    out = (char *)malloc((len + (count * 2) + 1) * sizeof (char));
-    assert (out != NULL);
-    memset(out, 0, len + (count * 2) + 1);
-
-    for (i = 0, ptr = out; i < len; i++)
-        switch (in[i]) {
-        case '*':
-            ptr[0] = '\\';
-            ptr[1] = '2';
-            ptr[2] = 'a';
-            ptr += 3;
-            break;
-        case '(':
-            ptr[0] = '\\';
-            ptr[1] = '2';
-            ptr[2] = '8';
-            ptr += 3;
-            break;
-        case ')':
-            ptr[0] = '\\';
-            ptr[1] = '2';
-            ptr[2] = '9';
-            ptr += 3;
-            break;
-        case '\\':
-            ptr[0] = '\\';
-            ptr[1] = '5';
-            ptr[2] = 'c';
-            ptr += 3;
+    size_t count;
+    const char special[] = "*()\\ #\"+,;<>";
+    struct k5buf buf;
+
+    krb5int_buf_init_dynamic(&buf);
+    while (TRUE) {
+        count = strcspn(in, special);
+        krb5int_buf_add_len(&buf, in, count);
+        in += count;
+        if (*in == '\0')
             break;
-        case '\0':
-            ptr[0] = '\\';
-            ptr[1] = '0';
-            ptr[2] = '0';
-            ptr += 3;
-            break;
-        default:
-            ptr[0] = in[i];
-            ptr += 1;
-            break;
-        }
-
-    /* ptr[count - 1] = '\0'; */
-
-    return out;
+        krb5int_buf_add_fmt(&buf, "\\%2x", (unsigned char)*in++);
+    }
+    return krb5int_buf_data(&buf);
 }
 
 static int