[SSSD] [PATCH] warn user if password is about to expire

Pavel Březina pbrezina at redhat.com
Thu Nov 29 11:55:27 UTC 2012


On 11/28/2012 05:39 PM, Sumit Bose wrote:
> On Wed, Nov 28, 2012 at 04:54:45PM +0100, Pavel Březina wrote:
>> On 11/28/2012 11:15 AM, Sumit Bose wrote:
>>> On Tue, Nov 20, 2012 at 11:18:22AM +0100, Pavel Březina wrote:
>>>> On 11/19/2012 04:59 PM, Pavel Březina wrote:
>>>>> https://fedorahosted.org/sssd/ticket/1638
>>>>
>>>> Self nack.
>>>>
>>>>> -            if (pwd_exp_warning == 0 ||
>>>>> -                difftime(now + pwd_exp_warning, ppolicy->expire) > 0.0) {
>>>>> +            if (pwd_exp_warning != 0 &&
>>>>> +                difftime(pwd_exp_warning, ppolicy->expire) > 0.0) {
>>>>>                  goto done;
>>>>>              }
>>>>
>>>> We don't need to test if pwd_exp_warning != 0, because if it is 0 then
>>>> difftime returns value <= 0 and the condition fails.
>>>>
>>>> New patch is attached.
>>>>
>>>
>>>>  From 3bb970535dd1d12016ed8ba431039b843be9da7c Mon Sep 17 00:00:00 2001
>>>> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
>>>> Date: Mon, 19 Nov 2012 16:52:36 +0100
>>>> Subject: [PATCH] warn user if password is about to expire
>>>>
>>>> https://fedorahosted.org/sssd/ticket/1638
>>>>
>>>> If pwd_exp_warning == 0, expiry warning should be printed if it is
>>>> returned by server.
>>>>
>>>> If pwd_exp_warning > 0, expiry warning should be printed only if
>>>> the password will expire in time <= pwd_exp_warning.
>>>>
>>>> ppolicy->expiry contains period in seconds after which the password
>>>> expires. Not the exact timestamp. Thus we should not add 'now' to
>>>> pwd_exp_warning.
>>>> ---
>>>>   src/providers/ldap/ldap_auth.c | 4 +---
>>>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c
>>>> index 32a2e04ea959a3cc81b88f5b2b19575c813e8adf..04fd24ffa4708dce53fb937e92efbbc102bef97c 100644
>>>> --- a/src/providers/ldap/ldap_auth.c
>>>> +++ b/src/providers/ldap/ldap_auth.c
>>>> @@ -212,7 +212,6 @@ static errno_t check_pwexpire_ldap(struct pam_data *pd,
>>>>       if (ppolicy->grace > 0 || ppolicy->expire > 0) {
>>>>           uint32_t *data;
>>>>           uint32_t *ptr;
>>>> -        time_t now = time(NULL);
>>>>           int ret;
>>>>
>>>>           if (pwd_exp_warning < 0) {
>>>> @@ -231,8 +230,7 @@ static errno_t check_pwexpire_ldap(struct pam_data *pd,
>>>>               ptr++;
>>>>               *ptr = ppolicy->grace;
>>>>           } else if (ppolicy->expire > 0) {
>>>> -            if (pwd_exp_warning == 0 ||
>>>> -                difftime(now + pwd_exp_warning, ppolicy->expire) > 0.0) {
>>>> +            if (difftime(pwd_exp_warning, ppolicy->expire) > 0.0) {
>>>>                   goto done;
>>>>               }
>>>>               *ptr = SSS_PAM_USER_INFO_EXPIRE_WARN;
>>>
>>> I haven't tested the patch yet, but I would recommend to drop difftime()
>>> because pwd_exp_warning and ppolicy->expire are integers and just using
>>> '<' or '>' would be easier and better to read.
>>>
>>> That said I think the current logic is flawed. As you said of
>>> pwd_exp_warning is 0 difftime returns value <= 0 and the expire time is
>>> added to the response. But if pwd_exp_warning is positive
>>> ppolicy->expire must be larger than pwd_exp_warning to be added to the
>>> response. This means the warning is only printed if it is still longer
>>> than the time given in pwd_expiration_warning.
>>>
>>> bye,
>>> Sumit
>>
>>
>> You are correct sir. Thank you. Looks like I forgot to test the option.
>>
>> I'm sending a new patch. I don't have time to test it myself right now,
>> but I don't want to delay 1.9.3 if with any luck it is going to be
>> released today.
>>
>> I'll try it tomorrow if it's still pending, though I'm quite
>> confident it will work.
>>
>>
>
> Patch looks good, applies to master and compiles without warning. I
> didn't had a chance to test it either so I'll give a tentative ACK to
> not delay 1.9.3. I'll try to do some testing later today.
>
> bye,
> Sumit

Thanks,
I just tried it and it works as expected.




More information about the sssd-devel mailing list