Re: ACL block ignored in 2.0.25
On Mon, Nov 22, 2021 at 12:59 AM Olivier Houchard wrote:
> Hi Bart,
>
> On Thu, Nov 18, 2021 at 08:06:55PM +0100, Bart van der Schans wrote:
> > 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);
>
> Your patch has been committed as feeccc6e40e4d61b1e9fa84aca961aceabe39371
>
> Thanks a lot !
>
>
Thank you for the fast turnaround!
Bart
Olivier
>
>
Re: ACL block ignored in 2.0.25
Hi Bart,
On Thu, Nov 18, 2021 at 08:06:55PM +0100, Bart van der Schans wrote:
> 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);
Your patch has been committed as feeccc6e40e4d61b1e9fa84aca961aceabe39371
Thanks a lot !
Olivier
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
