Re: [SSSD] [PATCH] PAM: new option pam_trusted_users

[Lukas Slebodnik]

On (29/09/14 11:10), Pavel Reichl wrote:
> Hello,
>
> please see attached patch, which is amended Daniel Gollubs' patch from thread
>
> [SSSD] [PATCH] PAM: Add domains= option to pam_sss (https://lists.fedorahosted.org/pipermail/sssd-devel/2014-July/020867.html).
>
> I have just fixed nitpick found by Jakub and rebased it on top of previous patch in this thread.
>
> and I also changed this:
>
> Because I think dom->name would be uninitialized. However, I haven't tested
> this patch.
>
> @@ -836,12 +867,22 @@ static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd)
>             ret = ENOENT;
>             goto done;
>         }
>    /* skip this domain if not requested */
>    if (!is_domain_requested(pd, dom->name)) {
>    if (!is_domain_requested(pd, pd->domain)) {
>  ret = ENOENT;
>  goto done;
> }
>
> Thanks!
>
> On 09/27/2014 12:39 PM, Pavel Reichl wrote:
> > Attached patch now contains both options.
> >
> > Thanks.
> >
> > On 09/25/2014 03:57 PM, Pavel Reichl wrote:
> > > Hello,
> > >
> > > please see attached patch which adds first of two options proposed in
> > > https://fedorahosted.org/sssd/wiki/DesignDocs/RestrictDomainsInPAM
> > >
> > > Thanks!
> From 7ea2724932de7ae651d73b0442c66aa88b4c8f2f Mon Sep 17 00:00:00 2001
> From: Daniel Gollub <dgollub at brocade.com>
> Date: Sat, 27 Sep 2014 12:06:44 +0100
> Subject: [PATCH] PAM: Add domains= option to pam_sss
>
> Design document:
> https://fedorahosted.org/sssd/wiki/DesignDocs/RestrictDomainsInPAM
>
> Fixes:
> https://fedorahosted.org/sssd/ticket/1021
>
> Signed-off-by: Pavel Reichl 
> Reviewed-by: Sven-Thorsten Dietrich <sven at brocade.com>

//snip
> @@ -1284,6 +1298,14 @@ static void eval_argv(pam_handle_t *pamh, int argc, const char **argv,
>             *flags |= FLAGS_USE_FIRST_PASS;
>         } else if (strcmp(*argv, "use_authtok") == 0) {
>             *flags |= FLAGS_USE_AUTHTOK;
>    } else if (strncmp(*argv, OPT_DOMAINS_KEY, strlen(OPT_DOMAINS_KEY)) == 0) {
>        if (*(*argv+strlen(OPT_DOMAINS_KEY)) == '\0') {
>            logger(pamh, LOG_ERR, "Missing argument to option domains.");
>            *domains = '\0';

^^^^^^^^
                  Type of "*domains" is constant pointer to char (const char *)
                  and character '\0' is assigned to pointer, which is
                  interpreted as NULL.

It will be better to assign zero length string ("") or NULL to this output
argument.

LS

BTW: you can resent patches together after fixing nitpicks.
It will be simpler to push them together from one mail.