[PATCH] winbindd: error handling in rpc_try_lookup_sids3()
Ralph Böhme
slow at samba.org
Tue Apr 4 10:50:06 UTC 2017
On Mon, Apr 03, 2017 at 11:49:36AM -0700, Jeremy Allison wrote:
> Ralph, trying to push and I'm getting:
>
> [655(3442)/2083 at 1h12s] idmap.alloc(ad_member_rfc2307)
> Domain SAMBADOMAIN has SID S-1-5-21-3583314470-906957706-621714972
> id: 66666: no such user
> failed to call wbcStringToSid: WBC_ERR_INVALID_SID
> Could not lookup sid S-1-5-21-3583314470-906957706-621714972\66666
> Using non-existing SID S-1-5-21-3583314470-906957706-621714972-66666 to check no id allocation is done by the backend
> wbinfo returned: S-1-5-21-3583314470-906957706-621714972-66666 -> uid/gid 1166666
> UNEXPECTED(failure): idmap.alloc.wbinfo SID to xid returns unmapped for unknown SID(ad_member_rfc2307)
> REASON: Exception: Exception:
>
> Can you take a look (sorry).
sorry for the hassle! Was only running make test TESTS=wbinfo on the last
incarnation as I already have full private autobuilds running with additional
sids2xids patches on top.
The good thing is, that this actually found another hidden small bug. Patch 4/5
fixes it.
This one passed a full autobuild (hopefully I ran it with the correct sha...).
Please review & push if ok.
Cheerio!
-slow
-------------- next part --------------
From e41caf31b1cc741cc8fc1eb47bff14982951ccc5 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 1 Apr 2017 16:44:45 +0200
Subject: [PATCH 1/5] s3/include: add NT_STATUS_LOOKUP_ERR
Useful helper macro to check the return value of LSA and SAMR
translations.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12728
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/include/lsa.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/source3/include/lsa.h b/source3/include/lsa.h
index 7681aed..c23e942 100644
--- a/source3/include/lsa.h
+++ b/source3/include/lsa.h
@@ -22,4 +22,8 @@ int init_lsa_ref_domain_list(TALLOC_CTX *mem_ctx,
const char *dom_name,
struct dom_sid *dom_sid);
+#define NT_STATUS_LOOKUP_ERR(status) \
+ (!NT_STATUS_IS_OK(status) && \
+ !NT_STATUS_EQUAL(status, STATUS_SOME_UNMAPPED) && \
+ !NT_STATUS_EQUAL(status, NT_STATUS_NONE_MAPPED))
#endif
--
2.9.3
From b47aac15e26e722eff6c3ad25b9d2e27e410f024 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 1 Apr 2017 16:56:39 +0200
Subject: [PATCH 2/5] s3/rpc_client: use NT_STATUS_LOOKUP_ERR
No change in behaviour.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12728
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/rpc_client/cli_lsarpc.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/source3/rpc_client/cli_lsarpc.c b/source3/rpc_client/cli_lsarpc.c
index 518489a..70952f6 100644
--- a/source3/rpc_client/cli_lsarpc.c
+++ b/source3/rpc_client/cli_lsarpc.c
@@ -256,10 +256,7 @@ static NTSTATUS dcerpc_lsa_lookup_sids_noalloc(struct dcerpc_binding_handle *h,
return status;
}
- if (!NT_STATUS_IS_OK(result) &&
- !NT_STATUS_EQUAL(result, NT_STATUS_NONE_MAPPED) &&
- !NT_STATUS_EQUAL(result, STATUS_SOME_UNMAPPED))
- {
+ if (NT_STATUS_LOOKUP_ERR(result)) {
*presult = result;
return status;
}
--
2.9.3
From 72578b671d798860ee6212d6a7daef7a6652f549 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 1 Apr 2017 16:51:07 +0200
Subject: [PATCH 3/5] s3/rpc_client: lookupsids error handling of
NT_STATUS_NONE_MAPPED
NT_STATUS_NONE_MAPPED is not a fatal error, it just means we must return
all lsa_TranslatedName's as type SID_NAME_UNKNOWN.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12728
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/rpc_client/cli_lsarpc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/source3/rpc_client/cli_lsarpc.c b/source3/rpc_client/cli_lsarpc.c
index 70952f6..41c1ef4 100644
--- a/source3/rpc_client/cli_lsarpc.c
+++ b/source3/rpc_client/cli_lsarpc.c
@@ -28,6 +28,7 @@
#include "rpc_client/cli_lsarpc.h"
#include "rpc_client/init_lsa.h"
#include "../libcli/security/security.h"
+#include "lsa.h"
/** @defgroup lsa LSA - Local Security Architecture
* @ingroup rpc_client
@@ -221,7 +222,7 @@ static NTSTATUS dcerpc_lsa_lookup_sids_noalloc(struct dcerpc_binding_handle *h,
return status;
}
- if(!NT_STATUS_IS_ERR(result)) {
+ if (!NT_STATUS_LOOKUP_ERR(result)) {
lsa_names.count = lsa_names2.count;
lsa_names.names = talloc_array(mem_ctx,
struct lsa_TranslatedName,
--
2.9.3
From dd1b2f8ab3162c2e6e3215142dc819044dfb9e94 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 4 Apr 2017 07:34:39 +0200
Subject: [PATCH 4/5] winbindd: set domain for lookups that returned
SID_NAME_UNKNOWN
SIDs that resulted in SID_NAME_UNKNOWN are resent through
wb_lookupsid(). It's callback wb_lookupsids_single_done() uses the
domain as part of determening the idmap domain.
The above fallback code already sets the domain and for the case
dcerpc_wbint_LookupSids internally resulted in NT_STATUS_NONE_MAPPED the
fallback was triggerd because NT_STATUS_IS_ERR(NT_STATUS_NONE_MAPPED) is
true.
A subsequent commit will modify dcerpc_wbint_LookupSids() to return
NT_STATUS_OK for this case, so fallback logic in wb_lookupsids_done will
not be triggered anymore.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12728
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/winbindd/wb_lookupsids.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/source3/winbindd/wb_lookupsids.c b/source3/winbindd/wb_lookupsids.c
index 858616b..a1df4fb 100644
--- a/source3/winbindd/wb_lookupsids.c
+++ b/source3/winbindd/wb_lookupsids.c
@@ -519,6 +519,7 @@ static void wb_lookupsids_done(struct tevent_req *subreq)
*/
state->single_sids[state->num_single_sids] =
res_sid_index;
+ state->single_domains[state->num_single_sids] = d;
state->num_single_sids += 1;
continue;
}
--
2.9.3
From 93bd15cade29491fd3b4ec84d0a013bf7d9b8e7e Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 26 Mar 2017 08:22:13 +0200
Subject: [PATCH 5/5] winbindd: error handling in rpc_lookup_sids()
NT_STATUS_NONE_MAPPED and NT_STATUS_SOME_NOT_MAPPED should not be
treated as fatal error. We should continue processing the results and
not bail out.
In case we got NT_STATUS_NONE_MAPPED we must have to ensure all
lsa_TranslatedName are of type SID_NAME_UNKNOWN.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12728
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/winbindd/winbindd_rpc.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/source3/winbindd/winbindd_rpc.c b/source3/winbindd/winbindd_rpc.c
index 3dd4f77..0023e2a 100644
--- a/source3/winbindd/winbindd_rpc.c
+++ b/source3/winbindd/winbindd_rpc.c
@@ -32,6 +32,7 @@
#include "rpc_client/cli_samr.h"
#include "rpc_client/cli_lsarpc.h"
#include "../libcli/security/security.h"
+#include "lsa.h"
/* Query display info for a domain */
NTSTATUS rpc_query_user_list(TALLOC_CTX *mem_ctx,
@@ -981,7 +982,7 @@ static NTSTATUS rpc_try_lookup_sids3(TALLOC_CTX *mem_ctx,
if (!NT_STATUS_IS_OK(status)) {
return status;
}
- if (NT_STATUS_IS_ERR(result)) {
+ if (NT_STATUS_LOOKUP_ERR(result)) {
return result;
}
if (sids->num_sids != lsa_names2.count) {
@@ -1010,7 +1011,7 @@ static NTSTATUS rpc_try_lookup_sids3(TALLOC_CTX *mem_ctx,
return NT_STATUS_INVALID_NETWORK_RESPONSE;
}
}
- return result;
+ return NT_STATUS_OK;
}
NTSTATUS rpc_lookup_sids(TALLOC_CTX *mem_ctx,
@@ -1043,7 +1044,7 @@ NTSTATUS rpc_lookup_sids(TALLOC_CTX *mem_ctx,
if (!NT_STATUS_IS_OK(status)) {
return status;
}
- if (NT_STATUS_IS_ERR(result)) {
+ if (NT_STATUS_LOOKUP_ERR(result)) {
return result;
}
@@ -1063,5 +1064,5 @@ NTSTATUS rpc_lookup_sids(TALLOC_CTX *mem_ctx,
}
}
- return result;
+ return NT_STATUS_OK;
}
--
2.9.3
More information about the samba-technical
mailing list