[squid-dev] [PATCH] Non-HTTP bypass

Alex Rousskov rousskov at measurement-factory.com
Wed Dec 31 07:54:25 UTC 2014


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.


More information about the squid-dev mailing list