-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 12/03/2010 05:12 AM, Sumit Bose wrote: > On Wed, Dec 01, 2010 at 03:12:02PM -0500, Stephen Gallagher wrote: > Patch 0001: Ack. > Patch 0002: This patch won't apply. Is it dependent on another patch > currently in review? I tried atop master and atop the nine patches for > TGT renewal, but it is still missing blobs according to git. > >> This need to be applied on top of the ticket renewal patches, because >> both change krb5_child. Maybe some changes in master causes this issue, >> I have attached rebased versions of the patches.
Patch 0001: Ack. Patch 0002: Nack. + if (strncmp(krb5_princ_realm(ctx, entry.principal)->data, realm, + krb5_princ_realm(ctx, entry.principal)->length) == 0) { I don't think you want to use strncmp() here. If the keytab has a principal for realm MYCOMPANY, but you're looking for a key for MYCOMPANY-IT, this comparison is going to get a false positive. If you really want strncmp(), base the length on the sought-after realm, not the one in the keytab. Please explain the rationale for arbitrarily taking the last entry in the keytab, regardless of whether it's our realm. This doesn't make much sense to me. I'd think that if we got to the end and hadn't matched our realm, that would be an error. Can we avoid the stat() in check_fast_ccache()? It's a race-condition bug waiting to happen (we stat, then the file gets deleted before we read it). If we're just testing for existence, isn't it just as easy to rely on get_tgt_times() returning krberr from krb5_cc_resolve()? The manpage says that setting use_fast to "try" or "demand" should throw an error on systems that don't support it. However, sss_krb5_get_init_creds_opt_set_fast_ccache_name() returns 0. It should return KRB5KDC_ERR_BADOPTION, probably (maybe there's a better code for this?) + + use_fast_str = dp_opt_get_string(opts, KRB5_USE_FAST); + if (use_fast_str != NULL) { + ret = check_fast(use_fast_str, &krb5_ctx->use_fast); + if (ret != EOK) { + DEBUG(1, ("check_fast failed.\n")); + return ret; + } + + ret = setenv(SSSD_KRB5_USE_FAST, use_fast_str, 1); + if (ret != EOK) { + DEBUG(2, ("setenv [%s] failed.\n", SSSD_KRB5_USE_FAST)); + } + } + If we're not setting the environment variable when use_fast_str == NULL, we should probably also avoid setting it if krb5_ctx->use_fast has returned false. Just to avoid an unnecessary and slow syscall. - -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkz5DJMACgkQeiVVYja6o6PxXwCfYu0cQcCmdFYIDT8h5//7CKTb hrYAn2T0g4Q2ZUQQrRq+yLHJLtKy79EI =TIMw -----END PGP SIGNATURE----- _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel