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. > 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. 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. > 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. 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. 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? 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. Thank you, Alex. _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
