Re: [PATCH] Add auth_param request_format, request_realm to proxy authentication schemes

On 16/11/2013 6:13 a.m., Alex Rousskov wrote:
> On 11/15/2013 08:11 AM, Amos Jeffries wrote:
>> On 30/10/2013 5:13 a.m., Tsantilas Christos wrote:
>>> The attached patch add the "auth_param request_format" and "auth_param
>>> request_realm" to proxy authentication schemes.
>>>
>>> The request_format value used to define the format of the helper request
>>> line. It is a "quoted string" with logformat %macro support. A new
>>> %credentials macro can be used to supply user password and other
>>> scheme-dependent information to the helper.
>>>
>>> The request_realm is an authenticated users cache key format, needed
>>> when request_format feature is used to authenticate different users with
>>> identical user names (e.g., when user authentication depends on http_port).
>
>
>> I dont think the idea made it out of the IRC planning discussion properly.
>
> There was a detailed RFC posted after the informal IRC discussion. The
> RFC email date is October 10, 2013. It is rather unfortunate that your
> objections come so late. The primary purpose of RFCs is to prevent the
> waste of resources and confusion related to changing the primary
> functionality of the developed, tested, and often deployed features!
>

Which did not look much different to what we discussed on IRC.
You discussed there that teh request_realm parameter as alternative to
request_format, same as on IRC. This patch implementation puts the
simultaneously operating. Which will get us into trouble.

>
>> We need only _one_ format called realm_format.
>
> In other words, you want to restrict the proposed request_realm to its
> proposed default value, eliminating the need for an explicit
> request_realm configuration option, right?

No. Other way around. realm_format is nicely being appended to the
existing cache key. It needs likewise to be an append on the existing
helper lookup line instead of a full replacement of that line (which is
what request_format does here).

>
> Here is the proposed request_realm default, quoted from the patch:
>
>> + "request_realm" format
>> + Specifies a custom format for the authenticated user cache key.
>> + ... By default, Squid
>> + uses request_format as request_realm.
>
>
> Please note that here I am asking about the essence of your "_one_
> format" objection, not about the new option(s) names ("realm_format" is
> not a good name choice but we can discuss that minor detail separately)
> and not about restricting request_format values (which is a separate
> topic discussed below).

Yes. It clashes a bit with some schemes use of a realm parameter in
their protocols. BUT, it provides the same semantics and meaning as that
realm parameter.

Semantically for all auth schemes; we send a helper lookup "C R" where C
is credentils and R is the realm/domain/scope in which those credentials
are supposed to exist. It returns a true/false result about whether
those C exist within the R.

Some schemes provide a R within their on-wire syntax. Some do not. We
are permitted to manipulate that by increasing/shrinking an existing one
based on other information, or adding an R of our own when none is given
(ie Basic).

The core of what we need to protect is that:
1) the C part remains unchanged both on-wire and what we send to the helper.
2) the C+R part can be deterministically found by the proxy itself in
cache without a new helper lookup.

Replacing the entire helper lookup line allows that guarantee (1) to be
undermined.
Using separate values for cache and lookup keys allows the guarantee at
(2) to become non-deterministic.

>
>> The parameters sent to the helpers today are essentially mandatory for
>> those schemes validation process to operate correctly. The auth helpers
>> are semantically a validation API. The possibility of sending other
>> details to widen the authentication space around that validation is an
>> optional extra, not a replacement for any one of the mandatory parts or
>> the validation itself.
>> So we can add, but not allow subtraction from the details arleady sent
>
> I am OK with prohibiting the admin from subtracting (until there is a
> valid use case to relax that restriction). AFAICT, it means adding more
> code to police configured values (in hope to prevent an admin from
> misconfiguring their Squid).
>

No. I think it means a smaller set of patch changes.
- Removing half of the string management,
- that append can be done always without policing code,
- using append instead of replace avoids shuffling existing lookup
generation

Amos
Received on Sat Nov 16 2013 - 03:56:21 MST