Re: Spaces in ACL values

On 15/09/2012 2:23 a.m., Alex Rousskov wrote:
> On 09/14/2012 05:41 AM, Amos Jeffries wrote:
>> On 14/09/2012 10:16 a.m., Alex Rousskov wrote:
>>> Hello,
>>>
>>> As you know, most old squid.conf options cannot handle spaces in
>>> parameter values as they do not support quoted strings or other ways of
>>> escaping the space separator. A few new options, support quoted strings,
>>> but many old ones do not. This is not going to be pretty because there
>>> is no backward compatible way to introduce quoted strings (or
>>> equivalent) that would work for every option. This email focuses on ACLs.
>>>
>>> A good specific example is user_cert and ca_cert ACLs that are supposed
>>> to match on various certificate attributes. In many, if not most, cases,
>>> attributes like Organization Name and Common Name contain spaces. For
>>> example:
>>>
>>> O: Google Inc
>>> O: Thawte Consulting (Pty) Ltd.
>>> CN: Thawte SGC CA
>>>
>>> All of the above cannot be matched using user_cert and ca_cert ACLs.
>>>
>>> I see a few reasonable ways to address this (see below for a list). They
>>> all revolve around introducing support for "quoted strings" in ACL
>>> values[1]. For loading files, they could all use file:"/path/name.ext"
>>> syntax.
>>>
>>> Each approach differs in how it addresses backward compatibility:
>>>
>>>
>>> 0) Screw backward compatibility and just start interpreting "quoted
>>> strings" in ACL values as such. We can minimize breakage by detecting
>>> unusual patterns such as quotes"in"the"middle and
>>> "/quotes/without/any/spaces", treating them as the old parser would
>>> (with a warning). We can also refuse to honor more than one value per
>>> ACL when a value is quoted. Very few old configurations will be broken
>>> by this, but I bet there will be some!
>>>
>>> # this will work OK
>>> acl badOne1 user_cert CN "Bad Guy"
>>>
>>> # this will also work as before, with a warning
>>> acl badOnes user_cert CN "/var/share/bad.guys"
>>>
>>> # this will be silently broken
>>> acl badOnes user_cert CN "/var/share/my bad guys list"
>>>
>>> # new file loading syntax required here:
>>> acl badOnes user_cert CN file:"/var/share/my bad guys list"
>> I vote for this, with a few more changes that will reduce the breakages.
> Interesting. I did not expect much support for this, but two out of
> three responses so far suggest this approach, essentially. When the dust
> settles, perhaps we should post to squid-users as well to get more feedback?
>
>
>> IMO, the above final line should be invalid. "" should delimit one item,
>> be that a value with spaces or a whole file: URI with spaces. ie:
>> acl badOnes user_cert CN "file:/var/share/my bad guys list"
> Agreed.
>
>
>> This can be done in stages to remove problems.
>> 0a) add file: schema support and upgrade-warn on all "" entries. (3.3?)
> We can do that in v3.2, I think, because it does not force admins to
> rewrite their configs and does not change the meaning of their configs.
> It is just a deprecation warning.
>
>> stat() on the quoted entry can be used to verify it is a file and
>> assume a value if not. Don't bother with pattern checks we won't get
>> those reliable enough. stat() can do it.
>> 0b) drop default assumption that "" is a file entirely (3.x?)
> Then this can be done in v3.3 (if the code is ready in time for that
> release).

Unless you have something ready to merge now I think it won't be.
Branching occurs in just a few days. The file: deprecation as you say is
seamless and suitabe for back-port.

Amos
Received on Fri Sep 14 2012 - 23:21:55 MDT