[squid-dev] [PATCH] invalid certificates and spliced connections

[Alex Rousskov]

On 12/22/2014 08:24 AM, Tsantilas Christos wrote:
> On 12/19/2014 10:03 PM, Amos Jeffries wrote:

>> Possibly a fast ACL is appropriate:
>>     ssl_bump_error allow/deny [acls...]
>>
>> Which is run to make the above decision. ONLY in the event that TLS
>> protocol syntax errors or malformations.  Not for cert/cipher/option
>> issues such as bad combinations of valid things, or insecure settings.

I predict it is going to be increasingly difficult to draw the line
around "protocol syntax errors or malformations" to deterministically
isolate those problems from everything else, including "bad combinations
of valid things, or insecure settings". There is usually a significant
gray area between "malformed" and "valid", where the decision depends on
the deployment environment and business logic.

We already have a very flexible certificate validator API and an
ACL-controlled SSL error handling logic. That ought to be sufficient as
far as allowing or denying SSL errors is concerned.

We lack configuration options to control honored error handling, but
that is a different problem. The default "terminate server and bump the
client to serve the error message" behavior suggested by Christos is
secure. It will not satisfy everybody; so future configuration options
will make that behavior configurable.


> I believe it is OK to let this patch as is for now.
> This patch will abort imediatelly:
>  - If no certificate found and checked using squid ssl verify procedure
> (mostly because the server response was malformed, or unsupported)
>  - if a server certificate validation failed
> 
> I believe this is handles most of the cases. The cipher or option issues
> may still be ignored, but this errors may caused because of unsupported
> features by openSSL library we are using.
> But still we have verify server certificates, and this is should be
> enough for security.


I agree with Christos here. SslBump configuration is already quite
complex. We should not add more configuration options unless absolutely
necessary. The default behavior that Christos is proposing solves the
security problem at hand. Let's postpone adding more options until there
are specific use cases that require them.


Thank you,

Alex.