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

On 12.07.2012 03:33, Alex Rousskov wrote:
> On 07/11/2012 06:38 AM, Amos Jeffries wrote:
>
>> * why is is FATAL: error to have old ssl_bump allow/deny values in
>> the
>> config?
>> can we auto-convert the "allow" to client-first and deny to
>> bump-none
>> with very loud ERROR: messages instead? we can calculate what the
>> implicit negate would have been and add it explicitly with another
>> loud
>> warning.
>
> I have struggled with this decision. Yes, we could auto-convert old
> configs (using client-first for the implicit allow rule, if any), but
> then some folks will start using a mixture of old and new keywords or
> would expect a simple 's/allow/server-first/;s/deny/none/' to work. I
> think it is safer to force a manual intervention here, especially
> since
> SslBump config should not be taken lightly, and client-first is
> really
> not the best default.
>
> If the above arguments did not convince you, or others insist on
> auto-conversion, we will add it. We would still reject a mixture of
> old
> and new keywords though.
>
> Do you still think auto-conversion is the best approach here?
>
>
>> * Why are error pages delayed until after bumping?
>
> Because many browsers do not display error responses to CONNECT
> requests
> (browser developers do not want to implement a special "proxy"
> context
> that securely handling such responses requires).
>
>> have you investigated
>> how this interacts with deny_info redirection to 4xx and 5xx pages?
>> (particularly 403 and 511 "web login required" splash pages)
>
> I do not think we have, but perhaps those who requested this feature
> have. I will ask around, and we will "investigate" if nobody did.
>
>> src/url.cc:
>> * this breaks the canonical URI "cache" when URL parsing is changing
>> the
>> request URL pieces. Please revert.
>
> It does not. The cache is now invalidated in HttpRequest::setHost(),
> a
> call to which follows the removed explicit cache invalidation line.
> The
> whole canonical cache API is bad, of course, but we are not trying to
> fix it in this project. I can revert if you insist, but it will just
> result in us trying to free the canonical cache twice.

I understand, but in order to work with the bad API safely we need to,
for now, erase canonical whenever any of the URI parts change. The
double-free is why we use safe_free().

Amos
Received on Wed Jul 11 2012 - 22:58:18 MDT