r21606: Implement escaping function for ldap RDN values

Fix escaping of DN components and filters around the code
Add some notes to commandline help messages about how to pass DNs

revert jra's "concistency" commit to nsswitch/winbindd_ads.c, as it was
incorrect.
The 2 functions use DNs in different ways.

- lookup_usergroups_member() uses the DN in a search filter,
and must use the filter escaping function to escape it
Escaping filters that include escaped DNs ("\," becomes "\5c,") is the
correct way to do it (tested against W2k3).

- lookup_usergroups_memberof() instead uses the DN ultimately as a base dn.
Both functions do NOT need any DN escaping function as DNs can't be reliably
escaped when in a string form, intead each single RDN value must be escaped
separately.

DNs coming from other ldap calls (like ads_get_dn()), do not need escaping as
they come already escaped on the wire and passed as is by the ldap libraries

DN filtering has been tested.
For example now it is possible to do something like:
'net ads add user joe#5' as now the '#' character is correctly escaped when
building the DN, previously such a call failed with Invalid DN Syntax.

Simo.

diff --git a/source/lib/ldap_escape.c b/source/lib/ldap_escape.c
index 26230884341deb1952064b16c7d36dcc9cb7f98c..8907399be4a71ef8908ee80097e792f4dc380f76 100644 (file)
--- a/source/lib/ldap_escape.c
+++ b/source/lib/ldap_escape.c
@@ -89,3 +89,47 @@ char *escape_ldap_string_alloc(const char *s)
        *p = '\0';
        return output;
 }
+
+char *escape_rdn_val_string_alloc(const char *s)
+{
+       char *output, *p;
+
+       /* The maximum size of the escaped string can be twice the actual size */
+       output = (char *)SMB_MALLOC(2*strlen(s) + 1);
+
+       if (output == NULL) {
+               return NULL;
+       }
+
+       p = output;
+       
+       while (*s)
+       {
+               switch (*s)
+               {
+               case ',':
+               case '=':
+               case '+':
+               case '<':
+               case '>':
+               case '#':
+               case ';':
+               case '\\':
+               case '\"':
+                       *p++ = '\\';
+                       *p++ = *s;
+                       break;
+               default:
+                       *p = *s;
+                       p++;
+               }
+               
+               s++;
+       }
+       
+       *p = '\0';
+
+       /* resize the string to the actual final size */
+       output = (char *)SMB_REALLOC(output, strlen(output) + 1);
+       return output;
+}

diff --git a/source/lib/smbldap_util.c b/source/lib/smbldap_util.c
index aff4eff6f6d0fa6f8c40287112a7c12eb80d7af5..11b27bf98fb44c4fd1d8dbdeeef4977020f31afb 100644 (file)
--- a/source/lib/smbldap_util.c
+++ b/source/lib/smbldap_util.c
@@ -39,12 +39,21 @@ static NTSTATUS add_new_domain_account_policies(struct smbldap_state *ldap_state
        const char *policy_attr = NULL;
        pstring dn;
        LDAPMod **mods = NULL;
+       char *escape_domain_name;
 
        DEBUG(3,("add_new_domain_account_policies: Adding new account policies for domain\n"));
-       
+
+       escape_domain_name = escape_rdn_val_string_alloc(domain_name);
+       if (!escape_domain_name) {
+               DEBUG(0, ("Out of memory!\n"));
+               return NT_STATUS_NO_MEMORY;
+       }
+
        pstr_sprintf(dn, "%s=%s,%s", 
                get_attr_key2string(dominfo_attr_list, LDAP_ATTR_DOMAIN),
-               domain_name, lp_ldap_suffix());
+               escape_domain_name, lp_ldap_suffix());
+
+       SAFE_FREE(escape_domain_name);
 
        for (i=1; decode_account_policy_name(i) != NULL; i++) {
 
@@ -104,10 +113,20 @@ static NTSTATUS add_new_domain_info(struct smbldap_state *ldap_state,
        LDAPMessage *result = NULL;
        int num_result;
        const char **attr_list;
+       char *escape_domain_name;
+
+       /* escape for filter */
+       escape_domain_name = escape_ldap_string_alloc(domain_name);
+       if (!escape_domain_name) {
+               DEBUG(0, ("Out of memory!\n"));
+               return NT_STATUS_NO_MEMORY;
+       }
 
        slprintf (filter, sizeof (filter) - 1, "(&(%s=%s)(objectclass=%s))", 
                  get_attr_key2string(dominfo_attr_list, LDAP_ATTR_DOMAIN), 
-                 domain_name, LDAP_OBJ_DOMINFO);
+                 escape_domain_name, LDAP_OBJ_DOMINFO);
+
+       SAFE_FREE(escape_domain_name);
 
        attr_list = get_attr_list( NULL, dominfo_attr_list );
        rc = smbldap_search_suffix(ldap_state, filter, attr_list, &result);
@@ -129,9 +148,18 @@ static NTSTATUS add_new_domain_info(struct smbldap_state *ldap_state,
        /* Check if we need to add an entry */
        DEBUG(3,("add_new_domain_info: Adding new domain\n"));
 
+       /* this time escape for DN */
+       escape_domain_name = escape_rdn_val_string_alloc(domain_name);
+       if (!escape_domain_name) {
+               DEBUG(0, ("Out of memory!\n"));
+               return NT_STATUS_NO_MEMORY;
+       }
+
        pstr_sprintf(dn, "%s=%s,%s",
                     get_attr_key2string(dominfo_attr_list, LDAP_ATTR_DOMAIN),
-                    domain_name, lp_ldap_suffix());
+                    escape_domain_name, lp_ldap_suffix());
+
+       SAFE_FREE(escape_domain_name);
 
        /* Free original search */
        ldap_msgfree(result);
@@ -210,11 +238,20 @@ NTSTATUS smbldap_search_domain_info(struct smbldap_state *ldap_state,
        int rc;
        const char **attr_list;
        int count;
+       char *escape_domain_name;
+       
+       escape_domain_name = escape_ldap_string_alloc(domain_name);
+       if (!escape_domain_name) {
+               DEBUG(0, ("Out of memory!\n"));
+               return NT_STATUS_NO_MEMORY;
+       }
 
        pstr_sprintf(filter, "(&(objectClass=%s)(%s=%s))",
                LDAP_OBJ_DOMINFO,
                get_attr_key2string(dominfo_attr_list, LDAP_ATTR_DOMAIN), 
-               domain_name);
+               escape_domain_name);
+
+       SAFE_FREE(escape_domain_name);
 
        DEBUG(2, ("smbldap_search_domain_info: Searching for:[%s]\n", filter));
 

diff --git a/source/libads/ldap.c b/source/libads/ldap.c
index dfc68fdc2b14eda2bd8f79e3bacf96a4587dac7a..e9d8bf6bbf283bfd1b30c85ff617e7de484bc358 100644 (file)
--- a/source/libads/ldap.c
+++ b/source/libads/ldap.c
@@ -1635,6 +1635,7 @@ ADS_STATUS ads_create_machine_acct(ADS_STRUCT *ads, const char *machine_name,
        char *samAccountName, *controlstr;
        TALLOC_CTX *ctx;
        ADS_MODLIST mods;
+       char *machine_escaped;
        char *new_dn;
        const char *objectClass[] = {"top", "person", "organizationalPerson",
                                     "user", "computer", NULL};
@@ -1647,8 +1648,13 @@ ADS_STATUS ads_create_machine_acct(ADS_STRUCT *ads, const char *machine_name,
                return ADS_ERROR(LDAP_NO_MEMORY);
 
        ret = ADS_ERROR(LDAP_NO_MEMORY);
-               
-       new_dn = talloc_asprintf(ctx, "cn=%s,%s", machine_name, org_unit);
+
+       machine_escaped = escape_rdn_val_string_alloc(machine_name);
+       if (!machine_escaped) {
+               goto done;
+       }
+
+       new_dn = talloc_asprintf(ctx, "cn=%s,%s", machine_escaped, org_unit);
        samAccountName = talloc_asprintf(ctx, "%s$", machine_name);
 
        if ( !new_dn || !samAccountName ) {

diff --git a/source/libads/ldap_user.c b/source/libads/ldap_user.c
index 66d94d29d3acdea94b3d3778f4f9673c9a2e3aa5..afbbc0b421a7d2e09eed72de0ee02ef22ae38a09 100644 (file)
--- a/source/libads/ldap_user.c
+++ b/source/libads/ldap_user.c
@@ -50,6 +50,7 @@ ADS_STATUS ads_add_user_acct(ADS_STRUCT *ads, const char *user,
        ADS_MODLIST mods;
        ADS_STATUS status;
        const char *upn, *new_dn, *name, *controlstr;
+       char *name_escaped = NULL;
        const char *objectClass[] = {"top", "person", "organizationalPerson",
                                     "user", NULL};
 
@@ -63,7 +64,9 @@ ADS_STATUS ads_add_user_acct(ADS_STRUCT *ads, const char *user,
 
        if (!(upn = talloc_asprintf(ctx, "%s@%s", user, ads->config.realm)))
                goto done;
-       if (!(new_dn = talloc_asprintf(ctx, "cn=%s,%s,%s", name, container,
+       if (!(name_escaped = escape_rdn_val_string_alloc(name)))
+               goto done;
+       if (!(new_dn = talloc_asprintf(ctx, "cn=%s,%s,%s", name_escaped, container,
                                       ads->config.bind_path)))
                goto done;
        if (!(controlstr = talloc_asprintf(ctx, "%u", (UF_NORMAL_ACCOUNT | UF_ACCOUNTDISABLE))))
@@ -81,6 +84,7 @@ ADS_STATUS ads_add_user_acct(ADS_STRUCT *ads, const char *user,
        status = ads_gen_add(ads, new_dn, mods);
 
  done:
+       SAFE_FREE(name_escaped);
        talloc_destroy(ctx);
        return status;
 }
@@ -92,6 +96,7 @@ ADS_STATUS ads_add_group_acct(ADS_STRUCT *ads, const char *group,
        ADS_MODLIST mods;
        ADS_STATUS status;
        char *new_dn;
+       char *name_escaped = NULL;
        const char *objectClass[] = {"top", "group", NULL};
 
        if (!(ctx = talloc_init("ads_add_group_acct")))
@@ -99,7 +104,9 @@ ADS_STATUS ads_add_group_acct(ADS_STRUCT *ads, const char *group,
 
        status = ADS_ERROR(LDAP_NO_MEMORY);
 
-       if (!(new_dn = talloc_asprintf(ctx, "cn=%s,%s,%s", group, container,
+       if (!(name_escaped = escape_rdn_val_string_alloc(group)))
+               goto done;
+       if (!(new_dn = talloc_asprintf(ctx, "cn=%s,%s,%s", name_escaped, container,
                                       ads->config.bind_path)))
                goto done;
        if (!(mods = ads_init_mods(ctx)))
@@ -114,6 +121,7 @@ ADS_STATUS ads_add_group_acct(ADS_STRUCT *ads, const char *group,
        status = ads_gen_add(ads, new_dn, mods);
 
  done:
+       SAFE_FREE(name_escaped);
        talloc_destroy(ctx);
        return status;
 }

diff --git a/source/nsswitch/winbindd_ads.c b/source/nsswitch/winbindd_ads.c
index 7fcdec0b7dc43f6e3534bbf49d202ad099fb1cc0..9c8f23b1cf77f0410f8fe08a47aca43f64e92dfe 100644 (file)
--- a/source/nsswitch/winbindd_ads.c
+++ b/source/nsswitch/winbindd_ads.c
@@ -607,7 +607,6 @@ static NTSTATUS lookup_usergroups_memberof(struct winbindd_domain *domain,
        const char *attrs[] = {"memberOf", NULL};
        size_t num_groups = 0;
        DOM_SID *group_sids = NULL;
-       char *escaped_dn;
        int i;
 
        DEBUG(3,("ads: lookup_usergroups_memberof\n"));
@@ -619,16 +618,9 @@ static NTSTATUS lookup_usergroups_memberof(struct winbindd_domain *domain,
                goto done;
        }
 
-       if (!(escaped_dn = escape_ldap_string_alloc(user_dn))) {
-               status = NT_STATUS_NO_MEMORY;
-               goto done;
-       }
-
-       rc = ads_search_retry_extended_dn(ads, &res, escaped_dn, attrs, 
+       rc = ads_search_retry_extended_dn(ads, &res, user_dn, attrs, 
                                          ADS_EXTENDED_DN_HEX_STRING);
        
-       SAFE_FREE(escaped_dn);
-
        if (!ADS_ERR_OK(rc) || !res) {
                DEBUG(1,("lookup_usergroups_memberof ads_search member=%s: %s\n", 
                        user_dn, ads_errstr(rc)));

diff --git a/source/passdb/pdb_ldap.c b/source/passdb/pdb_ldap.c
index 533b936efd9186869844b4f7c6f685dae463aa7f..4edffe97d9f975dfeb7f5c5d213f16fc0f874649 100644 (file)
--- a/source/passdb/pdb_ldap.c
+++ b/source/passdb/pdb_ldap.c
@@ -2049,14 +2049,25 @@ static NTSTATUS ldapsam_add_sam_account(struct pdb_methods *my_methods, struct s
        TALLOC_FREE( attr_list );
 
        if (num_result == 0) {
+               char *escape_username;
                /* Check if we need to add an entry */
                DEBUG(3,("ldapsam_add_sam_account: Adding new user\n"));
                ldap_op = LDAP_MOD_ADD;
+
+               escape_username = escape_rdn_val_string_alloc(username);
+               if (!escape_username) {
+                       DEBUG(0, ("Out of memory!\n"));
+                       ldap_msgfree(result);
+                       return NT_STATUS_NO_MEMORY;
+               }
+
                if (username[strlen(username)-1] == '$') {
-                       slprintf (dn, sizeof (dn) - 1, "uid=%s,%s", username, lp_ldap_machine_suffix ());
+                       slprintf (dn, sizeof (dn) - 1, "uid=%s,%s", escape_username, lp_ldap_machine_suffix ());
                } else {
-                       slprintf (dn, sizeof (dn) - 1, "uid=%s,%s", username, lp_ldap_user_suffix ());
+                       slprintf (dn, sizeof (dn) - 1, "uid=%s,%s", escape_username, lp_ldap_user_suffix ());
                }
+
+               SAFE_FREE(escape_username);
        }
 
        if (!init_ldap_from_sam(ldap_state, entry, &mods, newpwd,
@@ -2415,11 +2426,21 @@ static NTSTATUS ldapsam_enum_group_members(struct pdb_methods *methods,
                }
 
                for (memberuid = values; *memberuid != NULL; memberuid += 1) {
-                       filter = talloc_asprintf_append(filter, "(uid=%s)", *memberuid);
+                       char *escape_memberuid;
+
+                       escape_memberuid = escape_ldap_string_alloc(*memberuid);
+                       if (escape_memberuid == NULL) {
+                               ret = NT_STATUS_NO_MEMORY;
+                               goto done;
+                       }
+                       
+                       filter = talloc_asprintf_append(filter, "(uid=%s)", escape_memberuid);
                        if (filter == NULL) {
                                ret = NT_STATUS_NO_MEMORY;
                                goto done;
                        }
+
+                       SAFE_FREE(escape_memberuid);
                }
 
                filter = talloc_asprintf_append(filter, "))");
@@ -4773,6 +4794,8 @@ static NTSTATUS ldapsam_create_user(struct pdb_methods *my_methods,
        smbldap_set_mod(&mods, LDAP_MOD_ADD, "objectClass", LDAP_OBJ_SAMBASAMACCOUNT);
 
        if (add_posix) {
+               char *escape_name;
+
                DEBUG(3,("ldapsam_create_user: Creating new posix user\n"));
 
                /* retrieve the Domain Users group gid */
@@ -4799,12 +4822,21 @@ static NTSTATUS ldapsam_create_user(struct pdb_methods *my_methods,
                }
                uidstr = talloc_asprintf(tmp_ctx, "%d", uid);
                gidstr = talloc_asprintf(tmp_ctx, "%d", gid);
+
+               escape_name = escape_rdn_val_string_alloc(name);
+               if (!escape_name) {
+                       DEBUG (0, ("ldapsam_create_user: Out of memory!\n"));
+                       return NT_STATUS_NO_MEMORY;
+               }
+
                if (is_machine) {
-                       dn = talloc_asprintf(tmp_ctx, "uid=%s,%s", name, lp_ldap_machine_suffix ());
+                       dn = talloc_asprintf(tmp_ctx, "uid=%s,%s", escape_name, lp_ldap_machine_suffix ());
                } else {
-                       dn = talloc_asprintf(tmp_ctx, "uid=%s,%s", name, lp_ldap_user_suffix ());
+                       dn = talloc_asprintf(tmp_ctx, "uid=%s,%s", escape_name, lp_ldap_user_suffix ());
                }
 
+               SAFE_FREE(escape_name);
+
                if (!homedir || !shell || !uidstr || !gidstr || !dn) {
                        DEBUG (0, ("ldapsam_create_user: Out of memory!\n"));
                        return NT_STATUS_NO_MEMORY;
@@ -4986,6 +5018,8 @@ static NTSTATUS ldapsam_create_dom_group(struct pdb_methods *my_methods,
        }
 
        if (num_result == 0) {
+               char *escape_name;
+
                DEBUG(3,("ldapsam_create_user: Creating new posix group\n"));
 
                is_new_entry = True;
@@ -4997,7 +5031,16 @@ static NTSTATUS ldapsam_create_dom_group(struct pdb_methods *my_methods,
                }
 
                gidstr = talloc_asprintf(tmp_ctx, "%d", gid);
-               dn = talloc_asprintf(tmp_ctx, "cn=%s,%s", name, lp_ldap_group_suffix());
+
+               escape_name = escape_rdn_val_string_alloc(name);
+               if (!escape_name) {
+                       DEBUG (0, ("ldapsam_create_group: Out of memory!\n"));
+                       return NT_STATUS_NO_MEMORY;
+               }
+
+               dn = talloc_asprintf(tmp_ctx, "cn=%s,%s", escape_name, lp_ldap_group_suffix());
+
+               SAFE_FREE(escape_name);
 
                if (!gidstr || !dn) {
                        DEBUG (0, ("ldapsam_create_group: Out of memory!\n"));
@@ -5335,6 +5378,7 @@ static NTSTATUS ldapsam_set_primary_group(struct pdb_methods *my_methods,
        uint32 num_result;
        LDAPMod **mods = NULL;
        char *filter;
+       char *escape_username;
        char *gidstr;
        const char *dn = NULL;
        gid_t gid;
@@ -5351,14 +5395,22 @@ static NTSTATUS ldapsam_set_primary_group(struct pdb_methods *my_methods,
                DEBUG(0,("ldapsam_set_primary_group: Out of Memory!\n"));
                return NT_STATUS_NO_MEMORY;
        }
-       
+
+       escape_username = escape_ldap_string_alloc(pdb_get_username(sampass));
+       if (escape_username== NULL) {
+               return NT_STATUS_NO_MEMORY;
+       }
+
        filter = talloc_asprintf(mem_ctx,
                                 "(&(uid=%s)"
                                 "(objectClass=%s)"
                                 "(objectClass=%s))",
-                                pdb_get_username(sampass),
+                                escape_username,
                                 LDAP_OBJ_POSIXACCOUNT,
                                 LDAP_OBJ_SAMBASAMACCOUNT);
+
+       SAFE_FREE(escape_username);
+
        if (filter == NULL) {
                return NT_STATUS_NO_MEMORY;
        }

diff --git a/source/printing/nt_printing.c b/source/printing/nt_printing.c
index 652726c401846b40138923556eb80b525f93b292..77b31bc51795f91b83fec058c1a6a578d1ac51ef 100644 (file)
--- a/source/printing/nt_printing.c
+++ b/source/printing/nt_printing.c
@@ -3034,7 +3034,7 @@ static WERROR nt_printer_publish_ads(ADS_STRUCT *ads,
 {
        ADS_STATUS ads_rc;
        LDAPMessage *res;
-       char *prt_dn = NULL, *srv_dn, *srv_cn_0;
+       char *prt_dn = NULL, *srv_dn, *srv_cn_0, *srv_cn_escaped, *sharename_escaped;
        char *srv_dn_utf8, **srv_cn_utf8;
        TALLOC_CTX *ctx;
        ADS_MODLIST mods;
@@ -3080,11 +3080,29 @@ static WERROR nt_printer_publish_ads(ADS_STRUCT *ads,
        ldap_memfree(srv_dn_utf8);
        ldap_memfree(srv_cn_utf8);
 
-       asprintf(&prt_dn, "cn=%s-%s,%s", srv_cn_0, 
-                printer->info_2->sharename, srv_dn);
+       srv_cn_escaped = escape_rdn_val_string_alloc(srv_cn_0);
+       if (!srv_cn_escaped) {
+               SAFE_FREE(srv_cn_0);
+               ldap_memfree(srv_dn_utf8);
+               ads_destroy(&ads);
+               return WERR_SERVER_UNAVAILABLE;
+       }
+       sharename_escaped = escape_rdn_val_string_alloc(printer->info_2->sharename);
+       if (!sharename_escaped) {
+               SAFE_FREE(srv_cn_escaped);
+               SAFE_FREE(srv_cn_0);
+               ldap_memfree(srv_dn_utf8);
+               ads_destroy(&ads);
+               return WERR_SERVER_UNAVAILABLE;
+       }
+
+
+       asprintf(&prt_dn, "cn=%s-%s,%s", srv_cn_escaped, sharename_escaped, srv_dn);
 
        SAFE_FREE(srv_dn);
        SAFE_FREE(srv_cn_0);
+       SAFE_FREE(srv_cn_escaped);
+       SAFE_FREE(sharename_escaped);
 
        /* build the ads mods */
        ctx = talloc_init("nt_printer_publish_ads");

diff --git a/source/utils/net_ads.c b/source/utils/net_ads.c
index f2fa8073226d0ca1861cd3369142a488bc30c17e..cb5b08c672e9062e52e9f05e2234589d2b245308 100644 (file)
--- a/source/utils/net_ads.c
+++ b/source/utils/net_ads.c
@@ -1819,6 +1819,7 @@ static int net_ads_printer_publish(int argc, const char **argv)
        TALLOC_CTX *mem_ctx = talloc_init("net_ads_printer_publish");
        ADS_MODLIST mods = ads_init_mods(mem_ctx);
        char *prt_dn, *srv_dn, **srv_cn;
+       char *srv_cn_escaped, *printername_escaped;
        LDAPMessage *res = NULL;
 
        if (!ADS_ERR_OK(ads_startup(True, &ads))) {
@@ -1870,7 +1871,15 @@ static int net_ads_printer_publish(int argc, const char **argv)
        srv_dn = ldap_get_dn((LDAP *)ads->ld, (LDAPMessage *)res);
        srv_cn = ldap_explode_dn(srv_dn, 1);
 
-       asprintf(&prt_dn, "cn=%s-%s,%s", srv_cn[0], printername, srv_dn);
+       srv_cn_escaped = escape_rdn_val_string_alloc(srv_cn[0]);
+       printername_escaped = escape_rdn_val_string_alloc(printername);
+       if (!srv_cn_escaped || !printername_escaped) {
+               d_fprintf(stderr, "Internal error, out of memory!");
+               ads_destroy(&ads);
+               return -1;
+       }
+
+       asprintf(&prt_dn, "cn=%s-%s,%s", srv_cn_escaped, printername_escaped, srv_dn);
 
        pipe_hnd = cli_rpc_pipe_open_noauth(cli, PI_SPOOLSS, &nt_status);
        if (!pipe_hnd) {
@@ -2158,6 +2167,7 @@ static int net_ads_dn_usage(int argc, const char **argv)
                "The DN standard LDAP DN, and the attributes are a list of LDAP fields \n"\
                "to show in the results\n\n"\
                "Example: net ads dn 'CN=administrator,CN=Users,DC=my,DC=domain' sAMAccountName\n\n"
+               "Note: the DN must be provided properly escaped. See RFC 4514 for details\n\n"
                );
        net_common_flags_usage(argc, argv);
        return -1;

diff --git a/source/utils/net_ads_gpo.c b/source/utils/net_ads_gpo.c
index 1865aee3d41be1f18e16f9b380025bb4388fd8f3..9cdc69d9899a01c0e4bca5dbe23a3a10a3e3c618 100644 (file)
--- a/source/utils/net_ads_gpo.c
+++ b/source/utils/net_ads_gpo.c
@@ -351,6 +351,7 @@ static int net_ads_gpo_add_link(int argc, const char **argv)
 
        if (argc < 2) {
                printf("usage: net ads gpo addlink <linkdn> <gpodn> [options]\n");
+               printf("note: DNs must be provided properly escaped.\n      See RFC 4514 for details");
                return -1;
        }