Re: [Freeipa-devel] Topology plugin quirks
On 06/03/2015 11:37 AM, Martin Babinsky wrote: Hi everyone, I have been playing with the topology related patches and I have encountered a few issues that I would like to address in this thread: 1.) When replica install for whatever reason crashes _after_ the setup of replication agreements etc., it leaves the topology plugin with dangling segment pointing to the dysfunctional node. An attempt to delete it leads to: ipa: ERROR: Server is unwilling to perform: Removal of Segment disconnects topology.Deletion not allowed. if the endpoints of the segments are still in the managed master list and there is no other path connecting these two nodes the behaviour is correct. you need to remove the master first, teh segment should be removed automatically. ipa-replica-manage del should do this, it worked for me with the latest patches. can you provide a scenario where it fails ? And you cannot reinstall the crashed replica because it complains about existing replication agreements. It would probably help to be able to force-remove the segments if one of the endpoints doesn't exist/respond. 2.) I was not able to figure out a way remove replica from the topology without explosions or tampering 'cn=masters,cn=ipa,cn=etc,$SUFFIX'. Obviously ipa-replica-manage del doesn't work anymore (I have tried just for fun, it leads to SIGSEGV of the host's dirsrv and leaves dangling segments to offending replica, leading to point 1). I managed to remove replica from the topology only by directly uninstalling FreeIPA on the node and then deleting its' host entry from 'cn=masters'. Only after this was the plugin able to automagically removed the segments pointing to/from removed node. The design page suggests that it should be enough to uninstall IPA server on the replica. The plugin would then pick-up the dangling segments and remove them automatically. However, this behavior seems to require additional modification of the uninstall procedure (e.g. the uninstalling replica should remove its' entry from cn=masters). 3.) It seems that the removal of topology suffixes containing functioning segments is not handled well. I once tried to do this and it led to segmentation fault on the dirsrv instance. What is the expected behavior in this scenario? -- 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] 857 topology: ipa management commands
On 06/03/2015 10:59 AM, Martin Babinsky wrote: On 06/03/2015 10:52 AM, Martin Babinsky wrote: On 05/26/2015 03:31 PM, Petr Vobornik wrote: On 05/26/2015 12:19 PM, Petr Vobornik wrote: this patch is based on top of my patch #856 and tbabej' s 325-9. Obsoletes Ludwig's 0006. ipalib part of topology management Design: - http://www.freeipa.org/page/V4/Manage_replication_topology https://fedorahosted.org/freeipa/ticket/4302 New version attached: - domainlevel_show usage changed to domainlevel_get - updated VERSION - added more attrs to default_attributes Hi Petr, the commands themselves seem to work just fine. I had encountered some quirks in the underlying topology plugin, but I will address them in a different thread in order to keep the discussion relevant to the reviewed patch. I have some minor coomments below: 1.) IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=121 -# Last change: pvoborni - added server-find and server-show +IPA_API_VERSION_MINOR=122 +# Last change: pvoborni - added topology management commands Several people were touching API in the meantime so please double-check that you have correct VERSION and regenerate API.txt Patch rebased. 2.) +Str( +'nsds5replicatedattributelist?', +cli_name='replattrs', +label='Attributes to replicate', +doc=_('Attributes that are not replicated to a consumer server ' + 'during a fractional update. E.g., `(objectclass=*) ' + '$ EXCLUDE accountlockout memberof'), +), +Str( +'nsds5replicatedattributelisttotal?', +cli_name='replattrstotal', +label=_('Attributes for total update'), +doc=_('Attributes that are not replicated to a consumer server ' + 'during a total update. E.g. (objectclass=*) $ EXCLUDE ' + 'accountlockout'), The descriptions of these two options confused me greatly, are these attributes supposed to be replicated or not, or is there some more complex logic behind them that I failed to grasp? I am cc'ing Ludwig, he can probably explain them to us and then we can decide whether we may alter the descriptions to be less confusing. 3.) +takes_params = ( +Str( +'cn', +cli_name='name', +primary_key=True, +label=_('Suffix name'), +), +Str( +'iparepltopoconfroot', +maxlength=255, +cli_name='suffix', +label=_('Suffix to be managed'), +normalizer=lambda value: value.lower(), +), +) This also confused me at first, I suggest to change the label of 'iparepltopoconfroot' to something like 'LDAP suffix to be managed' or 'LDAP subtree to be managed'. Changed to 'LDAP suffix to be managed' 4.) There is currently no way to rename existing topology segments/suffixes. In the case of hosts with funky FQDN's (pointing at you, ABC lab), the segment cn's created during replica installs are mearly impossible to remember and it would be nice to rename them to something more manageable. However, this is not related to core functionality and can be a subject of a separate patch once this gets pushed. That's all from my side. I also forgot to ask what is the expected policy when deleting a non-empty topology suffix. If this is not supported and you have to first remove all segments and then the suffix itself, the 'topologysuffix-del' command should issue an error pointing the user to correct procedure. Do we have a use case for creation or deletion of topology suffix? -- Petr Vobornik From ea383de2037b63e0ec725fff1fbd7bd69673d40d Mon Sep 17 00:00:00 2001 From: Petr Vobornik pvobo...@redhat.com Date: Fri, 22 May 2015 09:50:09 +0200 Subject: [PATCH] topology: ipa management commands ipalib part of topology management Design: - http://www.freeipa.org/page/V4/Manage_replication_topology https://fedorahosted.org/freeipa/ticket/4302 --- API.txt| 155 ++ VERSION| 4 +- ipalib/constants.py| 1 + ipalib/plugins/topology.py | 383 + 4 files changed, 541 insertions(+), 2 deletions(-) create mode 100644 ipalib/plugins/topology.py diff --git a/API.txt b/API.txt index 6520f2c428342cdd30b0db830ed4ddbc89e4302a..0e42fadc66c129e53c3860fb7eeec69c1f148147 100644 --- a/API.txt +++ b/API.txt @@ -4494,6 +4494,161 @@ option: Str('version?', exclude='webui') output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) output: Output('summary', (type 'unicode', type 'NoneType'), None) output: PrimaryKey('value', None, None) +command: topologysegment_add +args: 2,13,3 +arg: Str('topologysuffixcn', cli_name='topologysuffix', multivalue=False, primary_key=True, query=True, required=True) +arg: Str('cn', attribute=True, cli_name='name', maxlength=255,
Re: [Freeipa-devel] Topology plugin quirks
On 06/03/2015 01:32 PM, Oleg Fayans wrote: Hi Ludwig On 06/03/2015 12:23 PM, Ludwig Krispenz wrote: On 06/03/2015 11:51 AM, Oleg Fayans wrote: I confirm every point of this. did you test with all the latest patches applied ? In your issues you refer to crashes, the crashes reported should be resolved, if you still have crashes, pleas provide a core dump or scenario to reproduce the crash. With patch0009 ipa-replica-manage del worked for me Yep, patch 0009 is applied. The full list of patches applied on top of the master branch (at it's state yesterday at 10 PM) is as follows: freeipa-lkrispen-0007-replica-install-fails-with-domain-level-1.patch freeipa-lkrispen-0008-plugin-uses-1-as-minimum-domain-level-to-become-acti.patch freeipa-lkrispen-0009-crash-when-removing-a-replica.patch freeipa-mbasti-0262-Installers-fix-remove-temporal-ccache.patch freeipa-pvoborni-0857-1-topology-ipa-management-commands.patch freeipa-pvoborni-0858-1-webui-IPA.command_dialog-a-new-dialog-base-class.patch freeipa-pvoborni-0859-1-webui-use-command_dialog-as-a-base-class-for-passwor.patch freeipa-pvoborni-0860-1-webui-make-usage-of-all-in-details-facet-optional.patch freeipa-pvoborni-0861-2-webui-topology-plugin.patch freeipa-pvoborni-0862-webui-configurable-refresh-command.patch The scenario is pretty basic: 1. 3 fedora-21 vms with the latest directory server packages from mreynolds repo: 389-ds-base-2015_06_02-1.fc21.x86_64 2. setup master on one of them, prepare gpg files for two replicas 3. setup replicas using these gpg files. 4. Try to remove one of the replicas using command `ipa topologysegment-del` this should remove a segment, not a replica and it should be rejected 5. Try to create a new user via web UI on any of the replicas On 06/03/2015 11:37 AM, Martin Babinsky wrote: Hi everyone, I have been playing with the topology related patches and I have encountered a few issues that I would like to address in this thread: 1.) When replica install for whatever reason crashes _after_ the setup of replication agreements etc., it leaves the topology plugin with dangling segment pointing to the dysfunctional node. An attempt to delete it leads to: ipa: ERROR: Server is unwilling to perform: Removal of Segment disconnects topology.Deletion not allowed. Furthermore, any attempts to delete a segment (even a properly setup one) lead to the same very error. And you cannot reinstall the crashed replica because it complains about existing replication agreements. It would probably help to be able to force-remove the segments if one of the endpoints doesn't exist/respond. 2.) I was not able to figure out a way remove replica from the topology without explosions or tampering 'cn=masters,cn=ipa,cn=etc,$SUFFIX'. Obviously ipa-replica-manage del doesn't work anymore (I have tried just for fun, it leads to SIGSEGV of the host's dirsrv and leaves dangling segments to offending replica, leading to point 1). I managed to remove replica from the topology only by directly uninstalling FreeIPA on the node and then deleting its' host entry from 'cn=masters'. Only after this was the plugin able to automagically removed the segments pointing to/from removed node. The design page suggests that it should be enough to uninstall IPA server on the replica. The plugin would then pick-up the dangling segments and remove them automatically. However, this behavior seems to require additional modification of the uninstall procedure (e.g. the uninstalling replica should remove its' entry from cn=masters). 3.) It seems that the removal of topology suffixes containing functioning segments is not handled well. I once tried to do this and it led to segmentation fault on the dirsrv instance. What is the expected behavior in this scenario? -- 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] Topology plugin quirks
I confirm every point of this. On 06/03/2015 11:37 AM, Martin Babinsky wrote: Hi everyone, I have been playing with the topology related patches and I have encountered a few issues that I would like to address in this thread: 1.) When replica install for whatever reason crashes _after_ the setup of replication agreements etc., it leaves the topology plugin with dangling segment pointing to the dysfunctional node. An attempt to delete it leads to: ipa: ERROR: Server is unwilling to perform: Removal of Segment disconnects topology.Deletion not allowed. Furthermore, any attempts to delete a segment (even a properly setup one) lead to the same very error. And you cannot reinstall the crashed replica because it complains about existing replication agreements. It would probably help to be able to force-remove the segments if one of the endpoints doesn't exist/respond. 2.) I was not able to figure out a way remove replica from the topology without explosions or tampering 'cn=masters,cn=ipa,cn=etc,$SUFFIX'. Obviously ipa-replica-manage del doesn't work anymore (I have tried just for fun, it leads to SIGSEGV of the host's dirsrv and leaves dangling segments to offending replica, leading to point 1). I managed to remove replica from the topology only by directly uninstalling FreeIPA on the node and then deleting its' host entry from 'cn=masters'. Only after this was the plugin able to automagically removed the segments pointing to/from removed node. The design page suggests that it should be enough to uninstall IPA server on the replica. The plugin would then pick-up the dangling segments and remove them automatically. However, this behavior seems to require additional modification of the uninstall procedure (e.g. the uninstalling replica should remove its' entry from cn=masters). 3.) It seems that the removal of topology suffixes containing functioning segments is not handled well. I once tried to do this and it led to segmentation fault on the dirsrv instance. What is the expected behavior in this scenario? -- Oleg Fayans Quality Engineer FreeIPA team RedHat. -- 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 0014 v3] Support multiple user and host certificates
On 02/06/15 16:03, Jan Cholasta wrote: Dne 2.6.2015 v 12:36 Martin Basti napsal(a): On 02/06/15 11:42, Fraser Tweedale wrote: On Mon, Jun 01, 2015 at 02:50:45PM +0200, Martin Basti wrote: On 01/06/15 06:40, Fraser Tweedale wrote: New version of patch; ``{host,service}-show --out=FILE`` now writes all certs to FILE. Rebased on latest master. Thanks, Fraser On Thu, May 28, 2015 at 09:18:04PM +1000, Fraser Tweedale wrote: Updated patch attached. Notably restores/adds revocation behaviour to host-mod and service-mod. Thanks, Fraser On Wed, May 27, 2015 at 06:12:50PM +0200, Martin Basti wrote: On 27/05/15 15:53, Fraser Tweedale wrote: This patch adds supports for multiple user / host certificates. No schema change is needed ('usercertificate' attribute is already multi-value). The revoke-previous-cert behaviour of host-mod and user-mod has been removed but revocation behaviour of -del and -disable is preserved. The latest profiles/caacl patchset (0001..0013 v5) depends on this patch for correct cert-request behaviour. There is one design question (or maybe more, let me know): the `--out=FILENAME' option to {host,service} show saves ONE certificate to the named file. I propose to either: a) write all certs, suffixing suggested filename with either a sequential numerical index, e.g. cert.pem becomes cert.pem.1, cert.pem.2, and so on; or b) as above, but suffix with serial number and, if there are different issues, some issuer-identifying information. Let me know your thoughts. Thanks, Fraser Is there a possible way how to store certificates into one file? I read about possibilities to have multiple certs in one .pem file, but I'm not cert guru :) I personally vote for serial number in case there are multiple certificates, if ^ is no possible. 1) +if len(certs) 0: please use only, if certs: 2) You need to re-generate API/ACI.txt in this patch 3) syntax error: +for dercert in certs_der 4) command ipa user-mod ca_user --certificate=ceritifcate removes the current certificate from the LDAP, by design. Should be the old certificate(s) revoked? You removed that part in the code. only the --addattr='usercertificate=cert' appends new value there -- Martin Basti My objections/proposed solutions in attached patch. * VERSION * In the previous version normalized values was stored in LDAP, so I added it back. (I dont know why there is no normalization in param settings, but normalization for every certificate is done in callback. I will file a ticket for this) * IMO only normalized certificates should be compared in the old certificates detection I incorporated your suggested changes in new patch (attached). There were no proposed changes to the other patchset (0001..0013) since rebase. Thanks, Fraser Thank you, ACK Martin^2 Pushed to master: 7f7c247bb5a4b0030d531f4f14c156162e808212 Regression found. Patch to fix the issue is attached. -- Martin Basti From 6b489fc6a04b60a38e768009558fe1d9b1164b45 Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Wed, 3 Jun 2015 13:11:58 +0200 Subject: [PATCH] Fix: regression in host and service plugin Test failures: * wrong error message * mod operation always delete usercertificates https://fedorahosted.org/freeipa/ticket/4238 --- ipalib/plugins/host.py| 10 +++--- ipalib/plugins/service.py | 11 +++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py index 9ad087e26250d86b15fbe723a98cca278ef29adf..e81dca94e124b080a3d68a3b1cfd079710e30336 100644 --- a/ipalib/plugins/host.py +++ b/ipalib/plugins/host.py @@ -871,8 +871,11 @@ class host_mod(LDAPUpdate): x509.verify_cert_subject(ldap, keys[-1], cert) # revoke removed certificates -if self.api.Command.ca_is_enabled()['result']: -entry_attrs_old = ldap.get_entry(dn, ['usercertificate']) +if certs and self.api.Command.ca_is_enabled()['result']: +try: +entry_attrs_old = ldap.get_entry(dn, ['usercertificate']) +except errors.NotFound: +self.obj.handle_not_found(*keys) old_certs = entry_attrs_old.get('usercertificate', []) old_certs_der = map(x509.normalize_certificate, old_certs) removed_certs_der = set(old_certs_der) - set(certs_der) @@ -899,7 +902,8 @@ class host_mod(LDAPUpdate): nsprerr.args[1]) else: raise nsprerr -entry_attrs['usercertificate'] = certs_der +if certs: +entry_attrs['usercertificate'] = certs_der if options.get('random'): entry_attrs['userpassword'] = ipa_generate_password(characters=host_pwd_chars) diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py index c290344cf6c14155ec1b103525ff8642a7a8e2af..d8bd03523089cd8446377d6b0c85bc318e69b809 100644 ---
Re: [Freeipa-devel] [PATCH 0014 v3] Support multiple user and host certificates
On 06/03/2015 01:17 PM, Martin Basti wrote: On 02/06/15 16:03, Jan Cholasta wrote: Dne 2.6.2015 v 12:36 Martin Basti napsal(a): On 02/06/15 11:42, Fraser Tweedale wrote: On Mon, Jun 01, 2015 at 02:50:45PM +0200, Martin Basti wrote: On 01/06/15 06:40, Fraser Tweedale wrote: New version of patch; ``{host,service}-show --out=FILE`` now writes all certs to FILE. Rebased on latest master. Thanks, Fraser On Thu, May 28, 2015 at 09:18:04PM +1000, Fraser Tweedale wrote: Updated patch attached. Notably restores/adds revocation behaviour to host-mod and service-mod. Thanks, Fraser On Wed, May 27, 2015 at 06:12:50PM +0200, Martin Basti wrote: On 27/05/15 15:53, Fraser Tweedale wrote: This patch adds supports for multiple user / host certificates. No schema change is needed ('usercertificate' attribute is already multi-value). The revoke-previous-cert behaviour of host-mod and user-mod has been removed but revocation behaviour of -del and -disable is preserved. The latest profiles/caacl patchset (0001..0013 v5) depends on this patch for correct cert-request behaviour. There is one design question (or maybe more, let me know): the `--out=FILENAME' option to {host,service} show saves ONE certificate to the named file. I propose to either: a) write all certs, suffixing suggested filename with either a sequential numerical index, e.g. cert.pem becomes cert.pem.1, cert.pem.2, and so on; or b) as above, but suffix with serial number and, if there are different issues, some issuer-identifying information. Let me know your thoughts. Thanks, Fraser Is there a possible way how to store certificates into one file? I read about possibilities to have multiple certs in one .pem file, but I'm not cert guru :) I personally vote for serial number in case there are multiple certificates, if ^ is no possible. 1) +if len(certs) 0: please use only, if certs: 2) You need to re-generate API/ACI.txt in this patch 3) syntax error: +for dercert in certs_der 4) command ipa user-mod ca_user --certificate=ceritifcate removes the current certificate from the LDAP, by design. Should be the old certificate(s) revoked? You removed that part in the code. only the --addattr='usercertificate=cert' appends new value there -- Martin Basti My objections/proposed solutions in attached patch. * VERSION * In the previous version normalized values was stored in LDAP, so I added it back. (I dont know why there is no normalization in param settings, but normalization for every certificate is done in callback. I will file a ticket for this) * IMO only normalized certificates should be compared in the old certificates detection I incorporated your suggested changes in new patch (attached). There were no proposed changes to the other patchset (0001..0013) since rebase. Thanks, Fraser Thank you, ACK Martin^2 Pushed to master: 7f7c247bb5a4b0030d531f4f14c156162e808212 Regression found. Patch to fix the issue is attached. The fix works, thanks. Milan -- 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] Password vault
Dne 2.6.2015 v 20:40 Simo Sorce napsal(a): On Tue, 2015-06-02 at 07:07 -0500, Endi Sukma Dewata wrote: On 6/2/2015 1:10 AM, Martin Kosek wrote: Hi Endi, Quickly skimming through your patches raised couple questions on my side: 1) Will it be possible to also store plain text password via Vault? It talks about taking in the binary data or the text file, but will it also work with plain user secrets (passwords)? I am talking about use like this: # ipa vault-archive name --user mkosek --data Secret123 For security the plain text password should be stored in a file first: # vi password.txt # ipa vault-archive name --user mkosek --in password.txt It's also possible to specify the password as base-64 encoded data: # echo -n Secret123 | base64 # ipa vault-archive name --user mkosek --data U2VjcmV0MTIz But it's not recommended since the data will be stored in the command history and someone could see and decode it. I think passing a plain text password as command line argument would be even worse. The --data parameter is mainly used for unit testing. Later we might be able to add an option to read from standard input: # cat password.txt | ipa vault-archive name --user mkosek --std-in Yes please, a way to pass in via stdin is extremely useful, as leaving files on the filesystem is also a big risk. This will not work well, it should use the normal prompting mechanism: $ ipa vault-archive name --user user Data: type data here -- 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] Password vault
On 06/02/2015 11:22 PM, Alexander Bokovoy wrote: On Tue, 02 Jun 2015, Endi Sukma Dewata wrote: Please take a look at the new patch. On 6/2/2015 10:05 AM, Martin Kosek wrote: 4) In the vault-archive forward method, you use pki module. However, this module will be only available on FreeIPA PKI-powered servers and not on FreeIPA clients - so this will not work unless freeipa-client gets a dependency on pki-base - which is definitely not something we want... In my opinion it should be fine to require pki-base on the client because it contains just the client library, unless you have other concerns? Any objections to having pki-nss and pki-cryptography dependencies on the client? Even if we can change the client code not to depend on pki module, since in this framework the client and server code are written in the same plugin, the import pki still cannot be removed since it's still needed by the server code, and I don't think conditional import is a good programming practice. I have major concerns here. Look at the different between installing freeipa-client and freeipa-client + pki-base on my F21: ~~ $ sudo yum install freeipa-client ... Install 1 Package (+4 Dependent packages) Total download size: 2.6 M Installed size: 14 M ~~ $ sudo yum install freeipa-client pki-base ... Install 2 Packages (+288 Dependent packages) Total download size: 160 M Installed size: 235 M ~~ This is obviously a no-go for client. The conditional import is smaller concern that big dependency growth on the client. We do them in trust plugin for example and it works fine (though I agree it is not ideal programming practice). IMO, we should limit new freeipa-client dependencies only to python-cryptography (or also python-nss in the worst case, in case python-cryptography is not enough) - there should be no pki dependencies at all, these should be only on the server side. OK. I opened a ticket to split the pki-base into separate Python and Java packages: https://fedorahosted.org/pki/ticket/1399 For now in this patch I added conditional imports for pki.account and pki.key which are needed to access KRA on the server side. I removed dependency on pki.crypto on the client side and replaced it with direct python-nss code. On 6/2/2015 1:40 PM, Simo Sorce wrote: I have coded in python (jwcrypto) support for some key wrapping not yet available in python-cryptography and can lend you the code as needed. Thanks. I'll get back to you when it's time to add support for python-cryptography in KRA: https://fedorahosted.org/pki/ticket/1400 On 6/2/2015 10:16 AM, Alexander Bokovoy wrote: Yes, please use conditional import here, it is perfectly valid use case for the client side. On 6/2/2015 1:40 PM, Simo Sorce wrote: conditional import is just fine The conditional imports that I've seen usually are used for importing different versions of the same module, which I think is acceptable because the dependency always exists. In the vault case we're selectively importing a module depending on where the code runs. I think that is bad because it adds complexity and it's easy to make mistakes. Any code that depends on that module would have to be (a) guarded: if pki_is_loaded: ... call pki ... or (b) used in a method that's only called if the module is loaded: def do_something(self): # runs only on server ... call pki ... The (a) is similar to #ifdef's which should be avoidable using OOD, and in (b) we may inadvertently call a wrong method indirectly. I think ideally the client and server code should be in separate files (so they can be deployed separately too), but the framework doesn't seem to allow that. This exactly the case we have to use here and we are using that in trusts case as well -- some code has to run on server only and shouldn't cause to install Samba related packages on the client. This is because IPA client is actually using the same IPA plugins that server uses, to have access to the API calls metadata and client-side callbacks are defined in the same place where server-side callbacks are. It is IPA framework design, so we have to use what we have. This is planned to be changed BTW, when we start with the Thin Client concept and have different code/plugins for FreeIPA server side and client side. In other words, it is not necessarily an evil under conditions we are dealing with. -- 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] Database error on replicas
On 06/03/2015 10:33 AM, Oleg Fayans wrote: Hi, With the latest freeipa code containing Topology plugin patches, I am unable to make any changes in replicas. I have the following topology: replica1 = master = replica3 Here is the output of the ipa topologysegment-find command: Suffix name: realm -- 2 segments matched -- Segment name: replica1.zaeba.li-to-testmaster.zaeba.li Left node: replica1.zaeba.li Right node: testmaster.zaeba.li Connectivity: both Segment name: replica3.zaeba.li-to-testmaster.zaeba.li Left node: replica3.zaeba.li Right node: testmaster.zaeba.li Connectivity: both Number of entries returned 2 Any changes on master get replicated to replicas successfully. However, any attempts to change anything on replicas, for example, create a user, result in the error message about DatabaseError (attached). The corresponding part of the dirsrv log looks like this: 03/Jun/2015:04:11:55 -0400] slapi_ldap_bind - Error: could not perform interactive bind for id [] authentication mechanism [GSSAPI]: error -1 (Can't contact LDAP server) [03/Jun/2015:04:15:02 -0400] slapi_ldap_bind - Error: could not send startTLS request: error -1 (Can't contact LDAP server) errno 0 (Success) [03/Jun/2015:04:16:55 -0400] slapd_ldap_sasl_interactive_bind - Error: could not perform interactive bind for id [] mech [GSSAPI]: LDAP error -1 (Can't contact LDAP server) ((null)) errno 2 (No such file or directory) [03/Jun/2015:04:16:55 -0400] slapi_ldap_bind - Error: could not perform interactive bind for id [] authentication mechanism [GSSAPI]: error -1 (Can't contact LDAP server) The full log is attached Ludwig, could this be caused by the Topology plugin? -- 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] 857 topology: ipa management commands
On 05/26/2015 03:31 PM, Petr Vobornik wrote: On 05/26/2015 12:19 PM, Petr Vobornik wrote: this patch is based on top of my patch #856 and tbabej' s 325-9. Obsoletes Ludwig's 0006. ipalib part of topology management Design: - http://www.freeipa.org/page/V4/Manage_replication_topology https://fedorahosted.org/freeipa/ticket/4302 New version attached: - domainlevel_show usage changed to domainlevel_get - updated VERSION - added more attrs to default_attributes Hi Petr, the commands themselves seem to work just fine. I had encountered some quirks in the underlying topology plugin, but I will address them in a different thread in order to keep the discussion relevant to the reviewed patch. I have some minor coomments below: 1.) IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=121 -# Last change: pvoborni - added server-find and server-show +IPA_API_VERSION_MINOR=122 +# Last change: pvoborni - added topology management commands Several people were touching API in the meantime so please double-check that you have correct VERSION and regenerate API.txt 2.) +Str( +'nsds5replicatedattributelist?', +cli_name='replattrs', +label='Attributes to replicate', +doc=_('Attributes that are not replicated to a consumer server ' + 'during a fractional update. E.g., `(objectclass=*) ' + '$ EXCLUDE accountlockout memberof'), +), +Str( +'nsds5replicatedattributelisttotal?', +cli_name='replattrstotal', +label=_('Attributes for total update'), +doc=_('Attributes that are not replicated to a consumer server ' + 'during a total update. E.g. (objectclass=*) $ EXCLUDE ' + 'accountlockout'), The descriptions of these two options confused me greatly, are these attributes supposed to be replicated or not, or is there some more complex logic behind them that I failed to grasp? I am cc'ing Ludwig, he can probably explain them to us and then we can decide whether we may alter the descriptions to be less confusing. 3.) +takes_params = ( +Str( +'cn', +cli_name='name', +primary_key=True, +label=_('Suffix name'), +), +Str( +'iparepltopoconfroot', +maxlength=255, +cli_name='suffix', +label=_('Suffix to be managed'), +normalizer=lambda value: value.lower(), +), +) This also confused me at first, I suggest to change the label of 'iparepltopoconfroot' to something like 'LDAP suffix to be managed' or 'LDAP subtree to be managed'. 4.) There is currently no way to rename existing topology segments/suffixes. In the case of hosts with funky FQDN's (pointing at you, ABC lab), the segment cn's created during replica installs are mearly impossible to remember and it would be nice to rename them to something more manageable. However, this is not related to core functionality and can be a subject of a separate patch once this gets pushed. That's all from my side. -- 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] 857 topology: ipa management commands
On 06/03/2015 10:52 AM, Martin Babinsky wrote: On 05/26/2015 03:31 PM, Petr Vobornik wrote: On 05/26/2015 12:19 PM, Petr Vobornik wrote: this patch is based on top of my patch #856 and tbabej' s 325-9. Obsoletes Ludwig's 0006. ipalib part of topology management Design: - http://www.freeipa.org/page/V4/Manage_replication_topology https://fedorahosted.org/freeipa/ticket/4302 New version attached: - domainlevel_show usage changed to domainlevel_get - updated VERSION - added more attrs to default_attributes Hi Petr, the commands themselves seem to work just fine. I had encountered some quirks in the underlying topology plugin, but I will address them in a different thread in order to keep the discussion relevant to the reviewed patch. I have some minor coomments below: 1.) IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=121 -# Last change: pvoborni - added server-find and server-show +IPA_API_VERSION_MINOR=122 +# Last change: pvoborni - added topology management commands Several people were touching API in the meantime so please double-check that you have correct VERSION and regenerate API.txt 2.) +Str( +'nsds5replicatedattributelist?', +cli_name='replattrs', +label='Attributes to replicate', +doc=_('Attributes that are not replicated to a consumer server ' + 'during a fractional update. E.g., `(objectclass=*) ' + '$ EXCLUDE accountlockout memberof'), +), +Str( +'nsds5replicatedattributelisttotal?', +cli_name='replattrstotal', +label=_('Attributes for total update'), +doc=_('Attributes that are not replicated to a consumer server ' + 'during a total update. E.g. (objectclass=*) $ EXCLUDE ' + 'accountlockout'), The descriptions of these two options confused me greatly, are these attributes supposed to be replicated or not, or is there some more complex logic behind them that I failed to grasp? I am cc'ing Ludwig, he can probably explain them to us and then we can decide whether we may alter the descriptions to be less confusing. 3.) +takes_params = ( +Str( +'cn', +cli_name='name', +primary_key=True, +label=_('Suffix name'), +), +Str( +'iparepltopoconfroot', +maxlength=255, +cli_name='suffix', +label=_('Suffix to be managed'), +normalizer=lambda value: value.lower(), +), +) This also confused me at first, I suggest to change the label of 'iparepltopoconfroot' to something like 'LDAP suffix to be managed' or 'LDAP subtree to be managed'. 4.) There is currently no way to rename existing topology segments/suffixes. In the case of hosts with funky FQDN's (pointing at you, ABC lab), the segment cn's created during replica installs are mearly impossible to remember and it would be nice to rename them to something more manageable. However, this is not related to core functionality and can be a subject of a separate patch once this gets pushed. That's all from my side. I also forgot to ask what is the expected policy when deleting a non-empty topology suffix. If this is not supported and you have to first remove all segments and then the suffix itself, the 'topologysuffix-del' command should issue an error pointing the user to correct procedure. -- 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] Password vault
On 06/02/2015 08:34 PM, Simo Sorce wrote: On Tue, 2015-06-02 at 12:04 +0200, Jan Cholasta wrote: Dne 2.6.2015 v 02:02 Endi Sukma Dewata napsal(a): On 5/28/2015 12:46 AM, Jan Cholasta wrote: On a related note, since KRA is optional, can we move the vaults container to cn=kra,cn=vaults? This is the convetion used by the other optional components (DNS and recently CA). I mean cn=vaults,cn=kra of course. If you are talking about the o=kra,PKI suffix, I'm not sure whether the IPA framework will work with it. If you are talking about adding a new cn=kra,IPA suffix entry on top of cn=vaults, what is the purpose of this entry? Is the entry going to be created/deleted automatically when the KRA is installed/removed? Is it going to be used for something else other than vaults? I'm talking about cn=kra,IPA suffix. It should be created only when KRA is installed, although I think this can be done later after the release, moving vaults to cn=kra should be good enough for now. It's going to be used for everything KRA-specific. There are a lot of questions that need to be answered before we can make this change. This is about sticking to a convention, which everyone should do, and everyone except KRA already does. I'm sorry I didn't realize this earlier, but the change must be done now. We probably should revisit this issue after the core vault functionality is added. We can't revisit it later because after release we are stuck with whatever is there forever. See attachment for a patch which implements the change. Shouldn't we s/kra/vault/ ? After all the feature is called Vault, not KRA. I thought we are naming it by the name of the optional subsystem, not the feature itself. If for example, another feature from KRA is used, it would still live in cn=kra, no? -- 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] Database error on replicas
On 06/03/2015 10:44 AM, Martin Kosek wrote: On 06/03/2015 10:33 AM, Oleg Fayans wrote: Hi, With the latest freeipa code containing Topology plugin patches, I am unable to make any changes in replicas. I have the following topology: replica1 = master = replica3 Here is the output of the ipa topologysegment-find command: Suffix name: realm -- 2 segments matched -- Segment name: replica1.zaeba.li-to-testmaster.zaeba.li Left node: replica1.zaeba.li Right node: testmaster.zaeba.li Connectivity: both Segment name: replica3.zaeba.li-to-testmaster.zaeba.li Left node: replica3.zaeba.li Right node: testmaster.zaeba.li Connectivity: both Number of entries returned 2 Any changes on master get replicated to replicas successfully. However, any attempts to change anything on replicas, for example, create a user, result in the error message about DatabaseError (attached). The corresponding part of the dirsrv log looks like this: 03/Jun/2015:04:11:55 -0400] slapi_ldap_bind - Error: could not perform interactive bind for id [] authentication mechanism [GSSAPI]: error -1 (Can't contact LDAP server) [03/Jun/2015:04:15:02 -0400] slapi_ldap_bind - Error: could not send startTLS request: error -1 (Can't contact LDAP server) errno 0 (Success) [03/Jun/2015:04:16:55 -0400] slapd_ldap_sasl_interactive_bind - Error: could not perform interactive bind for id [] mech [GSSAPI]: LDAP error -1 (Can't contact LDAP server) ((null)) errno 2 (No such file or directory) [03/Jun/2015:04:16:55 -0400] slapi_ldap_bind - Error: could not perform interactive bind for id [] authentication mechanism [GSSAPI]: error -1 (Can't contact LDAP server) The full log is attached Ludwig, could this be caused by the Topology plugin? maybe, don't know yet -- 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] Database error on replicas
On 06/03/2015 10:33 AM, Oleg Fayans wrote: Hi, With the latest freeipa code containing Topology plugin patches, I am unable to make any changes in replicas. I have the following topology: replica1 = master = replica3 Here is the output of the ipa topologysegment-find command: Suffix name: realm -- 2 segments matched -- Segment name: replica1.zaeba.li-to-testmaster.zaeba.li Left node: replica1.zaeba.li Right node: testmaster.zaeba.li Connectivity: both Segment name: replica3.zaeba.li-to-testmaster.zaeba.li Left node: replica3.zaeba.li Right node: testmaster.zaeba.li Connectivity: both Number of entries returned 2 Any changes on master get replicated to replicas successfully. However, any attempts to change anything on replicas, for example, create a user, result in the error message about DatabaseError (attached). The corresponding part of the dirsrv log looks like this: 03/Jun/2015:04:11:55 -0400] slapi_ldap_bind - Error: could not perform interactive bind for id [] authentication mechanism [GSSAPI]: error -1 (Can't contact LDAP server) [03/Jun/2015:04:15:02 -0400] slapi_ldap_bind - Error: could not send startTLS request: error -1 (Can't contact LDAP server) errno 0 (Success) [03/Jun/2015:04:16:55 -0400] slapd_ldap_sasl_interactive_bind - Error: could not perform interactive bind for id [] mech [GSSAPI]: LDAP error -1 (Can't contact LDAP server) ((null)) errno 2 (No such file or directory) [03/Jun/2015:04:16:55 -0400] slapi_ldap_bind - Error: could not perform interactive bind for id [] authentication mechanism [GSSAPI]: error -1 (Can't contact LDAP server) The full log is attached Hi Oleg, could you also post the output of 'journalctl -xe' related to dirsrv (on master and also on replicas)? I have seen a couple of segfaults there during reviewing Petr Vobornik's topology* commands. -- 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] 1112 Add service constraint delegation plugin
Dne 3.6.2015 v 11:34 Martin Basti napsal(a): On 02/06/15 22:51, Rob Crittenden wrote: Martin Basti wrote: On 31/05/15 04:07, Rob Crittenden wrote: Petr Vobornik wrote: On 05/27/2015 08:17 PM, Martin Basti wrote: On 27/05/15 19:27, Rob Crittenden wrote: Martin Basti wrote: Thank you. I haven't finished review yet, but I have few notes in case you will modify the patch. Please fix following issues: 3) There are many PEP8 errors, can you fix some of them,? Is PEP8 a concern? What kinds of errors do we fix? For example, the current model for defining options generates a slew of indention errors. In old modules it's preferred to keep the old indentation style for options(not to mix 2 styles). New modules should use following pep8 compliant style: Str( 'cn', cli_name='name', primary_key=True, label=_('Server name'), doc=_('IPA server hostname'), ), We try to keep PEP8 in new code, mainly indentation, blank lines, too long lines. Yes in test definitions and option definitions, is better to keep the same style, but other parts of code should be PEP8. For example these should be fixed ./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:37:13: E225 missing whitespace around operator ./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:39:1: E302 expected 2 blank lines, found 1 ./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:42:1: E302 expected 2 blank lines, found 1 I'll wait and see what falls out of the API review before making any real changes. rob Updated API and addressed Martin's concerns. The regex must have been a bad copy/paste, it is fixed now. The design page has been updated as well. rob Hello, comments below, in the right thread: 1) +Str( +'memberprincipal', +label=_('Failed principals'), +), +Str( +'ipaallowedtarget', +label=_('Failed targets'), +), +Str( +'servicedelegationrule', +label=_('principal member'), +), Are these names correct? # ipa servicedelegationrule-find -- 1 service delegation rule matched -- Delegation name: ipa-http-delegation Allowed Target: ipa-ldap-delegation-targets, ipa-cifs-delegation-targets Failed principals: HTTP/vm-093.example@example.com Fixed. 2) + pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', +pattern_errmsg='may only include letters, numbers, _, -, ., ' + 'and a space inside', This regex does not allow space inside In [6]: print re.match(pattern, 'lalalala lalala') None Fixed. I'm tempted to just drop this regex entirely. Other plugins have no such restrictions, but this should work better now. 3) +yield Str('%s*' % name, cli_name='%ss' % name, doc=doc, + label=_('member %s') % name, + csv=True, alwaysask=True) IMHO CSV values should not be supported. Honza told me, the option doesn't work anyway. Yeah, a copy and paste issue. Patch with minor fixes attached. I removed unused code and PEP8 complains Incorporated and fixed a number of other things, including some typos in the doc examples. rob Thank you, ACK! Pushed to master: a92328452dced34d6d6df7ad6fe585563bb909f6 -- 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] Topology plugin quirks
On 06/03/2015 12:23 PM, Ludwig Krispenz wrote: On 06/03/2015 11:51 AM, Oleg Fayans wrote: I confirm every point of this. did you test with all the latest patches applied ? In your issues you refer to crashes, the crashes reported should be resolved, if you still have crashes, pleas provide a core dump or scenario to reproduce the crash. With patch0009 ipa-replica-manage del worked for me I thing I have missed this patch before, I will test it again with patch 0009 applied. On 06/03/2015 11:37 AM, Martin Babinsky wrote: Hi everyone, I have been playing with the topology related patches and I have encountered a few issues that I would like to address in this thread: 1.) When replica install for whatever reason crashes _after_ the setup of replication agreements etc., it leaves the topology plugin with dangling segment pointing to the dysfunctional node. An attempt to delete it leads to: ipa: ERROR: Server is unwilling to perform: Removal of Segment disconnects topology.Deletion not allowed. Furthermore, any attempts to delete a segment (even a properly setup one) lead to the same very error. And you cannot reinstall the crashed replica because it complains about existing replication agreements. It would probably help to be able to force-remove the segments if one of the endpoints doesn't exist/respond. 2.) I was not able to figure out a way remove replica from the topology without explosions or tampering 'cn=masters,cn=ipa,cn=etc,$SUFFIX'. Obviously ipa-replica-manage del doesn't work anymore (I have tried just for fun, it leads to SIGSEGV of the host's dirsrv and leaves dangling segments to offending replica, leading to point 1). I managed to remove replica from the topology only by directly uninstalling FreeIPA on the node and then deleting its' host entry from 'cn=masters'. Only after this was the plugin able to automagically removed the segments pointing to/from removed node. The design page suggests that it should be enough to uninstall IPA server on the replica. The plugin would then pick-up the dangling segments and remove them automatically. However, this behavior seems to require additional modification of the uninstall procedure (e.g. the uninstalling replica should remove its' entry from cn=masters). 3.) It seems that the removal of topology suffixes containing functioning segments is not handled well. I once tried to do this and it led to segmentation fault on the dirsrv instance. What is the expected behavior in this scenario? -- 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] Topology plugin quirks
On 06/03/2015 11:37 AM, Martin Babinsky wrote: Hi everyone, I have been playing with the topology related patches and I have encountered a few issues that I would like to address in this thread: Additional stuff: 1. was able to add duplicate segment - same left and right node - same direction - different cn It did not allow me to remove it: Server is unwilling to perform: Removal of Segment disconnects topology.Deletion not allowed. 2. topology plugin allows to create reflexive relation from the invalid duplicates(#1): A - B A - B to A - A B - B I.E. effective disconnect it is forbidden in `ipa topologysegment-mod` but I think that even the plugin should not allow that 3. attempt to delete the invalid reflexive or duplicate segment ends with: Server is unwilling to perform: Removal of Segment disconnects topology.Deletion not allowed. -- 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] Topology plugin quirks
Hi Ludwig On 06/03/2015 12:23 PM, Ludwig Krispenz wrote: On 06/03/2015 11:51 AM, Oleg Fayans wrote: I confirm every point of this. did you test with all the latest patches applied ? In your issues you refer to crashes, the crashes reported should be resolved, if you still have crashes, pleas provide a core dump or scenario to reproduce the crash. With patch0009 ipa-replica-manage del worked for me Yep, patch 0009 is applied. The full list of patches applied on top of the master branch (at it's state yesterday at 10 PM) is as follows: freeipa-lkrispen-0007-replica-install-fails-with-domain-level-1.patch freeipa-lkrispen-0008-plugin-uses-1-as-minimum-domain-level-to-become-acti.patch freeipa-lkrispen-0009-crash-when-removing-a-replica.patch freeipa-mbasti-0262-Installers-fix-remove-temporal-ccache.patch freeipa-pvoborni-0857-1-topology-ipa-management-commands.patch freeipa-pvoborni-0858-1-webui-IPA.command_dialog-a-new-dialog-base-class.patch freeipa-pvoborni-0859-1-webui-use-command_dialog-as-a-base-class-for-passwor.patch freeipa-pvoborni-0860-1-webui-make-usage-of-all-in-details-facet-optional.patch freeipa-pvoborni-0861-2-webui-topology-plugin.patch freeipa-pvoborni-0862-webui-configurable-refresh-command.patch The scenario is pretty basic: 1. 3 fedora-21 vms with the latest directory server packages from mreynolds repo: 389-ds-base-2015_06_02-1.fc21.x86_64 2. setup master on one of them, prepare gpg files for two replicas 3. setup replicas using these gpg files. 4. Try to remove one of the replicas using command `ipa topologysegment-del` 5. Try to create a new user via web UI on any of the replicas On 06/03/2015 11:37 AM, Martin Babinsky wrote: Hi everyone, I have been playing with the topology related patches and I have encountered a few issues that I would like to address in this thread: 1.) When replica install for whatever reason crashes _after_ the setup of replication agreements etc., it leaves the topology plugin with dangling segment pointing to the dysfunctional node. An attempt to delete it leads to: ipa: ERROR: Server is unwilling to perform: Removal of Segment disconnects topology.Deletion not allowed. Furthermore, any attempts to delete a segment (even a properly setup one) lead to the same very error. And you cannot reinstall the crashed replica because it complains about existing replication agreements. It would probably help to be able to force-remove the segments if one of the endpoints doesn't exist/respond. 2.) I was not able to figure out a way remove replica from the topology without explosions or tampering 'cn=masters,cn=ipa,cn=etc,$SUFFIX'. Obviously ipa-replica-manage del doesn't work anymore (I have tried just for fun, it leads to SIGSEGV of the host's dirsrv and leaves dangling segments to offending replica, leading to point 1). I managed to remove replica from the topology only by directly uninstalling FreeIPA on the node and then deleting its' host entry from 'cn=masters'. Only after this was the plugin able to automagically removed the segments pointing to/from removed node. The design page suggests that it should be enough to uninstall IPA server on the replica. The plugin would then pick-up the dangling segments and remove them automatically. However, this behavior seems to require additional modification of the uninstall procedure (e.g. the uninstalling replica should remove its' entry from cn=masters). 3.) It seems that the removal of topology suffixes containing functioning segments is not handled well. I once tried to do this and it led to segmentation fault on the dirsrv instance. What is the expected behavior in this scenario? -- Oleg Fayans Quality Engineer FreeIPA team RedHat. -- 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] 857 topology: ipa management commands
On 06/03/2015 02:38 PM, Martin Babinsky wrote: On 06/03/2015 01:34 PM, Petr Vobornik wrote: On 06/03/2015 10:59 AM, Martin Babinsky wrote: On 06/03/2015 10:52 AM, Martin Babinsky wrote: On 05/26/2015 03:31 PM, Petr Vobornik wrote: On 05/26/2015 12:19 PM, Petr Vobornik wrote: this patch is based on top of my patch #856 and tbabej' s 325-9. Obsoletes Ludwig's 0006. ipalib part of topology management Design: - http://www.freeipa.org/page/V4/Manage_replication_topology https://fedorahosted.org/freeipa/ticket/4302 New version attached: - domainlevel_show usage changed to domainlevel_get - updated VERSION - added more attrs to default_attributes Hi Petr, the commands themselves seem to work just fine. I had encountered some quirks in the underlying topology plugin, but I will address them in a different thread in order to keep the discussion relevant to the reviewed patch. I have some minor coomments below: 1.) IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=121 -# Last change: pvoborni - added server-find and server-show +IPA_API_VERSION_MINOR=122 +# Last change: pvoborni - added topology management commands Several people were touching API in the meantime so please double-check that you have correct VERSION and regenerate API.txt Patch rebased. 2.) +Str( +'nsds5replicatedattributelist?', +cli_name='replattrs', +label='Attributes to replicate', +doc=_('Attributes that are not replicated to a consumer server ' + 'during a fractional update. E.g., `(objectclass=*) ' + '$ EXCLUDE accountlockout memberof'), +), +Str( +'nsds5replicatedattributelisttotal?', +cli_name='replattrstotal', +label=_('Attributes for total update'), +doc=_('Attributes that are not replicated to a consumer server ' + 'during a total update. E.g. (objectclass=*) $ EXCLUDE ' + 'accountlockout'), The descriptions of these two options confused me greatly, are these attributes supposed to be replicated or not, or is there some more complex logic behind them that I failed to grasp? I am cc'ing Ludwig, he can probably explain them to us and then we can decide whether we may alter the descriptions to be less confusing. 3.) +takes_params = ( +Str( +'cn', +cli_name='name', +primary_key=True, +label=_('Suffix name'), +), +Str( +'iparepltopoconfroot', +maxlength=255, +cli_name='suffix', +label=_('Suffix to be managed'), +normalizer=lambda value: value.lower(), +), +) This also confused me at first, I suggest to change the label of 'iparepltopoconfroot' to something like 'LDAP suffix to be managed' or 'LDAP subtree to be managed'. Changed to 'LDAP suffix to be managed' 4.) There is currently no way to rename existing topology segments/suffixes. In the case of hosts with funky FQDN's (pointing at you, ABC lab), the segment cn's created during replica installs are mearly impossible to remember and it would be nice to rename them to something more manageable. However, this is not related to core functionality and can be a subject of a separate patch once this gets pushed. That's all from my side. I also forgot to ask what is the expected policy when deleting a non-empty topology suffix. If this is not supported and you have to first remove all segments and then the suffix itself, the 'topologysuffix-del' command should issue an error pointing the user to correct procedure. Do we have a use case for creation or deletion of topology suffix? That's a good question. Anyway, I have noticed couple more things: 1.) it seems that there some of unused imports in topology.py. Please investigate whether all of them are really needed. Fixed 2.) +from ipalib.plugins.baseldap import * +from ipalib.plugins import baseldap I do not like that starred import at all. Either import the particular classes you use (like e.g. in basuser.py), or just leave the second import statetement and use the appropriate namespace (baseldap.LDAPObject etc.). Fixed 3.) there are couple of pep8 complaints, please try to fix them unless it impairs readability: ./ipalib/constants.py:121:80: E501 line too long (81 79 characters) ./ipalib/plugins/topology.py:72:80: E501 line too long (88 79 characters) ./ipalib/plugins/topology.py:73:26: E131 continuation line unaligned for hanging indent ./ipalib/plugins/topology.py:73:80: E501 line too long (93 79 characters) ./ipalib/plugins/topology.py:103:80: E501 line too long (80 79 characters) ./ipalib/plugins/topology.py:111:80: E501 line too long (80 79 characters) ./ipalib/plugins/topology.py:207:80: E501 line too long (80 79 characters) ./ipalib/plugins/topology.py:232:80: E501 line too long (80 79 characters) won't fix
Re: [Freeipa-devel] [PATCH] Password vault
On Wed, 03 Jun 2015, Endi Sukma Dewata wrote: On 6/3/2015 1:41 AM, Martin Kosek wrote: On 06/02/2015 11:22 PM, Alexander Bokovoy wrote: On Tue, 02 Jun 2015, Endi Sukma Dewata wrote: I think ideally the client and server code should be in separate files (so they can be deployed separately too), but the framework doesn't seem to allow that. This exactly the case we have to use here and we are using that in trusts case as well -- some code has to run on server only and shouldn't cause to install Samba related packages on the client. This is because IPA client is actually using the same IPA plugins that server uses, to have access to the API calls metadata and client-side callbacks are defined in the same place where server-side callbacks are. It is IPA framework design, so we have to use what we have. This is planned to be changed BTW, when we start with the Thin Client concept and have different code/plugins for FreeIPA server side and client side. Is there a ticket for this? In other words, it is not necessarily an evil under conditions we are dealing with. Having to use the same plugins for client and server is a framework limitation/poor design. Having to use conditional imports to work around the limitation is a bad programming practice. The fact that trust plugin has to implement a similar workaround is not a justification, it just shows that the problem is not vault-specific. There is another thing. Even when splitting client/server sides, we'll need to check on the server side that certain functionality is available. In trust case we have ID Views (a separate plugin) which does use information about trusts to resolve users from AD to their normalized references (SIDs) and few other places would be depending on functionality only provided when Samba packages are installed. To continue your approach, we would need to split also server-side parts of plugins into separate callable units that would only be provided and called when appropriate rpm subpackages are installed. This is unneeded complication in place where we can simply handle dependencies in run time and make sure the packaging deps are managed separately. -- / 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 0026-0028] Fix nits in user-visible output
On 14/04/15 09:43, Petr Spacek wrote: On 14.4.2015 09:10, Martin Kosek wrote: On 04/13/2015 05:05 PM, Petr Spacek wrote: Hello, documentation team proposed few changes in user-visible messages so here it is. It was not worth a ticket and related overhead. The changes look OK to me. I would just have one (prudish) request to not add nazi reference to our git history - whether they are grammar or not. Please keep the git technical :-) Sure, here is the same patch with modified commit message. 0026 ACK 0027-2 ACK 0028 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] Password vault
On 6/3/2015 8:52 AM, Alexander Bokovoy wrote: Having to use the same plugins for client and server is a framework limitation/poor design. Having to use conditional imports to work around the limitation is a bad programming practice. The fact that trust plugin has to implement a similar workaround is not a justification, it just shows that the problem is not vault-specific. There is another thing. Even when splitting client/server sides, we'll need to check on the server side that certain functionality is available. In trust case we have ID Views (a separate plugin) which does use information about trusts to resolve users from AD to their normalized references (SIDs) and few other places would be depending on functionality only provided when Samba packages are installed. To continue your approach, we would need to split also server-side parts of plugins into separate callable units that would only be provided and called when appropriate rpm subpackages are installed. This is unneeded complication in place where we can simply handle dependencies in run time and make sure the packaging deps are managed separately. So there are two issues: 1. Conditional imports due to combined client and server plugin. 2. Conditional imports due to feature availability. Issue #1 is generic and I think we pretty much agree that this is supposed to be fixed somehow. Issue #2 is plugin-specific. I think there are different ways to solve this, some might be better than others. The solution that you pick will only affect that particular plugin and should not be generalized to other plugins or to justify #1. In my opinion a code should have a fixed dependency, but some features can be enabled/disabled based on the configuration. Enabling a feature shouldn't be based on package installation because people might install a package for different reasons. So the code may look something like this: import module if feature is enabled: do something with the module It shouldn't be like this: if package is installed: import module if package is installed: do something with the module Of course this assumes that the package is lightweight enough to be installed regardless whether it will be used. I don't know if it's applicable to your case, but again, there are different ways to address issue #2. -- Endi S. Dewata -- 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 0010] KeyError raised upon replica installation
On 06/03/2015 04:10 PM, Petr Vobornik wrote: On 06/02/2015 02:20 PM, Ludwig Krispenz wrote: replicas installed from older versions do not have a binddn group just accept the errror ACK Pushed to master: 8457edc14dade724b486540800bcdafb7d9a6f76 Note that this group will be populated later. IMHO it should be done as a part of domain-level raise procedure before setting the new level. As said in other mail, I am not sure why we should be overloading domain-level raise command that way. I thought, we will create this group when the first replica upgrades to 4.2. Whenever a new replica is added/upgraded, it's principal will be added to the group also (even if Domain Level is 0). Domain Level 1 means that all replicas are 4.2 and thus the group is fully populated and Topology can be used. -- 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 0010] KeyError raised upon replica installation
On 06/02/2015 02:20 PM, Ludwig Krispenz wrote: replicas installed from older versions do not have a binddn group just accept the errror ACK Pushed to master: 8457edc14dade724b486540800bcdafb7d9a6f76 Note that this group will be populated later. IMHO it should be done as a part of domain-level raise procedure before setting the new level. -- 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 0010] KeyError raised upon replica installation
On 06/03/2015 04:10 PM, Petr Vobornik wrote: On 06/02/2015 02:20 PM, Ludwig Krispenz wrote: replicas installed from older versions do not have a binddn group just accept the errror ACK Pushed to master: 8457edc14dade724b486540800bcdafb7d9a6f76 Note that this group will be populated later. if you start with 4.2 the group is created and populated when the ldap principals are added to the replica as binddns. Only if you install the replica from an older version the database is initialized from the older master and it is gone. so it has to be populated later. IMHO it should be done as a part of domain-level raise procedure before setting the new level. It could also be populated as soon as the first 4.2 replica is installed, it doesn't require any schema changes and can be added/replicated to older serevrs as well -- 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] Use Exception class instead of BaseException
On 06/01/2015 06:33 AM, Niranjan wrote: Greetings, I would like to present patch for replacing StandardError exception with Exception class in ipapython/adminutil.py. Also replacing BaseException class with Exception class. Though the use of StandardError is many places. I would like to start with ipapython/adminutil.py This is my first patch. Please let me know if my approach on this is correct. Regards Niranjan 0001-Use-Exception-class-instead-of-BaseException.patch From 018312f76952ea86c8c6e2396657e0531d2d61ba Mon Sep 17 00:00:00 2001 From: Niranjan Mallapadi mrniran...@redhat.com Date: Mon, 1 Jun 2015 09:41:05 +0530 Subject: [PATCH] Use Exception class instead of BaseException 1. Replace BaseException with Exception class. I don't see a reason for this change. This is top-level CLI code that handles calling our Python library. We really do want to catch all exceptions here, including KeyboardInterrupt and SystemExit. 2. Remove StandardError and use Exception class. StandError is deprecated (Python3) I'm okay with this change, as long as tests still pass. 3 .From python3.0 use of , is not recommended, instead use as keyword (PEP 3110) +1 -- Petr Viktorin -- 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] Use Exception class instead of BaseException
On 01/06/15 06:33, Niranjan wrote: Greetings, I would like to present patch for replacing StandardError exception with Exception class in ipapython/adminutil.py. Also replacing BaseException class with Exception class. Though the use of StandardError is many places. I would like to start with ipapython/adminutil.py This is my first patch. Please let me know if my approach on this is correct. Regards Niranjan Thank you, I have another objection: 1) Please do not copy/paste code, use this for except except (Exception, SystemExit) as exception: 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] KeyError raised upon replica installation
BTW, Ludwig, it seems you forgot to attach the 0010 patch to your email. At least, your first letter from 06/02/2015 05:08 PM, containing PATCH 0010 does not have the actual patch On 06/03/2015 02:53 PM, Oleg Fayans wrote: Hi Ludwig, I'll rebuild the packages again with the whole set of patches including 0010 and 0011 and try again. Thanks! On 06/03/2015 02:21 PM, Ludwig Krispenz wrote: On 06/03/2015 02:05 PM, Oleg Fayans wrote: Update: The original error occurs ONLY when installing a replica from a gpg file prepared on a master running FreeIPA 4.1.2. but this should be covere with patch 0010 If The master runs the upstream code, it works. On 06/02/2015 02:11 PM, Martin Babinsky wrote: On 06/02/2015 02:07 PM, Martin Babinsky wrote: On 06/02/2015 12:09 PM, Oleg Fayans wrote: Hi all, The following error was caught during replica installation (I used all the latest patches from Ludwig and Martin Basti): root@localhost:/home/ofayans/rpms]$ ipa-replica-install --setup-ca --setup-dns --forwarder 10.38.5.26 /var/lib/ipa/replica-info-replica1.zaeba.li.gpg Directory Manager (existing master) password: Existing BIND configuration detected, overwrite? [no]: yes Adding [192.168.122.210 replica1.zaeba.li] to your /etc/hosts file Checking forwarders, please wait ... Using reverse zone(s) 122.168.192.in-addr.arpa. Run connection check to master Check connection from replica to remote master 'upgrademaster.zaeba.li': Directory Service: Unsecure port (389): OK Directory Service: Secure port (636): OK Kerberos KDC: TCP (88): OK Kerberos Kpasswd: TCP (464): OK HTTP Server: Unsecure port (80): OK HTTP Server: Secure port (443): OK The following list of ports use UDP protocol and would need to be checked manually: Kerberos KDC: UDP (88): SKIPPED Kerberos Kpasswd: UDP (464): SKIPPED Connection from replica to master is OK. Start listening on required ports for remote master check Get credentials to log in to remote master ad...@zaeba.li password: Check SSH connection to remote master Execute check on remote master Check connection from master to remote replica 'replica1.zaeba.li': Directory Service: Unsecure port (389): OK Directory Service: Secure port (636): OK Kerberos KDC: TCP (88): OK Kerberos KDC: UDP (88): OK Kerberos Kpasswd: TCP (464): OK Kerberos Kpasswd: UDP (464): OK HTTP Server: Unsecure port (80): OK HTTP Server: Secure port (443): OK Connection from master to replica is OK. Connection check OK Configuring NTP daemon (ntpd) [1/4]: stopping ntpd [2/4]: writing configuration [3/4]: configuring ntpd to start on boot [4/4]: starting ntpd Done configuring NTP daemon (ntpd). Configuring directory server (dirsrv): Estimated time 1 minute [1/37]: creating directory server user [2/37]: creating directory server instance [3/37]: adding default schema [4/37]: enabling memberof plugin [5/37]: enabling winsync plugin [6/37]: configuring replication version plugin [7/37]: enabling IPA enrollment plugin [8/37]: enabling ldapi [9/37]: configuring uniqueness plugin [10/37]: configuring uuid plugin [11/37]: configuring modrdn plugin [12/37]: configuring DNS plugin [13/37]: enabling entryUSN plugin [14/37]: configuring lockout plugin [15/37]: configuring topology plugin [16/37]: creating indices [17/37]: enabling referential integrity plugin [18/37]: configuring ssl for ds instance [19/37]: configuring certmap.conf [20/37]: configure autobind for root [21/37]: configure new location for managed entries [22/37]: configure dirsrv ccache [23/37]: enable SASL mapping fallback [24/37]: restarting directory server [25/37]: setting up initial replication Starting replication, please wait until this has completed. Update in progress, 7 seconds elapsed Update succeeded [26/37]: updating schema [27/37]: setting Auto Member configuration [28/37]: enabling S4U2Proxy delegation [29/37]: importing CA certificates from LDAP [30/37]: initializing group membership [31/37]: adding master entry ipa : CRITICAL Failed to load master-entry.ldif: Command ''/usr/bin/ldapmodify' '-v' '-f' '/tmp/tmpFlM3mD' '-H' 'ldap://replica1.zaeba.li:389' '-x' '-D' 'cn=Directory Manager' '-y' '/tmp/tmpk_R0Lm'' returned non-zero exit status 68 [32/37]: initializing domain level [33/37]: configuring Posix uid/gid generation [34/37]: adding replication acis [35/37]: enabling compatibility plugin [36/37]: tuning directory server [37/37]: configuring directory to start on boot Done configuring directory server (dirsrv). Configuring certificate server (pki-tomcatd): Estimated time 3 minutes 30 seconds [1/21]: creating certificate server user [2/21]: configuring certificate server instance [3/21]: stopping certificate server instance to update CS.cfg [4/21]: backing up CS.cfg [5/21]: disabling nonces [6/21]: set up CRL publishing
Re: [Freeipa-devel] [PATCH] Password vault
On 6/3/2015 1:41 AM, Martin Kosek wrote: On 06/02/2015 11:22 PM, Alexander Bokovoy wrote: On Tue, 02 Jun 2015, Endi Sukma Dewata wrote: I think ideally the client and server code should be in separate files (so they can be deployed separately too), but the framework doesn't seem to allow that. This exactly the case we have to use here and we are using that in trusts case as well -- some code has to run on server only and shouldn't cause to install Samba related packages on the client. This is because IPA client is actually using the same IPA plugins that server uses, to have access to the API calls metadata and client-side callbacks are defined in the same place where server-side callbacks are. It is IPA framework design, so we have to use what we have. This is planned to be changed BTW, when we start with the Thin Client concept and have different code/plugins for FreeIPA server side and client side. Is there a ticket for this? In other words, it is not necessarily an evil under conditions we are dealing with. Having to use the same plugins for client and server is a framework limitation/poor design. Having to use conditional imports to work around the limitation is a bad programming practice. The fact that trust plugin has to implement a similar workaround is not a justification, it just shows that the problem is not vault-specific. -- Endi S. Dewata -- 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] KeyError raised upon replica installation
On 06/03/2015 02:05 PM, Oleg Fayans wrote: Update: The original error occurs ONLY when installing a replica from a gpg file prepared on a master running FreeIPA 4.1.2. but this should be covere with patch 0010 If The master runs the upstream code, it works. On 06/02/2015 02:11 PM, Martin Babinsky wrote: On 06/02/2015 02:07 PM, Martin Babinsky wrote: On 06/02/2015 12:09 PM, Oleg Fayans wrote: Hi all, The following error was caught during replica installation (I used all the latest patches from Ludwig and Martin Basti): root@localhost:/home/ofayans/rpms]$ ipa-replica-install --setup-ca --setup-dns --forwarder 10.38.5.26 /var/lib/ipa/replica-info-replica1.zaeba.li.gpg Directory Manager (existing master) password: Existing BIND configuration detected, overwrite? [no]: yes Adding [192.168.122.210 replica1.zaeba.li] to your /etc/hosts file Checking forwarders, please wait ... Using reverse zone(s) 122.168.192.in-addr.arpa. Run connection check to master Check connection from replica to remote master 'upgrademaster.zaeba.li': Directory Service: Unsecure port (389): OK Directory Service: Secure port (636): OK Kerberos KDC: TCP (88): OK Kerberos Kpasswd: TCP (464): OK HTTP Server: Unsecure port (80): OK HTTP Server: Secure port (443): OK The following list of ports use UDP protocol and would need to be checked manually: Kerberos KDC: UDP (88): SKIPPED Kerberos Kpasswd: UDP (464): SKIPPED Connection from replica to master is OK. Start listening on required ports for remote master check Get credentials to log in to remote master ad...@zaeba.li password: Check SSH connection to remote master Execute check on remote master Check connection from master to remote replica 'replica1.zaeba.li': Directory Service: Unsecure port (389): OK Directory Service: Secure port (636): OK Kerberos KDC: TCP (88): OK Kerberos KDC: UDP (88): OK Kerberos Kpasswd: TCP (464): OK Kerberos Kpasswd: UDP (464): OK HTTP Server: Unsecure port (80): OK HTTP Server: Secure port (443): OK Connection from master to replica is OK. Connection check OK Configuring NTP daemon (ntpd) [1/4]: stopping ntpd [2/4]: writing configuration [3/4]: configuring ntpd to start on boot [4/4]: starting ntpd Done configuring NTP daemon (ntpd). Configuring directory server (dirsrv): Estimated time 1 minute [1/37]: creating directory server user [2/37]: creating directory server instance [3/37]: adding default schema [4/37]: enabling memberof plugin [5/37]: enabling winsync plugin [6/37]: configuring replication version plugin [7/37]: enabling IPA enrollment plugin [8/37]: enabling ldapi [9/37]: configuring uniqueness plugin [10/37]: configuring uuid plugin [11/37]: configuring modrdn plugin [12/37]: configuring DNS plugin [13/37]: enabling entryUSN plugin [14/37]: configuring lockout plugin [15/37]: configuring topology plugin [16/37]: creating indices [17/37]: enabling referential integrity plugin [18/37]: configuring ssl for ds instance [19/37]: configuring certmap.conf [20/37]: configure autobind for root [21/37]: configure new location for managed entries [22/37]: configure dirsrv ccache [23/37]: enable SASL mapping fallback [24/37]: restarting directory server [25/37]: setting up initial replication Starting replication, please wait until this has completed. Update in progress, 7 seconds elapsed Update succeeded [26/37]: updating schema [27/37]: setting Auto Member configuration [28/37]: enabling S4U2Proxy delegation [29/37]: importing CA certificates from LDAP [30/37]: initializing group membership [31/37]: adding master entry ipa : CRITICAL Failed to load master-entry.ldif: Command ''/usr/bin/ldapmodify' '-v' '-f' '/tmp/tmpFlM3mD' '-H' 'ldap://replica1.zaeba.li:389' '-x' '-D' 'cn=Directory Manager' '-y' '/tmp/tmpk_R0Lm'' returned non-zero exit status 68 [32/37]: initializing domain level [33/37]: configuring Posix uid/gid generation [34/37]: adding replication acis [35/37]: enabling compatibility plugin [36/37]: tuning directory server [37/37]: configuring directory to start on boot Done configuring directory server (dirsrv). Configuring certificate server (pki-tomcatd): Estimated time 3 minutes 30 seconds [1/21]: creating certificate server user [2/21]: configuring certificate server instance [3/21]: stopping certificate server instance to update CS.cfg [4/21]: backing up CS.cfg [5/21]: disabling nonces [6/21]: set up CRL publishing [7/21]: enable PKIX certificate path discovery and validation [8/21]: starting certificate server instance [9/21]: creating RA agent certificate database [10/21]: importing CA chain to RA certificate database [11/21]: fixing RA database permissions [12/21]: setting up signing cert profile [13/21]: set certificate subject base [14/21]: enabling Subject Key Identifier [15/21]:
[Freeipa-devel] [PATCH 0011] check-for-existing-and-self-referential-segments
Hi, this should prevent adding duplicate segments or segments with same start and end node From 759790e3c6c87ebe75610fdcfda06c6d4bc00475 Mon Sep 17 00:00:00 2001 From: Ludwig Krispenz lkris...@redhat.com Date: Wed, 3 Jun 2015 14:22:52 +0200 Subject: [PATCH] check for existing and self referential segments --- daemons/ipa-slapi-plugins/topology/topology_pre.c | 30 +++ 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/daemons/ipa-slapi-plugins/topology/topology_pre.c b/daemons/ipa-slapi-plugins/topology/topology_pre.c index 528f72b69dfe0e57c5312e558c6e0ac5f58801fb..0a0cd65b592e2dc796a179e035598e5f641bb01e 100644 --- a/daemons/ipa-slapi-plugins/topology/topology_pre.c +++ b/daemons/ipa-slapi-plugins/topology/topology_pre.c @@ -279,21 +279,31 @@ ipa_topo_check_connect_reject(Slapi_PBlock *pb) if (pi 0 == strcasecmp(pi, ipa_topo_get_plugin_id())) { return 0; } -slapi_pblock_get(pb,SLAPI_DELETE_EXISTING_ENTRY,add_entry); +slapi_pblock_get(pb,SLAPI_ADD_ENTRY,add_entry); if (TOPO_SEGMENT_ENTRY != ipa_topo_check_entry_type(add_entry)) { return 0; } else { /* a new segment is added * verify that the segment does not yet exist */ -TopoReplicaSegment *tsegm; -TopoReplica *tconf = ipa_topo_util_get_conf_for_segment(add_entry); -tsegm = ipa_topo_util_find_segment(tconf, add_entry); -if (tsegm) { -slapi_log_error(SLAPI_LOG_FATAL, IPA_TOPO_PLUGIN_SUBSYSTEM, -segment to be added does already exist\n); -rc = 1; +char *leftnode = slapi_entry_attr_get_charptr(add_entry,ipaReplTopoSegmentLeftNode); +char *rightnode = slapi_entry_attr_get_charptr(add_entry,ipaReplTopoSegmentRightNode); +if (0 == strcasecmp(leftnode,rightnode)) { +slapi_log_error(SLAPI_LOG_FATAL, IPA_TOPO_PLUGIN_SUBSYSTEM, +segment is self referential\n); +rc = 1; +} else { +TopoReplicaSegment *tsegm; +TopoReplica *tconf = ipa_topo_util_get_conf_for_segment(add_entry); +tsegm = ipa_topo_util_find_segment(tconf, add_entry); +if (tsegm) { +slapi_log_error(SLAPI_LOG_FATAL, IPA_TOPO_PLUGIN_SUBSYSTEM, +segment to be added does already exist\n); +rc = 1; +} } +slapi_ch_free_string(leftnode); +slapi_ch_free_string(rightnode); } return rc; } @@ -378,8 +388,8 @@ int ipa_topo_pre_add(Slapi_PBlock *pb) } else if (ipa_topo_check_connect_reject(pb)) { int rc = LDAP_UNWILLING_TO_PERFORM; char *errtxt; -errtxt = slapi_ch_smprintf(Segment already exists in topology. - Add rejected.\n); +errtxt = slapi_ch_smprintf(Segment already exists in topology or +is self referential. Add rejected.\n); slapi_pblock_set(pb, SLAPI_PB_RESULT_TEXT, errtxt); slapi_pblock_set(pb, SLAPI_RESULT_CODE, rc); result = SLAPI_PLUGIN_FAILURE; -- 2.1.0 -- 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] Password vault
On 6/2/2015 1:34 PM, Simo Sorce wrote: On Tue, 2015-06-02 at 12:04 +0200, Jan Cholasta wrote: Dne 2.6.2015 v 02:02 Endi Sukma Dewata napsal(a): On 5/28/2015 12:46 AM, Jan Cholasta wrote: On a related note, since KRA is optional, can we move the vaults container to cn=kra,cn=vaults? This is the convetion used by the other optional components (DNS and recently CA). I mean cn=vaults,cn=kra of course. If you are talking about the o=kra,PKI suffix, I'm not sure whether the IPA framework will work with it. If you are talking about adding a new cn=kra,IPA suffix entry on top of cn=vaults, what is the purpose of this entry? Is the entry going to be created/deleted automatically when the KRA is installed/removed? Is it going to be used for something else other than vaults? I'm talking about cn=kra,IPA suffix. It should be created only when KRA is installed, although I think this can be done later after the release, moving vaults to cn=kra should be good enough for now. It's going to be used for everything KRA-specific. There are a lot of questions that need to be answered before we can make this change. This is about sticking to a convention, which everyone should do, and everyone except KRA already does. I'm sorry I didn't realize this earlier, but the change must be done now. We probably should revisit this issue after the core vault functionality is added. We can't revisit it later because after release we are stuck with whatever is there forever. See attachment for a patch which implements the change. Shouldn't we s/kra/vault/ ? After all the feature is called Vault, not KRA. Simo. Here are the options: 1. the original code uses cn=vaults,IPA suffix. 2. Honza proposed cn=vaults,cn=kra,IPA suffix, ACKed by Martin. Are you proposing a third option cn=vaults,cn=vault,IPA suffix or did you mean the first option? I think the first option would make more sense since we're not introducing KRA to the end user, but I'll let the IPA team decide. My next patch will be based on whatever pushed at the time. -- Endi S. Dewata -- 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] [PATCHES 0233-0234] DNSSEC: forwarders validation
On 18.5.2015 13:48, Martin Basti wrote: On 15/05/15 18:11, Petr Spacek wrote: On 7.5.2015 18:12, Martin Basti wrote: On 07/05/15 12:19, Petr Spacek wrote: On 7.5.2015 08:59, David Kupka wrote: On 05/06/2015 03:20 PM, Martin Basti wrote: On 05/05/15 15:00, Martin Basti wrote: On 30/04/15 15:37, David Kupka wrote: On 04/24/2015 02:56 PM, Martin Basti wrote: Patches attached. Hi, thanks for patches. 1. You changed message in DNSServerNotRespondingWarning class but not the test in ipatest/test_xmlrpc/test_dns_plugin.py nitpick. Please spell 'edns' correctly. I've seen several instances of 'ends'. Thank you, updated patches attached: * new error messages * logging to debug log server output if exception was raised * fixed test * fixed spelling Fixed tests (again) Updated patches attached The code looks good to me and tests are no longer broken. (I would prefer better fix of the tests but given that the priorities are different now it can wait.) Petr, can you please confirm that the patch set works for you? Sorry, NACK: $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236 Server will check DNS forwarder(s). This may take some time, please wait ... ipa: ERROR: an internal error has occurred # /var/log/httpd/error_log ipa: ERROR: non-public: AssertionError: Traceback (most recent call last): File /usr/lib/python2.7/site-packages/ipaserver/rpcserver.py, line 350, in wsgi_execute result = self.Command[name](*args, **options) File /usr/lib/python2.7/site-packages/ipalib/frontend.py, line 443, in __call__ ret = self.run(*args, **options) File /usr/lib/python2.7/site-packages/ipalib/frontend.py, line 760, in run return self.execute(*args, **options) File /usr/lib/python2.7/site-packages/ipalib/plugins/dns.py, line , in execute **options) File /usr/lib/python2.7/site-packages/ipalib/plugins/dns.py, line 4405, in _warning_if_forwarders_do_not_work log=self.log) File /usr/lib/python2.7/site-packages/ipalib/util.py, line 715, in validate_dnssec_zone_forwarder_step2 timeout=timeout) File /usr/lib/python2.7/site-packages/ipalib/util.py, line 610, in _resolve_record assert isinstance(nameserver_ip, basestring) AssertionError ipa: INFO: [jsonserver_session] admin@IPA.EXAMPLE: dnsforwardzone_add(DNS name ptr.test., idnsforwarders=(u'10.34.47.236',), all=False, raw=False, version=u'2.116'): AssertionError This is constantly reproducible in my vm-090.abc. Let me know if you want to take a look. I'm attaching little response.patch which improves compatibility with older python-dns packages. This patch allows IPA to work while error messages are simply not as nice as they could be with latest python-dns :-) check_fwd_msg.patch is a little nitpick, just to make sure everyone understands the message. BTW why some messages in check_forwarders() are printed using 'print' and others using logger? I would prefer to use logger for everything to make sure that logs contain all the information, including warnings. Thank you for your time! Thank you, fixed. I added missing except block after forwarders validation step2. I confirm that this works but I just discovered another deficiency. Setup: - DNSSEC validation is enabled on IPA server - forwarders uses fake TLD, e.g. 'test.' - remote DNS server is responding, supports EDNS0 and so on $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236 Server will check DNS forwarder(s). This may take some time, please wait ... ipa: WARNING: DNS server 10.34.78.90: query 'ptr.test. SOA': The DNS query name does not exist: ptr.test.. Huh? Let's check named log: forward zone 'ptr.test': loaded validating ./SOA: got insecure response; parent indicates it should be secure Sometimes I get SERVFAIL from IPA server, too. Unfortunately this check was the main reason for writing this patchset so we need to improve it. Maybe validate_dnssec_zone_forwarder_step2() could special-case NXDOMAIN and print the DNSSEC-validation-failed error, too? The problem is that it could trigger some false positives because NXDOMAIN may simply be caused by a delay somewhere. Any ideas? I add catch block for NXDOMAIN By the way, this is also weird: $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236 Server will check DNS forwarder(s). This may take some time, please wait ... ipa: ERROR: DNS forward zone with name ptr.test. already exists Is it actually doing the check even if the forward zone exists already? (This is just nitpick, not a blocker!) The first part is written by IPA client, it is not response from server. It is just written when user use --forwarder option. Updated patch attached. NACK, it does not work for me - it explodes when I try to add a forward zone: $ ipa dnsforwardzone-add ptr.test. --forwarder=192.0.2.1 ipa: ERROR: non-public: TypeError:
Re: [Freeipa-devel] [PATCH] Password vault
Dne 3.6.2015 v 14:58 Endi Sukma Dewata napsal(a): On 6/2/2015 1:34 PM, Simo Sorce wrote: On Tue, 2015-06-02 at 12:04 +0200, Jan Cholasta wrote: Dne 2.6.2015 v 02:02 Endi Sukma Dewata napsal(a): On 5/28/2015 12:46 AM, Jan Cholasta wrote: On a related note, since KRA is optional, can we move the vaults container to cn=kra,cn=vaults? This is the convetion used by the other optional components (DNS and recently CA). I mean cn=vaults,cn=kra of course. If you are talking about the o=kra,PKI suffix, I'm not sure whether the IPA framework will work with it. If you are talking about adding a new cn=kra,IPA suffix entry on top of cn=vaults, what is the purpose of this entry? Is the entry going to be created/deleted automatically when the KRA is installed/removed? Is it going to be used for something else other than vaults? I'm talking about cn=kra,IPA suffix. It should be created only when KRA is installed, although I think this can be done later after the release, moving vaults to cn=kra should be good enough for now. It's going to be used for everything KRA-specific. There are a lot of questions that need to be answered before we can make this change. This is about sticking to a convention, which everyone should do, and everyone except KRA already does. I'm sorry I didn't realize this earlier, but the change must be done now. We probably should revisit this issue after the core vault functionality is added. We can't revisit it later because after release we are stuck with whatever is there forever. See attachment for a patch which implements the change. Shouldn't we s/kra/vault/ ? After all the feature is called Vault, not KRA. Simo. Here are the options: 1. the original code uses cn=vaults,IPA suffix. 2. Honza proposed cn=vaults,cn=kra,IPA suffix, ACKed by Martin. Are you proposing a third option cn=vaults,cn=vault,IPA suffix or did you mean the first option? I think the first option would make more sense since we're not introducing KRA to the end user, but I'll let the IPA team decide. My next patch will be based on whatever pushed at the time. The DNs are not exposed to the end user, they are only relevant for our internal organization of entries. -- 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] Password vault
Dne 2.6.2015 v 02:00 Endi Sukma Dewata napsal(a): Please take a look at the updated patch. On 5/27/2015 12:39 AM, Jan Cholasta wrote: 21) vault_archive is not a retrieve operation, it should be based on LDAPUpdate instead of LDAPRetrieve. Or Command actually, since it does not do anything with LDAP. The same applies to vault_retrieve. The vault_archive does not actually modify the LDAP entry because it stores the data in KRA. It is actually an LDAPRetrieve operation because it needs to get the vault info before it can perform the archival operation. Same thing with vault_retrieve. It is not a LDAPRetrieve operation, because it has different semantics. Please use Command as base class and either use ldap2 for direct LDAP or call vault_show instead of hacking around LDAPRetrieve. It's been changed to inherit from LDAPQuery instead. NACK, it's not a LDAPQuery operation, because it has different semantics. There is more to a command than executing code, so you should use a correct base class. Changed to inherit from Command as requested. Now these commands no longer have a direct access to the vault object (self.obj) although they are accessing vault objects like other vault commands. Also now the vault name argument has to be added explicitly on each command. You can inherit from crud.Retrieve and crud.Update to get self.obj and the argument back. I tried this: class vault_retrieve(Command, crud.Retrieve): and it gave me an error: TypeError: Error when calling the metaclass bases Cannot create a consistent method resolution order (MRO) for bases Retrieve, Command I'm sticking with the original code since it works fine although not ideal. I'm not a Python expert, so if you know how to fix this properly please feel free to post a patch on top of this. The class hierarchy is as follows: frontend.Command frontend.Method crud.PKQuery crud.Retrieve cdur.Update So removing Command from the list of base classes should fix it. If KRA is not installed, vault-archive and vault-retrieve fail with internal error. Added a code to check KRA installation in all vault commands. If you know a way not to load the vault plugin if the KRA is not installed please let me know, that's probably even better. Not sure how that will work on the client side though. I see this has been already resolved in the other thread. The commands still behave differently based on whether they were called from API which was initialized with in_server set to True or False. That is unfortunately a restriction imposed by the framework. In order to guarantee the security, the vault is designed to have separate client and server code. The client code encrypts the secret, the server code forwards the encrypted secret to KRA. To archive a secret into a vault properly, you are supposed to call the client code. If you're calling the server code directly, you are responsible to do your own encryption (i.e. generating session key, nonce, and vault data). I understand why the code has to be separated, what I don't understand is why it is in fact *not* separated and crammed into a single command, making weird and undefined behavior possible. If another plugin wants to use vault, it should implement a client code which calls the vault client code to perform the archival from the client side. What is the use case for calling the vault API from the server side anyway? Wouldn't that defeat the purpose of having a vault? If a secret exists on the server side in an unencrypted form doesn't it mean the secret may already have been compromised? Server API is used not only by the server itself, but also by installers for example. Anyway the point is that there *can't* be a broken API like this, you should at least raise an error if the command is called from server API, although actually separating it into client and server parts would be preferable. There is no point in exposing the session_key, nonce and vault_data options in CLI when their value is always overwritten in forward(). I agree there is no need to expose them in CLI, but in this framework the API also defines the CLI. If there's a way to keep them in the server API but not expose them in the CLI please let me know. Or, if there's a way to define completely separate server API (without a matching client CLI) and client CLI (without a matching server API) that will work too. As I suggested above, you can split the commands into separate client and server commands. The client command should inherit from frontend.Local so that it is always executed locally and the server command should have a NO_CLI = True attribute so that it is not available in the CLI. Will this always succeed? +# deactivate vault record in KRA +response = kra_client.keys.list_keys( +client_key_id, pki.key.KeyClient.KEY_STATUS_ACTIVE) Yes. If there's no active keys it will
Re: [Freeipa-devel] KeyError raised upon replica installation
Update: The original error occurs ONLY when installing a replica from a gpg file prepared on a master running FreeIPA 4.1.2. If The master runs the upstream code, it works. On 06/02/2015 02:11 PM, Martin Babinsky wrote: On 06/02/2015 02:07 PM, Martin Babinsky wrote: On 06/02/2015 12:09 PM, Oleg Fayans wrote: Hi all, The following error was caught during replica installation (I used all the latest patches from Ludwig and Martin Basti): root@localhost:/home/ofayans/rpms]$ ipa-replica-install --setup-ca --setup-dns --forwarder 10.38.5.26 /var/lib/ipa/replica-info-replica1.zaeba.li.gpg Directory Manager (existing master) password: Existing BIND configuration detected, overwrite? [no]: yes Adding [192.168.122.210 replica1.zaeba.li] to your /etc/hosts file Checking forwarders, please wait ... Using reverse zone(s) 122.168.192.in-addr.arpa. Run connection check to master Check connection from replica to remote master 'upgrademaster.zaeba.li': Directory Service: Unsecure port (389): OK Directory Service: Secure port (636): OK Kerberos KDC: TCP (88): OK Kerberos Kpasswd: TCP (464): OK HTTP Server: Unsecure port (80): OK HTTP Server: Secure port (443): OK The following list of ports use UDP protocol and would need to be checked manually: Kerberos KDC: UDP (88): SKIPPED Kerberos Kpasswd: UDP (464): SKIPPED Connection from replica to master is OK. Start listening on required ports for remote master check Get credentials to log in to remote master ad...@zaeba.li password: Check SSH connection to remote master Execute check on remote master Check connection from master to remote replica 'replica1.zaeba.li': Directory Service: Unsecure port (389): OK Directory Service: Secure port (636): OK Kerberos KDC: TCP (88): OK Kerberos KDC: UDP (88): OK Kerberos Kpasswd: TCP (464): OK Kerberos Kpasswd: UDP (464): OK HTTP Server: Unsecure port (80): OK HTTP Server: Secure port (443): OK Connection from master to replica is OK. Connection check OK Configuring NTP daemon (ntpd) [1/4]: stopping ntpd [2/4]: writing configuration [3/4]: configuring ntpd to start on boot [4/4]: starting ntpd Done configuring NTP daemon (ntpd). Configuring directory server (dirsrv): Estimated time 1 minute [1/37]: creating directory server user [2/37]: creating directory server instance [3/37]: adding default schema [4/37]: enabling memberof plugin [5/37]: enabling winsync plugin [6/37]: configuring replication version plugin [7/37]: enabling IPA enrollment plugin [8/37]: enabling ldapi [9/37]: configuring uniqueness plugin [10/37]: configuring uuid plugin [11/37]: configuring modrdn plugin [12/37]: configuring DNS plugin [13/37]: enabling entryUSN plugin [14/37]: configuring lockout plugin [15/37]: configuring topology plugin [16/37]: creating indices [17/37]: enabling referential integrity plugin [18/37]: configuring ssl for ds instance [19/37]: configuring certmap.conf [20/37]: configure autobind for root [21/37]: configure new location for managed entries [22/37]: configure dirsrv ccache [23/37]: enable SASL mapping fallback [24/37]: restarting directory server [25/37]: setting up initial replication Starting replication, please wait until this has completed. Update in progress, 7 seconds elapsed Update succeeded [26/37]: updating schema [27/37]: setting Auto Member configuration [28/37]: enabling S4U2Proxy delegation [29/37]: importing CA certificates from LDAP [30/37]: initializing group membership [31/37]: adding master entry ipa : CRITICAL Failed to load master-entry.ldif: Command ''/usr/bin/ldapmodify' '-v' '-f' '/tmp/tmpFlM3mD' '-H' 'ldap://replica1.zaeba.li:389' '-x' '-D' 'cn=Directory Manager' '-y' '/tmp/tmpk_R0Lm'' returned non-zero exit status 68 [32/37]: initializing domain level [33/37]: configuring Posix uid/gid generation [34/37]: adding replication acis [35/37]: enabling compatibility plugin [36/37]: tuning directory server [37/37]: configuring directory to start on boot Done configuring directory server (dirsrv). Configuring certificate server (pki-tomcatd): Estimated time 3 minutes 30 seconds [1/21]: creating certificate server user [2/21]: configuring certificate server instance [3/21]: stopping certificate server instance to update CS.cfg [4/21]: backing up CS.cfg [5/21]: disabling nonces [6/21]: set up CRL publishing [7/21]: enable PKIX certificate path discovery and validation [8/21]: starting certificate server instance [9/21]: creating RA agent certificate database [10/21]: importing CA chain to RA certificate database [11/21]: fixing RA database permissions [12/21]: setting up signing cert profile [13/21]: set certificate subject base [14/21]: enabling Subject Key Identifier [15/21]: enabling Subject Alternative Name [16/21]: enabling CRL and OCSP extensions for
Re: [Freeipa-devel] [PATCH 0014 v3] Support multiple user and host certificates
On Wed, Jun 03, 2015 at 01:55:47PM +0200, Milan Kubik wrote: On 06/03/2015 01:17 PM, Martin Basti wrote: On 02/06/15 16:03, Jan Cholasta wrote: Dne 2.6.2015 v 12:36 Martin Basti napsal(a): On 02/06/15 11:42, Fraser Tweedale wrote: On Mon, Jun 01, 2015 at 02:50:45PM +0200, Martin Basti wrote: On 01/06/15 06:40, Fraser Tweedale wrote: New version of patch; ``{host,service}-show --out=FILE`` now writes all certs to FILE. Rebased on latest master. Thanks, Fraser On Thu, May 28, 2015 at 09:18:04PM +1000, Fraser Tweedale wrote: Updated patch attached. Notably restores/adds revocation behaviour to host-mod and service-mod. Thanks, Fraser On Wed, May 27, 2015 at 06:12:50PM +0200, Martin Basti wrote: On 27/05/15 15:53, Fraser Tweedale wrote: This patch adds supports for multiple user / host certificates. No schema change is needed ('usercertificate' attribute is already multi-value). The revoke-previous-cert behaviour of host-mod and user-mod has been removed but revocation behaviour of -del and -disable is preserved. The latest profiles/caacl patchset (0001..0013 v5) depends on this patch for correct cert-request behaviour. There is one design question (or maybe more, let me know): the `--out=FILENAME' option to {host,service} show saves ONE certificate to the named file. I propose to either: a) write all certs, suffixing suggested filename with either a sequential numerical index, e.g. cert.pem becomes cert.pem.1, cert.pem.2, and so on; or b) as above, but suffix with serial number and, if there are different issues, some issuer-identifying information. Let me know your thoughts. Thanks, Fraser Is there a possible way how to store certificates into one file? I read about possibilities to have multiple certs in one .pem file, but I'm not cert guru :) I personally vote for serial number in case there are multiple certificates, if ^ is no possible. 1) +if len(certs) 0: please use only, if certs: 2) You need to re-generate API/ACI.txt in this patch 3) syntax error: +for dercert in certs_der 4) command ipa user-mod ca_user --certificate=ceritifcate removes the current certificate from the LDAP, by design. Should be the old certificate(s) revoked? You removed that part in the code. only the --addattr='usercertificate=cert' appends new value there -- Martin Basti My objections/proposed solutions in attached patch. * VERSION * In the previous version normalized values was stored in LDAP, so I added it back. (I dont know why there is no normalization in param settings, but normalization for every certificate is done in callback. I will file a ticket for this) * IMO only normalized certificates should be compared in the old certificates detection I incorporated your suggested changes in new patch (attached). There were no proposed changes to the other patchset (0001..0013) since rebase. Thanks, Fraser Thank you, ACK Martin^2 Pushed to master: 7f7c247bb5a4b0030d531f4f14c156162e808212 Regression found. Patch to fix the issue is attached. The fix works, thanks. Milan Thanks for finding, fixing and testing! ACK from me. I also present a fix of my own. It fixes a problem where service-mod deleted all certificates when '--addattr usercertificate=XXX' was used instead of '--usercertificate=XXX' options. Cheers, Fraser From 5816c655b75a516068301186b20ddc36b966073c Mon Sep 17 00:00:00 2001 From: Fraser Tweedale ftwee...@redhat.com Date: Wed, 3 Jun 2015 02:49:28 -0400 Subject: [PATCH] Fix certificate management with service-mod Adding or removing certificates from a service via --addattr or --delattr is broken. Get certificates from entry_attrs instead of options. --- ipalib/plugins/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py index c290344cf6c14155ec1b103525ff8642a7a8e2af..369321d76a7b8e4e0a0d572fa1d26145cca010f4 100644 --- a/ipalib/plugins/service.py +++ b/ipalib/plugins/service.py @@ -598,7 +598,7 @@ class service_mod(LDAPUpdate): (service, hostname, realm) = split_principal(keys[-1]) # verify certificates -certs = options.get('usercertificate') or [] +certs = entry_attrs.get('usercertificate') or [] certs_der = map(x509.normalize_certificate, certs) for dercert in certs_der: x509.verify_cert_subject(ldap, hostname, dercert) -- 2.1.0 -- 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] 857 topology: ipa management commands
On 06/03/2015 01:34 PM, Petr Vobornik wrote: On 06/03/2015 10:59 AM, Martin Babinsky wrote: On 06/03/2015 10:52 AM, Martin Babinsky wrote: On 05/26/2015 03:31 PM, Petr Vobornik wrote: On 05/26/2015 12:19 PM, Petr Vobornik wrote: this patch is based on top of my patch #856 and tbabej' s 325-9. Obsoletes Ludwig's 0006. ipalib part of topology management Design: - http://www.freeipa.org/page/V4/Manage_replication_topology https://fedorahosted.org/freeipa/ticket/4302 New version attached: - domainlevel_show usage changed to domainlevel_get - updated VERSION - added more attrs to default_attributes Hi Petr, the commands themselves seem to work just fine. I had encountered some quirks in the underlying topology plugin, but I will address them in a different thread in order to keep the discussion relevant to the reviewed patch. I have some minor coomments below: 1.) IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=121 -# Last change: pvoborni - added server-find and server-show +IPA_API_VERSION_MINOR=122 +# Last change: pvoborni - added topology management commands Several people were touching API in the meantime so please double-check that you have correct VERSION and regenerate API.txt Patch rebased. 2.) +Str( +'nsds5replicatedattributelist?', +cli_name='replattrs', +label='Attributes to replicate', +doc=_('Attributes that are not replicated to a consumer server ' + 'during a fractional update. E.g., `(objectclass=*) ' + '$ EXCLUDE accountlockout memberof'), +), +Str( +'nsds5replicatedattributelisttotal?', +cli_name='replattrstotal', +label=_('Attributes for total update'), +doc=_('Attributes that are not replicated to a consumer server ' + 'during a total update. E.g. (objectclass=*) $ EXCLUDE ' + 'accountlockout'), The descriptions of these two options confused me greatly, are these attributes supposed to be replicated or not, or is there some more complex logic behind them that I failed to grasp? I am cc'ing Ludwig, he can probably explain them to us and then we can decide whether we may alter the descriptions to be less confusing. 3.) +takes_params = ( +Str( +'cn', +cli_name='name', +primary_key=True, +label=_('Suffix name'), +), +Str( +'iparepltopoconfroot', +maxlength=255, +cli_name='suffix', +label=_('Suffix to be managed'), +normalizer=lambda value: value.lower(), +), +) This also confused me at first, I suggest to change the label of 'iparepltopoconfroot' to something like 'LDAP suffix to be managed' or 'LDAP subtree to be managed'. Changed to 'LDAP suffix to be managed' 4.) There is currently no way to rename existing topology segments/suffixes. In the case of hosts with funky FQDN's (pointing at you, ABC lab), the segment cn's created during replica installs are mearly impossible to remember and it would be nice to rename them to something more manageable. However, this is not related to core functionality and can be a subject of a separate patch once this gets pushed. That's all from my side. I also forgot to ask what is the expected policy when deleting a non-empty topology suffix. If this is not supported and you have to first remove all segments and then the suffix itself, the 'topologysuffix-del' command should issue an error pointing the user to correct procedure. Do we have a use case for creation or deletion of topology suffix? That's a good question. Anyway, I have noticed couple more things: 1.) it seems that there some of unused imports in topology.py. Please investigate whether all of them are really needed. 2.) +from ipalib.plugins.baseldap import * +from ipalib.plugins import baseldap I do not like that starred import at all. Either import the particular classes you use (like e.g. in basuser.py), or just leave the second import statetement and use the appropriate namespace (baseldap.LDAPObject etc.). 3.) there are couple of pep8 complaints, please try to fix them unless it impairs readability: ./ipalib/constants.py:121:80: E501 line too long (81 79 characters) ./ipalib/plugins/topology.py:72:80: E501 line too long (88 79 characters) ./ipalib/plugins/topology.py:73:26: E131 continuation line unaligned for hanging indent ./ipalib/plugins/topology.py:73:80: E501 line too long (93 79 characters) ./ipalib/plugins/topology.py:103:80: E501 line too long (80 79 characters) ./ipalib/plugins/topology.py:111:80: E501 line too long (80 79 characters) ./ipalib/plugins/topology.py:207:80: E501 line too long (80 79 characters) ./ipalib/plugins/topology.py:232:80: E501 line too long (80 79 characters) ./ipalib/plugins/topology.py:269:80: E501 line too long (84 79 characters)
Re: [Freeipa-devel] KeyError raised upon replica installation
Hi Ludwig, I'll rebuild the packages again with the whole set of patches including 0010 and 0011 and try again. Thanks! On 06/03/2015 02:21 PM, Ludwig Krispenz wrote: On 06/03/2015 02:05 PM, Oleg Fayans wrote: Update: The original error occurs ONLY when installing a replica from a gpg file prepared on a master running FreeIPA 4.1.2. but this should be covere with patch 0010 If The master runs the upstream code, it works. On 06/02/2015 02:11 PM, Martin Babinsky wrote: On 06/02/2015 02:07 PM, Martin Babinsky wrote: On 06/02/2015 12:09 PM, Oleg Fayans wrote: Hi all, The following error was caught during replica installation (I used all the latest patches from Ludwig and Martin Basti): root@localhost:/home/ofayans/rpms]$ ipa-replica-install --setup-ca --setup-dns --forwarder 10.38.5.26 /var/lib/ipa/replica-info-replica1.zaeba.li.gpg Directory Manager (existing master) password: Existing BIND configuration detected, overwrite? [no]: yes Adding [192.168.122.210 replica1.zaeba.li] to your /etc/hosts file Checking forwarders, please wait ... Using reverse zone(s) 122.168.192.in-addr.arpa. Run connection check to master Check connection from replica to remote master 'upgrademaster.zaeba.li': Directory Service: Unsecure port (389): OK Directory Service: Secure port (636): OK Kerberos KDC: TCP (88): OK Kerberos Kpasswd: TCP (464): OK HTTP Server: Unsecure port (80): OK HTTP Server: Secure port (443): OK The following list of ports use UDP protocol and would need to be checked manually: Kerberos KDC: UDP (88): SKIPPED Kerberos Kpasswd: UDP (464): SKIPPED Connection from replica to master is OK. Start listening on required ports for remote master check Get credentials to log in to remote master ad...@zaeba.li password: Check SSH connection to remote master Execute check on remote master Check connection from master to remote replica 'replica1.zaeba.li': Directory Service: Unsecure port (389): OK Directory Service: Secure port (636): OK Kerberos KDC: TCP (88): OK Kerberos KDC: UDP (88): OK Kerberos Kpasswd: TCP (464): OK Kerberos Kpasswd: UDP (464): OK HTTP Server: Unsecure port (80): OK HTTP Server: Secure port (443): OK Connection from master to replica is OK. Connection check OK Configuring NTP daemon (ntpd) [1/4]: stopping ntpd [2/4]: writing configuration [3/4]: configuring ntpd to start on boot [4/4]: starting ntpd Done configuring NTP daemon (ntpd). Configuring directory server (dirsrv): Estimated time 1 minute [1/37]: creating directory server user [2/37]: creating directory server instance [3/37]: adding default schema [4/37]: enabling memberof plugin [5/37]: enabling winsync plugin [6/37]: configuring replication version plugin [7/37]: enabling IPA enrollment plugin [8/37]: enabling ldapi [9/37]: configuring uniqueness plugin [10/37]: configuring uuid plugin [11/37]: configuring modrdn plugin [12/37]: configuring DNS plugin [13/37]: enabling entryUSN plugin [14/37]: configuring lockout plugin [15/37]: configuring topology plugin [16/37]: creating indices [17/37]: enabling referential integrity plugin [18/37]: configuring ssl for ds instance [19/37]: configuring certmap.conf [20/37]: configure autobind for root [21/37]: configure new location for managed entries [22/37]: configure dirsrv ccache [23/37]: enable SASL mapping fallback [24/37]: restarting directory server [25/37]: setting up initial replication Starting replication, please wait until this has completed. Update in progress, 7 seconds elapsed Update succeeded [26/37]: updating schema [27/37]: setting Auto Member configuration [28/37]: enabling S4U2Proxy delegation [29/37]: importing CA certificates from LDAP [30/37]: initializing group membership [31/37]: adding master entry ipa : CRITICAL Failed to load master-entry.ldif: Command ''/usr/bin/ldapmodify' '-v' '-f' '/tmp/tmpFlM3mD' '-H' 'ldap://replica1.zaeba.li:389' '-x' '-D' 'cn=Directory Manager' '-y' '/tmp/tmpk_R0Lm'' returned non-zero exit status 68 [32/37]: initializing domain level [33/37]: configuring Posix uid/gid generation [34/37]: adding replication acis [35/37]: enabling compatibility plugin [36/37]: tuning directory server [37/37]: configuring directory to start on boot Done configuring directory server (dirsrv). Configuring certificate server (pki-tomcatd): Estimated time 3 minutes 30 seconds [1/21]: creating certificate server user [2/21]: configuring certificate server instance [3/21]: stopping certificate server instance to update CS.cfg [4/21]: backing up CS.cfg [5/21]: disabling nonces [6/21]: set up CRL publishing [7/21]: enable PKIX certificate path discovery and validation [8/21]: starting certificate server instance [9/21]: creating RA agent certificate database [10/21]: importing CA chain to RA certificate database
Re: [Freeipa-devel] [PATCH 424] install: Introduce installer framework ipapython.install
On 02/06/15 15:21, Jan Cholasta wrote: Dne 11.5.2015 v 13:41 Jan Cholasta napsal(a): Dne 6.5.2015 v 08:22 Jan Cholasta napsal(a): Dne 6.5.2015 v 08:11 Martin Kosek napsal(a): On 04/29/2015 06:25 PM, Jan Cholasta wrote: Dne 20.4.2015 v 16:56 Jan Cholasta napsal(a): Dne 20.4.2015 v 15:14 Martin Basti napsal(a): On 17/04/15 16:15, Jan Cholasta wrote: Dne 16.4.2015 v 16:46 Jan Cholasta napsal(a): Hi, the attached patch adds the basics of the new installer framework. As a next step, I plan to convert the install scripts to use the framework with their old code (the old code will be gradually ported to the framework later). (Note I didn't manage to write docstrings today, expect update tomorrow.) Added some docstrings. Also updated the patch to reflect little brainstorming David and I had this morning. Honza Hello, see comments bellow: 1) We started using new shorter License header in files: # # Copyright (C) 2015 FreeIPA Contributors see COPYING for license # OK. 2) IMO this will not work, NoneType has no 'obj' attribute +else: +if isinstance(value, from_): +value = None +stack.append(value.obj) +continue Right. 3) Multiple inheritance. I do not like it much. +class CompositeInstaller(Installer, CompositeConfigurator): I guess you are antagonistic to multiple inheritance because of how other languages (like C++) do it. In Python it can be pretty elegant and is basis for e.g. the mixin design pattern. Installer and CompositeConfigurator inherites from Configurator class, and all of them implements _generator method. Both of them call super()._generator(), so it's no problem (same for other methods). If I understand correctly (https://www.python.org/download/releases/2.3/mro/) the Installer._generator method will be used in this case. However in case when CompositeConfigurator has more levels (respectively it is more specialized) of inheritance, it could take precedence and its _generator method may be used instead. The order of precedence is defined by the order of base classes in the class definition. I'm afraid this may suddenly stop working. Maybe I'm wrong, please fix me. As long as you call the super class, it will work fine. And Multiple inheritance is not easily readable, this is even a diamond inheritance model. Cooperative inheritance is used by design and IMHO is easily readable if you know how to read it. Every class defines a single bit of behavior. Without cooperative inheritance, it would have to be hardcoded and/or hacked around, which I wanted to avoid. This blog post explains it nicely: https://rhettinger.wordpress.com/2011/05/26/super-considered-super/. Updated patch attached. Also attached is patch 425 which migrates ipa-server-install to the install framework. Good job there. I am just curious, will this framework and new option processing be friendly to other types of option passing than just via options? I mean tickets https://fedorahosted.org/freeipa/ticket/4517 https://fedorahosted.org/freeipa/ticket/4468 Especially 4517 is important, we need to be able to run # cat install.conf ds_password=Secret123 admin_password=Secret456 ip_address=123456 setup_dns=False # ipa-server-install --unattended --conf install.conf I assume yes, but I am just making sure. Yes, definitely. Updated patches attached. Another update, patches attached. thank you, 1) ipa-server-install --uninstall prints 0 ... Unconfiguring ipa_memcached Unconfiguring ipa-otpd 0 The ipa-server-install command was successful 2) ipa-server-install --setup-dns 'ServerOptions' object has no attribute 'dnssec_master' 3) For record, this will be fixed in extra patch. info messages from ldapupdate are printed to console 4) +if default is not _missing: +class_dict['default'] = default Why is new _missing object needed? Isn't NoneType enough? -- 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] Password vault
On Wed, 2015-06-03 at 09:27 +0200, Martin Kosek wrote: On 06/02/2015 08:34 PM, Simo Sorce wrote: On Tue, 2015-06-02 at 12:04 +0200, Jan Cholasta wrote: Dne 2.6.2015 v 02:02 Endi Sukma Dewata napsal(a): On 5/28/2015 12:46 AM, Jan Cholasta wrote: On a related note, since KRA is optional, can we move the vaults container to cn=kra,cn=vaults? This is the convetion used by the other optional components (DNS and recently CA). I mean cn=vaults,cn=kra of course. If you are talking about the o=kra,PKI suffix, I'm not sure whether the IPA framework will work with it. If you are talking about adding a new cn=kra,IPA suffix entry on top of cn=vaults, what is the purpose of this entry? Is the entry going to be created/deleted automatically when the KRA is installed/removed? Is it going to be used for something else other than vaults? I'm talking about cn=kra,IPA suffix. It should be created only when KRA is installed, although I think this can be done later after the release, moving vaults to cn=kra should be good enough for now. It's going to be used for everything KRA-specific. There are a lot of questions that need to be answered before we can make this change. This is about sticking to a convention, which everyone should do, and everyone except KRA already does. I'm sorry I didn't realize this earlier, but the change must be done now. We probably should revisit this issue after the core vault functionality is added. We can't revisit it later because after release we are stuck with whatever is there forever. See attachment for a patch which implements the change. Shouldn't we s/kra/vault/ ? After all the feature is called Vault, not KRA. I thought we are naming it by the name of the optional subsystem, not the feature itself. If for example, another feature from KRA is used, it would still live in cn=kra, no? For services so far we have CA, not dogtag, and LDAP, not 389ds, also KDC not krb5kdc and kpasswd not kadmind, etc... we normally named everything after the function. Now kra is probably a somewhat generic term, but I have not been able to find what it means exactly in 5 minutes, and it is quite obscure as a name. OTOH cn=Vault would make it really clear what's in it. I do not have a very strong opinion but a generic and clear name is important for the DIT. 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] Password vault
Dne 3.6.2015 v 15:20 Simo Sorce napsal(a): On Wed, 2015-06-03 at 09:27 +0200, Martin Kosek wrote: On 06/02/2015 08:34 PM, Simo Sorce wrote: On Tue, 2015-06-02 at 12:04 +0200, Jan Cholasta wrote: Dne 2.6.2015 v 02:02 Endi Sukma Dewata napsal(a): On 5/28/2015 12:46 AM, Jan Cholasta wrote: On a related note, since KRA is optional, can we move the vaults container to cn=kra,cn=vaults? This is the convetion used by the other optional components (DNS and recently CA). I mean cn=vaults,cn=kra of course. If you are talking about the o=kra,PKI suffix, I'm not sure whether the IPA framework will work with it. If you are talking about adding a new cn=kra,IPA suffix entry on top of cn=vaults, what is the purpose of this entry? Is the entry going to be created/deleted automatically when the KRA is installed/removed? Is it going to be used for something else other than vaults? I'm talking about cn=kra,IPA suffix. It should be created only when KRA is installed, although I think this can be done later after the release, moving vaults to cn=kra should be good enough for now. It's going to be used for everything KRA-specific. There are a lot of questions that need to be answered before we can make this change. This is about sticking to a convention, which everyone should do, and everyone except KRA already does. I'm sorry I didn't realize this earlier, but the change must be done now. We probably should revisit this issue after the core vault functionality is added. We can't revisit it later because after release we are stuck with whatever is there forever. See attachment for a patch which implements the change. Shouldn't we s/kra/vault/ ? After all the feature is called Vault, not KRA. I thought we are naming it by the name of the optional subsystem, not the feature itself. If for example, another feature from KRA is used, it would still live in cn=kra, no? For services so far we have CA, not dogtag, and LDAP, not 389ds, also KDC not krb5kdc and kpasswd not kadmind, etc... we normally named everything after the function. Now kra is probably a somewhat generic term, but I have not been able to find what it means exactly in 5 minutes, and it is quite obscure as a name. OTOH cn=Vault would make it really clear what's in it. I do not have a very strong opinion but a generic and clear name is important for the DIT. There is also ipa-kra-install and I guess cn=KRA,cn=fqdn,cn=masters,cn=ipa,cn=etc. If we rename it, it should be renamed everywhere, and I'm not sure if that's worth it. Also vault is too generic, it should be password vault, but that's too long, so IMO KRA is better, as it's short and descriptive. Are vaults the only feature KRA provides? If there are more possible features provided by KRA, it's another reason to keep it KRA. -- 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] Topology plugin quirks
Hi Petr, good catch. I didn't check for self referential segments. There is a check for existing segments, but unfortuantely the entry lookup in the pblock was incorrect and the test always passed. For the removal, there is teh assumption that no duplicate segments exist and so removal of A-B only succeeds if there is another path from A to B. I'm building a patch and will sen to the list soon Ludwig On 06/03/2015 12:51 PM, Petr Vobornik wrote: On 06/03/2015 11:37 AM, Martin Babinsky wrote: Hi everyone, I have been playing with the topology related patches and I have encountered a few issues that I would like to address in this thread: Additional stuff: 1. was able to add duplicate segment - same left and right node - same direction - different cn It did not allow me to remove it: Server is unwilling to perform: Removal of Segment disconnects topology.Deletion not allowed. 2. topology plugin allows to create reflexive relation from the invalid duplicates(#1): A - B A - B to A - A B - B I.E. effective disconnect it is forbidden in `ipa topologysegment-mod` but I think that even the plugin should not allow that 3. attempt to delete the invalid reflexive or duplicate segment ends with: Server is unwilling to perform: Removal of Segment disconnects topology.Deletion not allowed. -- 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 0001] Migrate now accepts scope as argument
On 02/06/15 22:32, Drew Erny wrote: Sorry, the email address on that patch is wrong. It picked the old one off my personal box when I migrated my dotfiles. I don't know if that's important, but if the merger could s/dpe...@crimson.ua.edu/de...@redhat.com/g, that would be better. Sorry about that, I'll fix it in my next patch. On 06/02/2015 04:23 PM, Drew Erny wrote: Hi, all, This is my first patch, which fixes Ticket #2547 at https://fedorahosted.org/freeipa/ticket/2547 It introduces a --scope option to ipa migrate-ds which allows the user to specify the search depth of a migration. The previous default behavior is the same as --scope=onelevel. To search nested OUs, the user uses --scope=subtree. --scope=base will cause the migrate script not to find anything, but has been included for completeness. Any other option is invalid and will cause the command to abort. Please review this one carefully, because I'm only like 98% confident it doesn't break anything. The only thing I'm not sure about is that if you run ipa migrate-ds without --scope specified, it gives an interactive input for that option; I'm not sure if it's supposed to do that. Thanks, Drew Erny de...@redhat.com Hello, thank you for your patch. 1) Please don't use backslash +doc=_('LDAP search scope for users and groups: base, onelevel, or '\ + 'subtree. Defaults to onelevel'), 2) You can use dictionary: _default_scope = 'onelevel' # I do not like hardcoded index there _supported_scopes = {'base': ldap.SCOPE_BASE, _default_scope: ldap.SCOPE_ONELEVEL, ...} StrEnum( values=_supported_scopes.keys(), default=_default_scope ) scope = _supported_scopes[options.get('scope', _default_scope)] # or autofill=True should be in StrEnum param for scope instead, I'm not sure, you must test it :-) 3) do not forget to change the email PS: I did not test the code, it is just example. Martin^2 -- 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] [PATCHES 0233-0234] DNSSEC: forwarders validation
On 03/06/15 14:57, Petr Spacek wrote: On 18.5.2015 13:48, Martin Basti wrote: On 15/05/15 18:11, Petr Spacek wrote: On 7.5.2015 18:12, Martin Basti wrote: On 07/05/15 12:19, Petr Spacek wrote: On 7.5.2015 08:59, David Kupka wrote: On 05/06/2015 03:20 PM, Martin Basti wrote: On 05/05/15 15:00, Martin Basti wrote: On 30/04/15 15:37, David Kupka wrote: On 04/24/2015 02:56 PM, Martin Basti wrote: Patches attached. Hi, thanks for patches. 1. You changed message in DNSServerNotRespondingWarning class but not the test in ipatest/test_xmlrpc/test_dns_plugin.py nitpick. Please spell 'edns' correctly. I've seen several instances of 'ends'. Thank you, updated patches attached: * new error messages * logging to debug log server output if exception was raised * fixed test * fixed spelling Fixed tests (again) Updated patches attached The code looks good to me and tests are no longer broken. (I would prefer better fix of the tests but given that the priorities are different now it can wait.) Petr, can you please confirm that the patch set works for you? Sorry, NACK: $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236 Server will check DNS forwarder(s). This may take some time, please wait ... ipa: ERROR: an internal error has occurred # /var/log/httpd/error_log ipa: ERROR: non-public: AssertionError: Traceback (most recent call last): File /usr/lib/python2.7/site-packages/ipaserver/rpcserver.py, line 350, in wsgi_execute result = self.Command[name](*args, **options) File /usr/lib/python2.7/site-packages/ipalib/frontend.py, line 443, in __call__ ret = self.run(*args, **options) File /usr/lib/python2.7/site-packages/ipalib/frontend.py, line 760, in run return self.execute(*args, **options) File /usr/lib/python2.7/site-packages/ipalib/plugins/dns.py, line , in execute **options) File /usr/lib/python2.7/site-packages/ipalib/plugins/dns.py, line 4405, in _warning_if_forwarders_do_not_work log=self.log) File /usr/lib/python2.7/site-packages/ipalib/util.py, line 715, in validate_dnssec_zone_forwarder_step2 timeout=timeout) File /usr/lib/python2.7/site-packages/ipalib/util.py, line 610, in _resolve_record assert isinstance(nameserver_ip, basestring) AssertionError ipa: INFO: [jsonserver_session] admin@IPA.EXAMPLE: dnsforwardzone_add(DNS name ptr.test., idnsforwarders=(u'10.34.47.236',), all=False, raw=False, version=u'2.116'): AssertionError This is constantly reproducible in my vm-090.abc. Let me know if you want to take a look. I'm attaching little response.patch which improves compatibility with older python-dns packages. This patch allows IPA to work while error messages are simply not as nice as they could be with latest python-dns :-) check_fwd_msg.patch is a little nitpick, just to make sure everyone understands the message. BTW why some messages in check_forwarders() are printed using 'print' and others using logger? I would prefer to use logger for everything to make sure that logs contain all the information, including warnings. Thank you for your time! Thank you, fixed. I added missing except block after forwarders validation step2. I confirm that this works but I just discovered another deficiency. Setup: - DNSSEC validation is enabled on IPA server - forwarders uses fake TLD, e.g. 'test.' - remote DNS server is responding, supports EDNS0 and so on $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236 Server will check DNS forwarder(s). This may take some time, please wait ... ipa: WARNING: DNS server 10.34.78.90: query 'ptr.test. SOA': The DNS query name does not exist: ptr.test.. Huh? Let's check named log: forward zone 'ptr.test': loaded validating ./SOA: got insecure response; parent indicates it should be secure Sometimes I get SERVFAIL from IPA server, too. Unfortunately this check was the main reason for writing this patchset so we need to improve it. Maybe validate_dnssec_zone_forwarder_step2() could special-case NXDOMAIN and print the DNSSEC-validation-failed error, too? The problem is that it could trigger some false positives because NXDOMAIN may simply be caused by a delay somewhere. Any ideas? I add catch block for NXDOMAIN By the way, this is also weird: $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236 Server will check DNS forwarder(s). This may take some time, please wait ... ipa: ERROR: DNS forward zone with name ptr.test. already exists Is it actually doing the check even if the forward zone exists already? (This is just nitpick, not a blocker!) The first part is written by IPA client, it is not response from server. It is just written when user use --forwarder option. Updated patch attached. NACK, it does not work for me - it explodes when I try to add a forward zone: $ ipa dnsforwardzone-add ptr.test. --forwarder=192.0.2.1 ipa: ERROR: non-public: TypeError: _warning_if_forwarders_do_not_work() got multiple values for
Re: [Freeipa-devel] Topology plugin quirks
On Wed, 2015-06-03 at 11:37 +0200, Martin Babinsky wrote: 3.) It seems that the removal of topology suffixes containing functioning segments is not handled well. I once tried to do this and it led to segmentation fault on the dirsrv instance. What is the expected behavior in this scenario? Dirsrv crashes are always critical bugs, please file tickets if you see any. 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 0010] KeyError raised upon replica installation
On Wed, 2015-06-03 at 16:10 +0200, Petr Vobornik wrote: On 06/02/2015 02:20 PM, Ludwig Krispenz wrote: replicas installed from older versions do not have a binddn group just accept the errror ACK Pushed to master: 8457edc14dade724b486540800bcdafb7d9a6f76 Note that this group will be populated later. IMHO it should be done as a part of domain-level raise procedure before setting the new level. Creating this group and populating it should be part of ipa-ldap-update (sorry forgot the right name) and should be done when we install new rpms. Each server must care by itself to populate this group with its own membership. In particular this *should* not be done when the domain level is raised, it is already late then. 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] 857 topology: ipa management commands
On 06/03/2015 03:53 PM, Petr Vobornik wrote: On 06/03/2015 02:38 PM, Martin Babinsky wrote: On 06/03/2015 01:34 PM, Petr Vobornik wrote: On 06/03/2015 10:59 AM, Martin Babinsky wrote: On 06/03/2015 10:52 AM, Martin Babinsky wrote: On 05/26/2015 03:31 PM, Petr Vobornik wrote: On 05/26/2015 12:19 PM, Petr Vobornik wrote: this patch is based on top of my patch #856 and tbabej' s 325-9. Obsoletes Ludwig's 0006. ipalib part of topology management Design: - http://www.freeipa.org/page/V4/Manage_replication_topology https://fedorahosted.org/freeipa/ticket/4302 New version attached: - domainlevel_show usage changed to domainlevel_get - updated VERSION - added more attrs to default_attributes Hi Petr, the commands themselves seem to work just fine. I had encountered some quirks in the underlying topology plugin, but I will address them in a different thread in order to keep the discussion relevant to the reviewed patch. I have some minor coomments below: 1.) IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=121 -# Last change: pvoborni - added server-find and server-show +IPA_API_VERSION_MINOR=122 +# Last change: pvoborni - added topology management commands Several people were touching API in the meantime so please double-check that you have correct VERSION and regenerate API.txt Patch rebased. 2.) +Str( +'nsds5replicatedattributelist?', +cli_name='replattrs', +label='Attributes to replicate', +doc=_('Attributes that are not replicated to a consumer server ' + 'during a fractional update. E.g., `(objectclass=*) ' + '$ EXCLUDE accountlockout memberof'), +), +Str( +'nsds5replicatedattributelisttotal?', +cli_name='replattrstotal', +label=_('Attributes for total update'), +doc=_('Attributes that are not replicated to a consumer server ' + 'during a total update. E.g. (objectclass=*) $ EXCLUDE ' + 'accountlockout'), The descriptions of these two options confused me greatly, are these attributes supposed to be replicated or not, or is there some more complex logic behind them that I failed to grasp? I am cc'ing Ludwig, he can probably explain them to us and then we can decide whether we may alter the descriptions to be less confusing. 3.) +takes_params = ( +Str( +'cn', +cli_name='name', +primary_key=True, +label=_('Suffix name'), +), +Str( +'iparepltopoconfroot', +maxlength=255, +cli_name='suffix', +label=_('Suffix to be managed'), +normalizer=lambda value: value.lower(), +), +) This also confused me at first, I suggest to change the label of 'iparepltopoconfroot' to something like 'LDAP suffix to be managed' or 'LDAP subtree to be managed'. Changed to 'LDAP suffix to be managed' 4.) There is currently no way to rename existing topology segments/suffixes. In the case of hosts with funky FQDN's (pointing at you, ABC lab), the segment cn's created during replica installs are mearly impossible to remember and it would be nice to rename them to something more manageable. However, this is not related to core functionality and can be a subject of a separate patch once this gets pushed. That's all from my side. I also forgot to ask what is the expected policy when deleting a non-empty topology suffix. If this is not supported and you have to first remove all segments and then the suffix itself, the 'topologysuffix-del' command should issue an error pointing the user to correct procedure. Do we have a use case for creation or deletion of topology suffix? That's a good question. Anyway, I have noticed couple more things: 1.) it seems that there some of unused imports in topology.py. Please investigate whether all of them are really needed. Fixed 2.) +from ipalib.plugins.baseldap import * +from ipalib.plugins import baseldap I do not like that starred import at all. Either import the particular classes you use (like e.g. in basuser.py), or just leave the second import statetement and use the appropriate namespace (baseldap.LDAPObject etc.). Fixed 3.) there are couple of pep8 complaints, please try to fix them unless it impairs readability: ./ipalib/constants.py:121:80: E501 line too long (81 79 characters) ./ipalib/plugins/topology.py:72:80: E501 line too long (88 79 characters) ./ipalib/plugins/topology.py:73:26: E131 continuation line unaligned for hanging indent ./ipalib/plugins/topology.py:73:80: E501 line too long (93 79 characters) ./ipalib/plugins/topology.py:103:80: E501 line too long (80 79 characters) ./ipalib/plugins/topology.py:111:80: E501 line too long (80 79 characters) ./ipalib/plugins/topology.py:207:80: E501 line too long (80 79 characters) ./ipalib/plugins/topology.py:232:80: E501 line too long (80 79 characters)
Re: [Freeipa-devel] [PATCH 0014 v3] Support multiple user and host certificates
On 03/06/15 15:21, Fraser Tweedale wrote: On Wed, Jun 03, 2015 at 01:55:47PM +0200, Milan Kubik wrote: On 06/03/2015 01:17 PM, Martin Basti wrote: On 02/06/15 16:03, Jan Cholasta wrote: Dne 2.6.2015 v 12:36 Martin Basti napsal(a): On 02/06/15 11:42, Fraser Tweedale wrote: On Mon, Jun 01, 2015 at 02:50:45PM +0200, Martin Basti wrote: On 01/06/15 06:40, Fraser Tweedale wrote: New version of patch; ``{host,service}-show --out=FILE`` now writes all certs to FILE. Rebased on latest master. Thanks, Fraser On Thu, May 28, 2015 at 09:18:04PM +1000, Fraser Tweedale wrote: Updated patch attached. Notably restores/adds revocation behaviour to host-mod and service-mod. Thanks, Fraser On Wed, May 27, 2015 at 06:12:50PM +0200, Martin Basti wrote: On 27/05/15 15:53, Fraser Tweedale wrote: This patch adds supports for multiple user / host certificates. No schema change is needed ('usercertificate' attribute is already multi-value). The revoke-previous-cert behaviour of host-mod and user-mod has been removed but revocation behaviour of -del and -disable is preserved. The latest profiles/caacl patchset (0001..0013 v5) depends on this patch for correct cert-request behaviour. There is one design question (or maybe more, let me know): the `--out=FILENAME' option to {host,service} show saves ONE certificate to the named file. I propose to either: a) write all certs, suffixing suggested filename with either a sequential numerical index, e.g. cert.pem becomes cert.pem.1, cert.pem.2, and so on; or b) as above, but suffix with serial number and, if there are different issues, some issuer-identifying information. Let me know your thoughts. Thanks, Fraser Is there a possible way how to store certificates into one file? I read about possibilities to have multiple certs in one .pem file, but I'm not cert guru :) I personally vote for serial number in case there are multiple certificates, if ^ is no possible. 1) +if len(certs) 0: please use only, if certs: 2) You need to re-generate API/ACI.txt in this patch 3) syntax error: +for dercert in certs_der 4) command ipa user-mod ca_user --certificate=ceritifcate removes the current certificate from the LDAP, by design. Should be the old certificate(s) revoked? You removed that part in the code. only the --addattr='usercertificate=cert' appends new value there -- Martin Basti My objections/proposed solutions in attached patch. * VERSION * In the previous version normalized values was stored in LDAP, so I added it back. (I dont know why there is no normalization in param settings, but normalization for every certificate is done in callback. I will file a ticket for this) * IMO only normalized certificates should be compared in the old certificates detection I incorporated your suggested changes in new patch (attached). There were no proposed changes to the other patchset (0001..0013) since rebase. Thanks, Fraser Thank you, ACK Martin^2 Pushed to master: 7f7c247bb5a4b0030d531f4f14c156162e808212 Regression found. Patch to fix the issue is attached. The fix works, thanks. Milan Thanks for finding, fixing and testing! ACK from me. I also present a fix of my own. It fixes a problem where service-mod deleted all certificates when '--addattr usercertificate=XXX' was used instead of '--usercertificate=XXX' options. Cheers, Fraser 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] Topology plugin quirks
On Wed, 2015-06-03 at 12:51 +0200, Petr Vobornik wrote: On 06/03/2015 11:37 AM, Martin Babinsky wrote: Hi everyone, I have been playing with the topology related patches and I have encountered a few issues that I would like to address in this thread: Additional stuff: 1. was able to add duplicate segment - same left and right node - same direction - different cn It did not allow me to remove it: Server is unwilling to perform: Removal of Segment disconnects topology.Deletion not allowed. Odd, I would think that if you have 2 segments then either one would satisfy the topology requirement. Ludwig, why is the plugin allowing 2 segments and then does not recognize there is another one at removal time ? 2. topology plugin allows to create reflexive relation from the invalid duplicates(#1): A - B A - B to A - A B - B I.E. effective disconnect it is forbidden in `ipa topologysegment-mod` but I think that even the plugin should not allow that Yes, the plugin must forbid this case on its own. 3. attempt to delete the invalid reflexive or duplicate segment ends with: Server is unwilling to perform: Removal of Segment disconnects topology.Deletion not allowed. The plugin should not allow duplicates or reflexive segments in the first place, so this should never be required then. 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 0011] check-for-existing-and-self-referential-segments
On Wed, 2015-06-03 at 14:53 +0200, Ludwig Krispenz wrote: Hi, this should prevent adding duplicate segments or segments with same start and end node 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
Re: [Freeipa-devel] [PATCH] 822 webui: topology plugin
On 05/27/2015 04:14 PM, Petr Vobornik wrote: On 05/26/2015 12:22 PM, Petr Vobornik wrote: On 05/15/2015 01:50 PM, Petr Vobornik wrote: On 04/21/2015 04:09 PM, Petr Vobornik wrote: First iteration of Topology plugin Web UI. It reflects current state of topology plugin python part which is implemented in [PATCH] manage replication topology in the shared tree and my wip patch. I expect that the server API part will change a bit therefore this will as well. Graphical visualization/management (ticket 4286) will be implemented in separate patch. https://fedorahosted.org/freeipa/ticket/4997 http://www.freeipa.org/page/V4/Manage_replication_topology New version attached. It requires stage user web ui patches in order to apply (I expect that user life cycle backend will be pushed sooner than topology) Changes: - Left host and Right host fields are now host comboboxes - Connectivity are radio buttons with both, left-right, right-left, none options - segment name is not a required field in its adder dialog IMHO Attributes to strip, Attributes to replicate, Attributes for total update, Initialize replica, Session timeout, Replication agreement enabled fields should not be just free-form textboxes, but they should be more specific, e.g. a checkbox for Replication agreement enabled or integer for Session timeout, but that should be modified first in the backend python plugin. New patchset which replaces the old patch. Contains Web UI for: - topologysuffix, topologysegment, domain level, server Backend is implemented in patches: - tbabej 325-9 - pvoborni 855, 857 New update which reflects the API change in domain level patches. (domainlevel-show changed to domainlevel-get). Now it depends only on pvoborni 857-2, the rest was pushed. The patches seem to do what they are supposed to do. However, I have not found any UI element implementing the 'topologysegment-refresh' functionality. Was this only an oversight or do you plan to implement it in next patch? -- 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 0010] KeyError raised upon replica installation
On Wed, 2015-06-03 at 16:50 +0200, Martin Kosek wrote: On 06/03/2015 04:10 PM, Petr Vobornik wrote: On 06/02/2015 02:20 PM, Ludwig Krispenz wrote: replicas installed from older versions do not have a binddn group just accept the errror ACK Pushed to master: 8457edc14dade724b486540800bcdafb7d9a6f76 Note that this group will be populated later. IMHO it should be done as a part of domain-level raise procedure before setting the new level. As said in other mail, I am not sure why we should be overloading domain-level raise command that way. +1 I thought, we will create this group when the first replica upgrades to 4.2. Whenever a new replica is added/upgraded, it's principal will be added to the group also (even if Domain Level is 0). +1 Domain Level 1 means that all replicas are 4.2 and thus the group is fully populated and Topology can be used. +1 -- 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 0014] Support multiple user and host certificates
On 02/06/15 16:56, Petr Vobornik wrote: On 05/27/2015 03:53 PM, Fraser Tweedale wrote: This patch adds supports for multiple user / host certificates. No schema change is needed ('usercertificate' attribute is already multi-value). The revoke-previous-cert behaviour of host-mod and user-mod has been removed but revocation behaviour of -del and -disable is preserved. The latest profiles/caacl patchset (0001..0013 v5) depends on this patch for correct cert-request behaviour. There is one design question (or maybe more, let me know): the `--out=FILENAME' option to {host,service} show saves ONE certificate to the named file. I propose to either: a) write all certs, suffixing suggested filename with either a sequential numerical index, e.g. cert.pem becomes cert.pem.1, cert.pem.2, and so on; or b) as above, but suffix with serial number and, if there are different issues, some issuer-identifying information. Let me know your thoughts. Thanks, Fraser Has anybody tried it with Web UI? Currently Web UI is designed only for one cert. I wonder if it still works even with just one. We should probably file a ticket. If there are 2 certificates in a host entry, then the WebUI just shows: Status Valid Certificate Present Then 'view certificate' shows the second certificate the 'Get certificate' shows the first certificate I will file a ticket. Martin^2 -- 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] [PATCHES 0001-0013 v7] Profiles and CA ACLs
On 03/06/15 16:17, Fraser Tweedale wrote: On Tue, Jun 02, 2015 at 06:37:42PM +0200, Martin Basti wrote: On 02/06/15 14:11, Fraser Tweedale wrote: On Mon, Jun 01, 2015 at 05:22:28PM +1000, Fraser Tweedale wrote: On Mon, Jun 01, 2015 at 05:10:58PM +1000, Fraser Tweedale wrote: On Fri, May 29, 2015 at 01:03:46PM +0200, Martin Kosek wrote: On 05/29/2015 11:21 AM, Martin Basti wrote: On 29/05/15 06:17, Fraser Tweedale wrote: On Thu, May 28, 2015 at 02:42:53PM +0200, Martin Basti wrote: On 28/05/15 11:48, Martin Basti wrote: On 27/05/15 16:04, Fraser Tweedale wrote: Hello all, Fresh certificate management patchset; Changelog: - Now depends on patch freeipa-ftweedal-0014 for correct cert-request behaviour with host and service principals. - Updated Dogtag dependency to 10.2.4-1. Should should be in f22 soon, but for f22 right now or for f21, please grab from my copr: https://copr.fedoraproject.org/coprs/ftweedal/freeipa/ Martin^1 could you please add to the quasi-official freeipa copr? SRPM lives at https://frase.id.au/pki-core-10.2.4-1.fc21.src.rpm. - cert-request now verifies that for user principals, CSR CN matches uid and, DN emailAddress and SAN rfc822Name match user's email address, if either of those is present. - Fixed one or two other sneaky little bugs. On Wed, May 27, 2015 at 01:59:30AM +1000, Fraser Tweedale wrote: Hi all, Please find attached the latest certificate management patchset, which introduces the `caacl' plugin and various fixes and improvement to earlier patches. One important change to earlier patches is reverting the name of the default profile to 'caIPAserviceCert' and using the existing instance of this profile on upgrade (but not install) in case it has been modified. Other notes: - Still have changes in ipa-server-install (fewer lines now, though) - Still have the ugly import hack. It is not a high priority for me, i.e. I think it should wait until after alpha - Still need to update 'service' and 'host' plugins to support multiple certificates. (The userCertificate attribute schema itself is multi-valued, so there are no schema issues here) - The TODOs in [1]; mostly certprofile CLI conveniences and supporting multiple profiles for hosts and services (which requires changes to framework only, not schema). [1]: http://idm.etherpad.corp.redhat.com/rhel72-cert-mgmt-progress Happy reviewing! I am pleased with the initial cut of the caacl plugin but I'm sure you will find some things to be fixed :) Cheers, Fraser [root@vm-093 ~]# ipa-replica-prepare vm-094.example.com --ip-address 10.34.78.94 Directory Manager (existing master) password: Preparing replica for vm-094.example.com from vm-093.example.com Creating SSL certificate for the Directory Server not well-formed (invalid token): line 2, column 14 I cannot create replica file. It work on the upgraded server, but it doesn't work on the newly installed server. I'm not sure if this causes your patches which modifies the ca-installer, or the newer version of dogtag. Or if there was any other changes in master, I will continue to investigate with new RPM from master branch. Martin^2 ipa-replica-prepare works for: * master branch * master branch + pki-ca 10.2.4-1 So something in your patches is breaking it Martin^2 Martin, master + my patches with pki 10.2.4-1 is working for me on f21 and f22. Can you provide ipa-replica-prepare --debug output and Dogtag debug log? ( /var/log/pki/pki-tomcat/ca/debug ) Thanks, Fraser I can not reproduce it today. And I already recycled the VMs from yesterday. :-( In that case I would suggest ACKingpushing the patch and fixing the bug if it comes again. The tree may now be a bit unstable, given the number of patches going in. My main motivation here is to unblock Fraser. Thanks, Martin Rebased patchset attached; no other changes. Heads up: I just discovered I have introduced a bug with ipa-replica-install, when it is spawning the CA instance. I think replication it only causes issues with ``--setup-ca``. I will try and sort it out tomorrow or later tonight (I have to head out for a few hours now, though); and I'm not suggesting it should block the push but it's something to be aware of. Cheers, Fraser New patchset attached ; haven't gotten to the bottom of the ipa-replica-install issue mentioned above, but it fixes an upgrade bug. The change is: diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py index c288282..c5f4d37 100644 --- a/ipaserver/install/server/upgrade.py +++ b/ipaserver/install/server/upgrade.py @@ -316,7 +316,7 @@ def ca_enable_ldap_profile_subsystem(ca): caconfig.CS_CFG_PATH, directive, separator='=') -if value == 'ProfileSubsystem': +if value == 'com.netscape.cmscore.profile.ProfileSubsystem': needs_update = True break except OSError, e: @@ -328,7 +328,7 @@ def
Re: [Freeipa-devel] [PATCH] Use Exception class instead of BaseException
Petr Viktorin wrote: On 06/01/2015 06:33 AM, Niranjan wrote: Greetings, I would like to present patch for replacing StandardError exception with Exception class in ipapython/adminutil.py. Also replacing BaseException class with Exception class. Though the use of StandardError is many places. I would like to start with ipapython/adminutil.py This is my first patch. Please let me know if my approach on this is correct. Regards Niranjan 0001-Use-Exception-class-instead-of-BaseException.patch From 018312f76952ea86c8c6e2396657e0531d2d61ba Mon Sep 17 00:00:00 2001 From: Niranjan Mallapadi mrniran...@redhat.com Date: Mon, 1 Jun 2015 09:41:05 +0530 Subject: [PATCH] Use Exception class instead of BaseException 1. Replace BaseException with Exception class. I don't see a reason for this change. This is top-level CLI code that handles calling our Python library. We really do want to catch all exceptions here, including KeyboardInterrupt and SystemExit. 2. Remove StandardError and use Exception class. StandError is deprecated (Python3) I'm okay with this change, as long as tests still pass. 3 .From python3.0 use of , is not recommended, instead use as keyword (PEP 3110) +1 I will send a modified patch and also run tests before sending them, Thanks a lot for the review. -- Petr Viktorin pgpD9hPJvOkkW.pgp Description: PGP 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
[Freeipa-devel] [PATCH 0001 v2] Migrate now accepts scope as argument
Hi, all, This is an updated patch, with the code changes suggested by Martin Batsi in my test email. The biggest difference is that I had to do from ldap import SCOPE_BASE, SCOPE_ONELEVEL, SCOPE_SUBTREE To get access to those constants in the global scope. This seems like a fairly clean solution, but if it's a code smell, feel free to suggest improvements. This should have identical behavior to the last patch, except it will autofill scope and no longer prompt interactively. Thanks, Drew Erny de...@redhat.com From 168e910aef41bd1df661317168236287b2994822 Mon Sep 17 00:00:00 2001 From: Drew Erny de...@redhat.com Date: Wed, 27 May 2015 09:52:42 -0400 Subject: [PATCH] Migration now accepts scope as argument Adds a new option to command ipa migrate-ds, --scope=[base,onelevel,subtree], which allows the user to specify LDAP search depth for users and groups. 'onelevel' was the previous default level. Specify 'subtree' to to search nested OUs for users and groups. fedorahosted.org/freeipa/ticket/2547 --- API.txt | 3 ++- ipalib/plugins/migration.py | 18 +- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/API.txt b/API.txt index d987bc949948a280018f0f20d5af93838ecaeb20..da124c2d659510cf81d25a5708835cf8ed176efa 100644 --- a/API.txt +++ b/API.txt @@ -2450,7 +2450,7 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA output: Output('summary', (type 'unicode', type 'NoneType'), None) output: PrimaryKey('value', None, None) command: migrate_ds -args: 2,18,4 +args: 2,19,4 arg: Str('ldapuri', cli_name='ldap_uri') arg: Password('bindpw', cli_name='password', confirm=False) option: DNParam('basedn?', cli_name='base_dn') @@ -2466,6 +2466,7 @@ option: Str('groupignoreobjectclass*', autofill=True, cli_name='group_ignore_obj option: Str('groupobjectclass+', autofill=True, cli_name='group_objectclass', csv=True, default=(u'groupOfUniqueNames', u'groupOfNames')) option: Flag('groupoverwritegid', autofill=True, cli_name='group_overwrite_gid', default=False) option: StrEnum('schema?', autofill=True, cli_name='schema', default=u'RFC2307bis', values=(u'RFC2307bis', u'RFC2307')) +option: StrEnum('scope', autofill=True, cli_name='scope', default=u'onelevel', values=(u'base', u'subtree', u'onelevel')) option: DNParam('usercontainer', autofill=True, cli_name='user_container', default=ipapython.dn.DN('ou=people')) option: Str('userignoreattribute*', autofill=True, cli_name='user_ignore_attribute', csv=True, default=()) option: Str('userignoreobjectclass*', autofill=True, cli_name='user_ignore_objectclass', csv=True, default=()) diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py index c8379420d539ac35901d99f981b4c8e2f0f89ffc..d922d67cbf1a91a201b3b985af36a34e7956300a 100644 --- a/ipalib/plugins/migration.py +++ b/ipalib/plugins/migration.py @@ -35,6 +35,8 @@ from ipapython.ipautil import write_tmp_file import datetime from ipaplatform.paths import paths +from ldap import SCOPE_BASE, SCOPE_ONELEVEL, SCOPE_SUBTREE + __doc__ = _( Migration to IPA @@ -140,6 +142,9 @@ _dn_err_msg = _('Malformed DN') _supported_schemas = (u'RFC2307bis', u'RFC2307') +# search scopes for users and groups when migrating +_supported_scopes = {u'base': SCOPE_BASE, u'onelevel': SCOPE_ONELEVEL, u'subtree': SCOPE_SUBTREE} +_default_scope = u'onelevel' def _pre_migrate_user(ldap, pkey, dn, entry_attrs, failed, config, ctx, **kwargs): assert isinstance(dn, DN) @@ -607,6 +612,15 @@ class migrate_ds(Command): doc=_('Load CA certificate of LDAP server from FILE'), default=None ), +StrEnum('scope', +cli_name='scope', +label=_('Search scope'), +doc=_('LDAP search scope for users and groups: base, onelevel, or ' + 'subtree. Defaults to onelevel'), +values=tuple(_supported_scopes.keys()), +default=_default_scope, +autofill=True, +), ) has_output = ( @@ -711,13 +725,15 @@ can use their Kerberos accounts.''') exclude = options['exclude_%ss' % to_cli(ldap_obj_name)] context = dict(ds_ldap = ds_ldap) +scope = _supported_scopes[options.get('scope')] + migrated[ldap_obj_name] = [] failed[ldap_obj_name] = {} try: entries, truncated = ds_ldap.find_entries( search_filter, ['*'], search_bases[ldap_obj_name], -ds_ldap.SCOPE_ONELEVEL, +scope, time_limit=0, size_limit=-1, search_refs=True# migrated DS may contain search references ) -- 2.4.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] [PATCH 0014 v3] Support multiple user and host certificates
Dne 3.6.2015 v 17:44 Martin Basti napsal(a): On 03/06/15 15:21, Fraser Tweedale wrote: On Wed, Jun 03, 2015 at 01:55:47PM +0200, Milan Kubik wrote: On 06/03/2015 01:17 PM, Martin Basti wrote: On 02/06/15 16:03, Jan Cholasta wrote: Dne 2.6.2015 v 12:36 Martin Basti napsal(a): On 02/06/15 11:42, Fraser Tweedale wrote: On Mon, Jun 01, 2015 at 02:50:45PM +0200, Martin Basti wrote: On 01/06/15 06:40, Fraser Tweedale wrote: New version of patch; ``{host,service}-show --out=FILE`` now writes all certs to FILE. Rebased on latest master. Thanks, Fraser On Thu, May 28, 2015 at 09:18:04PM +1000, Fraser Tweedale wrote: Updated patch attached. Notably restores/adds revocation behaviour to host-mod and service-mod. Thanks, Fraser On Wed, May 27, 2015 at 06:12:50PM +0200, Martin Basti wrote: On 27/05/15 15:53, Fraser Tweedale wrote: This patch adds supports for multiple user / host certificates. No schema change is needed ('usercertificate' attribute is already multi-value). The revoke-previous-cert behaviour of host-mod and user-mod has been removed but revocation behaviour of -del and -disable is preserved. The latest profiles/caacl patchset (0001..0013 v5) depends on this patch for correct cert-request behaviour. There is one design question (or maybe more, let me know): the `--out=FILENAME' option to {host,service} show saves ONE certificate to the named file. I propose to either: a) write all certs, suffixing suggested filename with either a sequential numerical index, e.g. cert.pem becomes cert.pem.1, cert.pem.2, and so on; or b) as above, but suffix with serial number and, if there are different issues, some issuer-identifying information. Let me know your thoughts. Thanks, Fraser Is there a possible way how to store certificates into one file? I read about possibilities to have multiple certs in one .pem file, but I'm not cert guru :) I personally vote for serial number in case there are multiple certificates, if ^ is no possible. 1) +if len(certs) 0: please use only, if certs: 2) You need to re-generate API/ACI.txt in this patch 3) syntax error: +for dercert in certs_der 4) command ipa user-mod ca_user --certificate=ceritifcate removes the current certificate from the LDAP, by design. Should be the old certificate(s) revoked? You removed that part in the code. only the --addattr='usercertificate=cert' appends new value there -- Martin Basti My objections/proposed solutions in attached patch. * VERSION * In the previous version normalized values was stored in LDAP, so I added it back. (I dont know why there is no normalization in param settings, but normalization for every certificate is done in callback. I will file a ticket for this) * IMO only normalized certificates should be compared in the old certificates detection I incorporated your suggested changes in new patch (attached). There were no proposed changes to the other patchset (0001..0013) since rebase. Thanks, Fraser Thank you, ACK Martin^2 Pushed to master: 7f7c247bb5a4b0030d531f4f14c156162e808212 Regression found. Patch to fix the issue is attached. The fix works, thanks. Milan Thanks for finding, fixing and testing! ACK from me. I also present a fix of my own. It fixes a problem where service-mod deleted all certificates when '--addattr usercertificate=XXX' was used instead of '--usercertificate=XXX' options. Cheers, Fraser ACK Pushed both to master: 62e98671142cbc30366109a2a1b631c1ef0cae5c -- 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