[SSSD] [PATCHES] KRB5: Add support for KEYRING:persistent

Stephen Gallagher sgallagh at redhat.com
Tue Aug 27 14:47:21 UTC 2013


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/27/2013 10:45 AM, Jakub Hrozek wrote:
> On Mon, Aug 26, 2013 at 07:10:35AM -0400, Stephen Gallagher wrote:
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>> 
>> On 08/26/2013 05:19 AM, Jakub Hrozek wrote:
>>> On Fri, Aug 23, 2013 at 02:37:48PM -0400, Stephen Gallagher
>>> wrote:
>>>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>>>> 
>>>> Patch 0001: Adds a couple debug messages I found useful to
>>>> figure out if a bug was in SSSD or libkrb5. Feel free to skip
>>>> its inclusion if you feel it's unnecessary (it's at log level
>>>> 9)
>>> 
>>> ACK
>>> 
>>>> 
>>>> Patch 0002: KRB5: Remove unnecessary call to become_user() By
>>>> the time that the create_ccache_in_dir() routine is called,
>>>> we are already guaranteed to have dropped privileges. This
>>>> has either happened because we dropped them before the exec()
>>>> in the normal operation case or because we dropped them
>>>> explicitly after we completed the TGT validation step if that
>>>> or FAST is configured. This code is actually completely
>>>> harmless, since it checks internally to see if we've already
>>>> dropped privileges before it does so, but it's unnecessary.
>>> 
>>> ACK. There is one patch that I thought wouldn't be handled at 
>>> first (validate==true && offline==true) but in that case the
>>> child is spawned as the user already.
>>> 
>>>> 
>>>> Patch 0003: KRB5: Add support for KEYRING cache type This is
>>>> the Big One. This adds support for KEYRING types but notes in
>>>>  sssd-krb5(5) that we only support KEYRING:persistent (this
>>>> is because the other keyring types may break if validation or
>>>> FAST are used). Other than that, this patch should be
>>>> reasonably straightforward. I have tested this code against
>>>> Simo's preliminary Kerberos patches along with David Howells'
>>>> kernel patch for persistent keyrings. We hit (and fixed) a
>>>> couple bugs in krb5. This patch should be safe to put into
>>>> the upstream master immediately; it does not change any
>>>> behavior with the existing FILE or DIR cache types.
>>> 
>>> Where can I get the dependencies to test these patches? So far
>>> I only read the code and it looks OK to me. One question:
>>> 
>> 
>> I'm attaching a repo file to point at Simo's experimental
>> repository. It requires an updated kernel, keyutils and krb5
>> packages.
>> 
>> 
>>>> +errno_t +cc_keyring_check_existing(const char *location,
>>>> uid_t uid, +                          const char *realm,
>>>> const char *princ, +                          const char
>>>> *cc_template, bool *_active, +                          bool
>>>> *_valid) +{ + errno_t ret; +    bool active; +    bool valid;
>>>> +    const char *residual; + +    residual = 
>>>> sss_krb5_residual_check_type(location,
>>>> SSS_KRB5_TYPE_KEYRING); + if (!residual) { +
>>>> DEBUG(SSSDBG_CRIT_FAILURE, + ("%s is not of type KEYRING:\n",
>>>> location)); +        return EINVAL; +    } + +    /* The
>>>> keyring cache is always active */ + active = true; + +    /*
>>>> Check if any user is actively using this cache */
>>> 
>>> The comment sounds like you got the two (active and valid)
>>> mixed up.
>>> 
>>> check_cc_validity checks if the ccache contains a credential
>>> that is not expired yet, not that a user is actively using it.
>>> 
>> 
>> Are you saying I go the comments wrong or the functionality? I'm
>> not clear here.
> 
> Just the comment (now that I actually tested the code).
> 
> I tested that: * logins work * parallel logins reuse the same
> ccache and extend it (with the right krb5 package) * new ccache is
> correctly generated when the old one goes away (I only tested with
> kdestroy, I didn't remove the key from the keyring per se) *
> renewals work
> 
> So ACK to the code. I can either remove the comment before pushing
> or resend it with the comment removed.
> 

Thanks very much for testing, Jakub. Feel free to remove the comment
before pushing. Also, while you're in there, would you mind adding
that it resolves https://fedorahosted.org/sssd/ticket/2036 to the
commit message as well?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlIcu/kACgkQeiVVYja6o6PZBACeNbUXG53SlwVKzsvTkQMaAd/X
WnIAn1vuFmLlrFndAeuCvxrmeuREHSCX
=ps6f
-----END PGP SIGNATURE-----



More information about the sssd-devel mailing list