Re: [squid-dev] [PATCH] Non-HTTP bypass
Patch applied to trunk as r13853 On 01/14/2015 06:00 PM, Amos Jeffries wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 14/01/2015 7:21 a.m., Tsantilas Christos wrote: I made all requested changes/fixes. The patch also ported to latest trunk. Okay, +1 for commit FYI: Alex, kinkie, and myself had a debate on IRC and came to an agreement for calling the new directive on_unsupported_protocol instead of on_first_request_error. Please feel free to make that naming switch when comitting if you like, it does not require another review IMO. Amos -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (MingW32) iQEcBAEBAgAGBQJUtpKRAAoJELJo5wb/XPRjtLwH/1L0K9u80Yl95ymszoroP2MB TivdghsRQcFO8BIbUkWxVp3M7FghUQY9h/famsxX5R55SiAPOgMmXxoCSWTPe+ID 6VPlYdhr8XsUkWuJZ0MwNA1iJO4yM5jGhU9E/kwH4PSbJqD4aP38Wdt+iuG/+753 px76GFBIVhiW6hVORxW1vXGcnrMcHKaoRwgfnEFSK4QyyDeVr5xVEAQOE0vOluyO AWYGd8pEeMl1gcegcYm+OsdBXdQyvoJBSC74andl2PFOqEu/2wybKCZa86s6IXLi 0PrwtiGWXlOI868ZNlD0TCRTvrES11OZsxx2P9245HNpWo0IULjYlBui4NDVolA= =KsmY -END PGP SIGNATURE- ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] [PATCH] Non-HTTP bypass
I am preparing this patch for commit, but I have many problems with tests/testHttp1Parser tester. The most of the problems caused because the changes I made in Http1Parser aborts immediately parsing when no valid characters found for the request method. These problems can be fixed however there are 1-2 cases where I am not sure about correct fix. For example Http1PArser without my fixes considers as valid methods: - with tabs inside method name, for example \tGET - with '\0' at the end of method name About the \t probably we should eat tabs with spaces in skipGarbageLines. About the '\0' do we have such cases? The true is that I remember in the past, cases where a '\0' is appeared inside HTTP request headers. But maybe in these cases we must not include it in HTTP request method, but consider it as a space. On 01/14/2015 06:00 PM, Amos Jeffries wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 14/01/2015 7:21 a.m., Tsantilas Christos wrote: I made all requested changes/fixes. The patch also ported to latest trunk. Okay, +1 for commit FYI: Alex, kinkie, and myself had a debate on IRC and came to an agreement for calling the new directive on_unsupported_protocol instead of on_first_request_error. Please feel free to make that naming switch when comitting if you like, it does not require another review IMO. Amos -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (MingW32) iQEcBAEBAgAGBQJUtpKRAAoJELJo5wb/XPRjtLwH/1L0K9u80Yl95ymszoroP2MB TivdghsRQcFO8BIbUkWxVp3M7FghUQY9h/famsxX5R55SiAPOgMmXxoCSWTPe+ID 6VPlYdhr8XsUkWuJZ0MwNA1iJO4yM5jGhU9E/kwH4PSbJqD4aP38Wdt+iuG/+753 px76GFBIVhiW6hVORxW1vXGcnrMcHKaoRwgfnEFSK4QyyDeVr5xVEAQOE0vOluyO AWYGd8pEeMl1gcegcYm+OsdBXdQyvoJBSC74andl2PFOqEu/2wybKCZa86s6IXLi 0PrwtiGWXlOI868ZNlD0TCRTvrES11OZsxx2P9245HNpWo0IULjYlBui4NDVolA= =KsmY -END PGP SIGNATURE- ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] [PATCH] Non-HTTP bypass
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 17/01/2015 12:31 a.m., Tsantilas Christos wrote: I am preparing this patch for commit, but I have many problems with tests/testHttp1Parser tester. The most of the problems caused because the changes I made in Http1Parser aborts immediately parsing when no valid characters found for the request method. These problems can be fixed however there are 1-2 cases where I am not sure about correct fix. For example Http1PArser without my fixes considers as valid methods: - with tabs inside method name, for example \tGET - with '\0' at the end of method name About the \t probably we should eat tabs with spaces in skipGarbageLines. About the '\0' do we have such cases? The true is that I remember in the past, cases where a '\0' is appeared inside HTTP request headers. But maybe in these cases we must not include it in HTTP request method, but consider it as a space. Both of them are not valid HTTP as of RFC 7230, this is a clarification since 2616 which could be interpreted as allowing them. Your parser change is correct in rejecting. The tests are in thsi condition because I have not yet re-coded that part of parser to be fully RFC 7230 compliant. In other words, alter those tests as needed to pass with your method characterset change. FWIW: the RFC 7320 now explicitly states the characters which may exist in and around the method: * method is a token (http://tools.ietf.org/html/rfc7230#section-3.2.6) made only of valid tchar, * with tolerant parsing method MAY be prefixed by an LF, * method is followed by specifically an SP character, * invalid method (thus invalid request-line) SHOULD be rejected with a 400 status message. Amos -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (MingW32) iQEcBAEBAgAGBQJUuRjuAAoJELJo5wb/XPRj6KcIAKfjmziUKWcJsvqpOAEaiRy8 zKt835ShvoHC7je8cxhZv5tjRRMR7ShmWu9dIYTinZXbHkoSRSuvjRofR9ef8UJH e1NMJ4vfaLsZDI2HeMKUg1RQE2VECSwiGYmHaIRdF1VLvUPabwMRLyVgEVpMtZn2 XnnFBaPEQmf+oBh+7qUcREY2wPI/YKEt2fAUOn2irHFoRJ1NP2m6UU1dvq0O+QdQ +6gzZJi0Uk7CjW85a7dzRmN4M81i3wsZsGHtrVF3NwFAzXj+Z7Q8/Talvl759+RJ cwvXByulOV1qKHEMJnFa1uKLDdbjWNU1NJ9I47g9jnnlutUBu7wxFcnkvpDbFqU= =hkOk -END PGP SIGNATURE- ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] [PATCH] Non-HTTP bypass
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 14/01/2015 7:21 a.m., Tsantilas Christos wrote: I made all requested changes/fixes. The patch also ported to latest trunk. Okay, +1 for commit FYI: Alex, kinkie, and myself had a debate on IRC and came to an agreement for calling the new directive on_unsupported_protocol instead of on_first_request_error. Please feel free to make that naming switch when comitting if you like, it does not require another review IMO. Amos -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (MingW32) iQEcBAEBAgAGBQJUtpKRAAoJELJo5wb/XPRjtLwH/1L0K9u80Yl95ymszoroP2MB TivdghsRQcFO8BIbUkWxVp3M7FghUQY9h/famsxX5R55SiAPOgMmXxoCSWTPe+ID 6VPlYdhr8XsUkWuJZ0MwNA1iJO4yM5jGhU9E/kwH4PSbJqD4aP38Wdt+iuG/+753 px76GFBIVhiW6hVORxW1vXGcnrMcHKaoRwgfnEFSK4QyyDeVr5xVEAQOE0vOluyO AWYGd8pEeMl1gcegcYm+OsdBXdQyvoJBSC74andl2PFOqEu/2wybKCZa86s6IXLi 0PrwtiGWXlOI868ZNlD0TCRTvrES11OZsxx2P9245HNpWo0IULjYlBui4NDVolA= =KsmY -END PGP SIGNATURE- ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] [PATCH] Non-HTTP bypass
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 31/12/2014 11:40 p.m., Tsantilas Christos wrote:
On 12/31/2014 09:54 AM, Alex Rousskov wrote:
On 12/30/2014 06:19 PM, Amos Jeffries wrote:
On 31/12/2014 7:30 a.m., Alex Rousskov wrote:
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.
OK I will post an updated patch. If the problem is only the name
then we can easily change it.
It seems the updated patch submission did not get through to me for
review. Sorry about that and the delay its caused. Here is the review:
* Regarding ERR_WRONG_PROTOCOL
Please call this ERR_PROTOCOL_UNKNOWN. There is no clear wrong to
be fixed.
* add the copyright blurb from scripts/boilerplate.h to the top of all
new source code files.
- please see the latest version of other ERR_ templates for how to
copyright the new template.
in src/acl/SquidError.h:
* dont add dead code: //virtual bool requiresRequest() ...
in src/acl/SquidErrorData.cc:
* please bump the debugs level in ACLSquidErrorData::match() to
something deeper (eg 3 or 4). We dont need a high level log entry on
every attempted value match.
* ACLSquidErrorData::parse() should be able to have token declared n
the while() scope, like so:
while (char *token = ...) {
* debugs error message missing FATAL: prefix in
ACLSquidErrorData::parse().
- Also, this is a relatively minor config issue for which the
self_destruct() can be skipped when doing -k parse (if global variable
opt_parse_cfg_only is set non-0). Though leave the FATAL debugs
message displaying.
in src/cf.data.pre:
* instead of DEFAULT: none please use DEFAULT_DOC: ... to state a
one-line about what happens if there are none of these directive lines
configured.
eg. DEFAULT_DOC: Respond with an error message to unidentifiable traffic.
in src/client_side.cc:
* dont add HERE in new debugs() in clientTunnelOnError()
* fix \retval documentation on Squid_SSL_accept()
- \retval takes two parameters: VALUE MEANING_OF_VALUE
* clientPeekAndSpliceSSL() has the ConnStateData object (conn)
available with its conn-clientConnection.
- IF (only if) the fd being closed is supposed to be the client
connection rather than other FD, then please use
conn-clientConnection-close() to close it cleanly.
in src/client_side.h:
* issues with \retval again for spliceOnError()
* grammar in docs for preservedClientData: s/as is/as-is/
* lowercase 'True' in docs for receivedFirstByte_
in src/http/one/RequestParser.cc:
* accepting the full range of valid method characters is important,
the new implementation will make us start to fail some coadvisor
testing for special cased methods like the dot (.) method, and WebDAV
extension methods with hyphens or underscores, etc.
NP: I am not aware of any other characters being used, but if the
mechanism to test is easily extended we can fill others in later.
in src/servers/HttpServer.cc:
* missing body to two if-statements in buildHttpRequest().
- if (!clientTunnelOnError(...)) {/* missing code ?? */}
please fix or document why nothing is done.
HTH
Amos
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (MingW32)
iQEcBAEBAgAGBQJUs/paAAoJELJo5wb/XPRjf3sIAOC85ezF2m2OvqbLSxDwGoO1
dkLmHHGqQv1X5qcyWzDXlS2XFP67zNCwcCiuLnT5nqsF4tcGTAZ3Phi9+LQTlR18
H67Qp6uSJLzKLeVCdjEEbvAOPtJ8EldkkhlMFOEaKzhVKqNStqXjg8SUOkYJz/V7
Ouo8G/Re0StVp3w4WtHND0zDGh6Dupw4B+E7EBfxMu61MjqXB4WzAcgzIqIQeT8/
phJT4ObSYyOvHEPRIeAUGrMrryr5wwmKTynhB6xbT9Q4Kks0bzirdjhaiKVffIdm
mMTHKuUbJesouAGZSTueXBBU9AobpRGVSecfDiLfFTJL7Fy417jIKtfqDsnsHUU=
=wIBa
-END PGP SIGNATURE-
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] [PATCH] Non-HTTP bypass
Hi all, I am posting a new patch. Sorry for the delay but the patch was a little old, and many changes required.. This patch updated to apply to the latest squid sources, and uses the new http parser. This patch modify the http parser to reject a method which include non alphanumeric characters in first request line and also if no method or no url exist in request then return the ERR_WRONG_PROTOCOL error instead of the ERR_INVALID_REQ error. I am fully understand that non alphanumeric chars in method or a missing url are not enough to consider a wrong protocol but I believe it is not bad. Please read the patch preamble for more information. Regards, Christos Non-HTTP bypass Intercepting proxies often receive non-HTTP connections. Squid cannot currently deal with such connections well because it assumes that a given port receives HTTP, FTP, or HTTPS traffic exclusively. This patch allows Squid to tunnel unexpected connections instead of terminating them with an error. In this project, we define an unexpected connection as a connection that resulted in a Squid error during first request parsing. Which errors trigger tunneling behavior is configurable by the admin using ACLs. Here is a configuration sketch: # define what Squid errors indicate receiving non-HTTP traffic: acl foreignProtocol squid_error ERR_WRONG_PROTOCOL ERR_TOO_BIG # define what Squid errors indicate receiving nothing: acl serverTalksFirstProtocol squid_error ERR_REQUEST_START_TIMEOUT # tunnel everything that does not look like HTTP: on_first_request_error tunnel foreignProtocol # tunnel if we think the client waits for the server to talk first: on_first_request_error tunnel serverTalksFirstProtocol # in all other error cases, just send an HTTP error page response: on_first_request_error respond all # Configure how long to wait for the first byte on the incoming # connection before raising an ERR_REQUEST_START_TIMEOUT error. request_start_timeout 5 seconds The overall intent of this TCP tunnel is to get Squid out of the communication loop to the extent possible. Once the decision to tunnel is made, no Squid errors are going to be sent to the client and tunneled traffic is not going to be sent to Squid adaptation services or logged to access.log (except for a single summary line at the end of the transaction). Connection closure at the server (or client) end of the tunnel is propagated to the other end by closing the corresponding connection. This patch also: Add 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. Add squid_error, a new ACL type to match transactions that triggered a given Squid error. Squid error IDs are used to configure one or more errors to match. This is similar to the existing ssl_error ACL type but works with Squid-generated errors rather than SSL library errors. Add ERR_WRONG_PROTOCOL, a new Squid error triggered for http_port connections that start with something that lacks even basic HTTP request structure. This error is triggered by the HTTP request parser, and probably only when/after the current parsing code detects an error. That is, we do not want to introduce new error conditions, but we want to treat some of the currently triggered parsing errors as a wrong protocol error, possibly after checking the parsing state or the input buffer for some clues. There is no known way to reliably distinguish malformed HTTP requests from non-HTTP traffic so the parser has to use some imprecise heuristics to make a decision in some cases. In the future, it would be possible to add code to reliably detect some popular non-HTTP protocols, but adding such code is outside this project scope. Add request_start_timeout, a new squid.conf directive to trigger a new Squid ERR_REQUEST_START_TIMEOUT error if no bytes are received from the client on a newly established http_port connection during the configured time period. Applies to all http_ports (for now). No support for tunneling through cache_peers is included. Configurations that direct outgoing traffic through a peer may break Squid. This is a Measurement Factory project === modified file 'errors/template.list' --- errors/template.list 2014-12-20 18:12:02 + +++ errors/template.list 2014-12-31 10:43:41 + @@ -30,21 +30,22 @@ templates/ERR_FTP_UNAVAILABLE \ templates/ERR_GATEWAY_FAILURE \ templates/ERR_ICAP_FAILURE \ templates/ERR_INVALID_REQ \ templates/ERR_INVALID_RESP \ templates/ERR_INVALID_URL \ templates/ERR_LIFETIME_EXP \ templates/ERR_NO_RELAY \ templates/ERR_ONLY_IF_CACHED_MISS \ templates/ERR_PRECONDITION_FAILED \ templates/ERR_READ_ERROR \ templates/ERR_READ_TIMEOUT \
Re: [squid-dev] [PATCH] Non-HTTP bypass
On 12/31/2014 02:31 PM, Alex Rousskov wrote: On 12/31/2014 03:33 AM, Marcus Kool wrote: On 12/31/2014 05:54 AM, Alex Rousskov wrote: 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. From an administrators point of view, the admins that want Squid to filter internet access, definitely want (B). They want (B) to block audio, video, SSH tunnnels, VPNs, chat, file sharing, webdisks and all sorts of applications (but not all!) that use port 443. Agreed, except this is not limited to port 443. The scope includes intercepted port 80 connections and even CONNECT tunnels. If CONNECT tunnels are in scope, then so are all the applications that use it, including webdisk, audio, video, SSH etc. I think it was Amos who said that application builders hould use application-specific ports, but reality is that all firewalls block those ports by default. Skype was one of the first applications that worked everywhere, even behind a corporate firewall and it was done using CONNECT to the web proxy. And from a security point of view I think that administrators prefer that applications use CONNECT to the web proxy to have more control and logging about what traffic is going from a LAN to the internet. Basically this means that admins desire a more fine-grained control about what to do with each tunnel. There are two different needs here, actually: 1. A choice of actions (i.e., what to do) when dealing with an unsupported protocol. Currently, there is only one action: Send an HTTP error response. The proposed feature adds another action (tunnel) and, more importantly, adds a configuration interface to support more actions later. Sending an HTTP error to an application that does not speak HTTP is not very useful. Skype, SSH, videoplayers etc. only get confused at best. Simply closing the tunnel may be better and may result in an end user message 'cannot connect to ...' instead of 'server sends garbage' or 'undefined protocol'. Marcus 2. A way to further classify an unsupported protocol (i.e., fine-grained control). I started a new thread on this topic as it is not about the proposed bypass feature. Cheers, Alex. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] [PATCH] Non-HTTP bypass
On 01/02/2015 09:16 AM, Marcus Kool wrote: On 12/31/2014 02:31 PM, Alex Rousskov wrote: On 12/31/2014 03:33 AM, Marcus Kool wrote: On 12/31/2014 05:54 AM, Alex Rousskov wrote: 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. From an administrators point of view, the admins that want Squid to filter internet access, definitely want (B). They want (B) to block audio, video, SSH tunnnels, VPNs, chat, file sharing, webdisks and all sorts of applications (but not all!) that use port 443. Agreed, except this is not limited to port 443. The scope includes intercepted port 80 connections and even CONNECT tunnels. If CONNECT tunnels are in scope, then so are all the applications that use it, including webdisk, audio, video, SSH etc. Yes, of course. The receiving Squid port and current connection state affects what protocol Squid expects to find/parse, whether Squid may perform (or has performed) bumping, etc., but all those decisions and actions happen before and/or after the new feature kicks in. The detection of an unsupported protocol and computation of the corresponding tunnel/respond decision is what we focus on here. I think it was Amos who said that application builders hould use application-specific ports, but reality is that all firewalls block those ports by default. Yes, the each application has a dedicated port idea[l] is too far from reality to matter. Basically this means that admins desire a more fine-grained control about what to do with each tunnel. There are two different needs here, actually: 1. A choice of actions (i.e., what to do) when dealing with an unsupported protocol. Currently, there is only one action: Send an HTTP error response. The proposed feature adds another action (tunnel) and, more importantly, adds a configuration interface to support more actions later. Sending an HTTP error to an application that does not speak HTTP is not very useful. Skype, SSH, videoplayers etc. only get confused at best. Simply closing the tunnel may be better and may result in an end user message 'cannot connect to ...' instead of 'server sends garbage' or 'undefined protocol'. I may have already mentioned that terminate is another candidate action. For now, it is possible to emulate that action in some deployments by using custom TCP_RESET error bodies, but that hack is too rigid: It does not discriminate the error context/environment. That is what ACLs in the new feature are meant to do. Whether it is better to simply terminate the connection or send some diagnostic text to the user client should be up to the admin to decide. Different deployment environments will benefit from different actions. The proposed feature builds the initial framework for those decisions. Cheers, Alex. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] [PATCH] Non-HTTP bypass
On 12/31/2014 05:54 AM, Alex Rousskov wrote: [...] 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. From an administrators point of view, the admins that want Squid to filter internet access, definitely want (B). They want (B) to block audio, video, SSH tunnnels, VPNs, chat, file sharing, webdisks and all sorts of applications (but not all!) that use port 443. Basically this means that admins desire a more fine-grained control about what to do with each tunnel. The current functionality of filtering is divided between Squid itself and 3rd party software (ICAP daemons and URL redirectors). I plea for an interface where an external helper can decide what to do with an unknown protocol inside a tunnel because it is much more flexible than using ACLs and extending Squid with detection of (many) protocols. A while back when we discussed the older sslBump not being able to cope with Skype I suggested to use ICAP so that the ICAP daemon receives a REQMOD/RESPMOD message with CONNECT and intercepted content, which also is a valid option for me. I wish to all a Blessful and Happy New Year! Marcus [...] Thank you, Alex. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] [PATCH] Non-HTTP bypass
On 12/31/2014 03:33 AM, Marcus Kool wrote: On 12/31/2014 05:54 AM, Alex Rousskov wrote: 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. From an administrators point of view, the admins that want Squid to filter internet access, definitely want (B). They want (B) to block audio, video, SSH tunnnels, VPNs, chat, file sharing, webdisks and all sorts of applications (but not all!) that use port 443. Agreed, except this is not limited to port 443. The scope includes intercepted port 80 connections and even CONNECT tunnels. Basically this means that admins desire a more fine-grained control about what to do with each tunnel. There are two different needs here, actually: 1. A choice of actions (i.e., what to do) when dealing with an unsupported protocol. Currently, there is only one action: Send an HTTP error response. The proposed feature adds another action (tunnel) and, more importantly, adds a configuration interface to support more actions later. 2. A way to further classify an unsupported protocol (i.e., fine-grained control). I started a new thread on this topic as it is not about the proposed bypass feature. Cheers, Alex. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] [PATCH] Non-HTTP bypass
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. What a nasty name for an access list. Please try to be more specific, especially when you are being negative about others work. Nasty can mean many things to many people. In this particular case, your comment carries no [positive] value at all. It is difficult for the code author to make something less nasty because they do not know what nasty means to you in this context. BTW, on_first_request_error does not name an access list. I believe tcp_tunnel or tunnel_transparent would be more descriptive of the test is *checking* for. * If we want to tell Squid when to tunnel, then tcp_tunnel is a good solution (but this is not what this proposal tries to do). * If we want to tell Squid what to do with connections it does not recognize, then tcp_tunnel is the worst alternative of the three I can think of (see below for details). The tcp_tunnel idea currently lacks context: Is it applied when we accept a connection, when we authenticate the user, when we parse 10th request, etc.? There are many very different cases where we may want to start tunneling, for many different reasons. This lack of specifics makes the idea look simple and, hence, appealing, but the devil is usually revealed in the details. 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. The on_error concept is not new in IT and should be familiar to many admins. The suggested feature can be converted into a tcp_tunnel feature if: * we add a hard-coded first_request_error ACL * agree that tcp_tunnel rules are evaluated when Squid realizes that it does not recognize the first request on a connection (at least). Then, instead of writing: on_first_request_error tunnel some on_first_request_error respond other on_first_request_error future_action yet_another folks will write: tcp_tunnel on_first_request_error some respond_with_error on_first_request_error other future_action on_first_request_error yet_another which is clearly (I hope!) worse from configuration clarity and convenience point of view. However, folks would also be able to write: tcp_tunnel some_other_error some respond_with_error some_other_place other future_action some_other_category yet_another which is an improvement over the more rigid on_first_request_error scheme that restricts application to only certain kinds of errors in certain places. The design space can be summarized like this: # extreme left: general-purpose on-error handling: on_error action error_kind_acl acl ... # proposed feature: unrecognized connection handling; # the error_kind_acl is implicit in this design: on_first_request_error action acl ... # suggested extreme right: general-purpose actions: tcp_tunnel error_detection_acl acl ... ssl_tunnel error_detection_acl acl ... respond_with_error error_detection_acl acl ... terminate error_detection_acl acl ... Since many Squid actions are likely to be appropriate under the same conditions (tunnel, respond with an error, terminate, ssl-tunnel, etc.) I do not think the suggested extreme is the best design. We should either move left to the general on_error configuration or stay with the proposal currently under review. The only checked on first request failure detail can be left in the directive documentation. It would be wrong to provide a general-looking tcp_tunnel option that can only be applied in one specific case. The initial implementation may be restricted to one case, of course, but a general feature needs enough knobs to be usable in many contexts. In this case, this means a group of hard-coded ACL guards that only match in some specific supported contexts (see above for examples). At the end of your email, you have already identified an error-unrelated case where the tcp_tunnel directive would be required... I also suspect that is a false statement since anything we do with a CONNECT body that fails will need a matching check applied. Not sure what you mean here, but please note that the first request inside a bumped CONNECT tunnel is still the first request as far as the proposed feature is concerned. To avoid misunderstanding, I am not saying that on_first_request_error name or its description is perfect. Better alternatives are more than welcome (and similar names/descriptions would be needed even if we move towards a more general on_error design)! ssl-bump calls its version of tunnelling ssl_bump splice. So if we are continuing with the splice vs tunnel
Re: [squid-dev] [PATCH] Non-HTTP bypass
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 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. What a nasty name for an access list. Please try to be more specific, especially when you are being negative about others work. Nasty can mean many things to many people. In this particular case, your comment carries no [positive] value at all. It is difficult for the code author to make something less nasty because they do not know what nasty means to you in this context. nasty - horrible, mean (but not cruel), very annoying. Henrik and I had a discussion about directive naming years back and decided that long and nasty names like this should be reserved for directives that we really, really did not want people using but there was actually a rare use case for. Emphasis being on rare and not using. The length and horribleness of the name helping to discourage use, not documenting being another. IMO this directive needs to be friendly and clear as its going to be somewhat popular. BTW, on_first_request_error does not name an access list. By access list I mean a multi-line directive, whose line ordering is meaningful, using the syntax: directive_name action acl [acl ...] Christos configuration sketch: # tunnel if we think the client waits for the server to talk first: on_first_request_error tunnel serverTalksFirstProtocol # in all other error cases, just send an HTTP error page response: on_first_request_error respond all The name reads like the value is a one-liner / event handler. i.e. an action without ACL list. Like we have one-liner directives error_directory /foo/, enable_icmp on/off, etc. 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. So in detail: 1) on_ - all directives in Squid.conf are essentially declarative and access control could almost all be prefixed with on_. This adds no value but makes the directive longer and harder to write. 2) first_request_error as a declarative statement - implies the value *is* the error to be supplied on first request. It is actually declaring an action to perform *instead* of producing errors ... and the ACL tests to determine that action. 3) due to legacy Squid being HTTP-oriented the directives we have without explicit component names (http_, ftp_, dns_, ssl_, icp_htcp_, wccp_ etc) are implying http_ relevance. This directive is not relevant to HTTP, but to SSL/TLS. So something like ssl_ or tls_ or tcp_ is more appropriate. 4) is that error on first request, or first request with errors? the name is not clear. 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. 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 believe tcp_tunnel or tunnel_transparent would be more descriptive of the test is *checking* for. * If we want to tell Squid when to tunnel, then tcp_tunnel is a good solution (but this is not what this proposal tries to do). * If we want to tell Squid what to do with connections it does not recognize, then tcp_tunnel is the worst alternative of the three I can think of (see below for details). The tcp_tunnel idea currently lacks context: Is it applied when we accept a connection, when we authenticate the user, when we parse 10th request, etc.? There are many very different cases where we may want to start tunneling, for many different reasons. This lack of specifics makes the idea look simple and, hence, appealing, but the devil is usually revealed in the details. It also depends on where the decision is taking place. And needs to be agnostic of TLS. If the decision is to allow/deny TCP tunneling it's rightfully called tcp_tunnel allow/deny regardless of how many pathways lead to that decision point. i.e. we do not have 9 different protocol-specific ways/names to define cache_peer_access just because 9 protocols (HTTP, HTTPS, FTP, Gopher, Wais, tunnel, ICP, HTCP and ICMP) can all use peers - we have one and its tested by each component as and when necessary. 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
Re: [squid-dev] [PATCH] Non-HTTP bypass
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
Re: [squid-dev] [PATCH] Non-HTTP bypass
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 22/10/2014 9:12 p.m., Tsantilas Christos wrote: On 10/21/2014 04:29 PM, Amos Jeffries wrote: 2) All changes in src/tunnel.cc seem to be needless. Some changes are required! - tunnelStartShovelling() should *always* be the entrypoint to begin transmit on a tunnel in any direction. At that point there is maybe client data to send to server ... Yes. - The socket may not even permit one whole TCP_RCV_BUF worth of bytes to be written in a single action. comm_write can handle that just fine. If comm_write is unable to handle all the available conn-in.buf in one comm_write() call then that would be a bug in comm to fix separate from all this. This is true. However we are using two buffers to read/write to/from server/client. The TunnelState::client::buf and TunnelState::server::buf which are of size SQUID_TCP_SO_RCVBUF At early stages of this patch I tried to convert these buffers to SBufs , but required many changes in the tunnel.cc code, so I revert the changes and finally implemented the TunnelStateData::copyClientBytes member. Arg, sorry you are right. I forgot that my previous conversion to SBuf did not make it through Alex audit requirements. Is this an urgent need? or can we work on fixing that problem first to reduce the changes this feature requires? There is simply no reason to add an extra if (preReadClientData.length()) conditional in *every* packet I/O cycle. As you can see the old code copies the ConnStateData::in-buf data to TunnelStateData-client.buf buffer. This is because in the case you are read a CONNECT request you can be sure that the extra bytes you read are not hugger than the SQUID_TCP_SO_RCVBUF. But for this patch imagine the case where: - Squid configured with: request_header_max_size 70kbs - Client sends something which looks like an HTTP request eg: AMETHOD [spaces]* string [spaces]* ... Other data Also assume that this is more than the size of 70k. - Squid will read 70k before decide that this is not an HTTP request. - The 70k are not fit to TunnelStateData-client.buf An alternate solution is to add some code to build/fix TunnelStateData-client.buf to have the required size. I need to recheck, but looks a good solution. Is it ok? Okay. I find it acceptible as a workaround for the buffer limitation. I would rather see the limit fixed though. Alex and I have now almost reached agreement on the Comm::Write API needed to SBuf the tunnel I/O fully. If this bypass feature can wait for a while I will divert my efforts to getting that completed now. Amos -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (MingW32) iQEcBAEBAgAGBQJUR6a1AAoJELJo5wb/XPRjZ4EH+gL4YoDwKFWIzV9FvzYLGE0n wxY1We5Nk3PqP3YH/BTMnoA0XMdQ9+SPd4sHFVhU3lnx/xUztI83i/2k18+6d4SH Dl1NHS/pBsIkCvwJ6hc3ZDgG3+LTkHv5A0UuYqU5PbLkU63TPoNkFT6elfXZMNkL 8K/vSw75GpI+0AYGTw7NbqHuuG8XLM1CL8Y7jKug9SiZn9w/9RFsa4BmOZ4LYBa6 nQdjmus5dcdLk0OMF4oAGnR6XytzwjO47TiOywE9FcbzismKO/aRCdq5pV5sLJHe xDTfcmB9OJiOvPbMLhlilWtc1oveoYtWsyKQy1kPk1tXj+/KPLi8HKEaN2onjg8= =+Iz9 -END PGP SIGNATURE- ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
