[squid-dev] [PATCH] auth_schemes directive

[Amos Jeffries]

On 28/11/2016 3:34 p.m., Alex Rousskov wrote:
> On 11/19/2016 02:15 AM, Amos Jeffries wrote:
>> On 19/11/2016 12:56 p.m., Eduard Bagdasaryan wrote:
>>> This patch introduces a new 'auth_schemes' squid.conf directive.
>>>
>>> This directive may be used to customize authentication
>>> schemes presence and order in Squid's HTTP 401 (Unauthorized) and 407
>>> (Proxy Authentication Required) responses. The defaults remain the same.
>>>
>>> Also refactored configuration code for options with custom
>>> ACL-controlled actions
> 
>> -1. This patch is conflating two major feature additions.
> 
> The patch does not add two major features. It only adds one regular feature.
> 
> 
>> The logic changes adding support for access controls 'kind' other than
>> allow/deny should be proposed as a separate feature addition.
> 
> Squid already supports many actions other than allow/deny. AFAIK, the
> proposed patch reduces code duplication in that existing code area while
> reusing the already working approach. No logic changes related to
> non-allow/deny actions are being proposed here. While splitting the code
> improvements from the new feature may be possible, their coexistence
> certainly does not warrant a -1 vote!
> 
> 
>> Secondly;
>>
>> IMO the access control for schemes is implemented wrong.
>>
>> For the past 10 years admin who have been requesting control over what
>> schemes get advertised to clients have been driven by two main use cases:
>>
>>  1) segmented networks where auth is done differently on each segment
>> (ie WAN vs LAN) but all segments share the one proxy.
>>  - either access controls to prevent some scheme being advertised to one
>> segment,
>>  - or being able to use one scheme name in several different ways
>> (different helper or realm etc.) .
> 
> 
> Or being able to control the order of schemes presented to the user.

Any HTTP client implementation which was coded to be properly compliant
with RFC 2616 and 2617 *will not* obey any ordering presented by Squid.
(Yes that was relaxed in RFC723x, but only to "ought to" and most
clients still obey the older MUST requirement).

We *already* have ordering ability in squid.conf. Many users have tried
to make use of it. The fact that it is not solving their issues is a
good demonstration about why/how the above RFC requirement is a killer
for any attempt to use ordering to control the clients behaviour.

IE6 was a notable exception that did follow the ordering when Basic was
first and why that ordering of auth_param was made significant back in
the early Squid-2.x sometime. It is now almost completely dead/absent as
a client though.

The current worst problem is Safari - for exactly the opposite reason.
It demonstrably ignores the list of schemes and uses only (and forever
looping) its first choice of "best" scheme. No others are considered nor
attempted.

I'm also aware of Firefix NTLM issues and Java, but those are not order
related. Are there any other client software with scheme ordering issues
I should be aware of?


> 
>> To resolve (1) fully we will need to have multiple Config objects with
>> the same scheme name and different config details.
> 
> The use cases covered by the proposed patch do not need different
> configurations for the same scheme.

Indeed, and that is probably why it works well for its own use cases.

> The proposed changes do not preclude
> such future support. Moreover, such future support will not address some
> of the use cases covered by the proposed patch. In other words, what has
> been implemented and your suggestion are complementary, not mutually
> exclusive.

For the use cases this patch resolves, that is true. For the other cases
my per-scheme ACLs proposal seeks to resolve as well, it is not.

We could add both ways. But that I expect will just lead to more
confusion. So lets try to avoid that.

> 
>> To add auth scheme access controls in a way that does not actively
>> prevent (1) from being implemented later IMO we need to add access
>> controls as a member of the Config object, *not* as a global access list.
> 
> Per-scheme access controls do not work for controlling the order of the
> presented schemes. The order is currently hard-coded. The proposed patch
> makes it configurable. Future patches may certainly add per scheme
> access controls and other knobs as needed. I see no reason why the two
> complementary mechanisms cannot coexist.

See above, ordering is provided already in current auth_param design.

My proposal uses that existing logic instead of adding a second config
setting that can contradict it and cause confusion.

For the uncommon case of needing different order of the same schemes for
different clients, multiple auth_param sections with different ACL lists
can be used. It is a little verbose, yes, but not a common need so as
you have argued in the past: we should not be optimizing for that type
of case at expense of the more common ones.


> 
> Also, while working on the proposed feature, we have discovered why
> adding per-scheme ACLs (or a special NONE value to auth_schemes) will
> create a negative side effect that the posted implementation avoids: The
> implemented code does not change authentication decision; auth_schemes
> cannot accidentally create a situation where no schemes exist for a
> transaction.

That is exactly the case in HTTP Authentication where 403 status code is
supposed to be used. It is explicitly one of the permitted auth states.

RFC 7231 section 6.5.3
"
   The 403 (Forbidden) status code indicates that the server understood
   the request but refuses to authorize it.
   ...

   If authentication credentials were provided in the request, the
   server considers them insufficient to grant access.  The client
   SHOULD NOT automatically repeat the request with the same
   credentials.  The client MAY repeat the request with new or different
   credentials.  However, a request might be forbidden for reasons
   unrelated to the credentials.
"

with some tie-in of "reasons unrelated" at the RFC 7235 section 2.1
final paragraph:
"
   HTTP does not restrict applications to this simple challenge-response
   framework for access authentication.  Additional mechanisms can be
   used, such as authentication at the transport level or via message
   encapsulation, and with additional header fields specifying
   authentication information.  However, such additional mechanisms are
   not defined by this specification.
"

Essentially anyone configuring these ACLs has constructed a non-standard
auth mechanism with additional special criteria surrounding the
credentials validity - and has told Squid how to process those extra
criteria in the form of ACL checks.


> Interpreting such a situation correctly is going to be
> tricky: Does it mean "allow access without authentication" or "deny
> access"? Or should Squid keep evaluating other http_access rules because
> the decision/ACLs may change later?

It means 403 Forbidden in HTTP terms.

IMO, that means in terms of ACLs a non-match has ocured for the auth ACL
being tested.

For the external ACL case any credentials the client did supply should
be able to pass in to the helper but not as %LOGIN / %un, that should be
'-'.


> 
> This "NONE" problem does not imply that multiple same-scheme
> configurations are a bad idea, of course. It only illustrates why the
> "Should I use this configuration for scheme X?" and "What schemes should
> I use for this transaction?" are two different questions, each deserving
> an answer in various real-world situations.
> 
> Please reconsider your initial evaluation.

I have and still do not want to see this go in as one change.

I'm happy with the structural/internal part making non-allow/deny
actions possible anywhere. If that were split from the auth_schemes
directive part it could go in immediately without another review IMO.

Amos