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
The values won't change in the caller of check_cache.
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.
Hmm, this is something we should check. Will you file a ticket?
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?
Ok.
> /* 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?= <pbrezina(a)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.
Fine, I'll change that.
> }
>
> dpreq = sss_dp_get_account_send(cli_ctx, cli_ctx->rctx,
> --
> 1.9.3
>
bye,
Sumit
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel