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

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.

The other comments probably require changes/development so I will
address them with a new version of the patch.

Thank you,

Alex.
Received on Wed Jul 11 2012 - 15:33:56 MDT