Re: [PATCH] [MEDIUM] access control (block) rework - rfc

From: Willy Tarreau <w#1wt.eu>
Date: Thu, 29 Jan 2009 07:40:09 +0100


On Thu, Jan 29, 2009 at 01:38:13AM +0100, Krzysztof Oledzki wrote:
>
>
> On Wed, 28 Jan 2009, Willy Tarreau wrote:
>
> <CUT>
>
> >>We may start with implementing a sequential ordering for req (request) and
> >>supporting three default targets: deny, tarpit and allow.
> >
> >And "redirect" please ;-)
>
> If we do this right there will be no need to add a special "redirect"
> target as it should be possible to create it with:
> - insert header (Location)
> - deny with custom http reply code (3xx instead of 403)

Oops, I've just noticed that "redirect" was only added *after* 1.3.15, not before. Take a look at the doc, you'll notice it's more convenient and flexible than mangling headers and return codes ;-)

> Allowing such possibility *in the right way* is one of my current goals,
> however as each change requires some thinking and discussing I prefer to
> finish one stage before starting with a next one. I have developed too
> many custom patches for haproxy, I really need to clean my code and push
> them upstream, one by one. ;) Keeping them outside the mainline cost me
> too much.

No problem, I was not asking you to implement an add-on, just not to forget to include that one too, but I did not realize that it was not present in 1.3.15.

> >In fact, I think you should not touch "block" for now, as it's evaluated
> >very early, independant of this new sequence. We would just have to emit
> >a big warning when loading a new config using "block" and suggest moving
> >those lines to the top of the ACLs and changing it with "req deny".
>
> Or we may simply handle it as "req deny" + emitting standard "deprecated"
> warning.

No, because "req deny" should respect a sequence, while "block" is documented as always matching very early. Some users might have things like this :

    use_backend XXX if blah
    block if foo

"block" will block before "use_backend" here and is documented as such. If we simply convert it where it is, we'll cause trouble. However, I agree to convert it if we're able to merge them at the very beginning of the rules. It's a bit tricky, but it can be done (basically, for each "block", scan the list of rules, and insert a "req deny" after the end of the first "req deny" sequence).

(...)
> >hehe I thought about that this morning when going to work :-)
> >Indeed, parenthesis are a bad idea. But having a method for
> >grouping is still needed, or at least being able to specify a
> >modifier (-i), an operator (-gt, -lt) and a value. Maybe we
> >should imply enclose them by parenthesis, something like that :
> >
> > req allow if (hdr(host) -i www) (src 10.0.0.0/8)
>
> OK, but if we do this we should, at the same time, deny creating acls with
> names containing "acl" and/or parenthesis.

100% agreed.

> >>Maybe it would be better to add two new keywords: "ifacl" or "unlessacl"?
> >
> >If I understand what you propose, I don't think it will solve the
> >issue as I don't see how I could combine two ACLs with this method.
> >However, a minor modification might consist in having an "acl()"
> >function :
> >
> > req allow if acl(hdr(host) -i www) ...
>
> Ack, this looks wisely.

OK, let's agree on this then.

(...)
> >Yes I agree with it provided you also include "redirect" and you
> >keep in mind that later we'll make it evolve towards "in" and "out",
> >with probably "in" being the default when not specified.
>
> I think we need to discuss more on this "redirect" target. However I
> believe whatever solution we will choose it should be possible to add it
> by simply extending current syntax.

yes, look at the code, it's very simple.

> >BTW, may be you have noticed, I have updated the master git tree
> >on the site (and a snapshot was published yesterday). The HTTP
> >part is far more easy to modify without being perturbated by
> >lower layers, and vice-versa. Those who want tons of new features
> >and new L7 protocols (POP, SMTP) will be happy. At least, I hope :-)
>
> Yep. ;) I have a local copy of the git repository so if I only have
> enough time I trace each change as soon as you push it. ;)

hehe so you might have been waiting a long time, because I only restarted pushing a few days ago ;-)

You will save some time by diffing 1.3.15/doc/configuration.txt and git/doc/. I have not yet documented the new splice options though.

Best regards,
Willy Received on 2009/01/29 07:40

This archive was generated by hypermail 2.2.0 : 2009/01/29 07:45 CET