Re: friends of mod_proxy
+1 > On Oct 11, 2022, at 3:47 AM, Stefan Eissing via dev > wrote: > > > >> Am 10.10.2022 um 20:39 schrieb Ruediger Pluem : >> >> >> >> On 10/10/22 6:35 PM, Eric Covener wrote: >>> On Mon, Oct 10, 2022 at 11:55 AM Yann Ylavic wrote: On Fri, Oct 7, 2022 at 9:14 PM Ruediger Pluem wrote: > > On 10/7/22 7:11 PM, Stefan Eissing via dev wrote: >> >> >>> Am 07.10.2022 um 18:45 schrieb Yann Ylavic : >>> >> >> Thanks, Yann, for the detailed explanation on how this works and that >> the default does the right thing. > > +1. I missed the default of not reusing the connection in these cases but > having the possibility to override it like the user did. > But shouldn't we default to not reusing the connection in case of a $ > inside the port as well? We default to disabling connection reuse for any $ substitution already, the compat issue is for an explicit enablereuse=on which used to "work" because it was ignored.. How about a patch like the attached one now, which disables connection reuse definitively (regardless of enablereuse) if there is a $ substitution in the hostname or port part of the URL? The patch uses the existing "is_address_reusable" flag (set to false initially in this case), and since it's not configurable by the user we are safe from a connection reuse point of vue in any case. >>> >>> +1 in concept >>> >> >> Looks sensible. +1. > > I like the patch. +1 > > Cheers, > Stefan
Re: friends of mod_proxy
On Tue, Oct 11, 2022 at 9:47 AM Stefan Eissing via dev wrote: > > > Am 10.10.2022 um 20:39 schrieb Ruediger Pluem : > > > > On 10/10/22 6:35 PM, Eric Covener wrote: > >> On Mon, Oct 10, 2022 at 11:55 AM Yann Ylavic wrote: > >>> > >>> How about a patch like the attached one now, which disables connection > >>> reuse definitively (regardless of enablereuse) if there is a $ > >>> substitution in the hostname or port part of the URL? > >>> The patch uses the existing "is_address_reusable" flag (set to false > >>> initially in this case), and since it's not configurable by the user > >>> we are safe from a connection reuse point of vue in any case. > >> > >> +1 in concept > > > > Looks sensible. +1. > > I like the patch. +1 Thanks, r1904513. Cheers; Yann.
Re: friends of mod_proxy
> Am 10.10.2022 um 20:39 schrieb Ruediger Pluem : > > > > On 10/10/22 6:35 PM, Eric Covener wrote: >> On Mon, Oct 10, 2022 at 11:55 AM Yann Ylavic wrote: >>> >>> On Fri, Oct 7, 2022 at 9:14 PM Ruediger Pluem wrote: On 10/7/22 7:11 PM, Stefan Eissing via dev wrote: > > >> Am 07.10.2022 um 18:45 schrieb Yann Ylavic : >> > > Thanks, Yann, for the detailed explanation on how this works and that the > default does the right thing. +1. I missed the default of not reusing the connection in these cases but having the possibility to override it like the user did. But shouldn't we default to not reusing the connection in case of a $ inside the port as well? >>> >>> We default to disabling connection reuse for any $ substitution >>> already, the compat issue is for an explicit enablereuse=on which used >>> to "work" because it was ignored.. >>> >>> How about a patch like the attached one now, which disables connection >>> reuse definitively (regardless of enablereuse) if there is a $ >>> substitution in the hostname or port part of the URL? >>> The patch uses the existing "is_address_reusable" flag (set to false >>> initially in this case), and since it's not configurable by the user >>> we are safe from a connection reuse point of vue in any case. >> >> +1 in concept >> > > Looks sensible. +1. I like the patch. +1 Cheers, Stefan
Re: friends of mod_proxy
On 10/10/22 6:35 PM, Eric Covener wrote: > On Mon, Oct 10, 2022 at 11:55 AM Yann Ylavic wrote: >> >> On Fri, Oct 7, 2022 at 9:14 PM Ruediger Pluem wrote: >>> >>> On 10/7/22 7:11 PM, Stefan Eissing via dev wrote: > Am 07.10.2022 um 18:45 schrieb Yann Ylavic : > Thanks, Yann, for the detailed explanation on how this works and that the default does the right thing. >>> >>> +1. I missed the default of not reusing the connection in these cases but >>> having the possibility to override it like the user did. >>> But shouldn't we default to not reusing the connection in case of a $ >>> inside the port as well? >> >> We default to disabling connection reuse for any $ substitution >> already, the compat issue is for an explicit enablereuse=on which used >> to "work" because it was ignored.. >> >> How about a patch like the attached one now, which disables connection >> reuse definitively (regardless of enablereuse) if there is a $ >> substitution in the hostname or port part of the URL? >> The patch uses the existing "is_address_reusable" flag (set to false >> initially in this case), and since it's not configurable by the user >> we are safe from a connection reuse point of vue in any case. > > +1 in concept > Looks sensible. +1. Regards Rüdiger
Re: friends of mod_proxy
On Mon, Oct 10, 2022 at 11:55 AM Yann Ylavic wrote: > > On Fri, Oct 7, 2022 at 9:14 PM Ruediger Pluem wrote: > > > > On 10/7/22 7:11 PM, Stefan Eissing via dev wrote: > > > > > > > > >> Am 07.10.2022 um 18:45 schrieb Yann Ylavic : > > >> > > > > > > Thanks, Yann, for the detailed explanation on how this works and that the > > > default does the right thing. > > > > +1. I missed the default of not reusing the connection in these cases but > > having the possibility to override it like the user did. > > But shouldn't we default to not reusing the connection in case of a $ > > inside the port as well? > > We default to disabling connection reuse for any $ substitution > already, the compat issue is for an explicit enablereuse=on which used > to "work" because it was ignored.. > > How about a patch like the attached one now, which disables connection > reuse definitively (regardless of enablereuse) if there is a $ > substitution in the hostname or port part of the URL? > The patch uses the existing "is_address_reusable" flag (set to false > initially in this case), and since it's not configurable by the user > we are safe from a connection reuse point of vue in any case. +1 in concept
Re: friends of mod_proxy
On Fri, Oct 7, 2022 at 9:14 PM Ruediger Pluem wrote:
>
> On 10/7/22 7:11 PM, Stefan Eissing via dev wrote:
> >
> >
> >> Am 07.10.2022 um 18:45 schrieb Yann Ylavic :
> >>
> >
> > Thanks, Yann, for the detailed explanation on how this works and that the
> > default does the right thing.
>
> +1. I missed the default of not reusing the connection in these cases but
> having the possibility to override it like the user did.
> But shouldn't we default to not reusing the connection in case of a $ inside
> the port as well?
We default to disabling connection reuse for any $ substitution
already, the compat issue is for an explicit enablereuse=on which used
to "work" because it was ignored..
How about a patch like the attached one now, which disables connection
reuse definitively (regardless of enablereuse) if there is a $
substitution in the hostname or port part of the URL?
The patch uses the existing "is_address_reusable" flag (set to false
initially in this case), and since it's not configurable by the user
we are safe from a connection reuse point of vue in any case.
>
> >
> > My guess is that such configurations are pretty rare, as their security is
> > not good in general. The only thing that comes to mind is logging a warning
> > for such cases.
>
> +1
Done in the patch too, along with some docs update regarding this use case.
Regards;
Yann.
Index: modules/proxy/proxy_util.c
===
--- modules/proxy/proxy_util.c (revision 1903576)
+++ modules/proxy/proxy_util.c (working copy)
@@ -1851,6 +1851,7 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(ap
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;
/*
@@ -1879,6 +1880,12 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(ap
* 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, "://");
@@ -1885,6 +1892,9 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(ap
if (pos) {
pos += 3;
while (*pos && *pos != ':' && *pos != '/') {
+if (*pos == '$') {
+disable_reuse = 1;
+}
pos++;
}
if (*pos == ':') {
@@ -1904,6 +1914,7 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(ap
vec[1].iov_base = (void *)path;
vec[1].iov_len = strlen(path);
ptr = apr_pstrcatv(p, vec, 2, NULL);
+disable_reuse = 1;
}
}
}
@@ -1999,7 +2010,7 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(ap
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;
@@ -2008,7 +2019,31 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(ap
wshared->hash.def = ap_proxy_hashfunc(wshared->name_ex, PROXY_HASHFUNC_DEFAULT);
wshared->hash.fnv = ap_proxy_hashfunc(wshared->name_ex, 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.
Re: friends of mod_proxy
Yeah... The logic in not re-using a worker is that the pattern matching aspect made it both difficult and somewhat inconsistent due to the dynamic nature of the situation, hence the use of the "default" worker for these. We now have the functionality to handle this, but it is still a performance hit and in most cases enablereuse should be disabled. But we still make that an admin decision, and I think that's still the right move. Cheers! > On Oct 6, 2022, at 11:32 AM, Eric Covener wrote: > > On Thu, Oct 6, 2022 at 10:21 AM Stefan Eissing via dev > mailto:dev@httpd.apache.org>> wrote: >> >> Friends of mod_proxy, I have a question: >> >> In <https://github.com/icing/mod_h2/issues/235> someone reported wrong >> connection reuse for a config like: >> >> ProxyPassMatch ^/(prod|dev)/([-a-z0-9]+)/(.*)$ h2://$2.internal/$1/$2/$3 >> enablereuse=on keepalive=on >> >> Leaving aside the issue that such a pattern is insecure due to the client >> influencing the hostname, the fact remains that mod_proxy_http2 will use a >> previous connection, even if the matched hostname is different. I replicated >> that, using "just" ports in a test case: >> >> ProxyPassMatch ^/h2proxy/([0-9]+)/(.*)$ h2c://127.0.0.1:$1/$2 enablereuse=on >> keepalive=on >> >> Then >> 1. /h2proxy/5002/hello.py >> 2. /h2proxy/5004/hello.py >> >> results in 2) re-using the connection of 1). The log file says for 2): >> >> [proxy:debug] proxy_util.c(2538): AH00942: H2C: has acquired connection for >> (127.0.0.1:80) >> [proxy:debug] proxy_util.c(2596): [remote 127.0.0.1:60121] AH00944: >> connecting h2c://127.0.0.1:5004/hello.py to 127.0.0.1:5004 >> [proxy:debug] proxy_util.c(2819): [remote 127.0.0.1:60121] AH00947: >> connected /hello.py to 127.0.0.1:5002 >> [proxy_http2:trace1] mod_proxy_http2.c(374): [remote 127.0.0.1:60121] H2: >> determined connect to 127.0.0.1:5002 >> [proxy:trace2] proxy_util.c(3101): H2C: reusing backend connection >> 127.0.0.1:60120<>127.0.0.1:5002 >> >> and that looks wrong. >> >> Question: do we have a bug or do we consider such ProxyPassMatch as broken >> and require at least enablereuse=off? > > I can't find all the history, but some time ago proxypassmatch never > tried to find a matching worker > https://bz.apache.org/bugzilla/show_bug.cgi?id=62167 > <https://bz.apache.org/bugzilla/show_bug.cgi?id=62167> > > Later, we must have gotten some logic added but you still have to > opt-in, I think it is likely > http://home.apache.org/~ylavic/patches/httpd-2.4.x-ap_proxy_define_match_worker.patch > > <http://home.apache.org/~ylavic/patches/httpd-2.4.x-ap_proxy_define_match_worker.patch> > or adjacent > > Not sure if it helps
Re: friends of mod_proxy
On 10/7/22 7:11 PM, Stefan Eissing via dev wrote: > > >> Am 07.10.2022 um 18:45 schrieb Yann Ylavic : >> > > Thanks, Yann, for the detailed explanation on how this works and that the > default does the right thing. +1. I missed the default of not reusing the connection in these cases but having the possibility to override it like the user did. But shouldn't we default to not reusing the connection in case of a $ inside the port as well? > > My guess is that such configurations are pretty rare, as their security is > not good in general. The only thing that comes to mind is logging a warning > for such cases. +1 Regards Rüdiger
Re: friends of mod_proxy
> Am 07.10.2022 um 18:45 schrieb Yann Ylavic : > > On Thu, Oct 6, 2022 at 5:32 PM Eric Covener wrote: >> >> On Thu, Oct 6, 2022 at 10:21 AM Stefan Eissing via dev >> wrote: >>> >>> Friends of mod_proxy, I have a question: >>> >>> In <https://github.com/icing/mod_h2/issues/235> someone reported wrong >>> connection reuse for a config like: >>> >>> ProxyPassMatch ^/(prod|dev)/([-a-z0-9]+)/(.*)$ h2://$2.internal/$1/$2/$3 >>> enablereuse=on keepalive=on >>> >>> Leaving aside the issue that such a pattern is insecure due to the client >>> influencing the hostname, the fact remains that mod_proxy_http2 will use a >>> previous connection, even if the matched hostname is different. I >>> replicated that, using "just" ports in a test case: >>> >>> ProxyPassMatch ^/h2proxy/([0-9]+)/(.*)$ h2c://127.0.0.1:$1/$2 >>> enablereuse=on keepalive=on >>> >>> Then >>> 1. /h2proxy/5002/hello.py >>> 2. /h2proxy/5004/hello.py >>> >>> results in 2) re-using the connection of 1). The log file says for 2): >>> >>> [proxy:debug] proxy_util.c(2538): AH00942: H2C: has acquired connection for >>> (127.0.0.1:80) >>> [proxy:debug] proxy_util.c(2596): [remote 127.0.0.1:60121] AH00944: >>> connecting h2c://127.0.0.1:5004/hello.py to 127.0.0.1:5004 >>> [proxy:debug] proxy_util.c(2819): [remote 127.0.0.1:60121] AH00947: >>> connected /hello.py to 127.0.0.1:5002 >>> [proxy_http2:trace1] mod_proxy_http2.c(374): [remote 127.0.0.1:60121] H2: >>> determined connect to 127.0.0.1:5002 >>> [proxy:trace2] proxy_util.c(3101): H2C: reusing backend connection >>> 127.0.0.1:60120<>127.0.0.1:5002 >>> >>> and that looks wrong. >>> >>> Question: do we have a bug or do we consider such ProxyPassMatch as broken >>> and require at least enablereuse=off? >> >> I can't find all the history, but some time ago proxypassmatch never >> tried to find a matching worker >> https://bz.apache.org/bugzilla/show_bug.cgi?id=62167 >> >> Later, we must have gotten some logic added but you still have to >> opt-in, I think it is likely >> http://home.apache.org/~ylavic/patches/httpd-2.4.x-ap_proxy_define_match_worker.patch >> or adjacent > > Yes, we have [1] that disables ProxyPassMatch workers reuse if they do > any $n substitution, precisely because reuse can't work in all cases > and particularly when the hostname[:port] part is substituted. > > ProxyPassMatch workers were introduced in 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. > > Before that there were no parameters (enablereuse, keepalive, timeout, > ...) applying to ProxyPassMatch connections, they fell back to the > generic "proxy:reverse" worker, all parameters were ignored and > connections never reused. > > Since 2.4.47 we have ProxyPassMatch workers > (worker->s->is_name_matchable == true), their name (match prefix) is > everything up to the first '$', so with "h2c://127.0.0.1:$1/$2" the > name is "h2c://127.0.0.1:", which obviously will match all the > possible ports to the same worker.. > > So enablereuse=on is now possible with ProxyPassMatch, and makes sense > for things like: > ProxyPassMatch ^/(prod|dev)/([-a-z0-9]+)/(.*)$ > h2://backend.internal/$2/$1/$3 enablereuse=on > but it should not be used when the substitution involves the > hostname[:port] (besides the security issue..). > > The issue here (IIUC) is that the user had the explicit enablereuse=on > before 2.4.47 already but it was ignored, and now it breaks :/ > Maybe we can do something to use the generic worker as before (and > ignore parameters) when we detect that the substitution happens in the > hostname[:port] part of the URL, but the damage is already done and > I'm not sure it's worth the change given how this kind of > configuration is not recommended in the first place. Thanks, Yann, for the detailed explanation on how this works and that the default does the right thing. My guess is that such configurations are pretty rare, as their security is not good in general. The only thing that comes to mind is logging a warning for such cases. Kind Regards, Stefan > Regards; > Yann. > > [1] > https://github.com/apache/httpd/blob/2.4.x/modules/proxy/proxy_util.c#L2031
Re: friends of mod_proxy
On Thu, Oct 6, 2022 at 5:32 PM Eric Covener wrote: > > On Thu, Oct 6, 2022 at 10:21 AM Stefan Eissing via dev > wrote: > > > > Friends of mod_proxy, I have a question: > > > > In <https://github.com/icing/mod_h2/issues/235> someone reported wrong > > connection reuse for a config like: > > > > ProxyPassMatch ^/(prod|dev)/([-a-z0-9]+)/(.*)$ h2://$2.internal/$1/$2/$3 > > enablereuse=on keepalive=on > > > > Leaving aside the issue that such a pattern is insecure due to the client > > influencing the hostname, the fact remains that mod_proxy_http2 will use a > > previous connection, even if the matched hostname is different. I > > replicated that, using "just" ports in a test case: > > > > ProxyPassMatch ^/h2proxy/([0-9]+)/(.*)$ h2c://127.0.0.1:$1/$2 > > enablereuse=on keepalive=on > > > > Then > > 1. /h2proxy/5002/hello.py > > 2. /h2proxy/5004/hello.py > > > > results in 2) re-using the connection of 1). The log file says for 2): > > > > [proxy:debug] proxy_util.c(2538): AH00942: H2C: has acquired connection for > > (127.0.0.1:80) > > [proxy:debug] proxy_util.c(2596): [remote 127.0.0.1:60121] AH00944: > > connecting h2c://127.0.0.1:5004/hello.py to 127.0.0.1:5004 > > [proxy:debug] proxy_util.c(2819): [remote 127.0.0.1:60121] AH00947: > > connected /hello.py to 127.0.0.1:5002 > > [proxy_http2:trace1] mod_proxy_http2.c(374): [remote 127.0.0.1:60121] H2: > > determined connect to 127.0.0.1:5002 > > [proxy:trace2] proxy_util.c(3101): H2C: reusing backend connection > > 127.0.0.1:60120<>127.0.0.1:5002 > > > > and that looks wrong. > > > > Question: do we have a bug or do we consider such ProxyPassMatch as broken > > and require at least enablereuse=off? > > I can't find all the history, but some time ago proxypassmatch never > tried to find a matching worker > https://bz.apache.org/bugzilla/show_bug.cgi?id=62167 > > Later, we must have gotten some logic added but you still have to > opt-in, I think it is likely > http://home.apache.org/~ylavic/patches/httpd-2.4.x-ap_proxy_define_match_worker.patch > or adjacent Yes, we have [1] that disables ProxyPassMatch workers reuse if they do any $n substitution, precisely because reuse can't work in all cases and particularly when the hostname[:port] part is substituted. ProxyPassMatch workers were introduced in 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. Before that there were no parameters (enablereuse, keepalive, timeout, ...) applying to ProxyPassMatch connections, they fell back to the generic "proxy:reverse" worker, all parameters were ignored and connections never reused. Since 2.4.47 we have ProxyPassMatch workers (worker->s->is_name_matchable == true), their name (match prefix) is everything up to the first '$', so with "h2c://127.0.0.1:$1/$2" the name is "h2c://127.0.0.1:", which obviously will match all the possible ports to the same worker.. So enablereuse=on is now possible with ProxyPassMatch, and makes sense for things like: ProxyPassMatch ^/(prod|dev)/([-a-z0-9]+)/(.*)$ h2://backend.internal/$2/$1/$3 enablereuse=on but it should not be used when the substitution involves the hostname[:port] (besides the security issue..). The issue here (IIUC) is that the user had the explicit enablereuse=on before 2.4.47 already but it was ignored, and now it breaks :/ Maybe we can do something to use the generic worker as before (and ignore parameters) when we detect that the substitution happens in the hostname[:port] part of the URL, but the damage is already done and I'm not sure it's worth the change given how this kind of configuration is not recommended in the first place. Regards; Yann. [1] https://github.com/apache/httpd/blob/2.4.x/modules/proxy/proxy_util.c#L2031
Re: friends of mod_proxy
On Thu, Oct 6, 2022 at 10:21 AM Stefan Eissing via dev wrote: > > Friends of mod_proxy, I have a question: > > In <https://github.com/icing/mod_h2/issues/235> someone reported wrong > connection reuse for a config like: > > ProxyPassMatch ^/(prod|dev)/([-a-z0-9]+)/(.*)$ h2://$2.internal/$1/$2/$3 > enablereuse=on keepalive=on > > Leaving aside the issue that such a pattern is insecure due to the client > influencing the hostname, the fact remains that mod_proxy_http2 will use a > previous connection, even if the matched hostname is different. I replicated > that, using "just" ports in a test case: > > ProxyPassMatch ^/h2proxy/([0-9]+)/(.*)$ h2c://127.0.0.1:$1/$2 enablereuse=on > keepalive=on > > Then > 1. /h2proxy/5002/hello.py > 2. /h2proxy/5004/hello.py > > results in 2) re-using the connection of 1). The log file says for 2): > > [proxy:debug] proxy_util.c(2538): AH00942: H2C: has acquired connection for > (127.0.0.1:80) > [proxy:debug] proxy_util.c(2596): [remote 127.0.0.1:60121] AH00944: > connecting h2c://127.0.0.1:5004/hello.py to 127.0.0.1:5004 > [proxy:debug] proxy_util.c(2819): [remote 127.0.0.1:60121] AH00947: connected > /hello.py to 127.0.0.1:5002 > [proxy_http2:trace1] mod_proxy_http2.c(374): [remote 127.0.0.1:60121] H2: > determined connect to 127.0.0.1:5002 > [proxy:trace2] proxy_util.c(3101): H2C: reusing backend connection > 127.0.0.1:60120<>127.0.0.1:5002 > > and that looks wrong. > > Question: do we have a bug or do we consider such ProxyPassMatch as broken > and require at least enablereuse=off? I can't find all the history, but some time ago proxypassmatch never tried to find a matching worker https://bz.apache.org/bugzilla/show_bug.cgi?id=62167 Later, we must have gotten some logic added but you still have to opt-in, I think it is likely http://home.apache.org/~ylavic/patches/httpd-2.4.x-ap_proxy_define_match_worker.patch or adjacent Not sure if it helps
Re: friends of mod_proxy
On 10/6/22 4:20 PM, Stefan Eissing via dev wrote: > Friends of mod_proxy, I have a question: > > In <https://github.com/icing/mod_h2/issues/235> someone reported wrong > connection reuse for a config like: > > ProxyPassMatch ^/(prod|dev)/([-a-z0-9]+)/(.*)$ h2://$2.internal/$1/$2/$3 > enablereuse=on keepalive=on > > Leaving aside the issue that such a pattern is insecure due to the client > influencing the hostname, the fact remains that mod_proxy_http2 will use a > previous connection, even if the matched hostname is different. I replicated > that, using "just" ports in a test case: > > ProxyPassMatch ^/h2proxy/([0-9]+)/(.*)$ h2c://127.0.0.1:$1/$2 enablereuse=on > keepalive=on > > Then > 1. /h2proxy/5002/hello.py > 2. /h2proxy/5004/hello.py > > results in 2) re-using the connection of 1). The log file says for 2): > > [proxy:debug] proxy_util.c(2538): AH00942: H2C: has acquired connection for > (127.0.0.1:80) > [proxy:debug] proxy_util.c(2596): [remote 127.0.0.1:60121] AH00944: > connecting h2c://127.0.0.1:5004/hello.py to 127.0.0.1:5004 > [proxy:debug] proxy_util.c(2819): [remote 127.0.0.1:60121] AH00947: connected > /hello.py to 127.0.0.1:5002 > [proxy_http2:trace1] mod_proxy_http2.c(374): [remote 127.0.0.1:60121] H2: > determined connect to 127.0.0.1:5002 > [proxy:trace2] proxy_util.c(3101): H2C: reusing backend connection > 127.0.0.1:60120<>127.0.0.1:5002 > > and that looks wrong. > > Question: do we have a bug or do we consider such ProxyPassMatch as broken > and require at least enablereuse=off? Depends on your point of view :-). As you state such setups can be considered unsafe in general, but maybe we should set enablereuse=off implicitly if we detect a $ in the host or port component or we should reject them completely. But I guess enablereuse=off is more backwards compatible. If you still need to reuse connections for such cases you can still use RewriteRules and appropriate
friends of mod_proxy
Friends of mod_proxy, I have a question: In <https://github.com/icing/mod_h2/issues/235> someone reported wrong connection reuse for a config like: ProxyPassMatch ^/(prod|dev)/([-a-z0-9]+)/(.*)$ h2://$2.internal/$1/$2/$3 enablereuse=on keepalive=on Leaving aside the issue that such a pattern is insecure due to the client influencing the hostname, the fact remains that mod_proxy_http2 will use a previous connection, even if the matched hostname is different. I replicated that, using "just" ports in a test case: ProxyPassMatch ^/h2proxy/([0-9]+)/(.*)$ h2c://127.0.0.1:$1/$2 enablereuse=on keepalive=on Then 1. /h2proxy/5002/hello.py 2. /h2proxy/5004/hello.py results in 2) re-using the connection of 1). The log file says for 2): [proxy:debug] proxy_util.c(2538): AH00942: H2C: has acquired connection for (127.0.0.1:80) [proxy:debug] proxy_util.c(2596): [remote 127.0.0.1:60121] AH00944: connecting h2c://127.0.0.1:5004/hello.py to 127.0.0.1:5004 [proxy:debug] proxy_util.c(2819): [remote 127.0.0.1:60121] AH00947: connected /hello.py to 127.0.0.1:5002 [proxy_http2:trace1] mod_proxy_http2.c(374): [remote 127.0.0.1:60121] H2: determined connect to 127.0.0.1:5002 [proxy:trace2] proxy_util.c(3101): H2C: reusing backend connection 127.0.0.1:60120<>127.0.0.1:5002 and that looks wrong. Question: do we have a bug or do we consider such ProxyPassMatch as broken and require at least enablereuse=off? Thanks for your help, Stefan
