Re: ACL block ignored in 2.0.25
Here is the patch:
Fix "block" ACL by reverting accidental removal of code in
8ab2a364a8c8adf0965e74a41a2ff3cebd43e7a9
---
diff --git a/src/cfgparse.c b/src/cfgparse.c
index 857321e1..78f0ed76 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -2823,6 +2823,16 @@ int check_config_validity()
}
}
+ /* move any "block" rules at the beginning of the http-request
rules */
+ if (!LIST_ISEMPTY(>block_rules)) {
+ /* insert block_rules into http_req_rules at the beginning
*/
+ curproxy->block_rules.p->n= curproxy->http_req_rules.n;
+ curproxy->http_req_rules.n->p = curproxy->block_rules.p;
+ curproxy->block_rules.n->p= >http_req_rules;
+ curproxy->http_req_rules.n= curproxy->block_rules.n;
+ LIST_INIT(>block_rules);
+ }
+
/* check validity for 'tcp-request' layer 4/5/6/7 rules */
cfgerr += check_action_rules(>tcp_req.l4_rules,
curproxy, _code);
cfgerr += check_action_rules(>tcp_req.l5_rules,
curproxy, _code);
--
On Thu, Nov 18, 2021 at 6:11 PM Olivier Houchard wrote:
> On Thu, Nov 18, 2021 at 05:59:24PM +0100, Bart van der Schans wrote:
> > A bit more digging:
> >
> > It looks like this commit broke it:
> >
> http://git.haproxy.org/?p=haproxy-2.0.git;a=commitdiff;h=8ab2a364a8c8adf0965e74a41a2ff3cebd43e7a9
> >
> > Re-adding the following block on line 2826 in cfgparse.c in 2.0.25
> "solves"
> > the issue but I have no idea if that is the right thing to do:
> >
> > - /* move any "block" rules at the beginning of the
> > http-request rules */
> > - if (!LIST_ISEMPTY(>block_rules)) {
> > - /* insert block_rules into http_req_rules at
> > the beginning */
> > - curproxy->block_rules.p->n=
> > curproxy->http_req_rules.n;
> > - curproxy->http_req_rules.n->p =
> curproxy->block_rules.p;
> > - curproxy->block_rules.n->p=
> > >http_req_rules;
> > - curproxy->http_req_rules.n=
> curproxy->block_rules.n;
> > - LIST_INIT(>block_rules);
> > - }
> >
> > Thanks
> > Bart
> >
> I think you are totally correct, and this is the right thing to do. That
> block is unrelated to the other changes in that commit, and probably
> should not have been removed.
>
> Regards,
>
> Olivier
>
>
Re: ACL block ignored in 2.0.25
On Thu, Nov 18, 2021 at 05:59:24PM +0100, Bart van der Schans wrote:
> A bit more digging:
>
> It looks like this commit broke it:
> http://git.haproxy.org/?p=haproxy-2.0.git;a=commitdiff;h=8ab2a364a8c8adf0965e74a41a2ff3cebd43e7a9
>
> Re-adding the following block on line 2826 in cfgparse.c in 2.0.25 "solves"
> the issue but I have no idea if that is the right thing to do:
>
> - /* move any "block" rules at the beginning of the
> http-request rules */
> - if (!LIST_ISEMPTY(>block_rules)) {
> - /* insert block_rules into http_req_rules at
> the beginning */
> - curproxy->block_rules.p->n=
> curproxy->http_req_rules.n;
> - curproxy->http_req_rules.n->p =
> curproxy->block_rules.p;
> - curproxy->block_rules.n->p=
> >http_req_rules;
> - curproxy->http_req_rules.n=
> curproxy->block_rules.n;
> - LIST_INIT(>block_rules);
> - }
>
> Thanks
> Bart
>
I think you are totally correct, and this is the right thing to do. That
block is unrelated to the other changes in that commit, and probably
should not have been removed.
Regards,
Olivier
Re: ACL block ignored in 2.0.25
A bit more digging:
It looks like this commit broke it:
http://git.haproxy.org/?p=haproxy-2.0.git;a=commitdiff;h=8ab2a364a8c8adf0965e74a41a2ff3cebd43e7a9
Re-adding the following block on line 2826 in cfgparse.c in 2.0.25 "solves"
the issue but I have no idea if that is the right thing to do:
- /* move any "block" rules at the beginning of the
http-request rules */
- if (!LIST_ISEMPTY(>block_rules)) {
- /* insert block_rules into http_req_rules at
the beginning */
- curproxy->block_rules.p->n=
curproxy->http_req_rules.n;
- curproxy->http_req_rules.n->p = curproxy->block_rules.p;
- curproxy->block_rules.n->p=
>http_req_rules;
- curproxy->http_req_rules.n= curproxy->block_rules.n;
- LIST_INIT(>block_rules);
- }
Thanks
Bart
On Thu, Nov 18, 2021 at 5:47 PM Bart van der Schans
wrote:
> Hi good folks!
>
> We ran into an issue with 2.0.25 that it ignores the "block" (deprecated
> but still valid) statement. The following works in 2.0.24 but not in 2.0.25:
>
> frontend http-in
> bind :80
> mode http
> logglobal
> // snip, all the usual stuff
>
> # block any unwanted source IP addresses or networks
> acl forbidden_src src 127.0.0.1
> block if forbidden_src
>
> With 2.0.24 as expected:
> $ curl http://localhost/adsf
> 403 Forbidden
> Request forbidden by administrative rules.
>
>
> With 2.0.25 (503 because no real backend in my test setup):
> $ curl http://localhost/adsf
> 503 Service Unavailable
> No server is available to handle this request.
>
>
> Wondering if I'm missing something or if this is a security issue. For now
> we are have quickly "patched" our prod servers by replacing "block" with
> "http-request deny" which does show the correct behavior.
>
> Any comments or suggestions?
>
> Thanks
> Bart
>
>
ACL block ignored in 2.0.25
Hi good folks! We ran into an issue with 2.0.25 that it ignores the "block" (deprecated but still valid) statement. The following works in 2.0.24 but not in 2.0.25: frontend http-in bind :80 mode http logglobal // snip, all the usual stuff # block any unwanted source IP addresses or networks acl forbidden_src src 127.0.0.1 block if forbidden_src With 2.0.24 as expected: $ curl http://localhost/adsf 403 Forbidden Request forbidden by administrative rules. With 2.0.25 (503 because no real backend in my test setup): $ curl http://localhost/adsf 503 Service Unavailable No server is available to handle this request. Wondering if I'm missing something or if this is a security issue. For now we are have quickly "patched" our prod servers by replacing "block" with "http-request deny" which does show the correct behavior. Any comments or suggestions? Thanks Bart
Re: [PATCH 1/2] BUILD: SSL: add quictls build to scripts/build-ssl.sh
In theory, we can drop QUICTLS/gcc build, leaving only clang.
let us spend some time on fixing and decide
чт, 18 нояб. 2021 г. в 18:48, Amaury Denoyelle :
> On Thu, Nov 18, 2021 at 06:27:56PM +0500, Ilya Shipitsin wrote:
> > script/build-ssl.sh is used mostly in CI, let us introduce QUIC
> > OpenSSL fork support
> > ---
> > scripts/build-ssl.sh | 23 +++
> > 1 file changed, 23 insertions(+)
> > diff --git a/scripts/build-ssl.sh b/scripts/build-ssl.sh
> > index e1d89a0eb..d143cec55 100755
> > --- a/scripts/build-ssl.sh
> > +++ b/scripts/build-ssl.sh
> > @@ -86,6 +86,17 @@ download_boringssl () {
> > fi
> > }
> >
> > +download_quictls () {
> > +if [ ! -d "download-cache/quictls" ]; then
> > +git clone --depth=1 https://github.com/quictls/openssl
> download-cache/quictls
> > +else
> > + (
> > +cd download-cache/quictls
> > +git pull
> > + )
> > +fi
> > +}
> > +
> > if [ ! -z ${LIBRESSL_VERSION+x} ]; then
> > download_libressl
> > build_libressl
> > @@ -121,3 +132,15 @@ if [ ! -z ${BORINGSSL+x} ]; then
> > )
> > fi
> >
> > +if [ ! -z ${QUICTLS+x} ]; then
> > +(
> > +
> > +download_quictls
> > +cd download-cache/quictls
> > +
> > +./config shared --prefix="${HOME}/opt"
> --openssldir="${HOME}/opt" --libdir=lib -DPURIFY
> > +make -j$(nproc) build_sw
> > +make install_sw
> > +
> > +)
> > +fi
> > --
> > 2.29.2.windows.2
> >
>
> Thank you for your patches. However, we should probably fix the GCC
> error reproduced on your repository before merging it. This will prevent
> to have a red test indicator indefinitely.
>
> --
> Amaury Denoyelle
>
Re: [PATCH 1/2] BUILD: SSL: add quictls build to scripts/build-ssl.sh
On Thu, Nov 18, 2021 at 06:27:56PM +0500, Ilya Shipitsin wrote:
> script/build-ssl.sh is used mostly in CI, let us introduce QUIC
> OpenSSL fork support
> ---
> scripts/build-ssl.sh | 23 +++
> 1 file changed, 23 insertions(+)
> diff --git a/scripts/build-ssl.sh b/scripts/build-ssl.sh
> index e1d89a0eb..d143cec55 100755
> --- a/scripts/build-ssl.sh
> +++ b/scripts/build-ssl.sh
> @@ -86,6 +86,17 @@ download_boringssl () {
> fi
> }
>
> +download_quictls () {
> +if [ ! -d "download-cache/quictls" ]; then
> +git clone --depth=1 https://github.com/quictls/openssl
> download-cache/quictls
> +else
> + (
> +cd download-cache/quictls
> +git pull
> + )
> +fi
> +}
> +
> if [ ! -z ${LIBRESSL_VERSION+x} ]; then
> download_libressl
> build_libressl
> @@ -121,3 +132,15 @@ if [ ! -z ${BORINGSSL+x} ]; then
> )
> fi
>
> +if [ ! -z ${QUICTLS+x} ]; then
> +(
> +
> +download_quictls
> +cd download-cache/quictls
> +
> +./config shared --prefix="${HOME}/opt" --openssldir="${HOME}/opt"
> --libdir=lib -DPURIFY
> +make -j$(nproc) build_sw
> +make install_sw
> +
> +)
> +fi
> --
> 2.29.2.windows.2
>
Thank you for your patches. However, we should probably fix the GCC
error reproduced on your repository before merging it. This will prevent
to have a red test indicator indefinitely.
--
Amaury Denoyelle
[PATCH 2/2] BUILD: SSL: add QUICTLS to build matrix
---
.github/matrix.py | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/.github/matrix.py b/.github/matrix.py
index 568676001..53b5e0f88 100755
--- a/.github/matrix.py
+++ b/.github/matrix.py
@@ -113,12 +113,13 @@ for CC in ["gcc", "clang"]:
"OPENSSL_VERSION=3.0.0",
"LIBRESSL_VERSION=2.9.2",
"LIBRESSL_VERSION=3.3.3",
+"QUICTLS=yes",
#"BORINGSSL=yes",
]:
flags = ["USE_OPENSSL=1"]
-if ssl == "BORINGSSL=yes":
+if ssl == "BORINGSSL=yes" or ssl == "QUICTLS=yes":
flags.append("USE_QUIC=1")
-if "OPENSSL_VERSION=3.0.0" in ssl:
+if "OPENSSL_VERSION=3.0.0" in ssl or ssl == "QUICTLS=yes":
flags.append('DEBUG_CFLAGS="-g -Wno-deprecated-declarations"')
if ssl != "stock":
flags.append("SSL_LIB=${HOME}/opt/lib")
--
2.29.2.windows.2
[PATCH 1/2] BUILD: SSL: add quictls build to scripts/build-ssl.sh
script/build-ssl.sh is used mostly in CI, let us introduce QUIC
OpenSSL fork support
---
scripts/build-ssl.sh | 23 +++
1 file changed, 23 insertions(+)
diff --git a/scripts/build-ssl.sh b/scripts/build-ssl.sh
index e1d89a0eb..d143cec55 100755
--- a/scripts/build-ssl.sh
+++ b/scripts/build-ssl.sh
@@ -86,6 +86,17 @@ download_boringssl () {
fi
}
+download_quictls () {
+if [ ! -d "download-cache/quictls" ]; then
+git clone --depth=1 https://github.com/quictls/openssl
download-cache/quictls
+else
+ (
+cd download-cache/quictls
+git pull
+ )
+fi
+}
+
if [ ! -z ${LIBRESSL_VERSION+x} ]; then
download_libressl
build_libressl
@@ -121,3 +132,15 @@ if [ ! -z ${BORINGSSL+x} ]; then
)
fi
+if [ ! -z ${QUICTLS+x} ]; then
+(
+
+download_quictls
+cd download-cache/quictls
+
+./config shared --prefix="${HOME}/opt" --openssldir="${HOME}/opt"
--libdir=lib -DPURIFY
+make -j$(nproc) build_sw
+make install_sw
+
+)
+fi
--
2.29.2.windows.2
