From: Amos Jeffries Date: Sun, 29 Jul 2012 08:15:17 +0000 (-0600) Subject: Bug 3478: Allow peer selection X-Git-Tag: sourceformat-review-1~154 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7177edfbf71bde03342da16925b3c71d6c77f3dd;p=thirdparty%2Fsquid.git Bug 3478: Allow peer selection This re-enables Squid peer selection algorithms for intercepted traffic which has failed Host header verification. When host verification fails Squid will use, in order of preference: * an already PINNED server connection * the client ORIGINAL_DST details * cache_peer as chosen by selection algorithms NOTE: whenever DIRECT is selected by routing algorithms the ORIGINAL_DST is used instead. Peer selection results are updated to display PINNED and ORIGINAL_DST alongside DIRECT and cache_peer. SECURITY NOTE: At this point Squid will pass the request to cache_peer using the non-trusted Host header in their URLs. Meaning that the peers may still be poisoned by CVE-2009-0801 attacks. Only the initial intercepting proxy is protected. Full protection against CVE-2009-0801 can be enjoyed by building Squid with the -DSTRICT_HOST_VERIFY compile-time flag. This will make the peers unreachable for intercepted traffic where the Host verification has failed. --- diff --git a/src/cf.data.pre b/src/cf.data.pre index 43688ceed1..6347c4b0e0 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -1922,30 +1922,15 @@ DOC_START * Intercepted traffic which passes verification is handled normally. - For now it also forces suspicious requests to go DIRECT to the - original destination, overriding client_dst_passthru for - intercepted requests which fail Host: verification. + * Intercepted requests which fail verification are sent + to the client original destination instead of DIRECT. + This overrides 'client_dst_passthru off'. For now suspicious intercepted CONNECT requests are always responded to with an HTTP 409 (Conflict) error page. -DOC_END - -NAME: client_dst_passthru -TYPE: onoff -DEFAULT: on -LOC: Config.onoff.client_dst_passthru -DOC_START - With NAT or TPROXY intercepted traffic Squid may pass the request - directly to the original client destination IP or seek a faster - source. - This option (on by default) prevents cache_peer and alternative DNS - entries being used on intercepted traffic. Both of which lead to - the security vulnerability outlined below. - SECURITY WARNING: - - This directive should only be disabled if cache_peer are required. + SECURITY NOTE: As described in CVE-2009-0801 when the Host: header alone is used to determine the destination of a request it becomes trivial for @@ -1957,7 +1942,32 @@ DOC_START sandbox only verifies that the applet tries to contact the same IP as from where it was loaded at the IP level. The Host: header may be different from the connected IP and approved origin. + +DOC_END +NAME: client_dst_passthru +TYPE: onoff +DEFAULT: on +LOC: Config.onoff.client_dst_passthru +DOC_START + With NAT or TPROXY intercepted traffic Squid may pass the request + directly to the original client destination IP or seek a faster + source using the HTTP Host header. + + Using Host to locate alternative servers can provide faster + connectivity with a range of failure recovery options. + But can also lead to connectivity trouble when the client and + server are attempting stateful interactions unaware of the proxy. + + This option (on by default) prevents alternative DNS entries being + located to send intercepted traffic DIRECT to an origin server. + The clients original destination IP and port will be used instead. + + Regardless of this option setting, when dealing with intercepted + traffic Squid will verify the Host: header and any traffic which + fails Host verification will be treated as if this option were ON. + + see host_verify_strict for details on the verification process. DOC_END COMMENT_START diff --git a/src/forward.cc b/src/forward.cc index ccfab97e81..dffa87ae3e 100644 --- a/src/forward.cc +++ b/src/forward.cc @@ -134,12 +134,15 @@ void FwdState::start(Pointer aSelf) const bool useOriginalDst = Config.onoff.client_dst_passthru || (request && !request->flags.hostVerified); if (isIntercepted && useOriginalDst) { selectPeerForIntercepted(); - // destination "found". continue with the forwarding. +#if STRICT_ORIGINAL_DST + // 3.2 does not suppro re-wrapping inside CONNECT. + // our only alternative is to fake destination "found" and continue with the forwarding. startConnectionOrFail(); - } else { - // do full route options selection - peerSelect(&serverDestinations, request, entry, fwdPeerSelectionCompleteWrapper, this); + return; +#endif } + // do full route options selection + peerSelect(&serverDestinations, request, entry, fwdPeerSelectionCompleteWrapper, this); } /// bypasses peerSelect() when dealing with intercepted requests @@ -151,19 +154,22 @@ FwdState::selectPeerForIntercepted() if (ConnStateData *client = request->pinnedConnection()) p = client->validatePinnedConnection(request, NULL); - if (p != NULL && Comm::IsConnOpen(p)) { - debugs(17, 3, HERE << "reusing a pinned conn: " << *p); + if (Comm::IsConnOpen(p)) { /* duplicate peerSelectPinned() effects */ p->peerType = PINNED; entry->ping_status = PING_DONE; /* Skip ICP */ - } else { - p = new Comm::Connection(); - p->peerType = ORIGINAL_DST; - p->remote = clientConn->local; - getOutgoingAddress(request, p); - debugs(17, 3, HERE << "opening a new conn: " << *p); + + debugs(17, 3, HERE << "reusing a pinned conn: " << *p); + serverDestinations.push_back(p); } + // use client original destination as second preferred choice + p = new Comm::Connection(); + p->peerType = ORIGINAL_DST; + p->remote = clientConn->local; + getOutgoingAddress(request, p); + + debugs(17, 3, HERE << "using client original destination: " << *p); serverDestinations.push_back(p); } diff --git a/src/peer_select.cc b/src/peer_select.cc index 1240bda45b..6b604b428c 100644 --- a/src/peer_select.cc +++ b/src/peer_select.cc @@ -222,6 +222,34 @@ peerSelectDnsPaths(ps_state *psstate) { FwdServer *fs = psstate->servers; + // Bug 3243: CVE 2009-0801 + // Bypass of browser same-origin access control in intercepted communication + // To resolve this we must use only the original client destination when going DIRECT + // on intercepted traffic which failed Host verification + const HttpRequest *req = psstate->request; + const bool isIntercepted = !req->flags.redirected && + (req->flags.intercepted || req->flags.spoof_client_ip); + const bool useOriginalDst = Config.onoff.client_dst_passthru || !req->flags.hostVerified; + const bool choseDirect = fs && fs->code == HIER_DIRECT; + if (isIntercepted && useOriginalDst && choseDirect) { + // construct a "result" adding the ORIGINAL_DST to the set instead of DIRECT + Comm::ConnectionPointer p = new Comm::Connection(); + p->remote = req->clientConnectionManager->clientConnection->local; + p->peerType = fs->code; + p->setPeer(fs->_peer); + + // check for a configured outgoing address for this destination... + getOutgoingAddress(psstate->request, p); + psstate->paths->push_back(p); + + // clear the used fs and continue + psstate->servers = fs->next; + cbdataReferenceDone(fs->_peer); + memFree(fs, MEM_FWD_SERVER); + peerSelectDnsPaths(psstate); + return; + } + // convert the list of FwdServer destinations into destinations IP addresses if (fs && psstate->paths->size() < (unsigned int)Config.forward_max_tries) { // send the next one off for DNS lookup. @@ -247,6 +275,10 @@ peerSelectDnsPaths(ps_state *psstate) for (size_t i = 0; i < psstate->paths->size(); ++i) { if ((*psstate->paths)[i]->peerType == HIER_DIRECT) debugs(44, 2, " DIRECT = " << (*psstate->paths)[i]); + else if ((*psstate->paths)[i]->peerType == ORIGINAL_DST) + debugs(44, 2, " ORIGINAL_DST = " << (*psstate->paths)[i]); + else if ((*psstate->paths)[i]->peerType == PINNED) + debugs(44, 2, " PINNED = " << (*psstate->paths)[i]); else debugs(44, 2, " cache_peer = " << (*psstate->paths)[i]); }