Re: [PATCH] AND and OR ACLs v15

On 05/14/2013 01:34 AM, Amos Jeffries wrote:

> I'm still stuck on muliple all-of lines ORing together for the AND acl
> type. It really is much simpler to think of and describe "acl X foo ..."
> as X being a test of a set of values with operation foo applied to the
> whole set.

Yes, but ANDing all-of lines leads to serious inconsistency because all
other ACL types OR their lines. We would have to explain that
inconsistency too, which would be even harder and which would confuse
some admins just as much. Besides being more consistent, the proposed
implementation results in more succinct rules because it does not
require an extra any-of "aggregation" line in many cases.

Does anybody else have an opinion? The choice is between:

  # Implemented in the proposed patch:
  # What should we deny? One "bad" acl line per condition:
  acl bad all-of !authorized access_to_sensitive_area
  acl bad all-of !userA access_to_userA_area
  acl bad all-of !userB access_to_userB_area
  http_access deny bad

versus

  # Suggested as an easier-to-grok alternative:
  # What should we deny? One unique acl line per condition:
  acl bad1 all-of !authenticated access_to_users_area
  acl bad2 all-of !userA access_to_userA_area
  acl bad3 all-of !userB access_to_userB_area
  acl bad any-of bad1 bad2 bad3
  http_access deny bad

Both constructs result in essentially the same ACL expression tree. The
first (proposed) approach is consistent with how all other ACLs work and
allows for simple aggregation of rules, including rules loaded from
files using "quoted file name" syntax.

Note that if we AND all-of lines, loading all-of rules from an ACL file
would become impossible in most interesting cases because no natural
condition can be described as a long list of simple ANDed expressions.
By the very nature of the AND operation, AND lists tend to be short. OR
lists tend to be long. Current all-of implementation allows for loading
of long all-of lists from a file without creating a large number of
otherwise not needed unique ACLs.

If there is no consensus, I think we should remain consistent while
acknowledging the problem.

> In abstract I think we should define ACL types as each performing a
> *single* operation on a set of values.

That is not how ACL types are defined today. They all perform two
operations: a type-specific match followed by ORing of the type-specific
match results. This two-operation logic is often hidden behind splay
trees and other data structures, but it is there.

In most cases, the type-specific match has only one configurable
parameter, but there are ACLs that take more than one parameter per
type-specific match, including:

  acl aclname time [day-abbrevs] [h1:m1-h2:m2]
  acl aclname req_header header-name any\.regex\.here

The all-of ACL is similar, but has a variable number of parameters for a
type-specific match:

  acl aclname all-of acl1 [acl2 ...]

> With this patch all-of is
> applying a much more complex multi-type calculation which is not safe to
> describe as an AND test. I can't put my finger on a good example case
> but I'm sure this will come back to bite us (or a client) when we get to
> adding tests like this with large sets of values:
>
> acl magic all-of A
> acl magic all-of B
> acl voodoo all-of magic !B
>
> when A=true, B=false ==> voodoo is true.

I do not understand the point behind the above example, but I agree that
some admins will misinterpret and misapply new ACLs types. However, I
think that will happen no matter how we structure them.

If we want to play it safe at the expense of folks who know what they
are doing, we can prohibit multiple lines for all-of ACLs on the grounds
that they are "too confusing for admins". I do not think we should do
that though. It feels very wrong to cripple a powerful optional tool
just because somebody will misuse it. However, we may want to do that
when we add an "=" acl:

    acl good = (a or b) and !c

> I don't think we should be forcing this level of mathematical knowledge
> on people just to write ACLs.

We are not forcing the use of these new ACLs on anybody, but yes, basic
boolean arithmetic knowledge is required to write correct ACLs and yes,
folks will screw up more with more powerful tools.

I hope others will chime in on this discussion.

Thank you,

Alex.
Received on Tue May 14 2013 - 16:10:35 MDT