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

Jakub Hrozek jhrozek at redhat.com
Thu Aug 7 09:00:19 UTC 2014


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?



More information about the sssd-devel mailing list