Re: [Freeipa-devel] [PATCH] kdb: check for local realm in enterprise principals

2016-07-12 Thread Petr Vobornik
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

2016-07-11 Thread Martin Babinsky

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

2016-07-07 Thread Petr Spacek
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

2016-07-07 Thread Sumit Bose
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

2016-07-07 Thread Petr Vobornik

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

2016-07-06 Thread Jakub Hrozek
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

2016-07-06 Thread Sumit Bose
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