From: Christos Tsantilas Date: Fri, 16 Jan 2015 16:18:05 +0000 (+0200) Subject: Non-HTTP bypass X-Git-Tag: merge-candidate-3-v1~341 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=3248e962c06e59482489d3b903064ab9ff732ad4;p=thirdparty%2Fsquid.git 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_PROTOCOL_UNKNOWN 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_PROTOCOL_UNKNOWN", a 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 --- diff --git a/errors/template.list b/errors/template.list index e9185aed5e..55812c11d6 100644 --- a/errors/template.list +++ b/errors/template.list @@ -47,4 +47,5 @@ ERROR_TEMPLATES= \ templates/ERR_UNSUP_REQ \ templates/ERR_URN_RESOLVE \ templates/ERR_WRITE_ERROR \ - templates/ERR_ZERO_SIZE_OBJECT + templates/ERR_ZERO_SIZE_OBJECT \ + templates/ERR_PROTOCOL_UNKNOWN diff --git a/errors/templates/ERR_PROTOCOL_UNKNOWN b/errors/templates/ERR_PROTOCOL_UNKNOWN new file mode 100644 index 0000000000..ace9151299 --- /dev/null +++ b/errors/templates/ERR_PROTOCOL_UNKNOWN @@ -0,0 +1,38 @@ + + + + +ERROR: The requested URL could not be retrieved + + +
+

ERROR

+

The requested URL could not be retrieved

+
+
+ +
+

The following error was encountered while trying to retrieve the URL: %U

+ +
+

Unsupported Protocol

+
+ +

Squid does not support some access protocols. For example, the SSH protocol is currently not supported.

+ +

Your cache administrator is %w.

+
+
+ +
+ + diff --git a/src/AclRegs.cc b/src/AclRegs.cc index dcfc770030..b71d886ad9 100644 --- a/src/AclRegs.cc +++ b/src/AclRegs.cc @@ -69,6 +69,8 @@ #include "acl/SourceAsn.h" #include "acl/SourceDomain.h" #include "acl/SourceIp.h" +#include "acl/SquidError.h" +#include "acl/SquidErrorData.h" #if USE_OPENSSL #include "acl/Certificate.h" #include "acl/CertificateData.h" @@ -218,3 +220,5 @@ ACL::Prototype ACLAdaptationService::RegistryProtoype(&ACLAdaptationService::Reg ACLStrategised ACLAdaptationService::RegistryEntry_(new ACLAdaptationServiceData, ACLAdaptationServiceStrategy::Instance(), "adaptation_service"); #endif +ACL::Prototype ACLSquidError::RegistryProtoype(&ACLSquidError::RegistryEntry_, "squid_error"); +ACLStrategised ACLSquidError::RegistryEntry_(new ACLSquidErrorData, ACLSquidErrorStrategy::Instance(), "squid_error"); diff --git a/src/SquidConfig.h b/src/SquidConfig.h index 36fdac5346..6d10bc51b3 100644 --- a/src/SquidConfig.h +++ b/src/SquidConfig.h @@ -92,6 +92,7 @@ public: time_t pconnLifetime; ///< pconn_lifetime in squid.conf time_t siteSelect; time_t deadPeer; + time_t request_start_timeout; int icp_query; /* msec */ int icp_query_max; /* msec */ int icp_query_min; /* msec */ @@ -377,6 +378,7 @@ public: /// spoof_client_ip squid.conf acl. /// nil unless configured acl_access* spoof_client_ip; + acl_access *on_unsupported_protocol; acl_access *ftp_epsv; diff --git a/src/acl/FilledChecklist.cc b/src/acl/FilledChecklist.cc index 51cdbd5629..0db8f54f9a 100644 --- a/src/acl/FilledChecklist.cc +++ b/src/acl/FilledChecklist.cc @@ -36,6 +36,7 @@ ACLFilledChecklist::ACLFilledChecklist() : #if USE_OPENSSL sslErrors(NULL), #endif + requestErrorType(ERR_MAX), conn_(NULL), fd_(-1), destinationDomainChecked_(false), diff --git a/src/acl/FilledChecklist.h b/src/acl/FilledChecklist.h index 5339c2f041..09c06f511a 100644 --- a/src/acl/FilledChecklist.h +++ b/src/acl/FilledChecklist.h @@ -13,6 +13,7 @@ #include "acl/Checklist.h" #include "acl/forward.h" #include "base/CbcPointer.h" +#include "err_type.h" #include "ip/Address.h" #if USE_AUTH #include "auth/UserRequest.h" @@ -91,6 +92,8 @@ public: ExternalACLEntryPointer extacl_entry; + err_type requestErrorType; + private: ConnStateData * conn_; /**< hack for ident and NTLM */ int fd_; /**< may be available when conn_ is not */ diff --git a/src/acl/Makefile.am b/src/acl/Makefile.am index 49066e66ee..54af89b740 100644 --- a/src/acl/Makefile.am +++ b/src/acl/Makefile.am @@ -116,6 +116,10 @@ libacls_la_SOURCES = \ SourceDomain.h \ SourceIp.cc \ SourceIp.h \ + SquidError.h \ + SquidError.cc \ + SquidErrorData.cc \ + SquidErrorData.h \ Tag.cc \ Tag.h \ Url.cc \ diff --git a/src/acl/SquidError.cc b/src/acl/SquidError.cc new file mode 100644 index 0000000000..4dff3216dd --- /dev/null +++ b/src/acl/SquidError.cc @@ -0,0 +1,30 @@ +/* + * Copyright (C) 1996-2014 The Squid Software Foundation and contributors + * + * Squid software is distributed under GPLv2+ license and includes + * contributions from numerous individuals and organizations. + * Please see the COPYING and CONTRIBUTORS files for details. + */ + +#include "squid.h" +#include "acl/Checklist.h" +#include "acl/SquidError.h" +#include "HttpRequest.h" + +int +ACLSquidErrorStrategy::match (ACLData * &data, ACLFilledChecklist *checklist, ACLFlags &) +{ + if (checklist->requestErrorType != ERR_MAX) + return data->match(checklist->requestErrorType); + else if (checklist->request) + return data->match(checklist->request->errType); + return 0; +} + +ACLSquidErrorStrategy * +ACLSquidErrorStrategy::Instance() +{ + return &Instance_; +} + +ACLSquidErrorStrategy ACLSquidErrorStrategy::Instance_; diff --git a/src/acl/SquidError.h b/src/acl/SquidError.h new file mode 100644 index 0000000000..676b864953 --- /dev/null +++ b/src/acl/SquidError.h @@ -0,0 +1,43 @@ +/* + * Copyright (C) 1996-2014 The Squid Software Foundation and contributors + * + * Squid software is distributed under GPLv2+ license and includes + * contributions from numerous individuals and organizations. + * Please see the COPYING and CONTRIBUTORS files for details. + */ + +#ifndef SQUID_ACLSQUIDERROR_H +#define SQUID_ACLSQUIDERROR_H + +#include "acl/Strategised.h" +#include "acl/Strategy.h" +#include "err_type.h" + +class ACLSquidErrorStrategy : public ACLStrategy +{ + +public: + virtual int match (ACLData * &, ACLFilledChecklist *, ACLFlags &); + + static ACLSquidErrorStrategy *Instance(); + /* Not implemented to prevent copies of the instance. */ + /* Not private to prevent brain dead g+++ warnings about + * private constructors with no friends */ + ACLSquidErrorStrategy(ACLSquidErrorStrategy const &); + +private: + static ACLSquidErrorStrategy Instance_; + ACLSquidErrorStrategy() {} + + ACLSquidErrorStrategy&operator=(ACLSquidErrorStrategy const &); +}; + +class ACLSquidError +{ + +private: + static ACL::Prototype RegistryProtoype; + static ACLStrategised RegistryEntry_; +}; + +#endif /* SQUID_ACLSQUIDERROR_H */ diff --git a/src/acl/SquidErrorData.cc b/src/acl/SquidErrorData.cc new file mode 100644 index 0000000000..08d52f4e02 --- /dev/null +++ b/src/acl/SquidErrorData.cc @@ -0,0 +1,77 @@ +/* + * Copyright (C) 1996-2014 The Squid Software Foundation and contributors + * + * Squid software is distributed under GPLv2+ license and includes + * contributions from numerous individuals and organizations. + * Please see the COPYING and CONTRIBUTORS files for details. + */ + +#include "squid.h" +#include "acl/Data.h" +#include "acl/SquidErrorData.h" +#include "cache_cf.h" +#include "ConfigParser.h" +#include "Debug.h" +#include "err_type.h" +#include "fatal.h" +#include "wordlist.h" + +bool +ACLSquidErrorData::match(err_type err) +{ + CbDataListIterator iter(errors); + while (!iter.end()) { + err_type localErr = iter.next(); + debugs(28, 4, "check (" << err << "):" << errorTypeName(err) << " against " << errorTypeName(localErr)); + if (err == localErr) + return true; + } + + return false; +} + +SBufList +ACLSquidErrorData::dump() const +{ + SBufList sl; + CbDataListIterator iter(errors); + while (!iter.end()) { + err_type err = iter.next(); + const char *errName = errorTypeName(err); + sl.push_back(SBuf(errName)); + } + + return sl; +} + +void +ACLSquidErrorData::parse() +{ + while (char *token = ConfigParser::NextToken()) { + err_type err = errorTypeByName(token); + + if (err < ERR_MAX) + errors.push_back(err); + else { + debugs(28, DBG_CRITICAL, "FATAL: Invalid squid error name"); + if (!opt_parse_cfg_only) + self_destruct(); + } + } +} + +bool +ACLSquidErrorData::empty() const +{ + return errors.empty(); +} + +ACLData * +ACLSquidErrorData::clone() const +{ + if (!errors.empty()) + fatal("ACLSquidError::clone: attempt to clone used ACL"); + + return new ACLSquidErrorData (*this); +} + diff --git a/src/acl/SquidErrorData.h b/src/acl/SquidErrorData.h new file mode 100644 index 0000000000..8e04510bdf --- /dev/null +++ b/src/acl/SquidErrorData.h @@ -0,0 +1,34 @@ +/* + * Copyright (C) 1996-2014 The Squid Software Foundation and contributors + * + * Squid software is distributed under GPLv2+ license and includes + * contributions from numerous individuals and organizations. + * Please see the COPYING and CONTRIBUTORS files for details. + */ + +#ifndef SQUID_ACLSQUIDERRORDATA_H +#define SQUID_ACLSQUIDERRORDATA_H + +#include "acl/Data.h" +#include "base/CbDataList.h" +#include "err_type.h" + +/// \ingroup ACLAPI +class ACLSquidErrorData : public ACLData +{ + +public: + ACLSquidErrorData(): ACLData() {}; + + virtual ~ACLSquidErrorData() {} + virtual bool match(err_type err); + virtual SBufList dump() const; + virtual void parse(); + virtual bool empty() const; + virtual ACLData *clone() const; + +private: + CbDataListContainer errors; +}; + +#endif //SQUID_ACLSQUIDERRORDATA_H diff --git a/src/cache_cf.cc b/src/cache_cf.cc index f5fa838a61..6499ba8ee1 100644 --- a/src/cache_cf.cc +++ b/src/cache_cf.cc @@ -246,6 +246,9 @@ static int parseOneConfigFile(const char *file_name, unsigned int depth); static void parse_configuration_includes_quoted_values(bool *recognizeQuotedValues); static void dump_configuration_includes_quoted_values(StoreEntry *const entry, const char *const name, bool recognizeQuotedValues); static void free_configuration_includes_quoted_values(bool *recognizeQuotedValues); +static void parse_on_unsupported_protocol(acl_access **access); +static void dump_on_unsupported_protocol(StoreEntry *entry, const char *name, acl_access *access); +static void free_on_unsupported_protocol(acl_access **access); /* * LegacyParser is a parser for legacy code that uses the global @@ -5003,3 +5006,57 @@ free_configuration_includes_quoted_values(bool *) ConfigParser::StrictMode = false; } +static void +parse_on_unsupported_protocol(acl_access **access) +{ + char *tm; + if ((tm = ConfigParser::NextToken()) == NULL) { + self_destruct(); + return; + } + + allow_t action = allow_t(ACCESS_ALLOWED); + if (strcmp(tm, "tunnel") == 0) + action.kind = 1; + else if (strcmp(tm, "respond") == 0) + action.kind = 2; + else { + debugs(3, DBG_CRITICAL, "FATAL: unknown on_unsupported_protocol mode: " << tm); + self_destruct(); + return; + } + + Acl::AndNode *rule = new Acl::AndNode; + rule->context("(on_unsupported_protocol rule)", config_input_line); + rule->lineParse(); + // empty rule OK + + assert(access); + if (!*access) { + *access = new Acl::Tree; + (*access)->context("(on_unsupported_protocol rules)", config_input_line); + } + + (*access)->add(rule, action); +} + +static void +dump_on_unsupported_protocol(StoreEntry *entry, const char *name, acl_access *access) +{ + const char *on_error_tunnel_mode_str[] = { + "none", + "tunnel", + "respond", + NULL + }; + if (access) { + SBufList lines = access->treeDump(name, on_error_tunnel_mode_str); + dump_SBufList(entry, lines); + } +} + +static void +free_on_unsupported_protocol(acl_access **access) +{ + free_acl_access(access); +} diff --git a/src/cf.data.depend b/src/cf.data.depend index b2969935c3..b9adae9290 100644 --- a/src/cf.data.depend +++ b/src/cf.data.depend @@ -58,6 +58,7 @@ memcachemode note acl obsolete onoff +on_unsupported_protocol acl peer peer_access cache_peer acl pipelinePrefetch diff --git a/src/cf.data.pre b/src/cf.data.pre index 86780bff98..d15691cb34 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -1616,6 +1616,50 @@ DOC_START DOC_END +NAME: on_unsupported_protocol +TYPE: on_unsupported_protocol +LOC: Config.accessList.on_unsupported_protocol +DEFAULT: none +DEFAULT_DOC: Respond with an error message to unidentifiable traffic +DOC_START + Determines Squid behavior when encountering strange requests at the + beginning of an accepted TCP connection. This is especially useful in + interception environments where Squid is likely to see connections for + unsupported protocols that Squid should either terminate or tunnel at + TCP level. + + on_unsupported_protocol [!]acl ... + + The first matching action wins. + + Supported actions are: + + tunnel: Establish a TCP connection with the intended server and + blindly shovel TCP packets between the client and server. + + respond: Respond with an error message, using the transfer protocol + for the Squid port that received the request (e.g., HTTP + for connections intercepted at the http_port). This is the + default. + + Currently, this directive is ignored for non-intercepted connections + because Squid cannot know what their intended destination is. + + For example: + # define what Squid errors indicate receiving non-HTTP traffic: + acl foreignProtocol squid_error ERR_PROTOCOL_UNKNOWN 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_unsupported_protocol tunnel foreignProtocol + # tunnel if we think the client waits for the server to talk first: + on_unsupported_protocol tunnel serverTalksFirstProtocol + # in all other error cases, just send an HTTP "error page" response: + on_unsupported_protocol respond all + + See also: squid_error ACL +DOC_END + COMMENT_START NETWORK OPTIONS ----------------------------------------------------------------------------- @@ -6083,6 +6127,15 @@ DOC_START connection establishment. DOC_END +NAME: request_start_timeout +TYPE: time_t +LOC: Config.Timeout.request_start_timeout +DEFAULT: 5 minutes +DOC_START + How long to wait for the first request byte after initial + connection establishment. +DOC_END + NAME: client_idle_pconn_timeout persistent_request_timeout TYPE: time_t LOC: Config.Timeout.clientIdlePconn diff --git a/src/client_side.cc b/src/client_side.cc index 93fe3337d1..e8baa3ddf9 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -2152,6 +2152,8 @@ parseHttpRequest(ConnStateData *csd, const Http1::RequestParserPointer &hp) { const bool parsedOk = hp->parse(csd->in.buf); + if (csd->port->flags.isIntercepted() && Config.accessList.on_unsupported_protocol) + csd->preservedClientData = csd->in.buf; // sync the buffers after parsing. csd->in.buf = hp->remaining(); @@ -2457,6 +2459,62 @@ bool ConnStateData::serveDelayedError(ClientSocketContext *context) } #endif // USE_OPENSSL +/** + * Check on_unsupported_protocol checklist and return true if tunnel mode selected + * or false otherwise + */ +bool +clientTunnelOnError(ConnStateData *conn, ClientSocketContext *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->nrequests <= 1) { + 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) + context->removeFromConnectionList(conn); + Comm::SetSelect(conn->clientConnection->fd, COMM_SELECT_READ, NULL, NULL, 0); + + SBuf preReadData; + if (conn->preservedClientData.length()) + preReadData.append(conn->preservedClientData); + static char ip[MAX_IPSTRLEN]; + conn->clientConnection->local.toUrl(ip, sizeof(ip)); + conn->in.buf.assign("CONNECT ").append(ip).append(" HTTP/1.1\r\nHost: ").append(ip).append("\r\n\r\n").append(preReadData); + + bool ret = conn->handleReadData(); + if (ret) + ret = conn->clientParseRequests(); + + if (!ret) { + debugs(33, 2, "Failed to start fake CONNECT request for on_unsupported_protocol: " << conn->clientConnection); + conn->clientConnection->close(); + } + return true; + } else { + debugs(33, 3, "Continue with returning the error: " << requestError); + } + } + + if (context) { + conn->quitAfterError(request); + clientStreamNode *node = context->getClientReplyContext(); + clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); + assert (repContext); + + repContext->setReplyToError(requestError, errStatusCode, method, context->http->uri, conn->clientConnection->remote, NULL, requestErrorBytes, NULL); + + assert(context->http->out.offset == 0); + context->pullData(); + } // else Probably an ERR_REQUEST_START_TIMEOUT error so just return. + return false; +} + void clientProcessRequestFinished(ConnStateData *conn, const HttpRequest::Pointer &request) { @@ -2976,6 +3034,20 @@ ConnStateData::parseProxy2p0() return true; } +void +ConnStateData::receivedFirstByte() +{ + if (receivedFirstByte_) + return; + + receivedFirstByte_ = true; + // Set timeout to Config.Timeout.request + typedef CommCbMemFunT TimeoutDialer; + AsyncCall::Pointer timeoutCall = JobCallback(33, 5, + TimeoutDialer, this, ConnStateData::requestTimeout); + commSetConnTimeout(clientConnection, Config.Timeout.request, timeoutCall); +} + /** * Attempt to parse one or more requests from the input buffer. * Returns true after completing parsing of at least one request [header]. That @@ -3073,6 +3145,8 @@ ConnStateData::clientReadRequest(const CommIoCbParams &io) case Comm::OK: kb_incr(&(statCounter.client_http.kbytes_in), rd.size); + if (!receivedFirstByte_) + receivedFirstByte(); // may comm_close or setReplyToError if (!handleReadData()) return; @@ -3294,6 +3368,24 @@ ConnStateData::noteBodyConsumerAborted(BodyPipe::Pointer ) void ConnStateData::requestTimeout(const CommTimeoutCbParams &io) { + if (Config.accessList.on_unsupported_protocol && !receivedFirstByte_) { +#if USE_OPENSSL + if (serverBump() && (serverBump()->act.step1 == Ssl::bumpPeek || serverBump()->act.step1 == Ssl::bumpStare)) { + if (spliceOnError(ERR_REQUEST_START_TIMEOUT)) { + receivedFirstByte(); + return; + } + } else if (fd_table[io.conn->fd].ssl == NULL) +#endif + { + const HttpRequestMethod method; + if (clientTunnelOnError(this, NULL, NULL, method, ERR_REQUEST_START_TIMEOUT, Http::scNone, NULL)) { + // Tunnel established. Set receivedFirstByte to avoid loop. + receivedFirstByte(); + return; + } + } + } /* * Just close the connection to not confuse browsers * using persistent connections. Some browsers open @@ -3330,7 +3422,8 @@ ConnStateData::ConnStateData(const MasterXaction::Pointer &xact) : signAlgorithm(Ssl::algSignTrusted), #endif stoppedSending_(NULL), - stoppedReceiving_(NULL) + stoppedReceiving_(NULL), + receivedFirstByte_(false) { flags.readMore = true; // kids may overwrite flags.swanSang = false; @@ -3490,7 +3583,13 @@ httpsCreate(const Comm::ConnectionPointer &conn, SSL_CTX *sslContext) return NULL; } -static bool +/** + * + * \retval 1 on success + * \retval 0 when needs more data + * \retval -1 on error + */ +static int Squid_SSL_accept(ConnStateData *conn, PF *callback) { int fd = conn->clientConnection->fd; @@ -3504,18 +3603,16 @@ Squid_SSL_accept(ConnStateData *conn, PF *callback) case SSL_ERROR_WANT_READ: Comm::SetSelect(fd, COMM_SELECT_READ, callback, conn, 0); - return false; + return 0; case SSL_ERROR_WANT_WRITE: Comm::SetSelect(fd, COMM_SELECT_WRITE, callback, conn, 0); - return false; + return 0; case SSL_ERROR_SYSCALL: if (ret == 0) { debugs(83, 2, "Error negotiating SSL connection on FD " << fd << ": Aborted by client: " << ssl_error); - comm_close(fd); - return false; } else { int hard = 1; @@ -3524,28 +3621,23 @@ Squid_SSL_accept(ConnStateData *conn, PF *callback) debugs(83, hard ? 1 : 2, "Error negotiating SSL connection on FD " << fd << ": " << strerror(errno) << " (" << errno << ")"); - - comm_close(fd); - - return false; } + return -1; case SSL_ERROR_ZERO_RETURN: debugs(83, DBG_IMPORTANT, "Error negotiating SSL connection on FD " << fd << ": Closed by client"); - comm_close(fd); - return false; + return -1; default: debugs(83, DBG_IMPORTANT, "Error negotiating SSL connection on FD " << fd << ": " << ERR_error_string(ERR_get_error(), NULL) << " (" << ssl_error << "/" << ret << ")"); - comm_close(fd); - return false; + return -1; } /* NOTREACHED */ } - return true; + return 1; } /** negotiate an SSL connection */ @@ -3556,8 +3648,12 @@ clientNegotiateSSL(int fd, void *data) X509 *client_cert; SSL *ssl = fd_table[fd].ssl; - if (!Squid_SSL_accept(conn, clientNegotiateSSL)) + int ret; + if ((ret = Squid_SSL_accept(conn, clientNegotiateSSL)) <= 0) { + if (ret < 0) // An error + comm_close(fd); return; + } if (SSL_session_reused(ssl)) { debugs(83, 2, "clientNegotiateSSL: Session " << SSL_get_session(ssl) << @@ -4063,6 +4159,23 @@ ConnStateData::switchToHttps(HttpRequest *request, Ssl::BumpMode bumpServerMode) getSslContextStart(); } +bool +ConnStateData::spliceOnError(const err_type err) +{ + if (Config.accessList.on_unsupported_protocol) { + assert(serverBump()); + ACLFilledChecklist checklist(Config.accessList.on_unsupported_protocol, serverBump()->request.getRaw(), NULL); + checklist.requestErrorType = err; + checklist.conn(this); + allow_t answer = checklist.fastCheck(); + if (answer == ACCESS_ALLOWED && answer.kind == 1) { + splice(); + return true; + } + } + return false; +} + /** negotiate an SSL connection */ static void clientPeekAndSpliceSSL(int fd, void *data) @@ -4072,12 +4185,23 @@ clientPeekAndSpliceSSL(int fd, void *data) debugs(83, 5, "Start peek and splice on FD " << fd); - if (!Squid_SSL_accept(conn, clientPeekAndSpliceSSL)) + int ret = 0; + if ((ret = Squid_SSL_accept(conn, clientPeekAndSpliceSSL)) < 0) debugs(83, 2, "SSL_accept failed."); BIO *b = SSL_get_rbio(ssl); assert(b); Ssl::ClientBio *bio = static_cast(b->ptr); + if (ret < 0) { + const err_type err = bio->noSslClient() ? ERR_PROTOCOL_UNKNOWN : ERR_SECURE_ACCEPT_FAIL; + if (!conn->spliceOnError(err)) + conn->clientConnection->close(); + return; + } + + if (bio->rBufData().contentSize() > 0) + conn->receivedFirstByte(); + if (bio->gotHello()) { if (conn->serverBump()) { Ssl::Bio::sslFeatures const &features = bio->getFeatures(); @@ -4103,6 +4227,14 @@ void ConnStateData::startPeekAndSplice() return; // commSetConnTimeout() was called for this request before we switched. + // Fix timeout to request_start_timeout + typedef CommCbMemFunT TimeoutDialer; + AsyncCall::Pointer timeoutCall = JobCallback(33, 5, + TimeoutDialer, this, ConnStateData::requestTimeout); + commSetConnTimeout(clientConnection, Config.Timeout.request_start_timeout, timeoutCall); + // Also reset receivedFirstByte_ flag to allow this timeout work in the case we have + // a bumbed "connect" request on non transparent port. + receivedFirstByte_ = false; // Disable the client read handler until CachePeer selection is complete Comm::SetSelect(clientConnection->fd, COMM_SELECT_READ, NULL, NULL, 0); @@ -4143,44 +4275,49 @@ void httpsSslBumpStep2AccessCheckDone(allow_t answer, void *data) comm_close(connState->clientConnection->fd); } else if (bumpAction != Ssl::bumpSplice) { connState->startPeekAndSpliceDone(); - } else { - //Normally we can splice here, because we just got client hello message - SSL *ssl = fd_table[connState->clientConnection->fd].ssl; - BIO *b = SSL_get_rbio(ssl); - Ssl::ClientBio *bio = static_cast(b->ptr); - MemBuf const &rbuf = bio->rBufData(); - debugs(83,5, "Bio for " << connState->clientConnection << " read " << rbuf.contentSize() << " helo bytes"); - // Do splice: - fd_table[connState->clientConnection->fd].read_method = &default_read_method; - fd_table[connState->clientConnection->fd].write_method = &default_write_method; - - if (connState->transparent()) { - // set the current protocol to something sensible (was "HTTPS" for the bumping process) - // we are sending a faked-up HTTP/1.1 message wrapper, so go with that. - connState->transferProtocol = Http::ProtocolVersion(); - // fake a CONNECT request to force connState to tunnel - static char ip[MAX_IPSTRLEN]; - connState->clientConnection->local.toUrl(ip, sizeof(ip)); - connState->in.buf.assign("CONNECT ").append(ip).append(" HTTP/1.1\r\nHost: ").append(ip).append("\r\n\r\n").append(rbuf.content(), rbuf.contentSize()); - bool ret = connState->handleReadData(); - if (ret) - ret = connState->clientParseRequests(); + } else + connState->splice(); +} - if (!ret) { - debugs(33, 2, "Failed to start fake CONNECT request for ssl spliced connection: " << connState->clientConnection); - connState->clientConnection->close(); - } - } else { - // XXX: assuming that there was an HTTP/1.1 CONNECT to begin with... - - // reset the current protocol to HTTP/1.1 (was "HTTPS" for the bumping process) - connState->transferProtocol = Http::ProtocolVersion(); - // in.buf still has the "CONNECT ..." request data, reset it to SSL hello message - connState->in.buf.append(rbuf.content(), rbuf.contentSize()); - ClientSocketContext::Pointer context = connState->getCurrentContext(); - ClientHttpRequest *http = context->http; - tunnelStart(http, &http->out.size, &http->al->http.code, http->al); +void +ConnStateData::splice() +{ + //Normally we can splice here, because we just got client hello message + SSL *ssl = fd_table[clientConnection->fd].ssl; + BIO *b = SSL_get_rbio(ssl); + Ssl::ClientBio *bio = static_cast(b->ptr); + MemBuf const &rbuf = bio->rBufData(); + debugs(83,5, "Bio for " << clientConnection << " read " << rbuf.contentSize() << " helo bytes"); + // Do splice: + fd_table[clientConnection->fd].read_method = &default_read_method; + fd_table[clientConnection->fd].write_method = &default_write_method; + + if (transparent()) { + // set the current protocol to something sensible (was "HTTPS" for the bumping process) + // we are sending a faked-up HTTP/1.1 message wrapper, so go with that. + transferProtocol = Http::ProtocolVersion(); + // fake a CONNECT request to force connState to tunnel + static char ip[MAX_IPSTRLEN]; + clientConnection->local.toUrl(ip, sizeof(ip)); + in.buf.assign("CONNECT ").append(ip).append(" HTTP/1.1\r\nHost: ").append(ip).append("\r\n\r\n").append(rbuf.content(), rbuf.contentSize()); + bool ret = handleReadData(); + if (ret) + ret = clientParseRequests(); + + if (!ret) { + debugs(33, 2, "Failed to start fake CONNECT request for ssl spliced connection: " << clientConnection); + clientConnection->close(); } + } else { + // XXX: assuming that there was an HTTP/1.1 CONNECT to begin with... + + // reset the current protocol to HTTP/1.1 (was "HTTPS" for the bumping process) + transferProtocol = Http::ProtocolVersion(); + // in.buf still has the "CONNECT ..." request data, reset it to SSL hello message + in.buf.append(rbuf.content(), rbuf.contentSize()); + ClientSocketContext::Pointer context = getCurrentContext(); + ClientHttpRequest *http = context->http; + tunnelStart(http, &http->out.size, &http->al->http.code, http->al); } } @@ -4886,4 +5023,3 @@ ConnStateData::unpinConnection(const bool andClose) /* NOTE: pinning.pinned should be kept. This combined with fd == -1 at the end of a request indicates that the host * connection has gone away */ } - diff --git a/src/client_side.h b/src/client_side.h index 468859ac7a..341c735903 100644 --- a/src/client_side.h +++ b/src/client_side.h @@ -187,6 +187,9 @@ public: int getConcurrentRequestCount() const; bool isOpen() const; + /// Update flags and timeout after the first byte received + void receivedFirstByte(); + // HttpControlMsgSink API virtual void sendControlMsg(HttpControlMsg msg); @@ -347,6 +350,14 @@ public: /// called by FwdState when it is done bumping the server void httpsPeeked(Comm::ConnectionPointer serverConnection); + /// Splice a bumped client connection on peek-and-splice mode + void splice(); + + /// Check on_unsupported_protocol access list and splice if required + /// \retval true on splice + /// \retval false otherwise + bool spliceOnError(const err_type err); + /// Start to create dynamic SSL_CTX for host or uses static port SSL context. void getSslContextStart(); /** @@ -403,6 +414,9 @@ public: /// stop parsing the request and create context for relaying error info ClientSocketContext *abortRequestParsing(const char *const errUri); + /// client data which may need to forward as-is to server after an + /// on_unsupported_protocol tunnel decision. + SBuf preservedClientData; protected: void startDechunkingRequest(); void finishDechunkingRequest(bool withSuccess); @@ -472,6 +486,7 @@ private: AsyncCall::Pointer reader; ///< set when we are reading + bool receivedFirstByte_; ///< true if at least one byte received on this connection SBuf connectionTag_; ///< clt_conn_tag=Tag annotation for client connection }; diff --git a/src/err_type.h b/src/err_type.h index 8105140402..4896b39a93 100644 --- a/src/err_type.h +++ b/src/err_type.h @@ -27,6 +27,7 @@ typedef enum { ERR_WRITE_ERROR, ERR_CONNECT_FAIL, ERR_SECURE_CONNECT_FAIL, + ERR_SECURE_ACCEPT_FAIL, ERR_SOCKET_FAILURE, /* DNS Errors */ @@ -68,6 +69,8 @@ typedef enum { ERR_DIR_LISTING, /* Display of remote directory (FTP, Gopher) */ ERR_SQUID_SIGNATURE, /* not really an error */ ERR_SHUTTING_DOWN, + ERR_PROTOCOL_UNKNOWN, + ERR_REQUEST_START_TIMEOUT, // NOTE: error types defined below TCP_RESET are optional and do not generate // a log warning if the files are missing @@ -81,5 +84,24 @@ typedef enum { extern const char *err_type_str[]; +inline +err_type +errorTypeByName(const char *name) +{ + for (int i = 0; i < ERR_MAX; ++i) + if (strcmp(name, err_type_str[i]) == 0) + return (err_type)i; + return ERR_MAX; +} + +inline +const char * +errorTypeName(err_type err) +{ + if (err < ERR_NONE || err >= ERR_MAX) + return "UNKNOWN"; + return err_type_str[err]; +} + #endif /* _SQUID_ERR_TYPE_H */ diff --git a/src/http/one/RequestParser.cc b/src/http/one/RequestParser.cc index 74c8fccf63..4d55204d7a 100644 --- a/src/http/one/RequestParser.cc +++ b/src/http/one/RequestParser.cc @@ -152,6 +152,12 @@ Http::One::RequestParser::parseRequestFirstLine() request_parse_status = Http::scBadRequest; return -1; } + + // We are expecting printable ascii characters for method/first word + if (first_whitespace < 0 && (!xisascii(buf_[i]) || !xisprint(buf_[i]))) { + request_parse_status = Http::scBadRequest; + return -1; + } } if (req.end == -1) { diff --git a/src/servers/Http1Server.cc b/src/servers/Http1Server.cc index bdc78985df..28ca5c5488 100644 --- a/src/servers/Http1Server.cc +++ b/src/servers/Http1Server.cc @@ -52,7 +52,7 @@ Http::One::Server::start() typedef CommCbMemFunT TimeoutDialer; AsyncCall::Pointer timeoutCall = JobCallback(33, 5, TimeoutDialer, this, Http1::Server::requestTimeout); - commSetConnTimeout(clientConnection, Config.Timeout.request, timeoutCall); + commSetConnTimeout(clientConnection, Config.Timeout.request_start_timeout, timeoutCall); readSomeData(); } @@ -88,6 +88,7 @@ Http::One::Server::parseOneRequest() } void clientProcessRequestFinished(ConnStateData *conn, const HttpRequest::Pointer &request); +bool clientTunnelOnError(ConnStateData *conn, ClientSocketContext *context, HttpRequest *request, const HttpRequestMethod& method, err_type requestError, Http::StatusCode errStatusCode, const char *requestErrorBytes); bool Http::One::Server::buildHttpRequest(ClientSocketContext *context) @@ -95,14 +96,7 @@ Http::One::Server::buildHttpRequest(ClientSocketContext *context) HttpRequest::Pointer request; ClientHttpRequest *http = context->http; if (context->flags.parsed_ok == 0) { - clientStreamNode *node = context->getClientReplyContext(); debugs(33, 2, "Invalid Request"); - quitAfterError(NULL); - // setLogUri should called before repContext->setReplyToError - setLogUri(http, http->uri, true); - clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); - assert(repContext); - // determine which error page templates to use for specific parsing errors err_type errPage = ERR_INVALID_REQ; switch (parser_->request_parse_status) { @@ -118,27 +112,33 @@ Http::One::Server::buildHttpRequest(ClientSocketContext *context) errPage = ERR_UNSUP_HTTPVERSION; break; default: - // use default ERR_INVALID_REQ set above. + if (parser_->method() == METHOD_NONE || parser_->requestUri().length() == 0) + // no method or url parsed, probably is wrong protocol + errPage = ERR_PROTOCOL_UNKNOWN; + // else use default ERR_INVALID_REQ set above. break; } - repContext->setReplyToError(errPage, parser_->request_parse_status, parser_->method(), http->uri, - clientConnection->remote, NULL, in.buf.c_str(), NULL); - assert(context->http->out.offset == 0); - context->pullData(); + // setLogUri should called before repContext->setReplyToError + setLogUri(http, http->uri, true); + const char * requestErrorBytes = in.buf.c_str(); + if (!clientTunnelOnError(this, context, request.getRaw(), parser_->method(), errPage, parser_->request_parse_status, requestErrorBytes)) { + // HttpRequest object not build yet, there is no reason to call + // clientProcessRequestFinished method + } + return false; } if ((request = HttpRequest::CreateFromUrlAndMethod(http->uri, parser_->method())) == NULL) { - clientStreamNode *node = context->getClientReplyContext(); debugs(33, 5, "Invalid URL: " << http->uri); - quitAfterError(request.getRaw()); // setLogUri should called before repContext->setReplyToError setLogUri(http, http->uri, true); - clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); - assert(repContext); - repContext->setReplyToError(ERR_INVALID_URL, Http::scBadRequest, parser_->method(), http->uri, clientConnection->remote, NULL, NULL, NULL); - assert(context->http->out.offset == 0); - context->pullData(); + + const char * requestErrorBytes = in.buf.c_str(); + if (!clientTunnelOnError(this, context, request.getRaw(), parser_->method(), ERR_INVALID_URL, Http::scBadRequest, requestErrorBytes)) { + // HttpRequest object not build yet, there is no reason to call + // clientProcessRequestFinished method + } return false; } @@ -148,34 +148,26 @@ Http::One::Server::buildHttpRequest(ClientSocketContext *context) if ( (parser_->messageProtocol().major == 0 && parser_->messageProtocol().minor != 9) || (parser_->messageProtocol().major > 1) ) { - clientStreamNode *node = context->getClientReplyContext(); debugs(33, 5, "Unsupported HTTP version discovered. :\n" << parser_->messageProtocol()); - quitAfterError(request.getRaw()); // setLogUri should called before repContext->setReplyToError setLogUri(http, http->uri, true); - clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); - assert (repContext); - repContext->setReplyToError(ERR_UNSUP_HTTPVERSION, Http::scHttpVersionNotSupported, parser_->method(), http->uri, - clientConnection->remote, NULL, NULL, NULL); - assert(context->http->out.offset == 0); - context->pullData(); - clientProcessRequestFinished(this, request); + + const char * requestErrorBytes = NULL; //HttpParserHdrBuf(parser_); + if (!clientTunnelOnError(this, context, request.getRaw(), parser_->method(), ERR_UNSUP_HTTPVERSION, Http::scHttpVersionNotSupported, requestErrorBytes)) { + clientProcessRequestFinished(this, request); + } return false; } /* compile headers */ if (parser_->messageProtocol().major >= 1 && !request->parseHeader(*parser_.getRaw())) { - clientStreamNode *node = context->getClientReplyContext(); debugs(33, 5, "Failed to parse request headers:\n" << parser_->mimeHeader()); - quitAfterError(request.getRaw()); // setLogUri should called before repContext->setReplyToError setLogUri(http, http->uri, true); - clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); - assert(repContext); - repContext->setReplyToError(ERR_INVALID_REQ, Http::scBadRequest, parser_->method(), http->uri, clientConnection->remote, NULL, NULL, NULL); - assert(context->http->out.offset == 0); - context->pullData(); - clientProcessRequestFinished(this, request); + const char * requestErrorBytes = NULL; //HttpParserHdrBuf(parser_); + if (!clientTunnelOnError(this, context, request.getRaw(), parser_->method(), ERR_INVALID_REQ, Http::scBadRequest, requestErrorBytes)) { + clientProcessRequestFinished(this, request); + } return false; } diff --git a/src/ssl/bio.cc b/src/ssl/bio.cc index 69f43fed0f..a880278dce 100644 --- a/src/ssl/bio.cc +++ b/src/ssl/bio.cc @@ -231,6 +231,7 @@ Ssl::ClientBio::read(char *buf, int size, BIO *table) #endif } else { debugs(83, 7, "Not an SSL acceptable handshake message (SSLv2 message?)"); + wrongProtocol = true; return -1; } diff --git a/src/ssl/bio.h b/src/ssl/bio.h index 73930a64f7..0abbaa075b 100644 --- a/src/ssl/bio.h +++ b/src/ssl/bio.h @@ -100,7 +100,7 @@ class ClientBio: public Bio public: /// The ssl hello message read states typedef enum {atHelloNone = 0, atHelloStarted, atHelloReceived} HelloReadState; - explicit ClientBio(const int anFd): Bio(anFd), holdRead_(false), holdWrite_(false), helloState(atHelloNone), helloSize(0) {} + explicit ClientBio(const int anFd): Bio(anFd), holdRead_(false), holdWrite_(false), helloState(atHelloNone), helloSize(0), wrongProtocol(false) {} /// The ClientBio version of the Ssl::Bio::stateChanged method /// When the client hello message retrieved, fill the @@ -118,7 +118,8 @@ public: const Bio::sslFeatures &getFeatures() const {return features;} /// Prevents or allow writting on socket. void hold(bool h) {holdRead_ = holdWrite_ = h;} - + /// True if client does not looks like an SSL client + bool noSslClient() {return wrongProtocol;} private: /// True if the SSL state corresponds to a hello message bool isClientHello(int state); @@ -128,6 +129,7 @@ private: bool holdWrite_; ///< The write hold state of the bio. HelloReadState helloState; ///< The SSL hello read state int helloSize; ///< The SSL hello message sent by client size + bool wrongProtocol; ///< true if client SSL hello parsing failed }; /// BIO node to handle socket IO for squid server side diff --git a/src/tests/testHttp1Parser.cc b/src/tests/testHttp1Parser.cc index 817609492f..e94429f8ac 100644 --- a/src/tests/testHttp1Parser.cc +++ b/src/tests/testHttp1Parser.cc @@ -938,14 +938,13 @@ testHttp1Parser::testParseRequestLineMethods() // tab padded method (NP: tab is not SP so treated as any other binary) { input.append("\tGET / HTTP/1.1\n", 16); -#if WHEN_RFC_COMPLIANT struct resultSet expect = { .parsed = false, .needsMore = false, .parserState = Http1::HTTP_PARSE_DONE, .status = Http::scBadRequest, .msgStart = 0, - .msgEnd = (int)input.length()-1, + .msgEnd = -1, //Halt on \t, no valid method char .suffixSz = input.length(), .methodStart = -1, .methodEnd = -1, @@ -957,26 +956,6 @@ testHttp1Parser::testParseRequestLineMethods() .versionEnd = -1, .version = AnyP::ProtocolVersion() }; -#else // XXX: currently broken - struct resultSet expect = { - .parsed = false, - .needsMore = true, - .parserState = Http1::HTTP_PARSE_MIME, - .status = Http::scOkay, - .msgStart = 0, // garbage collection consumes the SP - .msgEnd = (int)input.length()-1, - .suffixSz = 0, - .methodStart = 0, - .methodEnd = 3, - .method = HttpRequestMethod(SBuf("\tGET")), - .uriStart = 5, - .uriEnd = 5, - .uri = "/", - .versionStart = 7, - .versionEnd = 14, - .version = AnyP::ProtocolVersion(AnyP::PROTO_HTTP,1,1) - }; -#endif output.clear(); testResults(__LINE__, input, output, expect); input.clear(); @@ -1097,14 +1076,13 @@ testHttp1Parser::testParseRequestLineInvalid() // binary code in method (invalid) { input.append("GET\x0B / HTTP/1.1\n", 16); -#if WHEN_RFC_COMPLIANT struct resultSet expect = { .parsed = false, .needsMore = false, .parserState = Http1::HTTP_PARSE_DONE, .status = Http::scBadRequest, .msgStart = 0, - .msgEnd = (int)input.length()-1, + .msgEnd = -1, //Halt on \x0B, no valid method char .suffixSz = input.length(), .methodStart = -1, .methodEnd = -1, @@ -1116,26 +1094,6 @@ testHttp1Parser::testParseRequestLineInvalid() .versionEnd = -1, .version = AnyP::ProtocolVersion() }; -#else - struct resultSet expect = { - .parsed = false, - .needsMore = true, - .parserState = Http1::HTTP_PARSE_MIME, - .status = Http::scOkay, - .msgStart = 0, // garbage collection consumes the SP - .msgEnd = (int)input.length()-1, - .suffixSz = 0, - .methodStart = 0, - .methodEnd = 3, - .method = HttpRequestMethod(SBuf("GET\x0B", 4)), - .uriStart = 5, - .uriEnd = 5, - .uri = "/", - .versionStart = 7, - .versionEnd = 14, - .version = AnyP::ProtocolVersion(AnyP::PROTO_HTTP,1,1) - }; -#endif output.clear(); testResults(__LINE__, input, output, expect); input.clear(); @@ -1171,7 +1129,6 @@ testHttp1Parser::testParseRequestLineInvalid() // binary code NUL! in method (strange but ...) { input.append("GET\0 / HTTP/1.1\n", 16); -#if WHEN_RFC_COMPLIANT struct resultSet expect = { .parsed = false, .needsMore = false, @@ -1190,26 +1147,6 @@ testHttp1Parser::testParseRequestLineInvalid() .versionEnd = -1, .version = AnyP::ProtocolVersion() }; -#else - struct resultSet expect = { - .parsed = false, - .needsMore = true, - .parserState = Http1::HTTP_PARSE_MIME, - .status = Http::scOkay, - .msgStart = 0, - .msgEnd = (int)input.length()-1, - .suffixSz = 0, - .methodStart = 0, - .methodEnd = 3, - .method = HttpRequestMethod(SBuf("GET\0",4)), - .uriStart = 5, - .uriEnd = 5, - .uri = "/", - .versionStart = 7, - .versionEnd = 14, - .version = AnyP::ProtocolVersion(AnyP::PROTO_HTTP,1,1) - }; -#endif output.clear(); testResults(__LINE__, input, output, expect); input.clear(); @@ -1276,9 +1213,9 @@ testHttp1Parser::testParseRequestLineInvalid() .parserState = Http1::HTTP_PARSE_DONE, .status = Http::scBadRequest, .msgStart = 0, - .msgEnd = (int)input.length()-1, + .msgEnd = -1, //Halt on \xB, no valid method char .suffixSz = input.length(), - .methodStart = 0, + .methodStart = -1, .methodEnd = -1, .method = HttpRequestMethod(), .uriStart = -1, @@ -1304,11 +1241,11 @@ testHttp1Parser::testParseRequestLineInvalid() .parserState = Http1::HTTP_PARSE_DONE, .status = Http::scBadRequest, .msgStart = 0, - .msgEnd = (int)input.length()-1, + .msgEnd = -1, //Halt on \t, no valid method char .suffixSz = input.length(), - .methodStart = 0, - .methodEnd = 0, - .method = HttpRequestMethod(SBuf("\t")), + .methodStart = -1, + .methodEnd = -1, + .method = HttpRequestMethod(), .uriStart = -1, .uriEnd = -1, .uri = NULL, diff --git a/src/tunnel.cc b/src/tunnel.cc index dc4077b874..fec72681ad 100644 --- a/src/tunnel.cc +++ b/src/tunnel.cc @@ -30,6 +30,7 @@ #include "LogTags.h" #include "MemBuf.h" #include "PeerSelectState.h" +#include "SBuf.h" #include "SquidConfig.h" #include "SquidTime.h" #include "StatCounters.h" @@ -150,6 +151,7 @@ public: LogTags *logTag_ptr; ///< pointer for logging Squid processing code MemBuf *connectRespBuf; ///< accumulates peer CONNECT response when we need it bool connectReqWriting; ///< whether we are writing a CONNECT request to a peer + SBuf preReadClientData; void copyRead(Connection &from, IOCB *completion); @@ -197,6 +199,7 @@ public: static void ReadConnectResponseDone(const Comm::ConnectionPointer &, char *buf, size_t len, Comm::Flag errcode, int xerrno, void *data); void readConnectResponseDone(char *buf, size_t len, Comm::Flag errcode, int xerrno); + void copyClientBytes(); }; static const char *const conn_established = "HTTP/1.1 200 Connection established\r\n\r\n"; @@ -591,7 +594,7 @@ TunnelStateData::writeServerDone(char *, size_t len, Comm::Flag flag, int xerrno const CbcPointer safetyLock(this); /* ??? should be locked by the caller... */ if (cbdataReferenceValid(this)) - copyRead(client, ReadClient); + copyClientBytes(); } /* Writes data from the server buffer to the client side */ @@ -693,6 +696,20 @@ TunnelStateData::readConnectResponse() server.bytesWanted(1, connectRespBuf->spaceSize()), call); } +void +TunnelStateData::copyClientBytes() +{ + if (preReadClientData.length()) { + size_t copyBytes = preReadClientData.length() > SQUID_TCP_SO_RCVBUF ? SQUID_TCP_SO_RCVBUF : preReadClientData.length(); + memcpy(client.buf, preReadClientData.rawContent(), copyBytes); + preReadClientData.consume(copyBytes); + client.bytesIn(copyBytes); + if (keepGoingAfterRead(copyBytes, Comm::OK, 0, client, server)) + copy(copyBytes, client, server, TunnelStateData::WriteServerDone); + } else + copyRead(client, ReadClient); +} + /** * Set the HTTP status for this request and sets the read handlers for client * and server side connections. @@ -714,18 +731,13 @@ tunnelStartShoveling(TunnelStateData *tunnelState) tunnelState->copy(tunnelState->server.len, tunnelState->server, tunnelState->client, TunnelStateData::WriteClientDone); } - // Bug 3371: shovel any payload already pushed into ConnStateData by the client request if (tunnelState->http.valid() && tunnelState->http->getConn() && !tunnelState->http->getConn()->in.buf.isEmpty()) { struct ConnStateData::In *in = &tunnelState->http->getConn()->in; debugs(26, DBG_DATA, "Tunnel client PUSH Payload: \n" << in->buf << "\n----------"); - - // We just need to ensure the bytes from ConnStateData are in client.buf already to deliver - memcpy(tunnelState->client.buf, in->buf.rawContent(), in->buf.length()); - // NP: readClient() takes care of buffer length accounting. - tunnelState->readClient(tunnelState->client.buf, in->buf.length(), Comm::OK, 0); + tunnelState->preReadClientData.append(in->buf); in->buf.consume(); // ConnStateData buffer accounting after the shuffle. - } else - tunnelState->copyRead(tunnelState->client, TunnelStateData::ReadClient); + } + tunnelState->copyClientBytes(); } }