[PR PATCH] idmap_rid: default group always set to "Domain Users"
Volker Lendecke
Volker.Lendecke at SerNet.DE
Thu Apr 19 19:05:07 UTC 2018
Hi!
I abstain from commenting on this.
Volker
On Thu, Apr 19, 2018 at 07:24:14PM +0200, Samuel Cabrero wrote:
> On Fri, 2018-04-13 at 16:59 +0200, Volker Lendecke via samba-technical
> wrote:
> > On Thu, Apr 12, 2018 at 09:47:55PM +0300, Uri Simchoni via samba-
> > technical wrote:
> > > The thing I'm less certain about is the "somehow". I'd guess an RPC
> > > to
> > > the DC would do it correctly irrespective of the winbindd backend,
> > > but I
> > > could be missing something here. In the original code we had a
> > > _wbint_QueryUser to deal with that on a per-backend basis, and it
> > > was
> > > removed in the series of commits that ended in
> > > 319d60285c92bbf86bc0a3f872f9c9f9d0530129. I'm not sure we really
> > > need
> > > this per-backend behavior though - all AD DC's support RPC, and the
> > > ad
> > > backend already does lots of RPC, it's far from pure ldap (and
> > > rightly so).
> >
> > wbint_QueryUser would have to use samr. This can at best (if at all)
> > with the domain we're member of. And even that is something we need
> > to
> > get rid of. Without a samlogon cache entry there is just no reliable
> > way to get that done. The only way out is (I believe) a s4u2self
> > client, something which is in the works somewhere.
> >
> > Volker
> >
>
> Hi,
>
> what do you think about this patch? It calls SAMR QueryUserInfo only if
> there is no samlogon cache entry and if the call succeed extracts the
> primary group SID, the account name and the full name. Then let the
> idmap backend override it (GetNssInfo call, only idmap_ad will do).
>
> Cheers
>
> From 9187c170a8ebe26502f6e3c40a4161e53c3926ac Mon Sep 17 00:00:00 2001
> From: Samuel Cabrero <scabrero at suse.de>
> Date: Fri, 13 Apr 2018 16:20:16 +0200
> Subject: [PATCH 1/3] Revert "winbind: Remove wbint_QueryUser"
>
> This reverts commit 5b2d74bd1116ef182b4a2a58cb8949ae8f10638f.
>
> Signed-off-by: Samuel Cabrero <scabrero at suse.de>
> ---
> librpc/idl/winbind.idl | 5 +++++
> source3/winbindd/winbindd_dual_srv.c | 15 +++++++++++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/librpc/idl/winbind.idl b/librpc/idl/winbind.idl
> index f5e3507bff5..60af90517e1 100644
> --- a/librpc/idl/winbind.idl
> +++ b/librpc/idl/winbind.idl
> @@ -85,6 +85,11 @@ interface winbind
> dom_sid group_sid;
> } wbint_userinfo;
>
> + NTSTATUS wbint_QueryUser(
> + [in] dom_sid *sid,
> + [out] wbint_userinfo *info
> + );
> +
> NTSTATUS wbint_GetNssInfo(
> [in,out] wbint_userinfo *info
> );
> diff --git a/source3/winbindd/winbindd_dual_srv.c b/source3/winbindd/winbindd_dual_srv.c
> index 8cb05f06db6..57e79d1e381 100644
> --- a/source3/winbindd/winbindd_dual_srv.c
> +++ b/source3/winbindd/winbindd_dual_srv.c
> @@ -299,6 +299,21 @@ NTSTATUS _wbint_AllocateGid(struct pipes_struct *p, struct wbint_AllocateGid *r)
> return NT_STATUS_OK;
> }
>
> +NTSTATUS _wbint_QueryUser(struct pipes_struct *p, struct wbint_QueryUser *r)
> +{
> + struct winbindd_domain *domain = wb_child_domain();
> + NTSTATUS status;
> +
> + if (domain == NULL) {
> + return NT_STATUS_REQUEST_NOT_ACCEPTED;
> + }
> +
> + status = wb_cache_query_user(domain, p->mem_ctx, r->in.sid,
> + r->out.info);
> + reset_cm_connection_on_error(domain, status);
> + return status;
> +}
> +
> NTSTATUS _wbint_GetNssInfo(struct pipes_struct *p, struct wbint_GetNssInfo *r)
> {
> struct idmap_domain *domain;
> --
> 2.16.3
>
>
> From 1df6e3e4257de39446f27e54cc4d8ad51e2bb7c1 Mon Sep 17 00:00:00 2001
> From: Samuel Cabrero <scabrero at suse.de>
> Date: Fri, 13 Apr 2018 16:42:42 +0200
> Subject: [PATCH 2/3] Revert "winbind: Remove rpc_query_user"
>
> This reverts commit a8ab48ee193f68217e7c53b71bf6c57d2d15f8d7.
>
> Signed-off-by: Samuel Cabrero <scabrero at suse.de>
> ---
> source3/winbindd/winbindd_rpc.c | 76 +++++++++++++++++++++++++++++++++++++++++
> source3/winbindd/winbindd_rpc.h | 8 +++++
> 2 files changed, 84 insertions(+)
>
> diff --git a/source3/winbindd/winbindd_rpc.c b/source3/winbindd/winbindd_rpc.c
> index 3a7bf7f7c7f..837c1e1ccf0 100644
> --- a/source3/winbindd/winbindd_rpc.c
> +++ b/source3/winbindd/winbindd_rpc.c
> @@ -440,6 +440,82 @@ NTSTATUS rpc_rids_to_names(TALLOC_CTX *mem_ctx,
> return NT_STATUS_OK;
> }
>
> +/* Lookup user information from a rid or username. */
> +NTSTATUS rpc_query_user(TALLOC_CTX *mem_ctx,
> + struct rpc_pipe_client *samr_pipe,
> + struct policy_handle *samr_policy,
> + const struct dom_sid *domain_sid,
> + const struct dom_sid *user_sid,
> + struct wbint_userinfo *user_info)
> +{
> + struct policy_handle user_policy;
> + union samr_UserInfo *info = NULL;
> + uint32_t user_rid;
> + NTSTATUS status, result;
> + struct dcerpc_binding_handle *b = samr_pipe->binding_handle;
> +
> + if (!sid_peek_check_rid(domain_sid, user_sid, &user_rid)) {
> + return NT_STATUS_UNSUCCESSFUL;
> + }
> +
> + /* Get user handle */
> + status = dcerpc_samr_OpenUser(b,
> + mem_ctx,
> + samr_policy,
> + SEC_FLAG_MAXIMUM_ALLOWED,
> + user_rid,
> + &user_policy,
> + &result);
> + if (!NT_STATUS_IS_OK(status)) {
> + return status;
> + }
> + if (!NT_STATUS_IS_OK(result)) {
> + return result;
> + }
> +
> + /* Get user info */
> + status = dcerpc_samr_QueryUserInfo(b,
> + mem_ctx,
> + &user_policy,
> + 0x15,
> + &info,
> + &result);
> + {
> + NTSTATUS _result;
> + dcerpc_samr_Close(b, mem_ctx, &user_policy, &_result);
> + }
> + if (!NT_STATUS_IS_OK(status)) {
> + return status;
> + }
> + if (!NT_STATUS_IS_OK(result)) {
> + return result;
> + }
> +
> + sid_compose(&user_info->user_sid, domain_sid, user_rid);
> + sid_compose(&user_info->group_sid, domain_sid,
> + info->info21.primary_gid);
> +
> + user_info->acct_name = talloc_strdup(user_info,
> + info->info21.account_name.string);
> + if (user_info->acct_name == NULL) {
> + return NT_STATUS_NO_MEMORY;
> + }
> +
> + user_info->full_name = talloc_strdup(user_info,
> + info->info21.full_name.string);
> + if ((info->info21.full_name.string != NULL) &&
> + (user_info->full_name == NULL))
> + {
> + return NT_STATUS_NO_MEMORY;
> + }
> +
> + user_info->homedir = NULL;
> + user_info->shell = NULL;
> + user_info->primary_gid = (gid_t)-1;
> +
> + return NT_STATUS_OK;
> +}
> +
> /* Lookup groups a user is a member of. */
> NTSTATUS rpc_lookup_usergroups(TALLOC_CTX *mem_ctx,
> struct rpc_pipe_client *samr_pipe,
> diff --git a/source3/winbindd/winbindd_rpc.h b/source3/winbindd/winbindd_rpc.h
> index dcde7a98d8c..955fd11cf54 100644
> --- a/source3/winbindd/winbindd_rpc.h
> +++ b/source3/winbindd/winbindd_rpc.h
> @@ -75,6 +75,14 @@ NTSTATUS rpc_rids_to_names(TALLOC_CTX *mem_ctx,
> char ***pnames,
> enum lsa_SidType **ptypes);
>
> +/* Lookup user information from a rid or username. */
> +NTSTATUS rpc_query_user(TALLOC_CTX *mem_ctx,
> + struct rpc_pipe_client *samr_pipe,
> + struct policy_handle *samr_policy,
> + const struct dom_sid *domain_sid,
> + const struct dom_sid *user_sid,
> + struct wbint_userinfo *user_info);
> +
> /* Lookup groups a user is a member of. */
> NTSTATUS rpc_lookup_usergroups(TALLOC_CTX *mem_ctx,
> struct rpc_pipe_client *samr_pipe,
> --
> 2.16.3
>
>
> From 0490ca7098ead823bb6a0a673327b2ff21ef63ed Mon Sep 17 00:00:00 2001
> From: Samuel Cabrero <scabrero at suse.de>
> Date: Mon, 16 Apr 2018 10:33:17 +0200
> Subject: [PATCH 3/3] s3: winbind: Call SAMR QueryUserInfo if no valid netlogon
> cache entry
>
> When there isn't a valid netlogon cache entry wb_queryuser defaults the
> user primary group sid to -513 (Domain users), and will become correct once
> there is a valid netlogon cache entry.
>
> In an effort to return as accurate as possible information, try to obtain
> the missing fields calling SAMR QueryUserInfo when it cannot be extracted
> from netlogon cache.
>
> Signed-off-by: Samuel Cabrero <scabrero at suse.de>
> ---
> source3/winbindd/wb_queryuser.c | 135 ++++++++++++++++++++++++++++++++---
> source3/winbindd/winbindd_dual_srv.c | 19 ++++-
> 2 files changed, 140 insertions(+), 14 deletions(-)
>
> diff --git a/source3/winbindd/wb_queryuser.c b/source3/winbindd/wb_queryuser.c
> index 17170c3352a..fd50a251e24 100644
> --- a/source3/winbindd/wb_queryuser.c
> +++ b/source3/winbindd/wb_queryuser.c
> @@ -25,13 +25,18 @@
>
> struct wb_queryuser_state {
> struct tevent_context *ev;
> - struct wbint_userinfo *info;
> bool tried_dclookup;
> + bool cached_samlogon;
> + /* Returned to caller */
> + struct wbint_userinfo *info;
> + /* Returned from SAMR QueryUserInfo */
> + struct wbint_userinfo *samr_info;
> };
>
> static void wb_queryuser_got_uid(struct tevent_req *subreq);
> static void wb_queryuser_got_domain(struct tevent_req *subreq);
> static void wb_queryuser_got_dc(struct tevent_req *subreq);
> +static void wb_queryuser_got_nssinfo(struct tevent_req *subreq);
> static void wb_queryuser_got_gid(struct tevent_req *subreq);
> static void wb_queryuser_got_group_name(struct tevent_req *subreq);
> static void wb_queryuser_done(struct tevent_req *subreq);
> @@ -77,8 +82,8 @@ static void wb_queryuser_got_uid(struct tevent_req *subreq)
> req, struct wb_queryuser_state);
> struct wbint_userinfo *info = state->info;
> struct netr_SamInfo3 *info3;
> - struct winbindd_child *child;
> struct unixid xid;
> + struct winbindd_child *child;
> NTSTATUS status;
>
> status = wb_sids2xids_recv(subreq, &xid, 1);
> @@ -115,7 +120,6 @@ static void wb_queryuser_got_uid(struct tevent_req *subreq)
>
> info3 = netsamlogon_cache_get(state, &info->user_sid);
> if (info3 != NULL) {
> -
> sid_compose(&info->group_sid, info3->base.domain_sid,
> info3->base.primary_gid);
> info->acct_name = talloc_move(
> @@ -127,6 +131,7 @@ static void wb_queryuser_got_uid(struct tevent_req *subreq)
> state, &info3->base.logon_domain.string);
>
> TALLOC_FREE(info3);
> + state->cached_samlogon = true;
> }
>
> if (info->domain_name == NULL) {
> @@ -138,6 +143,38 @@ static void wb_queryuser_got_uid(struct tevent_req *subreq)
> return;
> }
>
> + if (!state->cached_samlogon) {
> + struct winbindd_domain *domain;
> +
> + /* If not found in cache, call SAMR QueryUserInfo */
> + domain = find_lookup_domain_from_name(info->domain_name);
> + if (domain == NULL) {
> + DEBUG(5, ("Could not find domain for %s\n",
> + info->domain_name));
> + tevent_req_nterror(req, NT_STATUS_NONE_MAPPED);
> + return;
> + }
> +
> + state->samr_info = talloc_zero(state, struct wbint_userinfo);
> + if (tevent_req_nomem(state->samr_info, req)) {
> + return;
> + }
> +
> + subreq = dcerpc_wbint_QueryUser_send(
> + state, state->ev,
> + dom_child_handle(domain),
> + &info->user_sid, state->samr_info);
> + if (tevent_req_nomem(subreq, req)) {
> + return;
> + }
> + tevent_req_set_callback(subreq, wb_queryuser_done, req);
> + return;
> + }
> +
> + /* The user information should be correct at this point, even from
> + * the netlogon cache or retrived calling SAMR QueryUserInfo. Now
> + * let the idmap backend override the info. */
> +
> child = idmap_child();
>
> subreq = dcerpc_wbint_GetNssInfo_send(
> @@ -145,7 +182,7 @@ static void wb_queryuser_got_uid(struct tevent_req *subreq)
> if (tevent_req_nomem(subreq, req)) {
> return;
> }
> - tevent_req_set_callback(subreq, wb_queryuser_done, req);
> + tevent_req_set_callback(subreq, wb_queryuser_got_nssinfo, req);
> }
>
> static void wb_queryuser_got_domain(struct tevent_req *subreq)
> @@ -172,6 +209,37 @@ static void wb_queryuser_got_domain(struct tevent_req *subreq)
> return;
> }
>
> + if (!state->cached_samlogon) {
> + struct winbindd_domain *domain;
> +
> + /* If not found in cache, call SAMR QueryUserInfo */
> + domain = find_domain_from_name(info->domain_name);
> + if (domain == NULL) {
> + DEBUG(5, ("Could not find domain for name %s\n",
> + info->domain_name));
> + tevent_req_nterror(req, NT_STATUS_NONE_MAPPED);
> + return;
> + }
> +
> + state->samr_info = talloc_zero(state, struct wbint_userinfo);
> + if (tevent_req_nomem(state->samr_info, req)) {
> + return;
> + }
> +
> + subreq = dcerpc_wbint_QueryUser_send(
> + state, state->ev,
> + dom_child_handle(domain),
> + &info->user_sid, state->samr_info);
> + if (tevent_req_nomem(subreq, req)) {
> + return;
> + }
> + tevent_req_set_callback(subreq, wb_queryuser_done, req);
> + return;
> + }
> +
> + /* The user information should be correct at this point from
> + * the netlogon cache, let the idmap backend override the info. */
> +
> child = idmap_child();
>
> subreq = dcerpc_wbint_GetNssInfo_send(
> @@ -179,10 +247,51 @@ static void wb_queryuser_got_domain(struct tevent_req *subreq)
> if (tevent_req_nomem(subreq, req)) {
> return;
> }
> - tevent_req_set_callback(subreq, wb_queryuser_done, req);
> + tevent_req_set_callback(subreq, wb_queryuser_got_nssinfo, req);
> }
>
> static void wb_queryuser_done(struct tevent_req *subreq)
> +{
> + struct tevent_req *req = tevent_req_callback_data(
> + subreq, struct tevent_req);
> + struct wb_queryuser_state *state = tevent_req_data(
> + req, struct wb_queryuser_state);
> + struct winbindd_child *child;
> + NTSTATUS status, result;
> +
> + status = dcerpc_wbint_QueryUser_recv(subreq, state, &result);
> + TALLOC_FREE(subreq);
> +
> + if (tevent_req_nterror(req, status)) {
> + return;
> + }
> +
> + /* Ignore errors and let the idmap backend obtain the info. Otherwise
> + * the defaults will be returned */
> + if (NT_STATUS_IS_OK(result)) {
> + sid_copy(&state->info->group_sid,
> + &state->samr_info->group_sid);
> + state->info->acct_name = talloc_strdup(
> + state->info, state->samr_info->acct_name);
> + state->info->full_name = talloc_strdup(
> + state->info, state->samr_info->full_name);
> + }
> +
> + /* The user information should be correct at this point, retrived
> + * calling SAMR QueryUserInfo. Now let the idmap backend override
> + * the info. */
> +
> + child = idmap_child();
> +
> + subreq = dcerpc_wbint_GetNssInfo_send(
> + state, state->ev, child->binding_handle, state->info);
> + if (tevent_req_nomem(subreq, req)) {
> + return;
> + }
> + tevent_req_set_callback(subreq, wb_queryuser_got_nssinfo, req);
> +}
> +
> +static void wb_queryuser_got_nssinfo(struct tevent_req *subreq)
> {
> struct tevent_req *req = tevent_req_callback_data(
> subreq, struct tevent_req);
> @@ -209,13 +318,17 @@ static void wb_queryuser_done(struct tevent_req *subreq)
> }
> tevent_req_set_callback(subreq, wb_queryuser_got_dc, req);
> return;
> + } else if (!NT_STATUS_EQUAL(result, NT_STATUS_OK) &&
> + !NT_STATUS_EQUAL(result, NT_STATUS_REQUEST_NOT_ACCEPTED))
> + {
> + /* If the idmap backend returns NT_STATUS_REQUEST_NOT_ACCEPTED
> + * ignore, means it does not override the current values */
> + tevent_req_nterror(req, result);
> + return;
> }
>
> - /*
> - * Ignore failure in "result" here. We'll try to fill in stuff
> - * that misses further down.
> - */
> -
> + /* info->group_sid should be correct, even from the netlogon cache
> + * or obtained calling SAMR QueryUser */
> if (state->info->primary_gid == (gid_t)-1) {
> subreq = wb_sids2xids_send(
> state, state->ev, &info->group_sid, 1);
> @@ -279,7 +392,7 @@ static void wb_queryuser_got_dc(struct tevent_req *subreq)
> if (tevent_req_nomem(subreq, req)) {
> return;
> }
> - tevent_req_set_callback(subreq, wb_queryuser_done, req);
> + tevent_req_set_callback(subreq, wb_queryuser_got_nssinfo, req);
> }
>
> static void wb_queryuser_got_gid(struct tevent_req *subreq)
> diff --git a/source3/winbindd/winbindd_dual_srv.c b/source3/winbindd/winbindd_dual_srv.c
> index 57e79d1e381..0aca4dce445 100644
> --- a/source3/winbindd/winbindd_dual_srv.c
> +++ b/source3/winbindd/winbindd_dual_srv.c
> @@ -23,6 +23,7 @@
> #include "includes.h"
> #include "winbindd/winbindd.h"
> #include "winbindd/winbindd_proto.h"
> +#include "winbindd/winbindd_rpc.h"
> #include "rpc_client/cli_pipe.h"
> #include "ntdomain.h"
> #include "librpc/gen_ndr/srv_winbind.h"
> @@ -302,15 +303,27 @@ NTSTATUS _wbint_AllocateGid(struct pipes_struct *p, struct wbint_AllocateGid *r)
> NTSTATUS _wbint_QueryUser(struct pipes_struct *p, struct wbint_QueryUser *r)
> {
> struct winbindd_domain *domain = wb_child_domain();
> + struct rpc_pipe_client *samr_pipe;
> + struct policy_handle samr_domain_handle;
> NTSTATUS status;
>
> if (domain == NULL) {
> return NT_STATUS_REQUEST_NOT_ACCEPTED;
> }
>
> - status = wb_cache_query_user(domain, p->mem_ctx, r->in.sid,
> - r->out.info);
> - reset_cm_connection_on_error(domain, status);
> + status = cm_connect_sam(domain, p->mem_ctx, false,
> + &samr_pipe, &samr_domain_handle);
> + if (!NT_STATUS_IS_OK(status)) {
> + DEBUG(3, ("could not open handle to SAMR pipe: %s\n",
> + nt_errstr(status)));
> + return status;
> + }
> +
> + status = rpc_query_user(p->mem_ctx, samr_pipe,
> + &samr_domain_handle,
> + &domain->sid, r->in.sid,
> + r->out.info);
> +
> return status;
> }
>
> --
> 2.16.3
>
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
More information about the samba-technical
mailing list