On 30/10/2013 5:13 a.m., Tsantilas Christos wrote: > Hi all, > > 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. We need only _one_ format called realm_format. The helper lookup can be extended "$credentials $realm_format" and the cache key made "user $realm_format". Where $credentials is the data already sent to the helper. 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 an dhave to be very particular about making the extensions identical in both lookup line and cache key. For schemes like Digest doing it any other way compromises the message user+realm fields which are critical for correct nonce hash generation by exposing them unnecessarily to the admin. Similar nasty things happen in the other schemes. In Basic auth there is a lot more flexible about what the helper can do with the credentials, but that is no reason to elide them and make it an exception for the one helper type. The helper can simply ignore user+pass fields and use only the realm extensions details. Even in Basic allowing two distinct formats for lookup and cache key enables the dangerous configurations such as lookup "%credentials %>lp" mapped to cache key "%>a %>lp". > The Format::Format class used to produce the helper request line. This > class requires the AccessLogEntry, so passing this class to various > squid subsystems required. > > Also this patch try to fill AccessLogEntry::cache::caddr (%>A) and > AccessLogEntry::cache::port (%la, %lp) members at the time the > AccessLogEntry build, instead of computing them just before the request > logged, to make them available for Format::Format class. > > Regards, > Christos > in src/adaptation/AccessCheck.cc * please use const AccessLogEntry::Pointer & instead of AccessLogEntry::Pointer & for the al parameter of Adaptation::AccessCheck::Start(). - same in many other method parameter lists. in src/auth/Config.cc: * for Auth::Config::dump can you avoid use of the String::defined method please? I belive these make the same sense using size()>0 since we definitely do not want 0-length strings to be used. * for Auth::Config::done() we do not have to wrap delete in an if(exists) pattern. Just delete and set to NULL afterwards. in src/auth/User.cci: * please remove the useless new whitespace after xstrdup(aString);_________\n in src/auth/User.h: * please use SBuf for new variables instead of String. in src/auth/UserRequest.cc: * in Auth::UserRequest::helperFormatedRequest s/Assebled/Assembled/ * I can almost guarantee that somebody is going to want to send Cookie or other large details to the helper. Helpers are already required to handle up to 32KB lines anyway. - So the MemBuf might as well be initialized with the defaults with mb.init(); - OR, since its content is being printed to buf. Use size parameter as the upper limit for: mb.init(1024, size-1); in src/auth/basic/auth_basic.cc: * in authBasicAuthUserFindUsername() please remove HERE from debugs when altering the line. in src/auth/ntlm/UserRequest.cc: * the YR and KK are lookups codes, not part of the credentials. They must be first on the helper query line and not manipulable by the admin. - same problem in Negotiate as well. Amos
