On Fri, 2015-06-05 at 10:37 -0400, Drew Erny wrote: > On 06/04/2015 05:41 PM, Alexander Bokovoy wrote: > > On Thu, 04 Jun 2015, Drew Erny wrote: > >> https://fedorahosted.org/freeipa/ticket/2795 > >> > >> I've tracked down the source of this bug; it's nutty C stuff. > >> > >> So, in daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c, when you > >> update password, the expiration time appears to be set in the > >> function ipapwd_CheckPolicy on line 631, which looks like > >> > >> data->expireTime = data->timeNow + pol.max_pwd_life; > >> > >> So the bug has to be in how pol.max_pwd_life gets is value. So I > >> check around, pol is initialized like this: > >> > >> struct ipapwd_policy pol = {0}; > >> ... > >> pol.max_pwd_life = IPAPWD_DEFAULT_PWDLIFE; > >> > >> And IPAPWD_DEFAULT_PWDLIFE is a constant 90 days. > >> > >> But then the actual value of max_pwd_life is obtained by passing pol > >> into the function ipapwd_getPolicy on line 577 or 590, depending on > >> the password change type. > >> > >> Inside of ipapwd_getPolicy, there's a couple of lines starting at > >> line 393 > >> > >> tmpint = slapi_entry_attr_get_int(pe, "krbMaxPwdLife"); > >> if (tmpint != 0) { > >> policy->max_pwd_life = tmpint; > >> }: > >> > >> Which sets the max password life to the returned value, unless this > >> function returns 0. However, the documentation from > >> /usr/include/dirsrv/slapi-plugin.h says that that function, > >> slapi_entry_attr_get_int, returns 0 if the entry does not contain > >> that attribute. So, since the value 0 is returned, an error is > >> assumed to have occurred that member of the struct is left > >> untouched... which means it's still set to the value it was set to > >> when it was initialized, 90 days. > >> > >> So, when the expireTime is set at line 631, it's set to 90 days > >> because the value returned by slapi_entry_attr_get_int is 0. > >> > >> I've checked to see if we can get some error context out of the pe > >> variable passed in, but it appears to be an opaque struct that the > >> user isn't meant to see the internals of. > >> > >> I'm not really sure what to do with this knowledge. The only thing I > >> can think would be to use another sentinel value, like -1, to > >> indicate that the password does not expire; or, otherwise, to > >> document that there is no way to have non-expiring passwords, and > >> administrators can only set value to some far-future date, and then > >> close this bug. Or, we could just set the default expiration date to > >> be somewhere far in the future. I'm not really qualified to make a > >> call on how to proceed with this, but I'm capable of making the > >> change if someone more senior decides. > >> > >> I can also totally see this issue with the interface of slapi-plugin > >> being the possible cause of many bugs. > > You can use slapi_entry_attr_exists() to check if attribute does exist > > and then treat result of slapi_entry_attr_get_int() as actual value. > > > > Otherwise, that's a great investigation! > > Using slapi_entry_attr_exists() clears us of having to worry about > getting an error condition back, but I'm still not confident how to > handle the 0 maximum. Should I just put in a far-future date?
The current behavior is completely intentional, not a side effect of the code, the code was written that way intentionally. However me may consider an RFE that requests different behavior, we would have to devise a special value for krbMaxPwdLife that means "infinite". Simo. -- Simo Sorce * Red Hat, Inc * New York -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code