-----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-----