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

On 11/19/2013 04:01 PM, Amos Jeffries wrote:
> On 2013-11-20 08:11, Alex Rousskov wrote:
>> On 11/19/2013 02:54 AM, Tsantilas Christos wrote:
>>
>>> My understanding is that we need:
>>> 1) Allow configuring the request format using one of the following:
>>> a) Use a request_format configuration parameter plus the
>>> %credentials formating code
>>> b) Use the following request format:
>>> [prefix] credentials [key_extras]
>>
>> Yes. And since the official code implements (b) already (without
>> key_extras), it may be best to add key_extras processing to that code
>> rather than to add [hidden] %credentials functionality. To avoid redoing
>> this code for the third time, I recommend getting a clear answer from
>> Amos first:
>>
>> Amos, do you want Christos to yank all internal support for %credentials
>> macro, going back to the hard-coded generation of the credentials part
>> of helper requests? Or are you OK with Squid supporting %credentials
>> macro, and the patch using that internal support to generate the helper
>> request? In both cases, the admin will not use %credentials in
>> key_extras configuration (because credentials info will be automatically
>> prepended by Squid in both cases).
>
> I'm in two minds about the %credentiasl macro itself. It makes sens as a
> separate thing for logging those details, or using in other helpers
> formats (rewriter, exetrnal ACL, etc).

Agreed, it is only potentially useful. Note that, in most cases, it
would be useful keep KK/etc. a part of the %credentials value so that
the recipient of NTLM/Negotiate credentials can tell what the
logged/received value means.

> But it is definitely not necessary for what this patch needs to do, IMO.

Agreed, it is not essential, especially given your restrictions on the
overall functionality. Hard-coded credentials code is sufficient.

> If you want to omit it from this patch and do it as a second one that
> would be okay.

I do not like the idea of increasing the amount of work further by
splitting this feature into two. I think your choices are:

1) Allow %credentials to be used internally in the key_extras patch.
This is less work for Christos (because the code is already written) and
adds a potentially useful feature to Squid.

2) Object to %credentials in the key_extras patch. This makes the final
patch simpler/smaller.

Please pick one.

> The need for it would be quite rare, such as admin passing the helper
> values used directly as SQL query parameters.

Agreed.

Thank you,

Alex.
Received on Tue Nov 19 2013 - 23:49:25 MST