I am working on merging bump-ssl-server-first to to trunk. Please my comments bellow to be sure that all are OK, before merging..
First of all I had forgot to mention that in the latests patches I post here (t10 and t11 patches), a new log formating code has added the ssl::bump_mode which logs the SslBump decisions:it prints "none", "client-first", "server-first" or "-" for no ssl-bump enabled ports. It is a small feature implemented after the initial patch posted in squid-dev. On 07/17/2012 04:57 AM, Amos Jeffries wrote: > On 17.07.2012 04:15, Tsantilas Christos wrote: >> This is one more patch for bump-ssl-server-first feature. >> This is handle most of Amos comments and allow use old ssl_bump syntax: >> ssl_bump allow/deny acl ... >> >> This patch try to implement the following rules: >> 1. Convert allow to client-first, with a deprecation warning. One >> such warning per config. >> 2. Convert deny to none, with a deprecation warning. One such warning >> per config. I must note here that currently the patch I post only one message printed on squid start-up or reconfigure, a "deny to none" warning or a "allow to client-first" warning message. I am thinking now that we may need a "deny to none" AND an "allow to client-first" . >> 3. If there was a conversion, make the implicit negation rule >> explicit by adding either "none all" or "client-first all" as >> appropriate. Emit a warning specifying which rule has been added. This >> will need to be done after the entire configuration has been parsed, of >> course. It uses the rrFinalizeConfig Runner. >> 4. Issue a fatal error if a mixture of old and new keywords is found. >> >> >> I am attaching two patches here. The first is the changes over the >> original bump-ssl-server-first patch, which requested by Amos. And the >> second is the final patch. >> >> Regards, >> Christos > > > Thank you. > > Are the new WARNING really that important that they always be at > CRITICAL level? > I would think the impact was a bit less (no impact in the case of deny) > s/DBG_CRITICAL/DBG_PARSE_NOTE(2)/ for the deny conversion? > s/DBG_CRITICAL/DBG_PARSE_NOTE(DBG_IMPORTANT)/ for the allow conversion? > > ... using the DBG_PARSE_NOTE(x) macro which is now available in trunk, > it bumps the message up to CRITICAL when -k parse is used, but outputs > at the indicated value during normal startup/reconfigure operations. OK. > > > Also, the "allow" message would seem to qualify for the "SECURITY > NOTICE:" prefix instead of just "WARNING:". > http://wiki.squid-cache.org/SquidFaq/SquidLogs#Squid_Error_Messages Do you mean the messages inside sslBumpCfgRr::run method or the "allow to client-first conversion" warning message? > > > > That polish can be done on commit. > > +1 from me now, with or without the above. > > > Amos > >
