Mailing List Archive

Author: jim
Date: Tue Aug 11 12:05:11 2020
New Revision: 1880772

URL: http://svn.apache.org/viewvc?rev=1880772&view=rev
Log:
Merge r1609680, r1609688, r1641381, r1826289, r1826313, r1878467, r1878994, r1879000, r1879001, r1879002, r1879361 from trunk:

mod_proxy: add ap_proxy_define_match_worker() and use it for ProxyPassMatch
and ProxyMatch section to distinguish between normal workers and workers
with regex substitutions in the name. Implement handling of such workers
in ap_proxy_get_worker(). PR 43513


mod_proxy: better check for worker->s->is_name_matchable


Return a match whenever we get to the end of the worker name, regardless
of whether there is URL left.

ProxyPassMatch had been using the default worker in trunk.


Follow up to r1609680: simpler/faster ap_proxy_strcmp_ematch().

No functional change.


Follow up to r1609680: further simplify/optimize ap_proxy_strcmp_ematch().

While at it, same treatment for its mother ap_strcmp_match().


make sure the $n of the regular expressions is not included the name of the worker.
for example, the example:
ProxyPassMatch "^(/.*\.gif)$" "http://backend.example.com:8000$1"
was giving:
AH00526: Syntax error on line nnn of bla/conf/httpd.conf:
ProxyPass Unable to parse URL: http://backend.example.com:8000$1



ap_proxy_define_match_worker: don't copy the url unnecessarily.

And save a few cycles, when the duplication is needed, by not copying
the ignored part.


ap_proxy_define_match_worker: disable connection reuse by default.

To avoid compat issues with dns/connection reuse now that a worker with
dollar substitution can be elected.


CHANGES entry for ap_proxy_define_match_worker().

Oups, axe spurious copypasta.

mod_proxy: unfail mixed ProxyPass/<Proxy> and ProxyPassMatch/<ProxyMatch>.

It is not a failure in current 2.4.x, so to ease backport and to avoid compat
breakage simply warn about the second directive being ignored.

This commit can be reverted in trunk if we want next versions to fail in this
case.


[Reverted by r1879363]

Submitted by: jkaluza, covener, ylavic, ylavic, jfclere, ylavic, ylavic, ylavic, ylavic, ylavic
Reviewed by: ylavic, minfrin (with an MMN bump), jim (agree w/ minfrin)

Modified:
httpd/httpd/branches/2.4.x/ (props changed)
httpd/httpd/branches/2.4.x/CHANGES
httpd/httpd/branches/2.4.x/STATUS
httpd/httpd/branches/2.4.x/include/ap_mmn.h
httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c
httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.h
httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c
httpd/httpd/branches/2.4.x/server/util.c

Propchange: httpd/httpd/branches/2.4.x/
------------------------------------------------------------------------------
Merged /httpd/httpd/trunk:r1609680,1609688,1641381,1826289,1826313,1878467,1878994,1879000-1879002,1879361

Modified: httpd/httpd/branches/2.4.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1880772&r1=1880771&r2=1880772&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Tue Aug 11 12:05:11 2020
@@ -1,6 +1,11 @@
-*- coding: utf-8 -*-
Changes with Apache 2.4.47

+ *) mod_proxy: recognize parameters from ProxyPassMatch workers with dollar
+ substitution, such that they apply to the backend connection. Note that
+ connection reuse is disabled by default to avoid compatibility issues.
+ [Takashi Sato, Jan Kaluza, Eric Covener, Yann Ylavic, Jean-Frederic Clere]
+
Changes with Apache 2.4.46

*) SECURITY: CVE-2020-11984 (cve.mitre.org)

Modified: httpd/httpd/branches/2.4.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1880772&r1=1880771&r2=1880772&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/STATUS (original)
+++ httpd/httpd/branches/2.4.x/STATUS Tue Aug 11 12:05:11 2020
@@ -138,25 +138,6 @@ RELEASE SHOWSTOPPERS:
PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
[ start all new proposals below, under PATCHES PROPOSED. ]

- *) mod_proxy: Add ap_proxy_define_match_worker() and use it for ProxyPassMatch
- and ProxyMatch section to distinguish between normal workers and workers
- with regex substitutions in the name. Implement handling of such workers
- in ap_proxy_get_worker(). Fixes the bug when regex workers were not
- matched and used for request. PR 43513 and 64537.
- trunk patch: http://svn.apache.org/r1609680
- http://svn.apache.org/r1609688
- http://svn.apache.org/r1641381
- http://svn.apache.org/r1826289
- http://svn.apache.org/r1826313
- http://svn.apache.org/r1878467
- http://svn.apache.org/r1878994
- http://svn.apache.org/r1879000
- http://svn.apache.org/r1879001
- http://svn.apache.org/r1879002
- http://svn.apache.org/r1879361
- 2.4.x patch: http://people.apache.org/~ylavic/patches/httpd-2.4.x-ap_proxy_define_match_worker-v6.patch
- +1: ylavic, minfrin (with an MMN bump), jim (agree w/ minfrin)
-
*) mod_proxy_uwsgi: Avoid NULL pointer dereferences for empty environment variable values
http://svn.apache.org/r1879878
Backport version for 2.4.x of patch:

Modified: httpd/httpd/branches/2.4.x/include/ap_mmn.h
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/include/ap_mmn.h?rev=1880772&r1=1880771&r2=1880772&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/include/ap_mmn.h (original)
+++ httpd/httpd/branches/2.4.x/include/ap_mmn.h Tue Aug 11 12:05:11 2020
@@ -535,6 +535,7 @@
* 20120211.91 (2.4.42-dev) Add ap_is_chunked() in httpd.h
* 20120211.92 (2.4.42-dev) AP_REG_NO_DEFAULT macro in ap_regex.h
* 20120211.93 (2.4.44-dev) Add ap_parse_strict_length()
+ * 20120211.94 (2.4.47-dev) Add ap_proxy_define_match_worker()
*/

#define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
@@ -542,7 +543,7 @@
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 20120211
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 93 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 94 /* 0...n */

/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a

Modified: httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c?rev=1880772&r1=1880771&r2=1880772&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c (original)
+++ httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c Tue Aug 11 12:05:11 2020
@@ -1864,15 +1864,41 @@ static const char *
new->balancer = balancer;
}
else {
- proxy_worker *worker = ap_proxy_get_worker(cmd->temp_pool, NULL, conf, ap_proxy_de_socketfy(cmd->pool, r));
+ proxy_worker *worker = ap_proxy_get_worker(cmd->temp_pool, NULL, conf, new->real);
int reuse = 0;
if (!worker) {
- const char *err = ap_proxy_define_worker(cmd->pool, &worker, NULL, conf, r, 0);
+ const char *err;
+ if (use_regex) {
+ err = ap_proxy_define_match_worker(cmd->pool, &worker, NULL,
+ conf, r, 0);
+ }
+ else {
+ err = ap_proxy_define_worker(cmd->pool, &worker, NULL,
+ conf, r, 0);
+ }
if (err)
return apr_pstrcat(cmd->temp_pool, "ProxyPass ", err, NULL);

PROXY_COPY_CONF_PARAMS(worker, conf);
- } else {
+ }
+ else if ((use_regex != 0) ^ (worker->s->is_name_matchable != 0)) {
+ ap_log_error(APLOG_MARK, APLOG_WARNING, 0, cmd->server, APLOGNO(10249)
+ "ProxyPass/<Proxy> and ProxyPassMatch/<ProxyMatch> "
+ "can't be used altogether with the same worker "
+ "name (%s); ignoring ProxyPass%s",
+ worker->s->name, use_regex ? "Match" : "");
+ /* Rollback new alias */
+ if (cmd->path) {
+ dconf->alias = NULL;
+ dconf->alias_set = 0;
+ }
+ else {
+ memset(new, 0, sizeof(*new));
+ apr_array_pop(conf->aliases);
+ }
+ return NULL;
+ }
+ else {
reuse = 1;
ap_log_error(APLOG_MARK, APLOG_INFO, 0, cmd->server, APLOGNO(01145)
"Sharing worker '%s' instead of creating new worker '%s'",
@@ -2499,6 +2525,7 @@ static const char *proxysection(cmd_parm
char *word, *val;
proxy_balancer *balancer = NULL;
proxy_worker *worker = NULL;
+ int use_regex = 0;

const char *err = ap_check_cmd_context(cmd, NOT_IN_DIR_CONTEXT);
proxy_server_conf *sconf =
@@ -2537,6 +2564,7 @@ static const char *proxysection(cmd_parm
if (!r) {
return "Regex could not be compiled";
}
+ use_regex = 1;
}

/* initialize our config and fetch it */
@@ -2586,12 +2614,29 @@ static const char *proxysection(cmd_parm
worker = ap_proxy_get_worker(cmd->temp_pool, NULL, sconf,
ap_proxy_de_socketfy(cmd->temp_pool, (char*)conf->p));
if (!worker) {
- err = ap_proxy_define_worker(cmd->pool, &worker, NULL,
- sconf, conf->p, 0);
+ if (use_regex) {
+ err = ap_proxy_define_match_worker(cmd->pool, &worker, NULL,
+ sconf, conf->p, 0);
+ }
+ else {
+ err = ap_proxy_define_worker(cmd->pool, &worker, NULL,
+ sconf, conf->p, 0);
+ }
if (err)
return apr_pstrcat(cmd->temp_pool, thiscmd->name,
" ", err, NULL);
}
+ else if ((use_regex != 0) ^ (worker->s->is_name_matchable != 0)) {
+ ap_log_error(APLOG_MARK, APLOG_WARNING, 0, cmd->server, APLOGNO(10250)
+ "ProxyPass/<Proxy> and ProxyPassMatch/<ProxyMatch> "
+ "can't be used altogether with the same worker "
+ "name (%s); ignoring <Proxy%s>",
+ worker->s->name, use_regex ? "Match" : "");
+ /* Rollback new section */
+ ((void **)sconf->sec_proxy->elts)[sconf->sec_proxy->nelts - 1] = NULL;
+ apr_array_pop(sconf->sec_proxy);
+ goto cleanup;
+ }
if (!worker->section_config) {
worker->section_config = new_dir_conf;
}
@@ -2621,6 +2666,7 @@ static const char *proxysection(cmd_parm
}
}

+cleanup:
cmd->path = old_path;
cmd->override = old_overrides;


Modified: httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.h
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.h?rev=1880772&r1=1880771&r2=1880772&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.h (original)
+++ httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.h Tue Aug 11 12:05:11 2020
@@ -453,6 +453,7 @@ typedef struct {
unsigned int keepalive_set:1;
unsigned int disablereuse_set:1;
unsigned int was_malloced:1;
+ unsigned int is_name_matchable:1;
char hcuri[PROXY_WORKER_MAX_ROUTE_SIZE]; /* health check uri */
char hcexpr[PROXY_WORKER_MAX_SCHEME_SIZE]; /* name of condition expr for health check */
int passes; /* number of successes for check to pass */
@@ -750,6 +751,24 @@ PROXY_DECLARE(char *) ap_proxy_define_wo
proxy_worker **worker,
proxy_balancer *balancer,
proxy_server_conf *conf,
+ const char *url,
+ int do_malloc);
+
+/**
+ * Define and Allocate space for the ap_strcmp_match()able worker to proxy
+ * configuration.
+ * @param p memory pool to allocate worker from
+ * @param worker the new worker
+ * @param balancer the balancer that the worker belongs to
+ * @param conf current proxy server configuration
+ * @param url url containing worker name (produces match pattern)
+ * @param do_malloc true if shared struct should be malloced
+ * @return error message or NULL if successful (*worker is new worker)
+ */
+PROXY_DECLARE(char *) ap_proxy_define_match_worker(apr_pool_t *p,
+ proxy_worker **worker,
+ proxy_balancer *balancer,
+ proxy_server_conf *conf,
const char *url,
int do_malloc);


Modified: httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c?rev=1880772&r1=1880771&r2=1880772&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c Tue Aug 11 12:05:11 2020
@@ -1658,6 +1658,46 @@ PROXY_DECLARE(char *) ap_proxy_worker_na
return apr_pstrcat(p, "unix:", worker->s->uds_path, "|", worker->s->name, NULL);
}

+/*
+ * Taken from ap_strcmp_match() :
+ * Match = 0, NoMatch = 1, Abort = -1, Inval = -2
+ * Based loosely on sections of wildmat.c by Rich Salz
+ * Hmmm... shouldn't this really go component by component?
+ *
+ * Adds handling of the "\<any>" => "<any>" unescaping.
+ */
+static int ap_proxy_strcmp_ematch(const char *str, const char *expected)
+{
+ apr_size_t x, y;
+
+ for (x = 0, y = 0; expected[y]; ++y, ++x) {
+ if (expected[y] == '$' && apr_isdigit(expected[y + 1])) {
+ do {
+ y += 2;
+ } while (expected[y] == '$' && apr_isdigit(expected[y + 1]));
+ if (!expected[y])
+ return 0;
+ while (str[x]) {
+ int ret;
+ if ((ret = ap_proxy_strcmp_ematch(&str[x++], &expected[y])) != 1)
+ return ret;
+ }
+ return -1;
+ }
+ else if (!str[x]) {
+ return -1;
+ }
+ else if (expected[y] == '\\' && !expected[++y]) {
+ /* NUL is an invalid char! */
+ return -2;
+ }
+ if (str[x] != expected[y])
+ return 1;
+ }
+ /* We got all the way through the worker path without a difference */
+ return 0;
+}
+
PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
proxy_balancer *balancer,
proxy_server_conf *conf,
@@ -1720,11 +1760,15 @@ PROXY_DECLARE(proxy_worker *) ap_proxy_g
if ( ((worker_name_length = strlen(worker->s->name)) <= url_length)
&& (worker_name_length >= min_match)
&& (worker_name_length > max_match)
- && (strncmp(url_copy, worker->s->name, worker_name_length) == 0) ) {
+ && (worker->s->is_name_matchable
+ || strncmp(url_copy, worker->s->name,
+ worker_name_length) == 0)
+ && (!worker->s->is_name_matchable
+ || ap_proxy_strcmp_ematch(url_copy,
+ worker->s->name) == 0) ) {
max_worker = worker;
max_match = worker_name_length;
}
-
}
} else {
worker = (proxy_worker *)conf->workers->elts;
@@ -1732,7 +1776,12 @@ PROXY_DECLARE(proxy_worker *) ap_proxy_g
if ( ((worker_name_length = strlen(worker->s->name)) <= url_length)
&& (worker_name_length >= min_match)
&& (worker_name_length > max_match)
- && (strncmp(url_copy, worker->s->name, worker_name_length) == 0) ) {
+ && (worker->s->is_name_matchable
+ || strncmp(url_copy, worker->s->name,
+ worker_name_length) == 0)
+ && (!worker->s->is_name_matchable
+ || ap_proxy_strcmp_ematch(url_copy,
+ worker->s->name) == 0) ) {
max_worker = worker;
max_match = worker_name_length;
}
@@ -1866,6 +1915,7 @@ PROXY_DECLARE(char *) ap_proxy_define_wo
wshared->hash.def = ap_proxy_hashfunc(wshared->name, PROXY_HASHFUNC_DEFAULT);
wshared->hash.fnv = ap_proxy_hashfunc(wshared->name, PROXY_HASHFUNC_FNV);
wshared->was_malloced = (do_malloc != 0);
+ wshared->is_name_matchable = 0;
if (sockpath) {
if (PROXY_STRNCPY(wshared->uds_path, sockpath) != APR_SUCCESS) {
return apr_psprintf(p, "worker uds path (%s) too long", sockpath);
@@ -1888,6 +1938,38 @@ PROXY_DECLARE(char *) ap_proxy_define_wo
return NULL;
}

+PROXY_DECLARE(char *) ap_proxy_define_match_worker(apr_pool_t *p,
+ proxy_worker **worker,
+ proxy_balancer *balancer,
+ proxy_server_conf *conf,
+ const char *url,
+ int do_malloc)
+{
+ char *err;
+ const char *pdollar = ap_strchr_c(url, '$');
+
+ if (pdollar != NULL) {
+ url = apr_pstrmemdup(p, url, pdollar - url);
+ }
+ err = ap_proxy_define_worker(p, worker, balancer, conf, url, do_malloc);
+ if (err) {
+ return err;
+ }
+
+ (*worker)->s->is_name_matchable = 1;
+ if (pdollar) {
+ /* Before ap_proxy_define_match_worker() existed, a regex worker
+ * with dollar substitution was never matched against the actual
+ * URL thus the request fell through the generic worker. To avoid
+ * dns and connection reuse compat issues, let's disable connection
+ * reuse by default, it can still be overwritten by an explicit
+ * enablereuse=on.
+ */
+ (*worker)->s->disablereuse = 1;
+ }
+ return NULL;
+}
+
/*
* Create an already defined worker and free up memory
*/

Modified: httpd/httpd/branches/2.4.x/server/util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/server/util.c?rev=1880772&r1=1880771&r2=1880772&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/server/util.c (original)
+++ httpd/httpd/branches/2.4.x/server/util.c Tue Aug 11 12:05:11 2020
@@ -188,8 +188,6 @@ AP_DECLARE(int) ap_strcmp_match(const ch
int x, y;

for (x = 0, y = 0; expected[y]; ++y, ++x) {
- if ((!str[x]) && (expected[y] != '*'))
- return -1;
if (expected[y] == '*') {
while (expected[++y] == '*');
if (!expected[y])
@@ -201,6 +199,8 @@ AP_DECLARE(int) ap_strcmp_match(const ch
}
return -1;
}
+ else if (!str[x])
+ return -1;
else if ((expected[y] != '?') && (str[x] != expected[y]))
return 1;
}