On Fri, Feb 27, 2015 at 03:17:41PM +0100, Pavel Reichl wrote:
On 02/27/2015 12:01 PM, Sumit Bose wrote:
>On Tue, Feb 24, 2015 at 04:44:28PM +0100, Pavel Reichl wrote:
>>On 02/23/2015 11:47 AM, Sumit Bose wrote:
>>>On Fri, Feb 20, 2015 at 02:44:45PM +0100, Pavel Reichl wrote:
>>>>On 02/20/2015 02:33 PM, Lukas Slebodnik wrote:
>>>>>On (20/02/15 14:23), Pavel Reichl wrote:
>>>>>>On 02/19/2015 06:16 PM, Sumit Bose wrote:
>>>>>>>On Tue, Feb 17, 2015 at 10:56:43PM +0100, Pavel Reichl
wrote:
>>>>>>>>Hello,
>>>>>>>>
>>>>>>>>please see attached patches resolving
>...
>
>>>>index
9f2e9ac34add13e40d316374094024afdcc4ae31..4e3f3510250b19b5f397125fa3e3a376e0d3701f 100644
>>>>--- a/src/man/sssd-ldap.5.xml
>>>>+++ b/src/man/sssd-ldap.5.xml
>>>>@@ -1959,6 +1959,18 @@ ldap_access_filter = (employeeType=admin)
>>>> ldap_account_expire_policy
>>>> </para>
>>>> <para>
>>>>+
<emphasis>pwd_expire_policy</emphasis>:
>>>>+ This option is useful if users are
interested in
>>>>+ seeing password expiration warning when
authenticating
>>>>+ using different method then passwords - for
example
>>>>+ SSH keys.
>>>It's not about seeing a warning but about denying access based on an
>>>expired password. I think you should be more clear here.
>I'm sorry I wasn'T clear either. I meant to say 'it's not only about
a
>warning'. So you should mention both, the user will see a warning if the
>password is about to expire and will be rejected if the password is
>expired as with password authentication
>
>>>>+ </para>
>>>>+ <para>
>>>>+ Please note that 'access_provider =
ldap' must
>>>>+ be set for this feature to work. Also
'ldap_pwd_policy'
>>>>+ must be set to appropriate password policy.
>>>>+ </para>
>...
>
>>>>+ return ret;
>>>I think you have to modify the return code here to match the access
>>>control expectations. check_pwexpire_policy() will e.g. return
>>>ERR_PASSWORD_EXPIRED but the access control code expects
>>>ERR_ACCESS_DENIED. As a result I see the following in the logs:
>>In attached patch I modified sdap_access_done() instead:
>>
>>static void sdap_access_done(struct tevent_req *req)
>> case ERR_ACCOUNT_EXPIRED:
>> pam_status = PAM_ACCT_EXPIRED;
>> break;
>>+ case ERR_PASSWORD_EXPIRED:
>>+ pam_status = PAM_PERM_DENIED;
>>+ break;
>> default:
>> DEBUG(SSSDBG_CRIT_FAILURE, "Error retrieving access check
>>result.\n");
>> pam_status = PAM_SYSTEM_ERR;
>>
>>If you are OK with this approach would setting pam_status to
>>PAM_AUTHTOK_EXPIRED be more appropriate?
>That's a good point and it made me re-read to original tickets again. I
>was under the impression that they ask for rejecting access as well, but
>it looks thy only ask for a warning. Nevertheless I think it is a good
>idea to reject access and it relates a bit to #2534 as well.
>Nevertheless even if we reject your comment above made me think who we
>should react is the password is expired, just reject or ask for a new
>password as it would have been with password authentication.
>
>As a result I wonder if it would make sense to add 3 options:
>
>- pwd_expire_policy_reject: will warn and reject,
> pam_status = PAM_PERM_DENIED
>- pwd_expire_policy_warn: will only warn,
> pam_status = PAM_SUCCESS
>- pwd_expire_policy_renew: will warn and ask for new password
> pam_status = PAM_NEW_AUTHTOK_REQD
>
>What do you think?
>
>bye,
>Sumit
I think you are right, I just wonder if it would be better to create one
option
pwd_expire_policy_action which would have a value from {reject, warn, renew}
Default value would be reject?
But this would mean to add a new option for a quite limited use-case.
Jakub, do you have a preference?
bye,
Sumit