[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
Tue Aug 19 15:43:10 UTC 2014


On 08/19/2014 04:31 PM, Pavel Březina wrote:
> On 08/19/2014 02:05 PM, Pavel Reichl wrote:
>>
>> On 08/19/2014 12:43 PM, Pavel Březina wrote:
>>> On 08/19/2014 10:37 AM, Pavel Reichl wrote:
>>>>
>>>> On 08/18/2014 04:39 PM, Pavel Březina wrote:
>>>>> On 08/18/2014 04:24 PM, Pavel Reichl wrote:
>>>>>>
>>>>>> Thank you Pavel for comments, I was mostly able to fix patches as 
>>>>>> you
>>>>>> asked for but I have a little problem with default_dn. Could you
>>>>>> please
>>>>>> see my comments inline?
>>>>>>
>>>>>>
>>>>>> On 08/18/2014 12:13 PM, Pavel Březina wrote:
>>>>>>> On 08/15/2014 03:13 PM, Pavel Reichl wrote:
>>>>>>>>
>>>>>>>> On 08/11/2014 04:51 PM, Jakub Hrozek wrote:
>>>>>>>>> On Mon, Aug 11, 2014 at 04:19:22PM +0200, Pavel Reichl wrote:
>>>>>>>>>>>> Subject: [PATCH 7/9] SDAP: new option - DN to ppolicy on LDAP
>>>>>>>>>>> Do we need to make the DN configurable? When you install the
>>>>>>>>>>> ppolicy,
>>>>>>>>>>> can you actually set the DN?
>>>>>>>>>> I haven't tested setting different DN, but I believe it's
>>>>>>>>>> possible.
>>>>>>>>>> During weekend I was putting my notes for setting ppolicy on
>>>>>>>>>> OpenLDAP on
>>>>>>>>>> this wiki page
>>>>>>>>>> https://fedorahosted.org/sssd/wiki/openldap_ppolicy#Loadingppolicyoverlay 
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> (this is work in progress and I'm not even sure we care to have
>>>>>>>>>> such a
>>>>>>>>>> how-to on our wiki), but I believe you can see from there that
>>>>>>>>>> DN is
>>>>>>>>>> configurable.
>>>>>>>>> Any howto is good as long as it's up to date.
>>>>>>>>>
>>>>>>>>> And from the howto it seems clear that olcPPolicyDefault is 
>>>>>>>>> the DN
>>>>>>>>> setting, so yes, this should be configurable.
>>>>>>>>>
>>>>>>>>>> Still we could use a default DN if user didn't set DN himself:
>>>>>>>>>>
>>>>>>>>>> "cn=ppolicy,ou=policies,$search_base"
>>>>>>>>> That sounds like a good idea.
>>>>>>>>> _______________________________________________
>>>>>>>>> sssd-devel mailing list
>>>>>>>>> sssd-devel at lists.fedorahosted.org
>>>>>>>>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> please note that attached patches requires to be patches from 
>>>>>>>> "SDAP:
>>>>>>>> refactor sdap_access" thread to pushed first.
>>>>>>>
>>>>>>> Patch 7:
>>>>>>> Nack.
>>>>>>>
>>>>>>> +    /* cached results of access control checks */
>>>>>>>                      ^ cached_access contain only one result
>>>>>>>      bool cached_access;
>>>>>>>      const char *basedn;
>>>>>>>   };
>>>>>>>
>>>>>>> Please remove sdap_access_{filter|lockout}_decide_offline and use
>>>>>>> only
>>>>>>> sdap_access_decide_offline that will take bool instead of 
>>>>>>> tevent_req
>>>>>>> as parameter.
>>>>>>>
>>>>>>> Patch 8:
>>>>>>> Ack.
>>>>>>>
>>>>>>> Patch 9:
>>>>>>>
>>>>>>> +        case LDAP_ACCESS_LOCKOUT:
>>>>>>> +            ret = EOK;
>>>>>>> +
>>>>>>> +            subreq = sdap_access_lock_send(state, state->ev,
>>>>>>> state->be_ctx,
>>>>>>> + state->domain,
>>>>>>> + state->access_ctx,
>>>>>>> + state->conn,
>>>>>>> + state->pd->user,
>>>>>>> + state->user_entry);
>>>>>>> +            if (subreq == NULL) {
>>>>>>> +                DEBUG(SSSDBG_CRIT_FAILURE, "sdap_access_lock_send
>>>>>>> failed.\n");
>>>>>>> +                return ENOMEM;
>>>>>>> +            }
>>>>>>> +
>>>>>>> +            state->ac_type = SDAP_ACCESS_CONTROL_PPOLICY_LOCK;
>>>>>>> +
>>>>>>> +            tevent_req_set_callback(subreq, sdap_access_done, 
>>>>>>> req);
>>>>>>> +            return EAGAIN;
>>>>>>> +            break;
>>>>>>> +
>>>>>>>
>>>>>>> There is no need for ret = EOK and break.
>>>>>>>
>>>>>>> +static char*
>>>>>>> +get_default_ppolicy_dn(TALLOC_CTX *mem_ctx, struct sdap_options
>>>>>>> *opts)
>>>>>>> +{
>>>>>>> +    char *search_base;
>>>>>>> +
>>>>>>> +    search_base = dp_opt_get_string(opts->basic, 
>>>>>>> SDAP_SEARCH_BASE);
>>>>>>> +
>>>>>>> +    return talloc_asprintf(mem_ctx, "cn=ppolicy,ou=policies,%s",
>>>>>>> search_base);
>>>>>>> +}
>>>>>>>
>>>>>>> ldap_search_base option may contain more than one search base 
>>>>>>> and it
>>>>>>> may also contain filter. This option is parsed during 
>>>>>>> initialization
>>>>>>> of the provider and stored in sdap_domain->*search_base. But since
>>>>>>> there may be more, you need to iterate over them.
>>>>>> Could you please elaborate a bit more about this? I'm afraid I don't
>>>>>> fully understand.
>>>>>>
>>>>>> Would you like me to create an array of possible default DN's and
>>>>>> query
>>>>>> ldap server for every one? Or is there a way how to use multiple dn
>>>>>> in a
>>>>>> single ldap query?
>>>>>
>>>>> Yes, you have to iterate over all of them. There is no way how to
>>>>> search all of them in one query AFAIK.
>>>>>
>>>>> However, I think it is sufficient to use only domain components to
>>>>> create default dn, i.e. basedn of domain (dc=example,dc=com). This 
>>>>> can
>>>>> be obtained from sdap_domain structure. Then you wouldn't have to
>>>>> iterate over all search bases.
>>>>>
>>>>>>>
>>>>>>> However, I think it is sufficient to use only dc part here so 
>>>>>>> you can
>>>>>>> use basedn from sdap_domain structure.
>>>>>>>
>>>>>>> Also, I think it would be better if this function is removed and 
>>>>>>> the
>>>>>>> default value is set in ldap_get_options() with 
>>>>>>> dp_opt_set_string if
>>>>>>> the option is not set.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> sssd-devel mailing list
>>>>>>> sssd-devel at lists.fedorahosted.org
>>>>>>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>>>>>
>>>>>> _______________________________________________
>>>>>> sssd-devel mailing list
>>>>>> sssd-devel at lists.fedorahosted.org
>>>>>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>>>>
>>>>> _______________________________________________
>>>>> sssd-devel mailing list
>>>>> sssd-devel at lists.fedorahosted.org
>>>>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>>>
>>>> Hello, please see updated patches.
>>>>
>>>> Thanks,
>>>>
>>>> Pavel Reichl
>>>
>>> Hi,
>>> I have just last few nitpicks.
>>>
>>> +    /* option was configured */
>>> +    if (ppolicy_dn != NULL) {
>>> +        state->ppolicy_dns = talloc_array(state, const char*, 2);
>>> +        if (state->ppolicy_dns == NULL) {
>>> +            DEBUG(SSSDBG_CRIT_FAILURE, "Could not allocate
>>> ppolicy_dns.\n");
>>> +            tevent_req_error(req, ENOMEM);
>>> +            return;
>>> +        }
>>> +
>>> +        state->ppolicy_dns[0] = ppolicy_dn;
>>> +        state->ppolicy_dns[1] = NULL;
>>> +
>>> +    } else {
>>> +        /* try to determine default value */
>>> +        DEBUG(SSSDBG_CONF_SETTINGS,
>>> +              "ldap_pwdlockout_dn was not defined in configuration
>>> file.\n");
>>> +
>>> +        state->ppolicy_dns = get_default_ppolicy_dns(state,
>>> state->opts->sdom);
>>> +        if (state->ppolicy_dns == NULL) {
>>> +            tevent_req_error(req, ERR_ACCESS_DENIED);
>>> +            return;
>>> +        }
>>> +    }
>>>
>>> Why do you return ENOMEM on the top and ERR_ACCESS_DENIED on the
>>> bottom? Shouldn't it be the same?
>>>
>>> errno_t sdap_access_lock_step(struct tevent_req *req)
>>>
>>> When we iterate over an array, the convention is that the step
>>> function will return EOK if we are at the end, EAGAIN if it created
>>> another tevent_req and error on error.
>>>
>>> Also, I would prefer to use an index to access state->ppolicy_dns
>>> instead of dereferencing it and using pointer arithmetic.
>>>
>>>
>>
>> OK, I hope that attached patches address all your concerns!
>
> Ack.
>
> I just squash into the 3rd patch the following change:
>
> -                                   *state->ppolicy_dns,
> + state->ppolicy_dns[state->ppolicy_dns_index],
>
>
>
>
>
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Rebased patches for 1.11 (includes Pavel's mini patch)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140819/a6650752/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-SDAP-split-sdap_access_filter_get_access_done.patch
Type: text/x-patch
Size: 3991 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140819/a6650752/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-SDAP-refactor-sdap_access_filter_send.patch
Type: text/x-patch
Size: 3704 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140819/a6650752/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-SDAP-nitpicks-in-sdap_access_filter_get_access_done.patch
Type: text/x-patch
Size: 2278 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140819/a6650752/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-SDAP-refactor-sdap_access_filter_done.patch
Type: text/x-patch
Size: 6511 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140819/a6650752/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-SDAP-don-t-log-error-on-access-denied.patch
Type: text/x-patch
Size: 1290 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140819/a6650752/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-SDAP-refactor-AC-offline-checks.patch
Type: text/x-patch
Size: 3280 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140819/a6650752/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-SDAP-new-option-DN-to-ppolicy-on-LDAP.patch
Type: text/x-patch
Size: 5510 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140819/a6650752/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-SDAP-account-lockout-to-restrict-access-via-ssh-key.patch
Type: text/x-patch
Size: 25166 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140819/a6650752/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0009-MAN-options-lockout-and-ldap_pwdlockout_dn.patch
Type: text/x-patch
Size: 2611 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140819/a6650752/attachment-0008.bin>


More information about the sssd-devel mailing list