Re: [WIP] mod_authnz_ldap / mod_ldap support for APR v2 LDAP API

[Graham Leggett via dev]

On 19 Apr 2024, at 09:01, Ruediger Pluem  wrote:

> First of all I haven't gone through the patch. Hence I may be off below.
> 
> I think the core issue with the LDAP API we have in APR-UTIL 1.x today and 
> that let to the
> removal discussions on APR side in 2010 
> (https://lists.apache.org/thread/pz5c28g0gf248fvpds53zl1f8by9n8n4)
> and 2011 (https://lists.apache.org/thread/cslw4gj0jgx3bmr8trz0jtfq5bmv1tfl) 
> is that it is an incomplete
> abstraction of the LDAP libraries. mod_authnz_ldap and mod_ldap as consumers 
> of the current API
> have LDAP library specific code and need to link directly against the 
> specific LDAP library.

What you've described above is the exact thing that has been fixed.

This is the core of the patch:

https://github.com/apache/httpd/pull/438/files#diff-f309c2fcd650f484a21c930ddde995a646387421b671232f5b643328f7d2da06

diff --git a/modules/aaa/config.m4 b/modules/aaa/config.m4
index 1b59f99f492..b12e03b287d 100644
--- a/modules/aaa/config.m4
+++ b/modules/aaa/config.m4
@@ -49,21 +49,7 @@ APACHE_MODULE(authz_core, core authorization provider vector 
module, , , yes)
 
 dnl LDAP authentication module. This module has both the authn and authz
 dnl modules in one, so as to share the LDAP server config directives.
-APACHE_MODULE(authnz_ldap, LDAP based authentication, , , most, [
-  APACHE_CHECK_APR_HAS_LDAP
-  if test "$ac_cv_APR_HAS_LDAP" = "yes" ; then
-if test -z "$apu_config" ; then
-  LDAP_LIBS="`$apr_config --ldap-libs`"
-else
-  LDAP_LIBS="`$apu_config --ldap-libs`"
-fi
-APR_ADDTO(MOD_AUTHNZ_LDAP_LDADD, [$LDAP_LIBS])
-AC_SUBST(MOD_AUTHNZ_LDAP_LDADD)
-  else
-AC_MSG_WARN([apr/apr-util is compiled without ldap support])
-enable_authnz_ldap=no
-  fi
-])
+APACHE_MODULE(authnz_ldap, LDAP based authentication, , , most)
 
 dnl FastCGI authorizer interface, supporting authn and authz.
 APACHE_MODULE(authnz_fcgi,

LDAP is no longer special. DBD, DBM, SQL, LDAP, all the same.

> I agree that it is of limited sense to decide during runtime which LDAP 
> library to use as typically there is
> only one available on a particular system. This is substantially different 
> from the situation with databases.

This isn't the whole story.

Like databases, we have lots of different drivers with wildly different and 
incompatible implementations. Unlike databases, the majority of these drivers 
are based on a C API published as an RFC 30 years ago, and therefore are about 
80% common code / clashing symbols. It is the way that it is. The driver 
specific code involves SSL (which evolved) and SASL binding (which evolved), 
but the vast majority of the code is still standard.

For this reason it's pointless coming up with five separate drivers for five 
separate libraries. You can never load two at the same time because clashing 
symbols. This is true of all code, it has nothing to do with httpd 
specifically. Someone binds to library A in a module, and then someone else 
binds to library B in a different module, you are not loading those two modules 
into the server at the same time, it's just not happening.

A further problem is that the libraries diverged.

Many of the functions in OpenLDAP and Windows have been deprecated, replaced 
with _ext functions. The LDAP_DEPRECATED define is now gone.

Rebinding for example works totally differently if you're on IBM, or on 
OpenLDAP. One does not block, but has limited SASL capability. The other does 
SASL for you, but blocks. Windows expects you to rebind yourself.

SASL is wild west. OpenLDAP does SASL for you, but you have to do an intricate 
dance between two functions to make it work. Windows says "here you go, have a 
binary blob, pass it to the Windows specific SASL implementation will you". 
Again, different approaches, it is what it is.
 
Non-blocking behaviour is inconsistent. Some functions in some libraries block 
in some circumstances. You've got to be a masochist if every single consumer of 
LDAP has to deal with all of these inconsistencies, over and over again.

The "P" in APR stands for portability, and that is exactly what I as a consumer 
of an LDAP API want.

> The point back then on APR side was that this incomplete API should be either 
> "completed" or removed from APR
> and that its code should move to httpd which should deal then directly with 
> all the gory details of the various
> LDAP libraries not just with a subset. This happened on APR trunk side, but 
> not on httpd trunk side.

What happened in the past was that code was removed from APR without a 
replacement implementation having been provided first. The expectation was that 
other people (me) would provide free labour to replace that removed 
implementation to make the server whole again.

I have been working on this on and off for years, providing custom patched 
httpd's to get the immediate job done. It has reached the point where I can't 
be supporting custom patches and this needed finishing.

People have been very 

Re: [WIP] mod_authnz_ldap / mod_ldap support for APR v2 LDAP API

[Graham Leggett via dev]

On 19 Apr 2024, at 09:07, Ruediger Pluem  wrote:

> Would it be possible to provide this patch as a PR against trunk on Github? I 
> think that would ease reviewing it.
> I am quite aware that this moves some gory detail discussion of that patch 
> away from the dev list here and over
> to the PR where it is likely harder to find later. Nevertheless I think that 
> the easier way to review the patch combined with
> the tests applied to the PR via Github actions outweigh this drawback.

Take a look here:

https://github.com/apache/httpd/pull/438

The key change is that the new apr-ldap API is 100% asynchronous, with helpful 
patterns to make it easy to use in synchronous code like present httpd. The 
intention is that future code will allow us to have an LDAP connection 
controlled by the MPM, with "guest" requests added by httpd requests and 
removed by pool cleanups. This is useful by anybody, not just httpd.

RFC1823 is 30-ish years old, we're not exposing this old API any more.

https://www.rfc-editor.org/rfc/rfc1823.html

Regards,
Graham
--

Re: [WIP] mod_authnz_ldap / mod_ldap support for APR v2 LDAP API

[Graham Leggett via dev]

On 18 Apr 2024, at 14:21, Joe Orton  wrote:

> This design decision seems surprising to me, what does this add? Adding 
> another abstraction layer to allow runtime selection of the LDAP library 
> seems like a step backwards (a lot of complexity with no benefit). 
> Unlike with e.g. a database backend selection users don't care about 
> picking/configuring among many LDAP libraries.

Can you read through the following and confirm whether your questions are 
answered?

https://github.com/apache/apr/blob/trunk/include/apr_ldap.h#L27

If any specific questions aren't answered, can you confirm so I can make these 
clear in the docs?

Regards,
Graham
--

Re: [VOTE] Release httpd-2.4.59-rc1 as httpd-2.4.59

[Graham Leggett via dev]

On 03 Apr 2024, at 13:26, Eric Covener  wrote:

> [ ] +1: It's not just good, it's good enough!

+1 on RHEL9.

Regards,
Graham
--

Re: mod_systemd: refactor to get rid of libsystemd dependency?

[Graham Leggett via dev]

On 02 Apr 2024, at 11:25, Rainer Jung  wrote:

> in the light of the recent xz attack I was wondering, whether we should also 
> reduce our library dependencies by no longer using sd_notify() in mod_systemd 
> (thus loading libsystemd and all of its dependencies), but instead taking the 
> approach to hard code sd_notify functionality.
> 
> I guess the Linux distributors who patched sshd to use libsystemd for 
> notification are on their way to do the same for their sshd patches, so we 
> might soon get an idea how to do that properly.
> 
> This is not meant to become part of out next release (this week), but 
> hopefully we can manage to code it for the next one.
> 
> WDYT: does this make sense?

Definite +1.

The attack surface on systemd has always been too big, now is the time to fix 
that.

Regards,
Graham
--

Re: mod_ssl: Add support for loading keys from OpenSSL 3.x providers via STORE

[Graham Leggett via dev]

On 27 Nov 2023, at 15:02, Ingo Franzki  wrote:

> The mod_ssl module has support for loading keys and certificates from OpenSSL 
> engines via PKCS#11 URIs at SSLCertificateFile and SSLCertificateKeyFile, 
> e.g. using the PKCS#11 engine part of libp11 
> (https://github.com/OpenSC/libp11). 
> 
> This works fine, but with OpenSSL 3.0 engines got deprecated, and a new 
> provider concept is used.
> OpenSSL 1.1.1 is no longer supported by the OpenSSL organization 
> (https://www.openssl.org/blog/blog/2023/09/11/eol-111/), 
> and newer distributions all have OpenSSL 3.x included.
> Currently, engines do still work, bit since they are deprecated, they will at 
> some point in time no longer be working.
> 
> With OpenSSL 3.x providers one can implements loading of keys and 
> certificates by implementing a STORE method.
> With this, keys and certificates can be loaded for example from PKCS#11 
> modules via PKCS#11 URIs, just like it was possible with an PKCS#11 engine. 
> 
> Please find below some code changes required to support loading the server 
> private key and certificates from a PKCS#11 provider using OpenSSL STORE 
> providers. 

Definite +1 in principle.

> Index: docs/manual/mod/mod_ssl.html.en.utf8
> ===
> --- docs/manual/mod/mod_ssl.html.en.utf8  (revision 1914150)
> +++ docs/manual/mod/mod_ssl.html.en.utf8  (working copy)
> @@ -666,7 +666,7 @@

Would it be possible to patch mod_ssl.xml instead of the html file, the html is 
autogenerated.

> Index: modules/ssl/ssl_engine_config.c
> ===
> --- modules/ssl/ssl_engine_config.c   (revision 1914150)
> +++ modules/ssl/ssl_engine_config.c   (working copy)
> @@ -689,6 +689,11 @@
> if (strcEQ(arg, "builtin")) {
> mc->szCryptoDevice = NULL;
> }
> +#if MODSSL_USE_OPENSSL_STORE
> +else if (strcEQ(arg, "provider")) {
> +mc->szCryptoDevice = arg;
> +}
> +#endif
> #if MODSSL_HAVE_ENGINE_API

This patch isn’t applying for me, looks like the leading spaces have been lost. 
Would it be possible to try attach it as a file?

Regards,
Graham
—

Re: svn commit: r1913977 - /httpd/httpd/trunk/modules/aaa/mod_authnz_ldap.c

[Graham Leggett via dev]

On 24 Nov 2023, at 19:26, Ruediger Pluem  wrote:

>> We do - and we also need to apr_pstrdup() the dn to be consistent with the 
>> rest.
> 
> Why? dn is already apr_pstrdup from r->pool:
> 
> https://github.com/apache/httpd/blob/dc76ce4c43efb8c0c36a5990aeb0468a87458087/modules/ldap/util_ldap.c#L2133

Looks like we were consistently wasteful due to lack of const, fixed in 
r1914281.

Regards,
Graham
—

Re: svn commit: r1913977 - /httpd/httpd/trunk/modules/aaa/mod_authnz_ldap.c

[Graham Leggett via dev]

On 24 Nov 2023, at 14:25, Ruediger Pluem  wrote:

>> +req->dn = dn;
> 
> Don't we need to save the vals as well?

We do - and we also need to apr_pstrdup() the dn to be consistent with the rest.


Index: modules/aaa/mod_authnz_ldap.c
===
--- modules/aaa/mod_authnz_ldap.c   (revision 1914090)
+++ modules/aaa/mod_authnz_ldap.c   (working copy)
@@ -1453,7 +1453,6 @@
 t = require;
 
 if (t[0]) {
-const char **vals;
 
 ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02630)
   "auth_ldap authorize: checking filter %s", t);
@@ -1460,11 +1459,11 @@
 
 /* Search for the user DN */
 result = util_ldap_cache_getuserdn(r, ldc, sec->url, sec->basedn,
- sec->scope, sec->attributes, t, , );
+ sec->scope, sec->attributes, t, , &(req->vals));
 
 /* Make sure that the filtered search returned a single dn */
 if (result == LDAP_SUCCESS && dn) {
-req->dn = dn;
+req->dn = apr_pstrdup(r->pool, dn);
 ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02631)
   "auth_ldap authorize: require ldap-search: "
   "authorization successful");

Regards,
Graham
—

Re: svn commit: r1913977 - /httpd/httpd/trunk/modules/aaa/mod_authnz_ldap.c

[Graham Leggett via dev]

On 24 Nov 2023, at 10:03, Yann Ylavic  wrote:

> +1 this is pretty much what Rüdiger proposed earlier and it aligns
> with the proposed 2.4.x backport so I understand better :)

Rüdiger’s patch had conflicts in the time he posted it and now, I reapplied his 
changes in the above patch and confirmed they worked.

Who wants to commit? (Am happy to)

Regards,
Graham
—

Re: svn commit: r1913977 - /httpd/httpd/trunk/modules/aaa/mod_authnz_ldap.c

[Graham Leggett via dev]

On 20 Nov 2023, at 15:10, Ruediger Pluem  wrote:

> 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);

I completely misunderstood this - I had the idea that build_request_config() 
was being removed, when it was being left behind, sorry about that.

The patch that applies to trunk looks like this, and I just tested it and it 
works:

Index: modules/aaa/mod_authnz_ldap.c
===
--- modules/aaa/mod_authnz_ldap.c   (revision 1914067)
+++ modules/aaa/mod_authnz_ldap.c   (working copy)
@@ -1441,24 +1441,6 @@
 req = build_request_config(r);
 }
 ldc = get_connection_for_authz(r, LDAP_SEARCH);
-if (!req->dn && r->user) {
-authz_status rv;
-if (!*r->user) {
-ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(10487)
-  "ldap authorize: Userid is blank, AuthType=%s",
-  r->ap_auth_type);
-}
-rv = get_dn_for_nonldap_authn(r, ldc);
-if (rv != AUTHZ_GRANTED) {
-return rv;
-}
-if (req->dn == NULL || !*req->dn) {
-ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02636)
-  "auth_ldap authorize: require ldap-search: user's DN 
"
-  "has not been defined; failing authorization");
-return AUTHZ_DENIED;
-}
-}
 
 require = ap_expr_str_exec(r, expr, );
 if (err) {
@@ -1482,6 +1464,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,
Graham
—

Re: svn commit: r1914067 - in /httpd/httpd/trunk: changes-entries/ldap-optimise.txt modules/aaa/mod_authnz_ldap.c

[Graham Leggett via dev]

On 23 Nov 2023, at 13:28, Yann Ylavic  wrote:

>> Unfortunately the message is correct and we do just that via 
>> get_dn_for_nonldap_authn (line 1451). The DN you mention we look for
>> later in line 1480 via util_ldap_cache_getuserdn.
> 
> My guess was that if r->user is set we always want to use it for ldap
> requests/authn? Or maybe there should be another Require ldap-!search
> configured to achieve that (if wanted) since ldap-search is special..

We already have that, it’s ldap-filter.

In more detail, ldap-filter takes an LDAP filter, for example 
(objectclass=mailRecipient), and then it takes an r->user, for example 
f...@example.com, and then it combines them to create a new filter 
(&(uid=f...@example.com)(objectclass=mailRecipient)), and if exactly one (not 
zero, not two) objects exist, it is a match.

ldap-search ignores the r->user special case and broadens this to any 
expression you can image.

>>> On a side note, ldap-search requires that exactly one LDAP object matches, 
>>> we might want to support many matches, or a specific
>>> number of matches.
> 
> I still don't get where "Make sure that the filtered search returned a
> single dn" is enforced, is it because result != LDAP_SUCCESS
> otherwise, or dn == NULL if there are multiple objects/DNs?

It’s enforced here:

https://github.com/apache/httpd/blob/trunk/modules/ldap/util_ldap.c#L2118

For historic reasons, the function is called uldap_cache_getuserdn(), but what 
the function actually does is run a single LDAP query, and expect one and only 
one object in return.

ldap-search runs a single LDAP query, and expects one and only one object in 
return, it just isn’t a user.

Regards,
Graham
—

Re: svn commit: r1914067 - in /httpd/httpd/trunk: changes-entries/ldap-optimise.txt modules/aaa/mod_authnz_ldap.c

[Graham Leggett via dev]

On 23 Nov 2023, at 11:25, Ruediger Pluem  wrote:

>> -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;
>> +if (req->dn == NULL || !*req->dn) {
>> +ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02636)
>> +  "auth_ldap authorize: require ldap-search: user's 
>> DN "
>> +  "has not been defined; failing authorization");
>> +return AUTHZ_DENIED;
>> +}
> 
> Why do we need to get the dn in case that r->user is not NULL and why is it a 
> reason to fail if we don't get a dn for this user?

This message is misleading, the DN we care about is not the DN of the user, but 
rather the DN of the object returned in the ldap-search, which may or may not 
bear a relation to the user.

For example the ldap-search might be doing a lookup on (sucks thumb) the issuer 
of a certificate, which if it matches some LDAP object means it is allowed. The 
DN will not refer to the user, but something else.

On a side note, ldap-search requires that exactly one LDAP object matches, we 
might want to support many matches, or a specific number of matches.

Regards,
Graham
—

Re: svn commit: r1913977 - /httpd/httpd/trunk/modules/aaa/mod_authnz_ldap.c

[Graham Leggett via dev]

On 20 Nov 2023, at 15:32, Yann Ylavic  wrote:

> OK, I drop a new v3 here just in case and let those who know how LDAP
> authn/authz work take whatever :)
> This is just to show that there is some room for
> factorization/disambiguation in this code..
> 
> 
> Regards;
> Yann.
> 

Just tested this, and it appears to work fine.

One tiny detail, these is a logging line that refers to ldap-filter inside 
ldap-search, but other than that, +1.

+  "auth_ldap authorize: require ldap-filter: user's DN 
"

Regards,
Graham
—

Re: svn commit: r1913977 - /httpd/httpd/trunk/modules/aaa/mod_authnz_ldap.c

[Graham Leggett via dev]

On 20 Nov 2023, at 15:10, Ruediger Pluem  wrote:

> 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);

Unfortunately removing this makes it impossible to use ldapsearch for authz 
only, as the setup process normally done in the authn phase never happens.

The typical use case for ldapsearch is without authn, because you’re allowing 
someone in based on something other than a user login, for example a property 
of the SSL connection, or an IP address, etc.

Regards,
Graham
—

Re: svn commit: r1913977 - /httpd/httpd/trunk/modules/aaa/mod_authnz_ldap.c

[Graham Leggett via dev]

On 20 Nov 2023, at 12:26, Ruediger Pluem  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.

Regards,
Graham
—

Re: svn commit: r1913962 - /httpd/httpd/trunk/modules/aaa/mod_authnz_ldap.c

[Graham Leggett via dev]

On 20 Nov 2023, at 12:10, Yann Ylavic  wrote:

>> Fine, but if r->user is NULL here we'll segfault (NULL dereference) in
>> "if (!*r->user)" here.
> 
> Probably an unfortunate copy/paste in trunk only (not in your backport
> patch3), fixed in r1913977.

Thanks for this - it’s been making my head bleed.

Regards,
Graham
—

Re: svn commit: r1913962 - /httpd/httpd/trunk/modules/aaa/mod_authnz_ldap.c

[Graham Leggett via dev]

On 20 Nov 2023, at 09:32, Ruediger Pluem  wrote:

>> -if (sec->host) {
>> +if (!sec->host) {
>> +ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01738)
> 
> This log message had the number 02636 before.

I’ve fixed these.

They don’t appear to affect the ldapsearch backport proposal, all the codes 
there are unique.

Regards,
Graham
—

Re: svn commit: r1913962 - /httpd/httpd/trunk/modules/aaa/mod_authnz_ldap.c

[Graham Leggett via dev]

On 20 Nov 2023, at 10:44, Yann Ylavic  wrote:

>> URL: http://svn.apache.org/viewvc?rev=1913962=rev
>> Log:
>> Apply earlier fix to the ldapsearch case:
>> 
>> Arrange for backend LDAP connections to be returned
>> to the pool by a fixup hook rather than staying locked
>> until the end of (a potentially slow) request.
> 
> It seems that this commit aligns the checks/setup of ldapsearch with
> the ones of ldapfilter, but nothing about LDAP connections
> recycling/reuse?

That’s correct - the recycling/reuse code is being backported separately, it 
all depends on this code.

The end goal is for all trunk changes to be applied to v2.4 and each is aligned.

> In ldapfilter_check_authorization() we bail out early if r->user is
> NULL but not here in ldapsearch_check_authorization(), can't it
> happen?

In ldapsearch we don’t care about the user, it’s purely whether the filter 
expression being applied in the search returns exactly one result.

In ldapfilter we amend the filter to add the user, which is what we don’t want 
in ldapsearch.

Regards,
Graham
—

Re: svn commit: r1591012 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/aaa/mod_authnz_ldap.c

[Graham Leggett via dev]

On 18 Nov 2023, at 17:14, Yann Ylavic  wrote:

> Oh it seems that the callers want the "filtbuf" to be \0 terminated
> (even in case of error), so this v2 probably..

Looking at this now.

Seems to be some differences between trunk and v2.4, seeing how they can be 
aligned to make this easier.

Regards,
Graham
—

Re: svn commit: r1913936 - /httpd/httpd/branches/2.4.x/STATUS

[Graham Leggett via dev]

On 18 Nov 2023, at 14:43, Yann Ylavic  wrote:

> Do we need an MMN bump for some ap_proxy_ functions added to
> proxy_util.h (non public header), though AP_PROXY_DECLARE()d because
> they need to be accessible from several proxy modules SSOs?

I double checked, thought proxy_util.h was public when it isn’t, looks fine 
without the bump.

Regards,
Graham
—

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

[Graham Leggett via dev]

On 17 Apr 2023, at 12:07, Yann Ylavic  wrote:

>> 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 called the directive AliasPreservePath which seemed a more accurate fit, and 
added it in r1911067.

Regards,
Graham
—

Re: [VOTE] Switch read/write repository from Subversion to Git

[Graham Leggett via dev]

On 04 May 2023, at 09:34, Ruediger Pluem  wrote:

> This is a formal vote on whether we should move our read/write repository 
> from Subversion to Git.
> This means that our latest read/write repository will be no longer available 
> via svn.apache.org. It
> will be available via Git at 
> https://gitbox.apache.org/repos/asf/httpd-site.git and 
> https://github.com/apache/httpd.git.
> Github also offers the possibility to use a Subversion client:
> https://docs.github.com/en/get-started/working-with-subversion-on-github/support-for-subversion-clients
> 
> 
> [ ]: Move the read/write repository from Subversion to Git and leverage the 
> features of Github (for now Actions and PR).
> [ ]: Move the read/write repository from Subversion to Git, but I don't want 
> to work with Github and I will only work with
> what gitbox.apache.org offers.
> [X]: Leave everything as is.

I would rather see proper SVN integration with Github. This is a vote of no 
confidence in our own projects.

Regards,
Graham
—

Re: ci vs PR approvals? (was: [apache/httpd] Fix a possible NULL pointer dereference in hook_uri2file (PR #355))

[Graham Leggett via dev]

On 25 Apr 2023, at 07:45, Ruediger Pluem  wrote:

> 2. Switching from Subversion to Git is mostly an emotional problem for me. We 
> have some closer ties to Subversion by some
>   overlaps in the community and via mod_dav_svn we kind of partially eat our 
> very own dogfood here by using Subversion.
>   We wouldn't do that any longer with Git. Plus it would switch another of 
> our development tools from an Apache license to GPL.
>   Apart from technical aspects that this change would create we should check 
> if all of the current active committers are fine
>   using Github. While people could use Gitbox and thus avoid Github when we 
> use Git I would like us to leverage the features of
>   Github when we would do this switch and I think this cannot be done if 
> active committers would have issues with Github.

+1.

I've always found the fight about “must be git” to be really tedious. Github 
supports both git and svn to this day, and people are free to use what they 
prefer by using the interface they are most familiar with.

While Github is popular today, this is only because the goals of the owners of 
Github are presently aligned with our goals. As Twitter has taught us, goals 
change at any time and without warning.

Regards,
Graham
—

Re: [Patch] mod_auth_bearer / mod_autht_jwt: An alternative to AJP

[Graham Leggett via dev]

On 19 Mar 2020, at 12:26, Graham Leggett  wrote:On 19 Mar 2020, at 02:40, Eric Covener  wrote:Neat, have you thought about mod_auth_form in relation to this?Something on my wishlist has been to not put the password in thesession / not continue to call the original auth provider.Yes - the two modules that will benefit from token support are mod_session (which mod_auth_form is just one possible “onramp” to obtain a session token), and mod_ssl, where the token is the cert.Getting back to this.Added in r1909409 and r1909411.There is a corresponding library for tomcat that allows it to receive bearer auth here: https://github.com/minfrin/tomcat-jwt-authenticatorRegards,Graham—

Re: svn commit: r1909135 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h server/core.c

[Graham Leggett via dev]

On 19 Apr 2023, at 19:08, Yann Ylavic  wrote:

> Currently all the tests (framework) are broken due to cmd->regex not
> being properly stacked/scoped.
> If cmd->regex is to be used to store the enclosing section's regex, it
> should be saved and restored by *all* the the sections (much like
> cmd->override is saved/restored using the old_overrides stack variable
> in most sections). It should also be set to NULL when parsing
> (sub-)sections with no possible regex, unless we want to inherit
> cmd->regex there.

This is done in r1909356.

> 
> But for instance some like:
>   
>  
># use /fake from 
>Alias /real
>  
>   
> won't work for Alias because  overwrites cmd->path with "*If".

I just found that - that’s definitely broken, it means you can’t do this:


  
DAV svn
SVNPath /home/trac/svn/extra
  


Looks like the same effect as the Alias bug, where every url is mapped to “*If”.

> Also, what about:
>   
>  
># which regex here?
>  
>   

In the above “which regex here” would be FilesMatch, which wouldn’t change.

> Or:
>   
>  
># 's regex usable here?
>  
>   
> ?

Combining regexes is probably a step too far. Right now DirectoryMatch’s regex 
isn’t usable at the point as the DirectoryMatch regex is not passed across 
Files. This behaviour doesn't change.

> Or third-party sections (unaware of cmd->regex) which contain
> directives that depend on cmd->regex?

Again there would be no change to behaviour that I can see, the regex was 
hidden before and the raw path (containing the regex) would be used, now 
cmd->regex would be set to NULL and the raw path (containing the regex) would 
be used.

> I'm not saying it's a bad idea but it needs more changes than this
> commit, changes that spread all over the code base it seems (modules
> can have their sections too), something like:
> $ grep -r 'AP_INIT\w\+("<' server/ modules/ 2>/dev/null
> server/core.c:AP_INIT_RAW_ARGS(" server/core.c:AP_INIT_RAW_ARGS(" server/core.c:AP_INIT_RAW_ARGS(" NULL, RSRC_CONF,
[snip]

cmd->regex is a function of cmd->path, so only places that touch cmd->path need 
touch cmd->regex, and this seems to be directory, location and file.

 seems broken right now, need to fix that separately.

> Maybe we only want to check that the parent directive is a 
> with Alias for now?

Right now it’s limited to not-in-directory:

https://github.com/apache/httpd/blob/trunk/modules/mappers/mod_alias.c#L146

If this is too difficult to backport it isn't the end of the world, the main 
thing is that it's fixed for the future.

Regards,
Graham
—

POC: updated ldapi:// support for mod_ldap

[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

[Graham Leggett via dev]

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.

Regards,
Graham
—

Re: mod_dav and PATH_INFO - MKCOL silently creates the parent directory

[Graham Leggett via dev]

On 13 Apr 2023, at 16:27, Graham Leggett  wrote:

> Changing the config to this makes it work:
> 
> Alias /storage /var/www/dav/storage
> 
> It looks like the detail that’s missing that we don’t identify the rest of 
> the URL after the alias and pass it on by calling alias_matches() to identify 
> the prefix:
> 
> https://github.com/apache/httpd/blob/trunk/modules/mappers/mod_alias.c#L549 
> 
This is fixed in http://svn.apache.org/viewvc?rev=1909137=rev 
.

Regards,
Graham
—

Re: mod_dav and PATH_INFO - MKCOL silently creates the parent directory

[Graham Leggett via dev]

On 13 Apr 2023, at 14:55, Graham Leggett via dev  wrote:

> Config looks like this:
> 
>  
>Alias /var/www/dav/storage
>Dav on
>AcceptPathInfo On
>  

Changing the config to this makes it work:

Alias /storage /var/www/dav/storage

It looks like the detail that’s missing that we don’t identify the rest of the 
URL after the alias and pass it on by calling alias_matches() to identify the 
prefix:

https://github.com/apache/httpd/blob/trunk/modules/mappers/mod_alias.c#L549

Regards,
Graham
—

mod_dav and PATH_INFO - MKCOL silently creates the parent directory

[Graham Leggett via dev]

Hi all,

I am having super fun time trying to turn on mod_dav on an httpd v2.4.53-11.el9 
(as shipped by RHEL9).

The request “MKCOL /storage/foo” returns a 201, but doesn't actually do 
anything. The “foo” directory is silently not created.

Stepping through the code, the code that creates the directory is creating the 
parent directory “/storage”, and not the actual directory in the request 
"/storage/foo”:

1137status = apr_dir_make(ctx->pathname, APR_OS_DEFAULT, ctx->pool);
(gdb) print ctx->pathname
$6 = 0x7fffbc013450 "/var/www/dav/storage"

This explains the “it’s doing nothing” - it created the parent directory, which 
already exists.

Looking at the whole request, it looks like the PATH_INFO mechanism hasn't done 
anything:

(gdb) print *r
$8 = {pool = 0x7fffb801a898, connection = 0x7fffe4033630, server = 
0x55689e28, next = 0x0, prev = 0x0, main = 0x0, 
  the_request = 0x7fffb801be48 "MKCOL /storage/foo/ HTTP/1.1", assbackwards = 
0, proxyreq = 0, header_only = 0, 
  proto_num = 1001, protocol = 0x7fffb801be3c "HTTP/1.1", hostname = 
0x7fffb801c028 “seawitch.xxx.xxx", 
  request_time = 1681392063530545, status_line = 0x0, status = 200, 
method_number = 10, method = 0x7fffb801be28 "MKCOL", 
  allowed = 67141423, allowed_xmethods = 0x0, allowed_methods = 0x7fffb801abf0, 
sent_bodyct = 0, bytes_sent = 0, mtime = 0, 
  range = 0x0, clength = 0, chunked = 0, read_body = 0, read_chunked = 0, 
expecting_100 = 0, kept_body = 0x0, body_table = 0x0, 
  remaining = 0, read_length = 0, headers_in = 0x7fffb801ac30, headers_out = 
0x7fffb801b4d0, err_headers_out = 0x7fffb801b718, 
  subprocess_env = 0x7fffb801b150, notes = 0x7fffb801ba58, content_type = 0x0, 
handler = 0x77701b49 "dav-handler", 
  content_encoding = 0x0, content_languages = 0x0, vlist_validator = 0x0, user 
= 0x0, ap_auth_type = 0x0, 
  unparsed_uri = 0x7fffb801be68 "/storage/foo/", uri = 0x7fffb801be78 
"/storage/foo/", 
  filename = 0x7fffb801c540 "/var/www/dav/storage", canonical_filename = 
0x7fffb801c540 "/var/www/dav/storage", 
  path_info = 0x7fffb801c484 "", args = 0x0, used_path_info = 0, eos_sent = 0, 
per_dir_config = 0x7fffbc013090, 
  request_config = 0x7fffb801bbf8, log = 0x55689e48, log_id = 
0x7fffb801c070 "McasmB+dDPA", htaccess = 0x0, 
  output_filters = 0x7fffb801bd28, input_filters = 0x7fffb801c000, 
proto_output_filters = 0x7fffb801bd28, 
  proto_input_filters = 0x7fffb801c000, no_cache = 0, no_local_copy = 0, 
invoke_mtx = 0x7fffb801c080, parsed_uri = {
scheme = 0x0, hostinfo = 0x0, user = 0x0, password = 0x0, hostname = 0x0, 
port_str = 0x0, 
path = 0x7fffb801be78 "/storage/foo/", query = 0x0, fragment = 0x0, hostent 
= 0x0, port = 0, is_initialized = 1, 
dns_looked_up = 0, dns_resolved = 0}, finfo = {pool = 0x7fffb801a898, valid 
= 7598960, protection = 18288, 
filetype = APR_NOFILE, user = 0, group = 48, inode = 17736438, device = 
64768, nlink = 2, size = 6, csize = 0, 
atime = 1681391795173106, mtime = 1681391869558474, ctime = 
1681391869558474, 
fname = 0x7fffb801c540 "/var/www/dav/storage", 
name = 0x77e517b1  
"H\203\304\bL\211\340]A\\\303\017\037@", filehand = 0x7fffb801c1e0}, 
  useragent_addr = 0x7fffe4033490, useragent_ip = 0x7fffe4033968 
“:::::::", 
  trailers_in = 0x7fffb801afb0, trailers_out = 0x7fffb801b8b8, useragent_host = 
0x0, double_reverse = 0, bnotes = 0}

Most specifically r->path_info is “", and r->used_path_info is 0.

Does this look familiar to anyone?

Config looks like this:

  
Alias /var/www/dav/storage
Dav on
AcceptPathInfo On
  

  
Options +Indexes

IndexOptions FancyIndexing HTMLTable VersionSort XHTML

AuthzSendForbiddenOnFailure on

SSLUserName SSL_CLIENT_CERT_RFC4523_CEA


  require valid-user


  

Regards,
Graham
—