Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] New Feature Template
Hello all, Most developers involved in 4.1.0 design updates already know, but we updated the Feature Template to better fit the needs of the project. See http://www.freeipa.org/page/Feature_template Major changes: - The main shift is that we want the design pages to be readable by both developers, but also the users. Especially the Overview, Use Cases, high level part of Design and the new How to Test section need to be understandable for them. This is after all the main and only upstream documentation of the feature so it needs to be ready for external consumers. - There is a new wiki template for creating the Feature Box in the top of the page, see for example http://www.freeipa.org/page/V4/OTP. Main goal is to provide a link from the feature to the release and trac ticket. I went through and updated most of 4.0.0 and 4.1.0 designs to have this box, this has immediate benefit of not only being more clear, but also with What Links Here part of releases pages being quite interesting: http://www.freeipa.org/page/Special:WhatLinksHere/Releases/4.0.0 http://www.freeipa.org/page/Special:WhatLinksHere/Releases/4.1.0 - Some sections of the previous template were not used in most design pages so they were just converted to optional subsections. - (minor) header level was changed from = Header = to == Header ==. Wiki pages are not really supposed to = level of headers as it generates h1 tag and there should be just one such tag - page title. Any other improvements or comments welcome. -- Martin Kosek mko...@redhat.com Supervisor, Software Engineering - Identity Management Team Red Hat Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0169 Update slapi-nis dependency to 0.54.1
On 11/06/2014 02:12 PM, Alexander Bokovoy wrote: Hi! I've released slapi-nis 0.54.1 to add LDAP BIND support for ID overrides in schema compat plugin and to ignore searches of the overrides themselves outside of the schema compat subtrees. FreeIPA 4.1 and later should depend on this version. I've pushed package updates to rawhide and F21, the latter will be released later today in a combined Bodhi update for FreeIPA 4.1. A patch to freeipa.spec.in is attached. Fedora build should be already handled, we just miss upstream fix. ACK. Pushed to master (with rebase) and ipa-4-1. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0285] specfile: Add BuildRequires for pki-base 10.2.1-0
On 11/06/2014 03:02 PM, Tomas Babej wrote: Hi, this solves the build errors we've been seeing recently on master branch. https://fedorahosted.org/freeipa/ticket/4688 Copr for pki-base 10.2.1-0 is available here: http://copr.fedoraproject.org/coprs/edewata/pki/ Thanks! ACK, pushed to master: b168a7f2d1793fbeb32790e1752b16dce0e5133d -- PetrĀ³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Releasing testing tools as standalone projects
On 11/06/2014 05:00 PM, Scott Poore wrote: - Original Message - From: Petr Viktorin pvikt...@redhat.com [...] I've opened a ticket to get the project space for the BeakerLib plugin: https://fedorahosted.org/fedora-infrastructure/ticket/4589 When that's done I'll point the package metadata to there, push to PyPI and open a Fedora review request. Awesome. Thanks, Petr. While we wait, it's available here: https://github.com/encukou/pytest-beakerlib https://copr.fedoraproject.org/coprs/pviktori/pytest-beakerlib Mind if I pass this on to some other QE teams that might be interested? Not at all, go ahead! You might want to wait until the project gets its permanent home, though. Do we need a version for EL 6? I'd need to check the pytest versions there, and build a newer pytest if necessary. Yes, I think we will want an EL6 version as well at least at some point. Let me know when that point comes :) The second part is the multi-host framework. I've looked at what parts are applicable to other projects than IPA, and came up with an initial design/README here: https://github.com/encukou/pytest-multihost I'll add a concrete example, code, and patches for IPA, soon. This is the paramiko/openssh stuff you've mentioned before right? I think this is the other piece I'd be very interested in. Yup, that's what it is. IPA also has/will have a plugin to run tests within a class in source order (respecting inheritance), rather than in pytest's unspecified order (usually alphabetically, IIRC). It can be extracted as well if there's interest. When you say in source order here, you mean source code order? So, we could actually order tests in the file as we see fit instead of relying on naming to define execution order? Yes. (Except it orders per class, not per source file.) It's not good practice to rely on test order, but it works for integration tests with big setup costs. Would this affect use of the built in setup/teardown fixtures? Or we should just stay away from those anyway? I'm not sure what you mean by built in setup/teardown fixtures? Anyway it should be safe, unless you use another test reordering plugin as well. (e.g. there's a 3rd party plugin for distributed running of tests, using that would not be a good idea) -- PetrĀ³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0157] Fix installer adds invalid zonemgr email
Ticket: https://fedorahosted.org/freeipa/ticket/4707 Patch attached -- Martin Basti From e978fbd94ef8f4d1d2a7cf44f38b5871e4b119b6 Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Fri, 7 Nov 2014 12:45:43 +0100 Subject: [PATCH] Fix: DNS installer adds invalid zonemgr email Installer adds zonemgr as relative (and invalid) address. This fix force installer to use absolute email. Ticket: https://fedorahosted.org/freeipa/ticket/4707 --- install/share/bind.zone.db.template | 2 +- ipaserver/install/bindinstance.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/install/share/bind.zone.db.template b/install/share/bind.zone.db.template index 6795bb01a7d8003a26dcc8a1cbc337550b3c296c..ec175c60825869ea9b86f7d1351a96189028b5d4 100644 --- a/install/share/bind.zone.db.template +++ b/install/share/bind.zone.db.template @@ -1,6 +1,6 @@ $$ORIGIN $DOMAIN. $$TTL 86400 -@ IN SOA $DOMAIN. $ZONEMGR. ( +@ IN SOA $DOMAIN. $ZONEMGR ( 01 ; serial 3H ; refresh 15M ; retry diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py index 6cf018e9cda3734a99a8ac5ac1df134e9e4c2293..16894de0a009aacb123cf76072f2556aebc5722f 100644 --- a/ipaserver/install/bindinstance.py +++ b/ipaserver/install/bindinstance.py @@ -563,7 +563,7 @@ class BindInstance(service.Service): self.no_dnssec_validation=no_dnssec_validation if not zonemgr: -self.zonemgr = 'hostmaster.%s' % self.domain +self.zonemgr = 'hostmaster.%s' % normalize_zone(self.domain) else: self.zonemgr = normalize_zonemgr(zonemgr) -- 1.8.3.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0019] Prefer TCP connections to UDP in krb5 clients
On Fri, 07 Nov 2014 08:02:02 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 01:46 AM, Simo Sorce wrote: On Thu, 06 Nov 2014 18:00:21 -0500 Nathaniel McCallum npmccal...@redhat.com wrote: On Fri, 2013-10-04 at 06:12 -0400, Simo Sorce wrote: - Original Message - On 3.10.2013 23:43, Nathaniel McCallum wrote: Patch attached. I'm curious - what is the purpose of this patch? To prevent 1 second timeouts and re-transmits when OTP is in place? What is the expected performance impact? Could it be configured for OTP separately - somehow? (I guess that it is not possible now ...) It benefits also communication of large packets (when large MS-PAC or CAMMAC AD Data are attached), so it is a better choice for IPA in general. Especially given we have multiple KDC processes configured we do not want clients wasting KDC resources by making multiple processes do the same operation. So apparently this patch never got reviewed over a year ago. It was related to a bug which was opened in SSSD. However, when it became clear we wanted to solve this in FreeIPA, the SSSD bug was closed but no corresponding FreeIPA bug was opened. The patch then fell through the cracks. Right - without an associated ticket tracking the patch, it is too easy to loose it unless the author prods people to review it. Without this patch, if OTP validation runs long we get retransmits and failures. One question I have is how to handle this for upgrades since (I think) this patch only handles new installs. Anyway, this patch is somewhat urgent now. So help is appreciated. I have attached a rebased version which has no other changes. Nathaniel I am not sure we can do much on updates, we do not have a client-update tool, I would just document it I guess. Otherwise we'd have to go back to sssd which can inject additional values in krb5.conf, however I am not sure it would be ok to set something like this in the sssd's pubconf includes ... Agreed, pubconf update does not sound right. However, we already update krb5.conf on client updates, in %post: %post client if [ $1 -gt 1 ] ; then # Has the client been configured? restore=0 test -f '/var/lib/ipa-client/sysrestore/sysrestore.index' restore=$(wc -l '/var/lib/ipa-client/sysrestore/sysrestore.index' | awk '{print $1}') if [ -f '/etc/sssd/sssd.conf' -a $restore -ge 2 ]; then if ! grep -E -q '/var/lib/sss/pubconf/krb5.include.d/' /etc/krb5.conf 2/dev/null ; then echo includedir /var/lib/sss/pubconf/krb5.include.d/ /etc/krb5.conf.ipanew cat /etc/krb5.conf /etc/krb5.conf.ipanew mv /etc/krb5.conf.ipanew /etc/krb5.conf /sbin/restorecon /etc/krb5.conf fi fi ... This particular update is more difficult as not a first line needs to be changed. Without adding ipa client update tool with some reasonable krb5.conf parser, we could do something along the lines of sed -E 0i 's/(forwardable = \w+)/\1\n udp_preference_limit = 0/g' /etc/krb5.conf (tested), but it is not pretty. What happen the next time you run it again ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0019] Prefer TCP connections to UDP in krb5 clients
On 11/07/2014 02:20 PM, Simo Sorce wrote: On Fri, 07 Nov 2014 08:02:02 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 01:46 AM, Simo Sorce wrote: On Thu, 06 Nov 2014 18:00:21 -0500 Nathaniel McCallum npmccal...@redhat.com wrote: On Fri, 2013-10-04 at 06:12 -0400, Simo Sorce wrote: - Original Message - On 3.10.2013 23:43, Nathaniel McCallum wrote: Patch attached. I'm curious - what is the purpose of this patch? To prevent 1 second timeouts and re-transmits when OTP is in place? What is the expected performance impact? Could it be configured for OTP separately - somehow? (I guess that it is not possible now ...) It benefits also communication of large packets (when large MS-PAC or CAMMAC AD Data are attached), so it is a better choice for IPA in general. Especially given we have multiple KDC processes configured we do not want clients wasting KDC resources by making multiple processes do the same operation. So apparently this patch never got reviewed over a year ago. It was related to a bug which was opened in SSSD. However, when it became clear we wanted to solve this in FreeIPA, the SSSD bug was closed but no corresponding FreeIPA bug was opened. The patch then fell through the cracks. Right - without an associated ticket tracking the patch, it is too easy to loose it unless the author prods people to review it. Without this patch, if OTP validation runs long we get retransmits and failures. One question I have is how to handle this for upgrades since (I think) this patch only handles new installs. Anyway, this patch is somewhat urgent now. So help is appreciated. I have attached a rebased version which has no other changes. Nathaniel I am not sure we can do much on updates, we do not have a client-update tool, I would just document it I guess. Otherwise we'd have to go back to sssd which can inject additional values in krb5.conf, however I am not sure it would be ok to set something like this in the sssd's pubconf includes ... Agreed, pubconf update does not sound right. However, we already update krb5.conf on client updates, in %post: %post client if [ $1 -gt 1 ] ; then # Has the client been configured? restore=0 test -f '/var/lib/ipa-client/sysrestore/sysrestore.index' restore=$(wc -l '/var/lib/ipa-client/sysrestore/sysrestore.index' | awk '{print $1}') if [ -f '/etc/sssd/sssd.conf' -a $restore -ge 2 ]; then if ! grep -E -q '/var/lib/sss/pubconf/krb5.include.d/' /etc/krb5.conf 2/dev/null ; then echo includedir /var/lib/sss/pubconf/krb5.include.d/ /etc/krb5.conf.ipanew cat /etc/krb5.conf /etc/krb5.conf.ipanew mv /etc/krb5.conf.ipanew /etc/krb5.conf /sbin/restorecon /etc/krb5.conf fi fi ... This particular update is more difficult as not a first line needs to be changed. Without adding ipa client update tool with some reasonable krb5.conf parser, we could do something along the lines of sed -E 0i 's/(forwardable = \w+)/\1\n udp_preference_limit = 0/g' /etc/krb5.conf (tested), but it is not pretty. What happen the next time you run it again ? Simo. It would of course be added again, you would need to first grep for presence of udp_preference_limit setting. Question is if this approach is safe enough to be in our client %post upgrade. We already upgrade krb5.conf here, just the change is much easier as we just add a line to the beginning of the file. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0157] Fix installer adds invalid zonemgr email
On 11/07/2014 01:33 PM, Martin Basti wrote: Ticket: https://fedorahosted.org/freeipa/ticket/4707 Patch attached ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel I think that it would be better to use relative value (e.g.: hostmaster instead of hostmaster.my.example.zone.). But this works, ACK. -- David Kupka ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] Gaps in upstream tests
FreeIPA team will soon grow with a new member focusing on upstream QE tests. I would like to collect ideas what are the biggest gaps in the current upstream test suite from your POV. Existing requests are tracked here: https://fedorahosted.org/freeipa/query?status=assignedstatus=newstatus=reopenedcomponent=Testscol=idcol=summarycol=componentcol=statuscol=ownercol=typecol=prioritycol=milestonegroup=milestoneorder=priority First idea that I head proposed are Upgrade tests. These are often done manually. I think that upgrade test from currently supported FreeIPA/Fedora version would go a long way (like 3.3.5 on F20 upgraded built RPMs and running unit tests). Second, it would be nice to try testing FreeIPA server in a container. Not only it would verify our container efforts, but it may also allow easy multi-master tests on one Jenkins VM or local host instead of expensive VM orchestration. Any other areas worth focusing on (besides of course testing newly developed features)? -- Martin Kosek mko...@redhat.com Supervisor, Software Engineering - Identity Management Team Red Hat Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0019] Prefer TCP connections to UDP in krb5 clients
On Fri, 07 Nov 2014 14:26:06 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 02:20 PM, Simo Sorce wrote: On Fri, 07 Nov 2014 08:02:02 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 01:46 AM, Simo Sorce wrote: On Thu, 06 Nov 2014 18:00:21 -0500 Nathaniel McCallum npmccal...@redhat.com wrote: On Fri, 2013-10-04 at 06:12 -0400, Simo Sorce wrote: - Original Message - On 3.10.2013 23:43, Nathaniel McCallum wrote: Patch attached. I'm curious - what is the purpose of this patch? To prevent 1 second timeouts and re-transmits when OTP is in place? What is the expected performance impact? Could it be configured for OTP separately - somehow? (I guess that it is not possible now ...) It benefits also communication of large packets (when large MS-PAC or CAMMAC AD Data are attached), so it is a better choice for IPA in general. Especially given we have multiple KDC processes configured we do not want clients wasting KDC resources by making multiple processes do the same operation. So apparently this patch never got reviewed over a year ago. It was related to a bug which was opened in SSSD. However, when it became clear we wanted to solve this in FreeIPA, the SSSD bug was closed but no corresponding FreeIPA bug was opened. The patch then fell through the cracks. Right - without an associated ticket tracking the patch, it is too easy to loose it unless the author prods people to review it. Without this patch, if OTP validation runs long we get retransmits and failures. One question I have is how to handle this for upgrades since (I think) this patch only handles new installs. Anyway, this patch is somewhat urgent now. So help is appreciated. I have attached a rebased version which has no other changes. Nathaniel I am not sure we can do much on updates, we do not have a client-update tool, I would just document it I guess. Otherwise we'd have to go back to sssd which can inject additional values in krb5.conf, however I am not sure it would be ok to set something like this in the sssd's pubconf includes ... Agreed, pubconf update does not sound right. However, we already update krb5.conf on client updates, in %post: %post client if [ $1 -gt 1 ] ; then # Has the client been configured? restore=0 test -f '/var/lib/ipa-client/sysrestore/sysrestore.index' restore=$(wc -l '/var/lib/ipa-client/sysrestore/sysrestore.index' | awk '{print $1}') if [ -f '/etc/sssd/sssd.conf' -a $restore -ge 2 ]; then if ! grep -E -q '/var/lib/sss/pubconf/krb5.include.d/' /etc/krb5.conf 2/dev/null ; then echo includedir /var/lib/sss/pubconf/krb5.include.d/ /etc/krb5.conf.ipanew cat /etc/krb5.conf /etc/krb5.conf.ipanew mv /etc/krb5.conf.ipanew /etc/krb5.conf /sbin/restorecon /etc/krb5.conf fi fi ... This particular update is more difficult as not a first line needs to be changed. Without adding ipa client update tool with some reasonable krb5.conf parser, we could do something along the lines of sed -E 0i 's/(forwardable = \w+)/\1\n udp_preference_limit = 0/g' /etc/krb5.conf (tested), but it is not pretty. What happen the next time you run it again ? Simo. It would of course be added again, you would need to first grep for presence of udp_preference_limit setting. Question is if this approach is safe enough to be in our client %post upgrade. We already upgrade krb5.conf here, just the change is much easier as we just add a line to the beginning of the file. Well the concern (aside of duplication) is that an admin may correct the krb5.conf file to remove that option (for example because his clients also connect to a differen (older) KDC and must use UDP in preference. But now we end up messing with its krb5.conf every time an update is released. An update tool that keep tracks of whether a specific update has already been applied and does not retry every time would be needed IMO. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0019] Prefer TCP connections to UDP in krb5 clients
On 11/07/2014 02:51 PM, Simo Sorce wrote: On Fri, 07 Nov 2014 14:26:06 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 02:20 PM, Simo Sorce wrote: On Fri, 07 Nov 2014 08:02:02 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 01:46 AM, Simo Sorce wrote: On Thu, 06 Nov 2014 18:00:21 -0500 Nathaniel McCallum npmccal...@redhat.com wrote: On Fri, 2013-10-04 at 06:12 -0400, Simo Sorce wrote: - Original Message - On 3.10.2013 23:43, Nathaniel McCallum wrote: Patch attached. I'm curious - what is the purpose of this patch? To prevent 1 second timeouts and re-transmits when OTP is in place? What is the expected performance impact? Could it be configured for OTP separately - somehow? (I guess that it is not possible now ...) It benefits also communication of large packets (when large MS-PAC or CAMMAC AD Data are attached), so it is a better choice for IPA in general. Especially given we have multiple KDC processes configured we do not want clients wasting KDC resources by making multiple processes do the same operation. So apparently this patch never got reviewed over a year ago. It was related to a bug which was opened in SSSD. However, when it became clear we wanted to solve this in FreeIPA, the SSSD bug was closed but no corresponding FreeIPA bug was opened. The patch then fell through the cracks. Right - without an associated ticket tracking the patch, it is too easy to loose it unless the author prods people to review it. Without this patch, if OTP validation runs long we get retransmits and failures. One question I have is how to handle this for upgrades since (I think) this patch only handles new installs. Anyway, this patch is somewhat urgent now. So help is appreciated. I have attached a rebased version which has no other changes. Nathaniel I am not sure we can do much on updates, we do not have a client-update tool, I would just document it I guess. Otherwise we'd have to go back to sssd which can inject additional values in krb5.conf, however I am not sure it would be ok to set something like this in the sssd's pubconf includes ... Agreed, pubconf update does not sound right. However, we already update krb5.conf on client updates, in %post: %post client if [ $1 -gt 1 ] ; then # Has the client been configured? restore=0 test -f '/var/lib/ipa-client/sysrestore/sysrestore.index' restore=$(wc -l '/var/lib/ipa-client/sysrestore/sysrestore.index' | awk '{print $1}') if [ -f '/etc/sssd/sssd.conf' -a $restore -ge 2 ]; then if ! grep -E -q '/var/lib/sss/pubconf/krb5.include.d/' /etc/krb5.conf 2/dev/null ; then echo includedir /var/lib/sss/pubconf/krb5.include.d/ /etc/krb5.conf.ipanew cat /etc/krb5.conf /etc/krb5.conf.ipanew mv /etc/krb5.conf.ipanew /etc/krb5.conf /sbin/restorecon /etc/krb5.conf fi fi ... This particular update is more difficult as not a first line needs to be changed. Without adding ipa client update tool with some reasonable krb5.conf parser, we could do something along the lines of sed -E 0i 's/(forwardable = \w+)/\1\n udp_preference_limit = 0/g' /etc/krb5.conf (tested), but it is not pretty. What happen the next time you run it again ? Simo. It would of course be added again, you would need to first grep for presence of udp_preference_limit setting. Question is if this approach is safe enough to be in our client %post upgrade. We already upgrade krb5.conf here, just the change is much easier as we just add a line to the beginning of the file. Well the concern (aside of duplication) is that an admin may correct the krb5.conf file to remove that option (for example because his clients also connect to a differen (older) KDC and must use UDP in preference. But now we end up messing with its krb5.conf every time an update is released. An update tool that keep tracks of whether a specific update has already been applied and does not retry every time would be needed IMO. Simo. In 4.1.x (as there is not much time to develop a separate client update tool), we could grep just for udp_preference_limit presence so that if admin changes it's value or comment it, it would not be added again. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hello Nathaniel, Few comments on the review: * in authcfg o in string_to_types, I would prefer you set the last element of 'map' to { NULL, 0 }. o in entry_to_window, you may declare the 'defaults' array as 'static const' o Would use define for ipaUserAuthType,ipaHOTPAuthWindow, ipaTOTPAuthWindow, ipaHOTPSyncWindow,ipaTOTPSyncWindow that are present multiple times o suffix_to_config: cfg is set (and returned) calling entry_to_config(entry). Now the entry_to_config returns a structure on the stack so it is not valid to access outside of the entry_to_config o authcfg_fini free the configs. config-cfg should have been allocated and must be freed (be care that configs-cfg may contains DEFAULTS) o authcfg_get_auth_types:322 should it return 'gbl' or AUTHCFG_AUTH_TYPE_PASSWORD o authcfg_get_auth_window/authcfg_get_sync_window returns a window structure that is on the stack. It is not valid outside of those functions thanks thierry ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0019] Prefer TCP connections to UDP in krb5 clients
On Fri, 07 Nov 2014 14:53:17 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 02:51 PM, Simo Sorce wrote: On Fri, 07 Nov 2014 14:26:06 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 02:20 PM, Simo Sorce wrote: On Fri, 07 Nov 2014 08:02:02 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 01:46 AM, Simo Sorce wrote: On Thu, 06 Nov 2014 18:00:21 -0500 Nathaniel McCallum npmccal...@redhat.com wrote: On Fri, 2013-10-04 at 06:12 -0400, Simo Sorce wrote: - Original Message - On 3.10.2013 23:43, Nathaniel McCallum wrote: Patch attached. I'm curious - what is the purpose of this patch? To prevent 1 second timeouts and re-transmits when OTP is in place? What is the expected performance impact? Could it be configured for OTP separately - somehow? (I guess that it is not possible now ...) It benefits also communication of large packets (when large MS-PAC or CAMMAC AD Data are attached), so it is a better choice for IPA in general. Especially given we have multiple KDC processes configured we do not want clients wasting KDC resources by making multiple processes do the same operation. So apparently this patch never got reviewed over a year ago. It was related to a bug which was opened in SSSD. However, when it became clear we wanted to solve this in FreeIPA, the SSSD bug was closed but no corresponding FreeIPA bug was opened. The patch then fell through the cracks. Right - without an associated ticket tracking the patch, it is too easy to loose it unless the author prods people to review it. Without this patch, if OTP validation runs long we get retransmits and failures. One question I have is how to handle this for upgrades since (I think) this patch only handles new installs. Anyway, this patch is somewhat urgent now. So help is appreciated. I have attached a rebased version which has no other changes. Nathaniel I am not sure we can do much on updates, we do not have a client-update tool, I would just document it I guess. Otherwise we'd have to go back to sssd which can inject additional values in krb5.conf, however I am not sure it would be ok to set something like this in the sssd's pubconf includes ... Agreed, pubconf update does not sound right. However, we already update krb5.conf on client updates, in %post: %post client if [ $1 -gt 1 ] ; then # Has the client been configured? restore=0 test -f '/var/lib/ipa-client/sysrestore/sysrestore.index' restore=$(wc -l '/var/lib/ipa-client/sysrestore/sysrestore.index' | awk '{print $1}') if [ -f '/etc/sssd/sssd.conf' -a $restore -ge 2 ]; then if ! grep -E -q '/var/lib/sss/pubconf/krb5.include.d/' /etc/krb5.conf 2/dev/null ; then echo includedir /var/lib/sss/pubconf/krb5.include.d/ /etc/krb5.conf.ipanew cat /etc/krb5.conf /etc/krb5.conf.ipanew mv /etc/krb5.conf.ipanew /etc/krb5.conf /sbin/restorecon /etc/krb5.conf fi fi ... This particular update is more difficult as not a first line needs to be changed. Without adding ipa client update tool with some reasonable krb5.conf parser, we could do something along the lines of sed -E 0i 's/(forwardable = \w+)/\1\n udp_preference_limit = 0/g' /etc/krb5.conf (tested), but it is not pretty. What happen the next time you run it again ? Simo. It would of course be added again, you would need to first grep for presence of udp_preference_limit setting. Question is if this approach is safe enough to be in our client %post upgrade. We already upgrade krb5.conf here, just the change is much easier as we just add a line to the beginning of the file. Well the concern (aside of duplication) is that an admin may correct the krb5.conf file to remove that option (for example because his clients also connect to a differen (older) KDC and must use UDP in preference. But now we end up messing with its krb5.conf every time an update is released. An update tool that keep tracks of whether a specific update has already been applied and does not retry every time would be needed IMO. Simo. In 4.1.x (as there is not much time to develop a separate client update tool), we could grep just for udp_preference_limit presence so that if admin changes it's value or comment it, it would not be added again. Ok then maybe we add this: # The following value has been added by a freeipa client update # if you want to disable it, please comment it, do not delete it # or it will be re-added on the next update udp_preference_limit = 0 What do you think ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0159] FIX: DNS policy upgrade raises assertion error
Ticket: https://fedorahosted.org/freeipa/ticket/4708 Patch attached. -- Martin Basti From 16b4b38d3feb09485c6af7033b46507ebb5df3a9 Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Fri, 7 Nov 2014 15:09:29 +0100 Subject: [PATCH] Fix: DNS policy upgrade raises asertion error Ticket: https://fedorahosted.org/freeipa/ticket/4708 --- ipaserver/install/plugins/dns.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipaserver/install/plugins/dns.py b/ipaserver/install/plugins/dns.py index 1aef837f63176cd307868c726460485fd4a004ed..f5bb6ac8cc4271195f369fcb15f289bb7673e194 100644 --- a/ipaserver/install/plugins/dns.py +++ b/ipaserver/install/plugins/dns.py @@ -65,7 +65,7 @@ class update_dnszones(PostUpdate): return (False, False, []) try: -zones = api.Command.dnszone_find(all=True)['result'] +zones = api.Command.dnszone_find(all=True, raw=True)['result'] except errors.NotFound: self.log.info('No DNS zone to update found') return (False, False, []) -- 1.8.3.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Gaps in upstream tests
Martin Kosek wrote: FreeIPA team will soon grow with a new member focusing on upstream QE tests. I would like to collect ideas what are the biggest gaps in the current upstream test suite from your POV. Existing requests are tracked here: https://fedorahosted.org/freeipa/query?status=assignedstatus=newstatus=reopenedcomponent=Testscol=idcol=summarycol=componentcol=statuscol=ownercol=typecol=prioritycol=milestonegroup=milestoneorder=priority First idea that I head proposed are Upgrade tests. These are often done manually. I think that upgrade test from currently supported FreeIPA/Fedora version would go a long way (like 3.3.5 on F20 upgraded built RPMs and running unit tests). Second, it would be nice to try testing FreeIPA server in a container. Not only it would verify our container efforts, but it may also allow easy multi-master tests on one Jenkins VM or local host instead of expensive VM orchestration. Any other areas worth focusing on (besides of course testing newly developed features)? Testing access control as non-admin. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0019] Prefer TCP connections to UDP in krb5 clients
On 11/07/2014 03:03 PM, Simo Sorce wrote: On Fri, 07 Nov 2014 14:53:17 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 02:51 PM, Simo Sorce wrote: On Fri, 07 Nov 2014 14:26:06 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 02:20 PM, Simo Sorce wrote: On Fri, 07 Nov 2014 08:02:02 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 01:46 AM, Simo Sorce wrote: On Thu, 06 Nov 2014 18:00:21 -0500 Nathaniel McCallum npmccal...@redhat.com wrote: On Fri, 2013-10-04 at 06:12 -0400, Simo Sorce wrote: - Original Message - On 3.10.2013 23:43, Nathaniel McCallum wrote: Patch attached. I'm curious - what is the purpose of this patch? To prevent 1 second timeouts and re-transmits when OTP is in place? What is the expected performance impact? Could it be configured for OTP separately - somehow? (I guess that it is not possible now ...) It benefits also communication of large packets (when large MS-PAC or CAMMAC AD Data are attached), so it is a better choice for IPA in general. Especially given we have multiple KDC processes configured we do not want clients wasting KDC resources by making multiple processes do the same operation. So apparently this patch never got reviewed over a year ago. It was related to a bug which was opened in SSSD. However, when it became clear we wanted to solve this in FreeIPA, the SSSD bug was closed but no corresponding FreeIPA bug was opened. The patch then fell through the cracks. Right - without an associated ticket tracking the patch, it is too easy to loose it unless the author prods people to review it. Without this patch, if OTP validation runs long we get retransmits and failures. One question I have is how to handle this for upgrades since (I think) this patch only handles new installs. Anyway, this patch is somewhat urgent now. So help is appreciated. I have attached a rebased version which has no other changes. Nathaniel I am not sure we can do much on updates, we do not have a client-update tool, I would just document it I guess. Otherwise we'd have to go back to sssd which can inject additional values in krb5.conf, however I am not sure it would be ok to set something like this in the sssd's pubconf includes ... Agreed, pubconf update does not sound right. However, we already update krb5.conf on client updates, in %post: %post client if [ $1 -gt 1 ] ; then # Has the client been configured? restore=0 test -f '/var/lib/ipa-client/sysrestore/sysrestore.index' restore=$(wc -l '/var/lib/ipa-client/sysrestore/sysrestore.index' | awk '{print $1}') if [ -f '/etc/sssd/sssd.conf' -a $restore -ge 2 ]; then if ! grep -E -q '/var/lib/sss/pubconf/krb5.include.d/' /etc/krb5.conf 2/dev/null ; then echo includedir /var/lib/sss/pubconf/krb5.include.d/ /etc/krb5.conf.ipanew cat /etc/krb5.conf /etc/krb5.conf.ipanew mv /etc/krb5.conf.ipanew /etc/krb5.conf /sbin/restorecon /etc/krb5.conf fi fi ... This particular update is more difficult as not a first line needs to be changed. Without adding ipa client update tool with some reasonable krb5.conf parser, we could do something along the lines of sed -E 0i 's/(forwardable = \w+)/\1\n udp_preference_limit = 0/g' /etc/krb5.conf (tested), but it is not pretty. What happen the next time you run it again ? Simo. It would of course be added again, you would need to first grep for presence of udp_preference_limit setting. Question is if this approach is safe enough to be in our client %post upgrade. We already upgrade krb5.conf here, just the change is much easier as we just add a line to the beginning of the file. Well the concern (aside of duplication) is that an admin may correct the krb5.conf file to remove that option (for example because his clients also connect to a differen (older) KDC and must use UDP in preference. But now we end up messing with its krb5.conf every time an update is released. An update tool that keep tracks of whether a specific update has already been applied and does not retry every time would be needed IMO. Simo. In 4.1.x (as there is not much time to develop a separate client update tool), we could grep just for udp_preference_limit presence so that if admin changes it's value or comment it, it would not be added again. Ok then maybe we add this: # The following value has been added by a freeipa client update # if you want to disable it, please comment it, do not delete it # or it will be re-added on the next update udp_preference_limit = 0 What do you think ? Simo. Sure, this could work (though it is quite lengthy). ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0158] FIX: upgrade refential integrity plugin configuration
Ticket: https://fedorahosted.org/freeipa/ticket/4622 Patch attached. -- Martin Basti From 524c486c5cfcfbe24a801a5ca58d6faa5a6f6e22 Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Fri, 7 Nov 2014 13:28:01 +0100 Subject: [PATCH] Fix upgrade referint plugin Mixing 'Old' and 'New' attr style for referential integrity plugin causes errors. Now old setting are migrated to new style setting before upgrade Ticket: https://fedorahosted.org/freeipa/ticket/4622 --- install/updates/25-referint.update | 13 +--- ipaserver/install/plugins/Makefile.am| 1 + ipaserver/install/plugins/update_referint.py | 90 3 files changed, 92 insertions(+), 12 deletions(-) create mode 100644 ipaserver/install/plugins/update_referint.py diff --git a/install/updates/25-referint.update b/install/updates/25-referint.update index a43d21ad5152358cb939c3545f0eef9d251e7fe0..609eaba74f0fcde6ce875093587315681fbd4584 100644 --- a/install/updates/25-referint.update +++ b/install/updates/25-referint.update @@ -1,19 +1,8 @@ # Expand attributes checked by Referential Integrity plugin # pres and eq indexes defined in 20-indices.update must be set for all these # attributes +# NOTE: migration to new style is done in update_referint.py dn: cn=referential integrity postoperation,cn=plugins,cn=config -remove: nsslapd-pluginArg7: manager -remove: nsslapd-pluginArg8: secretary -remove: nsslapd-pluginArg9: memberuser -remove: nsslapd-pluginArg10: memberhost -remove: nsslapd-pluginArg11: sourcehost -remove: nsslapd-pluginArg12: memberservice -remove: nsslapd-pluginArg13: managedby -remove: nsslapd-pluginArg14: memberallowcmd -remove: nsslapd-pluginArg15: memberdenycmd -remove: nsslapd-pluginArg16: ipasudorunas -remove: nsslapd-pluginArg17: ipasudorunasgroup -remove: nsslapd-pluginArg18: ipatokenradiusconfiglink add: referint-membership-attr: manager add: referint-membership-attr: secretary add: referint-membership-attr: memberuser diff --git a/ipaserver/install/plugins/Makefile.am b/ipaserver/install/plugins/Makefile.am index 635877d8c2160a91208276498cdb4cd9bc82d56b..d651297ac141b0f05831e7fabbb9b561cdd239c7 100644 --- a/ipaserver/install/plugins/Makefile.am +++ b/ipaserver/install/plugins/Makefile.am @@ -11,6 +11,7 @@ app_PYTHON = \ update_services.py \ update_anonymous_aci.py \ update_pacs.py \ + update_referint.py \ ca_renewal_master.py \ update_uniqueness.py \ $(NULL) diff --git a/ipaserver/install/plugins/update_referint.py b/ipaserver/install/plugins/update_referint.py new file mode 100644 index ..1b7411035b27ebba04246a7ee6f220d470b46688 --- /dev/null +++ b/ipaserver/install/plugins/update_referint.py @@ -0,0 +1,90 @@ +# +# Copyright (C) 2014 FreeIPA Contributors see COPYING for license +# + +from ipaserver.install.plugins import MIDDLE +from ipaserver.install.plugins.baseupdate import PreUpdate +from ipalib import api, errors +from ipapython.dn import DN +from ipapython.ipa_log_manager import root_logger + +class update_referint(PreUpdate): + +Update referential integrity configuration to new style +http://directory.fedoraproject.org/docs/389ds/design/ri-plugin-configuration.html + +old attr - new attr +nsslapd-pluginArg0- referint-update-delay +nsslapd-pluginArg1- referint-logfile +nsslapd-pluginArg2- referint-logchanges +nsslapd-pluginArg3..N - referint-membership-attr [3..N] + +Old and new style cannot be mixed, all nslapd-pluginArg* attrs have to be removed + + +order = MIDDLE + +referint_dn = DN(('cn', 'referential integrity postoperation'), + ('cn', 'plugins'), ('cn', 'config')) + +def execute(self, **options): + +root_logger.debug(Upgrading referential integrity plugin configuration) +ldap = self.obj.backend +try: +entry = ldap.get_entry(self.referint_dn) +except errors.NotFound: +root_logger.error(Referential integrity configuration not found) +return False, False, [] + +referint_membership_attrs = [] + +root_logger.debug(Initial value: %s, repr(entry)) + +# nsslapd-pluginArg0- referint-update-delay +update_delay = entry.get('nsslapd-pluginArg0') +if update_delay: +root_logger.debug(add: referint-update-delay: %s, update_delay) +entry['referint-update-delay'] = update_delay +entry['nsslapd-pluginArg0'] = None +else: +root_logger.info(Plugin already uses new style, skipping) +return False, False, [] + +# nsslapd-pluginArg1- referint-logfile +logfile = entry.get('nsslapd-pluginArg1') +if logfile: +root_logger.debug(add: referint-logfile: %s, logfile) +entry['referint-logfile'] = logfile +entry['nsslapd-pluginArg1'] = None + +# nsslapd-pluginArg2-
[Freeipa-devel] [PATCH] 0027 Produce better error in group-add command.
https://fedorahosted.org/freeipa/ticket/4611 -- David Kupka From a3e735c0309c740186d14f2430bdcf84c7d752b4 Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Wed, 5 Nov 2014 02:40:10 -0500 Subject: [PATCH] Produce better error in group-add command. https://fedorahosted.org/freeipa/ticket/4611 --- ipalib/plugins/group.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipalib/plugins/group.py b/ipalib/plugins/group.py index 03e6893e3c7604268b503b28ea39ed3f610aec47..9f3f1ba16b3db0e04897736979f08ba7a3bdfecd 100644 --- a/ipalib/plugins/group.py +++ b/ipalib/plugins/group.py @@ -287,7 +287,7 @@ class group_add(LDAPCreate): if options['external']: entry_attrs['objectclass'].append('ipaexternalgroup') if 'gidnumber' in options: -raise errors.RequirementError(name='gid') +raise errors.InvocationError(message=_('Invalid combination of parameters: \'gid\', \'external\'')) elif not options['nonposix']: entry_attrs['objectclass'].append('posixgroup') if not 'gidnumber' in options: -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0028 Remove unneeded internal methods. Move code to public, methods.
-- David Kupka From 0269a920231a992b67da713d40e29a28fdd32430 Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Thu, 6 Nov 2014 17:57:26 -0500 Subject: [PATCH] Remove unneeded internal methods. Move code to public methods. --- ipaplatform/base/services.py | 36 +--- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/ipaplatform/base/services.py b/ipaplatform/base/services.py index 961c368e6b4d81d337cf0a8601075f052352ecbf..01d95b39cc7b845bdc612d40b3eea29d6de2961a 100644 --- a/ipaplatform/base/services.py +++ b/ipaplatform/base/services.py @@ -436,7 +436,11 @@ class SystemdService(PlatformService): except: pass else: -self.__disable(instance_name) +try: +ipautil.run([paths.SYSTEMCTL, disable, + self.service_instance(instance_name)]) +except ipautil.CalledProcessError: +pass def mask(self, instance_name=): if instance_name != : @@ -444,40 +448,26 @@ class SystemdService(PlatformService): # remove instance file or link before masking if os.path.islink(srv_tgt): os.unlink(srv_tgt) - -self.__mask(instance_name) - -def unmask(self, instance_name=): -self.__unmask(instance_name) - -def __enable(self, instance_name=): -try: -ipautil.run([paths.SYSTEMCTL, enable, - self.service_instance(instance_name)]) -except ipautil.CalledProcessError: -pass - -def __disable(self, instance_name=): -try: -ipautil.run([paths.SYSTEMCTL, disable, - self.service_instance(instance_name)]) -except ipautil.CalledProcessError: -pass - -def __mask(self, instance_name=): try: ipautil.run([paths.SYSTEMCTL, mask, self.service_instance(instance_name)]) except ipautil.CalledProcessError: pass -def __unmask(self, instance_name=): +def unmask(self, instance_name=): try: ipautil.run([paths.SYSTEMCTL, unmask, self.service_instance(instance_name)]) except ipautil.CalledProcessError: pass +def __enable(self, instance_name=): +try: +ipautil.run([paths.SYSTEMCTL, enable, + self.service_instance(instance_name)]) +except ipautil.CalledProcessError: +pass + def install(self): self.enable() -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0019] Prefer TCP connections to UDP in krb5 clients
On Nov 7, 2014, at 9:21 AM, Martin Kosek mko...@redhat.com wrote: On 11/07/2014 03:03 PM, Simo Sorce wrote: On Fri, 07 Nov 2014 14:53:17 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 02:51 PM, Simo Sorce wrote: On Fri, 07 Nov 2014 14:26:06 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 02:20 PM, Simo Sorce wrote: On Fri, 07 Nov 2014 08:02:02 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 01:46 AM, Simo Sorce wrote: On Thu, 06 Nov 2014 18:00:21 -0500 Nathaniel McCallum npmccal...@redhat.com wrote: On Fri, 2013-10-04 at 06:12 -0400, Simo Sorce wrote: - Original Message - On 3.10.2013 23:43, Nathaniel McCallum wrote: Patch attached. I'm curious - what is the purpose of this patch? To prevent 1 second timeouts and re-transmits when OTP is in place? What is the expected performance impact? Could it be configured for OTP separately - somehow? (I guess that it is not possible now ...) It benefits also communication of large packets (when large MS-PAC or CAMMAC AD Data are attached), so it is a better choice for IPA in general. Especially given we have multiple KDC processes configured we do not want clients wasting KDC resources by making multiple processes do the same operation. So apparently this patch never got reviewed over a year ago. It was related to a bug which was opened in SSSD. However, when it became clear we wanted to solve this in FreeIPA, the SSSD bug was closed but no corresponding FreeIPA bug was opened. The patch then fell through the cracks. Right - without an associated ticket tracking the patch, it is too easy to loose it unless the author prods people to review it. Without this patch, if OTP validation runs long we get retransmits and failures. One question I have is how to handle this for upgrades since (I think) this patch only handles new installs. Anyway, this patch is somewhat urgent now. So help is appreciated. I have attached a rebased version which has no other changes. Nathaniel I am not sure we can do much on updates, we do not have a client-update tool, I would just document it I guess. Otherwise we'd have to go back to sssd which can inject additional values in krb5.conf, however I am not sure it would be ok to set something like this in the sssd's pubconf includes ... Agreed, pubconf update does not sound right. However, we already update krb5.conf on client updates, in %post: %post client if [ $1 -gt 1 ] ; then # Has the client been configured? restore=0 test -f '/var/lib/ipa-client/sysrestore/sysrestore.index' restore=$(wc -l '/var/lib/ipa-client/sysrestore/sysrestore.index' | awk '{print $1}') if [ -f '/etc/sssd/sssd.conf' -a $restore -ge 2 ]; then if ! grep -E -q '/var/lib/sss/pubconf/krb5.include.d/' /etc/krb5.conf 2/dev/null ; then echo includedir /var/lib/sss/pubconf/krb5.include.d/ /etc/krb5.conf.ipanew cat /etc/krb5.conf /etc/krb5.conf.ipanew mv /etc/krb5.conf.ipanew /etc/krb5.conf /sbin/restorecon /etc/krb5.conf fi fi ... This particular update is more difficult as not a first line needs to be changed. Without adding ipa client update tool with some reasonable krb5.conf parser, we could do something along the lines of sed -E 0i 's/(forwardable = \w+)/\1\n udp_preference_limit = 0/g' /etc/krb5.conf (tested), but it is not pretty. What happen the next time you run it again ? Simo. It would of course be added again, you would need to first grep for presence of udp_preference_limit setting. Question is if this approach is safe enough to be in our client %post upgrade. We already upgrade krb5.conf here, just the change is much easier as we just add a line to the beginning of the file. Well the concern (aside of duplication) is that an admin may correct the krb5.conf file to remove that option (for example because his clients also connect to a differen (older) KDC and must use UDP in preference. But now we end up messing with its krb5.conf every time an update is released. An update tool that keep tracks of whether a specific update has already been applied and does not retry every time would be needed IMO. Simo. In 4.1.x (as there is not much time to develop a separate client update tool), we could grep just for udp_preference_limit presence so that if admin changes it's value or comment it, it would not be added again. Ok then maybe we add this: # The following value has been added by a freeipa client update # if you want to disable it, please comment it, do not delete it # or it will be re-added on the next update udp_preference_limit = 0 What do you think ? Simo. Sure, this could work (though it is quite lengthy). I think it would be sufficient to only perform the addition of udp_preference_limit if it does not already exist and if the
[Freeipa-devel] [PATCH] 0029 Remove service file even if it isn't link.
Depends on freeipa-dkupka-0028. https://fedorahosted.org/freeipa/ticket/4658 -- David Kupka From 247ab543ed26c8eafa471b8b1d38309dacec9dbb Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Thu, 6 Nov 2014 18:08:58 -0500 Subject: [PATCH] Remove service file even if it isn't link. (Link to) service file from /etc/systemd/system/ must be removed before masking systemd service. https://fedorahosted.org/freeipa/ticket/4658 --- ipaplatform/base/services.py | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/ipaplatform/base/services.py b/ipaplatform/base/services.py index 01d95b39cc7b845bdc612d40b3eea29d6de2961a..c3cd04bd61602bf3eccf5dd1276ff7371a90a768 100644 --- a/ipaplatform/base/services.py +++ b/ipaplatform/base/services.py @@ -443,11 +443,9 @@ class SystemdService(PlatformService): pass def mask(self, instance_name=): -if instance_name != : -srv_tgt = os.path.join(paths.ETC_SYSTEMD_SYSTEM_DIR, instance_name) -# remove instance file or link before masking -if os.path.islink(srv_tgt): -os.unlink(srv_tgt) +srv_tgt = os.path.join(paths.ETC_SYSTEMD_SYSTEM_DIR, self.service_instance(instance_name)) +if os.path.exists(srv_tgt): +os.unlink(srv_tgt) try: ipautil.run([paths.SYSTEMCTL, mask, self.service_instance(instance_name)]) -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0307] Send DNS NOTIFY message after any modification to the zone
Hello, Send DNS NOTIFY message after any modification to the zone. https://fedorahosted.org/bind-dyndb-ldap/ticket/144 -- Petr^2 Spacek From 8980758721b57789c0f984465845f89c4705b872 Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Fri, 7 Nov 2014 15:12:38 +0100 Subject: [PATCH] Send DNS NOTIFY message after any modification to the zone. https://fedorahosted.org/bind-dyndb-ldap/ticket/144 --- src/ldap_helper.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/ldap_helper.c b/src/ldap_helper.c index cb1ada64635406552f6b231cdb19a888a0f92244..091be5ddd473cce170e2b84c7477143cf4a6ee15 100644 --- a/src/ldap_helper.c +++ b/src/ldap_helper.c @@ -1017,7 +1017,7 @@ cleanup: * @warning Never call this on raw part of in-line secure zone. */ static isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT -load_zone(dns_zone_t *zone) { +load_zone(dns_zone_t *zone, isc_boolean_t log) { isc_result_t result; isc_boolean_t zone_dynamic; isc_uint32_t serial; @@ -1036,15 +1036,18 @@ load_zone(dns_zone_t *zone) { } CHECK(dns_zone_getserial2(raw, serial)); - dns_zone_log(raw, ISC_LOG_INFO, loaded serial %u, serial); + if (log == ISC_TRUE) + dns_zone_log(raw, ISC_LOG_INFO, loaded serial %u, serial); if (zone != NULL) { result = dns_zone_getserial2(zone, serial); - if (result == ISC_R_SUCCESS) + if (result == ISC_R_SUCCESS log == ISC_TRUE) dns_zone_log(zone, ISC_LOG_INFO, loaded serial %u, serial); /* in-line secure zone is loaded asynchonously in background */ else if (result == DNS_R_NOTLOADED) { - dns_zone_log(zone, ISC_LOG_INFO, signing in progress); + if (log == ISC_TRUE) +dns_zone_log(zone, ISC_LOG_INFO, + signing in progress); result = ISC_R_SUCCESS; } else goto cleanup; @@ -1154,7 +1157,7 @@ activate_zone(isc_task_t *task, ldap_instance_t *inst, dns_name_t *name) { goto cleanup; } - CHECK(load_zone(toview)); + CHECK(load_zone(toview, ISC_TRUE)); if (secure != NULL) { CHECK(zr_get_zone_settings(inst-zone_register, name, zone_settings)); @@ -2491,9 +2494,7 @@ ldap_parse_master_zoneentry(ldap_entry_t * const entry, dns_db_t * const olddb, if (isactive == ISC_TRUE) { if (new_zone == ISC_TRUE || activity_changed == ISC_TRUE) CHECK(publish_zone(task, inst, toview)); - if (data_changed == ISC_TRUE || olddb != NULL || - activity_changed == ISC_TRUE) - CHECK(load_zone(toview)); + CHECK(load_zone(toview, ISC_FALSE)); } else if (activity_changed == ISC_TRUE) { /* Zone was deactivated */ CHECK(unpublish_zone(inst, name, entry-dn)); dns_zone_log(toview, ISC_LOG_INFO, zone deactivated @@ -4668,9 +4669,9 @@ cleanup: reload triggered by change in '%s', pevent-dn); if (secure != NULL) - result = load_zone(secure); + result = load_zone(secure, ISC_TRUE); else if (raw != NULL) - result = load_zone(raw); + result = load_zone(raw, ISC_TRUE); if (result == ISC_R_SUCCESS || result == DNS_R_UPTODATE || result == DNS_R_DYNAMIC || result == DNS_R_CONTINUE) { /* zone reload succeeded, fire current event again */ -- 1.9.3 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0019] Prefer TCP connections to UDP in krb5 clients
On 11/07/2014 03:28 PM, Nathaniel McCallum wrote: On Nov 7, 2014, at 9:21 AM, Martin Kosek mko...@redhat.com wrote: On 11/07/2014 03:03 PM, Simo Sorce wrote: On Fri, 07 Nov 2014 14:53:17 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 02:51 PM, Simo Sorce wrote: On Fri, 07 Nov 2014 14:26:06 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 02:20 PM, Simo Sorce wrote: On Fri, 07 Nov 2014 08:02:02 +0100 Martin Kosek mko...@redhat.com wrote: On 11/07/2014 01:46 AM, Simo Sorce wrote: On Thu, 06 Nov 2014 18:00:21 -0500 Nathaniel McCallum npmccal...@redhat.com wrote: On Fri, 2013-10-04 at 06:12 -0400, Simo Sorce wrote: - Original Message - On 3.10.2013 23:43, Nathaniel McCallum wrote: Patch attached. I'm curious - what is the purpose of this patch? To prevent 1 second timeouts and re-transmits when OTP is in place? What is the expected performance impact? Could it be configured for OTP separately - somehow? (I guess that it is not possible now ...) It benefits also communication of large packets (when large MS-PAC or CAMMAC AD Data are attached), so it is a better choice for IPA in general. Especially given we have multiple KDC processes configured we do not want clients wasting KDC resources by making multiple processes do the same operation. So apparently this patch never got reviewed over a year ago. It was related to a bug which was opened in SSSD. However, when it became clear we wanted to solve this in FreeIPA, the SSSD bug was closed but no corresponding FreeIPA bug was opened. The patch then fell through the cracks. Right - without an associated ticket tracking the patch, it is too easy to loose it unless the author prods people to review it. Without this patch, if OTP validation runs long we get retransmits and failures. One question I have is how to handle this for upgrades since (I think) this patch only handles new installs. Anyway, this patch is somewhat urgent now. So help is appreciated. I have attached a rebased version which has no other changes. Nathaniel I am not sure we can do much on updates, we do not have a client-update tool, I would just document it I guess. Otherwise we'd have to go back to sssd which can inject additional values in krb5.conf, however I am not sure it would be ok to set something like this in the sssd's pubconf includes ... Agreed, pubconf update does not sound right. However, we already update krb5.conf on client updates, in %post: %post client if [ $1 -gt 1 ] ; then # Has the client been configured? restore=0 test -f '/var/lib/ipa-client/sysrestore/sysrestore.index' restore=$(wc -l '/var/lib/ipa-client/sysrestore/sysrestore.index' | awk '{print $1}') if [ -f '/etc/sssd/sssd.conf' -a $restore -ge 2 ]; then if ! grep -E -q '/var/lib/sss/pubconf/krb5.include.d/' /etc/krb5.conf 2/dev/null ; then echo includedir /var/lib/sss/pubconf/krb5.include.d/ /etc/krb5.conf.ipanew cat /etc/krb5.conf /etc/krb5.conf.ipanew mv /etc/krb5.conf.ipanew /etc/krb5.conf /sbin/restorecon /etc/krb5.conf fi fi ... This particular update is more difficult as not a first line needs to be changed. Without adding ipa client update tool with some reasonable krb5.conf parser, we could do something along the lines of sed -E 0i 's/(forwardable = \w+)/\1\n udp_preference_limit = 0/g' /etc/krb5.conf (tested), but it is not pretty. What happen the next time you run it again ? Simo. It would of course be added again, you would need to first grep for presence of udp_preference_limit setting. Question is if this approach is safe enough to be in our client %post upgrade. We already upgrade krb5.conf here, just the change is much easier as we just add a line to the beginning of the file. Well the concern (aside of duplication) is that an admin may correct the krb5.conf file to remove that option (for example because his clients also connect to a differen (older) KDC and must use UDP in preference. But now we end up messing with its krb5.conf every time an update is released. An update tool that keep tracks of whether a specific update has already been applied and does not retry every time would be needed IMO. Simo. In 4.1.x (as there is not much time to develop a separate client update tool), we could grep just for udp_preference_limit presence so that if admin changes it's value or comment it, it would not be added again. Ok then maybe we add this: # The following value has been added by a freeipa client update # if you want to disable it, please comment it, do not delete it # or it will be re-added on the next update udp_preference_limit = 0 What do you think ? Simo. Sure, this could work (though it is quite lengthy). I think it would be sufficient to only perform the addition of udp_preference_limit if it does not already exist and if the version number of the package we are upgrading from is
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0079] Catch USBError during YubiKey location
https://fedorahosted.org/freeipa/ticket/4693 From 39d7b6b255e7907f3615d1bbecacee3705f1a66b Mon Sep 17 00:00:00 2001 From: Nathaniel McCallum npmccal...@redhat.com Date: Fri, 7 Nov 2014 10:47:43 -0500 Subject: [PATCH] Catch USBError during YubiKey location https://fedorahosted.org/freeipa/ticket/4693 --- ipalib/plugins/otptoken_yubikey.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ipalib/plugins/otptoken_yubikey.py b/ipalib/plugins/otptoken_yubikey.py index e70ddb6e42b5ea34d7ebecb252d6bbd73ac64d03..be726bc89b045a4c28af3163a5f7e39427c6e9f4 100644 --- a/ipalib/plugins/otptoken_yubikey.py +++ b/ipalib/plugins/otptoken_yubikey.py @@ -26,6 +26,7 @@ from ipalib.plugins.otptoken import otptoken import os import yubico +from usb.core import USBError __doc__ = _( YubiKey Tokens @@ -81,7 +82,7 @@ class otptoken_add_yubikey(Command): # Open the YubiKey try: yk = yubico.find_yubikey() -except yubico.yubikey.YubiKeyError, e: +except (USBError, yubico.yubikey.YubiKeyError): raise NotFound(reason=_('No YubiKey found')) assert yk.version_num() = (2, 1) -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0021] Fix minimal version of BIND for Fedora 20 and 21
Hello, Fix minimal version of BIND for Fedora 20 and 21. We should build new mkosek/freeipa COPR package ASAP to solve conflicts on upgrade. -- Petr^2 Spacek From 822014a9ed130c05469d80b0cc200cda52d015c5 Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Fri, 7 Nov 2014 16:53:48 +0100 Subject: [PATCH] Fix minimal version of BIND for Fedora 20 and 21 --- freeipa.spec.in | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/freeipa.spec.in b/freeipa.spec.in index 36c2a35e7a0c60d4f68e2d945688ee30506e47c6..b2ff97a11dcbb675940086ab9af9aea9bf7988be 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -160,7 +160,13 @@ Obsoletes: freeipa-server-selinux 3.3.0 # IPA but if it is configured we need a way to require versions # that work for us. Conflicts: bind-dyndb-ldap 6.0-4 -Conflicts: bind 9.9.6-2 +%if 0%{?fedora} = 21 +Conflicts: bind 9.9.6-3 +Conflicts: bind-utils 9.9.6-3 +%else +Conflicts: bind 9.9.4-19 +Conflicts: bind-utils 9.9.4-19 +%endif # DNSSEC Conflicts: opendnssec 1.4.6-4 -- 1.9.3 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0021] Fix minimal version of BIND for Fedora 20 and 21
On Fri, 2014-11-07 at 16:55 +0100, Petr Spacek wrote: Hello, Fix minimal version of BIND for Fedora 20 and 21. We should build new mkosek/freeipa COPR package ASAP to solve conflicts on upgrade. ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0021] Fix minimal version of BIND for Fedora 20 and 21
On 7.11.2014 16:57, Nathaniel McCallum wrote: On Fri, 2014-11-07 at 16:55 +0100, Petr Spacek wrote: Hello, Fix minimal version of BIND for Fedora 20 and 21. We should build new mkosek/freeipa COPR package ASAP to solve conflicts on upgrade. ACK Pushed to: ipa-4-1: 4662f28750e5d68cc268a090d5612d6cfb51e3e1 master: 74e0a8cebca251bf89144597f521440407a469ba -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0035] fix misleading Directory Manager --help output
Hello. Fix for https://fedorahosted.org/freeipa/ticket/4694 Thanks, Gabe From 815b0e65889ba602359278b0d07c55236a32f2b5 Mon Sep 17 00:00:00 2001 From: Gabe redhatri...@gmail.com Date: Fri, 7 Nov 2014 08:47:56 -0700 Subject: [PATCH] ipa-server-install Directory Manager help incorrect https://fedorahosted.org/freeipa/ticket/4694 --- install/tools/ipa-server-install |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install index 2c3a28b527af44588b1b6ba332b2f5e796c5725d..cb79cfbbc09966950f4e5fb51984008b20600143 100755 --- a/install/tools/ipa-server-install +++ b/install/tools/ipa-server-install @@ -164,7 +164,7 @@ def parse_options(): basic_group.add_option(-n, --domain, dest=domain_name, help=domain name) basic_group.add_option(-p, --ds-password, dest=dm_password, - sensitive=True, help=admin password) + sensitive=True, help=Directory Manager password) basic_group.add_option(-P, --master-password, dest=master_password, sensitive=True, help=kerberos master password (normally autogenerated)) -- 1.7.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On Fri, 2014-11-07 at 15:02 +0100, thierry bordaz wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hello Nathaniel, Few comments on the review: * in authcfg * in string_to_types, I would prefer you set the last element of 'map' to { NULL, 0 }. Why? What I have is perfectly legal ISO C and is exactly the equivalent of your code. In a structure initializer, undefined structure elements are zero'd. * in entry_to_window, you may declare the 'defaults' array as 'static const' Fixed. * Would use define for ipaUserAuthType,ipaHOTPAuthWindow, ipaTOTPAuthWindow, ipaHOTPSyncWindow,ipaTOTPSyncWindow that are present multiple times Fixed. * suffix_to_config: cfg is set (and returned) calling entry_to_config(entry). Now the entry_to_config returns a structure on the stack so it is not valid to access outside of the entry_to_config Nope. We are not passing by reference but by copy. This is perfectly valid C. * authcfg_fini free the configs. config-cfg should have been allocated and must be freed (be care that configs-cfg may contains DEFAULTS) Nope. The config-cfg structure is allocated and freed when config is. This is assignment by copy not by reference. * authcfg_get_auth_types:322 should it return 'gbl' or AUTHCFG_AUTH_TYPE_PASSWORD If both the global and per-user auth type is unset, the default is AUTHCFG_AUTH_TYPE_PASSWORD. We special case this here so that we don't have to special case it everywhere else in the code. The code is correct as stands. * authcfg_get_auth_window/authcfg_get_sync_window returns a window structure that is on the stack. It is not valid outside of those functions Nope. Structure return by copy is perfectly legal ISO C. From 7c348a5816b782b32f40ab00e7fd7cc6455f9600 Mon Sep 17 00:00:00 2001 From: Nathaniel McCallum npmccal...@redhat.com Date: Thu, 23 Oct 2014 15:18:26 -0400 Subject: [PATCH] Make token window sizes configurable This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 --- API.txt | 6 +- VERSION | 4 +- daemons/ipa-slapi-plugins/ipa-pwd-extop/authcfg.c | 200 +- daemons/ipa-slapi-plugins/ipa-pwd-extop/authcfg.h | 17 ++ daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 79 +++-- daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c | 7 +- daemons/ipa-slapi-plugins/libotp/libotp.c | 133 ++ daemons/ipa-slapi-plugins/libotp/libotp.h
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 11/07/2014 05:40 PM, Nathaniel McCallum wrote: On Fri, 2014-11-07 at 15:02 +0100, thierry bordaz wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hello Nathaniel, Few comments on the review: * in authcfg * in string_to_types, I would prefer you set the last element of 'map' to { NULL, 0 }. Why? What I have is perfectly legal ISO C and is exactly the equivalent of your code. In a structure initializer, undefined structure elements are zero'd. * in entry_to_window, you may declare the 'defaults' array as 'static const' Fixed. * Would use define for ipaUserAuthType,ipaHOTPAuthWindow, ipaTOTPAuthWindow, ipaHOTPSyncWindow,ipaTOTPSyncWindow that are present multiple times Fixed. * suffix_to_config: cfg is set (and returned) calling entry_to_config(entry). Now the entry_to_config returns a structure on the stack so it is not valid to access outside of the entry_to_config Nope. We are not passing by reference but by copy. This is perfectly valid C. * authcfg_fini free the configs. config-cfg should have been allocated and must be freed (be care that configs-cfg may contains DEFAULTS) Nope. The config-cfg structure is allocated and freed when config is. This is assignment by copy not by reference. * authcfg_get_auth_types:322 should it return 'gbl' or AUTHCFG_AUTH_TYPE_PASSWORD If both the global and per-user auth type is unset, the default is AUTHCFG_AUTH_TYPE_PASSWORD. We special case this here so that we don't have to special case it everywhere else in the code. The code is correct as stands. * authcfg_get_auth_window/authcfg_get_sync_window returns a window structure that is on the stack. It is not valid outside of those functions Nope. Structure return by copy is perfectly legal ISO C. Hi Nathaniel, You are right, I am not use to structure assignment and all these assignments are valid. Sorry for the noise. The patch is fine for me. thanks theirry ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel