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:
+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.
+ ret = check_cc_validity(location, realm, princ, &valid);
+ if (ret != EOK) {
+ return ret;
+ }
+
+ *_active = active;
+ *_valid = valid;
+ return EOK;
+}