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. 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 >> 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 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. >> 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. >> 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 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. Thank you, Alex. _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev