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.
_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev
