On Tue, Aug 26, 2014 at 02:50:20PM +0200, Jakub Hrozek wrote:
On Tue, Aug 26, 2014 at 02:21:00PM +0200, Sumit Bose wrote:
> > > This is working as expected, but using krb5_cc_new_unique() and
> > > krb5_cc_get_name() from libkrb5 might even result in more simple code?
> >
> > In theory yes, I have a patch that I tested already:
> >
https://fedorapeople.org/cgit/jhrozek/public_git/sssd.git/commit/?h=ldapc...
> >
> > But I'm not sure this approach will work well for us, because:
> > 1) The ccache is always created in /tmp when using
> > krb5_cc_new_unique(). In the krb5 code it looks like
> > krb5_cc_new_unique calls krb5_fcc_generate_new() which
> > unconditionally uses a templatized file in /tmp. Because on many
> > systems, tmp is a tmpfs, the rename errored out with error 18
> > (Invalid cross-device link).
> >
> > 2) Rename from one directory to another is not friedly to SELinux.
> >
> > So while the code is way cleaner than handling the templates ourselves,
> > I think we would have a way to tell krb5_cc_new_unique() to accept a
> > directory to create the ccache at..or copy the file ourselves.
>
> ah, sorry, I didn't realized that the hint option is unused. I agree
> that you original approach is better. But please free the temporary
> strings.
OK, done. I used a tmp_ctx for this change, because we have several
other temporary strings that suffer from the same. I also added the
extra debug message you requested earlier.
Thank you, ACK to both.
bye,
Sumit
btw when I was testing the changes to make sure there is no
use-after-free, I realized we have many leaks in the ldap_child itself.
Using:
command = /usr/bin/valgrind --trace-children=yes /usr/libexec/sssd/sssd_be --domain
ipa.example.com --debug-level 3
in the domain section. I guess that's worth of a ticket, but not urgent
either.