On 12/30/2014 06:19 PM, Amos Jeffries wrote: > On 31/12/2014 7:30 a.m., Alex Rousskov wrote: >> On 10/21/2014 11:29 AM, Tsantilas Christos wrote: >>>>> - Adds "on_first_request_error", a new ACL-driven squid.conf >>>>> directive that can be used to establish a blind TCP tunnel >>>>> which relays all bytes from/to the intercepted connection >>>>> to/from the intended destination address. See the sketch >>>>> above. The on_first_request_error directive supports fast >>>>> ACLs only.
> IMO this directive needs to be friendly and clear as its going to be > somewhat popular. Agreed. I am not going to respond to most of your thoughts regarding the directive name. Suffice to say that I disagree with most of your analysis of natural language issues, but do not have enough cycles to engage you on that exciting topic. For now, let's focus on the proposed feature itself and decide on the final directive(s) naming later. Most of my post was not about naming at all. It was about three ways to control non-HTTP bypass functionality. In your response, you have not addressed those points at all, so I assume you interpreted them as a discussion about naming, which rendered my last email mostly useless. However, since you seem to be OK with the updated patch going in with some to-be-determined directive name, perhaps we are making progress after all! > The "on_error" as you say is already fairly well known. In particular > its known as an event handler hook in JavaScript and other languages. > Like I said above squid.conf is more a declarative language than those. Well-known event handlers use (and often combine) two very different concepts: * A declaration that an error is handled by a named handler: on_error=foo * An inline implementation of an anonymous handler: on_error="for (i = 0; i < count; ++i) { foo(i); ..." ACL-driven directives in Squid essentially combine handler declarations with a bunch of if statements similar to an inline implementation: on_error="if acl1 then foo1; elsif acl2 then foo2; else foo3;" What Squid uses is more declarative than some well-known on_error handler approaches and less declarative than others. Even if it were possible, deciding the exact degree of "declarativeness" is not going to help us make progress here IMO. What would help is to decide whether we want to focus on A) multiple conditions for establishing a TCP tunnel; B) multiple ways to handle an unrecognized protocol error; OR C) multiple ways to handle multiple errors. IMO, we want (B) or perhaps (C) while leaving (A) as a separate out-of-scope feature. The proposed patch implements (B). To implement (C), the patch needs to add an ACL type to distinguish an "unrecognized protocol" error from other errors. > 2) "first_request_error" [...] is actually > declaring an action to perform *instead* of producing errors No. It declares actions to perform when an error is detected. Those actions may and often do include "producing an error" to the user, of course. The fact that there may be many different actions is critical here because the tcp_tunnel alternative does not handle multiple actions well -- it focuses on one "tunnel" action only. > 3) This directive is not relevant to HTTP, but to SSL/TLS. Not exactly. The feature deals with protocols that Squid does not recognize. We may deal with plain and encrypted HTTP connections today, but we will probably have to deal with FTP, Web Sockets, and other protocol connections tomorrow. SSL/TLS is just a small slice of that puzzle. > Coming back to this now (and accepting your scope narrowing from tcp_ > to ssl_) we should probably call this ssl_unknown_protocol. Since > anything on port 443 which is syntactically HTTP but has some > invalidity is rightfully responded with an error, no need to ask the > admin at that point. We do _not_ want to limit this feature to receiving a non-SSL/TLS protocol on an intercepting https_port. We want to support, at least: * non-HTTP traffic on intercepted http_port * non-SSL/TLS traffic on intercepted https_port * non-HTTP traffic inside successfully bumped http_port CONNECT tunnel * non-HTTP traffic inside successfully bumped https_port SSL/TLS tunnel Moreover, if I am interpreting your "syntactically HTTP but invalid" condition correctly, then many admins will disagree with the universal rightfulness of sending the user an error under those conditions. > PS. can we please agree to start using tls_ instead of ssl_? even > SSLv3 is a dead duck now and everybody knows it. At least on new > directives. I am glad SSLv3 is dying, but the degree of SSL death, and what "SSL" means to admins is not relevant to this feature AFAICT: The feature is not specific to SSL or TLS. > If the decision is to allow/deny TCP tunneling It is not. There are several possible error-handling actions the admin has to pick from and the best action may depend on the nature of the error. >> The implemented "on_first_request_error" feature is much smaller >> and has relatively narrow context: When Squid realizes that it does >> not recognize the connection, it consults the proposed directive >> for instructions. > Okay. Then with the narrow scope ssl_unknown_protocol is better > naming. The feature is not specific to SSL/TLS, but, more importantly, we want the admin to be able to specify what to do when there is a problem, choosing among several actions. The name is not that important, but the ability to pick one of several actions is. Currently, there are only two supported actions: establish a TCP tunnel or respond with a port-specific error, but I suspect that there will be more actions in the future. Terminating a connection is one example. >> tcp_tunnel is a good name, but not for the problem we are trying >> to solve. I suspect we will eventually add a "tunnel tcp" or >> "tcp_tunnel" directive, which will be applied at certain processing >> points. However, we would still benefit from an on_error directive, >> before and after tcp_tunnel is added. >> Amos, if on_first_request_error is converted into on_error with a >> first_request (or similar) ACL, would you continue to block this >> frequently-requested bypass feature? > Mosly I am waiting to see an updated patch. The blocker was on waiting > for Parser-NG request handling re-write which is now in. I am interpreting your answer as a "no" and ask Christos to update the patch to better integrate with the new parsing code. AFAIK, the only integration is in detecting an "HTTP request parsing has failed" condition, which is hopefully easy. Thank you, Alex. _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev