The branch, master has been updated via 6d59be1e6d83d4faf145c9b6d574bab9f2acb36a (commit) via 9a7900fb38b9690bf51ab638c0f0629f2557b870 (commit) from 0e62bb39de93433dfeb7f822ec1026da7ed643f4 (commit)
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 6d59be1e6d83d4faf145c9b6d574bab9f2acb36a Author: Steven Danneman <[EMAIL PROTECTED]> Date: Sat Nov 15 13:07:15 2008 -0800 Fix extended DN parse error when AD object does not have a SID. Some AD objects, like Exchange Public Folders, can be members of Security Groups but do not have a SID attribute. This patch adds more granular return errors to ads_get_sid_from_extended_dn(). Callers can now determine if a parse error occured because of bad input, or the DN was valid but contained no SID. I updated all callers to ignore SIDless objects when appropriate. Also did some cleanup to the out paths of lookup_usergroups_memberof() commit 9a7900fb38b9690bf51ab638c0f0629f2557b870 Author: Steven Danneman <[EMAIL PROTECTED]> Date: Fri Nov 14 19:16:12 2008 -0800 Whitespace and >80 column cleanups. ----------------------------------------------------------------------- Summary of changes: source3/include/proto.h | 8 +- source3/libads/ldap.c | 78 ++++++++++++++---------- source3/winbindd/winbindd_ads.c | 125 +++++++++++++++++++++--------------- source3/winbindd/winbindd_group.c | 4 +- 4 files changed, 125 insertions(+), 90 deletions(-) Changeset truncated at 500 lines: diff --git a/source3/include/proto.h b/source3/include/proto.h index 3342584..1cdf6c9 100644 --- a/source3/include/proto.h +++ b/source3/include/proto.h @@ -1920,10 +1920,10 @@ ADS_STATUS ads_get_joinable_ous(ADS_STRUCT *ads, TALLOC_CTX *mem_ctx, char ***ous, size_t *num_ous); -bool ads_get_sid_from_extended_dn(TALLOC_CTX *mem_ctx, - const char *extended_dn, - enum ads_extended_dn_flags flags, - DOM_SID *sid); +ADS_STATUS ads_get_sid_from_extended_dn(TALLOC_CTX *mem_ctx, + const char *extended_dn, + enum ads_extended_dn_flags flags, + DOM_SID *sid); char* ads_get_dnshostname( ADS_STRUCT *ads, TALLOC_CTX *ctx, const char *machine_name ); char* ads_get_upn( ADS_STRUCT *ads, TALLOC_CTX *ctx, const char *machine_name ); char* ads_get_samaccountname( ADS_STRUCT *ads, TALLOC_CTX *ctx, const char *machine_name ); diff --git a/source3/libads/ldap.c b/source3/libads/ldap.c index 2dcd1fd..f55cfa7 100644 --- a/source3/libads/ldap.c +++ b/source3/libads/ldap.c @@ -3111,60 +3111,66 @@ ADS_STATUS ads_get_joinable_ous(ADS_STRUCT *ads, /** * pull a DOM_SID from an extended dn string - * @param mem_ctx TALLOC_CTX + * @param mem_ctx TALLOC_CTX * @param extended_dn string * @param flags string type of extended_dn * @param sid pointer to a DOM_SID - * @return boolean inidicating success + * @return NT_STATUS_OK on success, + * NT_INVALID_PARAMETER on error, + * NT_STATUS_NOT_FOUND if no SID present **/ -bool ads_get_sid_from_extended_dn(TALLOC_CTX *mem_ctx, - const char *extended_dn, - enum ads_extended_dn_flags flags, - DOM_SID *sid) +ADS_STATUS ads_get_sid_from_extended_dn(TALLOC_CTX *mem_ctx, + const char *extended_dn, + enum ads_extended_dn_flags flags, + DOM_SID *sid) { char *p, *q, *dn; if (!extended_dn) { - return False; + return ADS_ERROR_NT(NT_STATUS_INVALID_PARAMETER); } /* otherwise extended_dn gets stripped off */ if ((dn = talloc_strdup(mem_ctx, extended_dn)) == NULL) { - return False; + return ADS_ERROR_NT(NT_STATUS_INVALID_PARAMETER); } - /* + /* * ADS_EXTENDED_DN_HEX_STRING: * <GUID=238e1963cb390f4bb032ba0105525a29>;<SID=010500000000000515000000bb68c8fd6b61b427572eb04556040000>;CN=gd,OU=berlin,OU=suse,DC=ber,DC=suse,DC=de * * ADS_EXTENDED_DN_STRING (only with w2k3): - <GUID=63198e23-39cb-4b0f-b032-ba0105525a29>;<SID=S-1-5-21-4257769659-666132843-1169174103-1110>;CN=gd,OU=berlin,OU=suse,DC=ber,DC=suse,DC=de + * <GUID=63198e23-39cb-4b0f-b032-ba0105525a29>;<SID=S-1-5-21-4257769659-666132843-1169174103-1110>;CN=gd,OU=berlin,OU=suse,DC=ber,DC=suse,DC=de + * + * Object with no SID, such as an Exchange Public Folder + * <GUID=28907fb4bdf6854993e7f0a10b504e7c>;CN=public,CN=Microsoft Exchange System Objects,DC=sd2k3ms,DC=west,DC=isilon,DC=com */ p = strchr(dn, ';'); if (!p) { - return False; + return ADS_ERROR_NT(NT_STATUS_INVALID_PARAMETER); } if (strncmp(p, ";<SID=", strlen(";<SID=")) != 0) { - return False; + DEBUG(5,("No SID present in extended dn\n")); + return ADS_ERROR_NT(NT_STATUS_NOT_FOUND); } p += strlen(";<SID="); q = strchr(p, '>'); if (!q) { - return False; + return ADS_ERROR_NT(NT_STATUS_INVALID_PARAMETER); } - + *q = '\0'; DEBUG(100,("ads_get_sid_from_extended_dn: sid string is %s\n", p)); switch (flags) { - + case ADS_EXTENDED_DN_STRING: if (!string_to_sid(sid, p)) { - return False; + return ADS_ERROR_NT(NT_STATUS_INVALID_PARAMETER); } break; case ADS_EXTENDED_DN_HEX_STRING: { @@ -3173,21 +3179,21 @@ bool ads_get_sid_from_extended_dn(TALLOC_CTX *mem_ctx, buf_len = strhex_to_str(buf, sizeof(buf), p, strlen(p)); if (buf_len == 0) { - return False; + return ADS_ERROR_NT(NT_STATUS_INVALID_PARAMETER); } if (!sid_parse(buf, buf_len, sid)) { DEBUG(10,("failed to parse sid\n")); - return False; + return ADS_ERROR_NT(NT_STATUS_INVALID_PARAMETER); } break; } default: DEBUG(10,("unknown extended dn format\n")); - return False; + return ADS_ERROR_NT(NT_STATUS_INVALID_PARAMETER); } - return True; + return ADS_ERROR_NT(NT_STATUS_OK); } /** @@ -3200,18 +3206,19 @@ bool ads_get_sid_from_extended_dn(TALLOC_CTX *mem_ctx, * @param sids pointer to sid array to allocate * @return the count of SIDs pulled **/ - int ads_pull_sids_from_extendeddn(ADS_STRUCT *ads, - TALLOC_CTX *mem_ctx, - LDAPMessage *msg, + int ads_pull_sids_from_extendeddn(ADS_STRUCT *ads, + TALLOC_CTX *mem_ctx, + LDAPMessage *msg, const char *field, enum ads_extended_dn_flags flags, DOM_SID **sids) { int i; - size_t dn_count; + ADS_STATUS rc; + size_t dn_count, ret_count = 0; char **dn_strings; - if ((dn_strings = ads_pull_strings(ads, mem_ctx, msg, field, + if ((dn_strings = ads_pull_strings(ads, mem_ctx, msg, field, &dn_count)) == NULL) { return 0; } @@ -3223,18 +3230,25 @@ bool ads_get_sid_from_extended_dn(TALLOC_CTX *mem_ctx, } for (i=0; i<dn_count; i++) { - - if (!ads_get_sid_from_extended_dn(mem_ctx, dn_strings[i], - flags, &(*sids)[i])) { - TALLOC_FREE(*sids); - TALLOC_FREE(dn_strings); - return 0; + rc = ads_get_sid_from_extended_dn(mem_ctx, dn_strings[i], + flags, &(*sids)[i]); + if (!ADS_ERR_OK(rc)) { + if (NT_STATUS_EQUAL(ads_ntstatus(rc), + NT_STATUS_NOT_FOUND)) { + continue; + } + else { + TALLOC_FREE(*sids); + TALLOC_FREE(dn_strings); + return 0; + } } + ret_count++; } TALLOC_FREE(dn_strings); - return dn_count; + return ret_count; } /******************************************************************** diff --git a/source3/winbindd/winbindd_ads.c b/source3/winbindd/winbindd_ads.c index 1febddf..18cc1cb 100644 --- a/source3/winbindd/winbindd_ads.c +++ b/source3/winbindd/winbindd_ads.c @@ -641,9 +641,10 @@ done: tokenGroups are not available. */ static NTSTATUS lookup_usergroups_memberof(struct winbindd_domain *domain, TALLOC_CTX *mem_ctx, - const char *user_dn, + const char *user_dn, DOM_SID *primary_group, - size_t *p_num_groups, DOM_SID **user_sids) + size_t *p_num_groups, + DOM_SID **user_sids) { ADS_STATUS rc; NTSTATUS status = NT_STATUS_UNSUCCESSFUL; @@ -652,15 +653,15 @@ static NTSTATUS lookup_usergroups_memberof(struct winbindd_domain *domain, size_t num_groups = 0; DOM_SID *group_sids = NULL; int i; - char **strings; - size_t num_strings = 0; + char **strings = NULL; + size_t num_strings = 0, num_sids = 0; DEBUG(3,("ads: lookup_usergroups_memberof\n")); if ( !winbindd_can_contact_domain( domain ) ) { - DEBUG(10,("lookup_usergroups_memberof: No incoming trust for domain %s\n", - domain->name)); + DEBUG(10,("lookup_usergroups_memberof: No incoming trust for " + "domain %s\n", domain->name)); return NT_STATUS_OK; } @@ -668,19 +669,19 @@ static NTSTATUS lookup_usergroups_memberof(struct winbindd_domain *domain, if (!ads) { domain->last_status = NT_STATUS_SERVER_DISABLED; - goto done; + return NT_STATUS_UNSUCCESSFUL; } - rc = ads_search_retry_extended_dn_ranged(ads, mem_ctx, user_dn, attrs, - ADS_EXTENDED_DN_HEX_STRING, + rc = ads_search_retry_extended_dn_ranged(ads, mem_ctx, user_dn, attrs, + ADS_EXTENDED_DN_HEX_STRING, &strings, &num_strings); if (!ADS_ERR_OK(rc)) { - DEBUG(1,("lookup_usergroups_memberof ads_search member=%s: %s\n", - user_dn, ads_errstr(rc))); + DEBUG(1,("lookup_usergroups_memberof ads_search " + "member=%s: %s\n", user_dn, ads_errstr(rc))); return ads_ntstatus(rc); } - + *user_sids = NULL; num_groups = 0; @@ -693,21 +694,26 @@ static NTSTATUS lookup_usergroups_memberof(struct winbindd_domain *domain, group_sids = TALLOC_ZERO_ARRAY(mem_ctx, DOM_SID, num_strings + 1); if (!group_sids) { - TALLOC_FREE(strings); status = NT_STATUS_NO_MEMORY; goto done; } for (i=0; i<num_strings; i++) { - - if (!ads_get_sid_from_extended_dn(mem_ctx, strings[i], - ADS_EXTENDED_DN_HEX_STRING, - &(group_sids)[i])) { - TALLOC_FREE(group_sids); - TALLOC_FREE(strings); - status = NT_STATUS_NO_MEMORY; - goto done; + rc = ads_get_sid_from_extended_dn(mem_ctx, strings[i], + ADS_EXTENDED_DN_HEX_STRING, + &(group_sids)[i]); + if (!ADS_ERR_OK(rc)) { + /* ignore members without SIDs */ + if (NT_STATUS_EQUAL(ads_ntstatus(rc), + NT_STATUS_NOT_FOUND)) { + continue; + } + else { + status = ads_ntstatus(rc); + goto done; + } } + num_sids++; } if (i == 0) { @@ -716,7 +722,7 @@ static NTSTATUS lookup_usergroups_memberof(struct winbindd_domain *domain, goto done; } - for (i=0; i<num_strings; i++) { + for (i=0; i<num_sids; i++) { /* ignore Builtin groups from ADS - Guenther */ if (sid_check_is_in_builtin(&group_sids[i])) { @@ -728,14 +734,17 @@ static NTSTATUS lookup_usergroups_memberof(struct winbindd_domain *domain, if (!NT_STATUS_IS_OK(status)) { goto done; } - + } *p_num_groups = num_groups; status = (*user_sids != NULL) ? NT_STATUS_OK : NT_STATUS_NO_MEMORY; - DEBUG(3,("ads lookup_usergroups (memberof) succeeded for dn=%s\n", user_dn)); + DEBUG(3,("ads lookup_usergroups (memberof) succeeded for dn=%s\n", + user_dn)); + done: + TALLOC_FREE(strings); TALLOC_FREE(group_sids); return status; @@ -899,8 +908,8 @@ done: */ static NTSTATUS lookup_groupmem(struct winbindd_domain *domain, TALLOC_CTX *mem_ctx, - const DOM_SID *group_sid, uint32 *num_names, - DOM_SID **sid_mem, char ***names, + const DOM_SID *group_sid, uint32 *num_names, + DOM_SID **sid_mem, char ***names, uint32 **name_types) { ADS_STATUS rc; @@ -921,7 +930,7 @@ static NTSTATUS lookup_groupmem(struct winbindd_domain *domain, uint32 num_nocache = 0; TALLOC_CTX *tmp_ctx = NULL; - DEBUG(10,("ads: lookup_groupmem %s sid=%s\n", domain->name, + DEBUG(10,("ads: lookup_groupmem %s sid=%s\n", domain->name, sid_string_dbg(group_sid))); *num_names = 0; @@ -935,12 +944,12 @@ static NTSTATUS lookup_groupmem(struct winbindd_domain *domain, if ( !winbindd_can_contact_domain( domain ) ) { DEBUG(10,("lookup_groupmem: No incoming trust for domain %s\n", - domain->name)); + domain->name)); return NT_STATUS_OK; } ads = ads_cached_connection(domain); - + if (!ads) { domain->last_status = NT_STATUS_SERVER_DISABLED; goto done; @@ -952,8 +961,8 @@ static NTSTATUS lookup_groupmem(struct winbindd_domain *domain, } /* search for all members of the group */ - if (!(ldap_exp = talloc_asprintf(tmp_ctx, "(objectSid=%s)", - sidbinstr))) + if (!(ldap_exp = talloc_asprintf(tmp_ctx, "(objectSid=%s)", + sidbinstr))) { SAFE_FREE(sidbinstr); DEBUG(1, ("ads: lookup_groupmem: talloc_asprintf for ldap_exp failed!\n")); @@ -966,21 +975,21 @@ static NTSTATUS lookup_groupmem(struct winbindd_domain *domain, args.val = ADS_EXTENDED_DN_HEX_STRING; args.critical = True; - rc = ads_ranged_search(ads, tmp_ctx, LDAP_SCOPE_SUBTREE, ads->config.bind_path, + rc = ads_ranged_search(ads, tmp_ctx, LDAP_SCOPE_SUBTREE, ads->config.bind_path, ldap_exp, &args, "member", &members, &num_members); if (!ADS_ERR_OK(rc)) { DEBUG(0,("ads_ranged_search failed with: %s\n", ads_errstr(rc))); status = NT_STATUS_UNSUCCESSFUL; goto done; - } - + } + DEBUG(10, ("ads lookup_groupmem: got %d sids via extended dn call\n", (int)num_members)); - + /* Now that we have a list of sids, we need to get the * lists of names and name_types belonging to these sids. - * even though conceptually not quite clean, we use the - * RPC call lsa_lookup_sids for this since it can handle a + * even though conceptually not quite clean, we use the + * RPC call lsa_lookup_sids for this since it can handle a * list of sids. ldap calls can just resolve one sid at a time. * * At this stage, the sids are still hidden in the exetended dn @@ -988,7 +997,7 @@ static NTSTATUS lookup_groupmem(struct winbindd_domain *domain, * stated above: In extracting the sids from the member strings, * we try to resolve as many sids as possible from the * cache. Only the rest is passed to the lsa_lookup_sids call. */ - + if (num_members) { (*sid_mem) = TALLOC_ZERO_ARRAY(mem_ctx, DOM_SID, num_members); (*names) = TALLOC_ZERO_ARRAY(mem_ctx, char *, num_members); @@ -1015,11 +1024,23 @@ static NTSTATUS lookup_groupmem(struct winbindd_domain *domain, char *name, *domain_name; DOM_SID sid; - if (!ads_get_sid_from_extended_dn(tmp_ctx, members[i], args.val, &sid)) { - status = NT_STATUS_INVALID_PARAMETER; - goto done; + rc = ads_get_sid_from_extended_dn(tmp_ctx, members[i], args.val, + &sid); + if (!ADS_ERR_OK(rc)) { + if (NT_STATUS_EQUAL(ads_ntstatus(rc), + NT_STATUS_NOT_FOUND)) { + /* Group members can be objects, like Exchange + * Public Folders, that don't have a SID. Skip + * them. */ + continue; + } + else { + status = ads_ntstatus(rc); + goto done; + } } - if (lookup_cached_sid(mem_ctx, &sid, &domain_name, &name, &name_type)) { + if (lookup_cached_sid(mem_ctx, &sid, &domain_name, &name, + &name_type)) { DEBUG(10,("ads: lookup_groupmem: got sid %s from " "cache\n", sid_string_dbg(&sid))); sid_copy(&(*sid_mem)[*num_names], &sid); @@ -1052,23 +1073,23 @@ static NTSTATUS lookup_groupmem(struct winbindd_domain *domain, goto done; } - status = rpccli_lsa_lookup_sids(cli, tmp_ctx, + status = rpccli_lsa_lookup_sids(cli, tmp_ctx, &lsa_policy, - num_nocache, - sid_mem_nocache, - &domains_nocache, - &names_nocache, + num_nocache, + sid_mem_nocache, + &domains_nocache, + &names_nocache, &name_types_nocache); if (NT_STATUS_IS_OK(status) || - NT_STATUS_EQUAL(status, STATUS_SOME_UNMAPPED)) + NT_STATUS_EQUAL(status, STATUS_SOME_UNMAPPED)) { - /* Copy the entries over from the "_nocache" arrays - * to the result arrays, skipping the gaps the + /* Copy the entries over from the "_nocache" arrays + * to the result arrays, skipping the gaps the * lookup_sids call left. */ for (i=0; i < num_nocache; i++) { - if (((names_nocache)[i] != NULL) && - ((name_types_nocache)[i] != SID_NAME_UNKNOWN)) + if (((names_nocache)[i] != NULL) && + ((name_types_nocache)[i] != SID_NAME_UNKNOWN)) { sid_copy(&(*sid_mem)[*num_names], &sid_mem_nocache[i]); diff --git a/source3/winbindd/winbindd_group.c b/source3/winbindd/winbindd_group.c index 8e56138..3422fdb 100644 --- a/source3/winbindd/winbindd_group.c +++ b/source3/winbindd/winbindd_group.c @@ -585,7 +585,7 @@ static bool fill_grent_mem(struct winbindd_domain *domain, } /* Real work goes here. Create a list of group names to - expand startign with the initial one. Pass that to + expand starting with the initial one. Pass that to expand_groups() which returns a list of more group names to expand. Do this up to the max search depth. */ @@ -922,7 +922,7 @@ static void getgrsid_lookupsid_recv( void *private_data, bool success, nt_status = normalize_name_unmap(s->state->mem_ctx, raw_name, &mapped_name); - /* basiuc whitespace reversal */ + /* basic whitespace reversal */ if (NT_STATUS_IS_OK(nt_status)) { s->group_name = talloc_asprintf(s->state->mem_ctx, "%s%c%s", -- Samba Shared Repository