[squid-dev] [RFC] simplifying ssl_bump complexity

[Marcus Kool]

On 11/27/2016 11:20 PM, Alex Rousskov wrote:
> On 11/19/2016 07:06 PM, Amos Jeffries wrote:
>> On 20/11/2016 12:08 p.m., Marcus Kool wrote:
>>> The current ssl bump steps allow problematic configs where Squid
>>> bumps or stares in one step and to splice in an other step,
>>> which can be resolved (made impossible) in a new configuration syntax.
>
> It would be nice to prohibit truly impossible actions at the syntax
> level, but I suspect that the only way to make that possible is to focus
> on final actions [instead of steps] and require at *most* one ssl_bump
> rule for each of the supported final actions:
>
>   ssl_bump splice    ...rules that define when to splice...
>   ssl_bump bump      ...rules that define when to bump...
>   ssl_bump terminate ...rules that define when to terminate...
>   # no other ssl_bump lines allowed!
>
> The current intermediate actions (peek and stare) would have to go into
> the ACLs. There will be no ssl_bump rules for them at all. In other
> words, the admin would be required to _always_ write an equivalent of
>
>   if (a1() && a2() && ...)
>   then
>       splice
>   elsif (b1() && b2() && ...)
>   then
>       bump
>   elsif (c1() && c2() && ...)
>   then
>       terminate
>   else
>       splice or bump, depending on state
>       (or some other default; this decision is secondary)
>   endif
>
> where a1(), b2(), and other functions/ACLs may peek or stare as needed
> to get the required information.

The above if-then-else tree is clear.
I like your suggestion to drop steps in the configuration and make
Squid more intelligent to take decisions at the appropriate
moments (steps).

You mentioned admins being surprised about Squid bumping for a
notification of an error and one way to improve that is to replace
'terminate' by 'terminate_with_error' (with bumping) and 'quick_terminate'
(no bumping, just close fd).  The quick_terminate, if used, is also
faster, which is an added benefit.


> I am not sure such a change is desirable, but it is worth considering, I
> guess.
>
> Please note that I am ignoring the directives/actions naming issue for now.
>
>
> AFAICT, the syntax proposed by Amos (i.e., making stepN mandatory) does
> not solve this particular problem at all:
>
>   # Syntactically valid nonsense:
>   ssl_bump_step1 splice all
>   ssl_bump_step2 bump all
>
>
> and neither is yours:
>
>   # Syntactically valid nonsense?
>   tls_server_hello passthrough all
>   tls_client_hello terminate all
>

Correct.  It would be nice to have a better configuration syntax
where impossible rules are easier to avoid and/or Squid has
intelligence to detect nonsense rules and produce an error.

>>> Below is a new proposal to attempt to make the configuration
>>> more intuitive and less prone for admin misunderstandings.
>>>
>>> First the admin must define if there is any bumping at all.
>>> This could be done with
>>> https_decryption on|off
>>> This is similar to tls_new_connection peek|splice but much
>>> more intuitive.
>
> I do not see why
>
>   https_decryption off
>
> is more intuitive (or more precise) than
>
>   ssl_bump splice all

For me this is because of the used terminology and because
with 'https_decryption off' one does not write anything
that has 'bump' in it, so the admin does not even have to
read the documentation to learn that 'ssl_bump splice all'
means 'no decryption'.

> especially after you consider the order of directives.
>
> Again, I am ignoring the naming issue for now. You may assume any name
> you want for any directive or ACL.

allright for now.
The only comment that I want to make without starting a
new thread is that I think that conceptual terms are better
than technical terms (hence my preference for 'passthrough'
instead of 'splice').  But let's save this discussion for later.

>>> Iff https_decryption is on:
>>>
>>> 1) the "connection" step:
>>> When a browser uses "CONNECT <FQDN>" Squid does not need to make
>>> peek or splice decisions.
>>> When Squid intercepts a connection to "port 443 of <IP>" no peek
>>> or splice decision is made here any more.
>>> This step becomes obsolete in the proposed configuration.
>
>> I am still hoping that will happen. But also still getting pushback that
>> people want to terminate or splice without even looking at the
>> clear-text hello details.
>
> I suspect that not looking at some SSL Hellos will always be needed
> because some of those Hellos are not Hellos at all and it takes too much
> time/resources for the SSL Hellos parser to detect some non-SSL Hellos.
> Besides that, it is always nice to be able to selectively bypass the
> complex Hello parser code in emergencies.

Perhaps less resources are used if there is a two-stage parser:
1) quick scan of input for data layout of a ClientHello without
    semantic parsing the content, e.g. look at the CipherSuite and verify
    that the whole field has legal characters without verifying
    that the ciphersuite is a valid SSL string of ciphers.
2) do the complex parsing.

Stage 1 should be fast and can separate SSL ClientHello from other
protocols.

>>> 3) the "TLS server hello" step:
>>> Usually no directives are needed since rarely actions are taken
>>> based on the server hello message, so the default is
>>> tls_server_hello continue
>>
>> Once Squid gets to this stage of processing "continue" could mean either
>> splice or bump, but only one of the two is possible.
>>
>> Bump is not legal in a lot of situations but splice may not be possible.
>> So terminate is the only realistic default we can set for general use.
>> As you say, its uncommon to need it, and people who do tend to know what
>> they want done. So will configure that.
>
> Just like naming, the defaults are probably not that important yet: If
> we can agree on the new configuration approach, then we probably will be
> able to agree on the defaults.
>
> However, all proposals have to decide how to distinguish stare from peek
> during step2. Those two intermediate actions result in different bytes
> on the wire so they cannot be mapped to a single verb like "continue".

An issue that complicates things is an "impossible decision",
the desire to splice when "it is too late".  I suggest to
solve this issue by simply splicing a bumped connection.
On the bumped connection Squid can also splice/passthrough:
just do not interpret the data and copy it from client to server
and vice versa.
Yes I realize that the connection is bumped and that on technical
grounds one can argue that the connection is not really spliced,
but on a conceptual level the connection _is_ in a 'passthrough mode'
and that is what 'splice' means or should mean.

With the concept of splicing a bumped connection, the problem
of making which decision at step2, if not yet made by some acls,
can be solved.

And I think that we do not have to be afraid of CPU cycles spent
in TLS/SSL encryption since modern multi-core systems are fast
with encryption:
https://software.intel.com/en-us/articles/accelerating-ssl-load-balancers-with-intel-xeon-v3-processors
and Intel also has the quickassist hardware accelerator to create a powerful proxy:
http://www.intel.com/content/www/us/en/ethernet-products/gigabit-server-adapters/quickassist-adapter-for-servers.html
which is supported by OpenSSL:
http://www.intel.com/content/dam/www/public/us/en/documents/solution-briefs/accelerating-openssl-brief.pdf

>>> An example configuration looks like this:
>>> https_decryption on
>>> acl banks tls::client_servername .bank1.example.org
>>> acl no_sni tls::client_hello_missing_sni
>>> acl no_handshake tls::handshake_failure
>>> acl hacked_server tls::server_servername evil.example.com
>>> tls_client_hello passthrough banks
>>> tls_client_hello terminate no_sni
>>> tls_client_hello passthrough no_handshake
>>> tls_client_hello continue
>>> tls_server_hello terminate hacked_server
>>> tls_server_hello continue
>>> tls_no_http passthrough
>>>
>>> The configuration directives as I proposed are IMO intuitive and
>>> leave very little room for misunderstandings.
>
> ... if you use them correctly. However, the same is true for the current
> directives so this is not really an illustration of improvement AFAICT.
> For example, what does the following [mis]configuration do?
>
>   https_decryption off
>   tls_server_hello continue
>   tls_server_hello terminate hacked_server
>   https_decryption on
>   tls_client_hello continue
>   tls_client_hello passthrough banks
>
> If I understand the proposals correctly, the above will probably splice
> or perhaps bump everything but I am obviously not sure. I do not see an
> an improvement as far as possible confusion/misunderstanding is concerned.
>
> Again, I am ignoring the naming issue. If, for example, there is
> consensus that "passthrough" is better than "splice" or "continue2" is
> better than "stare" then we can, of course, add aliases and eventually
> completely get rid of the old names. Let's focus on the functionality first.

You are good in finding examples of nonsense configurations :-)
And I think that a syntax that does not permit nonsense configurations,
does not exist.  The question is what to do.

If a new syntax is not based on steps, the config can just have a list
of rules and instead of Squid obeying the order of the TLS rules at
all times, it can gain intelligence and at the time that Squid is done
reading the configuration, validates whatever it can validate and makes
the decisions about which acls are executed/verified at what time (and
also does some runtime checks at the appropriate times).
So the config for 'bump all except banks' could look like this:
tls decryption on
tls default bump
tls splice banks
tls terminate clients_from_subnet_x
tls terminate hacked_server
tls splice no_http_protocol_inside_tls_wrapper  # default
tls splice no_tls_protocol                      # default
tls terminate tls_without_sni

The above is elegant and I think that the required intelligence to
decide to evaluate which rule at what step is not impossible.

Squid must not allow a mix of 'tls decryption on' and
'tls decryption off'.

Simple examples like above 'work'.  The question is if it
is possible for Squid to detect most or all nonsense configurations
and generate a (fatal?) error.

Marcus

>
> Thank you,
>
> Alex.
>