Re: Expiration/grace warnings bug in nslcd/myldap.c

[Arthur de Jong <arthur [at] arthurdejong.org>]

On Thu, 2015-07-09 at 09:19 +0200, Mathieu wrote:
> If a password expiration warning (pwdExpireWarning) is set in slapd, 
> and the password is indeed about to expire, slapd sends the correct 
> control back to the client.

Sorry for not replying sooner but I wanted to do a thorough review
because this is pretty critical code and I didn't want to open a
security hole.

Anyway, thanks for the patch. I've gone over it and merged it with some
modifications. I've also tried to find documentation for most of the
values and added some comments to document the meanings.

> @@ -582,6 +581,9 @@
>    {
>      handle_ppasswd_controls(session, ld, responsectrls);
>      ldap_controls_free(responsectrls);
> +    if ((session->policy_response == NSLCD_PAM_SUCCESS) ||
> +        (session->policy_response == NSLCD_PAM_NEW_AUTHTOK_REQD))
> +      rc = LDAP_SUCCESS;
>    }
>    /* return the result of the BIND operation */
>    if (rc != LDAP_SUCCESS)

The above will result in any BIND failures (e.g. Invalid credentials)
to be logged as a success and the user would be allowed to log in. It
could be argued that overwriting the BIND result could be a good idea
if the ppolicy control resulted in NEW_AUTHTOK_REQD but I'm not
confident this is always a good idea and could be dangerous.

The remaining changes mostly consist of making the expire
(timeBeforeExpiration) value and grace logins (graceAuthNsRemaining)
not force password change but to just present an informational message
to the user.

I've done some further testing but I haven't gotten the pwdReset
attribute working with nslcd yet. If the password is reset slapd will
allow the BIND but refuse the search (which is part of our
authentication test) and nslcd will consider the authentication failed.

There was some discussion on not performing a search after BIND (or
making it configurable) and I'm more than willing to review and merge
patches to that effect but disabling the search is not yet possible.

It does seem that slapd returns PP_passwordExpired together with
LDAP_INVALID_CREDENTIALS. This means that the return value of
NEW_AUTHTOK_REQD is ignored in that case. The value is however logged
for informational purposes.

Thanks for your patch!

-- 
-- arthur - arthur@arthurdejong.org - http://arthurdejong.org/ --

-- 
To unsubscribe send an email to
nss-pam-ldapd-users-unsubscribe@lists.arthurdejong.org or see
http://lists.arthurdejong.org/nss-pam-ldapd-users/