[SSSD] [PATCH 2/2] Making the authtok structure really opaque.

Jakub Hrozek jhrozek at redhat.com
Mon Mar 25 11:44:53 UTC 2013


On Mon, Mar 25, 2013 at 08:45:38AM +0100, Lukas Slebodnik wrote:
> On (22/03/13 14:42), Jakub Hrozek wrote:
> >On Fri, Mar 22, 2013 at 09:19:13AM +0100, Lukas Slebodnik wrote:
> >> On (21/03/13 20:18), Jakub Hrozek wrote:
> >> >On Wed, Mar 20, 2013 at 07:17:28PM +0100, Lukas Slebodnik wrote:
> >> >> On (20/03/13 13:58), Jakub Hrozek wrote:
> >> >> >On Fri, Mar 15, 2013 at 12:46:34PM +0100, Ondrej Kos wrote:
> >> >> >> On 03/15/2013 10:34 AM, Lukas Slebodnik wrote:
> >> >> >> >On (15/03/13 09:13), Ondrej Kos wrote:
> >> >> >> >>On 03/14/2013 12:58 PM, Lukas Slebodnik wrote:
> >> >> >> >>>@@ -151,8 +167,10 @@ void pam_print_data(int l, struct pam_data *pd)
> >> >> >> >>>      DEBUG(l, ("tty: %s\n", PAM_SAFE_ITEM(pd->tty)));
> >> >> >> >>>      DEBUG(l, ("ruser: %s\n", PAM_SAFE_ITEM(pd->ruser)));
> >> >> >> >>>      DEBUG(l, ("rhost: %s\n", PAM_SAFE_ITEM(pd->rhost)));
> >> >> >> >>>-    DEBUG(l, ("authtok type: %d\n", sss_authtok_get_type(&pd->authtok)));
> >> >> >> >>>-    DEBUG(l, ("newauthtok type: %d\n", sss_authtok_get_type(&pd->newauthtok)));
> >> >> >> >>>+    DEBUG(SSSDBG_CRIT_FAILURE,
> >> >> >> >>>+          ("authtok type: %d\n", sss_authtok_get_type(pd->authtok)));
> >> >> >> >>>+    DEBUG(SSSDBG_CRIT_FAILURE,
> >> >> >> >>>+          ("newauthtok type: %d\n", sss_authtok_get_type(pd->newauthtok)));
> >> >> >> >>>      DEBUG(l, ("priv: %d\n", pd->priv));
> >> >> >> >>>      DEBUG(l, ("cli_pid: %d\n", pd->cli_pid));
> >> >> >> >>>  }
> >> >> >> >>
> >> >> >> >>The patches look good to me, just please use the debug level variable
> >> >> >> >>here, not hardcoded level.
> >> >> >> >>
> >> >> >> >>--
> >> >> >> >>Ondrej Kos
> >> >> >> >
> >> >> >> >Thanks.
> >> >> >> >I am attaching updated patches.
> >> >> >> >
> >> >> >> >LS
> >> >> >> >
> >> >> >> >
> >> >> >> >
> >> >> >> Ack from me
> >> >> >> 
> >> >> >> Ondra
> >> >> >
> >> >> >The second patch no longer applies on the current master, can you rebase
> >> >> >it?
> >> >> 
> >> >> I am attaching rebased patches.
> >> >> 
> >> >> LS
> >> >
> >> >> +struct sss_auth_token *sss_authtok_new_empty(TALLOC_CTX *mem_ctx)
> >> >> +{
> >> >> +	struct sss_auth_token *token;
> >> >> +
> >> >> +	token = talloc_zero(mem_ctx, struct sss_auth_token);
> >> >
> >> >^^^ tabs instead of spaces.
> >> >
> >> >> +    if (token == NULL) {
> >> >> +        DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_zero failed.\n"));
> >> >> +    }
> >> >> +
> >> >> +    return token;
> >> >> +}
> >> >> +
> >> >> +
> >> >
> >> >The rest looks good to me.
> >> 
> >> Thanks.
> >> I'm attaching updated patches.
> >> 
> >> LS
> >
> >> -    ret = sss_authtok_set_password(pd, &pd->authtok, password, keysize);
> >> +    ret = sss_authtok_set_password(pd, pd->authtok, password, keysize);
> >
> >We agreed with Lukas off-list that it would be safer to use pd->authtok
> >as the memory context here, because the memory blob should be owned by
> >the authtok structure, not the pam_data structure. It's unlikely we'll
> >ever steal or move the memory, but it's better to be safe.
> 
> Talloc contex updated.
> I am attaching updated patches
> 
> LS

Nack, I'm getting segfault on login with these patches, backtrace is
pointing to sss_authtok_copy



More information about the sssd-devel mailing list