diff --git a/changes-entries/enablereuse.txt b/changes-entries/enablereuse.txt new file mode 100644 index 0000000000..b8e1a3f617 --- /dev/null +++ b/changes-entries/enablereuse.txt @@ -0,0 +1,3 @@ + *) mod_proxy: Ignore (and warn about) enablereuse=on for ProxyPassMatch when + some dollar substitution (backreference) happens in the hostname or port + part of the URL. [Yann Ylavic] diff --git a/docs/manual/mod/mod_proxy.xml b/docs/manual/mod/mod_proxy.xml index a440f99ccc..811c242d3f 100644 --- a/docs/manual/mod/mod_proxy.xml +++ b/docs/manual/mod/mod_proxy.xml @@ -1511,6 +1511,9 @@ ProxyPassReverse "/mirror/foo/" "https://backend.example.com/" server configvirtual host directory +Since 2.4.47 the key=value Parameters are honored +when the url parameter contains backreference(s) (see note below). +

This directive is equivalent to ProxyPass @@ -1532,20 +1535,7 @@ ProxyPassMatch "^/(.*\.gif)$" "http://backend.example.com/$1"

will cause a local request for http://example.com/foo/bar.gif to be internally converted into a proxy request to http://backend.example.com/foo/bar.gif.

- Note -

The URL argument must be parsable as a URL before regexp - substitutions (as well as after). This limits the matches you can use. - For instance, if we had used

- -ProxyPassMatch "^(/.*\.gif)$" "http://backend.example.com:8000$1" - -

in our previous example, it would fail with a syntax error - at server startup. This is a bug (PR 46665 in the ASF bugzilla), - and the workaround is to reformulate the match:

- -ProxyPassMatch "^/(.*\.gif)$" "http://backend.example.com:8000/$1" - -
+

The ! directive is useful in situations where you don't want to reverse-proxy a subdirectory.

@@ -1564,6 +1554,20 @@ ProxyPassMatch "^/(.*\.gif)$" "http://backend.example.com:8000/$1" expression, the original URL will be appended to the URL parameter.

+ + <var><code>key=value</code> Parameters versus <var>url</var> with backreference(s) +

Since Apache HTTP Server 2.4.47, the key=value Parameters + are no longer ignored in a ProxyPassMatch using + an url with backreference(s). However to keep the existing + behavior regarding reuse/keepalive of backend connections (which were + never reused before for these URLs), the parameter enablereuse + (or disablereuse) default to off (resp. on) + in this case. Setting enablereuse=on explicitely allows to + reuse connections unless some backreference(s) belong in + the authority part (hostname and/or port) of the url + (this condition is enforced since Apache HTTP Server 2.4.55, and produces + a warning at startup because these URLs are not reusable per se).

+
Security Warning diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index 434e37c9d4..e4d9df9c48 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -1844,6 +1844,7 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(apr_pool_t *p, proxy_worker_shared *wshared; const char *ptr = NULL, *sockpath = NULL, *pdollars = NULL; apr_port_t port_of_scheme; + int disable_reuse = 0; apr_uri_t uri; /* @@ -1872,12 +1873,21 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(apr_pool_t *p, * to fail (e.g. "ProxyPassMatch ^/(a|b)(/.*)? http://host:port$2"). * So we trim all the $n from the :port and prepend them in uri.path * afterward for apr_uri_unparse() to restore the original URL below. + * If a dollar substitution is found in the hostname[:port] part of + * the URL, reusing address and connections in the same worker is not + * possible (the current implementation of active connections cache + * handles/assumes a single origin server:port per worker only), so + * we set disable_reuse here during parsing to take that into account + * in the worker settings below. */ #define IS_REF(x) (x[0] == '$' && apr_isdigit(x[1])) const char *pos = ap_strstr_c(ptr, "://"); if (pos) { pos += 3; while (*pos && *pos != ':' && *pos != '/') { + if (*pos == '$') { + disable_reuse = 1; + } pos++; } if (*pos == ':') { @@ -1897,6 +1907,7 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(apr_pool_t *p, vec[1].iov_base = (void *)path; vec[1].iov_len = strlen(path); ptr = apr_pstrcatv(p, vec, 2, NULL); + disable_reuse = 1; } } } @@ -1987,7 +1998,7 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(apr_pool_t *p, wshared->port = (uri.port) ? uri.port : port_of_scheme; wshared->flush_packets = flush_off; wshared->flush_wait = PROXY_FLUSH_WAIT; - wshared->is_address_reusable = 1; + wshared->is_address_reusable = !disable_reuse; wshared->lbfactor = 100; wshared->passes = 1; wshared->fails = 1; @@ -1996,7 +2007,31 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(apr_pool_t *p, 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 = (mask & AP_PROXY_WORKER_IS_MALLOCED) != 0; - wshared->is_name_matchable = 0; + if (mask & AP_PROXY_WORKER_IS_MATCH) { + wshared->is_name_matchable = 1; + + /* Before AP_PROXY_WORKER_IS_MATCH (< 2.4.47), a regex worker with + * dollar substitution was never matched against any actual URL, thus + * the requests fell through the generic worker. Now if a ProyPassMatch + * matches, a worker (and its parameters) is always used to determine + * the properties of the connection with the origin server. So for + * instance the same "timeout=" will be enforced for all the requests + * matched by the same ProyPassMatch worker, which is an improvement + * compared to the global/vhost [Proxy]Timeout applied by the generic + * worker. Likewise, address and connection reuse is the default for + * a ProyPassMatch worker with no dollar substitution, just like a + * "normal" worker. However to avoid DNS and connection reuse compat + * issues, connection reuse is disabled by default if there is any + * substitution in the uri-path (an explicit enablereuse=on can still + * opt-in), and reuse is even disabled definitively for substitutions + * happening in the hostname[:port] (disable_reuse was set above so + * address reuse is also disabled which will prevent enablereuse=on + * to apply anyway). + */ + if (disable_reuse || ap_strchr_c(wshared->name, '$')) { + wshared->disablereuse = 1; + } + } if (sockpath) { if (PROXY_STRNCPY(wshared->uds_path, sockpath) != APR_SUCCESS) { return apr_psprintf(p, "worker uds path (%s) too long", sockpath); @@ -2016,20 +2051,6 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(apr_pool_t *p, (*worker)->balancer = balancer; (*worker)->s = wshared; - if (mask & AP_PROXY_WORKER_IS_MATCH) { - (*worker)->s->is_name_matchable = 1; - if (ap_strchr_c((*worker)->s->name, '$')) { - /* Before AP_PROXY_WORKER_IS_MATCH (< 2.4.47), 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; } @@ -2115,12 +2136,22 @@ PROXY_DECLARE(apr_status_t) ap_proxy_initialize_worker(proxy_worker *worker, ser if (!worker->s->retry_set) { worker->s->retry = apr_time_from_sec(PROXY_WORKER_DEFAULT_RETRY); } - /* By default address is reusable unless DisableReuse is set */ + /* Consistently set address and connection reusabilty: when reuse + * is disabled by configuration, or when the address is known already + * to not be reusable for this worker (in any case, thus ignore/force + * DisableReuse). + */ if (worker->s->disablereuse) { worker->s->is_address_reusable = 0; } - else { - worker->s->is_address_reusable = 1; + else if (!worker->s->is_address_reusable) { + /* Explicit enablereuse=on can't work in this case, warn user. */ + if (worker->s->disablereuse_set && !worker->s->disablereuse) { + ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, APLOGNO(10400) + "enablereuse/disablereuse ignored for worker %s", + ap_proxy_worker_name(p, worker)); + } + worker->s->disablereuse = 1; } /*