<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 05:55 PM, Pavel Reichl
wrote:<br>
</div>
<blockquote cite="mid:53F37373.1010301@redhat.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<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 moz-do-not-send="true"
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 moz-do-not-send="true"
class="moz-txt-link-abbreviated"
href="mailto:sssd-devel@lists.fedorahosted.org">sssd-devel@lists.fedorahosted.org</a>
<br>
<a moz-do-not-send="true"
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 moz-do-not-send="true"
class="moz-txt-link-abbreviated"
href="mailto:sssd-devel@lists.fedorahosted.org">sssd-devel@lists.fedorahosted.org</a>
<br>
<a moz-do-not-send="true"
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 moz-do-not-send="true"
class="moz-txt-link-abbreviated"
href="mailto:sssd-devel@lists.fedorahosted.org">sssd-devel@lists.fedorahosted.org</a>
<br>
<a moz-do-not-send="true"
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 moz-do-not-send="true"
class="moz-txt-link-abbreviated"
href="mailto:sssd-devel@lists.fedorahosted.org">sssd-devel@lists.fedorahosted.org</a>
<br>
<a moz-do-not-send="true" 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>
</blockquote>
<br>
Thanks attached patches include this fix.<br>
<br>
</blockquote>
May we push these patches to the master?<br>
<blockquote cite="mid:53F37373.1010301@redhat.com" type="cite">
<blockquote cite="mid:53F35FC4.9040900@redhat.com" type="cite"> <br>
<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
sssd-devel mailing list
<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:sssd-devel@lists.fedorahosted.org">sssd-devel@lists.fedorahosted.org</a>
<a moz-do-not-send="true" 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>
<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>
<br>
</body>
</html>