On 11/20/23 3:46 PM, Yann Ylavic wrote:
> On Mon, Nov 20, 2023 at 2:33 PM Yann Ylavic <ylavic....@gmail.com> wrote:
>>
>> On Mon, Nov 20, 2023 at 1:57 PM Graham Leggett via dev
>> <dev@httpd.apache.org> wrote:
>>>
>>> On 20 Nov 2023, at 12:26, Ruediger Pluem <rpl...@apache.org> wrote:
>>>
>>> Or we need to ensure that authn_ldap_build_filter is NULL safe and returns 
>>> in a sensible way if user == NULL.
>>>
>>>
>>> This is the option we need I think - it’s possible that ldapsearch could be 
>>> used without a user.
>>
>> In the proposed 2.4.x backport of ldapsearch_check_authorization()
>> there is no call to get_dn_for_nonldap_authn() nor
>> authn_ldap_build_filter(). The Require expression is passed directly
>> to util_ldap_cache_getuserdn(), so what is building a filter with
>> r->user about in the ldapsearch case finally?
> 
> I mean, isn't what we need for something like the attached patch?
> This would call get_dn_for_nonldap_authn() only "if we have been
> authenticated by some other module than mod_auth_ldap" (per comment in
> the code), and do nothing about r->user otherwise.
> Again, I don't really know how mod_ldap is supposed to work so
> possibly this is all irrelevant..
> 

Revisiting this again. I guess the below patch should fix it.
I assume that only the checking on req != NULL is relevant.
r->user and req->dn do not matter. But we should set req->dn to dn
if util_ldap_cache_getuserdn found one.

BTW: get_connection_for_authz(r, LDAP_SEARCH); requires a valid req. It cannot 
deal with req == NULL.


Index: modules/aaa/mod_authnz_ldap.c
===================================================================
--- modules/aaa/mod_authnz_ldap.c       (revision 1913980)
+++ modules/aaa/mod_authnz_ldap.c       (working copy)
@@ -1442,23 +1442,10 @@
      */

     if (!req) {
-        authz_status rv = AUTHZ_DENIED;
         req = build_request_config(r);
-        ldc = get_connection_for_authz(r, LDAP_SEARCH);
-        if (AUTHZ_GRANTED != (rv = get_dn_for_nonldap_authn(r, ldc))) {
-            return rv;
-        }
     }
-    else {
-        ldc = get_connection_for_authz(r, LDAP_SEARCH);
-    }

-    if (req->dn == NULL || !*req->dn) {
-        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02636)
-                      "auth_ldap authorize: require ldap-filter: user's DN "
-                      "has not been defined; failing authorization");
-        return AUTHZ_DENIED;
-    }
+    ldc = get_connection_for_authz(r, LDAP_SEARCH);

     require = ap_expr_str_exec(r, expr, &err);
     if (err) {
@@ -1482,6 +1469,7 @@

         /* Make sure that the filtered search returned a single dn */
         if (result == LDAP_SUCCESS && dn) {
+            req->dn = dn;
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02631)
                           "auth_ldap authorize: require ldap-search: "
                           "authorization successful");

Regards

Rüdiger

Reply via email to