Re: [PATCH] openssl-compat: Fix getm_ defines
пн, 16 дек. 2019 г. в 12:47, William Lallemand : > Hello Rosen, > > On Sun, Dec 15, 2019 at 11:04:37PM -0800, Rosen Penev wrote: > > -#if (OPENSSL_VERSION_NUMBER < 0x1010L) || (LIBRESSL_VERSION_NUMBER > < 0x2070L) > > +#if (OPENSSL_VERSION_NUMBER < 0x1010L) || \ > > + (defined(LIBRESSL_VERSION_NUMBER) && (LIBRESSL_VERSION_NUMBER < > 0x2070L)) > > > > It's probably cleaner to use the HA_OPENSSL_VERSION_NUMBER function and > something like this: > @Rosen Penev there was some discussion whether to trust OPENSSL_VERSION_NUMBER or not. LibreSSL pollutes that macro, it sets it to 2.0.0 so ... new macro was introduced instead: HA_OPENSSL_VERSION_NUMBER https://github.com/haproxy/haproxy/blob/master/include/common/openssl-compat.h#L27-L36 > >#if (HA_OPENSSL_VERSION_NUMBER < 0x101fL) && > (LIBRESSL_VERSION_NUMBER < 0x207fL) > > -- > William Lallemand > >
Re: [RFC PATCH] MINOR: debug: allow debug converter in default build
Hi Lukas, On Sun, Dec 15, 2019 at 05:23:38PM +0100, Lukas Tribus wrote: > Currently this debug converter is only enabled when DEBUG_EXPR is > defined at build time (which is different than other debug build > options and unclear from the documentation). > > This moves the patch to the default build, so everyone can use it. I was thinking about repurposing this converter to use the ring buffer instead, so that it can be used even with live traffic and let one consult some history from the CLI. We could then have a few info such as the request ID, and config file+line number where the converter is present, followed by the pattern. We could imagine adding one (or multiple) optional arguments to force the output to another destination (e.g. stdout, stderr, other ring), and maybe another one for the format or to prepend a prefix. Then most likely we'd use the ring buffer by default as it's the least impacting one and the only self-sustaining output. And probably that we could switch to stderr by default in backports (or make it mandatory to force the destination). What do you think ? Cheers, Willy
Re: [RFC PATCH] MINOR: debug: allow debug converter in default build
On Mon, Dec 16, 2019 at 9:22 AM Willy Tarreau wrote: > Hi Lukas, > > On Sun, Dec 15, 2019 at 05:23:38PM +0100, Lukas Tribus wrote: > > Currently this debug converter is only enabled when DEBUG_EXPR is > > defined at build time (which is different than other debug build > > options and unclear from the documentation). > > > > This moves the patch to the default build, so everyone can use it. > > I was thinking about repurposing this converter to use the ring buffer > instead, so that it can be used even with live traffic and let one > consult some history from the CLI. We could then have a few info > such as the request ID, and config file+line number where the > converter is present, followed by the pattern. > > We could imagine adding one (or multiple) optional arguments to force > the output to another destination (e.g. stdout, stderr, other ring), > and maybe another one for the format or to prepend a prefix. Then most > likely we'd use the ring buffer by default as it's the least impacting > one and the only self-sustaining output. And probably that we could > switch to stderr by default in backports (or make it mandatory to > force the destination). > > What do you think ? > > Cheers, > Willy > > Hi, My 2 cents. I personally use a lot this converter, si I'd be more than happy to get it available in default built! I think Willy's idea to route its output wherever we want is great too for production purpose. Can we also use an env variable? So we can easily switch from stdout to ring buffer without updating the config file? Baptiste
Re: [RFC PATCH] MINOR: debug: allow debug converter in default build
On Mon, Dec 16, 2019 at 09:30:38AM +0100, Baptiste wrote: > My 2 cents. > I personally use a lot this converter, si I'd be more than happy to get it > available in default built! OK. > I think Willy's idea to route its output wherever we want is great too for > production purpose. At least the thing is that you cannot leave it enabled forever when it's on stdout or stderr because either this will be dropped entirely or will inflate the service logs. With the rotating buffer you can leave it enabled full time and only go to the CLI to consult it when needed. Later we could imagine passing an extra ID to selectively enable/disable each of them via the CLI. It would then provide something more or less comparable to what Cyril did many years ago with his ACL debugging patchset. > Can we also use an env variable? So we can easily switch from stdout to > ring buffer without updating the config file? It's orthogonal, environment variables are evaluated by the config parser :-) You could even imagine having something like : http-request some-action "sample_fetch${DEBUG},converter" Where DEBUG is either empty or ",debug". Willy
Re: [PATCH] openssl-compat: Fix getm_ defines
also, BoringSSL fails after applying your patch https://travis-ci.com/chipitsine/haproxy/jobs/267601286 пн, 16 дек. 2019 г. в 12:07, Rosen Penev : > LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition > always true. Check for the define before checking it. > > Signed-off-by: Rosen Penev > --- > include/common/openssl-compat.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/common/openssl-compat.h > b/include/common/openssl-compat.h > index 25102fbe3..8b1630110 100644 > --- a/include/common/openssl-compat.h > +++ b/include/common/openssl-compat.h > @@ -278,7 +278,8 @@ static inline void EVP_PKEY_up_ref(EVP_PKEY *pkey) > #define TLSEXT_signature_ecdsa 3 > #endif > > -#if (OPENSSL_VERSION_NUMBER < 0x1010L) || (LIBRESSL_VERSION_NUMBER < > 0x2070L) > +#if (OPENSSL_VERSION_NUMBER < 0x1010L) || \ > + (defined(LIBRESSL_VERSION_NUMBER) && (LIBRESSL_VERSION_NUMBER < > 0x2070L)) > #define X509_getm_notBefore X509_get_notBefore > #define X509_getm_notAfter X509_get_notAfter > #endif > -- > 2.23.0 > > >
Re: [PATCH] openssl-compat: Fix getm_ defines
Hello Rosen, > пн, 16 дек. 2019 г. в 12:07, Rosen Penev : >> >> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition >> always true. Check for the define before checking it. I cannot find this in the openssl sources, not in master and not in the 1.1.1 branch. Please clarify where this is defined. The SSL compatibility layer is already complex enough and needs continuous adjustments, we need to understand the reason for changes very well. Fast fixes are continually coming back to hunt us. On Mon, 16 Dec 2019 at 08:19, Илья Шипицин wrote: > please have a look at https://github.com/haproxy/haproxy/issues/367 (it still > misses germ part, I tried things like you send, but reg-tests fail. do you > have travis-ci passed ?) > also, there's a patch already sent, Lukas Tribus promised to review it Yeah, this one fell through the cracks. Give me a few days to catch up. Thanks, Lukas
Re: PROXY protocol and check port
Hello, I found what was wrong : I was using "load-server-state-from-file" and previous config file was using port 80 as server port. It seems using this instruction loads previous server state but also previous srv_port. Is this an expected behaviour ? Olivier Le ven. 13 déc. 2019 à 18:32, Olivier D a écrit : > Hello all, > I struggle with what seemed a very easy config : > > listen test:443 > id 20609 > bind-process 16 > balance source > hash-type consistent > mode tcp > bind x.x.x.x:443 > server s1 192.168.x.x:443 id 2158 check weight 5 send-proxy port 80 > server s2 192.168.x.x:443 id 2168 check weight 5 send-proxy port 80 > > I expect traffic to be routed to port 443 of backend server, and checks to > be performed without proxy-protocol on port 80. tests seem ok, but > traffic seems also routed to port 80 (I checked with tcpdump) instead of > 443. > > Can you at least confirm that my expectations are correct ? Or am I just > very tired on a friday evening ? > > I'm using HAProxy 1.9.12 > > Thank you ! >
Re: [RFC PATCH] MINOR: debug: allow debug converter in default build
Hello, On Mon, 16 Dec 2019 at 09:20, Willy Tarreau wrote: > > Hi Lukas, > > On Sun, Dec 15, 2019 at 05:23:38PM +0100, Lukas Tribus wrote: > > Currently this debug converter is only enabled when DEBUG_EXPR is > > defined at build time (which is different than other debug build > > options and unclear from the documentation). > > > > This moves the patch to the default build, so everyone can use it. > > I was thinking about repurposing this converter to use the ring buffer > instead, so that it can be used even with live traffic and let one > consult some history from the CLI. We could then have a few info > such as the request ID, and config file+line number where the > converter is present, followed by the pattern. > > We could imagine adding one (or multiple) optional arguments to force > the output to another destination (e.g. stdout, stderr, other ring), > and maybe another one for the format or to prepend a prefix. Then most > likely we'd use the ring buffer by default as it's the least impacting > one and the only self-sustaining output. And probably that we could > switch to stderr by default in backports (or make it mandatory to > force the destination). > > What do you think ? I like it, more flexibility to debug in production is always a good. Removing those 2 ifdef's is just a "low hanging fruit". Lukas
[PATCH] openssl-compat: Fix getm_ defines
LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition always true. Check for the define before checking it. Signed-off-by: Rosen Penev --- include/common/openssl-compat.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/common/openssl-compat.h b/include/common/openssl-compat.h index 25102fbe3..c5029d133 100644 --- a/include/common/openssl-compat.h +++ b/include/common/openssl-compat.h @@ -278,7 +278,7 @@ static inline void EVP_PKEY_up_ref(EVP_PKEY *pkey) #define TLSEXT_signature_ecdsa 3 #endif -#if (OPENSSL_VERSION_NUMBER < 0x1010L) || (LIBRESSL_VERSION_NUMBER < 0x2070L) +#if (HA_OPENSSL_VERSION_NUMBER < 0x101fL) && (LIBRESSL_VERSION_NUMBER < 0x207fL) #define X509_getm_notBefore X509_get_notBefore #define X509_getm_notAfter X509_get_notAfter #endif -- 2.23.0
Re: [PATCH] openssl-compat: Fix getm_ defines
On Mon, Dec 16, 2019 at 4:49 AM Lukas Tribus wrote: > > Hello Rosen, > > > пн, 16 дек. 2019 г. в 12:07, Rosen Penev : > >> > >> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition > >> always true. Check for the define before checking it. > > I cannot find this in the openssl sources, not in master and not in > the 1.1.1 branch. Please clarify where this is defined. Compile with -Wundef. Missing macros evaluate to 0. > > The SSL compatibility layer is already complex enough and needs > continuous adjustments, we need to understand the reason for changes > very well. Fast fixes are continually coming back to hunt us. > > > On Mon, 16 Dec 2019 at 08:19, Илья Шипицин wrote: > > please have a look at https://github.com/haproxy/haproxy/issues/367 (it > > still misses germ part, I tried things like you send, but reg-tests fail. > > do you have travis-ci passed ?) > > also, there's a patch already sent, Lukas Tribus promised to review it > > Yeah, this one fell through the cracks. Give me a few days to catch up. > > Thanks, > Lukas
Re: [PATCH] openssl-compat: Fix getm_ defines
пн, 16 дек. 2019 г. в 22:42, Rosen Penev : > LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition > always true. Check for the define before checking it. > > Signed-off-by: Rosen Penev > --- > include/common/openssl-compat.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/common/openssl-compat.h > b/include/common/openssl-compat.h > index 25102fbe3..c5029d133 100644 > --- a/include/common/openssl-compat.h > +++ b/include/common/openssl-compat.h > @@ -278,7 +278,7 @@ static inline void EVP_PKEY_up_ref(EVP_PKEY *pkey) > #define TLSEXT_signature_ecdsa 3 > #endif > > -#if (OPENSSL_VERSION_NUMBER < 0x1010L) || (LIBRESSL_VERSION_NUMBER < > 0x2070L) > +#if (HA_OPENSSL_VERSION_NUMBER < 0x101fL) && (LIBRESSL_VERSION_NUMBER > < 0x207fL) > assuming "&& (LIBRESSL_VERSION_NUMBER < 0x207fL)" part ... it is only relevant for LibreSSL, right ? if so, should we leave just second part and omit first ? > #define X509_getm_notBefore X509_get_notBefore > #define X509_getm_notAfter X509_get_notAfter > #endif > -- > 2.23.0 > > >
[2.1.1] http-request replace-uri does not work
Hello, This is an extract of my frontend configuration working perfectly on 2.0.11. frontend wwws bind 0.0.0.0:443 ssl crt /etc/haproxy/ssl/server.pem alpn h2,http/1.1 mode http acl is_dev_qd hdr(host) -i dev.q.d dev.qs.d acl is_qd hdr(host) -i q.d qs.d www.q.d www.qs.d http-request replace-uri ^/PPDSlide/(.*) /d3/PPDSlide/\1 if is_dev_qd http-request replace-uri ^/PPDSlide/(.*) /p3/PPDSlide/\1 if is_qd URLs like https://q.d/PPDSlide/testfile are correctly rewritten to https://q.d/p3/PPDSlide/testfile and forwarded to the backend. Once I switched to 2.1.1, haproxy no longer rewrites the URI and the URIs remains unchanged while forwarded to the backend. I had to downgrade to have the usual behaviour. Is it a bug or something changed in normal haproxy behaviour with 2.1 release ? -- Best regards, Artur
Re: [PATCH] openssl-compat: Fix getm_ defines
On Mon, Dec 16, 2019 at 10:00 AM Илья Шипицин wrote: > > > > пн, 16 дек. 2019 г. в 22:42, Rosen Penev : >> >> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition >> always true. Check for the define before checking it. >> >> Signed-off-by: Rosen Penev >> --- >> include/common/openssl-compat.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/common/openssl-compat.h >> b/include/common/openssl-compat.h >> index 25102fbe3..c5029d133 100644 >> --- a/include/common/openssl-compat.h >> +++ b/include/common/openssl-compat.h >> @@ -278,7 +278,7 @@ static inline void EVP_PKEY_up_ref(EVP_PKEY *pkey) >> #define TLSEXT_signature_ecdsa 3 >> #endif >> >> -#if (OPENSSL_VERSION_NUMBER < 0x1010L) || (LIBRESSL_VERSION_NUMBER < >> 0x2070L) >> +#if (HA_OPENSSL_VERSION_NUMBER < 0x101fL) && (LIBRESSL_VERSION_NUMBER < >> 0x207fL) > > > assuming "&& (LIBRESSL_VERSION_NUMBER < 0x207fL)" part ... it is only > relevant for LibreSSL, right ? > if so, should we leave just second part and omit first ? No. As I said previously, undefined macros evaluate to 0. OpenSSL does not define LIBRESSL_VERSION_NUMBER. > > >> >> #define X509_getm_notBefore X509_get_notBefore >> #define X509_getm_notAfter X509_get_notAfter >> #endif >> -- >> 2.23.0 >> >>
Re: [PATCH] openssl-compat: Fix getm_ defines
On Mon, 16 Dec 2019 at 19:00, Илья Шипицин wrote: > > > > пн, 16 дек. 2019 г. в 22:42, Rosen Penev : >> >> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition >> always true. Check for the define before checking it. >> >> Signed-off-by: Rosen Penev >> --- >> include/common/openssl-compat.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/common/openssl-compat.h >> b/include/common/openssl-compat.h >> index 25102fbe3..c5029d133 100644 >> --- a/include/common/openssl-compat.h >> +++ b/include/common/openssl-compat.h >> @@ -278,7 +278,7 @@ static inline void EVP_PKEY_up_ref(EVP_PKEY *pkey) >> #define TLSEXT_signature_ecdsa 3 >> #endif >> >> -#if (OPENSSL_VERSION_NUMBER < 0x1010L) || (LIBRESSL_VERSION_NUMBER < >> 0x2070L) >> +#if (HA_OPENSSL_VERSION_NUMBER < 0x101fL) && (LIBRESSL_VERSION_NUMBER < >> 0x207fL) > > > assuming "&& (LIBRESSL_VERSION_NUMBER < 0x207fL)" part ... it is only > relevant for LibreSSL, right ? > if so, should we leave just second part and omit first ? Any reason why would not just #ifndef X509_getm_notBefore, testing for what we actually want instead of those backbreaking version assumptions? Lukas
Re: [PATCH] openssl-compat: Fix getm_ defines
пн, 16 дек. 2019 г. в 22:40, Rosen Penev : > On Mon, Dec 16, 2019 at 4:49 AM Lukas Tribus wrote: > > > > Hello Rosen, > > > > > пн, 16 дек. 2019 г. в 12:07, Rosen Penev : > > >> > > >> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the > condition > > >> always true. Check for the define before checking it. > > > > I cannot find this in the openssl sources, not in master and not in > > the 1.1.1 branch. Please clarify where this is defined. > Compile with -Wundef. Missing macros evaluate to 0. > I checked haproxy source, it does not use such compiler flag. Any reason for introducing it ? if we want to make it first class citizen, maybe we should add it to proper Makefile ? or to our CI ? assuming "undefined macros may ACCIDENTLY become equal to 0" scares me > > > > The SSL compatibility layer is already complex enough and needs > > continuous adjustments, we need to understand the reason for changes > > very well. Fast fixes are continually coming back to hunt us. > > > > > > On Mon, 16 Dec 2019 at 08:19, Илья Шипицин wrote: > > > please have a look at https://github.com/haproxy/haproxy/issues/367 > (it still misses germ part, I tried things like you send, but reg-tests > fail. do you have travis-ci passed ?) > > > also, there's a patch already sent, Lukas Tribus promised to review it > > > > Yeah, this one fell through the cracks. Give me a few days to catch up. > > > > Thanks, > > Lukas >
CORS support
Hello Haproxy group- migrating from haproxy 2.0 to 2.1 and noticed some directives changed: === 2.0.10 === capture request header origin len 128 http-response add-header Access-Control-Allow-Origin %[capture.req.hdr(0)] if { capture.req.hdr(0) -m end aiqwest.com } http-response add-header Access-Control-Allow-Headers:\ Origin,\ X-Requested-With,\ Content-Type,\ Accept if { capture.req.hdr(0) -m found } === 2.1 === capture request header origin len 128 http-response add-header Access-Control-Allow-Origin %[capture.req.hdr(0)] if { capture.req.hdr(0) -m end aiqwest.com } rspadd add-header Access-Control-Allow-Headers:\ Origin,\ X-Requested-With,\ Content-Type,\ Accept if { capture.req.hdr(0) -m found } getting: The 'rspadd' directive is not supported anymore since HAProxy 2.1. Use 'http-response add-header' instead. when adding the said above, getting an error: 'http-response add-header' expects exactly 2 arguments. main question is, how can I convert the resadd statement to work with the new 2.1? Thank you!
Re: [2.1.1] http-request replace-uri does not work
Hi Artur, Le 16/12/2019 à 19:06, Artur a écrit : Hello, This is an extract of my frontend configuration working perfectly on 2.0.11. frontend wwws bind 0.0.0.0:443 ssl crt /etc/haproxy/ssl/server.pem alpn h2,http/1.1 mode http acl is_dev_qd hdr(host) -i dev.q.d dev.qs.d acl is_qd hdr(host) -i q.d qs.d www.q.d www.qs.d http-request replace-uri ^/PPDSlide/(.*) /d3/PPDSlide/\1 if is_dev_qd http-request replace-uri ^/PPDSlide/(.*) /p3/PPDSlide/\1 if is_qd URLs like https://q.d/PPDSlide/testfile are correctly rewritten to https://q.d/p3/PPDSlide/testfile and forwarded to the backend. Once I switched to 2.1.1, haproxy no longer rewrites the URI and the URIs remains unchanged while forwarded to the backend. I had to downgrade to have the usual behaviour. Is it a bug or something changed in normal haproxy behaviour with 2.1 release ? I can confirm the issue. It seems to happen with h2 requests only, since commit #30ee1efe67. haproxy normalizes the URI but replace-uri doesn't take into account this information. The fix should be easy for replace-uri (If someone wants to work on it, I won't have time this week). http://git.haproxy.org/?p=haproxy-2.1.git;a=commit;h=30ee1efe67 -- Cyril Bonté
Re: [PATCH] openssl-compat: Fix getm_ defines
On Mon, Dec 16, 2019 at 10:09 AM Lukas Tribus wrote: > > On Mon, 16 Dec 2019 at 19:00, Илья Шипицин wrote: > > > > > > > > пн, 16 дек. 2019 г. в 22:42, Rosen Penev : > >> > >> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition > >> always true. Check for the define before checking it. > >> > >> Signed-off-by: Rosen Penev > >> --- > >> include/common/openssl-compat.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/include/common/openssl-compat.h > >> b/include/common/openssl-compat.h > >> index 25102fbe3..c5029d133 100644 > >> --- a/include/common/openssl-compat.h > >> +++ b/include/common/openssl-compat.h > >> @@ -278,7 +278,7 @@ static inline void EVP_PKEY_up_ref(EVP_PKEY *pkey) > >> #define TLSEXT_signature_ecdsa 3 > >> #endif > >> > >> -#if (OPENSSL_VERSION_NUMBER < 0x1010L) || (LIBRESSL_VERSION_NUMBER < > >> 0x2070L) > >> +#if (HA_OPENSSL_VERSION_NUMBER < 0x101fL) && (LIBRESSL_VERSION_NUMBER > >> < 0x207fL) > > > > > > assuming "&& (LIBRESSL_VERSION_NUMBER < 0x207fL)" part ... it is only > > relevant for LibreSSL, right ? > > if so, should we leave just second part and omit first ? > > Any reason why would not just #ifndef X509_getm_notBefore, testing for > what we actually want instead of those backbreaking version > assumptions? X509_getm_notBefore is a function, not a define. > > > Lukas
Re: [PATCH] openssl-compat: Fix getm_ defines
On Mon, Dec 16, 2019 at 10:21 AM Илья Шипицин wrote: > > > > пн, 16 дек. 2019 г. в 22:40, Rosen Penev : >> >> On Mon, Dec 16, 2019 at 4:49 AM Lukas Tribus wrote: >> > >> > Hello Rosen, >> > >> > > пн, 16 дек. 2019 г. в 12:07, Rosen Penev : >> > >> >> > >> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the >> > >> condition >> > >> always true. Check for the define before checking it. >> > >> > I cannot find this in the openssl sources, not in master and not in >> > the 1.1.1 branch. Please clarify where this is defined. >> Compile with -Wundef. Missing macros evaluate to 0. > > > I checked haproxy source, it does not use such compiler flag. Any reason for > introducing it ? > > if we want to make it first class citizen, maybe we should add it to proper > Makefile ? or to our CI ? > > assuming "undefined macros may ACCIDENTLY become equal to 0" scares me You serious? This is basic C. Undefined macros always evaluate to 0. -Wundef only warns about it. > >> >> > >> > The SSL compatibility layer is already complex enough and needs >> > continuous adjustments, we need to understand the reason for changes >> > very well. Fast fixes are continually coming back to hunt us. >> > >> > >> > On Mon, 16 Dec 2019 at 08:19, Илья Шипицин wrote: >> > > please have a look at https://github.com/haproxy/haproxy/issues/367 (it >> > > still misses germ part, I tried things like you send, but reg-tests >> > > fail. do you have travis-ci passed ?) >> > > also, there's a patch already sent, Lukas Tribus promised to review it >> > >> > Yeah, this one fell through the cracks. Give me a few days to catch up. >> > >> > Thanks, >> > Lukas
Re: HAProxy 2.0.10 and 2.1.0 RPM's
On Tue, Nov 26, 2019 at 2:40 PM Russell Eason wrote: > Hello, > > Fedora upstream added it > https://src.fedoraproject.org/rpms/haproxy/c/45c57ba71174f308a5f59569bac0598bb31ef767 > , and can be seen as far back as F24 here > https://src.fedoraproject.org/rpms/haproxy/blob/f24/f/haproxy.spec . LUA > support is in the RHEL 8 version of HAProxy, but not in 7 (yet?). > Sorry for the late reply. I believe the reason that RHEL7 lacks LUA support is because the required version of LUA is not available in RHEL7. Enabling LUA support in haproxy at build time is easy, but if the underlying bits aren't available, there is nothing I can do. Cheers. Ryan
Re: [2.1.1] http-request replace-uri does not work
Hello Cyril, Thanks a lot for the confirmation. Le 16/12/2019 à 20:20, Cyril Bonté a écrit : > Hi Artur, > > Le 16/12/2019 à 19:06, Artur a écrit : >> Hello, >> >> This is an extract of my frontend configuration working perfectly on >> 2.0.11. >> >> frontend wwws >> bind 0.0.0.0:443 ssl crt /etc/haproxy/ssl/server.pem alpn >> h2,http/1.1 >> mode http >> acl is_dev_qd hdr(host) -i dev.q.d dev.qs.d >> acl is_qd hdr(host) -i q.d qs.d www.q.d www.qs.d >> http-request replace-uri ^/PPDSlide/(.*) /d3/PPDSlide/\1 if >> is_dev_qd >> http-request replace-uri ^/PPDSlide/(.*) /p3/PPDSlide/\1 if >> is_qd >> >> >> URLs like https://q.d/PPDSlide/testfile are correctly rewritten to >> https://q.d/p3/PPDSlide/testfile and forwarded to the backend. >> >> Once I switched to 2.1.1, haproxy no longer rewrites the URI and the >> URIs remains unchanged while forwarded to the backend. I had to >> downgrade to have the usual behaviour. >> >> Is it a bug or something changed in normal haproxy behaviour with 2.1 >> release ? > > I can confirm the issue. > > It seems to happen with h2 requests only, since commit #30ee1efe67. > haproxy normalizes the URI but replace-uri doesn't take into account > this information. The fix should be easy for replace-uri (If someone > wants to work on it, I won't have time this week). > > http://git.haproxy.org/?p=haproxy-2.1.git;a=commit;h=30ee1efe67 > > -- Best regards, Artur
Re: [PATCH] openssl-compat: Fix getm_ defines
вт, 17 дек. 2019 г. в 00:55, Rosen Penev : > On Mon, Dec 16, 2019 at 10:21 AM Илья Шипицин > wrote: > > > > > > > > пн, 16 дек. 2019 г. в 22:40, Rosen Penev : > >> > >> On Mon, Dec 16, 2019 at 4:49 AM Lukas Tribus wrote: > >> > > >> > Hello Rosen, > >> > > >> > > пн, 16 дек. 2019 г. в 12:07, Rosen Penev : > >> > >> > >> > >> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the > condition > >> > >> always true. Check for the define before checking it. > >> > > >> > I cannot find this in the openssl sources, not in master and not in > >> > the 1.1.1 branch. Please clarify where this is defined. > >> Compile with -Wundef. Missing macros evaluate to 0. > > > > > > I checked haproxy source, it does not use such compiler flag. Any reason > for introducing it ? > > > > if we want to make it first class citizen, maybe we should add it to > proper Makefile ? or to our CI ? > > > > assuming "undefined macros may ACCIDENTLY become equal to 0" scares me > You serious? This is basic C. Undefined macros always evaluate to 0. > > indeed, you're right. I checked both gcc and clang. unfortunately, I neither learned nor used that area of C preprocessor specification before. > -Wundef only warns about it. > > > >> > >> > > >> > The SSL compatibility layer is already complex enough and needs > >> > continuous adjustments, we need to understand the reason for changes > >> > very well. Fast fixes are continually coming back to hunt us. > >> > > >> > > >> > On Mon, 16 Dec 2019 at 08:19, Илья Шипицин > wrote: > >> > > please have a look at https://github.com/haproxy/haproxy/issues/367 > (it still misses germ part, I tried things like you send, but reg-tests > fail. do you have travis-ci passed ?) > >> > > also, there's a patch already sent, Lukas Tribus promised to review > it > >> > > >> > Yeah, this one fell through the cracks. Give me a few days to catch > up. > >> > > >> > Thanks, > >> > Lukas >
Re: HAProxy 2.0.10 and 2.1.0 RPM's
On Tue, Nov 26, 2019 at 9:20 PM Willy Tarreau wrote: > > Indeed that looks good. We'll need to include Ryan in this discussion, > he's the maintainer of the official RPMs for RHEL. I'm purposely not CCing > him as I know he's very busy this week, but I sense that we're starting to > see the light at the end of the tunnel here. > Indeed, I have been busy. Then on vacation. Then back in time for another deadline. I'm always open to hear feedback on the RHEL and Fedora packages (I maintain Fedora packages as well as RHEL). Thanks to Julien for creating those copr builds. That makes my life so much easier! A couple items about Fedora/RHEL packages. For Fedora, I try to be responsive for CVEs, minor release rebases, and enabling features (eg. Prometheus support). Lesser things like minor spec file changes usually get a much lower priority. Note that we do not rebase to a new, major release within a stable release. For example, if Fedora 31 has haproxy-2.0.x, it will never have haproxy-2.1.x. That is what copr is for. For RHEL, it is even more restrictive. I don't make the rules, just wanted to explain this since I get a lot of email asking about these things. Cheers. Ryan
haproxy 2.1 package for Debian 9 Stretch oldstable
Hello, While checking for haproxy 2.1 package for Debian Stretch on https://haproxy.debian.net/, I saw it wasn't available (yet ?). Do you plan to build haproxy deb packages for this version of Debian, it's still supported as oldstable for now ? -- Best regards, Artur
Re: HAProxy 2.0.10 and 2.1.0 RPM's
On 16 Dec 15:00, Ryan O'Hara wrote: > On Tue, Nov 26, 2019 at 2:40 PM Russell Eason wrote: > > > Hello, > > > > Fedora upstream added it > > https://src.fedoraproject.org/rpms/haproxy/c/45c57ba71174f308a5f59569bac0598bb31ef767 > > , and can be seen as far back as F24 here > > https://src.fedoraproject.org/rpms/haproxy/blob/f24/f/haproxy.spec . LUA > > support is in the RHEL 8 version of HAProxy, but not in 7 (yet?). > > > > Sorry for the late reply. I believe the reason that RHEL7 lacks LUA support > is because the required version of LUA is not available in RHEL7. Enabling > LUA support in haproxy at build time is easy, but if the underlying bits > aren't available, there is nothing I can do. Cheers. > > Ryan Alternatively I have updated https://copr.fedorainfracloud.org/coprs/roidelapluie/haproxy/ to 2.1.1 In ELEL7 it uses https://copr.fedorainfracloud.org/coprs/roidelapluie/lua/ which is the LUA for RHEL8 compiled for RHEL7. We statically compile it within haproxy. -- (o-Julien Pivotto //\Open-Source Consultant V_/_ Inuits - https://www.inuits.eu signature.asc Description: PGP signature
Re: [2.1.1] http-request replace-uri does not work
Hi Willy, Le 16/12/2019 à 22:06, Artur a écrit : [...] URLs like https://q.d/PPDSlide/testfile are correctly rewritten to https://q.d/p3/PPDSlide/testfile and forwarded to the backend. Once I switched to 2.1.1, haproxy no longer rewrites the URI and the URIs remains unchanged while forwarded to the backend. I had to downgrade to have the usual behaviour. Is it a bug or something changed in normal haproxy behaviour with 2.1 release ? I can confirm the issue. It seems to happen with h2 requests only, since commit #30ee1efe67. haproxy normalizes the URI but replace-uri doesn't take into account this information. The fix should be easy for replace-uri (If someone wants to work on it, I won't have time this week). http://git.haproxy.org/?p=haproxy-2.1.git;a=commit;h=30ee1efe67 I'm not 100% sure it's the right approach to fix the issue. Can you check if this patch may fix the issue in all conditions ? From what I've observed, it seems we can rely on the HTX_SL_F_NORMALIZED_URI flag to detect if the URI was normalized by haproxy and in that case, we should start at the path instead of the URI. -- Cyril Bonté diff --git a/src/http_act.c b/src/http_act.c index d6015d362..797d75275 100644 --- a/src/http_act.c +++ b/src/http_act.c @@ -141,6 +141,7 @@ static enum act_return http_action_replace_uri(struct act_rule *rule, struct pro { enum act_return ret = ACT_RET_ERR; struct buffer *replace, *output; + struct htx_sl *sl; struct ist uri; int len; @@ -148,7 +149,12 @@ static enum act_return http_action_replace_uri(struct act_rule *rule, struct pro output = alloc_trash_chunk(); if (!replace || !output) goto leave; - uri = htx_sl_req_uri(http_get_stline(htxbuf(&s->req.buf))); + + sl = http_get_stline(htxbuf(&s->req.buf)); + uri = htx_sl_req_uri(sl); + if (sl->flags & HTX_SL_F_NORMALIZED_URI) + uri = http_get_path(uri); + if (!regex_exec_match2(rule->arg.act.p[1], uri.ptr, uri.len, MAX_MATCH, pmatch, 0)) goto leave;
Re: PROXY protocol and check port
Hi, On Tue, Dec 17, 2019 at 2:55 AM Olivier D wrote: > Hello, > > I found what was wrong : I was using "load-server-state-from-file" and > previous config file was using port 80 as server port. > It seems using this instruction loads previous server state but also > previous srv_port. > Is this an expected behaviour ? > Yes, basically it is your responsibility to dump the current state into the file otherwise you'll gate outdated data as you noticed. For example I add: ExecReload=/bin/echo "show servers state" | /usr/bin/socat stdio /run/haproxy/admin.sock > /var/lib/haproxy/state in the systemd service file.
Re: ModSecurity testing
Hi Joao, On Sat, Dec 14, 2019 at 11:30 PM Joao Morais wrote: > > > > Em 13 de dez de 2019, à(s) 10:09, Christopher Faulet < > cfau...@haproxy.com> escreveu: > > > > Le 10/12/2019 à 05:24, Igor Cicimov a écrit : > >> > >> Testing with Haproxy 2.0.10 but same result with 1.8.23. The versions > of ModSecurity is 2.9.2 and the OWASP rules v3.0.2 > >> What am I doing wrong? Can anyone provide a request that should confirm > if the module is working or not from or share the experience from their own > setup? > > > > Hi Igor, > > > > First of all, I don't know how the modsecurity agent really work. But > I'm surprised to see it returns -101. In the code, -1, 0 or an HTTP status > code is expected. And only 0 or the HTTP status code is returned to > HAProxy. I don't know if -101 is a valid return value from modsecurity > point of view. But it is not from the agent one. > > > > Then, You don't have an error 403 because the variable txn.modsec.code > is negative, so the deny http-request rule is never triggered. So, I guess > your error 400 comes from your webserver. You can enabled HTTP log to have > more information. > > > > Finally, I notice some requests to the SPOA agent seems to have failed. > The variable is not set (- in the logs). You can try to enable SPOE logs in > your SPOE engine configuration. Take a look at the SPOE documentation > (doc/SPOE.txt) for more information. > > > Hi, perhaps this thread helps: > > https://www.mail-archive.com/haproxy@formilux.org/msg30061.html > > And perhaps this building of ModSecurity SPOA will also help: > > > https://github.com/jcmoraisjr/modsecurity-spoa/blob/v0.5/rootfs/Dockerfile > > ~jm > First thanks for your reply, I've been following your work on haproxy-ingress for Kubernetes (where I can see you have incorporated ModSecurity) and your input is certainly appreciated on this matter. I had some time today to quickly run the test again after enabling the log for SPOE: [modsecurity] spoe-agent modsecurity-agent log global messagescheck-request option var-prefix modsec option continue-on-error timeout hello 100ms timeout idle30s timeout processing 1s use-backend spoe-modsecurity spoe-message check-request args unique-id method path query req.ver req.hdrs_bin req.body_size req.body event on-frontend-http-request and can see that the empty values coming from SPOE are legit and are due to SEARCH method that is not allowed by Haproxy: Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1345:my-front.accept(000a)=0016 from [127.0.0.1:53206] ALPN= Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1345:my-front.clireq[0016:]: SEARCH / HTTP/1.1 Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1345:my-front.clihdr[0016:]: user-agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/74.0.3729.169 Safari/537.36 Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1345:my-front.clihdr[0016:]: content-type: application/x-www-form-urlencoded Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1345:my-front.clihdr[0016:]: content-length: 1 Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1345:my-front.clihdr[0016:]: host: localhost:9080 Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1343:my-front.srvcls[0017:0013] Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1343:my-front.clicls[0017:0013] Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1343:my-front.closed[0017:0013] Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1345:my-front.srvrep[0016:0013]: HTTP/1.1 405 Method Not Allowed Dec 17 00:40:01 ip-172-31-17-121 haproxy[21508]: SPOE: [modsecurity-agent] sid=4933 st=0 0/0/0/0/0 4/4 0/0 0/2768 Dec 17 00:40:01 ip-172-31-17-121 haproxy[21508]: The txn.modsec.code is: - Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1345:my-front.srvhdr[0016:0013]: server: ecstatic-3.3.2 Dec 17 00:40:01 ip-172-31-17-121 haproxy[21508]: The txn.modsec.code is: - Dec 17 00:40:01 ip-172-31-17-121 haproxy[21508]: SPOE: [modsecurity-agent] sid=4935 st=0 0/0/0/0/0 4/4 0/0 0/2769 Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1345:my-front.srvhdr[0016:0013]: date: Tue, 17 Dec 2019 00:40:01 GMT Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1345:my-front.srvhdr[0016:0013]: content-length: 0 Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1345:my-front.srvcls[0016:0013] Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1345:my-front.clicls[0016:0013] Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]: 1345:my-front.closed[0016:0013] and some other methods like TRACK, PROPFIND, DEBUG that NIkto tries out. Apart from that, I'm still equally stumped as I was before about the rules not working as expected. For example this request: $ curl -I -H "User-Agent: nikto" http://localhost:9080/ HTTP/1.1 200 OK server: ecstatic-3.3.2 cache-control: max-age=3600 last-modified: We
Re: CORS support
Hello Alex, On Mon, Dec 16, 2019 at 01:22:42PM -0500, Alex Evonosky wrote: > Hello Haproxy group- > > migrating from haproxy 2.0 to 2.1 and noticed some directives changed: > > === 2.0.10 === > > capture request header origin len 128 > http-response add-header Access-Control-Allow-Origin %[capture.req.hdr(0)] > if { capture.req.hdr(0) -m end aiqwest.com } > http-response add-header Access-Control-Allow-Headers:\ Origin,\ > X-Requested-With,\ Content-Type,\ Accept if { capture.req.hdr(0) -m found } > > > === 2.1 === > > capture request header origin len 128 > http-response add-header Access-Control-Allow-Origin %[capture.req.hdr(0)] > if { capture.req.hdr(0) -m end aiqwest.com } > rspadd add-header Access-Control-Allow-Headers:\ Origin,\ > X-Requested-With,\ Content-Type,\ Accept if { capture.req.hdr(0) -m found } > > > getting: > > The 'rspadd' directive is not supported anymore since HAProxy 2.1. Use > 'http-response add-header' instead. > > when adding the said above, getting an error: > > 'http-response add-header' expects exactly 2 arguments. You almost had it right, actually it's exactly the same as in your first rule: first arg is the header name, second one is the value: http-response add-header Access-Control-Allow-Headers Origin,\ X-Requested-With,\ Content-Type,\ Accept if { capture.req.hdr(0) -m found } Please let us know if anything is not clear enough in the message or in the doc so that we can improve it. Willy
Re: [2.1.1] http-request replace-uri does not work
Hi guys, On Mon, Dec 16, 2019 at 11:01:19PM +0100, Cyril Bonté wrote: > Hi Willy, > > Le 16/12/2019 à 22:06, Artur a écrit : > > > > [...] > > > > URLs like https://q.d/PPDSlide/testfile are correctly rewritten to > > > > https://q.d/p3/PPDSlide/testfile and forwarded to the backend. > > > > > > > > Once I switched to 2.1.1, haproxy no longer rewrites the URI and the > > > > URIs remains unchanged while forwarded to the backend. I had to > > > > downgrade to have the usual behaviour. > > > > > > > > Is it a bug or something changed in normal haproxy behaviour with 2.1 > > > > release ? > > > > > > I can confirm the issue. > > > > > > It seems to happen with h2 requests only, since commit #30ee1efe67. > > > haproxy normalizes the URI but replace-uri doesn't take into account > > > this information. The fix should be easy for replace-uri (If someone > > > wants to work on it, I won't have time this week). > > > > > > http://git.haproxy.org/?p=haproxy-2.1.git;a=commit;h=30ee1efe67 > > I'm not 100% sure it's the right approach to fix the issue. Can you check if > this patch may fix the issue in all conditions ? > > From what I've observed, it seems we can rely on the HTX_SL_F_NORMALIZED_URI > flag to detect if the URI was normalized by haproxy and in that case, we > should start at the path instead of the URI. In fact it's not a bug and is the intended behavior. The rule in Artur's config only matches origin URIs, not absolute ones, which most H2 clients use. In HTTP/1, there are two ways to send a request URI: origin form:GET /path/to/resource HTTP/1.1 absolute form: GET http://server.domain.tld/path/to/resource HTTP/1.1 Clients mostly use the origin form with servers, and mostly use the absolute form with proxies, and even then, the mapping is 99.99% and not 100% in either case. It's worth noting that Artur's rule already does not work with the absolute form, it only handles the origin form. In H2 you have the same distinction, except that clients were initially encouraged to use the absolute form only. The first non-HTX implementation used to implement only a subset of H2 and to map H2 to H1 using the Host header field, transforming incoming absolute URIs to origin URIs. This broke gRPC and end-to-end H2 because the scheme was lost. This also prevented such requests from being passed to proxies. So starting with 2.1 we now respect the format used by the client. It turns out that there seems to exist a number of configs in field which are already broken with H1 but which work by pure luck as they only handle the origin form. Arguably, many of these have evolved over the ages because initially they were designed to use the old reqrep matching which was very painful to deal with, and writing extra config just to handle an almost always absent scheme was not worth it. Nowadays we have multiple actions available depending on what we want to handle: - set-path to set only the path component - set-uri to set the whole URI - set-query to set the query string only - replace-uri to replace a part of the URI using parts from it In theory in order to perform some path rewrites, one would only need set-path. In Artur's case, it should be as simple as : http-request set-path /p3%[path] But now I'm starting to suspect that most of the problem comes from the fact that people who used to rely on regex in the past will not as easily perform their rewrites using set-path as they would using a replace rule which is very similar to the old set. So probably we'd need to introduce a "replace-path" action and suggest it in the warning emitted for reqrep. I think it is important that we properly address such needs and am willing to backport anything like this to 2.1 to ease the transition if that's the best solution. Please advise, Willy
Re: [2.1.1] http-request replace-uri does not work
On Tue, Dec 17, 2019 at 06:08:56AM +0100, Willy Tarreau wrote: > But now I'm starting to suspect that most of the problem comes from the > fact that people who used to rely on regex in the past will not as easily > perform their rewrites using set-path as they would using a replace rule > which is very similar to the old set. So probably we'd need to introduce > a "replace-path" action and suggest it in the warning emitted for reqrep. > > I think it is important that we properly address such needs and am > willing to backport anything like this to 2.1 to ease the transition if > that's the best solution. What about this ? It does exactly what's needed for me. It's self-contained enough that we could get it backported to 2.1 and maybe even to 2.0 (though it would require some adaptations to legacy mode there). Willy >From 290a8473a9a8d65685a6a3c4922a581eaa7c3377 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 17 Dec 2019 06:52:51 +0100 Subject: MINOR: http: add a new "replace-path" action This action is very similar to "replace-uri" except that it only acts on the path component. This is assumed to better match users' expectations when they used to rely on "replace-uri" in HTTP/1 because mostly origin forms were used in H1 while mostly absolute URI form is used in H2, and their rules very often start with a '/', and as such do not match. It could help users to get this backported to 2.0 and 2.1. --- doc/configuration.txt | 27 ++- src/cfgparse-listen.c | 2 +- src/http_act.c| 21 - 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index fdcdb04fae..3ba340c38d 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -4463,6 +4463,30 @@ http-request replace-header # outputs: User-Agent: foo +http-request replace-path + [ { if | unless } ] + + This works like "replace-header" except that it works on the request's path + component instead of a header. The path component starts at the first '/' + after an optional scheme+authority. It does contain the query string if any + is present. The replacement does not modify the scheme nor authority. + + It is worth noting that regular expressions may be more expensive to evaluate + than certain ACLs, so rare replacements may benefit from a condition to avoid + performing the evaluation at all if it does not match. + + Example: +# prefix /foo : turn /bar?q=1 into /foo/bar?q=1 : +http-request replace-path (.*) /foo\1 + +# suffix /foo : turn /bar?q=1 into /bar/foo?q=1 : +http-request replace-path ([^?]*)(\?(.*))? \1/foo\2 + +# strip /foo : turn /foo/bar?q=1 into /bar?q=1 +http-request replace-path /foo/(.*) /\1 +# or more efficient if only some requests match : +http-request replace-path /foo/(.*) /\1 if { url_beg /foo/ } + http-request replace-uri [ { if | unless } ] @@ -4484,7 +4508,8 @@ http-request replace-uri with HTTP/2, clients are encouraged to send absolute URIs only, which look like the ones HTTP/1 clients use to talk to proxies. Such partial replace-uri rules may then fail in HTTP/2 when they work in HTTP/1. Either the rules need - to be adapted to optionally match a scheme and authority. + to be adapted to optionally match a scheme and authority, or replace-path + should be used. Example: # rewrite all "http" absolute requests to "https": diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c index 507e071734..9975e46876 100644 --- a/src/cfgparse-listen.c +++ b/src/cfgparse-listen.c @@ -3694,7 +3694,7 @@ stats_error_parsing: } else if (!strcmp(args[0], "cliexp") || !strcmp(args[0], "reqrep")) { /* replace request header from a regex */ ha_alert("parsing [%s:%d] : The '%s' directive is not supported anymore since HAProxy 2.1. " -"Use 'http-request replace-uri' and 'http-request replace-header' instead.\n", +"Use 'http-request replace-path', 'http-request replace-uri' or 'http-request replace-header' instead.\n", file, linenum, args[0]); err_code |= ERR_ALERT | ERR_FATAL; goto out; diff --git a/src/http_act.c b/src/http_act.c index d6015d3624..61200ba368 100644 --- a/src/http_act.c +++ b/src/http_act.c @@ -142,6 +142,7 @@ static enum act_return http_action_replace_uri(struct act_rule *rule, struct pro enum act_return ret = ACT_RET_ERR; struct buffer *replace, *output; struct ist uri; + int action; int len; replace = alloc_trash_chunk(); @@ -149,6 +150,13 @@ static enum act_return http_action_replace_uri(struct act_rule *rule, struct pro if (!replace || !output) goto leave; uri = htx_sl_req_uri(http_get_stline(htxbuf(&s->req.buf))); + action = 3; // replace ur
Re: [2.1.1] http-request replace-uri does not work
On Tue, Dec 17, 2019 at 06:58:11AM +0100, Willy Tarreau wrote: > On Tue, Dec 17, 2019 at 06:08:56AM +0100, Willy Tarreau wrote: > > But now I'm starting to suspect that most of the problem comes from the > > fact that people who used to rely on regex in the past will not as easily > > perform their rewrites using set-path as they would using a replace rule > > which is very similar to the old set. So probably we'd need to introduce > > a "replace-path" action and suggest it in the warning emitted for reqrep. > > > > I think it is important that we properly address such needs and am > > willing to backport anything like this to 2.1 to ease the transition if > > that's the best solution. > > What about this ? It does exactly what's needed for me. It's self-contained > enough that we could get it backported to 2.1 and maybe even to 2.0 (though > it would require some adaptations to legacy mode there). Actually a slightly cleaner one that should even be backportable to 2.0. We could trivially extend it to also support the query string though I'm not convinced at all that it would make sense to have a regex-based "replace-query". Willy >From 126c472884112caa9a70ac6d2fea71062f36c5cb Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 17 Dec 2019 06:52:51 +0100 Subject: MINOR: http: add a new "replace-path" action This action is very similar to "replace-uri" except that it only acts on the path component. This is assumed to better match users' expectations when they used to rely on "replace-uri" in HTTP/1 because mostly origin forms were used in H1 while mostly absolute URI form is used in H2, and their rules very often start with a '/', and as such do not match. It could help users to get this backported to 2.0 and 2.1. --- doc/configuration.txt | 27 ++- src/cfgparse-listen.c | 2 +- src/http_act.c| 20 +++- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index fdcdb04fae..3ba340c38d 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -4463,6 +4463,30 @@ http-request replace-header # outputs: User-Agent: foo +http-request replace-path + [ { if | unless } ] + + This works like "replace-header" except that it works on the request's path + component instead of a header. The path component starts at the first '/' + after an optional scheme+authority. It does contain the query string if any + is present. The replacement does not modify the scheme nor authority. + + It is worth noting that regular expressions may be more expensive to evaluate + than certain ACLs, so rare replacements may benefit from a condition to avoid + performing the evaluation at all if it does not match. + + Example: +# prefix /foo : turn /bar?q=1 into /foo/bar?q=1 : +http-request replace-path (.*) /foo\1 + +# suffix /foo : turn /bar?q=1 into /bar/foo?q=1 : +http-request replace-path ([^?]*)(\?(.*))? \1/foo\2 + +# strip /foo : turn /foo/bar?q=1 into /bar?q=1 +http-request replace-path /foo/(.*) /\1 +# or more efficient if only some requests match : +http-request replace-path /foo/(.*) /\1 if { url_beg /foo/ } + http-request replace-uri [ { if | unless } ] @@ -4484,7 +4508,8 @@ http-request replace-uri with HTTP/2, clients are encouraged to send absolute URIs only, which look like the ones HTTP/1 clients use to talk to proxies. Such partial replace-uri rules may then fail in HTTP/2 when they work in HTTP/1. Either the rules need - to be adapted to optionally match a scheme and authority. + to be adapted to optionally match a scheme and authority, or replace-path + should be used. Example: # rewrite all "http" absolute requests to "https": diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c index 507e071734..9975e46876 100644 --- a/src/cfgparse-listen.c +++ b/src/cfgparse-listen.c @@ -3694,7 +3694,7 @@ stats_error_parsing: } else if (!strcmp(args[0], "cliexp") || !strcmp(args[0], "reqrep")) { /* replace request header from a regex */ ha_alert("parsing [%s:%d] : The '%s' directive is not supported anymore since HAProxy 2.1. " -"Use 'http-request replace-uri' and 'http-request replace-header' instead.\n", +"Use 'http-request replace-path', 'http-request replace-uri' or 'http-request replace-header' instead.\n", file, linenum, args[0]); err_code |= ERR_ALERT | ERR_FATAL; goto out; diff --git a/src/http_act.c b/src/http_act.c index d6015d3624..c8d9220feb 100644 --- a/src/http_act.c +++ b/src/http_act.c @@ -133,6 +133,8 @@ static enum act_parse_ret parse_set_req_line(const char **args, int *orig_arg, s * .arg.act.p[]. It builds a string in the trash from the format string * previously filled by function parse_replace_uri() an
Re: [2.1.1] http-request replace-uri does not work
On 17 Dec 06:58, Willy Tarreau wrote: > On Tue, Dec 17, 2019 at 06:08:56AM +0100, Willy Tarreau wrote: > > But now I'm starting to suspect that most of the problem comes from the > > fact that people who used to rely on regex in the past will not as easily > > perform their rewrites using set-path as they would using a replace rule > > which is very similar to the old set. So probably we'd need to introduce > > a "replace-path" action and suggest it in the warning emitted for reqrep. > > > > I think it is important that we properly address such needs and am > > willing to backport anything like this to 2.1 to ease the transition if > > that's the best solution. > > What about this ? It does exactly what's needed for me. It's self-contained > enough that we could get it backported to 2.1 and maybe even to 2.0 (though > it would require some adaptations to legacy mode there). > > Willy I am in favour of replace-path. Should we have a replace-domain or so for the first part of the abs url? -- (o-Julien Pivotto //\Open-Source Consultant V_/_ Inuits - https://www.inuits.eu signature.asc Description: PGP signature
Re: [2.1.1] http-request replace-uri does not work
On Tue, Dec 17, 2019 at 08:41:55AM +0100, Julien Pivotto wrote: > On 17 Dec 06:58, Willy Tarreau wrote: > > On Tue, Dec 17, 2019 at 06:08:56AM +0100, Willy Tarreau wrote: > > > But now I'm starting to suspect that most of the problem comes from the > > > fact that people who used to rely on regex in the past will not as easily > > > perform their rewrites using set-path as they would using a replace rule > > > which is very similar to the old set. So probably we'd need to introduce > > > a "replace-path" action and suggest it in the warning emitted for reqrep. > > > > > > I think it is important that we properly address such needs and am > > > willing to backport anything like this to 2.1 to ease the transition if > > > that's the best solution. > > > > What about this ? It does exactly what's needed for me. It's self-contained > > enough that we could get it backported to 2.1 and maybe even to 2.0 (though > > it would require some adaptations to legacy mode there). > > > > Willy > > I am in favour of replace-path. > Should we have a replace-domain or so for the first part of the abs url? Technically speaking it's called the authority. And since we take great care to keep it synchronized with the Host header field, actually one can simply decide to adjust the Host header to adjust this part. I'm just realizing that we should mention this in the doc as well. Willy