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

Pavel Březina pbrezina at redhat.com
Mon Aug 4 12:37:01 UTC 2014


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?

Also the ticket says that this policy is only enabled if pwdLockout is 
true. Is it possible to acquire the attribute if pwdLockout is false? I 
see you don't check it.

Those are questions that should be answered in the code if they are not 
bugs.

I haven't test the code yet - will it work also with 389ds or do you 
have an OpenLDAP server I could use handy?




More information about the sssd-devel mailing list