[Freeipa-devel] Password Maxlife 0 causes expiration of 90 days

[Simo Sorce]

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