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 _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
