Re: [PATCH] Support bump-ssl-server-first and mimic SSL server certificates

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
>
>
Received on Wed Jul 18 2012 - 11:47:39 MDT