[Freeipa-devel] Move 4.1.5 tickets to 4.2.1
With FreeIPA 4.2.0 released, I think we can move all the 4.1.5 tickets to FreeIPA 4.2.1 bucket and the fixes be based on that release. I would only do exception if some of the fixes are critical for platforms that did not adopt FreeIPA 4.2.0 yet (most of them ;-), but I am not aware of such tickets. -- Martin Kosek Supervisor, Software Engineering - Identity Management Team Red Hat Inc. -- 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] 4.2: ipa.ipaserver.install.cainstance.CAInstance: CRITICAL Failed to restart the Dogtag instance.
On Tue, Jul 28, 2015 at 03:56:47PM +0200, Jan Pazdziora wrote: > > INFO: Server startup in 5444 ms > INFO: Server startup in 5936 ms > INFO: Server startup in 5804 ms Running netstat at the time when the tomcat should have restarted and be ready shows # /usr/bin/netstat -tln Active Internet connections (only servers) Proto Recv-Q Send-Q Local Address Foreign Address State tcp6 0 0 127.0.0.1:8005 :::*LISTEN tcp6 0 0 :::389 :::*LISTEN tcp6 0 0 127.0.0.1:8009 :::*LISTEN tcp6 0 0 :::8443 :::*LISTEN The :::8080 is missing. Will try to figure out what causes 8443 listen to happen but not 8080. -- Jan Pazdziora Senior Principal Software Engineer, Identity Management Engineering, Red Hat -- 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] Replace stageuser-add --from-delete with user-undel --to-staged
On 28/07/15 12:34, Jan Cholasta wrote: Dne 28.7.2015 v 11:36 Lenka Doudova napsal(a): Dne 28.7.2015 v 11:27 Jan Cholasta napsal(a): Dne 27.7.2015 v 17:59 Martin Basti napsal(a): On 23/07/15 14:43, Martin Basti wrote: Hello, I tried to fix #5145 and I partially succeeded. However, I cannot fix this part of ticket, where user is prompted to write name and surname. $ ipa stageuser-add tuser --from-delete First name: this will be ignored Last name: this will be also ignored Added stage user "tuser" As the first name and last name are mandatory attributes of stageuser-add command, but they are not needed by when the --from-delete option is used. I would like to ask how to fix this issue, IMO this will be huge hack in internal API. Or should we just document this bug as known issue (thierry wrote that this is not use case that should be used often)? The best solution would be separate command, but this idea was rejected in thread "[Freeipa-devel] User life cycle: question regarding the design" Regards Martin^2 Hello, as was mentioned before, we have issue with current internal API and the stageuser-add --from-delete command. We discussed this today, and we did not find a nice way how to fix it, so we propose this (which is IMO the best solution): * stageuser-add --from-delete should be deprecated +1 * create new option for user-undel: used-undel --to-staged (or create new command) that will handle moving deleted users to staged area as --from-delete did. Make it new command please. Instead of stageuser-add and option --from-delete, which work totally different, the command user-undel does similar operation than stage-user --from-delete, it just uses different container. NACK on stuffing everything into a single command just because it does something similar. How about making it a 'stageuser-undel'? The 'user-undel' moves preserved user to active, so the 'stageuser-undel' would move preserved to staged. The action is similar, but has slightly different specifics (which attributes are preserved etc.), and for me the 'stageuser-undel' feels more natural than 'user-undel --to-staged' since it's basically the same as there is 'stageuser-add' for creating a staged user, not 'user-add --to-staged'. It would be in the same style as all the other commands concerning operations with users in staged container. Well, user-undel is the opposite of user-del, and stageuser-undel should be the opposite of stageuser-del. The stageuser-undel you are suggesting is not. Also I'm not sure if we want to (always) remove the deleted user once a staged user is created from it, but -undel behaves like that. I don't think the command should be limited to deleted users only. Active and deleted users share the same namespace, so it is an arbitrary limitation. I think that what we are looking for is the opposite of stageuser-activate. So maybe user-stage? Can we use stageuser-from-deleted ? -- Martin Basti -- 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] Replace stageuser-add --from-delete with user-undel --to-staged
Dne 28.7.2015 v 16:54 Martin Basti napsal(a): On 28/07/15 12:34, Jan Cholasta wrote: Dne 28.7.2015 v 11:36 Lenka Doudova napsal(a): Dne 28.7.2015 v 11:27 Jan Cholasta napsal(a): Dne 27.7.2015 v 17:59 Martin Basti napsal(a): On 23/07/15 14:43, Martin Basti wrote: Hello, I tried to fix #5145 and I partially succeeded. However, I cannot fix this part of ticket, where user is prompted to write name and surname. $ ipa stageuser-add tuser --from-delete First name: this will be ignored Last name: this will be also ignored Added stage user "tuser" As the first name and last name are mandatory attributes of stageuser-add command, but they are not needed by when the --from-delete option is used. I would like to ask how to fix this issue, IMO this will be huge hack in internal API. Or should we just document this bug as known issue (thierry wrote that this is not use case that should be used often)? The best solution would be separate command, but this idea was rejected in thread "[Freeipa-devel] User life cycle: question regarding the design" Regards Martin^2 Hello, as was mentioned before, we have issue with current internal API and the stageuser-add --from-delete command. We discussed this today, and we did not find a nice way how to fix it, so we propose this (which is IMO the best solution): * stageuser-add --from-delete should be deprecated +1 * create new option for user-undel: used-undel --to-staged (or create new command) that will handle moving deleted users to staged area as --from-delete did. Make it new command please. Instead of stageuser-add and option --from-delete, which work totally different, the command user-undel does similar operation than stage-user --from-delete, it just uses different container. NACK on stuffing everything into a single command just because it does something similar. How about making it a 'stageuser-undel'? The 'user-undel' moves preserved user to active, so the 'stageuser-undel' would move preserved to staged. The action is similar, but has slightly different specifics (which attributes are preserved etc.), and for me the 'stageuser-undel' feels more natural than 'user-undel --to-staged' since it's basically the same as there is 'stageuser-add' for creating a staged user, not 'user-add --to-staged'. It would be in the same style as all the other commands concerning operations with users in staged container. Well, user-undel is the opposite of user-del, and stageuser-undel should be the opposite of stageuser-del. The stageuser-undel you are suggesting is not. Also I'm not sure if we want to (always) remove the deleted user once a staged user is created from it, but -undel behaves like that. I don't think the command should be limited to deleted users only. Active and deleted users share the same namespace, so it is an arbitrary limitation. I think that what we are looking for is the opposite of stageuser-activate. So maybe user-stage? Can we use stageuser-from-deleted ? "from-deleted" is not a verb and like I said, restricting the command to deleted users only is rather arbitrary. -- Jan Cholasta -- 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] Replace stageuser-add --from-delete with user-undel --to-staged
On 07/28/2015 04:54 PM, Martin Basti wrote: On 28/07/15 12:34, Jan Cholasta wrote: Dne 28.7.2015 v 11:36 Lenka Doudova napsal(a): Dne 28.7.2015 v 11:27 Jan Cholasta napsal(a): Dne 27.7.2015 v 17:59 Martin Basti napsal(a): On 23/07/15 14:43, Martin Basti wrote: Hello, I tried to fix #5145 and I partially succeeded. However, I cannot fix this part of ticket, where user is prompted to write name and surname. $ ipa stageuser-add tuser --from-delete First name: this will be ignored Last name: this will be also ignored Added stage user "tuser" As the first name and last name are mandatory attributes of stageuser-add command, but they are not needed by when the --from-delete option is used. I would like to ask how to fix this issue, IMO this will be huge hack in internal API. Or should we just document this bug as known issue (thierry wrote that this is not use case that should be used often)? The best solution would be separate command, but this idea was rejected in thread "[Freeipa-devel] User life cycle: question regarding the design" Regards Martin^2 Hello, as was mentioned before, we have issue with current internal API and the stageuser-add --from-delete command. We discussed this today, and we did not find a nice way how to fix it, so we propose this (which is IMO the best solution): * stageuser-add --from-delete should be deprecated +1 * create new option for user-undel: used-undel --to-staged (or create new command) that will handle moving deleted users to staged area as --from-delete did. Make it new command please. Instead of stageuser-add and option --from-delete, which work totally different, the command user-undel does similar operation than stage-user --from-delete, it just uses different container. NACK on stuffing everything into a single command just because it does something similar. How about making it a 'stageuser-undel'? The 'user-undel' moves preserved user to active, so the 'stageuser-undel' would move preserved to staged. The action is similar, but has slightly different specifics (which attributes are preserved etc.), and for me the 'stageuser-undel' feels more natural than 'user-undel --to-staged' since it's basically the same as there is 'stageuser-add' for creating a staged user, not 'user-add --to-staged'. It would be in the same style as all the other commands concerning operations with users in staged container. Well, user-undel is the opposite of user-del, and stageuser-undel should be the opposite of stageuser-del. The stageuser-undel you are suggesting is not. Also I'm not sure if we want to (always) remove the deleted user once a staged user is created from it, but -undel behaves like that. I don't think the command should be limited to deleted users only. Active and deleted users share the same namespace, so it is an arbitrary limitation. I think that what we are looking for is the opposite of stageuser-activate. So maybe user-stage? Can we use stageuser-from-deleted ? user-stage sounds better to me than stageuser-from-deleted -- 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 020] Change internal rsa_(public|private)_key variable names
On Tue, 2015-07-28 at 16:18 +0200, Christian Heimes wrote: > In two places the vault plugin refers to rsa public or rsa private key > although the code can handle just any kind of asymmetric algorithms, > e.g. ECDSA. The patch just renames the occurences to avoid more > confusion in the future. LGTM. Simo. -- Simo Sorce * Red Hat, Inc * New York -- 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 020] Change internal rsa_(public|private)_key variable names
In two places the vault plugin refers to rsa public or rsa private key although the code can handle just any kind of asymmetric algorithms, e.g. ECDSA. The patch just renames the occurences to avoid more confusion in the future. From 1b09967de50aa3c73a9fcab1ff11aa6d1800bae5 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Tue, 28 Jul 2015 16:12:40 +0200 Subject: [PATCH] Change internal rsa_(public|private)_key variable names In two places the vault plugin refers to rsa public or rsa private key although the code can handle just any kind of asymmetric algorithms, e.g. ECDSA. The patch just renames the occurences to avoid more confusion in the future. --- ipalib/plugins/vault.py | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ipalib/plugins/vault.py b/ipalib/plugins/vault.py index 81197f9328c7ed890fa336f464bfcda475ac6189..a2b78f4dec143524d81a1a006733c22db0f90847 100644 --- a/ipalib/plugins/vault.py +++ b/ipalib/plugins/vault.py @@ -469,11 +469,11 @@ class vault(LDAPObject): return fernet.encrypt(data) elif public_key: -rsa_public_key = load_pem_public_key( +public_key_obj = load_pem_public_key( data=public_key, backend=default_backend() ) -return rsa_public_key.encrypt( +return public_key_obj.encrypt( data, padding.OAEP( mgf=padding.MGF1(algorithm=hashes.SHA1()), @@ -496,12 +496,12 @@ class vault(LDAPObject): elif private_key: try: -rsa_private_key = load_pem_private_key( +private_key_obj = load_pem_private_key( data=private_key, password=None, backend=default_backend() ) -return rsa_private_key.decrypt( +return private_key_obj.decrypt( data, padding.OAEP( mgf=padding.MGF1(algorithm=hashes.SHA1()), -- 2.4.3 signature.asc Description: OpenPGP digital signature -- 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] 4.2: ipa.ipaserver.install.cainstance.CAInstance: CRITICAL Failed to restart the Dogtag instance.
On Tue, Jul 28, 2015 at 03:25:50PM +0300, Alexander Bokovoy wrote: > On Tue, 28 Jul 2015, Jan Pazdziora wrote: > > > >I do run it in container so it could be related, so I'm mostly looking > >for blind hints about what might have changed in the installer or > >in dogtag itself in 4.2 that could cause this. For example, did we make > >the timeout shorter? > > The timeout is 300: > >2015-07-28T11:15:42Z DEBUG wait_for_open_ports: localhost [8080, 8443] > >timeout 300 > > You can look at dogtag's catalina-.log, to see how long did it > take: > # grep 'Server startup' /var/log/pki/pki-tomcat/catalina.2015-07-24.log > INFO: Server startup in 27159 ms > INFO: Server startup in 11323 ms > INFO: Server startup in 10472 ms > INFO: Server startup in 11158 ms > INFO: Server startup in 11194 ms INFO: Server startup in 5444 ms INFO: Server startup in 5936 ms INFO: Server startup in 5804 ms -- Jan Pazdziora Senior Principal Software Engineer, Identity Management Engineering, Red Hat -- 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 149] IPA KDB: allow case in-sensitive realm in AS request
On Tue, Jul 28, 2015 at 01:42:29PM +0200, Sumit Bose wrote: > On Tue, Jul 28, 2015 at 02:26:34PM +0300, Alexander Bokovoy wrote: > > On Tue, 28 Jul 2015, Simo Sorce wrote: > > >On Tue, 2015-07-28 at 12:15 +0200, Sumit Bose wrote: > > >>On Wed, Jul 22, 2015 at 09:41:51AM -0400, Simo Sorce wrote: > > >>> - Original Message - > > >>> > From: "Sumit Bose" > > >>> > To: "freeipa-devel" > > >>> > Sent: Tuesday, July 21, 2015 7:41:14 AM > > >>> > Subject: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive > > >>> > realm in AS request > > >>> > > > >>> > Hi, > > >>> > > > >>> > this patch is my suggestion to solve > > >>> > https://fedorahosted.org/freeipa/ticket/4844 . > > >>> > > > >>> > The original issue in the ticket has two part. One is a loop in > > >>> > libkrb5 > > >>> > which is already fixed. The other is to handle canonicalization > > >>> > better. > > >>> > > >>> Sorry Sumit, > > >>> I see several issues with this patck. > > >>> > > >>> first of all you should really not change ipadb_get_principal(), that's > > >>> the > > >>> wrong place to apply your logic. > > >>> > > >>> To support searching for the realm name case-insensitively all we > > >>> should do > > >>> is to always forcibly upper case the realm name at the same time we > > >>> build the > > >>> filter (in ipadb_fetch_principals(), if canonicalization was requested. > > >>> Because we will never store (code to prevent that should probably be > > >>> dded with > > >>> this patch) a realm name that is not all caps. > > >>> Then the post search matches should be done straight within > > >>> ipadb_find_principal(). > > >>> > > >>> > The general way to allow canonicalization on a principal is to add the > > >>> > attributes 'krbcanonicalname'[1] and 'ipakrbprincipalalias' together > > >>> > with the objectclass 'ipaKrbPrincipal' to the user object. > > >>> > > >>> We have already a ticket open since long to remove krbprincipalalias, > > >>> it was > > >>> a mistake to add it and any patch that depends on it will be nacked by > > >>> me. > > >>> We need to use krbPrincipalName and krbCanonicalName. > > >>> > > >>> > Then the IPA > > >>> > KDB backend will use 'ipakrbprincipalalias' for case in-sensitive > > >>> > matches and the principal from 'krbcanonicalname' will be the > > >>> > canonical > > >>> > principal used further on. The 'krbPrincipalName' is not suitable for > > >>> > either because it has caseExact* matching rules and is a multivalue > > >>> > attribute [2]. > > >>> > > >>> Case-exact match is a problem only if we do not canonicalize names when > > >>> storing > > >>> them, otherwise all you need to do is store a "search form" in > > >>> krbPrincipalName > > >>> and always change searches to that form (forcibly upper case realm, > > >>> forcibly > > >>> lowercase components) when canonicalization is requested. > > >>> > > >>> Additionally in the patch you are using stcasecmp(), that function is > > >>> not > > >>> acceptable, look at ipadb_find_principal() and you'll see we use > > >>> ulc_casecmp() > > >>> there. > > >>> Also modyfing the principal before searching is done wrong (you use > > >>> strchr() > > >>> to find the @ sign, but you could find an @ in the components this way, > > >>> you > > >>> should use strrchr() at the very least), and is dangerous if done > > >>> outside of > > >>> the inner functions because then we never have a way to know the > > >>> original > > >>> form should it be needed. In any case as said above realm should be > > >>> forcibly > > >>> uppercase, given a flag in the escape function instead. > > >> > > >>Thank for for the review and the comments. > > >> > > >>I changed the patch as you suggested to upper-case the realm in the > > >>escape function if the flag is set. > > >> > > >>I didn't add any checks to make sure that the realm of newly added > > >>principal attributes is always upper case. Since the attributes can be > > >>added via various ways I think the check should happen on the DS level > > > > > >We should indeed intercept add/modify operations and see if they try to > > >set krbPrincipalName/krbCanonicalName and then validate the name. > > >Return unwilling to perform if the case of the realm is different (or > > >fix it on the fly, up for discussion) from the default case as > > >configured in the server. > > Will break trusts -- ipasam does add these principals for krbtgt/IPA@AD. > > > > >>but I see this more in the context of full canonicalization fix covered > > >>by https://fedorahosted.org/freeipa/ticket/3864 . If you think this is a > > >>requirement for the patch attached I would suggest to drop > > >>https://fedorahosted.org/freeipa/ticket/4844 and solve it together with > > >>#3864. > > > > > >We should clsoe 4844 as fixed upstream (there *was* a bug in libkrb5). > > >I commented on #3864 about what we can do, and we can also avoid > > >changing the schema. > > Yep. > > > > >So on the new patches, what does "unify" means ? I
Re: [Freeipa-devel] 4.2: ipa.ipaserver.install.cainstance.CAInstance: CRITICAL Failed to restart the Dogtag instance.
On Tue, 28 Jul 2015, Jan Pazdziora wrote: Hello, ever since I started to run FreeIPA 4.2 installations (from upstream copr repo on Fedora 22), I often (but not always) get [13/25]: setting audit signing renewal to 2 years [14/25]: restarting certificate server ipa.ipaserver.install.cainstance.CAInstance: CRITICAL Failed to restart the Dogtag instance.See the installation log for details. [15/25]: requesting RA certificate from CA [error] error: [Errno 111] Connection refused In the ipaserver-install.log, there is 2015-07-28T11:15:42Z DEBUG Starting external process 2015-07-28T11:15:42Z DEBUG args='/bin/systemctl' 'is-active' 'pki-tomcatd@pki-tomcat.service' 2015-07-28T11:15:42Z DEBUG Process finished, return code=0 2015-07-28T11:15:42Z DEBUG stdout=active 2015-07-28T11:15:42Z DEBUG stderr= 2015-07-28T11:15:42Z DEBUG wait_for_open_ports: localhost [8080, 8443] timeout 300 2015-07-28T11:20:42Z DEBUG Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/ipaserver/install/dogtaginstance.py", line 183, in rest art_instance self.restart(self.dogtag_constants.PKI_INSTANCE_NAME) File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py", line 316, in restart self.service.restart(instance_name, capture_output=capture_output, wait=wait) File "/usr/lib/python2.7/site-packages/ipaplatform/redhat/services.py", line 250, in restart instance_name, capture_output=capture_output, wait=wait) File "/usr/lib/python2.7/site-packages/ipaplatform/base/services.py", line 317, in restart self.wait_for_open_ports(self.service_instance(instance_name)) File "/usr/lib/python2.7/site-packages/ipaplatform/base/services.py", line 272, in wait_for_op en_ports self.api.env.startup_timeout) File "/usr/lib/python2.7/site-packages/ipapython/ipautil.py", line 1180, in wait_for_open_port s raise socket.timeout("Timeout exceeded") timeout: Timeout exceeded I do run it in container so it could be related, so I'm mostly looking for blind hints about what might have changed in the installer or in dogtag itself in 4.2 that could cause this. For example, did we make the timeout shorter? The timeout is 300: 2015-07-28T11:15:42Z DEBUG wait_for_open_ports: localhost [8080, 8443] timeout 300 You can look at dogtag's catalina-.log, to see how long did it take: # grep 'Server startup' /var/log/pki/pki-tomcat/catalina.2015-07-24.log INFO: Server startup in 27159 ms INFO: Server startup in 11323 ms INFO: Server startup in 10472 ms INFO: Server startup in 11158 ms INFO: Server startup in 11194 ms -- / Alexander Bokovoy -- 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] 4.2: ipa.ipaserver.install.cainstance.CAInstance: CRITICAL Failed to restart the Dogtag instance.
Hello, ever since I started to run FreeIPA 4.2 installations (from upstream copr repo on Fedora 22), I often (but not always) get [13/25]: setting audit signing renewal to 2 years [14/25]: restarting certificate server ipa.ipaserver.install.cainstance.CAInstance: CRITICAL Failed to restart the Dogtag instance.See the installation log for details. [15/25]: requesting RA certificate from CA [error] error: [Errno 111] Connection refused In the ipaserver-install.log, there is 2015-07-28T11:15:42Z DEBUG Starting external process 2015-07-28T11:15:42Z DEBUG args='/bin/systemctl' 'is-active' 'pki-tomcatd@pki-tomcat.service' 2015-07-28T11:15:42Z DEBUG Process finished, return code=0 2015-07-28T11:15:42Z DEBUG stdout=active 2015-07-28T11:15:42Z DEBUG stderr= 2015-07-28T11:15:42Z DEBUG wait_for_open_ports: localhost [8080, 8443] timeout 300 2015-07-28T11:20:42Z DEBUG Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/ipaserver/install/dogtaginstance.py", line 183, in rest art_instance self.restart(self.dogtag_constants.PKI_INSTANCE_NAME) File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py", line 316, in restart self.service.restart(instance_name, capture_output=capture_output, wait=wait) File "/usr/lib/python2.7/site-packages/ipaplatform/redhat/services.py", line 250, in restart instance_name, capture_output=capture_output, wait=wait) File "/usr/lib/python2.7/site-packages/ipaplatform/base/services.py", line 317, in restart self.wait_for_open_ports(self.service_instance(instance_name)) File "/usr/lib/python2.7/site-packages/ipaplatform/base/services.py", line 272, in wait_for_op en_ports self.api.env.startup_timeout) File "/usr/lib/python2.7/site-packages/ipapython/ipautil.py", line 1180, in wait_for_open_port s raise socket.timeout("Timeout exceeded") timeout: Timeout exceeded I do run it in container so it could be related, so I'm mostly looking for blind hints about what might have changed in the installer or in dogtag itself in 4.2 that could cause this. For example, did we make the timeout shorter? -- Jan Pazdziora Senior Principal Software Engineer, Identity Management Engineering, Red Hat -- 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 0050] ACI plugin: correctly parse bind rules enclosed in parentheses
On 28/07/15 13:33, Martin Babinsky wrote: On 07/27/2015 05:10 PM, Martin Basti wrote: On 23/07/15 16:06, Martin Babinsky wrote: This is a quick fix for https://fedorahosted.org/freeipa/ticket/5037 NACK I do not like your change in first regexp too much. Can you try this instead? PermPat = re.compile(r'(\w+)\s*\(([^()]*)\)\s*(.*)', re.UNICODE) This just removes '(' and ') ' from pattern and accept all other characters. -- Martin Basti Attaching updated patch. ACK -- Martin Basti -- 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 149] IPA KDB: allow case in-sensitive realm in AS request
On Tue, 2015-07-28 at 14:26 +0300, Alexander Bokovoy wrote: > On Tue, 28 Jul 2015, Simo Sorce wrote: > >On Tue, 2015-07-28 at 12:15 +0200, Sumit Bose wrote: > >> On Wed, Jul 22, 2015 at 09:41:51AM -0400, Simo Sorce wrote: > >> > - Original Message - > >> > > From: "Sumit Bose" > >> > > To: "freeipa-devel" > >> > > Sent: Tuesday, July 21, 2015 7:41:14 AM > >> > > Subject: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive > >> > > realmin AS request > >> > > > >> > > Hi, > >> > > > >> > > this patch is my suggestion to solve > >> > > https://fedorahosted.org/freeipa/ticket/4844 . > >> > > > >> > > The original issue in the ticket has two part. One is a loop in libkrb5 > >> > > which is already fixed. The other is to handle canonicalization better. > >> > > >> > Sorry Sumit, > >> > I see several issues with this patck. > >> > > >> > first of all you should really not change ipadb_get_principal(), that's > >> > the > >> > wrong place to apply your logic. > >> > > >> > To support searching for the realm name case-insensitively all we should > >> > do > >> > is to always forcibly upper case the realm name at the same time we > >> > build the > >> > filter (in ipadb_fetch_principals(), if canonicalization was requested. > >> > Because we will never store (code to prevent that should probably be > >> > dded with > >> > this patch) a realm name that is not all caps. > >> > Then the post search matches should be done straight within > >> > ipadb_find_principal(). > >> > > >> > > The general way to allow canonicalization on a principal is to add the > >> > > attributes 'krbcanonicalname'[1] and 'ipakrbprincipalalias' together > >> > > with the objectclass 'ipaKrbPrincipal' to the user object. > >> > > >> > We have already a ticket open since long to remove krbprincipalalias, it > >> > was > >> > a mistake to add it and any patch that depends on it will be nacked by > >> > me. > >> > We need to use krbPrincipalName and krbCanonicalName. > >> > > >> > > Then the IPA > >> > > KDB backend will use 'ipakrbprincipalalias' for case in-sensitive > >> > > matches and the principal from 'krbcanonicalname' will be the > >> > > canonical > >> > > principal used further on. The 'krbPrincipalName' is not suitable for > >> > > either because it has caseExact* matching rules and is a multivalue > >> > > attribute [2]. > >> > > >> > Case-exact match is a problem only if we do not canonicalize names when > >> > storing > >> > them, otherwise all you need to do is store a "search form" in > >> > krbPrincipalName > >> > and always change searches to that form (forcibly upper case realm, > >> > forcibly > >> > lowercase components) when canonicalization is requested. > >> > > >> > Additionally in the patch you are using stcasecmp(), that function is not > >> > acceptable, look at ipadb_find_principal() and you'll see we use > >> > ulc_casecmp() > >> > there. > >> > Also modyfing the principal before searching is done wrong (you use > >> > strchr() > >> > to find the @ sign, but you could find an @ in the components this way, > >> > you > >> > should use strrchr() at the very least), and is dangerous if done > >> > outside of > >> > the inner functions because then we never have a way to know the original > >> > form should it be needed. In any case as said above realm should be > >> > forcibly > >> > uppercase, given a flag in the escape function instead. > >> > >> Thank for for the review and the comments. > >> > >> I changed the patch as you suggested to upper-case the realm in the > >> escape function if the flag is set. > >> > >> I didn't add any checks to make sure that the realm of newly added > >> principal attributes is always upper case. Since the attributes can be > >> added via various ways I think the check should happen on the DS level > > > >We should indeed intercept add/modify operations and see if they try to > >set krbPrincipalName/krbCanonicalName and then validate the name. > >Return unwilling to perform if the case of the realm is different (or > >fix it on the fly, up for discussion) from the default case as > >configured in the server. > Will break trusts -- ipasam does add these principals for krbtgt/IPA@AD. You misunderstood, we compare case-insenstively and adjust the case (or simply always uppercase). We do not refuse to add realm names that are completely different, I know about the cross realm principals :) > >> but I see this more in the context of full canonicalization fix covered > >> by https://fedorahosted.org/freeipa/ticket/3864 . If you think this is a > >> requirement for the patch attached I would suggest to drop > >> https://fedorahosted.org/freeipa/ticket/4844 and solve it together with > >> #3864. > > > >We should clsoe 4844 as fixed upstream (there *was* a bug in libkrb5). > >I commented on #3864 about what we can do, and we can also avoid > >changing the schema. > Yep. > > >So on the new patches, what does "unify" means ? I do not get what it > >mean
[Freeipa-devel] [PATCH] 906 webui: fix regressions failed auth messages
1. after logout, krb auth no longer shows "session expired" but correct "Authentication with Kerberos failed". 2. "The password or username you entered is incorrect." is showed on failed forms-based auth. https://fedorahosted.org/freeipa/ticket/5163 -- Petr Vobornik From 88181b081dc696ec5c7709081b4b7abf72a5015a Mon Sep 17 00:00:00 2001 From: Petr Vobornik Date: Tue, 28 Jul 2015 14:01:34 +0200 Subject: [PATCH] webui: fix regressions failed auth messages 1. after logout, krb auth no longer shows "session expired" but correct "Authentication with Kerberos failed". 2. "The password or username you entered is incorrect." is showed on failed forms-based auth. https://fedorahosted.org/freeipa/ticket/5163 --- install/ui/src/freeipa/ipa.js | 8 install/ui/src/freeipa/widgets/LoginScreen.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/install/ui/src/freeipa/ipa.js b/install/ui/src/freeipa/ipa.js index 75dd73c379815a0e0e1dc2c4d786fdcf3be7c1b0..ef7fcfaee873d97d96630b72365ecffe6b08 100644 --- a/install/ui/src/freeipa/ipa.js +++ b/install/ui/src/freeipa/ipa.js @@ -32,6 +32,7 @@ define([ './json2', './_base/i18n', './auth', +'./config', './datetime', './metadata', './builder', @@ -41,7 +42,8 @@ define([ './util', 'exports' ], function(declare, Deferred, Evented, keys, topic, $, JSON, i18n, auth, -datetime, metadata_provider, builder, reg, rpc, text, util, exports) { +config, datetime, metadata_provider, builder, reg, rpc, text, +util, exports) { /** * @class @@ -127,11 +129,9 @@ var IPA = function () { // if current path matches live server path, use live data if (that.url && window.location.pathname.substring(0, that.url.length) === that.url) { that.json_url = params.url || '/ipa/session/json'; -that.login_url = params.url || '/ipa/session/login_kerberos'; } else { // otherwise use fixtures that.json_path = params.url || "test/data"; -// that.login_url is not needed for fixtures } $.ajaxSetup(that.ajax_options); @@ -377,7 +377,7 @@ IPA.get_credentials = function() { } var request = { -url: IPA.login_url, +url: config.krb_login_url, cache: false, type: "GET", success: success_handler, diff --git a/install/ui/src/freeipa/widgets/LoginScreen.js b/install/ui/src/freeipa/widgets/LoginScreen.js index fb7c6c34d9c1c7115dd95809f3a39de488eb..eb95b9161f05eeac1ec9aed286c9730dada85d59 100644 --- a/install/ui/src/freeipa/widgets/LoginScreen.js +++ b/install/ui/src/freeipa/widgets/LoginScreen.js @@ -232,8 +232,8 @@ define(['dojo/_base/declare', this.set('view', 'reset'); val_summary.add_info('login', this.password_expired); } else { +password_f.set_value(''); val_summary.add_error('login', this.form_auth_failed); -password_f.set_value(''); } })); }, -- 2.4.3 -- 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 149] IPA KDB: allow case in-sensitive realm in AS request
On Tue, Jul 28, 2015 at 02:26:34PM +0300, Alexander Bokovoy wrote: > On Tue, 28 Jul 2015, Simo Sorce wrote: > >On Tue, 2015-07-28 at 12:15 +0200, Sumit Bose wrote: > >>On Wed, Jul 22, 2015 at 09:41:51AM -0400, Simo Sorce wrote: > >>> - Original Message - > >>> > From: "Sumit Bose" > >>> > To: "freeipa-devel" > >>> > Sent: Tuesday, July 21, 2015 7:41:14 AM > >>> > Subject: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive > >>> > realm in AS request > >>> > > >>> > Hi, > >>> > > >>> > this patch is my suggestion to solve > >>> > https://fedorahosted.org/freeipa/ticket/4844 . > >>> > > >>> > The original issue in the ticket has two part. One is a loop in libkrb5 > >>> > which is already fixed. The other is to handle canonicalization better. > >>> > >>> Sorry Sumit, > >>> I see several issues with this patck. > >>> > >>> first of all you should really not change ipadb_get_principal(), that's > >>> the > >>> wrong place to apply your logic. > >>> > >>> To support searching for the realm name case-insensitively all we should > >>> do > >>> is to always forcibly upper case the realm name at the same time we build > >>> the > >>> filter (in ipadb_fetch_principals(), if canonicalization was requested. > >>> Because we will never store (code to prevent that should probably be dded > >>> with > >>> this patch) a realm name that is not all caps. > >>> Then the post search matches should be done straight within > >>> ipadb_find_principal(). > >>> > >>> > The general way to allow canonicalization on a principal is to add the > >>> > attributes 'krbcanonicalname'[1] and 'ipakrbprincipalalias' together > >>> > with the objectclass 'ipaKrbPrincipal' to the user object. > >>> > >>> We have already a ticket open since long to remove krbprincipalalias, it > >>> was > >>> a mistake to add it and any patch that depends on it will be nacked by me. > >>> We need to use krbPrincipalName and krbCanonicalName. > >>> > >>> > Then the IPA > >>> > KDB backend will use 'ipakrbprincipalalias' for case in-sensitive > >>> > matches and the principal from 'krbcanonicalname' will be the canonical > >>> > principal used further on. The 'krbPrincipalName' is not suitable for > >>> > either because it has caseExact* matching rules and is a multivalue > >>> > attribute [2]. > >>> > >>> Case-exact match is a problem only if we do not canonicalize names when > >>> storing > >>> them, otherwise all you need to do is store a "search form" in > >>> krbPrincipalName > >>> and always change searches to that form (forcibly upper case realm, > >>> forcibly > >>> lowercase components) when canonicalization is requested. > >>> > >>> Additionally in the patch you are using stcasecmp(), that function is not > >>> acceptable, look at ipadb_find_principal() and you'll see we use > >>> ulc_casecmp() > >>> there. > >>> Also modyfing the principal before searching is done wrong (you use > >>> strchr() > >>> to find the @ sign, but you could find an @ in the components this way, > >>> you > >>> should use strrchr() at the very least), and is dangerous if done outside > >>> of > >>> the inner functions because then we never have a way to know the original > >>> form should it be needed. In any case as said above realm should be > >>> forcibly > >>> uppercase, given a flag in the escape function instead. > >> > >>Thank for for the review and the comments. > >> > >>I changed the patch as you suggested to upper-case the realm in the > >>escape function if the flag is set. > >> > >>I didn't add any checks to make sure that the realm of newly added > >>principal attributes is always upper case. Since the attributes can be > >>added via various ways I think the check should happen on the DS level > > > >We should indeed intercept add/modify operations and see if they try to > >set krbPrincipalName/krbCanonicalName and then validate the name. > >Return unwilling to perform if the case of the realm is different (or > >fix it on the fly, up for discussion) from the default case as > >configured in the server. > Will break trusts -- ipasam does add these principals for krbtgt/IPA@AD. > > >>but I see this more in the context of full canonicalization fix covered > >>by https://fedorahosted.org/freeipa/ticket/3864 . If you think this is a > >>requirement for the patch attached I would suggest to drop > >>https://fedorahosted.org/freeipa/ticket/4844 and solve it together with > >>#3864. > > > >We should clsoe 4844 as fixed upstream (there *was* a bug in libkrb5). > >I commented on #3864 about what we can do, and we can also avoid > >changing the schema. > Yep. > > >So on the new patches, what does "unify" means ? I do not get what it > >means (so probably it is a poor name), I guess you may want to call it > >"canonicalization" ? (or even 'canon' to shorten it a bit). > I have same question. I tried to understand why it is called unify and > failed. I didn't want to use 'canonical' because the result will not be the canonical name in
Re: [Freeipa-devel] [PATCH 0050] ACI plugin: correctly parse bind rules enclosed in parentheses
On 07/27/2015 05:10 PM, Martin Basti wrote: On 23/07/15 16:06, Martin Babinsky wrote: This is a quick fix for https://fedorahosted.org/freeipa/ticket/5037 NACK I do not like your change in first regexp too much. Can you try this instead? PermPat = re.compile(r'(\w+)\s*\(([^()]*)\)\s*(.*)', re.UNICODE) This just removes '(' and ') ' from pattern and accept all other characters. -- Martin Basti Attaching updated patch. -- Martin^3 Babinsky From af2b35c4971a7a13217307e52384edaf7255c7a4 Mon Sep 17 00:00:00 2001 From: Martin Babinsky Date: Thu, 23 Jul 2015 15:45:35 +0200 Subject: [PATCH] ACI plugin: correctly parse bind rules enclosed in parentheses Since bind rule such as `(userdn = "ldap:///anyone";)` is also a valid statement, the ipalib ACI parser was updated to handle this case. https://fedorahosted.org/freeipa/ticket/5037 --- ipalib/aci.py | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ipalib/aci.py b/ipalib/aci.py index a55732bf19e58d8a4b36fa18bee2725d5b6584da..f78c5327dbe659240f046ae15622e798c8552829 100755 --- a/ipalib/aci.py +++ b/ipalib/aci.py @@ -26,10 +26,11 @@ import re ACIPat = re.compile(r'\(version\s+3.0\s*;\s*ac[li]\s+\"([^\"]*)\"\s*;\s*([^;]*);\s*\)', re.UNICODE) # Break the permissions/bind_rules out -PermPat = re.compile(r'(\w+)\s*\((.*)\)\s+(.*)', re.UNICODE) +PermPat = re.compile(r'(\w+)\s*\(([^()]*)\)\s*(.*)', re.UNICODE) # Break the bind rule out -BindPat = re.compile(r'([a-zA-Z0-9;\.]+)\s*(\!?=)\s*(.*)', re.UNICODE) +BindPat = re.compile(r'\(?([a-zA-Z0-9;\.]+)\s*(\!?=)\s*\"(.*)\"\)?', + re.UNICODE) ACTIONS = ["allow", "deny"] @@ -193,6 +194,9 @@ class ACI: self.target['target']['operator'] = operator def set_bindrule(self, bindrule): +if bindrule.startswith('(') != bindrule.endswith(')'): +raise SyntaxError("non-matching parentheses in bindrule") + match = BindPat.match(bindrule) if not match or len(match.groups()) < 3: raise SyntaxError, "malformed bind rule" -- 2.4.3 -- 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] Is Backend.krb part of API?
On Tue, 28 Jul 2015, Simo Sorce wrote: On Tue, 2015-07-28 at 13:55 +0300, Alexander Bokovoy wrote: On Tue, 28 Jul 2015, Petr Vobornik wrote: >On 07/28/2015 10:57 AM, Michael Šimáček wrote: >>Hi, >> >>I'm working on porting FreeIPA away from python-krbV. Backend.krb and >>KRB5_CCache classes are mere wrappers around krbV bindings, so it would >>make sense to remove them. But I found the former used in the example in >>doc/examples/python-api.py. Is it part of FreeIPA's API? Shall I provide >>some partial compatibility layer for it? (only partial because some >>methods can take krbV objects as arguments) >> >>Thank you, >>Michael Simacek >> > >Does the replacement offer API which has all the methods as the >wrappers? If so we can remove them. > >Imho we can remove Backend.krb aka ipalib/plugins/kerberos.py. It's >used only in 2 files, both are not in production. But I'm not sure >about KRB5_CCache, the wrapper has some exception logic which might be >wanted to be kept. Backend.krb can go if you provide something similar to KRB5_CCache. We need to be able to initialize ccache with that class -- either by using existing ccache (we often marshall ccache content to memcached and then unmarshall it when the same session comes back) or by using a keytab. After ccache is provided, we need to be able to query default principal of the existing ccache. We should be able to do all this with python-gssapi and the store extensions. Yep. It would be good to have a helper, though. -- / Alexander Bokovoy -- 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 149] IPA KDB: allow case in-sensitive realm in AS request
On Tue, 28 Jul 2015, Simo Sorce wrote: On Tue, 2015-07-28 at 12:15 +0200, Sumit Bose wrote: On Wed, Jul 22, 2015 at 09:41:51AM -0400, Simo Sorce wrote: > - Original Message - > > From: "Sumit Bose" > > To: "freeipa-devel" > > Sent: Tuesday, July 21, 2015 7:41:14 AM > > Subject: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive realm in AS request > > > > Hi, > > > > this patch is my suggestion to solve > > https://fedorahosted.org/freeipa/ticket/4844 . > > > > The original issue in the ticket has two part. One is a loop in libkrb5 > > which is already fixed. The other is to handle canonicalization better. > > Sorry Sumit, > I see several issues with this patck. > > first of all you should really not change ipadb_get_principal(), that's the > wrong place to apply your logic. > > To support searching for the realm name case-insensitively all we should do > is to always forcibly upper case the realm name at the same time we build the > filter (in ipadb_fetch_principals(), if canonicalization was requested. > Because we will never store (code to prevent that should probably be dded with > this patch) a realm name that is not all caps. > Then the post search matches should be done straight within ipadb_find_principal(). > > > The general way to allow canonicalization on a principal is to add the > > attributes 'krbcanonicalname'[1] and 'ipakrbprincipalalias' together > > with the objectclass 'ipaKrbPrincipal' to the user object. > > We have already a ticket open since long to remove krbprincipalalias, it was > a mistake to add it and any patch that depends on it will be nacked by me. > We need to use krbPrincipalName and krbCanonicalName. > > > Then the IPA > > KDB backend will use 'ipakrbprincipalalias' for case in-sensitive > > matches and the principal from 'krbcanonicalname' will be the canonical > > principal used further on. The 'krbPrincipalName' is not suitable for > > either because it has caseExact* matching rules and is a multivalue > > attribute [2]. > > Case-exact match is a problem only if we do not canonicalize names when storing > them, otherwise all you need to do is store a "search form" in krbPrincipalName > and always change searches to that form (forcibly upper case realm, forcibly > lowercase components) when canonicalization is requested. > > Additionally in the patch you are using stcasecmp(), that function is not > acceptable, look at ipadb_find_principal() and you'll see we use ulc_casecmp() > there. > Also modyfing the principal before searching is done wrong (you use strchr() > to find the @ sign, but you could find an @ in the components this way, you > should use strrchr() at the very least), and is dangerous if done outside of > the inner functions because then we never have a way to know the original > form should it be needed. In any case as said above realm should be forcibly > uppercase, given a flag in the escape function instead. Thank for for the review and the comments. I changed the patch as you suggested to upper-case the realm in the escape function if the flag is set. I didn't add any checks to make sure that the realm of newly added principal attributes is always upper case. Since the attributes can be added via various ways I think the check should happen on the DS level We should indeed intercept add/modify operations and see if they try to set krbPrincipalName/krbCanonicalName and then validate the name. Return unwilling to perform if the case of the realm is different (or fix it on the fly, up for discussion) from the default case as configured in the server. Will break trusts -- ipasam does add these principals for krbtgt/IPA@AD. but I see this more in the context of full canonicalization fix covered by https://fedorahosted.org/freeipa/ticket/3864 . If you think this is a requirement for the patch attached I would suggest to drop https://fedorahosted.org/freeipa/ticket/4844 and solve it together with #3864. We should clsoe 4844 as fixed upstream (there *was* a bug in libkrb5). I commented on #3864 about what we can do, and we can also avoid changing the schema. Yep. So on the new patches, what does "unify" means ? I do not get what it means (so probably it is a poor name), I guess you may want to call it "canonicalization" ? (or even 'canon' to shorten it a bit). I have same question. I tried to understand why it is called unify and failed. I think the worst case for a utf8 string is more then length*2, probably more like length*6, unless there is some guarantee around case changes that I am not aware of, that said we could probably just allocate on the stack a fixed size string of a KiB or so, the longest DNS name is 256 chars IIRC and a service name can't be that much longer, also usernames can't be arbitrarily long. So 1/2 KiB should probably be fine for a full principal name. (avoids a malloc too which is good). Yes, sounds good. A hostname label can be up to 63 characters and full domain name including dots wou
Re: [Freeipa-devel] [PATCH 0294] ULC: fix stageuser-add --from-delete command
On 23/07/15 13:46, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/5145 Patch attached. This patch fixes only first part of problem -- the traceback. Removing promt for name and surname requires too big hacks in internal API, and I'm not sure if we will be able to do that. IMO this should be separate command, I will open a discussion. Works for me, ACK. It would be better to leave the ticket open until the issue is fully resolved. -- David Kupka -- 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] Is Backend.krb part of API?
On Tue, 2015-07-28 at 13:55 +0300, Alexander Bokovoy wrote: > On Tue, 28 Jul 2015, Petr Vobornik wrote: > >On 07/28/2015 10:57 AM, Michael Šimáček wrote: > >>Hi, > >> > >>I'm working on porting FreeIPA away from python-krbV. Backend.krb and > >>KRB5_CCache classes are mere wrappers around krbV bindings, so it would > >>make sense to remove them. But I found the former used in the example in > >>doc/examples/python-api.py. Is it part of FreeIPA's API? Shall I provide > >>some partial compatibility layer for it? (only partial because some > >>methods can take krbV objects as arguments) > >> > >>Thank you, > >>Michael Simacek > >> > > > >Does the replacement offer API which has all the methods as the > >wrappers? If so we can remove them. > > > >Imho we can remove Backend.krb aka ipalib/plugins/kerberos.py. It's > >used only in 2 files, both are not in production. But I'm not sure > >about KRB5_CCache, the wrapper has some exception logic which might be > >wanted to be kept. > Backend.krb can go if you provide something similar to KRB5_CCache. We > need to be able to initialize ccache with that class -- either by using > existing ccache (we often marshall ccache content to memcached and then > unmarshall it when the same session comes back) or by using a keytab. > After ccache is provided, we need to be able to query default principal > of the existing ccache. We should be able to do all this with python-gssapi and the store extensions. Simo. -- Simo Sorce * Red Hat, Inc * New York -- 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 149] IPA KDB: allow case in-sensitive realm in AS request
On Tue, 2015-07-28 at 12:15 +0200, Sumit Bose wrote: > On Wed, Jul 22, 2015 at 09:41:51AM -0400, Simo Sorce wrote: > > - Original Message - > > > From: "Sumit Bose" > > > To: "freeipa-devel" > > > Sent: Tuesday, July 21, 2015 7:41:14 AM > > > Subject: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive > > > realm in AS request > > > > > > Hi, > > > > > > this patch is my suggestion to solve > > > https://fedorahosted.org/freeipa/ticket/4844 . > > > > > > The original issue in the ticket has two part. One is a loop in libkrb5 > > > which is already fixed. The other is to handle canonicalization better. > > > > Sorry Sumit, > > I see several issues with this patck. > > > > first of all you should really not change ipadb_get_principal(), that's the > > wrong place to apply your logic. > > > > To support searching for the realm name case-insensitively all we should do > > is to always forcibly upper case the realm name at the same time we build > > the > > filter (in ipadb_fetch_principals(), if canonicalization was requested. > > Because we will never store (code to prevent that should probably be dded > > with > > this patch) a realm name that is not all caps. > > Then the post search matches should be done straight within > > ipadb_find_principal(). > > > > > The general way to allow canonicalization on a principal is to add the > > > attributes 'krbcanonicalname'[1] and 'ipakrbprincipalalias' together > > > with the objectclass 'ipaKrbPrincipal' to the user object. > > > > We have already a ticket open since long to remove krbprincipalalias, it was > > a mistake to add it and any patch that depends on it will be nacked by me. > > We need to use krbPrincipalName and krbCanonicalName. > > > > > Then the IPA > > > KDB backend will use 'ipakrbprincipalalias' for case in-sensitive > > > matches and the principal from 'krbcanonicalname' will be the canonical > > > principal used further on. The 'krbPrincipalName' is not suitable for > > > either because it has caseExact* matching rules and is a multivalue > > > attribute [2]. > > > > Case-exact match is a problem only if we do not canonicalize names when > > storing > > them, otherwise all you need to do is store a "search form" in > > krbPrincipalName > > and always change searches to that form (forcibly upper case realm, forcibly > > lowercase components) when canonicalization is requested. > > > > Additionally in the patch you are using stcasecmp(), that function is not > > acceptable, look at ipadb_find_principal() and you'll see we use > > ulc_casecmp() > > there. > > Also modyfing the principal before searching is done wrong (you use strchr() > > to find the @ sign, but you could find an @ in the components this way, you > > should use strrchr() at the very least), and is dangerous if done outside of > > the inner functions because then we never have a way to know the original > > form should it be needed. In any case as said above realm should be forcibly > > uppercase, given a flag in the escape function instead. > > Thank for for the review and the comments. > > I changed the patch as you suggested to upper-case the realm in the > escape function if the flag is set. > > I didn't add any checks to make sure that the realm of newly added > principal attributes is always upper case. Since the attributes can be > added via various ways I think the check should happen on the DS level We should indeed intercept add/modify operations and see if they try to set krbPrincipalName/krbCanonicalName and then validate the name. Return unwilling to perform if the case of the realm is different (or fix it on the fly, up for discussion) from the default case as configured in the server. > but I see this more in the context of full canonicalization fix covered > by https://fedorahosted.org/freeipa/ticket/3864 . If you think this is a > requirement for the patch attached I would suggest to drop > https://fedorahosted.org/freeipa/ticket/4844 and solve it together with > #3864. We should clsoe 4844 as fixed upstream (there *was* a bug in libkrb5). I commented on #3864 about what we can do, and we can also avoid changing the schema. > I added a second patch which makes the unit test a bit more robust if > the krb5.conf on the system running the tests is broken. Ok. So on the new patches, what does "unify" means ? I do not get what it means (so probably it is a poor name), I guess you may want to call it "canonicalization" ? (or even 'canon' to shorten it a bit). I think the worst case for a utf8 string is more then length*2, probably more like length*6, unless there is some guarantee around case changes that I am not aware of, that said we could probably just allocate on the stack a fixed size string of a KiB or so, the longest DNS name is 256 chars IIRC and a service name can't be that much longer, also usernames can't be arbitrarily long. So 1/2 KiB should probably be fine for a full principal name. (avoids a malloc t
Re: [Freeipa-devel] Is Backend.krb part of API?
On Tue, 28 Jul 2015, Petr Vobornik wrote: On 07/28/2015 10:57 AM, Michael Šimáček wrote: Hi, I'm working on porting FreeIPA away from python-krbV. Backend.krb and KRB5_CCache classes are mere wrappers around krbV bindings, so it would make sense to remove them. But I found the former used in the example in doc/examples/python-api.py. Is it part of FreeIPA's API? Shall I provide some partial compatibility layer for it? (only partial because some methods can take krbV objects as arguments) Thank you, Michael Simacek Does the replacement offer API which has all the methods as the wrappers? If so we can remove them. Imho we can remove Backend.krb aka ipalib/plugins/kerberos.py. It's used only in 2 files, both are not in production. But I'm not sure about KRB5_CCache, the wrapper has some exception logic which might be wanted to be kept. Backend.krb can go if you provide something similar to KRB5_CCache. We need to be able to initialize ccache with that class -- either by using existing ccache (we often marshall ccache content to memcached and then unmarshall it when the same session comes back) or by using a keytab. After ccache is provided, we need to be able to query default principal of the existing ccache. -- / Alexander Bokovoy -- 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] Is Backend.krb part of API?
On 07/28/2015 10:57 AM, Michael Šimáček wrote: Hi, I'm working on porting FreeIPA away from python-krbV. Backend.krb and KRB5_CCache classes are mere wrappers around krbV bindings, so it would make sense to remove them. But I found the former used in the example in doc/examples/python-api.py. Is it part of FreeIPA's API? Shall I provide some partial compatibility layer for it? (only partial because some methods can take krbV objects as arguments) Thank you, Michael Simacek Does the replacement offer API which has all the methods as the wrappers? If so we can remove them. Imho we can remove Backend.krb aka ipalib/plugins/kerberos.py. It's used only in 2 files, both are not in production. But I'm not sure about KRB5_CCache, the wrapper has some exception logic which might be wanted to be kept. -- 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] Replace stageuser-add --from-delete with user-undel --to-staged
Dne 28.7.2015 v 11:36 Lenka Doudova napsal(a): Dne 28.7.2015 v 11:27 Jan Cholasta napsal(a): Dne 27.7.2015 v 17:59 Martin Basti napsal(a): On 23/07/15 14:43, Martin Basti wrote: Hello, I tried to fix #5145 and I partially succeeded. However, I cannot fix this part of ticket, where user is prompted to write name and surname. $ ipa stageuser-add tuser --from-delete First name: this will be ignored Last name: this will be also ignored Added stage user "tuser" As the first name and last name are mandatory attributes of stageuser-add command, but they are not needed by when the --from-delete option is used. I would like to ask how to fix this issue, IMO this will be huge hack in internal API. Or should we just document this bug as known issue (thierry wrote that this is not use case that should be used often)? The best solution would be separate command, but this idea was rejected in thread "[Freeipa-devel] User life cycle: question regarding the design" Regards Martin^2 Hello, as was mentioned before, we have issue with current internal API and the stageuser-add --from-delete command. We discussed this today, and we did not find a nice way how to fix it, so we propose this (which is IMO the best solution): * stageuser-add --from-delete should be deprecated +1 * create new option for user-undel: used-undel --to-staged (or create new command) that will handle moving deleted users to staged area as --from-delete did. Make it new command please. Instead of stageuser-add and option --from-delete, which work totally different, the command user-undel does similar operation than stage-user --from-delete, it just uses different container. NACK on stuffing everything into a single command just because it does something similar. How about making it a 'stageuser-undel'? The 'user-undel' moves preserved user to active, so the 'stageuser-undel' would move preserved to staged. The action is similar, but has slightly different specifics (which attributes are preserved etc.), and for me the 'stageuser-undel' feels more natural than 'user-undel --to-staged' since it's basically the same as there is 'stageuser-add' for creating a staged user, not 'user-add --to-staged'. It would be in the same style as all the other commands concerning operations with users in staged container. Well, user-undel is the opposite of user-del, and stageuser-undel should be the opposite of stageuser-del. The stageuser-undel you are suggesting is not. Also I'm not sure if we want to (always) remove the deleted user once a staged user is created from it, but -undel behaves like that. I don't think the command should be limited to deleted users only. Active and deleted users share the same namespace, so it is an arbitrary limitation. I think that what we are looking for is the opposite of stageuser-activate. So maybe user-stage? -- Jan Cholasta -- 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 149] IPA KDB: allow case in-sensitive realm in AS request
On Wed, Jul 22, 2015 at 09:41:51AM -0400, Simo Sorce wrote: > - Original Message - > > From: "Sumit Bose" > > To: "freeipa-devel" > > Sent: Tuesday, July 21, 2015 7:41:14 AM > > Subject: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive realm > > in AS request > > > > Hi, > > > > this patch is my suggestion to solve > > https://fedorahosted.org/freeipa/ticket/4844 . > > > > The original issue in the ticket has two part. One is a loop in libkrb5 > > which is already fixed. The other is to handle canonicalization better. > > Sorry Sumit, > I see several issues with this patck. > > first of all you should really not change ipadb_get_principal(), that's the > wrong place to apply your logic. > > To support searching for the realm name case-insensitively all we should do > is to always forcibly upper case the realm name at the same time we build the > filter (in ipadb_fetch_principals(), if canonicalization was requested. > Because we will never store (code to prevent that should probably be dded with > this patch) a realm name that is not all caps. > Then the post search matches should be done straight within > ipadb_find_principal(). > > > The general way to allow canonicalization on a principal is to add the > > attributes 'krbcanonicalname'[1] and 'ipakrbprincipalalias' together > > with the objectclass 'ipaKrbPrincipal' to the user object. > > We have already a ticket open since long to remove krbprincipalalias, it was > a mistake to add it and any patch that depends on it will be nacked by me. > We need to use krbPrincipalName and krbCanonicalName. > > > Then the IPA > > KDB backend will use 'ipakrbprincipalalias' for case in-sensitive > > matches and the principal from 'krbcanonicalname' will be the canonical > > principal used further on. The 'krbPrincipalName' is not suitable for > > either because it has caseExact* matching rules and is a multivalue > > attribute [2]. > > Case-exact match is a problem only if we do not canonicalize names when > storing > them, otherwise all you need to do is store a "search form" in > krbPrincipalName > and always change searches to that form (forcibly upper case realm, forcibly > lowercase components) when canonicalization is requested. > > Additionally in the patch you are using stcasecmp(), that function is not > acceptable, look at ipadb_find_principal() and you'll see we use ulc_casecmp() > there. > Also modyfing the principal before searching is done wrong (you use strchr() > to find the @ sign, but you could find an @ in the components this way, you > should use strrchr() at the very least), and is dangerous if done outside of > the inner functions because then we never have a way to know the original > form should it be needed. In any case as said above realm should be forcibly > uppercase, given a flag in the escape function instead. Thank for for the review and the comments. I changed the patch as you suggested to upper-case the realm in the escape function if the flag is set. I didn't add any checks to make sure that the realm of newly added principal attributes is always upper case. Since the attributes can be added via various ways I think the check should happen on the DS level but I see this more in the context of full canonicalization fix covered by https://fedorahosted.org/freeipa/ticket/3864 . If you think this is a requirement for the patch attached I would suggest to drop https://fedorahosted.org/freeipa/ticket/4844 and solve it together with #3864. I added a second patch which makes the unit test a bit more robust if the krb5.conf on the system running the tests is broken. bye, Sumit > > > What I got from the comments in the ticket and the related bugzilla > > ticket is that it should be possible to get a TGT for a user even if the > > realm is given in lower-case if canonicalization is enabled. Please note > > that the client can only send such request because we have > > 'dns_lookup_kdc = true' in krb.conf and DNS is case in-sensitive. If you > > set 'dns_lookup_kdc = false' the client will fail immediately without > > sending a request at all, because it is not able to find a KDC for the > > lower-case realm. > > > > On the server-side the request is processed because of > > http://k5wiki.kerberos.org/wiki/Projects/Aliases which made parts of > > processing case in-sensitive. > > > > With the attached patch a second lookup is done if the lookup with the > > original input returned no result, canonicalization is > > enabled and the realm from the original input matches the IPA realm case > > in-sensitive. For the second lookup the realm is replace with the IPA > > realm. This approach adds a bit redundant code but does not add extra > > processing requests which would be successful before. > > > > Without the patch > > kinitipauser@IPA.REALM -> success > > kinit -C ipauser@IPA.REALM -> success > > kinitipauser@ipa.realm -> failure > > kinit -C ipauser@ipa.realm -> failure > > > > With the
Re: [Freeipa-devel] variable name 'rsa_public_key' in vault
On Fri, 2015-07-24 at 12:20 +0200, Christian Heimes wrote: > Hello, > > while I was working on https://fedorahosted.org/freeipa/ticket/5142 and > patch 019, I noticed the variable names rsa_public_key and > rsa_private_key in vault.py. load_pem_public_key() can load and return > other key formats (DSA, ECDSA), too. Does vault mean to support the > other algorithms? > > In case vault should support any kind of asymmetric cipher, I'd like to > change the variable names. It's confusing. Otherwise we should add a > check for RSA and prevent DSA and ECDSA keys. > > Christian > We certainly want to support ECDSA keys eventually, so I'd rename the vars. Simo. -- Simo Sorce * Red Hat, Inc * New York -- 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] Replace stageuser-add --from-delete with user-undel --to-staged
Dne 28.7.2015 v 11:27 Jan Cholasta napsal(a): Dne 27.7.2015 v 17:59 Martin Basti napsal(a): On 23/07/15 14:43, Martin Basti wrote: Hello, I tried to fix #5145 and I partially succeeded. However, I cannot fix this part of ticket, where user is prompted to write name and surname. $ ipa stageuser-add tuser --from-delete First name: this will be ignored Last name: this will be also ignored Added stage user "tuser" As the first name and last name are mandatory attributes of stageuser-add command, but they are not needed by when the --from-delete option is used. I would like to ask how to fix this issue, IMO this will be huge hack in internal API. Or should we just document this bug as known issue (thierry wrote that this is not use case that should be used often)? The best solution would be separate command, but this idea was rejected in thread "[Freeipa-devel] User life cycle: question regarding the design" Regards Martin^2 Hello, as was mentioned before, we have issue with current internal API and the stageuser-add --from-delete command. We discussed this today, and we did not find a nice way how to fix it, so we propose this (which is IMO the best solution): * stageuser-add --from-delete should be deprecated +1 * create new option for user-undel: used-undel --to-staged (or create new command) that will handle moving deleted users to staged area as --from-delete did. Make it new command please. Instead of stageuser-add and option --from-delete, which work totally different, the command user-undel does similar operation than stage-user --from-delete, it just uses different container. NACK on stuffing everything into a single command just because it does something similar. How about making it a 'stageuser-undel'? The 'user-undel' moves preserved user to active, so the 'stageuser-undel' would move preserved to staged. The action is similar, but has slightly different specifics (which attributes are preserved etc.), and for me the 'stageuser-undel' feels more natural than 'user-undel --to-staged' since it's basically the same as there is 'stageuser-add' for creating a staged user, not 'user-add --to-staged'. It would be in the same style as all the other commands concerning operations with users in staged container. Lenka We need to do this in 4.2.1 to affect as least as possible users. If you have any objections, please speak/write :) Martin^2 -- 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] Replace stageuser-add --from-delete with user-undel --to-staged
Dne 27.7.2015 v 17:59 Martin Basti napsal(a): On 23/07/15 14:43, Martin Basti wrote: Hello, I tried to fix #5145 and I partially succeeded. However, I cannot fix this part of ticket, where user is prompted to write name and surname. $ ipa stageuser-add tuser --from-delete First name: this will be ignored Last name: this will be also ignored Added stage user "tuser" As the first name and last name are mandatory attributes of stageuser-add command, but they are not needed by when the --from-delete option is used. I would like to ask how to fix this issue, IMO this will be huge hack in internal API. Or should we just document this bug as known issue (thierry wrote that this is not use case that should be used often)? The best solution would be separate command, but this idea was rejected in thread "[Freeipa-devel] User life cycle: question regarding the design" Regards Martin^2 Hello, as was mentioned before, we have issue with current internal API and the stageuser-add --from-delete command. We discussed this today, and we did not find a nice way how to fix it, so we propose this (which is IMO the best solution): * stageuser-add --from-delete should be deprecated +1 * create new option for user-undel: used-undel --to-staged (or create new command) that will handle moving deleted users to staged area as --from-delete did. Make it new command please. Instead of stageuser-add and option --from-delete, which work totally different, the command user-undel does similar operation than stage-user --from-delete, it just uses different container. NACK on stuffing everything into a single command just because it does something similar. We need to do this in 4.2.1 to affect as least as possible users. If you have any objections, please speak/write :) Martin^2 -- Jan Cholasta -- 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] Is Backend.krb part of API?
Hi, I'm working on porting FreeIPA away from python-krbV. Backend.krb and KRB5_CCache classes are mere wrappers around krbV bindings, so it would make sense to remove them. But I found the former used in the example in doc/examples/python-api.py. Is it part of FreeIPA's API? Shall I provide some partial compatibility layer for it? (only partial because some methods can take krbV objects as arguments) Thank you, Michael Simacek -- 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] 0035 client: Update DNS with all available local IP addresses.
On 27/07/15 16:45, David Kupka wrote: On 15/01/15 17:13, David Kupka wrote: On 01/15/2015 03:22 PM, David Kupka wrote: On 01/15/2015 12:43 PM, David Kupka wrote: On 01/12/2015 06:34 PM, Martin Basti wrote: On 09/01/15 14:43, David Kupka wrote: On 01/07/2015 04:15 PM, Martin Basti wrote: On 07/01/15 12:27, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/4249 Thank you for patch: 1) -root_logger.error("Cannot update DNS records! " - "Failed to connect to server '%s'.", server) +ips = get_local_ipaddresses() +except CalledProcessError as e: +root_logger.error("Cannot update DNS records. %s" % e) IMO the error message should be more specific, add there something like "Unable to get local IP addresses". at least in log.debug() 2) +lines = ipresult[0].replace('\\', '').split('\n') .replace() is not needed 3) +if len(ips) == 0: if not ips: is more pythonic by PEP8 Thanks for catching these. Updated patch attached. merciful NACK Thank you for the patch, unfortunately I hit one issue which needs to be resolved. If "sync PTR" is activated in zone settings, and reverse zone doesn't exists, nsupdate/BIND returns SERVFAIL and ipa-client-install print Error message, 'DNS update failed'. In fact, all A/ records was succesfully updated, only PTR records failed. Bind log: named-pkcs11[28652]: updating zone 'example.com/IN': adding an RR at 'vm-101.example.com' named-pkcs11[28652]: PTR record synchronization (addition) for A/ 'vm-101.example.com.' refused: unable to find active reverse zone for IP address '2620:52:0:104c:21a:4aff:fe10:4eaa': not found With IPv6 we have several addresses from different reverse zones and this situation may happen often. I suggest following: 1) Print list of addresses which will be updated. (Now if update fails, user needs to read log, which addresses installer tried to update) 2) Split nsupdates per A/ record. 3a) If failed, check with DNS query if A/ and PTR record are there and print proper error message 3b) Just print A/ (or PTR) record may not be updated for particular IP address. Any other suggestions are welcome. After long discussion with DNS and UX guru I've implemented it this way: 1. Call nsupdate only once with all updates. 2. Verify that the expected records are resolvable. 3. If no print list of missing A/, list of missing PTR records and list to mismatched PTR record. As this is running inside client we can't much more and it's up to user to check what's rotten in his DNS setup. Updated patch attached. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel One more change to behave well in -crazy- exotic environments that resolves more PTR records for single IP. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Yet another change to make language nerds and our UX guru happy :-) ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Rebased patch attached. Updated patch attached. -- David Kupka From 5c923daf7ce662e19b144e338557e1b8df7a061d Mon Sep 17 00:00:00 2001 From: David Kupka Date: Sun, 4 Jan 2015 15:04:18 -0500 Subject: [PATCH] client: Update DNS with all available local IP addresses. Detect all usable IP addresses assigned to any interface and create coresponding DNS records on server. https://fedorahosted.org/freeipa/ticket/4249 --- ipa-client/ipa-install/ipa-client-install | 174 +++--- 1 file changed, 113 insertions(+), 61 deletions(-) diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install index 91323ae115a27d221bcbc43fee887c56d99c8635..947cb10d98e950498b9ea1e4a3b715de1ee33e3b 100755 --- a/ipa-client/ipa-install/ipa-client-install +++ b/ipa-client/ipa-install/ipa-client-install @@ -32,6 +32,7 @@ try: from optparse import SUPPRESS_HELP, OptionGroup, OptionValueError import shutil from krbV import Krb5Error +import dns import nss.nss as nss import SSSDConfig @@ -1285,6 +1286,7 @@ def configure_sssd_conf(fstore, cli_realm, cli_domain, cli_server, options, clie if options.dns_updates: domain.set_option('dyndns_update', True) +domain.set_option('dyndns_iface', '*') if options.krb5_offline_passwords: domain.set_option('krb5_store_password_if_offline', True) @@ -1500,40 +1502,22 @@ def unconfigure_nisdomain(): if not enabled: services.knownservices.domainname.disable() - -def resolve_ipaddress(server): -""" Connect to the server's LDAP port in order to determine what ip -address this machine uses as "public" ip (relative to the server). - -Returns a