Bug report for Apache httpd-2 [2023/11/19]
+---+ | Bugzilla Bug ID | | +-+ | | Status: UNC=Unconfirmed NEW=New ASS=Assigned| | | OPN=ReopenedVER=Verified(Skipped Closed/Resolved) | | | +-+ | | | Severity: BLK=Blocker CRI=Critical REG=Regression MAJ=Major | | | | MIN=Minor NOR=NormalENH=Enhancement TRV=Trivial | | | | +-+ | | | | Date Posted | | | | | +--+ | | | | | Description | | | | | | | |10747|New|Maj|2002-07-12|ftp SIZE command and 'smart' ftp servers results i| |11580|Opn|Enh|2002-08-09|generate Content-Location headers | |12033|Opn|Nor|2002-08-26|Graceful restart immediately result in [warn] long| |13661|Ass|Enh|2002-10-15|Apache cannot not handle dynamic IP reallocation | |14104|Opn|Enh|2002-10-30|not documented: must restart server to load new CR| |16811|Ass|Maj|2003-02-05|mod_autoindex always return webpages in UTF-8.| |17244|Ass|Nor|2003-02-20|./configure --help gives false information regardi| |17497|Opn|Nor|2003-02-27|mod_mime_magic generates incorrect response header| |20036|Ass|Nor|2003-05-19|Trailing Dots stripped from PATH_INFO environment | |21260|Opn|Nor|2003-07-02|CacheMaxExpire directive not enforced ! | |21533|Ass|Cri|2003-07-11|Multiple levels of htacces files can cause mod_aut| |22484|Opn|Maj|2003-08-16|semaphore problem takes httpd down| |22686|Opn|Nor|2003-08-25|ab: apr_poll: The timeout specified has expired (7| |22898|Opn|Nor|2003-09-02|nph scripts with two HTTP header | |23911|Opn|Cri|2003-10-18|CGI processes left defunct/zombie under 2.0.54| |24095|Opn|Cri|2003-10-24|ERROR "Parent: child process exited with status 32| |24437|Opn|Nor|2003-11-05|mod_auth_ldap doubly-escapes backslash (\) charact| |24890|Opn|Nor|2003-11-21|Apache config parser should not be local aware ( g| |25469|Opn|Enh|2003-12-12|create AuthRoot for defining paths to auth files | |25484|Ass|Nor|2003-12-12|Non-service Apache cannot be stopped in WinXP | |26153|Opn|Cri|2004-01-15|Apache cygwin directory traversal vulnerability | |27257|Ass|Enh|2004-02-26|rotatelogs with getopt and setuid | |27715|Ass|Enh|2004-03-16|Client sending misformed Range "bytes = 0-100" ins| |29090|Ass|Enh|2004-05-19|MultiviewsMatch NegotiatedOnly extensions not resp| |29510|Ass|Enh|2004-06-10|ab does not support multiple cookies | |29644|Ver|Nor|2004-06-17|mod_proxy keeps downloading even after the client | |30259|Ass|Enh|2004-07-22|When proxy connects to backend, a DNS lookup is do| |30505|Ass|Enh|2004-08-05|Apache uses 'Error', and not lower level event typ| |31302|Opn|Cri|2004-09-19|suexec doesn't execute commands if they're not in | |31352|Ass|Enh|2004-09-21|RFE, Bind to LDAP server with browser supplier use| |31418|Opn|Nor|2004-09-25|SSLUserName is not usable by other modules| |32328|Opn|Enh|2004-11-19|Make mod_rewrite escaping optional / expose intern| |32750|Ass|Maj|2004-12-17|mod_proxy + Win32DisableAcceptEx = memory leak| |33089|New|Nor|2005-01-13|mod_include: Options +Includes (or IncludesNoExec)| |34519|New|Enh|2005-04-19|Directory index should emit valid XHTML | |35098|Ver|Maj|2005-05-27|Install fails using --prefix | |35154|Opn|Nor|2005-06-01|Support for NID_serialNumber, etc. in SSLUserName | |35652|Opn|Min|2005-07-07|Improve error message: "pcfg_openfile: unable to c| |35768|Opn|Nor|2005-07-17|Missing file logs at far too high of log level| |36676|New|Nor|2005-09-15|time() bug in httpd/os/win32/util_win32.c:wait_for| |36710|Opn|Blk|2005-09-19|CGI output not captured | |37006|Ver|Reg|2005-10-11|"pthread" error when compiling under AIX 5.3 using| |37290|Opn|Min|2005-10-28|DirectoryIndex don't work in scriptaliased directo| |37564|New|Enh|2005-11-19|Suggestion: mod_suexec SuexecUserGroup directive i| |38325|Opn|Nor|2006-01-20|impossible to determine AUTH_TYPE of interpreted r| |38571|New|Enh|2006-02-08|CustomLog directive checked by apachectl configtes| |38995|New|Nor|2006-03-16|httpd tries to communicate with the CGI daemon eve| |39275|Opn|Nor|2006-04-11|slow child_init causes MaxClients warning | |39287|New|Nor|2006-04-12|Incorrect If-Modified-Since validation (due to syn| |39727|Ass|Nor|2006-06-05|Incorrect ETag on gzip:ed content | |39748|New|Enh|2006-06-07|Header and POST support for mod_include |
Re: svn commit: r1589993 - in /httpd/httpd/trunk: CHANGES docs/manual/expr.xml docs/manual/mod/mod_authnz_ldap.xml modules/aaa/mod_authnz_ldap.c
On Wed, Apr 30, 2014 at 1:02 AM Yann Ylavic wrote: > > On Tue, Apr 29, 2014 at 10:54 PM, Christophe JAILLET > wrote: > > Hi, > > > > doc does not build because of below: > > > > CJ > > > > Le 25/04/2014 13:14, minf...@apache.org a écrit : > >> + > >> +LocationMatch ^/dav/(?[^/]+)/ > > > > ^ There > > > > Hmm, won't LocationMatch itself be broken by the inner <>s ? Wow, fortunately I didn't hold my breath on this one :) Someone needs to answer to this former/younger/naive me though and since I'm on this commit again: look Yann, this match is double-quoted now so we should be fine!
Re: svn commit: r1589993 - in /httpd/httpd/trunk: CHANGES docs/manual/expr.xml docs/manual/mod/mod_authnz_ldap.xml modules/aaa/mod_authnz_ldap.c
On Fri, Apr 25, 2014 at 1:15 PM wrote:
>
> Author: minfrin
> Date: Fri Apr 25 11:14:36 2014
> New Revision: 1589993
>
> URL: http://svn.apache.org/r1589993
> Log:
> Add the ldap-search option to mod_authnz_ldap, allowing authorization
> to be based on arbitrary expressions that do not include the username.
[]
>
> --- httpd/httpd/trunk/docs/manual/mod/mod_authnz_ldap.xml (original)
> +++ httpd/httpd/trunk/docs/manual/mod/mod_authnz_ldap.xml Fri Apr 25 11:14:36
> 2014
[]
> @@ -508,6 +514,28 @@ AuthLDAPMaxSubGroupDepth 1
>
>
>
> +Require ldap-search
> +
> +The Require ldap-search directive allows the
> +administrator to grant access based on a generic LDAP search filter
> using an
> +expression. If there is exactly one match to
> the search filter,
> +regardless of the distinguished name, access is granted.
I get from this that there should be one match..
>
> --- httpd/httpd/trunk/modules/aaa/mod_authnz_ldap.c (original)
> +++ httpd/httpd/trunk/modules/aaa/mod_authnz_ldap.c Fri Apr 25 11:14:36 2014
[]
>
> +static authz_status ldapsearch_check_authorization(request_rec *r,
> + const char *require_args,
> + const void
> *parsed_require_args)
> +{
> +int result = 0;
> +authn_ldap_config_t *sec =
> +(authn_ldap_config_t *)ap_get_module_config(r->per_dir_config,
> _ldap_module);
> +
> +util_ldap_connection_t *ldc = NULL;
> +
> +const char *err = NULL;
> +const ap_expr_info_t *expr = parsed_require_args;
> +const char *require;
> +const char *t;
> +const char *dn = NULL;
> +
> +if (!sec->have_ldap_url) {
> +return AUTHZ_DENIED;
> +}
> +
> +if (sec->host) {
> +ldc = get_connection_for_authz(r, LDAP_SEARCH);
> +apr_pool_cleanup_register(r->pool, ldc,
> + authnz_ldap_cleanup_connection_close,
> + apr_pool_cleanup_null);
> +}
> +else {
> +ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01738)
> + "auth_ldap authorize: no sec->host - weird...?");
> +return AUTHZ_DENIED;
> +}
> +
> +require = ap_expr_str_exec(r, expr, );
> +if (err) {
> +ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO()
> + "auth_ldap authorize: require ldap-search: Can't "
> + "evaluate require expression: %s", err);
> +return AUTHZ_DENIED;
> +}
> +
> +t = require;
> +
> +if (t[0]) {
> +const char **vals;
> +
> +ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO()
> + "auth_ldap authorize: checking filter %s", t);
> +
> +/* Search for the user DN */
> +result = util_ldap_cache_getuserdn(r, ldc, sec->url, sec->basedn,
> + sec->scope, sec->attributes, t, , );
> +
> +/* Make sure that the filtered search returned a single dn */
And it's restated here..
> +if (result == LDAP_SUCCESS && dn) {
> +ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO()
> + "auth_ldap authorize: require ldap-search: "
> + "authorization successful");
> +return AUTHZ_GRANTED;
I get that for "ldap-filter" (unlike for "ldap-search here) we'll do a
util_ldap_cache_comparedn() to (double) check the returned DN somehow
(sorry I don't really know how LDAP works), not here though because we
don't require a particular DN but just a single one.
But what makes sure that it's the case here?
> +}
> +else {
> +ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO()
> + "auth_ldap authorize: require ldap-search: "
> + "%s authorization failed [%s][%s]",
> + t, ldc->reason, ldap_err2string(result));
> +}
> +}
> +
> +ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO()
> + "auth_ldap authorize filter: authorization denied for "
> + "to %s", r->uri);
> +
> +return AUTHZ_DENIED;
> +}
Regards;
Yann.
Re: svn commit: r1591012 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/aaa/mod_authnz_ldap.c
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: r1591012 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/aaa/mod_authnz_ldap.c
On Sat, Nov 18, 2023 at 5:59 PM Yann Ylavic wrote:
>
> All in all, I'd rewrite this function like in the attached patch (not
> even compile tested, just to show what I'm talking about..).
Oh it seems that the callers want the "filtbuf" to be \0 terminated
(even in case of error), so this v2 probably..
>
> Regards;
> Yann.
Index: modules/aaa/mod_authnz_ldap.c
===
--- modules/aaa/mod_authnz_ldap.c (revision 1913813)
+++ modules/aaa/mod_authnz_ldap.c (working copy)
@@ -206,7 +206,7 @@ static const char* authn_ldap_xlate_password(reque
* search filter will be (&(posixid=*)(uid=userj)).
*/
#define FILTER_LENGTH MAX_STRING_LEN
-static apr_status_t authn_ldap_build_filter(char *filtbuf,
+static apr_status_t authn_ldap_build_filter(char filtbuf[FILTER_LENGTH],
request_rec *r,
const char *user,
const char *filter,
@@ -219,6 +219,7 @@ static const char* authn_ldap_xlate_password(reque
apr_size_t outbytes;
char *outbuf;
int nofilter = 0, len;
+apr_statut rv = APR_SUCCESS;
if (!filter) {
filter = sec->filter;
@@ -244,7 +245,7 @@ static const char* authn_ldap_xlate_password(reque
* config-supplied portions.
*/
-if ((nofilter = (filter && !strcasecmp(filter, "none" {
+if ((nofilter = (!filter || !*filter || !strcasecmp(filter, "none" {
len = apr_snprintf(filtbuf, FILTER_LENGTH, "(%s=", sec->attribute);
}
else {
@@ -256,12 +257,13 @@ static const char* authn_ldap_xlate_password(reque
* LDAP filter metachars are escaped.
*/
filtbuf_end = filtbuf + FILTER_LENGTH - 1;
+for (p = user, q = filtbuf + len; *p; ) {
+if (strchr("*()\\", *p) != NULL) {
#if APR_HAS_MICROSOFT_LDAPSDK
-for (p = user, q=filtbuf + len;
- *p && q < filtbuf_end; ) {
-if (strchr("*()\\", *p) != NULL) {
-if ( q + 3 >= filtbuf_end)
- break; /* Don't write part of escape sequence if we can't write all of it */
+if (q + 3 >= filtbuf_end) { /* accounts for final \0 */
+rv = APR_EGENERAL;
+goto out;
+}
*q++ = '\\';
switch ( *p++ )
{
@@ -281,23 +283,24 @@ static const char* authn_ldap_xlate_password(reque
*q++ = '5';
*q++ = 'c';
break;
-}
-}
-else
-*q++ = *p++;
-}
+}
#else
-for (p = user, q=filtbuf + len;
- *p && q < filtbuf_end; *q++ = *p++) {
-if (strchr("*()\\", *p) != NULL) {
+if (q + 2 >= filtbuf_end) { /* accounts for final \0 */
+rv = APR_EGENERAL;
+goto out;
+}
*q++ = '\\';
-if (q >= filtbuf_end) {
- break;
+*q++ = *p++;
+#endif
+}
+else {
+if (q + 1 >= filtbuf_end) { /* accounts for final \0 */
+rv = APR_EGENERAL;
+goto out;
}
+*q++ = *p++;
}
}
-#endif
-*q = '\0';
/*
* Append the closing parens of the filter, unless doing so would
@@ -305,23 +308,24 @@ static const char* authn_ldap_xlate_password(reque
*/
if (nofilter) {
-if (q + 1 <= filtbuf_end) {
-strcat(filtbuf, ")");
+if (q + 1 >= filtbuf_end) { /* accounts for final \0 */
+rv = APR_EGENERAL;
+goto out;
}
-else {
-return APR_EGENERAL;
-}
+*q++ = ')';
}
else {
-if (q + 2 <= filtbuf_end) {
-strcat(filtbuf, "))");
+if (q + 2 >= filtbuf_end) { /* accounts for final \0 */
+rv = APR_EGENERAL;
+goto out;
}
-else {
-return APR_EGENERAL;
-}
+*q++ = ')';
+*q++ = ')';
}
-return APR_SUCCESS;
+out:
+*q = '\0';
+return rv;
}
static void *create_authnz_ldap_dir_config(apr_pool_t *p, char *d)
Re: svn commit: r1591012 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/aaa/mod_authnz_ldap.c
On Tue, Apr 29, 2014 at 6:06 PM wrote:
>
> Author: minfrin
> Date: Tue Apr 29 16:05:56 2014
> New Revision: 1591012
>
> URL: http://svn.apache.org/r1591012
> Log:
> mod_authnz_ldap: Fail explicitly when the filter is too long. Remove
> unnecessary apr_pstrdup() and strlen().
It seems that the escaping cases don't error out if filtbuf is not
large enough? (see below)
>
> --- httpd/httpd/trunk/modules/aaa/mod_authnz_ldap.c (original)
> +++ httpd/httpd/trunk/modules/aaa/mod_authnz_ldap.c Tue Apr 29 16:05:56 2014
[]
> @@ -205,31 +202,23 @@ static const char* authn_ldap_xlate_pass
> * search filter will be (&(posixid=*)(uid=userj)).
> */
> #define FILTER_LENGTH MAX_STRING_LEN
> -static void authn_ldap_build_filter(char *filtbuf,
> +static apr_status_t authn_ldap_build_filter(char *filtbuf,
> request_rec *r,
> - const char* sent_user,
> - const char* sent_filter,
> + const char *user,
> + const char *filter,
> authn_ldap_config_t *sec)
> {
[]
> @@ -264,7 +253,7 @@ static void authn_ldap_build_filter(char
> */
> filtbuf_end = filtbuf + FILTER_LENGTH - 1;
> #if APR_HAS_MICROSOFT_LDAPSDK
> -for (p = user, q=filtbuf + strlen(filtbuf);
> +for (p = user, q=filtbuf + len;
> *p && q < filtbuf_end; ) {
> if (strchr("*()\\", *p) != NULL) {
> if ( q + 3 >= filtbuf_end)
Here we break the loop and fall through with no error.
> @@ -294,7 +283,7 @@ static void authn_ldap_build_filter(char
> *q++ = *p++;
> }
> #else
> -for (p = user, q=filtbuf + strlen(filtbuf);
> +for (p = user, q=filtbuf + len;
> *p && q < filtbuf_end; *q++ = *p++) {
> if (strchr("*()\\", *p) != NULL) {
> *q++ = '\\';
Next it's:
if (q >= filtbuf_end) {
break;
}
so same here, plus it's not consistent because for
APR_HAS_MICROSOFT_LDAPSDK in the previous hunk we break out before
'\\' while here it's after.
> @@ -312,14 +301,23 @@ static void authn_ldap_build_filter(char
> */
>
> if (nofilter) {
> -if (q + 1 <= filtbuf_end)
> +if (q + 1 <= filtbuf_end) {
> strcat(filtbuf, ")");
> +}
> +else {
> +return APR_EGENERAL;
> +}
> }
> else {
> -if (q + 2 <= filtbuf_end)
> +if (q + 2 <= filtbuf_end) {
> strcat(filtbuf, "))");
> +}
> +else {
> +return APR_EGENERAL;
> +}
> }
These final checks do not catch the escaping truncations either
because we might have left some room. Also the strcat()s look
inefficient since "q" points at the end of "filtbuf" already.
>
> +return APR_SUCCESS;
> }
All in all, I'd rewrite this function like in the attached patch (not
even compile tested, just to show what I'm talking about..).
Regards;
Yann.
Index: modules/aaa/mod_authnz_ldap.c
===
--- modules/aaa/mod_authnz_ldap.c (revision 1913813)
+++ modules/aaa/mod_authnz_ldap.c (working copy)
@@ -206,7 +206,7 @@ static const char* authn_ldap_xlate_password(reque
* search filter will be (&(posixid=*)(uid=userj)).
*/
#define FILTER_LENGTH MAX_STRING_LEN
-static apr_status_t authn_ldap_build_filter(char *filtbuf,
+static apr_status_t authn_ldap_build_filter(char filtbuf[FILTER_LENGTH],
request_rec *r,
const char *user,
const char *filter,
@@ -244,7 +244,7 @@ static const char* authn_ldap_xlate_password(reque
* config-supplied portions.
*/
-if ((nofilter = (filter && !strcasecmp(filter, "none" {
+if ((nofilter = (!filter || !*filter || !strcasecmp(filter, "none" {
len = apr_snprintf(filtbuf, FILTER_LENGTH, "(%s=", sec->attribute);
}
else {
@@ -256,12 +256,12 @@ static const char* authn_ldap_xlate_password(reque
* LDAP filter metachars are escaped.
*/
filtbuf_end = filtbuf + FILTER_LENGTH - 1;
+for (p = user, q = filtbuf + len; *p; ) {
+if (strchr("*()\\", *p) != NULL) {
#if APR_HAS_MICROSOFT_LDAPSDK
-for (p = user, q=filtbuf + len;
- *p && q < filtbuf_end; ) {
-if (strchr("*()\\", *p) != NULL) {
-if ( q + 3 >= filtbuf_end)
- break; /* Don't write part of escape sequence if we can't write all of it */
+if (q + 3 >= filtbuf_end) { /* accounts for final \0 */
+return APR_EGENERAL;
+}
*q++ = '\\';
switch ( *p++ )
{
@@ -281,23 +281,22 @@ static const char* authn_ldap_xlate_password(reque
*q++ = '5';
*q++ = 'c';
break;
-}
-}
-else
-*q++ = *p++;
-}
Re: svn commit: r1913936 - /httpd/httpd/branches/2.4.x/STATUS
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: r1913936 - /httpd/httpd/branches/2.4.x/STATUS
On Sat, Nov 18, 2023 at 3:43 PM Yann Ylavic wrote: > > On Sat, Nov 18, 2023 at 3:17 PM wrote: > > > > Author: minfrin > > Date: Sat Nov 18 14:17:11 2023 > > New Revision: 1913936 > > > > URL: http://svn.apache.org/viewvc?rev=1913936=rev > > Log: > > Update vote, note need for mmn bump. > > > > Modified: > > httpd/httpd/branches/2.4.x/STATUS > > > > Modified: httpd/httpd/branches/2.4.x/STATUS > > URL: > > http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1913936=1913935=1913936=diff > > == > > --- httpd/httpd/branches/2.4.x/STATUS (original) > > +++ httpd/httpd/branches/2.4.x/STATUS Sat Nov 18 14:17:11 2023 > > @@ -174,10 +174,13 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK: > >*) mod_proxy: prevent bad busy values and problems with bybusyness > > Trunk version of patch: > > https://svn.apache.org/r1912245 > > + https://svn.apache.org/r1913930 > > svn merge -c 1912245 ^/httpd/httpd/trunk . > > - +1: jfclere, minfrin > > +svn merge -c 1913930 ^/httpd/httpd/trunk . > > + +1: minfrin, ylavic > > ylavic: +1 with r1913930, we can't provide exported functions w/o an > > ap_[proxy_] prefix. > > + minfrin: r1913930 added, votes reset, mmn bump needed > > 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 meant DSOs here.. > > > Regards; > Yann.
Re: svn commit: r1913936 - /httpd/httpd/branches/2.4.x/STATUS
On Sat, Nov 18, 2023 at 3:17 PM wrote: > > Author: minfrin > Date: Sat Nov 18 14:17:11 2023 > New Revision: 1913936 > > URL: http://svn.apache.org/viewvc?rev=1913936=rev > Log: > Update vote, note need for mmn bump. > > Modified: > httpd/httpd/branches/2.4.x/STATUS > > Modified: httpd/httpd/branches/2.4.x/STATUS > URL: > http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1913936=1913935=1913936=diff > == > --- httpd/httpd/branches/2.4.x/STATUS (original) > +++ httpd/httpd/branches/2.4.x/STATUS Sat Nov 18 14:17:11 2023 > @@ -174,10 +174,13 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK: >*) mod_proxy: prevent bad busy values and problems with bybusyness > Trunk version of patch: > https://svn.apache.org/r1912245 > + https://svn.apache.org/r1913930 > svn merge -c 1912245 ^/httpd/httpd/trunk . > - +1: jfclere, minfrin > +svn merge -c 1913930 ^/httpd/httpd/trunk . > + +1: minfrin, ylavic > ylavic: +1 with r1913930, we can't provide exported functions w/o an > ap_[proxy_] prefix. > + minfrin: r1913930 added, votes reset, mmn bump needed 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? Regards; Yann.
