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