[SSSD] [PATCH] Every time return directory for krb5 cache collection.

Sumit Bose sbose at redhat.com
Wed Jun 19 10:20:59 UTC 2013


On Mon, Jun 17, 2013 at 12:02:02PM +0200, Lukas Slebodnik wrote:
> On (12/06/13 16:05), Jakub Hrozek wrote:
> >On Wed, Jun 05, 2013 at 09:37:28AM +0200, Lukas Slebodnik wrote:
> >> On (02/06/13 23:14), Jakub Hrozek wrote:
> >> >On Sat, 2013-06-01 at 11:43 +0200, Lukas Slebodnik wrote:
> >> >> On (31/05/13 21:29), Jakub Hrozek wrote:
> >> >> >On Wed, 2013-05-29 at 16:09 +0200, Lukas Slebodnik wrote:
> >> >> >> On (29/05/13 08:44), Simo Sorce wrote:
> >> >> >> >On Wed, 2013-05-29 at 11:30 +0200, Lukas Slebodnik wrote:
> >> >> >> >> On (29/05/13 11:07), Lukas Slebodnik wrote:
> >> >> >> >> >ehlo,
> >> >> >> >> >
> >> >> >> >> >Function krb5_cc_get_full_name is called only as a way to validate that,
> >> >> >> >> >we have the right cache. Instead of returned name, location will be returned
> >> >> >> >> >from function cc_dir_cache_for_princ.
> >> >> >> >> >
> >> >> >> >> >https://fedorahosted.org/sssd/ticket/1936
> >> >> >> >> >
> >> >> >> >> >Patch is attached.
> >> >> >> >> >
> >> >> >> >> >LS
> >> >> >> >> 
> >> >> >> >> self NACK
> >> >> >> >> 
> >> >> >> >> this patch store to cache DIR:/run/user/325600000/krb5cc
> >> >> >> >>                             ^^^^
> >> >> >> >>                      missing colon?
> >> >> >> >
> >> >> >> >No, this is the correct form.
> >> >> >> I found out, that it is a correct form.
> >> >> >> Problem was with checking ccname in function sss_krb5_cc_file_path.
> >> >> >> 
> >> >> >> New patches attached.
> >> >> >> 
> >> >> >> LS
> >> >> >
> >> >> >Sorry for the reply from gmail. My OTP token decided the best password
> >> >> >for me on a Friday evening is "Err", so I can't access my redhat.com
> >> >> >account at the moment.
> >> >> >
> >> >> >These patches break one assumption we want to keep -- if there is a user
> >> >> >logged in and the same user logs in for example from another terminal,
> >> >> >they should have the same ccache. With your patches, I'm getting a new
> >> >> >one when I log in simultaneously.
> >> >> >
> >> >> >I haven't tested that, but I guess this is because path to collection is
> >> >> >always passed to the krb5_child now. I think that in the case user is
> >> >> >already logged in (in krb5 code we denote this with "ccache is active"),
> >> >> >then you should pass the full path to the ccache to the krb5_child.
> >> >> >
> >> >> Simo wrote in ticket comment (https://fedorahosted.org/sssd/ticket/1936#comment:10)
> >> >> 
> >> >>    > Do we really want to store only DIR:/run/user/$uid/krb5cc/ to cache?
> >> >>    Yes, this is exactly what we want as a ccache.
> >> >> 
> >> >> LS
> >> >
> >> >Yes, I saw that comment and I agree with Simo.
> >> >
> >> >But I think we should examine the ccache collection and in the case
> >> >there already is a valid cache present there, we should reuse it just
> >> >like we did before.
> >> >
> >> 
> >> I did not realize, that new ccache is created after another login.
> >> Thank you for review.
> >> 
> >> Updated patches are attached.
> >> 
> >> LS
> >
> >I'm afraid the detection works too well :-)
> >
> >I tested with an expired ccache:
> >$ sudo klist /run/user/208800000/ccdir/tktGihEUV
> >Ticket cache: FILE:/run/user/208800000/ccdir/tktGihEUV
> >Default principal: admin at EXAMPLE.COM
> >
> >Valid starting       Expires              Service principal
> >06/07/2013 14:57:16  06/08/2013 14:57:16  krbtgt/EXAMPLE.COM at EXAMPLE.COM
> >                     ^^^^^^^^^^^^^^^^^^
> >                     only valid until 8th of July
> >
> >    renew until 06/14/2013 14:57:16
> >
> >And SSSD assigned me this ccache. I would expect that if there already
> >is a ccache for the user, but it is expired (there is a function in the
> >krb5 provider that checks if ccache is valid and active), then it would
> >be overwritten with a fresh one.
> 
> With new patches you will get only valid cache.
> 
> LS

I would prefer if the checks can be done in the responder and not in the
krb5_child. The current logic is documented and evaluated in
krb5_auth_prepare_ccache_file(), do you think it would be possible to
enhance krb5_auth_prepare_ccache_file() with the needed new checks?
Additionally there is already code in the responder e.g. to check for a
valid TGT which might help to simplify your patch.

The responder then can send either

DIR::/run/user/325600000/krb5cc/ to indicate that a new ccache should be
created in the collection, and that existing ccaches in the collection
for the principal can be removed, because they are invalid and not used

or

DIR::/run/user/325600000/krb5cc/tktt1yNP8 to indicated that the
specified ccache in the collection should be reused

to the krb5_child. Does this make sense?



> @@ -1100,13 +1257,19 @@ static krb5_error_code get_and_save_tgt(struct krb5_req *kr,
>  
>      DEBUG(SSSDBG_TRACE_FUNC,
>            ("Attempting kinit for realm [%s]\n",realm_name));
> -    kerr = krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ,
> -                                        discard_const(password),
> -                                        sss_krb5_prompter, kr, 0,
> -                                        NULL, kr->options);
> -    if (kerr != 0) {
> -        KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);
> -        return kerr;
> +
> +    have_old_cache = retrieved_valid_cred_from_cache(kr->ctx, kr->princ,
> +                                                     realm_name, kr->ccname,
> +                                                     &kr->creds);
> +    if (!have_old_cache) {

we always want to get a new TGT and update the existing cache to
increase the use time of the ticket, otherwise we would be limited by
the lifetime of the ticket and the
use-the-screensaver-to-get-a-new-valid-ticket trick wouldn't work
anymore.

> +        kerr = krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ,
> +                                            discard_const(password),
> +                                            sss_krb5_prompter, kr, 0,
> +                                            NULL, kr->options);
> +        if (kerr != 0) {
> +            KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);
> +            return kerr;
> +        }
>      }
>  
>      if (kr->validate) {

bye,
Sumit



More information about the sssd-devel mailing list