On 07/18/2016 01:27 AM, Amos Jeffries wrote:
> On 15/07/2016 10:38 p.m., Evgeniy Kononov wrote:
>> With this setup I have problem with group chats, calls and attachments in messages.
> The problem is with identifying it in fairly reliable way from all the
> other traffic. That is where we are currently all stuck.
I cannot offer a comprehensive solution for all Skype problems, but I
can share an in-progress triage that we are doing for one particular
problem related to Skype group chats. According to some of the logs I
have seen, group chat uses MSNP(?) messages instead of HTTP. Squid fails
to parse MSNP, as expected:
> RequestParser.cc(340) parse: Parse buf={length=68, data='CNT 1 CON 185
>
> <connect><ver>2</ver><agent><os>Windows</os><osVer>'}
> RequestParser.cc(228) parseHttpVersionField: invalid request-line: not HTTP
AFAICT, Squid then hits an on_unsupported_protocol bug: When deciding
whether to tunnel an intercepted unsupported protocol, Squid never
tunnels traffic on connections that have seen more than one HTTP request
already. The intent behind that check is noble (if a connection started
with a valid HTTP request, then it is probably an HTTP connection), but
the actual result is unfortunate:
1. Intercepted connections start with one or two fake SslBump CONNECT
requests that are counted as "seen HTTP requests".
2. The invalid HTTP request that we just failed to parse is also counted
as a "seen HTTP request".
In the particular case I have seen, once Squid bumps the Skype
connection and receives a non-HTTP MSNP request, the "seen requests"
counter probably reaches 2, and the on_unsupported_protocol option is
not checked.
I am trying to come up with a use case that would justify the current
request counting check. Would switching to a blind tunnel in the middle
of an intercepted connection expose Squid (or its users) to any
_additional_ risks compared to switching to a blind tunnel only when the
bumped connection starts with an invalid HTTP request? If not, it is
trivial to remove the check as the attached patch illustrates.
Thank you,
Alex.
Apply on_unsupported_protocol logic to any malformed intercepted request.
Warning: The validity of this untested patch has not been determined.
When dealing with intercepted connections, the old code was trying to
limit on_unsupported_protocol applications to cases where the very first
request was malformed. However, bumped SSL connections start with fake
CONNECT requests and those requests count as "first requests" on the
connection. If a bumped SSL connection started with a malformed request,
that request was not "first", and Squid ignored on_unsupported_protocol.
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2016-07-11 10:57:11 +0000
+++ src/client_side.cc 2016-07-18 22:47:41 +0000
@@ -1544,41 +1544,41 @@ bool ConnStateData::serveDelayedError(Ht
repContext->setReplyToError(request->method, err);
assert(context->http->out.offset == 0);
context->pullData();
return true;
}
}
}
return false;
}
#endif // USE_OPENSSL
/**
* Check on_unsupported_protocol checklist and return true if tunnel mode selected
* or false otherwise
*/
bool
clientTunnelOnError(ConnStateData *conn, Http::Stream *context, HttpRequest *request, const HttpRequestMethod& method, err_type requestError, Http::StatusCode errStatusCode, const char *requestErrorBytes)
{
if (conn->port->flags.isIntercepted() &&
- Config.accessList.on_unsupported_protocol && conn->pipeline.nrequests <= 1) {
+ Config.accessList.on_unsupported_protocol) {
ACLFilledChecklist checklist(Config.accessList.on_unsupported_protocol, request, NULL);
checklist.requestErrorType = requestError;
checklist.src_addr = conn->clientConnection->remote;
checklist.my_addr = conn->clientConnection->local;
checklist.conn(conn);
allow_t answer = checklist.fastCheck();
if (answer == ACCESS_ALLOWED && answer.kind == 1) {
debugs(33, 3, "Request will be tunneled to server");
if (context) {
// XXX: Either the context is finished() or it should stay queued.
// The below may leak client streams BodyPipe objects. BUT, we need
// to check if client-streams detatch is safe to do here (finished() will detatch).
assert(conn->pipeline.front() == context); // XXX: still assumes HTTP/1 semantics
conn->pipeline.popMe(Http::StreamPointer(context));
}
Comm::SetSelect(conn->clientConnection->fd, COMM_SELECT_READ, NULL, NULL, 0);
return conn->fakeAConnectRequest("unknown-protocol", conn->preservedClientData);
} else {
debugs(33, 3, "Continue with returning the error: " << requestError);
}
_______________________________________________
squid-users mailing list
squid-users@xxxxxxxxxxxxxxxxxxxxx
http://lists.squid-cache.org/listinfo/squid-users
