[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