On Wed, Oct 07, 2015 at 04:22:31PM +0200, Sumit Bose wrote:
> On Wed, Oct 07, 2015 at 02:42:18PM +0200, Pavel Březina wrote:
> > On 10/06/2015 03:16 PM, Pavel Březina wrote:
> > >On 10/01/2015 05:06 PM, Sumit Bose wrote:
> > >>On Thu, Sep 17, 2015 at 11:06:30AM +0200, Pavel Březina wrote:
> > >>>On 09/17/2015 10:32 AM, Petr Cech wrote:
> > >>>>Hi Pavel!
> > >>>>
> > >>>>There is some code between my last end and this continuation. I was
> > >>>>read
> > >>>>it and did't find anything wrong.
> > >>>
> > >>>Hi Petr,
> > >>>thank you for you review. New patches are attached. All comments from
> > >>>the
> > >>>previous mail should be addressed.
> > >>>
> > >>
> > >>
> > >>[PATCH 2/3] cache_req: add support for UPN
> > >>
> > >>>diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
> > >>>index
> > >>>5f33b225a31e1c1add7f776215d500fb09e127ca..a72a8bac594dc10ff9aa337399f27d9936830a8e
> > >>>100644
> > >>>--- a/src/db/sysdb_search.c
> > >>>+++ b/src/db/sysdb_search.c
> > >>>@@ -295,6 +295,59 @@ static char *enum_filter(TALLOC_CTX *mem_ctx,
> > >>> return filter;
> > >>> }
> > >>>
> > >>>+int sysdb_getpwupn(TALLOC_CTX *mem_ctx,
> > >>>+ struct sss_domain_info *domain,
> > >>>+ const char *upn,
> > >>>+ struct ldb_result **_res)
> > >>>+{
> > >>>+ TALLOC_CTX *tmp_ctx;
> > >>>+ struct ldb_message *msg;
> > >>>+ struct ldb_result *res;
> > >>>+ static const char *attrs[] = SYSDB_PW_ATTRS;
> > >>>+ errno_t ret;
> > >>>+
> > >>>+ tmp_ctx = talloc_new(NULL);
> > >>>+ if (tmp_ctx == NULL) {
> > >>>+ DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
> > >>>+ return ENOMEM;
> > >>>+ }
> > >>>+
> > >>>+ ret = sysdb_search_user_by_upn(tmp_ctx, domain, upn, attrs, &msg);
> > >>>+ if (ret != EOK && ret != ENOENT) {
> > >>>+ DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_user_by_upn()
> > >>>failed.\n");
> > >>>+ goto done;
> > >>>+ }
> > >>
> > >>The call stack here would be
> > >>
> > >>sysdb_getpwupn()->sysdb_search_user_by_upn()->sysdb_search_entry()->ldb_search()
> > >>
> > >>
> > >>and the returned results, starting from ldb_search()
> > >>
> > >>ldb_result -> ldb_message (array) -> ldb_message -> ldb_result
> > >>
> > >>I think it would be easier and safe some allocations if it would be
> > >>
> > >>sysdb_search_user_by_upn()->sysdb_getpwupn()->ldb_search() .
> > >>
> > >>Do you think this change makes sense?
> > >
> > >Hi, thanks for the review. Done.
> > >
> > >Although I created a new function sysdb_search_user_by_upn_res since you
> > >can't pass attrs into sysdb_getpwnupn (unless we are inconsistent with
> > >api).
> > >
> > >>
> > > const char *orig_name;
> > >>
> > >>Instead of adding a new member which again will hold the originally
> > >>provided name I would suggest to refactor the change added by
> > >>e87b2a6e94c1066b3044fe683825ff5b4f8716c2 (cache_req: parse input name if
> > >>needed) by introducing a member called e.g. parsed_name which keeps the
> > >>name returned by sss_parse_inp_recv() and return it in cache_req_recv().
> > >>
> > >>In e87b2a6e94c1066b3044fe683825ff5b4f8716c2 it made sense to overwrite
> > >>orig_name because no additional member was needed. But now I think the
> > >>code would be more clear if orig_name will be kept unmodified.
> > >
> > >Done in separate patch so the changes are more visible. It can be
> > >squashed to the second patch.
> >
> > Sumit found that test won't pass. New patchset is attached.
> >
> >
>
> Thank you, the tests pass now. I've seen a compiler warning which is
> seen with the first patch I attached.
>
> During testing I found that UPN lookups for sub-domain users do not
> work even with the original nss responder code. I attached fixes for
> your patches and the nss responder since the sysdb change depends on you
> patches.
and now with patches ...
>
> bye,
> Sumit
> _______________________________________________
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
From 617c1a39fab16d12f9a346073eaee4a8bee4ac08 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Wed, 7 Oct 2015 16:11:56 +0200
Subject: [PATCH 1/3] fix ldb_search usage
---
src/db/sysdb_ops.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index
5c086cb0a25b452a38ca717b2459c76fdb3f22ff..fafa0207cd068b22151f805a217e2cd55b357d17
100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -483,7 +483,6 @@ int sysdb_search_user_by_upn_res(TALLOC_CTX *mem_ctx,
TALLOC_CTX *tmp_ctx;
struct ldb_result *res;
struct ldb_dn *base_dn;
- char *filter;
int ret;
const char *def_attrs[] = { SYSDB_NAME, SYSDB_UPN, SYSDB_CANONICAL_UPN,
NULL };
@@ -500,15 +499,9 @@ int sysdb_search_user_by_upn_res(TALLOC_CTX *mem_ctx,
goto done;
}
- filter = talloc_asprintf(tmp_ctx, SYSDB_PWUPN_FILTER, upn, upn);
- if (filter == NULL) {
- ret = ENOMEM;
- goto done;
- }
-
ret = ldb_search(domain->sysdb->ldb, tmp_ctx, &res,
base_dn, LDB_SCOPE_SUBTREE, attrs ? attrs : def_attrs,
- filter);
+ SYSDB_PWUPN_FILTER, upn, upn);
if (ret != EOK) {
ret = sysdb_error_to_errno(ret);
goto done;
--
2.1.0
From 454c0bde9228a48623029094c1c5d0a5ecf03390 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Wed, 7 Oct 2015 16:12:45 +0200
Subject: [PATCH 2/3] fix upn cache_req for sub-domain users
---
src/responder/common/responder_cache_req.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/responder/common/responder_cache_req.c
b/src/responder/common/responder_cache_req.c
index
be7fe5f29c1696bdd9c7d9b7a9ddee8319592938..ab73401b39cf26175af87de67fb16be3d45d3765
100644
--- a/src/responder/common/responder_cache_req.c
+++ b/src/responder/common/responder_cache_req.c
@@ -981,7 +981,8 @@ static errno_t cache_req_next_domain(struct tevent_req *req)
/* If it is a domainless search, skip domains that require fully
* qualified names instead. */
while (state->domain != NULL && state->check_next
- && state->domain->fqnames) {
+ && state->domain->fqnames
+ && !cache_req_input_is_upn(state->input)) {
state->domain = get_next_domain(state->domain, false);
}
@@ -1009,7 +1010,11 @@ static errno_t cache_req_next_domain(struct tevent_req
*req)
/* we will continue with the following domain the next time */
if (state->check_next) {
- state->domain = get_next_domain(state->domain, false);
+ if (cache_req_input_is_upn(state->input)) {
+ state->domain = get_next_domain(state->domain, true);
+ } else {
+ state->domain = get_next_domain(state->domain, false);
+ }
}
return EAGAIN;
--
2.1.0
From 3d5352977ca07f8a1a6d3bbaa57d11b54fd705d0 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Wed, 7 Oct 2015 15:22:34 +0200
Subject: [PATCH 3/3] nss: fix UPN lookups for sub-domain users
---
src/db/sysdb_ops.c | 2 +-
src/responder/nss/nsssrv_cmd.c | 12 ++++++++++--
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index
fafa0207cd068b22151f805a217e2cd55b357d17..f71f4f88122c9cffa9e5b32cf4ae803130f6ee70
100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -493,7 +493,7 @@ int sysdb_search_user_by_upn_res(TALLOC_CTX *mem_ctx,
goto done;
}
- base_dn = sysdb_user_base_dn(tmp_ctx, domain);
+ base_dn = sysdb_base_dn(domain->sysdb, tmp_ctx);
if (base_dn == NULL) {
ret = ENOMEM;
goto done;
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index
3e95a3f5a96a91515745a17fad5d20cbc9063d28..4f151b1a1fcf157d67f141742f2d8a4658e577ab
100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -895,7 +895,11 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx
*dctx)
name, dom->name);
/* if a multidomain search, try with next */
if (cmdctx->check_next) {
- dom = get_next_domain(dom, false);
+ if (cmdctx->name_is_upn) {
+ dom = get_next_domain(dom, true);
+ } else {
+ dom = get_next_domain(dom, false);
+ }
continue;
}
/* There are no further domains or this was a
@@ -970,7 +974,11 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx
*dctx)
/* if a multidomain search, try with next */
if (cmdctx->check_next) {
- dom = get_next_domain(dom, false);
+ if (cmdctx->name_is_upn) {
+ dom = get_next_domain(dom, true);
+ } else {
+ dom = get_next_domain(dom, false);
+ }
if (dom) continue;
}
--
2.1.0
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
