Re: [Freeipa-devel] [PATCH] kdb: check for local realm in enterprise principals
On 07/11/2016 05:15 PM, Martin Babinsky wrote: > On 07/06/2016 07:01 PM, Sumit Bose wrote: >> Hi, >> >> although enterprise principals for trusted domains now are working as >> expected they do not work for the local domain: >> >> # kinit -E admin@IPA.DEVEL >> kinit: Client 'admin\@IPA.DEVEL@IPA.DEVEL' not found in Kerberos >> database while getting initial credentials >> >> Attached patch handles this case. It is not that nice because of the >> duplication of ipadb_fetch_principals() and ipadb_find_principal(). But >> I think there was a reason I do not remember why we didn't check for >> enterprise principals before checking the local database. If there is no >> such reason it might make sense to check for enterprise principals >> before doing the lookup. Please let me know if I should change the patch >> accordingly or if the current version is ok, >> >> bye, >> Sumit >> >> >> > Code looks ok to me and the patch fixes the issue, so ACK. > master: * 6d6da6b281173737bd31ba4845af11a097846c05 kdb: check for local realm in enterprise principals -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] kdb: check for local realm in enterprise principals
On 07/06/2016 07:01 PM, Sumit Bose wrote: Hi, although enterprise principals for trusted domains now are working as expected they do not work for the local domain: # kinit -E admin@IPA.DEVEL kinit: Client 'admin\@IPA.DEVEL@IPA.DEVEL' not found in Kerberos database while getting initial credentials Attached patch handles this case. It is not that nice because of the duplication of ipadb_fetch_principals() and ipadb_find_principal(). But I think there was a reason I do not remember why we didn't check for enterprise principals before checking the local database. If there is no such reason it might make sense to check for enterprise principals before doing the lookup. Please let me know if I should change the patch accordingly or if the current version is ok, bye, Sumit Code looks ok to me and the patch fixes the issue, so ACK. -- Martin^3 Babinsky -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] kdb: check for local realm in enterprise principals
On 7.7.2016 13:52, Sumit Bose wrote: > On Thu, Jul 07, 2016 at 01:31:03PM +0200, Petr Vobornik wrote: >> On 07/06/2016 07:01 PM, Sumit Bose wrote: >>> Hi, >>> >>> although enterprise principals for trusted domains now are working as >>> expected they do not work for the local domain: >>> >>> # kinit -E admin@IPA.DEVEL >>> kinit: Client 'admin\@IPA.DEVEL@IPA.DEVEL' not found in Kerberos >>> database while getting initial credentials >>> >>> Attached patch handles this case. It is not that nice because of the >>> duplication of ipadb_fetch_principals() and ipadb_find_principal(). But >>> I think there was a reason I do not remember why we didn't check for >>> enterprise principals before checking the local database. If there is no >>> such reason it might make sense to check for enterprise principals >>> before doing the lookup. Please let me know if I should change the patch >>> accordingly or if the current version is ok, I can't see the reason why we should not allow enterprise principals ... Personally I like rule of thumb 'design is not documented -> change it as you see fit & document it this time'. Petr^2 Spacek >>> >>> bye, >>> Sumit >>> >> >> Hi Sumit, >> >> thanks for the patch. This patch should have a ticket. It will help >> downstream planning. > > sure, I created https://fedorahosted.org/freeipa/ticket/6036. Please > clone it to suitable downstream tickets. > > Please note that we didn't released a patch for SSSD to enable enterprise > principals automatically if the IPA server (should) support them because > of this issues. Since 4.4.0 is already released I think we have to wait > on the SSSD side until a new FreeIPA version with a fix is released. -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] kdb: check for local realm in enterprise principals
On Thu, Jul 07, 2016 at 01:31:03PM +0200, Petr Vobornik wrote: > On 07/06/2016 07:01 PM, Sumit Bose wrote: > > Hi, > > > > although enterprise principals for trusted domains now are working as > > expected they do not work for the local domain: > > > > # kinit -E admin@IPA.DEVEL > > kinit: Client 'admin\@IPA.DEVEL@IPA.DEVEL' not found in Kerberos > > database while getting initial credentials > > > > Attached patch handles this case. It is not that nice because of the > > duplication of ipadb_fetch_principals() and ipadb_find_principal(). But > > I think there was a reason I do not remember why we didn't check for > > enterprise principals before checking the local database. If there is no > > such reason it might make sense to check for enterprise principals > > before doing the lookup. Please let me know if I should change the patch > > accordingly or if the current version is ok, > > > > bye, > > Sumit > > > > Hi Sumit, > > thanks for the patch. This patch should have a ticket. It will help > downstream planning. sure, I created https://fedorahosted.org/freeipa/ticket/6036. Please clone it to suitable downstream tickets. Please note that we didn't released a patch for SSSD to enable enterprise principals automatically if the IPA server (should) support them because of this issues. Since 4.4.0 is already released I think we have to wait on the SSSD side until a new FreeIPA version with a fix is released. bye, Sumit > > -- > Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] kdb: check for local realm in enterprise principals
On 07/06/2016 07:01 PM, Sumit Bose wrote: Hi, although enterprise principals for trusted domains now are working as expected they do not work for the local domain: # kinit -E admin@IPA.DEVEL kinit: Client 'admin\@IPA.DEVEL@IPA.DEVEL' not found in Kerberos database while getting initial credentials Attached patch handles this case. It is not that nice because of the duplication of ipadb_fetch_principals() and ipadb_find_principal(). But I think there was a reason I do not remember why we didn't check for enterprise principals before checking the local database. If there is no such reason it might make sense to check for enterprise principals before doing the lookup. Please let me know if I should change the patch accordingly or if the current version is ok, bye, Sumit Hi Sumit, thanks for the patch. This patch should have a ticket. It will help downstream planning. -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] kdb: check for local realm in enterprise principals
On Wed, Jul 06, 2016 at 07:01:50PM +0200, Sumit Bose wrote: > Hi, > > although enterprise principals for trusted domains now are working as > expected they do not work for the local domain: > > # kinit -E admin@IPA.DEVEL > > > kinit: Client 'admin\@IPA.DEVEL@IPA.DEVEL' not found in Kerberos database > while getting initial credentials > > Attached patch handles this case. It is not that nice because of the > duplication of ipadb_fetch_principals() and ipadb_find_principal(). But > I think there was a reason I do not remember why we didn't check for > enterprise principals before checking the local database. If there is no > such reason it might make sense to check for enterprise principals > before doing the lookup. Please let me know if I should change the patch > accordingly or if the current version is ok, > > bye, > Sumit > The patch fixes IPA logins for me, so functional ACK, but I'm not sure I know enough about the code to actually review the code.. -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [PATCH] kdb: check for local realm in enterprise principals
Hi, although enterprise principals for trusted domains now are working as expected they do not work for the local domain: # kinit -E admin@IPA.DEVEL kinit: Client 'admin\@IPA.DEVEL@IPA.DEVEL' not found in Kerberos database while getting initial credentials Attached patch handles this case. It is not that nice because of the duplication of ipadb_fetch_principals() and ipadb_find_principal(). But I think there was a reason I do not remember why we didn't check for enterprise principals before checking the local database. If there is no such reason it might make sense to check for enterprise principals before doing the lookup. Please let me know if I should change the patch accordingly or if the current version is ok, bye, Sumit From a1ca7928148a58a1ac61f6d418750200866a4a63 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Wed, 6 Jul 2016 17:29:37 +0200 Subject: [PATCH] kdb: check for local realm in enterprise principals --- daemons/ipa-kdb/ipa_kdb_principals.c | 52 +++- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c index 6cdfa909452a4b55912b2a5a74648abd2053482a..5b80909475565d6bb4fa8cba67629094daf51eb3 100644 --- a/daemons/ipa-kdb/ipa_kdb_principals.c +++ b/daemons/ipa-kdb/ipa_kdb_principals.c @@ -1198,30 +1198,58 @@ krb5_error_code ipadb_get_principal(krb5_context kcontext, /* skip '@' and use part after '@' as an enterprise realm for comparison */ realm++; -kerr = ipadb_is_princ_from_trusted_realm(kcontext, - realm, - upn->length - (realm - upn->data), - &trusted_realm); -if (kerr == 0) { -kentry = calloc(1, sizeof(krb5_db_entry)); -if (!kentry) { +/* check for our realm */ +if (strncasecmp(ipactx->realm, realm, +upn->length - (realm - upn->data)) == 0) { +/* it looks like it is ok to use malloc'ed strings as principal */ +krb5_free_unparsed_name(kcontext, principal); +principal = strndup((const char *) upn->data, upn->length); +if (principal == NULL) { kerr = ENOMEM; goto done; } -kerr = krb5_parse_name(kcontext, principal, - &kentry->princ); + +ldap_msgfree(res); +res = NULL; +kerr = ipadb_fetch_principals(ipactx, flags, principal, &res); if (kerr != 0) { goto done; } -kerr = krb5_set_principal_realm(kcontext, kentry->princ, trusted_realm); +kerr = ipadb_find_principal(kcontext, flags, res, &principal, +&lentry); if (kerr != 0) { goto done; } -*entry = kentry; +} else { + +kerr = ipadb_is_princ_from_trusted_realm(kcontext, + realm, + upn->length - (realm - upn->data), + &trusted_realm); +if (kerr == 0) { +kentry = calloc(1, sizeof(krb5_db_entry)); +if (!kentry) { +kerr = ENOMEM; +goto done; +} +kerr = krb5_parse_name(kcontext, principal, + &kentry->princ); +if (kerr != 0) { +goto done; +} + +kerr = krb5_set_principal_realm(kcontext, kentry->princ, trusted_realm); +if (kerr != 0) { +goto done; +} +*entry = kentry; +} +goto done; } +} else { +goto done; } -goto done; } kerr = ipadb_parse_ldap_entry(kcontext, principal, lentry, entry, &pol); -- 2.4.11 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code