On Wed, Jun 26, 2013 at 10:23:59AM +0200, Lukas Slebodnik wrote:
On (25/06/13 11:40), Jakub Hrozek wrote:
>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.
I would like to describe situation, because I am not really sure, where is the
problem.
String "ptest\\@FOO.BAR(a)SSSD-AD.TEST" is sent to krb5_child with enterprise
An enterprise principal should never be send to the krb5_child. (There
was an issue with AD, because AD expected the canonicalization flag
explicitly set even for enterprise principals, which lead to enterprise
principals stored in SSSD's cache, but this is already fixed) Either the
principal from the SSSD cache or a constructed one (username@REALM)
should be send to krb5_child and both should not be enterprise
principals.
The principal is then expanded to an enterprise principal in krb5_child
if configured.
principal. We have empty cache collection. New ccache is created
(after login).
This new ccache has principal name "ptest(a)SSSD-AD.TEST"
Output from klist -l
#> Principal name Cache name
#> -------------- ----------
#> ptest(a)SSSD-AD.TEST DIR::/run/user/947005188/krb5cc/tktceNIlD
The principal form the cache (the canonicalized principal) is returned
to the krb5 provider together with the ccache path and should be saved
to the cache.
User decides to log on machine second time. Attribute ccacheFile has
value
"DIR:/run/user/947005188/krb5cc/", therefore we need find ccache in
collection.
With my patches, sssd uses function krb5_cc_cache_match to find ccache by
principal. krb5_cc_cache_match internally uses krb5_principal_compare_flags to
compare two principals and here is the problem
The second login should pick the principal from SSSD's cache which
should be the one from above and hence the one from the ccache file. If
configured the principal is expanded to an enterprise principal quite
early in krb5_child. So you either make the comparison before the
expansion, save the original principal in the context of the krb5_child
and use it for the comparison or if possible do the comparison already
in the krb5 provider.
HTH
bye,
Sumit
By default, krb5_principal_compare_flags is called with flags argument 0.
In this case following principals are compared
principal->data principal->realm
------------------------------
ptest SSSD-AD.TEST //Principal obtained from existing ccache
ptest(a)FOO.BAR SSSD-AD.TEST //Principal created by parsing string
I tried to call krb5_principal_compare_flags with argument
flags = KRB5_PRINCIPAL_COMPARE_ENTERPRISE and in this case following principals
were compared.
principal->data principal->realm
------------------------------
ptest SSSD-AD.TEST //Principal obtained from existing ccache
ptest FOO.BAR //Principal created by parsing string
This is a reason, why ccache can't be found in collection and
new ccache is created every time.
LS
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel