[SSSD] [PATCHES] SDAP: Be able to configure sssd to honor openldap account lock to restrict access via ssh key

Pavel Reichl preichl at redhat.com
Thu Aug 7 09:21:50 UTC 2014


On 08/07/2014 11:17 AM, Jakub Hrozek wrote:
> On Thu, Aug 07, 2014 at 11:13:34AM +0200, Pavel Reichl wrote:
>> On 08/07/2014 11:00 AM, Jakub Hrozek wrote:
>>> On Thu, Aug 07, 2014 at 10:52:37AM +0200, Pavel Reichl wrote:
>>>> On 08/07/2014 10:45 AM, Pavel Březina wrote:
>>>>> On 08/06/2014 06:36 PM, Pavel Reichl wrote:
>>>>>> On 08/04/2014 02:37 PM, Pavel Březina wrote:
>>>>>>> On 08/01/2014 07:48 PM, Pavel Reichl wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> please see attached patches. Every patch is documented in its commit
>>>>>>>> message.
>>>>>>>>
>>>>>>>> Generally speaking the first 6 patches are preparation for patch #8.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Pavel Reichl
>>>>>>>>
>>>>>>>> PS: I also attached output of my testing to make it more obvious how
>>>>>>>> the
>>>>>>>> patches are supposed to work.
>>>>>>>>
>>>>>>>> ------------------------------------------------------------------------
>>>>>>>>
>>>>>>>> # john, people, example.com
>>>>>>>> dn: uid=john,ou=people,dc=example,dc=com
>>>>>>>> pwdAccountLockedTime: 000001010000Z
>>>>>>>>
>>>>>>>> # max, people, example.com
>>>>>>>> dn: uid=max,ou=people,dc=example,dc=com
>>>>>>>>
>>>>>>>> # dick, people, example.com
>>>>>>>> dn: uid=dick,ou=people,dc=example,dc=com
>>>>>>>> pwdAccountLockedTime: 20140801115742Z
>>>>>>>> --------------------------------------------------------------------------
>>>>>>>>
>>>>>>>>
>>>>>>>> $ ssh -l john at openldap `hostname`
>>>>>>>> Connection closed by UNKNOWN
>>>>>>>>
>>>>>>>> $ ssh -l max at openldap `hostname`
>>>>>>>> Last login: Fri Aug  1 15:16:21 2014 from sssd.dev.work
>>>>>>>>
>>>>>>>> $ ssh -l dick at openldap `hostname`
>>>>>>>> Last login: Fri Aug  1 12:57:33 2014
>>>>>>> Patch #1:
>>>>>>> Tentative ack, although you don't have to use tmp_ctx in
>>>>>>> sdap_save_user_cache_bool(). You can just allocate attrs on NULL since
>>>>>>> it is the only talloc context you use there.
>>>>>>>
>>>>>>> Patch #2:
>>>>>>> Nack. I see it is safe here but please avoid initializing ret to EOK.
>>>>>>> Set it to EOK at the end of the function instead.
>>>>>>>
>>>>>>> Patch #3:
>>>>>>> Ack.
>>>>>>>
>>>>>>> Patch #4:
>>>>>>> Typo in commit message "Also remove *sdap_access_filter_recv()* as it
>>>>>>> is replaced by *sdap_access_filter_recv()*."
>>>>>>>
>>>>>>> But it's nack from me anyway.
>>>>>>>
>>>>>>> Tevent naming convention seems to be broken all around this code and
>>>>>>> this patch actually makes it a little bit better. But you shouldn't
>>>>>>> use function named "sdap_access_recv" inside "sdap_access_done".
>>>>>>>
>>>>>>> I see that sdap_access_send can invoke two different requests
>>>>>>> depending on the configuration. Ideally both should have custom _recv
>>>>>>> and you should use some kind of switch to determine which one should
>>>>>>> be called.
>>>>>>>
>>>>>>> I would be willing to tentatively ack a patch with let's say
>>>>>>> "sdap_access_check_recv" or "sdap_access_subreq_recv" or similar used
>>>>>>> in sdap_access_done.
>>>>>>>
>>>>>>> Patch #5:
>>>>>>> Ack.
>>>>>>>
>>>>>>> Patch #6:
>>>>>>> Ack.
>>>>>>>
>>>>>>> Patch #7:
>>>>>>> Ack. But please swap this ticket with #8. The feature should be first
>>>>>>> implemented then documented.
>>>>>>>
>>>>>>> Patch #8:
>>>>>>> I believe the following comment is copy & paste? It shouldn't say
>>>>>>> filter request :)
>>>>>>> +    /* Connection to LDAP succeeded
>>>>>>> +     * Send filter request
>>>>>>> +     */
>>>>>>>
>>>>>>> +            if (strcasecmp(pwdAccountLockedTime,
>>>>>>> +                           PERMANENTLY_LOCKED_ACCOUNT) == 0) {
>>>>>>> +                locked = true;
>>>>>>> +            } else {
>>>>>>> +                DEBUG(SSSDBG_TRACE_FUNC,
>>>>>>> +                      "Account of: %s is beeing blocked by password
>>>>>>> policy, "
>>>>>>> +                      "but value: [%s] value is ignored by SSSD.\n",
>>>>>>> +                      state->username, pwdAccountLockedTime);
>>>>>>> +            }
>>>>>>>
>>>>>>> Why do we ignore the value? Isn't the value a time? Shouldn't we check
>>>>>>> if lock_time < machine_time?
>>>>>> Yes, it's time, but we care only if its the special value
>>>>>> '000001010000Z' otherwise we grant the access.
>>>>> Why?
>>>> Because that's how it's described in BZ
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1099290 - or do you understand
>>>> it differently than me?
>>> Can you please check with the openldap maintainer if that value indeed
>>> has some special meaning?
>> Sure, but I'm not sure what should I be asking exactly?
>>
>> This is from man slapo-ppolicy:
>>
>>         pwdAccountLockedTime
>>
>>         This attribute contains the time that the user's account was locked.  If the account has been locked, the password may no longer be used to authenticate the user to the  directory.   If  pwdAccount‐
>>
>>         LockedTime is set to 000001010000Z, the user's account has been permanently locked and may only be unlocked by an administrator. Note that account locking only takes effect when the pwdLockout pass‐
>>
>>         word policy attribute is set to "TRUE".
>>
> Then you're right :-) I haven't read the man page TBH. This would be a
> nice addition to the commit message, too!
OK, I'll do that, pbrezina had similar request. I'll just wait for more 
comments to address them all at once.
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel




More information about the sssd-devel mailing list