[SSSD] [PATCH] Every time return directory for krb5 cache collection.
Jakub Hrozek
jhrozek at redhat.com
Fri Jun 21 18:45:55 UTC 2013
On Thu, Jun 20, 2013 at 11:11:17AM +0200, Lukas Slebodnik wrote:
> Rewritten patches are attached.
>
> LS
Two nitpicks:
> +static char * get_ccache_name_by_principal(TALLOC_CTX *mem_ctx,
> + krb5_context ctx,
> + krb5_principal principal,
> + const char *ccname)
> +{
> + krb5_error_code kerr;
> + krb5_ccache tmp_cc = NULL;
> + char *tmp_ccname = NULL;
> + char *ret_ccname = NULL;
> +
> + kerr = krb5_cc_set_default_name(ctx, ccname);
> + if (kerr != 0) {
> + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);
> + return NULL;
> + }
> +
> + kerr = krb5_cc_cache_match(ctx, principal, &tmp_cc);
> + if (kerr != 0) {
> + KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);
> + return NULL;
> + }
> +
> + kerr = krb5_cc_get_full_name(ctx, tmp_cc, &tmp_ccname);
> + if (kerr !=0) {
> + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);
> + }
> +
> + ret_ccname = talloc_strdup(mem_ctx, tmp_ccname);
> + if (ret_ccname == NULL) {
> + DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed (ENOMEM).\n"));
> + }
> +
> +done:
This done label is unused and GCC emits a warning over that. It should
be used if krb5_cc_get_full_name() fails.
> + if (tmp_cc != NULL) {
> + kerr = krb5_cc_close(ctx, tmp_cc);
> + if (kerr != 0) {
> + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);
> + }
> + }
> + krb5_free_string(ctx, tmp_ccname);
> +
> + return ret_ccname;
> +}
> +
>
> + kerr = krb5_unparse_name(ctx, princ, &full_name);
> + if (kerr !=0) {
> + KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);
> + goto done;
> + }
> +
> + if (0 == strcmp(default_full_name, full_name)) {
> + ret = true;
> + }
> +
If you're changing the other patch, can you also add DEBUG message here
saying what has been compared?
> +done:
> + if (default_cc != NULL) {
> + kerr = krb5_cc_close(ctx, default_cc);
> + if (kerr != 0) {
> + KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr);
> + goto done;
> + }
> + }
> +
> + /* all functions can be safely called with NULL. */
> + krb5_free_principal(ctx, default_princ);
> + krb5_free_unparsed_name(ctx, default_full_name);
> + krb5_free_unparsed_name(ctx, full_name);
> +
> + return ret;
> +}
So far I ran a number of tests against IPA. I should still check AD
provider especially with enterprise principals to see if they still
work. If they do I'll ack.
But I will also file a ticket for 1.10.1 to move the checks to the
backend so they are all done on one place as Sumit suggested earlier.
I don't think it's necessary to delay 1.10.0 any longer for this issue,
but it's the right thing to do eventually.
More information about the sssd-devel
mailing list