Re: PATCH #40075 - using ldap groups that contain DNs and usernames for AuthZ
On Mon, Dec 4, 2006 at 1:00 PM, in message
[EMAIL PROTECTED], Johanna Bromberg Craig
[EMAIL PROTECTED] wrote:
Hi,
I've addressed the feedback I received on my patch from Brad Nicholes
as follows:
I've reviewed all instances of util_ldap_compare() and
util_ldap_cache_comparedn() to confirm that each is protected from
cases where req- dn might be NULL or '\0'.
I've addressed the differences between AuthLDAPGroupAttributeDN,
AuthLDAPGroupAttribute, and AuthzLDAPRequireDN.
Thanks,
Johanna
I finally got some time to take a closer look at the patch. Although I like
the concept, I am still uncomfortable with the implementation from a
configuration point of view. I have attached a patch which is actually closer
to your first patch except it maintains the original functionality while
enhancing the AuthLDAPGroupAttribute directive to support attributes that may
contain a full DN. Actually, I think that was the original intent of
AuthLDAPGroupAttributeIsDN but it appears to have been broken along the way.
Anyway the proposed new syntax for AuthLDAPGroupAttribute is:
AuthLDAPGroupAttribute attribute [DN | UN] ...
where the keywords DN (Distinguished Name) and UN (User Name) can
optionally follow each attribute in the list. If neither of the keywords are
specified, then the attribute type follows the AuthLDAPGroupAttributeIsDN
setting. The AuthLDAPGroupAttributeIsDN setting determines if a DN is required
in the group comparison or not. If the AuthLDAPGroupAttribute list contains
any UN's, then AuthLDAPGroupAttributeIsDN must be set to OFF otherwise the
authorization will fail since it would be expecting to be able to resolve the
user object to a DN within the LDAP directory.
Let me know if this works for you,
BTW, this patch is against trunk rather than the 2.2.x branch.
Brad
Index: mod_authnz_ldap.c
===
--- mod_authnz_ldap.c (revision 489925)
+++ mod_authnz_ldap.c (working copy)
@@ -84,6 +84,7 @@
struct mod_auth_ldap_groupattr_entry_t {
char *name;
+char *type;
};
module AP_MODULE_DECLARE_DATA authnz_ldap_module;
@@ -647,8 +648,10 @@
#endif
grp = apr_array_push(sec-groupattr);
grp-name = member;
+grp-type = NULL;
grp = apr_array_push(sec-groupattr);
grp-name = uniquemember;
+grp-type = NULL;
#if APR_HAS_THREADS
apr_thread_mutex_unlock(sec-lock);
#endif
@@ -682,7 +685,6 @@
if(result != LDAP_SUCCESS) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
auth_ldap authorise: User DN not found, %s, ldc-reason);
-return AUTHZ_DENIED;
}
req = (authn_ldap_request_t *)apr_pcalloc(r-pool,
@@ -719,13 +721,30 @@
getpid(), t);
for (i = 0; i sec-groupattr-nelts; i++) {
-ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
- [% APR_PID_T_FMT ] auth_ldap authorize: require
group:
- testing for %s: %s (%s), getpid(),
- ent[i].name, sec-group_attrib_is_dn ? req-dn :
req-user, t);
+result = 0;
-result = util_ldap_cache_compare(r, ldc, sec-url, t, ent[i].name,
- sec-group_attrib_is_dn ? req-dn : req-user);
+if (ent[i].type == NULL) {
+ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+ [% APR_PID_T_FMT ] auth_ldap authorize: require
group:
+ testing for %s: %s (%s), getpid(),
+ ent[i].name, sec-group_attrib_is_dn ? req-dn :
req-user, t);
+
+result = util_ldap_cache_compare(r, ldc, sec-url, t, ent[i].name,
+ sec-group_attrib_is_dn ? req-dn
: req-user);
+} else if (req-dn != NULL strlen(req-dn) != 0
strcasecmp(ent[i].type, dn) == 0) {
+ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+ [% APR_PID_T_FMT ] auth_ldap authorise: require
group:
+ testing for %s: %s (%s), getpid(),
+ ent[i].name, req-dn, t);
+result = util_ldap_cache_compare(r, ldc, sec-url, t, ent[i].name,
req-dn);
+} else if (req-user != NULL strlen(req-user) != 0
strcasecmp(ent[i].type, un) == 0) {
+ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+ [% APR_PID_T_FMT ] auth_ldap authorise: require
group:
+ testing for %s: %s (%s), getpid(),
+ ent[i].name, req-user, t);
+result = util_ldap_cache_compare(r, ldc, sec-url, t, ent[i].name,
req-user);
+}
+
switch(result) {
case LDAP_COMPARE_TRUE: {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
@@ -1252,15 +1271,29 @@
static const char *mod_auth_ldap_add_group_attribute(cmd_parms *cmd, void
*config, const char *arg)
{
PATCH #40075 - using ldap groups that contain DNs and usernames for AuthZ
Hey, I know that Brad said he's been slammed at day job work and as such has not been able to find time to review my most recent changes to my patch, but I'm just sending out my weekly nudge as I'm also under pressure at work to get this done. :) Thanks, Johanna
Re: PATCH #40075 - using ldap groups that contain DNs and usernames for AuthZ
On 12/18/2006 08:27 PM, Johanna Bromberg Craig wrote: Hey, I know that Brad said he's been slammed at day job work and as such has not been able to find time to review my most recent changes to my patch, but I'm just sending out my weekly nudge as I'm also under I had a look at the patch, but I currently cannot help as I am not that in depth in the ldap code as Brad is. Sorry for that. But you are showing the perfect virtues for submitting a patch (the three P's): Patient, Polite, Persistent :-) So keep on buging us. I am pretty sure that you will have success. Regards Rüdiger
PATCH #40075 - using ldap groups that contain DNs and usernames for AuthZ
Hey, I've addressed the last rounds of comments to my patch to mod_authnz_ldap. I haven't heard anything for a week, so I'm wondering, can someone please review these changes? Thanks, Johanna
Re: PATCH #40075 - using ldap groups that contain DNs and usernames for AuthZ
On 12/11/2006 at 12:36 PM, in message [EMAIL PROTECTED], Johanna Bromberg Craig [EMAIL PROTECTED] wrote: Hey, I've addressed the last rounds of comments to my patch to mod_authnz_ldap. I haven't heard anything for a week, so I'm wondering, can someone please review these changes? Thanks, Johann Johanna, Sorry I haven't been able to get back to this quickly. I have been swamped with my day job lately. I will try to find some time to review the patch and hopefully have something to commit soon. Brad
PATCH #40075 - using ldap groups that contain DNs and usernames for AuthZ
Hi, I've addressed the feedback I received on my patch from Brad Nicholes as follows: I've reviewed all instances of util_ldap_compare() and util_ldap_cache_comparedn() to confirm that each is protected from cases where req-dn might be NULL or '\0'. I've addressed the differences between AuthLDAPGroupAttributeDN, AuthLDAPGroupAttribute, and AuthzLDAPRequireDN. Thanks, Johanna
PATCH #40075 - using ldap groups that contain DNs and usernames for AuthZ
Hi, I've addressed the feedback I received on my patch from Brad Nicholes as follows: I've restored AuthLDAPGroupAttribute to its former syntax and added a new directive, AuthLDAPGroupAttributeDN, whose attribute type is taken to be dn regardless of the value of AuthLDAPGroupAttributeIsDN. AuthLDAPGroupAttributeDN uses the same syntax as AuthLDAPGroupAttribute for the sake of clarity. Thanks, Johanna
Re: PATCH #40075 - using ldap groups that contain DNs and usernames for AuthZ
On 11/7/2006 at 1:07 PM, in message [EMAIL PROTECTED], Johanna Bromberg Craig [EMAIL PROTECTED] wrote: Hi, I've addressed the feedback I received on my patch from Brad Nicholes as follows: I've restored AuthLDAPGroupAttribute to its former syntax and added a new directive, AuthLDAPGroupAttributeDN, whose attribute type is taken to be dn regardless of the value of AuthLDAPGroupAttributeIsDN. AuthLDAPGroupAttributeDN uses the same syntax as AuthLDAPGroupAttribute for the sake of clarity. Thanks, Johann Hopefully I can get some time here soon to take a closer look at it. Brad
PATCH #40075 - using ldap groups that contain DNs and usernames for AuthZ
I'm a web developer at the University of Michigan and one of the authors of cosign ( http://weblogin.org ) which we use for (web) authentication. As to authorization, we've been tweaking and tuning mod_authz_ldap ( http://authzldap.othello.ch/ ) but we're really not happy with the code base. We're currently still running Apache 1, but the new mod_authnz_ldap has given us a very good reason to migrate to Apache 2.2 this summer. The only trouble is, we need to extend the functionality a bit. We have identities at Michigan that don't have their own entries in our LDAP directory, but they do appear in groups. This brings up a few issues. 1) We'd like some way to say if we can't find a DN for this identity, that's OK. 2) Since some of our users are in the directory ( have a person entry ) and some are not, AuthLDAPGroupAttributeisDN is not rich enough for us. Many of our groups contain both DNs and usernames. We'd like to extend AuthLDAPGroupAttribute to say whether the attribute in question is a DN or username, and thus be able to authorize both DNs and usernames for the same resource. I've proposed a patch to mod_authnz_ldap that adds: a) A new directive - AuthzLDAPRequireDN On | Off. On is the behavior we're looking for in issue #1 above, Off is the current default behavior, and this defaults to Off. b) A second argument to AuthLDAPGroupAttribute - a second argument, dn, allows us the finer grain control we're looking for in issue #2 above. If the dn option is given, the attribute ( member, say ) must be a DN. If this type is not set, the global AuthLDAPGroupAttributeisDN is obeyed. ie it works as before. Both of these changes are meant to be fully backward compatible with the behavior described in the existing documentation so no server admin should experience a surprise change upon an upgrade if this patch were accepted. I expect this functionality to be useful to any site that splits their authN/authZ. In particular, any site that uses WebSSO ( Cosign, CAS, PubCookie, etc. ) for authN but LDAP for authZ. Thanks. -J
Re: PATCH #40075 - using ldap groups that contain DNs and usernames for AuthZ
b) A second argument to AuthLDAPGroupAttribute - a second argument,dn, allows us the finer grain control we're looking for in issue #2 above. Ifthe dn option is given, the attribute ( member, say ) mustbe a DN. If this type is not set, the globalAuthLDAPGroupAttributeisDN is obeyed. ie it works as before. If you are referring to dynamic group support, I started a patch to provide this. It resides at http://issues.apache.org/bugzilla/show_bug.cgi?id=38515 . That should give you a start. I could use some help developing the patch, though.Gregory Szorc[EMAIL PROTECTED]
