[SSSD] [PATCH] Fix krbcc dir creation issue with MIT krb5 1.11

Simo Sorce simo at redhat.com
Wed Apr 10 14:21:16 UTC 2013


On Wed, 2013-04-10 at 15:09 +0200, Jakub Hrozek wrote:
> On Wed, Apr 10, 2013 at 08:48:08AM -0400, Simo Sorce wrote:
> > On Wed, 2013-04-10 at 11:45 +0200, Lukas Slebodnik wrote:
> > > On (04/04/13 12:09), Simo Sorce wrote:
> > > >On Thu, 2013-04-04 at 12:14 +0200, Lukas Slebodnik wrote:
> > > >> ehlo,
> > > >> 
> > > >> In krb5-libs 1.11, function krb5_cc_resolve verify if credential cache dir
> > > >> exists. If it doesn't exist, than it will be created. We
> > > >> have to ensure, that it will be created with right uid, gid and permissions.
> > > >> 
> > > >> There is check in krb_auth_send, whether cached user data in ldb contains
> > > >> SYSDB_CCACHE_FILE attribute. If it is available, then old ccache is checked in
> > > >> function check_old_ccache. Gdm clean user directory (in /run/user/)
> > > >> after logout. This is the reason why SYSDB_CCACHE_FILE attribute was found
> > > >> in ldb cache and directory did not exist and then in function krb5_cc_resolve
> > > >> was created with wrong permissions.
> > > >> 
> > > >> Patch attached.
> > > >
> > > >Although this approach may work I am not really comfortable with it.
> > > >
> > > >Here is an alternative approach:
> > > >
> > > >1. Before calling krb5_cc_resolve() do a stat.
> > > function cc_residual_is_used call stat, but nonexisting file(directory)
> > > was ignored (handled like EOK)
> > 
> > Time to un-ignore it :-)
> > 
> > > >2. If the file/dir does not exist just return an error w/o calling any
> > > >krb5 function at all.
> > > >
> > > >I think this should also solve the issue, and will not involve change of
> > > >uid/gid.
> > > >
> > > >This method has a very small race that can hardly be triggered though,
> > > >it would require someone to remove the cache after the stat and before
> > > >krb5_cc_resolve() is called. In that case auth would fail, but we should
> > > >be able to recover on the next authentication by removing the file if it
> > > >has the wrong permissions (I think this is already done, if not we can
> > > >fix it with a second patch that adds checking for uid/gid after the stat
> > > >and a call to cc_file_rmove/cc_dir_remove).
> > > cc_dir_remove call cc_file_remove, so directory will not be removed.
> > 
> > That is fine, we nee to check for dir/primary in dir case I think.
> > 
> > Simo.
> 
> This patch fixes the error by the way. Basic sanity testing also went
> fine and the code looks good to me.

Sorry, meant to say it looks good to me too, but haven't tested.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York




More information about the sssd-devel mailing list