I abstain from commenting on this.
Post by Samuel Cabrero via samba-technicalOn Fri, 2018-04-13 at 16:59 +0200, Volker Lendecke via samba-technical
On Thu, Apr 12, 2018 at 09:47:55PM +0300, Uri Simchoni via samba-
Post by Uri Simchoni via samba-technicalThe 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
Date: Fri, 13 Apr 2018 16:20:16 +0200
Subject: [PATCH 1/3] Revert "winbind: Remove wbint_QueryUser"
This reverts commit 5b2d74bd1116ef182b4a2a58cb8949ae8f10638f.
---
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
Date: Fri, 13 Apr 2018 16:42:42 +0200
Subject: [PATCH 2/3] Revert "winbind: Remove rpc_query_user"
This reverts commit a8ab48ee193f68217e7c53b71bf6c57d2d15f8d7.
---
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
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.
---
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