[SSSD] [PATCH] local overrides: don't concact server with overriden name/id

Pavel Březina pbrezina at redhat.com
Tue Oct 13 10:22:56 UTC 2015


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?= <pbrezina at 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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-nss-send-original-name-and-id-with-local-views-if-po.patch
Type: text/x-patch
Size: 5360 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151013/325c9f17/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-sudo-search-with-view-even-if-user-is-found.patch
Type: text/x-patch
Size: 1385 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151013/325c9f17/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-sudo-send-original-name-and-id-with-local-views-if-p.patch
Type: text/x-patch
Size: 2658 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151013/325c9f17/attachment-0005.bin>


More information about the sssd-devel mailing list