Re: [PATCH] Added ability to look up operational attributes

William Orr Thu, 04 Apr 2013 19:56:09 -0700

I'm happy to answer!

Russ Allbery

April 4, 2013 9:40 PM

Thanks for the patch! I had a couple of questions about the details:

You create a new distinguished list of operational attributes of
interest, but the above code doesn't seem to ever use that list? Instead,
it looks like it just retrieves every operational attribute and puts them
all into the environment. Was that the problem that you're trying to
solve?

Look around line 301 of mod_webauthldap.c. I pop the values from the list, and only put the values that I'm looking for into envvars. That's the list that gets checked wrt dumping variables into the environment

A quick check with ldapsearch that I probably should have done earlier
seems to indicate that if one just adds a particular operational attribute
to the search filter, that works and returns that attribute, so I think
that, for the per-attribute code, nothing special has to be done to handle
operational attributes. But maybe you're currently using "all" and the
goal was to come up with a version of "all" that was inclusive of
operational attributes?

Yes, that is how ldapsearch works, however I modeled my code after yours, which seems to dump all of the attributes and then only set the ones we're interested in into the environment, as well as the default set.

Because I didn't want to modify the code too heavily, and because I didn't want to be responsible for making a list of default variables, I opted for this method.

You also do the search twice, but it looks like you can specify
operational attributes in the same search as regular attributes.

Yes, but not if you're dumping all of the ldap attributes, which is the way it currently operates in mod_webauthldap.

If the goal is to retrieve all operational attributes, I'm wondering if
would make sense to use some sort of keyword (like +) in the
WebAuthLdapAttribute directive to specify that all operational attributes
should be included.

I do, the constant LDAP_ALL_OPERATIONAL_ATTRIBUTES resolves to a +.

Basically, in sum, I implemented it this way because it changed as little of the original operation of the module as possible.

Thanks!

William Orr

April 4, 2013 10:33 AM

- operational attributes are specified with new conf directive,
WebAuthLdapOperationalAttribute
- same semantics as WebAuthLdapAttribute
- also placed into environment
---
docs/mod_webauthldap.xml | 31 +++++++++++++++++-
modules/ldap/config.c | 15 +++++++--
modules/ldap/mod_webauthldap.c | 71 +++++++++++++++++++++++++++++++++++++++++-
modules/ldap/mod_webauthldap.h | 2 ++
4 files changed, 115 insertions(+), 4 deletions(-)

diff --git a/docs/mod_webauthldap.xml b/docs/mod_webauthldap.xml
index 78c135c..9a680b8 100644
--- a/docs/mod_webauthldap.xml
+++ b/docs/mod_webauthldap.xml
@@ -202,7 +202,8 @@ override this behavior, see
<a href="">WebAuthLdapSeparator</a>.

The attributes can be any attribute found in your LDAP server that
-the server using this module has access to read.
+the server using this module has access to read, except for operational
+attributes, like entryUUID.

<example><title>Example</title>
<Location /private/> 
@@ -216,6 +217,34 @@ WebAuthLdapAttribute suUnivid 
</usage>
</directivesynopsis>

+<directivesynopsis>
+<name>WebAuthLdapOperationalAttribute</name>
+<description>LDAP operational attribute to place in the environment</description>
+<syntax>WebAuthLdapOperationalAttributeoper_attribute [oper_attribute] ...</syntax>
+<default>none</default>
+<contextlist>
+<context>directory</context>
+<context>.htaccess</context>
+</contextlist>
+
+<usage>
+All attributes defined by this directive will be looked up additionally
+and their values will be inserted into the environment. This directive can
+also be used multiple times.
+
+Like<a href="">WebAuthLdapAttribute</a>, the name
+of the enviornment variable is formed by prepending WEBAUTH_LDAP_ to the
+uppercased name. Multivalued attributes work exactly the same as well.
+
+<example><title>Example</title></example>
+<Location /private/> 
+AuthType WebAuth 
+Require privgroup stanford:staff 
+WebAuthLdapOperationalAttribute entryUUID 
+</Location> 
+</example>
+</usage>
+</directivesynopsis>

<directivesynopsis>
<name>WebAuthLdapAuthorizationAttribute</name>
diff --git a/modules/ldap/config.c b/modules/ldap/config.c
index e44e6e0..6e7fd5c 100644
--- a/modules/ldap/config.c
+++ b/modules/ldap/config.c
@@ -45,6 +45,7 @@ APLOG_USE_MODULE(webauthldap);
 static const type DF_ ## name = def;

DIRN(Attribute, "additional LDAP attributes to retrieve")
+DIRN(OperationalAttribute, "operational LDAP attributes to retrieve")
DIRN(AuthorizationAttribute, "LDAP attribute for privilege groups")
DIRD(Authrule, "whether to display the authorization rule",
 bool, true)
@@ -63,6 +64,7 @@ DIRN(TktCache, "Kerberos ticket cache for LDAP")

enum {
 E_Attribute,
+ E_OperationalAttribute,
 E_AuthorizationAttribute,
 E_Authrule,
 E_Base,
@@ -75,7 +77,7 @@ enum {
 E_Privgroup,
 E_Separator,
 E_SSL,
- E_TktCache
+ E_TktCache,
};

/*
@@ -199,6 +201,7 @@ mwl_dir_config_merge(apr_pool_t *pool, void *basev, void *overv)

 /* FIXME: Should probably remove duplicates. */
 MERGE_ARRAY(attribs);
+ MERGE_ARRAY(oper_attribs);
 MERGE_ARRAY(privgroups);
 return conf;
}
@@ -317,7 +320,7 @@ cfg_str(cmd_parms *cmd, void *mconf, const char *arg)
 struct server_config *sconf;
 struct dir_config *dconf = mconf;
 const char *err = NULL;
- const char **attrib, **privgroup;
+ const char **attrib, **privgroup, **oper_attrib;

 sconf = ap_get_module_config(cmd->server->module_config,
 &webauthldap_module);
@@ -358,6 +361,13 @@ cfg_str(cmd_parms *cmd, void *mconf, const char *arg)
 attrib = apr_array_push(dconf->attribs);
 *attrib = apr_pstrdup(cmd->pool, arg);
 break;
+ case E_OperationalAttribute:
+ if (dconf->oper_attribs == NULL)
+ dconf->oper_attribs
+ = apr_array_make(cmd->pool, 5, sizeof(const char *));
+ oper_attrib = apr_array_push(dconf->oper_attribs);
+ *oper_attrib = apr_pstrdup(cmd->pool, arg);
+ break;
 case E_Privgroup:
 if (dconf->privgroups == NULL)
 dconf->privgroups
@@ -467,6 +477,7 @@ const command_rec webauthldap_cmds[] = {
 DIRECTIVE(AP_INIT_TAKE1, cfg_str, RSRC_CONF, TktCache),

 DIRECTIVE(AP_INIT_ITERATE, cfg_str, OR_AUTHCFG, Attribute),
+ DIRECTIVE(AP_INIT_ITERATE, cfg_str, OR_AUTHCFG, OperationalAttribute),
 DIRECTIVE(AP_INIT_ITERATE, cfg_str, OR_AUTHCFG, Privgroup),

 { NULL, { NULL }, NULL, OR_NONE, RAW_ARGS, NULL }
diff --git a/modules/ldap/mod_webauthldap.c b/modules/ldap/mod_webauthldap.c
index 5701720..bcea718 100644
--- a/modules/ldap/mod_webauthldap.c
+++ b/modules/ldap/mod_webauthldap.c
@@ -263,7 +263,7 @@ webauthldap_init(MWAL_LDAP_CTXT* lc)
 int i;
 char** attrib;
 char *p, *privgroup;
- apr_array_header_t* attribs;
+ apr_array_header_t* attribs, *oper_attribs;

 if (lc->sconf->debug)
 ap_log_error(APLOG_MARK, APLOG_INFO, 0, lc->r->server, "%s %s",
@@ -298,6 +298,21 @@ webauthldap_init(MWAL_LDAP_CTXT* lc)
 }
 }

+ if (lc->dconf->oper_attribs) {
+ oper_attribs = apr_array_copy(lc->r->pool, lc->dconf->oper_attribs);
+
+ for (i = 0; ((attrib = apr_array_pop(oper_attribs)) != NULL); i++) {
+ for (p = *attrib; *p != '\0'; p++)
+ *p = toupper(*p);
+ apr_table_set(lc->envvars, *attrib, *attrib);
+
+ if (lc->sconf->debug)
+ ap_log_error(APLOG_MARK, APLOG_INFO, 0, lc->r->server,
+ "webauthldap(%s): oper attribute to put into env: %s",
+ lc->r->user, *attrib);
+ }
+ }
+
 /* Allocate the privgroups table, and populate its keys with the
 privgroups we've been asked to check and export. We do not care about
 the values in this table; we're only using it to generate a set of
@@ -1177,6 +1192,7 @@ auth_checker_hook(request_rec * r)
 /* if we have attributes to set or privgroups to check, we need to keep
 going */
 if (!apr_is_empty_array((const apr_array_header_t *)lc->dconf->attribs) ||
+ !apr_is_empty_array((const apr_array_header_t *)lc->dconf->oper_attribs) ||
 !apr_is_empty_array((const apr_array_header_t *)lc->dconf->privgroups))
 needs_further_handling = 1;
 else if (reqs_arr) {
@@ -1301,6 +1317,34 @@ auth_checker_hook(request_rec * r)

 apr_table_do(webauthldap_exportprivgroup, lc, lc->privgroups, NULL);

+ /*
+ * If configured to look for operational attributes, query LDAP again for
+ * all operational attributes and export them into the environment.
+ */
+ if (lc->dconf->oper_attribs != NULL) {
+ if (lc->sconf->debug)
+ ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server,
+ "webauthldap: looking up operational attributes");
+
+ lc->attrs = apr_pcalloc(lc->r->pool, (sizeof(char*) * 2));
+ lc->attrs[0] = LDAP_ALL_OPERATIONAL_ATTRIBUTES;
+ lc->attrs[1] = NULL;
+
+ if (webauthldap_dosearch(lc) != 0) {
+ apr_thread_mutex_unlock(lc->sconf->totalmutex); /* error: unlock */
+ return DECLINED;
+ }
+
+ /* Cool, we got the oper attrs, now set the envvars */
+ for (i = 0; i< lc->numEntries; i++)
+ apr_table_do(webauthldap_exportattrib, lc, lc->entries[i], NULL);
+ apr_table_do(webauthldap_attribnotfound, lc, lc->envvars, NULL);
+
+ if (lc->sconf->debug)
+ ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server,
+ "webauthldap: finished looking up params");
+ }
+
 webauthldap_returnconn(lc);
 apr_thread_mutex_unlock(lc->sconf->totalmutex); /**** FINAL UNLOCKING! ****/

@@ -1506,6 +1550,31 @@ fixups_hook(request_rec *r)
 return DECLINED;
 }
 apr_table_do(webauthldap_exportprivgroup, lc, lc->privgroups, NULL);
+
+ /*
+ * If configured to look for operational attributes, query LDAP again for
+ * all operational attributes and export them into the environment.
+ */
+ if (lc->dconf->oper_attribs != NULL) {
+ if (lc->sconf->debug)
+ ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server,
+ "webauthldap: looking up operational attributes");
+
+ lc->attrs = apr_pcalloc(lc->r->pool, (sizeof(char*) * 2));
+ lc->attrs[0] = LDAP_ALL_OPERATIONAL_ATTRIBUTES;
+ lc->attrs[1] = NULL;
+
+ if (webauthldap_dosearch(lc) != 0) {
+ apr_thread_mutex_unlock(lc->sconf->totalmutex); /* error: unlock */
+ return DECLINED;
+ }
+
+ /* Cool, we got the oper attrs, now set the envvars */
+ for (i = 0; i< lc->numEntries; i++)
+ apr_table_do(webauthldap_exportattrib, lc, lc->entries[i], NULL);
+ apr_table_do(webauthldap_attribnotfound, lc, lc->envvars, NULL);
+ }
+
 webauthldap_returnconn(lc);
 apr_thread_mutex_unlock(lc->sconf->totalmutex); /**** FINAL UNLOCKING! ****/

diff --git a/modules/ldap/mod_webauthldap.h b/modules/ldap/mod_webauthldap.h
index 675fc63..ead6932 100644
--- a/modules/ldap/mod_webauthldap.h
+++ b/modules/ldap/mod_webauthldap.h
@@ -89,6 +89,7 @@ struct server_config {
struct dir_config {
 apr_array_header_t *attribs; /* Array of const char * */
 apr_array_header_t *privgroups; /* Array of const char * */
+ apr_array_header_t *oper_attribs; /* Array of const char * */
};

/* Used for passing things around */
@@ -108,6 +109,7 @@ typedef struct {

 LDAP *ld;
 char **attrs; /* attributes to retrieve from LDAP, (null = all)
+ * (+ = operational)
 */
 char *filter;
 int port;

Re: [PATCH] Added ability to look up operational attributes

Reply via email to