Re: [Freeipa-devel] [PATCH] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).
On 07/01/2016 11:31 AM, David Kupka wrote: On 01/07/16 11:22, thierry bordaz wrote: On 07/01/2016 10:46 AM, David Kupka wrote: Hello Thierry! Thanks for looking into it. I will try to answer your questions and comments inline. On 01/07/16 10:26, thierry bordaz wrote: Hi David, The patch looks good but being not familiar with that code, my comments may be absolutely wrong In ipadb_get_pwd_expiration, if it is not 'self' we set '*export=mod_time'. If for some reason 'mod_time==0', it has now a specific meaning 'not expiring' . Does it match the comment '* not 'self', so reset */' mod_time should be timestamp of the modification operation. So mod_time == 0 should happen only when the change was performed in the very beginning of 70's. Right. My fault I did not understand that code. In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just before it adds in the mods krbPasswordExpiration=0 or krbPasswordExpiration=entry->pw_expiration Could we skip those mods if entry->pw_expiration==0 or expire_time==0 ? That is exactly what I thought. But in that part of code I have no chance to check if the attribute is present in the entry or not. Also I can't catch and ignore the resulting error when deleting nonexistent attribute. Here only LDAPMod structures are filled but the execution is done in an other part of code. So I choose the easy path and always set the attribute and delete it right after if necessary. I think there is something a bit strange here. To be able to delete the attribute we first need to set it to a specific value then deleting the value we manage to delete the attribute. I did not find a routine like ipa_get_ldap_mod_delattr. With such routine I wonder if we could to something like: if (entry->pw_expiration == 0) { kerr = ipadb_get_ldap_mod_delattr(imods, "krbPasswordExpiration"); } else { kerr = ipadb_get_ldap_mod_time(imods, "krbPasswordExpiration", entry->pw_expiration, mod_op); } Instead of kerr = ipadb_get_ldap_mod_time(imods, "krbPasswordExpiration", entry->pw_expiration, mod_op); if (entry->pw_expiration == 0) { kerr = ipadb_get_ldap_mod_time(imods, "krbPasswordExpiration", entry->pw_expiration, LDAP_MOD_DELETE); } At some point I have added such routine and tried same approach as you do. But when the krbPasswordExpiration is not present in the entry (user already has non-expiring password) there is an error later when the mods are applied (err=16 ~ no such attribute). When I found this I decided to always LDAP_MOD_REPLACE the attribute (replace operation does not fail when the attribute is missing). And then remove it when it shouldn't be there. After that I decided to remove my _del_attr routine because I didn't want to add new unnecessary code. Yes I agree and the code is working fine ! My concern is that ipadb_mods_new is trying to find empty mod slot. Hopefully there is no such empty slot currently. But in theory the mods can have a random order and I think it is dangerous. Adding a value to be able to delete the attribute afterward can fail if the ops are done in the opposite order. In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above. Something that I am not sure is what is the expected relation between passwordexpirationtime and krbPasswordExpiration I'm not sure either. I think we don't use passwordexpirationtime attribute. I think they should be somehow linked, in fact it is looking it is what happen in ipapwd_write_krb_keys. But it looks it happens only when the krb keys are created. Probablby, I don't recall seeing passwordexpirationtime attribute used anywhere. thanks thierry On 06/30/2016 09:34 PM, David Kupka wrote: On 04/05/16 17:22, Pavel Vomacka wrote: On 05/04/2016 04:36 PM, Simo Sorce wrote: On Wed, 2016-05-04 at 15:39 +0200, Martin Kosek wrote: On 05/02/2016 02:28 PM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/2795 That patch looks suspiciously short given the struggles I saw in http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html :-) Instead of setting to IPAPWD_END_OF_TIME, should we instead avoid filling "krbPasswordExpiration" attribute at all, i.e. have password *without* expiration? Or is krbPasswordExpiration mandatory? So I looked at the MIT code, and it seem like they are coping just fine with a missing (ie value = 0 internally) pw_expiration attribute. So if we make our code cope with omitting any expiration if the attribute is missing then yes, we can mark no expiration with simply removing (or not setting) the krbPasswordExpiration attribute. The attribute itself is optional and can be omitted. I think this is a good idea, and is definitely better than inventing a a magic value.
Re: [Freeipa-devel] [PATCH] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).
On 01/07/16 11:22, thierry bordaz wrote: On 07/01/2016 10:46 AM, David Kupka wrote: Hello Thierry! Thanks for looking into it. I will try to answer your questions and comments inline. On 01/07/16 10:26, thierry bordaz wrote: Hi David, The patch looks good but being not familiar with that code, my comments may be absolutely wrong In ipadb_get_pwd_expiration, if it is not 'self' we set '*export=mod_time'. If for some reason 'mod_time==0', it has now a specific meaning 'not expiring' . Does it match the comment '* not 'self', so reset */' mod_time should be timestamp of the modification operation. So mod_time == 0 should happen only when the change was performed in the very beginning of 70's. Right. My fault I did not understand that code. In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just before it adds in the mods krbPasswordExpiration=0 or krbPasswordExpiration=entry->pw_expiration Could we skip those mods if entry->pw_expiration==0 or expire_time==0 ? That is exactly what I thought. But in that part of code I have no chance to check if the attribute is present in the entry or not. Also I can't catch and ignore the resulting error when deleting nonexistent attribute. Here only LDAPMod structures are filled but the execution is done in an other part of code. So I choose the easy path and always set the attribute and delete it right after if necessary. I think there is something a bit strange here. To be able to delete the attribute we first need to set it to a specific value then deleting the value we manage to delete the attribute. I did not find a routine like ipa_get_ldap_mod_delattr. With such routine I wonder if we could to something like: if (entry->pw_expiration == 0) { kerr = ipadb_get_ldap_mod_delattr(imods, "krbPasswordExpiration"); } else { kerr = ipadb_get_ldap_mod_time(imods, "krbPasswordExpiration", entry->pw_expiration, mod_op); } Instead of kerr = ipadb_get_ldap_mod_time(imods, "krbPasswordExpiration", entry->pw_expiration, mod_op); if (entry->pw_expiration == 0) { kerr = ipadb_get_ldap_mod_time(imods, "krbPasswordExpiration", entry->pw_expiration, LDAP_MOD_DELETE); } At some point I have added such routine and tried same approach as you do. But when the krbPasswordExpiration is not present in the entry (user already has non-expiring password) there is an error later when the mods are applied (err=16 ~ no such attribute). When I found this I decided to always LDAP_MOD_REPLACE the attribute (replace operation does not fail when the attribute is missing). And then remove it when it shouldn't be there. After that I decided to remove my _del_attr routine because I didn't want to add new unnecessary code. In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above. Something that I am not sure is what is the expected relation between passwordexpirationtime and krbPasswordExpiration I'm not sure either. I think we don't use passwordexpirationtime attribute. I think they should be somehow linked, in fact it is looking it is what happen in ipapwd_write_krb_keys. But it looks it happens only when the krb keys are created. Probablby, I don't recall seeing passwordexpirationtime attribute used anywhere. thanks thierry On 06/30/2016 09:34 PM, David Kupka wrote: On 04/05/16 17:22, Pavel Vomacka wrote: On 05/04/2016 04:36 PM, Simo Sorce wrote: On Wed, 2016-05-04 at 15:39 +0200, Martin Kosek wrote: On 05/02/2016 02:28 PM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/2795 That patch looks suspiciously short given the struggles I saw in http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html :-) Instead of setting to IPAPWD_END_OF_TIME, should we instead avoid filling "krbPasswordExpiration" attribute at all, i.e. have password *without* expiration? Or is krbPasswordExpiration mandatory? So I looked at the MIT code, and it seem like they are coping just fine with a missing (ie value = 0 internally) pw_expiration attribute. So if we make our code cope with omitting any expiration if the attribute is missing then yes, we can mark no expiration with simply removing (or not setting) the krbPasswordExpiration attribute. The attribute itself is optional and can be omitted. I think this is a good idea, and is definitely better than inventing a a magic value. Simo. Just a note: I tested David's patch and it actually doesn't work when the new password policy for ipausers group is created (priority = 0, which should be the highest priority). The maxlife and minlife values are empty. Even if I set the new password policy maxlife and minlife to 0 the result was that password will expire in 90 days. The patch worked correctly when I changed value of maxlife and minl
Re: [Freeipa-devel] [PATCH] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).
On 07/01/2016 10:46 AM, David Kupka wrote: > Hello Thierry! > > Thanks for looking into it. I will try to answer your questions and > comments inline. > > On 01/07/16 10:26, thierry bordaz wrote: >> Hi David, >> >> The patch looks good but being not familiar with that code, my comments >> may be absolutely wrong >> >> In ipadb_get_pwd_expiration, if it is not 'self' we set >> '*export=mod_time'. >> If for some reason 'mod_time==0', it has now a specific meaning 'not >> expiring' . Does it match the comment '* not 'self', so reset */' > > mod_time should be timestamp of the modification operation. So mod_time > == 0 should happen only when the change was performed in the very > beginning of 70's. > >> >> In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just >> before it adds in the mods krbPasswordExpiration=0 or >> krbPasswordExpiration=entry->pw_expiration >> Could we skip those mods if entry->pw_expiration==0 or expire_time==0 ? > > That is exactly what I thought. But in that part of code I have no > chance to check if the attribute is present in the entry or not. Also I > can't catch and ignore the resulting error when deleting nonexistent > attribute. Here only LDAPMod structures are filled but the execution is > done in an other part of code. > So I choose the easy path and always set the attribute and delete it > right after if necessary. > >> >> In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above. >> >> Something that I am not sure is what is the expected relation between >> passwordexpirationtime and krbPasswordExpiration > > I'm not sure either. I think we don't use passwordexpirationtime attribute. > >> >> thanks >> thierry I don't see a strict NACK here. Given pvomacka's functional ACK, I have pushed it. We can fix corner cases later. Pushed to master: d2cb9ed327ee4003598d5e45d80ab7918b89eeed >> >> On 06/30/2016 09:34 PM, David Kupka wrote: >>> On 04/05/16 17:22, Pavel Vomacka wrote: On 05/04/2016 04:36 PM, Simo Sorce wrote: > On Wed, 2016-05-04 at 15:39 +0200, Martin Kosek wrote: >> On 05/02/2016 02:28 PM, David Kupka wrote: >>> https://fedorahosted.org/freeipa/ticket/2795 >> That patch looks suspiciously short given the struggles I saw in >> http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html >> :-) >> >> Instead of setting to IPAPWD_END_OF_TIME, should we instead avoid >> filling >> "krbPasswordExpiration" attribute at all, i.e. have password >> *without* >> expiration? Or is krbPasswordExpiration mandatory? > So I looked at the MIT code, and it seem like they are coping just > fine > with a missing (ie value = 0 internally) pw_expiration attribute. > > So if we make our code cope with omitting any expiration if the > attribute is missing then yes, we can mark no expiration with simply > removing (or not setting) the krbPasswordExpiration attribute. > The attribute itself is optional and can be omitted. > > I think this is a good idea, and is definitely better than inventing > a a > magic value. > > Simo. > Just a note: I tested David's patch and it actually doesn't work when the new password policy for ipausers group is created (priority = 0, which should be the highest priority). The maxlife and minlife values are empty. Even if I set the new password policy maxlife and minlife to 0 the result was that password will expire in 90 days. The patch worked correctly when I changed value of maxlife and minlife to 0 in 'global_policy'. Then the password expiration was set to 2038-01-01. >>> >>> Hello! >>> >>> I hope I've finally find all the places in ipa-kdb and ipa-pwd-extop >>> plugins to tickle in order to have password that don't expire. Updated >>> patch attached. >>> >>> https://fedorahosted.org/freeipa/ticket/2795 >>> >> -- 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] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).
On 07/01/2016 10:46 AM, David Kupka wrote: Hello Thierry! Thanks for looking into it. I will try to answer your questions and comments inline. On 01/07/16 10:26, thierry bordaz wrote: Hi David, The patch looks good but being not familiar with that code, my comments may be absolutely wrong In ipadb_get_pwd_expiration, if it is not 'self' we set '*export=mod_time'. If for some reason 'mod_time==0', it has now a specific meaning 'not expiring' . Does it match the comment '* not 'self', so reset */' mod_time should be timestamp of the modification operation. So mod_time == 0 should happen only when the change was performed in the very beginning of 70's. Right. My fault I did not understand that code. In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just before it adds in the mods krbPasswordExpiration=0 or krbPasswordExpiration=entry->pw_expiration Could we skip those mods if entry->pw_expiration==0 or expire_time==0 ? That is exactly what I thought. But in that part of code I have no chance to check if the attribute is present in the entry or not. Also I can't catch and ignore the resulting error when deleting nonexistent attribute. Here only LDAPMod structures are filled but the execution is done in an other part of code. So I choose the easy path and always set the attribute and delete it right after if necessary. I think there is something a bit strange here. To be able to delete the attribute we first need to set it to a specific value then deleting the value we manage to delete the attribute. I did not find a routine like ipa_get_ldap_mod_delattr. With such routine I wonder if we could to something like: if (entry->pw_expiration == 0) { kerr = ipadb_get_ldap_mod_delattr(imods, "krbPasswordExpiration"); } else { kerr = ipadb_get_ldap_mod_time(imods, "krbPasswordExpiration", entry->pw_expiration, mod_op); } Instead of kerr = ipadb_get_ldap_mod_time(imods, "krbPasswordExpiration", entry->pw_expiration, mod_op); if (entry->pw_expiration == 0) { kerr = ipadb_get_ldap_mod_time(imods, "krbPasswordExpiration", entry->pw_expiration, LDAP_MOD_DELETE); } In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above. Something that I am not sure is what is the expected relation between passwordexpirationtime and krbPasswordExpiration I'm not sure either. I think we don't use passwordexpirationtime attribute. I think they should be somehow linked, in fact it is looking it is what happen in ipapwd_write_krb_keys. But it looks it happens only when the krb keys are created. thanks thierry On 06/30/2016 09:34 PM, David Kupka wrote: On 04/05/16 17:22, Pavel Vomacka wrote: On 05/04/2016 04:36 PM, Simo Sorce wrote: On Wed, 2016-05-04 at 15:39 +0200, Martin Kosek wrote: On 05/02/2016 02:28 PM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/2795 That patch looks suspiciously short given the struggles I saw in http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html :-) Instead of setting to IPAPWD_END_OF_TIME, should we instead avoid filling "krbPasswordExpiration" attribute at all, i.e. have password *without* expiration? Or is krbPasswordExpiration mandatory? So I looked at the MIT code, and it seem like they are coping just fine with a missing (ie value = 0 internally) pw_expiration attribute. So if we make our code cope with omitting any expiration if the attribute is missing then yes, we can mark no expiration with simply removing (or not setting) the krbPasswordExpiration attribute. The attribute itself is optional and can be omitted. I think this is a good idea, and is definitely better than inventing a a magic value. Simo. Just a note: I tested David's patch and it actually doesn't work when the new password policy for ipausers group is created (priority = 0, which should be the highest priority). The maxlife and minlife values are empty. Even if I set the new password policy maxlife and minlife to 0 the result was that password will expire in 90 days. The patch worked correctly when I changed value of maxlife and minlife to 0 in 'global_policy'. Then the password expiration was set to 2038-01-01. Hello! I hope I've finally find all the places in ipa-kdb and ipa-pwd-extop plugins to tickle in order to have password that don't expire. Updated patch attached. https://fedorahosted.org/freeipa/ticket/2795 -- 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] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).
Hello Thierry! Thanks for looking into it. I will try to answer your questions and comments inline. On 01/07/16 10:26, thierry bordaz wrote: Hi David, The patch looks good but being not familiar with that code, my comments may be absolutely wrong In ipadb_get_pwd_expiration, if it is not 'self' we set '*export=mod_time'. If for some reason 'mod_time==0', it has now a specific meaning 'not expiring' . Does it match the comment '* not 'self', so reset */' mod_time should be timestamp of the modification operation. So mod_time == 0 should happen only when the change was performed in the very beginning of 70's. In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just before it adds in the mods krbPasswordExpiration=0 or krbPasswordExpiration=entry->pw_expiration Could we skip those mods if entry->pw_expiration==0 or expire_time==0 ? That is exactly what I thought. But in that part of code I have no chance to check if the attribute is present in the entry or not. Also I can't catch and ignore the resulting error when deleting nonexistent attribute. Here only LDAPMod structures are filled but the execution is done in an other part of code. So I choose the easy path and always set the attribute and delete it right after if necessary. In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above. Something that I am not sure is what is the expected relation between passwordexpirationtime and krbPasswordExpiration I'm not sure either. I think we don't use passwordexpirationtime attribute. thanks thierry On 06/30/2016 09:34 PM, David Kupka wrote: On 04/05/16 17:22, Pavel Vomacka wrote: On 05/04/2016 04:36 PM, Simo Sorce wrote: On Wed, 2016-05-04 at 15:39 +0200, Martin Kosek wrote: On 05/02/2016 02:28 PM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/2795 That patch looks suspiciously short given the struggles I saw in http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html :-) Instead of setting to IPAPWD_END_OF_TIME, should we instead avoid filling "krbPasswordExpiration" attribute at all, i.e. have password *without* expiration? Or is krbPasswordExpiration mandatory? So I looked at the MIT code, and it seem like they are coping just fine with a missing (ie value = 0 internally) pw_expiration attribute. So if we make our code cope with omitting any expiration if the attribute is missing then yes, we can mark no expiration with simply removing (or not setting) the krbPasswordExpiration attribute. The attribute itself is optional and can be omitted. I think this is a good idea, and is definitely better than inventing a a magic value. Simo. Just a note: I tested David's patch and it actually doesn't work when the new password policy for ipausers group is created (priority = 0, which should be the highest priority). The maxlife and minlife values are empty. Even if I set the new password policy maxlife and minlife to 0 the result was that password will expire in 90 days. The patch worked correctly when I changed value of maxlife and minlife to 0 in 'global_policy'. Then the password expiration was set to 2038-01-01. Hello! I hope I've finally find all the places in ipa-kdb and ipa-pwd-extop plugins to tickle in order to have password that don't expire. Updated patch attached. https://fedorahosted.org/freeipa/ticket/2795 -- David Kupka -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).
Hi David, I did a functional review, and everything works well, so functional-ACK. But I did not do the code review. On 07/01/2016 10:26 AM, thierry bordaz wrote: Hi David, The patch looks good but being not familiar with that code, my comments may be absolutely wrong In ipadb_get_pwd_expiration, if it is not 'self' we set '*export=mod_time'. If for some reason 'mod_time==0', it has now a specific meaning 'not expiring' . Does it match the comment '* not 'self', so reset */' In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just before it adds in the mods krbPasswordExpiration=0 or krbPasswordExpiration=entry->pw_expiration Could we skip those mods if entry->pw_expiration==0 or expire_time==0 ? In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above. Something that I am not sure is what is the expected relation between passwordexpirationtime and krbPasswordExpiration thanks thierry On 06/30/2016 09:34 PM, David Kupka wrote: On 04/05/16 17:22, Pavel Vomacka wrote: On 05/04/2016 04:36 PM, Simo Sorce wrote: On Wed, 2016-05-04 at 15:39 +0200, Martin Kosek wrote: On 05/02/2016 02:28 PM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/2795 That patch looks suspiciously short given the struggles I saw in http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html :-) Instead of setting to IPAPWD_END_OF_TIME, should we instead avoid filling "krbPasswordExpiration" attribute at all, i.e. have password *without* expiration? Or is krbPasswordExpiration mandatory? So I looked at the MIT code, and it seem like they are coping just fine with a missing (ie value = 0 internally) pw_expiration attribute. So if we make our code cope with omitting any expiration if the attribute is missing then yes, we can mark no expiration with simply removing (or not setting) the krbPasswordExpiration attribute. The attribute itself is optional and can be omitted. I think this is a good idea, and is definitely better than inventing a a magic value. Simo. Just a note: I tested David's patch and it actually doesn't work when the new password policy for ipausers group is created (priority = 0, which should be the highest priority). The maxlife and minlife values are empty. Even if I set the new password policy maxlife and minlife to 0 the result was that password will expire in 90 days. The patch worked correctly when I changed value of maxlife and minlife to 0 in 'global_policy'. Then the password expiration was set to 2038-01-01. Hello! I hope I've finally find all the places in ipa-kdb and ipa-pwd-extop plugins to tickle in order to have password that don't expire. Updated patch attached. https://fedorahosted.org/freeipa/ticket/2795 -- Pavel^3 Vomacka -- 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] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).
Hi David, The patch looks good but being not familiar with that code, my comments may be absolutely wrong In ipadb_get_pwd_expiration, if it is not 'self' we set '*export=mod_time'. If for some reason 'mod_time==0', it has now a specific meaning 'not expiring' . Does it match the comment '* not 'self', so reset */' In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just before it adds in the mods krbPasswordExpiration=0 or krbPasswordExpiration=entry->pw_expiration Could we skip those mods if entry->pw_expiration==0 or expire_time==0 ? In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above. Something that I am not sure is what is the expected relation between passwordexpirationtime and krbPasswordExpiration thanks thierry On 06/30/2016 09:34 PM, David Kupka wrote: On 04/05/16 17:22, Pavel Vomacka wrote: On 05/04/2016 04:36 PM, Simo Sorce wrote: On Wed, 2016-05-04 at 15:39 +0200, Martin Kosek wrote: On 05/02/2016 02:28 PM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/2795 That patch looks suspiciously short given the struggles I saw in http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html :-) Instead of setting to IPAPWD_END_OF_TIME, should we instead avoid filling "krbPasswordExpiration" attribute at all, i.e. have password *without* expiration? Or is krbPasswordExpiration mandatory? So I looked at the MIT code, and it seem like they are coping just fine with a missing (ie value = 0 internally) pw_expiration attribute. So if we make our code cope with omitting any expiration if the attribute is missing then yes, we can mark no expiration with simply removing (or not setting) the krbPasswordExpiration attribute. The attribute itself is optional and can be omitted. I think this is a good idea, and is definitely better than inventing a a magic value. Simo. Just a note: I tested David's patch and it actually doesn't work when the new password policy for ipausers group is created (priority = 0, which should be the highest priority). The maxlife and minlife values are empty. Even if I set the new password policy maxlife and minlife to 0 the result was that password will expire in 90 days. The patch worked correctly when I changed value of maxlife and minlife to 0 in 'global_policy'. Then the password expiration was set to 2038-01-01. Hello! I hope I've finally find all the places in ipa-kdb and ipa-pwd-extop plugins to tickle in order to have password that don't expire. Updated patch attached. https://fedorahosted.org/freeipa/ticket/2795 -- 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] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).
On 30/06/16 21:34, David Kupka wrote: On 04/05/16 17:22, Pavel Vomacka wrote: On 05/04/2016 04:36 PM, Simo Sorce wrote: On Wed, 2016-05-04 at 15:39 +0200, Martin Kosek wrote: On 05/02/2016 02:28 PM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/2795 That patch looks suspiciously short given the struggles I saw in http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html :-) Instead of setting to IPAPWD_END_OF_TIME, should we instead avoid filling "krbPasswordExpiration" attribute at all, i.e. have password *without* expiration? Or is krbPasswordExpiration mandatory? So I looked at the MIT code, and it seem like they are coping just fine with a missing (ie value = 0 internally) pw_expiration attribute. So if we make our code cope with omitting any expiration if the attribute is missing then yes, we can mark no expiration with simply removing (or not setting) the krbPasswordExpiration attribute. The attribute itself is optional and can be omitted. I think this is a good idea, and is definitely better than inventing a a magic value. Simo. Just a note: I tested David's patch and it actually doesn't work when the new password policy for ipausers group is created (priority = 0, which should be the highest priority). The maxlife and minlife values are empty. Even if I set the new password policy maxlife and minlife to 0 the result was that password will expire in 90 days. The patch worked correctly when I changed value of maxlife and minlife to 0 in 'global_policy'. Then the password expiration was set to 2038-01-01. Hello! I hope I've finally find all the places in ipa-kdb and ipa-pwd-extop plugins to tickle in order to have password that don't expire. Updated patch attached. https://fedorahosted.org/freeipa/ticket/2795 Updated patch attached. -- David Kupka From af5e8516cf743544f529c2cd234af91e5251380e Mon Sep 17 00:00:00 2001 From: David Kupka Date: Thu, 30 Jun 2016 08:52:33 +0200 Subject: [PATCH] Allow unexpiring passwords Treat maxlife=0 in password policy as "never expire". Delete krbPasswordExpiration in user entry when password should never expire. https://fedorahosted.org/freeipa/ticket/2795 --- daemons/ipa-kdb/ipa_kdb_passwords.c | 6 +- daemons/ipa-kdb/ipa_kdb_principals.c | 11 +++ daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c | 22 -- daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 4 ipaserver/plugins/pwpolicy.py | 2 +- 5 files changed, 33 insertions(+), 12 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb_passwords.c b/daemons/ipa-kdb/ipa_kdb_passwords.c index ad57181d5049f36c69044bb2c9cfe183d7e4ea25..a3d4fe2436da60d081040754780d3e815acb1473 100644 --- a/daemons/ipa-kdb/ipa_kdb_passwords.c +++ b/daemons/ipa-kdb/ipa_kdb_passwords.c @@ -253,7 +253,11 @@ krb5_error_code ipadb_get_pwd_expiration(krb5_context context, if (truexp) { if (ied->pol) { -*expire_time = mod_time + ied->pol->max_pwd_life; +if (ied->pol->max_pwd_life) { +*expire_time = mod_time + ied->pol->max_pwd_life; +} else { +*expire_time = 0; +} } else { *expire_time = mod_time + IPAPWD_DEFAULT_PWDLIFE; } diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c index f1d3e9e89c2016b8a9ebad9c0c6fd46487a33a4b..6cdfa909452a4b55912b2a5a74648abd2053482a 100644 --- a/daemons/ipa-kdb/ipa_kdb_principals.c +++ b/daemons/ipa-kdb/ipa_kdb_principals.c @@ -1850,6 +1850,11 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext, "krbPasswordExpiration", entry->pw_expiration, mod_op); +if (entry->pw_expiration == 0) { +kerr = ipadb_get_ldap_mod_time(imods, + "krbPasswordExpiration", + entry->pw_expiration, LDAP_MOD_DELETE); +} if (kerr) { goto done; } @@ -2105,6 +2110,12 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext, kerr = ipadb_get_ldap_mod_time(imods, "krbPasswordExpiration", expire_time, mod_op); +if (expire_time == 0) { +kerr = ipadb_get_ldap_mod_time(imods, + "krbPasswordExpiration", + expire_time, LDAP_MOD_DELETE); +} + if (kerr) { goto done; } diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c index 5dc606d22305cf63a16feca30aab2728bb20b80d..0bb50fc319e2b2520d36534d369ad42f95c80c8e 100644 --- a/daemons/ipa-slapi-plugins/ip
Re: [Freeipa-devel] [PATCH] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).
On 04/05/16 17:22, Pavel Vomacka wrote: On 05/04/2016 04:36 PM, Simo Sorce wrote: On Wed, 2016-05-04 at 15:39 +0200, Martin Kosek wrote: On 05/02/2016 02:28 PM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/2795 That patch looks suspiciously short given the struggles I saw in http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html :-) Instead of setting to IPAPWD_END_OF_TIME, should we instead avoid filling "krbPasswordExpiration" attribute at all, i.e. have password *without* expiration? Or is krbPasswordExpiration mandatory? So I looked at the MIT code, and it seem like they are coping just fine with a missing (ie value = 0 internally) pw_expiration attribute. So if we make our code cope with omitting any expiration if the attribute is missing then yes, we can mark no expiration with simply removing (or not setting) the krbPasswordExpiration attribute. The attribute itself is optional and can be omitted. I think this is a good idea, and is definitely better than inventing a a magic value. Simo. Just a note: I tested David's patch and it actually doesn't work when the new password policy for ipausers group is created (priority = 0, which should be the highest priority). The maxlife and minlife values are empty. Even if I set the new password policy maxlife and minlife to 0 the result was that password will expire in 90 days. The patch worked correctly when I changed value of maxlife and minlife to 0 in 'global_policy'. Then the password expiration was set to 2038-01-01. Hello! I hope I've finally find all the places in ipa-kdb and ipa-pwd-extop plugins to tickle in order to have password that don't expire. Updated patch attached. https://fedorahosted.org/freeipa/ticket/2795 -- David Kupka From dc2250869ca4f527b9b353913ed5a750a6f0f6e5 Mon Sep 17 00:00:00 2001 From: David Kupka Date: Thu, 30 Jun 2016 08:52:33 +0200 Subject: [PATCH] Allow unexpiring passwords Treat maxlife=0 in password policy as "never expire". Delete krbPasswordExpiration in user entry when password should never expire. https://fedorahosted.org/freeipa/ticket/2795 --- daemons/ipa-kdb/ipa_kdb_passwords.c | 6 +- daemons/ipa-kdb/ipa_kdb_principals.c | 11 +++ daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c | 11 ++- daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 4 ipaserver/plugins/pwpolicy.py | 2 +- 5 files changed, 31 insertions(+), 3 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb_passwords.c b/daemons/ipa-kdb/ipa_kdb_passwords.c index ad57181d5049f36c69044bb2c9cfe183d7e4ea25..a3d4fe2436da60d081040754780d3e815acb1473 100644 --- a/daemons/ipa-kdb/ipa_kdb_passwords.c +++ b/daemons/ipa-kdb/ipa_kdb_passwords.c @@ -253,7 +253,11 @@ krb5_error_code ipadb_get_pwd_expiration(krb5_context context, if (truexp) { if (ied->pol) { -*expire_time = mod_time + ied->pol->max_pwd_life; +if (ied->pol->max_pwd_life) { +*expire_time = mod_time + ied->pol->max_pwd_life; +} else { +*expire_time = 0; +} } else { *expire_time = mod_time + IPAPWD_DEFAULT_PWDLIFE; } diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c index f1d3e9e89c2016b8a9ebad9c0c6fd46487a33a4b..6cdfa909452a4b55912b2a5a74648abd2053482a 100644 --- a/daemons/ipa-kdb/ipa_kdb_principals.c +++ b/daemons/ipa-kdb/ipa_kdb_principals.c @@ -1850,6 +1850,11 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext, "krbPasswordExpiration", entry->pw_expiration, mod_op); +if (entry->pw_expiration == 0) { +kerr = ipadb_get_ldap_mod_time(imods, + "krbPasswordExpiration", + entry->pw_expiration, LDAP_MOD_DELETE); +} if (kerr) { goto done; } @@ -2105,6 +2110,12 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext, kerr = ipadb_get_ldap_mod_time(imods, "krbPasswordExpiration", expire_time, mod_op); +if (expire_time == 0) { +kerr = ipadb_get_ldap_mod_time(imods, + "krbPasswordExpiration", + expire_time, LDAP_MOD_DELETE); +} + if (kerr) { goto done; } diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c index 5dc606d22305cf63a16feca30aab2728bb20b80d..de295e0b4cbd8eeff556d96be3db68705114b994 100644 --- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c +++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
Re: [Freeipa-devel] [PATCH] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).
On 05/04/2016 04:36 PM, Simo Sorce wrote: On Wed, 2016-05-04 at 15:39 +0200, Martin Kosek wrote: On 05/02/2016 02:28 PM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/2795 That patch looks suspiciously short given the struggles I saw in http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html :-) Instead of setting to IPAPWD_END_OF_TIME, should we instead avoid filling "krbPasswordExpiration" attribute at all, i.e. have password *without* expiration? Or is krbPasswordExpiration mandatory? So I looked at the MIT code, and it seem like they are coping just fine with a missing (ie value = 0 internally) pw_expiration attribute. So if we make our code cope with omitting any expiration if the attribute is missing then yes, we can mark no expiration with simply removing (or not setting) the krbPasswordExpiration attribute. The attribute itself is optional and can be omitted. I think this is a good idea, and is definitely better than inventing a a magic value. Simo. Just a note: I tested David's patch and it actually doesn't work when the new password policy for ipausers group is created (priority = 0, which should be the highest priority). The maxlife and minlife values are empty. Even if I set the new password policy maxlife and minlife to 0 the result was that password will expire in 90 days. The patch worked correctly when I changed value of maxlife and minlife to 0 in 'global_policy'. Then the password expiration was set to 2038-01-01. -- 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] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).
On Wed, 2016-05-04 at 15:39 +0200, Martin Kosek wrote: > On 05/02/2016 02:28 PM, David Kupka wrote: > > https://fedorahosted.org/freeipa/ticket/2795 > > That patch looks suspiciously short given the struggles I saw in > http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html > :-) > > Instead of setting to IPAPWD_END_OF_TIME, should we instead avoid filling > "krbPasswordExpiration" attribute at all, i.e. have password *without* > expiration? Or is krbPasswordExpiration mandatory? So I looked at the MIT code, and it seem like they are coping just fine with a missing (ie value = 0 internally) pw_expiration attribute. So if we make our code cope with omitting any expiration if the attribute is missing then yes, we can mark no expiration with simply removing (or not setting) the krbPasswordExpiration attribute. The attribute itself is optional and can be omitted. I think this is a good idea, and is definitely better than inventing a a magic value. Simo. -- 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] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).
On 05/02/2016 02:28 PM, David Kupka wrote: > https://fedorahosted.org/freeipa/ticket/2795 That patch looks suspiciously short given the struggles I saw in http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html :-) Instead of setting to IPAPWD_END_OF_TIME, should we instead avoid filling "krbPasswordExpiration" attribute at all, i.e. have password *without* expiration? Or is krbPasswordExpiration mandatory? -- 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