On 10/13/2015 03:51 PM, Jakub Hrozek wrote:
On Tue, Oct 13, 2015 at 12:08:22PM +0200, Pavel Březina wrote:
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
The values won't change in the caller of check_cache.
But it's still really bad taste to change input parameters, can we have
a separate variable we assign to?
Ok, new patches are attached.
>From e4fe1bbf24b6739c7dce43a25a06b7b891ecc97e 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 | 131 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 128 insertions(+), 3 deletions(-)
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 3e95a3f5a96a91515745a17fad5d20cbc9063d28..22685af1c1c2d62fbcb686ae168fec46d31fb849 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,
@@ -617,6 +735,8 @@ errno_t check_cache(struct nss_dom_ctx *dctx,
struct tevent_req *req = NULL;
struct dp_callback_ctx *cb_ctx = NULL;
uint64_t cacheExpire = 0;
+ const char *name = opt_name;
+ uint32_t id = opt_id;
/* when searching for a user or netgroup, more than one reply is a
* db error
@@ -628,6 +748,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,
+ &name, &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)
@@ -672,10 +797,10 @@ errno_t check_cache(struct nss_dom_ctx *dctx,
* immediately.
*/
DEBUG(SSSDBG_TRACE_FUNC,
- "Performing midpoint cache update on [%s]\n", opt_name);
+ "Performing midpoint cache update on [%s]\n", name);
req = sss_dp_get_account_send(cctx, cctx->rctx, dctx->domain, true,
- req_type, opt_name, opt_id, extra);
+ req_type, name, id, extra);
if (!req) {
DEBUG(SSSDBG_CRIT_FAILURE,
"Out of memory sending out-of-band data provider "
@@ -704,7 +829,7 @@ errno_t check_cache(struct nss_dom_ctx *dctx,
}
req = sss_dp_get_account_send(cctx, cctx->rctx, dctx->domain, true,
- req_type, opt_name, opt_id, extra);
+ req_type, name, id, extra);
if (!req) {
DEBUG(SSSDBG_CRIT_FAILURE,
"Out of memory sending data provider request\n");
--
1.9.3
>From 21ad142974edeaaa487d188e27546f69356c9257 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 9d5e53c02eee2ac0d1aebbbcca542f69a0ce9feb 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
