On 10/12/2015 05:43 PM, Sumit Bose wrote:
On Sun, Oct 11, 2015 at 06:10:50PM +0200, Pavel Březina wrote:
On 10/11/2015 06:03 PM, Pavel Březina wrote:
https://fedorahosted.org/sssd/ticket/2833
We can't search of overridden name/id in LDAP when using local
overrides. These patches fix this for nss and sudo responder, please let
me know if there is any other place.
Similar fix needs to be done in cache_req interface, but I'll send it as
another patch since I have some work in progress in that area.
Now with patches.
@@ -611,6 +716,7 @@ errno_t check_cache(struct nss_dom_ctx *dctx,
sss_dp_callback_t callback,
void *pvt)
{
+ TALLOC_CTX *tmp_ctx;
errno_t ret;
struct nss_cmd_ctx *cmdctx = dctx->cmdctx;
struct cli_ctx *cctx = cmdctx->cctx;
@@ -628,6 +734,17 @@ errno_t check_cache(struct nss_dom_ctx *dctx,
return ENOENT;
}
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
+ return ENOMEM;
+ }
+
+ /* In case of local view we have to always contant DP with the original
+ * name or id. */
+ get_dp_name_and_id(tmp_ctx, dctx->domain, req_type, opt_name, opt_id,
+ &opt_name, &opt_id);
+
I don't like overriding opt_name and opt_id here, the caller of
check_cache() does not expect that those values change (especially
because opt_name is declared const in the header). Additionally I'm not
sure if we would run into issues in the case where e.g. the names of two
users are swapped with the help of local overrides.
I'm not sure if a temporary memory context is ok here or if the name is
needed after check_cache() is finished? Wouldn't it be more safe to
allocate the new name on cmdctx? Then you wouldn't need the temporary
context at all?
/* if we have any reply let's check cache validity, but ignore netgroups
* if refresh_expired_interval is set (which implies that another method
* is used to refresh netgroups)
@@ -725,12 +842,16 @@ errno_t check_cache(struct nss_dom_ctx *dctx,
tevent_req_set_callback(req, nsssrv_dp_send_acct_req_done, cb_ctx);
+ talloc_free(tmp_ctx);
return EAGAIN;
}
+ talloc_free(tmp_ctx);
return EOK;
error:
+ talloc_free(tmp_ctx);
+
ret = nss_cmd_send_error(cmdctx, ret);
if (ret != EOK) {
NSS_CMD_FATAL_ERROR_CODE(cctx, ret);
--
1.9.3
From c8125859c950ace5b621a98949e5ff211028801c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Sun, 11 Oct 2015 17:38:34 +0200
Subject: [PATCH 2/3] sudo: search with view even if user is found
If an overriden name is provided and the user is already cache we fail
to refresh it since we won't search with VIEW flag. This patch fix
it.
---
src/responder/sudo/sudosrv_get_sudorules.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/responder/sudo/sudosrv_get_sudorules.c
b/src/responder/sudo/sudosrv_get_sudorules.c
index
a0b09e69b71f963c353c9c6331c0708cc364924c..c29c0b1f242482795565ff1b74153619acdf0bd2
100644
--- a/src/responder/sudo/sudosrv_get_sudorules.c
+++ b/src/responder/sudo/sudosrv_get_sudorules.c
@@ -79,6 +79,7 @@ static errno_t sudosrv_get_user(struct sudo_dom_ctx *dctx)
struct dp_callback_ctx *cb_ctx;
const char *original_name = NULL;
const char *extra_flag = NULL;
+ const char *overriden_name = NULL;
char *name = NULL;
uid_t uid = 0;
errno_t ret;
@@ -160,8 +161,16 @@ static errno_t sudosrv_get_user(struct sudo_dom_ctx *dctx)
if ((user->count == 0 || cache_expire < time(NULL))
&& dctx->check_provider) {
- if (DOM_HAS_VIEWS(dom) && user->count == 0) {
- extra_flag = EXTRA_INPUT_MAYBE_WITH_VIEW;
+ if (DOM_HAS_VIEWS(dom)) {
+ if (user->count == 0) {
+ extra_flag = EXTRA_INPUT_MAYBE_WITH_VIEW;
+ } else {
+ overriden_name = ldb_msg_find_attr_as_string(user->msgs[0],
+ OVERRIDE_PREFIX SYSDB_NAME, NULL);
+ if (overriden_name != NULL) {
+ extra_flag = EXTRA_INPUT_MAYBE_WITH_VIEW;
+ }
+ }
There is a more compact version of this logic in nsssrv_cmd.c
if (DOM_HAS_VIEWS(dom) && (dctx->res->count == 0
|| ldb_msg_find_attr_as_string(dctx->res->msgs[0],
OVERRIDE_PREFIX SYSDB_NAME,
NULL) != NULL)) {
extra_flag = EXTRA_INPUT_MAYBE_WITH_VIEW;
} else {
extra_flag = NULL;
}
If you think that it is not too obfuscated I would prefer to use the
same here so that it would be better recognisable that they server the
same purpose.
}
dpreq = sss_dp_get_account_send(cli_ctx, cli_ctx->rctx,
--
1.9.3
New patches are attached.
>From e1f9b0d8d9dd48e02ffa74c2c800de0047833286 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Sun, 11 Oct 2015 16:45:19 +0200
Subject: [PATCH 1/3] nss: send original name and id with local views if
possible
Resolves:
https://fedorahosted.org/sssd/ticket/2833
---
src/responder/nss/nsssrv_cmd.c | 123 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 123 insertions(+)
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 3e95a3f5a96a91515745a17fad5d20cbc9063d28..070cc425b829280d03a691461157b50b742d95de 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -600,6 +600,124 @@ is_refreshed_on_bg(enum sss_dp_acct_type req_type,
static void nsssrv_dp_send_acct_req_done(struct tevent_req *req);
+static void get_dp_name_and_id(TALLOC_CTX *mem_ctx,
+ struct sss_domain_info *dom,
+ enum sss_dp_acct_type req_type,
+ const char *opt_name,
+ uint32_t opt_id,
+ const char **_name,
+ uint32_t *_id)
+{
+ TALLOC_CTX *tmp_ctx;
+ struct ldb_result *res = NULL;
+ const char *attr;
+ const char *name;
+ uint32_t id;
+ errno_t ret;
+
+ /* First set the same values to make things easier. */
+ *_name = opt_name;
+ *_id = opt_id;
+
+ if (!DOM_HAS_VIEWS(dom) || !is_local_view(dom->view_name)) {
+ DEBUG(SSSDBG_TRACE_FUNC, "Not a LOCAL view, continuing with "
+ "provided values.\n");
+ return;
+ }
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
+ return;
+ }
+
+ if (opt_name != NULL) {
+ switch (req_type) {
+ case SSS_DP_USER:
+ case SSS_DP_INITGROUPS:
+ ret = sysdb_getpwnam_with_views(tmp_ctx, dom, opt_name, &res);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CONF_SETTINGS,
+ "sysdb_getpwnam_with_views() failed [%d]: %s\n",
+ ret, sss_strerror(ret));
+ goto done;
+ }
+ break;
+ case SSS_DP_GROUP:
+ ret = sysdb_getgrnam_with_views(tmp_ctx, dom, opt_name, &res);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CONF_SETTINGS,
+ "sysdb_getgrnam_with_views() failed [%d]: %s\n",
+ ret, sss_strerror(ret));
+ goto done;
+ }
+ break;
+ default:
+ goto done;
+ }
+
+ if (res == NULL || res->count != 1) {
+ /* This should not happen with LOCAL view and overridden value. */
+ DEBUG(SSSDBG_TRACE_FUNC, "Entry is missing?! Continuing with "
+ "provided values.\n");
+ goto done;
+ }
+
+ name = ldb_msg_find_attr_as_string(res->msgs[0], SYSDB_NAME, NULL);
+ if (name == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Bug: name cannot be NULL\n");
+ goto done;
+ }
+
+ *_name = talloc_steal(mem_ctx, name);
+ } else if (opt_id != 0) {
+ switch (req_type) {
+ case SSS_DP_USER:
+ ret = sysdb_getpwuid_with_views(tmp_ctx, dom, opt_id, &res);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CONF_SETTINGS,
+ "sysdb_getpwuid_with_views() failed [%d]: %s\n",
+ ret, sss_strerror(ret));
+ goto done;
+ }
+
+ attr = SYSDB_UIDNUM;
+ break;
+ case SSS_DP_GROUP:
+ ret = sysdb_getgrgid_with_views(tmp_ctx, dom, opt_id, &res);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CONF_SETTINGS,
+ "sysdb_getgrgid_with_views() failed [%d]: %s\n",
+ ret, sss_strerror(ret));
+ goto done;
+ }
+
+ attr = SYSDB_GIDNUM;
+ break;
+ default:
+ goto done;
+ }
+
+ if (res == NULL || res->count != 1) {
+ /* This should not happen with LOCAL view and overridden value. */
+ DEBUG(SSSDBG_TRACE_FUNC, "Entry is missing?! Continuing with "
+ "provided values.\n");
+ goto done;
+ }
+
+ id = ldb_msg_find_attr_as_uint64(res->msgs[0], attr, 0);
+ if (id == 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Bug: id cannot be 0\n");
+ goto done;
+ }
+
+ *_id = id;
+ }
+
+done:
+ talloc_free(tmp_ctx);
+}
+
/* FIXME: do not check res->count, but get in a msgs and check in parent */
errno_t check_cache(struct nss_dom_ctx *dctx,
struct nss_ctx *nctx,
@@ -628,6 +746,11 @@ errno_t check_cache(struct nss_dom_ctx *dctx,
return ENOENT;
}
+ /* In case of local view we have to always contant DP with the original
+ * name or id. */
+ get_dp_name_and_id(dctx->cmdctx, dctx->domain, req_type, opt_name, opt_id,
+ &opt_name, &opt_id);
+
/* if we have any reply let's check cache validity, but ignore netgroups
* if refresh_expired_interval is set (which implies that another method
* is used to refresh netgroups)
--
1.9.3
>From 297136817568ba31c5c5f1fabf3eafc339cf26dd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Sun, 11 Oct 2015 17:38:34 +0200
Subject: [PATCH 2/3] sudo: search with view even if user is found
If an overriden name is provided and the user is already cache we fail
to refresh it since we won't search with VIEW flag. This patch fix
it.
---
src/responder/sudo/sudosrv_get_sudorules.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/responder/sudo/sudosrv_get_sudorules.c b/src/responder/sudo/sudosrv_get_sudorules.c
index a0b09e69b71f963c353c9c6331c0708cc364924c..cc06977d97e3319584251bdab26e85855d275e8a 100644
--- a/src/responder/sudo/sudosrv_get_sudorules.c
+++ b/src/responder/sudo/sudosrv_get_sudorules.c
@@ -160,7 +160,10 @@ static errno_t sudosrv_get_user(struct sudo_dom_ctx *dctx)
if ((user->count == 0 || cache_expire < time(NULL))
&& dctx->check_provider) {
- if (DOM_HAS_VIEWS(dom) && user->count == 0) {
+ if (DOM_HAS_VIEWS(dom) && (user->count == 0
+ || ldb_msg_find_attr_as_string(user->msgs[0],
+ OVERRIDE_PREFIX SYSDB_NAME,
+ NULL) != NULL)) {
extra_flag = EXTRA_INPUT_MAYBE_WITH_VIEW;
}
--
1.9.3
>From 095da3b40e85ad16f73252e80a9a36bbe3f1adad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Sun, 11 Oct 2015 17:53:28 +0200
Subject: [PATCH 3/3] sudo: send original name and id with local views if
possible
Resolves:
https://fedorahosted.org/sssd/ticket/2833
---
src/responder/sudo/sudosrv_get_sudorules.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/src/responder/sudo/sudosrv_get_sudorules.c b/src/responder/sudo/sudosrv_get_sudorules.c
index cc06977d97e3319584251bdab26e85855d275e8a..c3336960eeac18ee63167de81891984aa764540c 100644
--- a/src/responder/sudo/sudosrv_get_sudorules.c
+++ b/src/responder/sudo/sudosrv_get_sudorules.c
@@ -79,6 +79,7 @@ static errno_t sudosrv_get_user(struct sudo_dom_ctx *dctx)
struct dp_callback_ctx *cb_ctx;
const char *original_name = NULL;
const char *extra_flag = NULL;
+ const char *search_name = NULL;
char *name = NULL;
uid_t uid = 0;
errno_t ret;
@@ -160,16 +161,23 @@ static errno_t sudosrv_get_user(struct sudo_dom_ctx *dctx)
if ((user->count == 0 || cache_expire < time(NULL))
&& dctx->check_provider) {
- if (DOM_HAS_VIEWS(dom) && (user->count == 0
- || ldb_msg_find_attr_as_string(user->msgs[0],
- OVERRIDE_PREFIX SYSDB_NAME,
- NULL) != NULL)) {
+ search_name = cmd_ctx->username;
+ if (is_local_view(dom->view_name)) {
+ /* Search with original name in case of local view. */
+ if (user->count != 0) {
+ search_name = ldb_msg_find_attr_as_string(user->msgs[0],
+ SYSDB_NAME, NULL);
+ }
+ } else if (DOM_HAS_VIEWS(dom) && (user->count == 0
+ || ldb_msg_find_attr_as_string(user->msgs[0],
+ OVERRIDE_PREFIX SYSDB_NAME,
+ NULL) != NULL)) {
extra_flag = EXTRA_INPUT_MAYBE_WITH_VIEW;
}
dpreq = sss_dp_get_account_send(cli_ctx, cli_ctx->rctx,
dom, false, SSS_DP_INITGROUPS,
- cmd_ctx->username, 0, extra_flag);
+ search_name, 0, extra_flag);
if (!dpreq) {
DEBUG(SSSDBG_CRIT_FAILURE,
"Out of memory sending data provider request\n");
--
1.9.3
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
