From 678dbe173a2e42164f680821b228f3ec8c823ab9 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 29 Dec 2015 21:33:20 +0000 Subject: [PATCH 1/4] winbind: Simplify _wbint_Sids2UnixIDs Same number of lines, but from my point of view quite a bit simpler now that we only have to handle one domain. Signed-off-by: Volker Lendecke Reviewed-by: Ralph Boehme (cherry picked from commit 4d5680e9ae531c6dc4d0a6687abe6293b5d4f4f2) --- source3/winbindd/winbindd_dual_srv.c | 153 ++++++++++++++++++----------------- 1 file changed, 77 insertions(+), 76 deletions(-) diff --git a/source3/winbindd/winbindd_dual_srv.c b/source3/winbindd/winbindd_dual_srv.c index cdd9bbd..2f4b0e2 100644 --- a/source3/winbindd/winbindd_dual_srv.c +++ b/source3/winbindd/winbindd_dual_srv.c @@ -121,102 +121,103 @@ NTSTATUS _wbint_LookupName(struct pipes_struct *p, struct wbint_LookupName *r) NTSTATUS _wbint_Sids2UnixIDs(struct pipes_struct *p, struct wbint_Sids2UnixIDs *r) { - uint32_t i, j; - struct id_map *ids = NULL; - struct id_map **id_ptrs = NULL; + uint32_t i; + + struct lsa_DomainInfo *d; + struct wbint_TransID *ids; + uint32_t num_ids; + + struct id_map *id_maps = NULL; + struct id_map **id_map_ptrs = NULL; struct dom_sid *sids = NULL; - uint32_t *id_idx = NULL; + struct idmap_domain *dom; NTSTATUS status = NT_STATUS_NO_MEMORY; - for (i=0; iin.domains->count; i++) { - struct lsa_DomainInfo *d = &r->in.domains->domains[i]; - struct idmap_domain *dom; - uint32_t num_ids; + if (r->in.domains->count != 1) { + return NT_STATUS_INVALID_PARAMETER; + } - dom = idmap_find_domain_with_sid(d->name.string, d->sid); - if (dom == NULL) { - DEBUG(10, ("idmap domain %s:%s not found\n", - d->name.string, sid_string_dbg(d->sid))); - continue; - } + d = &r->in.domains->domains[0]; + ids = r->in.ids->ids; + num_ids = r->in.ids->num_ids; - num_ids = 0; + dom = idmap_find_domain_with_sid(d->name.string, d->sid); + if (dom == NULL) { + DEBUG(10, ("idmap domain %s:%s not found\n", + d->name.string, sid_string_dbg(d->sid))); - for (j=0; jin.ids->num_ids; j++) { - if (r->in.ids->ids[j].domain_index == i) { - num_ids += 1; - } - } + for (i=0; iin.ids->num_ids; j++) { - struct wbint_TransID *id = &r->in.ids->ids[j]; + id_maps = talloc_array(talloc_tos(), struct id_map, num_ids); + if (id_maps == NULL) { + goto nomem; + } + id_map_ptrs = talloc_array(talloc_tos(), struct id_map *, num_ids+1); + if (id_map_ptrs == NULL) { + goto nomem; + } + sids = talloc_array(talloc_tos(), struct dom_sid, num_ids); + if (sids == NULL) { + goto nomem; + } - if (id->domain_index != i) { - continue; - } - id_idx[num_ids] = j; - id_ptrs[num_ids] = &ids[num_ids]; - - ids[num_ids].sid = &sids[num_ids]; - sid_compose(ids[num_ids].sid, d->sid, id->rid); - ids[num_ids].xid.type = id->type; - ids[num_ids].status = ID_UNKNOWN; - num_ids += 1; - } - id_ptrs[num_ids] = NULL; + /* + * Convert the input data into a list of id_map structs + * suitable for handing in to the idmap sids_to_unixids + * method. + */ + + for (i=0; isid, ids[i].rid); + + id_maps[i] = (struct id_map) { + .sid = &sids[i], + .xid.type = ids[i].type, + .status = ID_UNKNOWN + }; + + id_map_ptrs[i] = &id_maps[i]; + } + id_map_ptrs[num_ids] = NULL; + + status = dom->methods->sids_to_unixids(dom, id_map_ptrs); - status = dom->methods->sids_to_unixids(dom, id_ptrs); + if (!NT_STATUS_IS_OK(status)) { DEBUG(10, ("sids_to_unixids returned %s\n", nt_errstr(status))); + goto done; + } - /* - * Extract the results for handing them back to the caller. - */ - for (j=0; jin.ids->ids[id_idx[j]]; + /* + * Extract the results for handing them back to the caller. + */ - if (ids[j].status != ID_MAPPED) { - id->xid.id = UINT32_MAX; - id->xid.type = ID_TYPE_NOT_SPECIFIED; - continue; - } + for (i=0; ixid = ids[j].xid; + if (id_maps[i].status == ID_MAPPED) { + ids[i].xid = id_maps[i].xid; + } else { + ids[i].xid.id = UINT32_MAX; + ids[i].xid.type = ID_TYPE_NOT_SPECIFIED; } } - status = NT_STATUS_OK; + + goto done; nomem: - TALLOC_FREE(ids); - TALLOC_FREE(id_ptrs); - TALLOC_FREE(id_idx); + status = NT_STATUS_NO_MEMORY; +done: + TALLOC_FREE(id_maps); + TALLOC_FREE(id_map_ptrs); TALLOC_FREE(sids); return status; } -- 2.9.2 From 0a424d6c4485f2b23cc89454dfcaf73c62c52fe6 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Fri, 4 Mar 2016 14:23:51 +0100 Subject: [PATCH 2/4] winbind: Introduce id_map_ptrs_init This simplifies _wbint_Sids2UnixIDs a bit and will be re-used in _wbint_UnixIDs2Sids Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison (cherry picked from commit 50aef48e18e7e0a265c348d2486f687ddad839a0) --- source3/winbindd/idmap_proto.h | 2 ++ source3/winbindd/idmap_util.c | 31 +++++++++++++++++++++++++++++++ source3/winbindd/winbindd_dual_srv.c | 33 ++++++++------------------------- 3 files changed, 41 insertions(+), 25 deletions(-) diff --git a/source3/winbindd/idmap_proto.h b/source3/winbindd/idmap_proto.h index a12e5b4..0e124cd 100644 --- a/source3/winbindd/idmap_proto.h +++ b/source3/winbindd/idmap_proto.h @@ -59,6 +59,8 @@ struct id_map *idmap_find_map_by_sid(struct id_map **maps, struct dom_sid *sid); char *idmap_fetch_secret(const char *backend, const char *domain, const char *identity); +struct id_map **id_map_ptrs_init(TALLOC_CTX *mem_ctx, size_t num_ids); + /* max number of ids requested per LDAP batch query */ #define IDMAP_LDAP_MAX_IDS 30 diff --git a/source3/winbindd/idmap_util.c b/source3/winbindd/idmap_util.c index f90565f..7591530 100644 --- a/source3/winbindd/idmap_util.c +++ b/source3/winbindd/idmap_util.c @@ -238,3 +238,34 @@ char *idmap_fetch_secret(const char *backend, const char *domain, return ret; } + +struct id_map **id_map_ptrs_init(TALLOC_CTX *mem_ctx, size_t num_ids) +{ + struct id_map **ptrs; + struct id_map *maps; + struct dom_sid *sids; + size_t i; + + ptrs = talloc_array(mem_ctx, struct id_map *, num_ids+1); + if (ptrs == NULL) { + return NULL; + } + maps = talloc_array(ptrs, struct id_map, num_ids); + if (maps == NULL) { + TALLOC_FREE(ptrs); + return NULL; + } + sids = talloc_zero_array(ptrs, struct dom_sid, num_ids); + if (sids == NULL) { + TALLOC_FREE(ptrs); + return NULL; + } + + for (i=0; isid, ids[i].rid); - - id_maps[i] = (struct id_map) { - .sid = &sids[i], - .xid.type = ids[i].type, - .status = ID_UNKNOWN - }; - - id_map_ptrs[i] = &id_maps[i]; + sid_compose(m->sid, d->sid, ids[i].rid); + m->status = ID_UNKNOWN; + m->xid = (struct unixid) { .type = ids[i].type }; } - id_map_ptrs[num_ids] = NULL; status = dom->methods->sids_to_unixids(dom, id_map_ptrs); @@ -203,9 +187,10 @@ NTSTATUS _wbint_Sids2UnixIDs(struct pipes_struct *p, */ for (i=0; istatus == ID_MAPPED) { + ids[i].xid = m->xid; } else { ids[i].xid.id = UINT32_MAX; ids[i].xid.type = ID_TYPE_NOT_SPECIFIED; @@ -216,9 +201,7 @@ NTSTATUS _wbint_Sids2UnixIDs(struct pipes_struct *p, nomem: status = NT_STATUS_NO_MEMORY; done: - TALLOC_FREE(id_maps); TALLOC_FREE(id_map_ptrs); - TALLOC_FREE(sids); return status; } -- 2.9.2 From 79005004c636470e05b5f5bf9ebc83d89cc5f6d2 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Mon, 15 Aug 2016 23:07:33 +0200 Subject: [PATCH 3/4] idmap: don't generally forbid id==0 from idmap_unix_id_is_in_range() If the range allows it, then id==0 should not be forbidden. This seems to have been taken in from idmap_ldap when the function was originally created. See 634cd2e0451d4388c3e3f78239495cf595368b15 . The other backends don't seem to have had that extra check for id == 0. The reasoning for this change is that the range check should apply to all cases. If the range includes the 0, then it should be possible to get it as result. In particular, this way, the function becomes applicable also to the passdb backend case, e.g. in a samba4-ad-dc setup where the Admin gets uid == 0. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12155 Signed-off-by: Michael Adam Reviewed-by: Volker Lendecke Reviewed-by: Andreas Schneider (cherry picked from commit c21976d4b1c604699299f2c0f768c1add93b349d) --- source3/winbindd/idmap_util.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/source3/winbindd/idmap_util.c b/source3/winbindd/idmap_util.c index 7591530..40a5f21 100644 --- a/source3/winbindd/idmap_util.c +++ b/source3/winbindd/idmap_util.c @@ -160,11 +160,6 @@ backend: */ bool idmap_unix_id_is_in_range(uint32_t id, struct idmap_domain *dom) { - if (id == 0) { - /* 0 is not an allowed unix id for id mapping */ - return false; - } - if ((dom->low_id && (id < dom->low_id)) || (dom->high_id && (id > dom->high_id))) { -- 2.9.2 From 7a92cc3e0699989d0d5bb746030e8611f5d48fa7 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Tue, 9 Aug 2016 18:25:12 +0200 Subject: [PATCH 4/4] idmap: centrally check that unix IDs returned by the idmap backends are in range Note: in the long run, it might be good to move this kind of exit check (before handing the result back to the client) to the parent winbindd code. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12155 Signed-off-by: Michael Adam Reviewed-by: Volker Lendecke Reviewed-by: Andreas Schneider Autobuild-User(master): Michael Adam Autobuild-Date(master): Wed Aug 17 01:21:39 CEST 2016 on sn-devel-144 (cherry picked from commit b2bf61307cffd8ff7b6fb9852c107ab763653119) --- source3/winbindd/winbindd_dual_srv.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source3/winbindd/winbindd_dual_srv.c b/source3/winbindd/winbindd_dual_srv.c index 3afdb55..92f80b6 100644 --- a/source3/winbindd/winbindd_dual_srv.c +++ b/source3/winbindd/winbindd_dual_srv.c @@ -189,6 +189,10 @@ NTSTATUS _wbint_Sids2UnixIDs(struct pipes_struct *p, for (i=0; ixid.id, dom)) { + m->status = ID_UNMAPPED; + } + if (m->status == ID_MAPPED) { ids[i].xid = m->xid; } else { -- 2.9.2