POC: updated ldapi:// support for mod_ldap

2023-04-18 Thread Graham Leggett via dev
Hi all,

This is a patch for httpd that adds support for ldapi:// URLs to mod_ldap and 
friends.

It depends on a patch for apr-util posted to the dev@apr list.

Regards,
Graham
—


Index: include/util_ldap.h
===
--- include/util_ldap.h (revision 1909117)
+++ include/util_ldap.h (working copy)
@@ -45,6 +45,10 @@
 /* this whole thing disappears if LDAP is not enabled */
 #if APR_HAS_LDAP
 
+#ifndef APR_HAS_LDAP_INITIALIZE
+#define APR_HAS_LDAP_INITIALIZE 0
+#endif
+
 #if defined(LDAP_UNAVAILABLE) || APR_HAS_MICROSOFT_LDAPSDK
 #define AP_LDAP_IS_SERVER_DOWN(s)((s) == LDAP_SERVER_DOWN \
 ||(s) == LDAP_UNAVAILABLE)
@@ -126,8 +130,8 @@
 const char *reason; /* Reason for an error failure */
 
 struct util_ldap_connection_t *next;
-struct util_ldap_state_t *st;/* The LDAP vhost config this 
connection belongs to */
-int keep;/* Will this connection be kept when 
it's unlocked */
+struct util_ldap_state_t *st;   /* The LDAP vhost config this 
connection belongs to */
+int keep;   /* Will this connection be kept when 
it's unlocked */
 
 int ChaseReferrals; /* [on|off] (default = 
AP_LDAP_CHASEREFERRALS_ON)*/
 int ReferralHopLimit;   /* # of referral hops to follow 
(default = AP_LDAP_DEFAULT_HOPLIMIT) */
@@ -136,6 +140,12 @@
 int must_rebind;/* The connection was last bound with 
other then binddn/bindpw */
 request_rec *r; /* request_rec used to find this 
util_ldap_connection_t */
 apr_time_t last_backend_conn;   /* the approximate time of the last 
backend LDAP request */
+
+#if APR_HAS_LDAP_INITIALIZE
+apr_ldap_err_t result;  /* result of prior operations on this 
connection */
+const char *url;/* URL of the LDAP server (or space 
separated list) */
+apr_ldap_t *ld;
+#endif
 } util_ldap_connection_t;
 
 typedef struct util_ldap_config_t {
@@ -241,6 +251,7 @@
  * @fn util_ldap_connection_t *util_ldap_connection_find(request_rec *r, const 
char *host, int port,
  *   const char 
*binddn, const char *bindpw, deref_options deref,
  *   int netscapessl, 
int starttls)
+ * @deprecated Replaced by uldap_connection_find_ex()
  */
 APR_DECLARE_OPTIONAL_FN(util_ldap_connection_t 
*,uldap_connection_find,(request_rec *r, const char *host, int port,
   const char *binddn, const 
char *bindpw, deref_options deref,
@@ -247,6 +258,26 @@
   int secure));
 
 /**
+ * Find a connection in a list of connections
+ * @param r The request record
+ * @param url The URL to connect to (multiple URLs space separated)
+ * @param binddn The DN to bind with
+ * @param bindpw The password to bind with
+ * @param deref The dereferencing behavior
+ * @param secure use SSL on the connection
+ * @tip Once a connection is found and returned, a lock will be acquired to
+ *  lock that particular connection, so that another thread does not try 
and
+ *  use this connection while it is busy. Once you are finished with a 
connection,
+ *  apr_ldap_connection_close() must be called to release this connection.
+ * @fn util_ldap_connection_t *util_ldap_connection_find_ex(request_rec *r, 
const char *url,
+ *   const char 
*binddn, const char *bindpw, deref_options deref,
+ *   int netscapessl, 
int starttls)
+ */
+APR_DECLARE_OPTIONAL_FN(util_ldap_connection_t 
*,uldap_connection_find_ex,(request_rec *r, const char *url,
+  const char *binddn, const 
char *bindpw, deref_options deref,
+  int secure));
+
+/**
  * Compare two DNs for sameness
  * @param r The request record
  * @param ldc The LDAP connection being used.
Index: modules/aaa/mod_authnz_ldap.c
===
--- modules/aaa/mod_authnz_ldap.c   (revision 1909117)
+++ modules/aaa/mod_authnz_ldap.c   (working copy)
@@ -104,7 +104,7 @@
 module AP_MODULE_DECLARE_DATA authnz_ldap_module;
 
 static APR_OPTIONAL_FN_TYPE(uldap_connection_close) 
*util_ldap_connection_close;
-static APR_OPTIONAL_FN_TYPE(uldap_connection_find) *util_ldap_connection_find;
+static APR_OPTIONAL_FN_TYPE(uldap_connection_find_ex) 
*util_ldap_connection_find_ex;
 static APR_OPTIONAL_FN_TYPE(uldap_cache_comparedn) *util_ldap_cache_comparedn;
 static APR_OPTIONAL_FN_TYPE(uldap_cache_compare) *util_ldap_cache_compare;
 static APR_OPTIONAL_FN_TYPE(uldap_cache_check_subgroups) 
*util_ldap_cache_check_subgroups;
@@ -453,9 

Re: svn commit: r1909137 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_alias.c

2023-04-18 Thread Ruediger Pluem



On 4/17/23 1:07 PM, Yann Ylavic wrote:
> On Mon, Apr 17, 2023 at 9:08 AM Ruediger Pluem  wrote:
>>
>> On 4/16/23 12:20 PM, Graham Leggett via dev wrote:
>>> On 14 Apr 2023, at 17:18, Ruediger Pluem >> > wrote:
>>>
 Would that break configs like

 

 Alias /filesystemprefix/%{HTTP:X-example-header}

 

 where the expression evaluation determines the complete filesystem path 
 without adding the remainder of the URL?
>>>
>>> It would, which alas might mean we can't backport this, which isn’t great.
>>>
 I admit that the above looks like a strange setup and is probably a bad 
 example, but the parameter to Alias could be an arbitrary
 complex expression that evaluates to the final filesystem resource (like 
 AliasMatch). Wouldn't we need a kind of way to figure out
 if the users wants the remainder of the URI added or not even if we do not 
 use a regular expression in the surrounding Location
 block?
>>>
>>> LocationMatch is the correct way to do this - the config explicitly 
>>> declares the exact URL to match, and the Alias gives the exact
>>> unambiguous mapping.
>>>
>>> Having a prefix in the “Alias /foo /bar” case and then arbitrarily not 
>>> having a prefix in the “Alias /bar” case sent me on a huge
>>> wild goose chase - what made it worse was the client was a webdav client, 
>>> so the behaviour just made no sense. I am seeing people
>>> asking why they’re getting a 405 and not getting answers, so I think this 
>>> is tripping up other people too.
>>
>> I completely agree that Alias in a Location should behave as it does after 
>> r1909137 and the behavior change is not a problem in
>> trunk, but I think due this behavior change it is not possible to backport 
>> it as is.
> 
> If we change the behaviour in trunk such that:
>   1. Alias /fake /real
> is the same as:
>   2. 
>Alias /real
>  
> I'd suggest that we treat the /real part the same too (currently /real
> is a plain path in 1. and an expr in 2).

+1, but I guess this creates further backport problems one way or the other.

> It seems that:
>   3. 
>Alias /fake
>  
> could be a thing too (though /real would never be an expr here).

Is 3. really allowed today? I don't think so.

> 
> Likewise "AliasMatch /fake /real" and LocationMatch + "AliasMatch
> /real" only should probably work the same, where /real could use
> captures from the LocationMatch (just like
> LocationMatch+ProxyPassMatch if I'm not mistaken).

I think AliasMatch inside LocationMatch does not work currently.
If we would want to make it work it could be kind of challenging to get the 
captures
without executing the regex twice. Furthermore we need to be careful how we 
intermix
capture replacements and expression execution. With the current code you could 
use
Alias in LocationMatch and if you use named captures you can use them via 
environment
variables in the expression execution which seems kind of safe.

> 
> I also find the names in alias_dir_conf quite confusing, the "alias"
> corresponds to "real" in alias_entry used by alias_server_conf, while
> "alias_fake" corresponds to "alias". ISTM that alias_dir_conf should
> embed an alias_entry rather than duplicating all/most of its fields.
> Maybe we could even get rid of alias_server_conf actually and handle
> everything as perdir merging?
> 
>> The question is if we find an approach to
>> make it backportable e.g. by adding some kind of 2.4.x only configuration 
>> switch to get the behavior of r1909137.
> 
> Maybe "AliasPerDirRelative on" (default: "off", context: "server
> config, virtual host, directory").
> And then we change the default to "on" in trunk.

I am not sure how we get out of this. All stuff sounds messy.
Probably we should create a clean solution for trunk and leave stuff in 2.4.x 
as is.
For the case where the issue was discovered there is always a way to move the 
Alias from the Location to virtual host context.
Probably we could add a warning log message in 2.4.x if the Alias in a Location 
is just a plain string, that it might not behave
as the user expects.

Regards

Rüdiger