On Mon, Jun 24, 2013 at 10:54:30PM +0200, Lukas Slebodnik wrote:
On (24/06/13 22:06), Jakub Hrozek wrote:
>On Sat, Jun 22, 2013 at 01:55:51PM +0200, Lukas Slebodnik wrote:
>> On (21/06/13 20:45), Jakub Hrozek wrote:
>> >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 removed checks in former updates. There left only mapping
>> from collection ccache (DIR:<cc_dir_name>) to ccache
>> (DIR::<cc_dir_name>/<ticket_name>)
>>
>> I tried some modifications on backend side (with gdb), but I could not get an
aim
>> 1. store collection ccache name stored in sysdb
>> 2. environment variable KRB5CCNAME also contains collection ccache name
>> 3. do not have more ccaches with same principal name
>>
>> >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.
>>
>> Updated patches are attached.
>>
>> LS
>
>I still see two issues, sorry:
>
>1) The cc_residual_is_used check seems to be broken:
>
>[sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x1000): User [1983201109]
>is still active, reusing ccache [/run/user/1983201109].
>[sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x0200): Cache file
>[/run/user/1983201109/primary] does not exist, it will be recreated
> ^^^^^^^^^^^^^^^^^
> This path cannot exist, I think SSSD should be checking for
> /run/user/1983201109/krb5cc/primary instead
>
>[sssd[be[AD.EXAMPLE.COM]]] [check_for_valid_tgt] (0x0020): krb5_cc_retrieve_cred
failed.
>[sssd[be[AD.EXAMPLE.COM]]] [fo_resolve_service_send] (0x0100): Trying to resolve
service 'AD'
>
>2) When using enterprise credentials, multiple logins always get a new
>ccache. I don't see this happening with origin/master, sorry.
I thought, that it is aim of cache collection to have separate ccache for
different logins.
Yes, but these were two separate logins of the same principal. I've
demonstrated the issue to Lukas in a VM to make sure we both see the
problem.