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

[Alexander Bokovoy]

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!
-- 
/ Alexander Bokovoy