]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Audit fallout
authorAmos Jeffries <squid3@treenet.co.nz>
Sun, 12 Sep 2010 05:18:37 +0000 (17:18 +1200)
committerAmos Jeffries <squid3@treenet.co.nz>
Sun, 12 Sep 2010 05:18:37 +0000 (17:18 +1200)
12 files changed:
src/CommCalls.h
src/ProtoPort.cc
src/ProtoPort.h
src/client_side.cc
src/dns_internal.cc
src/eui/Eui48.cc
src/eui/Eui48.h
src/eui/Eui64.cc
src/eui/Eui64.h
src/ftp.cc
src/ident/Ident.cc
src/ident/Ident.h

index e44d33caa524da877fac172050ff026462951792..f258d34c2db4bfed7277569b95fc38f4ed5e9329 100644 (file)
@@ -262,7 +262,10 @@ public:
 
     // parameter cannot be const because getDialer() cannot be const
     // getDialer() cannot because Comm IO syncWithComm() alters the object params data
-    inline CommCbFunPtrCallT(Pointer &p) : dialer(*dynamic_cast<Dialer*>(p->getDialer())) {}
+    inline CommCbFunPtrCallT(const Pointer &p) :
+            AsyncCall(p->debugSection, p->debugLevel, p->name),
+            dialer(p->dialer)
+        {}
 
     virtual CallDialer* getDialer() { return &dialer; }
 
index 0050e5aafc83cfbcf3cba8898acce8ea4e47e096..f0b943800adb2d679761f74c97a5124b2f96f94c 100644 (file)
@@ -12,8 +12,8 @@ http_port_list::http_port_list(const char *aProtocol)
 
 http_port_list::~http_port_list()
 {
-    listener->unsubscribe(); // trigger self-close
-    listener = NULL;
+    if (Comm::IsConnOpen(listenConn))
+        listenConn->close();
 
     safe_free(name);
     safe_free(defaultsite);
index d3d3769ca2742816acd21c4c2d4123453d633ce9..2d349a6ed0b22c84e6dd02e7aeaabbbc0eae8401 100644 (file)
@@ -2,7 +2,7 @@
 #define SQUID_PROTO_PORT_H
 
 #include "cbdata.h"
-#include "comm/ConnAcceptor.h"
+#include "comm/Connection.h"
 
 struct http_port_list {
     http_port_list(const char *aProtocol);
@@ -34,12 +34,7 @@ struct http_port_list {
         unsigned int timeout;
     } tcp_keepalive;
 
-    /**
-     * The FD listening socket handler.
-     * If not NULL we are actively listening for client requests.
-     * delete to close the socket.
-     */
-    Comm::ConnAcceptor *listener;
+    Comm::ConnectionPointer listenConn;   ///< Socket used by a ConnAcceptor for incoming clients.
 
 #if USE_SSL
     // XXX: temporary hack to ease move of SSL options to http_port
index eeb35963732d1fec61395c8a5d4a936d8b7160f2..17e08490ab921d6ed44214b4476754352397f001 100644 (file)
@@ -85,6 +85,7 @@
 
 #include "acl/FilledChecklist.h"
 #include "auth/UserRequest.h"
+#include "base/Subscription.h"
 #include "base/TextException.h"
 #include "ChunkedCodingParser.h"
 #include "client_side.h"
@@ -3100,8 +3101,7 @@ connStateCreate(const Comm::ConnectionPointer &conn, http_port_list *port)
 
 /** Handle a new connection on HTTP socket. */
 void
-httpAccept(int sock, int unused, Comm::ConnectionPointer &details,
-           comm_err_t flag, int xerrno, void *data)
+httpAccept(int sock, const Comm::ConnectionPointer &details, comm_err_t flag, int xerrno, void *data)
 {
     http_port_list *s = (http_port_list *)data;
     ConnStateData *connState = NULL;
@@ -3162,14 +3162,15 @@ httpAccept(int sock, int unused, Comm::ConnectionPointer &details,
 
 /** Create SSL connection structure and update fd_table */
 static SSL *
-httpsCreate(Comm::ConnectionPointer &details, SSL_CTX *sslContext)
+httpsCreate(const Comm::ConnectionPointer &details, SSL_CTX *sslContext)
 {
     SSL *ssl = SSL_new(sslContext);
 
     if (!ssl) {
         const int ssl_error = ERR_get_error();
         debugs(83, 1, "httpsAccept: Error allocating handle: " << ERR_error_string(ssl_error, NULL)  );
-        details->close();
+        Comm::ConnectionPointer nonConst = details;
+        nonConst->close();
         return NULL;
     }
 
@@ -3305,8 +3306,7 @@ clientNegotiateSSL(int fd, void *data)
 
 /** handle a new HTTPS connection */
 static void
-httpsAccept(int sock, int newfd, Comm::ConnectionPointer& details,
-            comm_err_t flag, int xerrno, void *data)
+httpsAccept(int sock, const Comm::ConnectionPointer& details, comm_err_t flag, int xerrno, void *data)
 {
     https_port_list *s = (https_port_list *)data;
     SSL_CTX *sslContext = s->sslContext;
@@ -3479,17 +3479,22 @@ clientHttpConnectionOpened(int fd, int, http_port_list *s)
 
     Must(s);
 
-    s->listener = new Comm::ConnAcceptor(fd, true);
-    s->listener->subscribe(5,5, "httpAccept", new CommAcceptCbPtrFun(httpAccept, s));
-    AsyncJob::Start(s->listener);
+    s->listenConn = new Comm::Connection();
+    s->listenConn->local = s->s;
+    s->listenConn->fd = fd;
 
-    debugs(1, 1, "Accepting " <<
+    // TODO: hide most of this subscription stuff away.
+    typedef CommCbFunPtrCallT<CommAcceptCbPtrFun> AcceptCall;
+    RefCount<AcceptCall> call = commCbCall(5, 5, "httpAccept", CommAcceptCbPtrFun(httpAccept, s));
+    Subscription::Pointer sub = new CallSubscription<AcceptCall>(call);
+    AsyncJob::Start(new Comm::ConnAcceptor(s->listenConn, "HTTP Listener", sub));
+
+    debugs(1, 1, "Accepting" <<
            (s->intercepted ? " intercepted" : "") <<
            (s->spoof_client_ip ? " spoofing" : "") <<
            (s->sslBump ? " bumpy" : "") <<
            (s->accel ? " accelerated" : "")
-           << " HTTP connections at " << s->s
-           << ", FD " << fd << "." );
+           << " HTTP connections at " << s->s << ", FD " << fd << ".");
 
     Must(AddOpenedHttpSocket(fd)); // otherwise, we have received a fd we did not ask for
 }
@@ -3532,9 +3537,15 @@ clientHttpsConnectionOpened(int fd, int, http_port_list *s)
 
     Must(s);
 
-    s->listener = new Comm::ConnAcceptor(fd, true);
-    s->listener->subscribe(5,5, "httpsAccept", new CommAcceptCbPtrFun(httpsAccept, s));
-    AsyncJob::Start(s->listener);
+    s->listenConn = new Comm::Connection();
+    s->listenConn->local = s->s;
+    s->listenConn->fd = fd;
+
+    // TODO: hide most of this subscription stuff away.
+    typedef CommCbFunPtrCallT<CommAcceptCbPtrFun> AcceptCall;
+    RefCount<AcceptCall> call = commCbCall(5, 5, "httpsAccept", CommAcceptCbPtrFun(httpsAccept, s));
+    Subscription::Pointer sub = new CallSubscription<AcceptCall>(call);
+    AsyncJob::Start(new Comm::ConnAcceptor(s->listenConn, "HTTPS Listener", sub));
 
     debugs(1, 1, "Accepting HTTPS connections at " << s->s << ", FD " << fd << ".");
 
@@ -3559,19 +3570,19 @@ void
 clientHttpConnectionsClose(void)
 {
     for (http_port_list *s = Config.Sockaddr.http; s; s = s->next) {
-        if (s->listener) {
+        if (s->listenConn != NULL) {
             debugs(1, 1, "Closing HTTP port " << s->s);
-            s->listener->unsubscribe();
-            s->listener = NULL;
+            s->listenConn->close();
+            s->listenConn = NULL;
         }
     }
 
 #if USE_SSL
     for (http_port_list *s = Config.Sockaddr.https; s; s = s->next) {
-        if (s->listener) {
+        if (s->listenConn != NULL) {
             debugs(1, 1, "Closing HTTPS port " << s->s);
-            s->listener->unsubscribe();
-            s->listener = NULL;
+            s->listenConn->close();
+            s->listenConn = NULL;
         }
     }
 #endif
index 259e688c60d126f564fd09ceaea984304af4b55c..fdba37056ce880f7139db88a606866a6a4b48ac8 100644 (file)
@@ -127,7 +127,7 @@ struct _idns_query {
 
 struct _nsvc {
     int ns;
-    int fd;
+    Comm::ConnectionPointer conn;
     unsigned short msglen;
     int read_msglen;
     MemBuf *msg;
@@ -177,6 +177,7 @@ static void idnsParseWIN32SearchList(const char *);
 static void idnsCacheQuery(idns_query * q);
 static void idnsSendQuery(idns_query * q);
 static CNCB idnsInitVCConnected;
+
 static IOCB idnsReadVCHeader;
 static void idnsDoSendQueryVC(nsvc *vc);
 
@@ -697,15 +698,15 @@ idnsDoSendQueryVC(nsvc *vc)
 
     vc->busy = 1;
 
-    commSetTimeout(vc->fd, Config.Timeout.idns_query, NULL, NULL);
+    commSetTimeout(vc->conn->fd, Config.Timeout.idns_query, NULL, NULL);
 
-    comm_write_mbuf(vc->fd, mb, idnsSentQueryVC, vc);
+    comm_write_mbuf(vc->conn->fd, mb, idnsSentQueryVC, vc);
 
     delete mb;
 }
 
 static void
-idnsInitVCConnected(const Comm::ConnectionPointer &conn, const DnsLookupDetails &details, comm_err_t status, int xerrno, void *data)
+idnsInitVCConnected(const Comm::ConnectionPointer &conn, comm_err_t status, int xerrno, void *data)
 {
     nsvc * vc = (nsvc *)data;
 
@@ -713,13 +714,11 @@ idnsInitVCConnected(const Comm::ConnectionPointer &conn, const DnsLookupDetails
         char buf[MAX_IPSTRLEN] = "";
         if (vc->ns < nns)
             nameservers[vc->ns].S.NtoA(buf,MAX_IPSTRLEN);
-        debugs(78, 1, HERE << "Failed to connect to nameserver " << buf << " using TCP: " << details);
-        Comm::ConnectionPOinter nonConst = conn;
-        nonConst->close();
+        debugs(78, 1, HERE << "Failed to connect to nameserver " << buf << " using TCP.");
         return;
     }
 
-    vc->fd = conn->fd; // TODO: make the vc store the conn instead?
+    vc->conn = conn;
 
     comm_add_close_handler(conn->fd, idnsVCClosed, vc);
     comm_read(conn->fd, (char *)&vc->msglen, 2 , idnsReadVCHeader, vc);
@@ -733,6 +732,7 @@ idnsVCClosed(int fd, void *data)
     nsvc * vc = (nsvc *)data;
     delete vc->queue;
     delete vc->msg;
+    vc->conn = NULL;
     if (vc->ns < nns) // XXX: idnsShutdown may have freed nameservers[]
         nameservers[vc->ns].vc = NULL;
     cbdataFree(vc);
@@ -743,13 +743,14 @@ idnsInitVC(int ns)
 {
     nsvc *vc = cbdataAlloc(nsvc);
     assert(ns < nns);
+    assert(vc->conn == NULL); // MUST be NULL from the construction process!
     nameservers[ns].vc = vc;
     vc->ns = ns;
     vc->queue = new MemBuf;
     vc->msg = new MemBuf;
     vc->busy = 1;
 
-    Comm::ConnectionPointer conn = new Comm::Connection;
+    Comm::ConnectionPointer conn = new Comm::Connection();
 
     if (!Config.Addrs.udp_outgoing.IsNoAddr())
         conn->local = Config.Addrs.udp_outgoing;
@@ -1448,8 +1449,8 @@ idnsShutdown(void)
 
     for (int i = 0; i < nns; i++) {
         if (nsvc *vc = nameservers[i].vc) {
-            if (vc->fd >= 0)
-                comm_close(vc->fd);
+            if (Comm::IsConnOpen(vc->conn))
+                vc->conn->close();
         }
     }
 
index b9311984ab9597d5d3a12487a21eb142bcdd801b..3db77d5ac6149f932c556340c6307eae84e85359 100644 (file)
@@ -152,7 +152,7 @@ Eui::Eui48::encode(char *buf, const int len)
 
 // return binary representation of the EUI
 bool
-Eui::Eui48::lookup(Ip::Address &c)
+Eui::Eui48::lookup(const Ip::Address &c)
 {
     struct arpreq arpReq;
 #if !_SQUID_WIN32_
index 023a985a4c89e15eb9891e11aea8cf40208d2e39..982db871101eb29aa7951cc24fdd0a7d8c604a0a 100644 (file)
@@ -66,7 +66,7 @@ public:
     bool encode(char *buf, const int len);
 
     // lookup an EUI-48 / MAC address via ARP
-    bool lookup(Ip::Address &c);
+    bool lookup(const Ip::Address &c);
 
 private:
     unsigned char eui[SZ_EUI48_BUF];
index a9a9d261b27cc86943fbd9dccea6e8eb7b411334..bfbc33b8757be5655e40b2aa9656361ffba3f798 100644 (file)
@@ -36,7 +36,7 @@ Eui::Eui64::encode(char *buf, const int len)
 
 // return binary representation of the EUI
 bool
-Eui::Eui64::lookup(Ip::Address &c)
+Eui::Eui64::lookup(const Ip::Address &c)
 {
     /* try to short-circuit slow OS lookups by using SLAAC data */
     if (lookupSlaac(c)) return true;
@@ -46,7 +46,7 @@ Eui::Eui64::lookup(Ip::Address &c)
 }
 
 bool
-Eui::Eui64::lookupSlaac(Ip::Address &c)
+Eui::Eui64::lookupSlaac(const Ip::Address &c)
 {
     /* RFC 4291 Link-Local unicast addresses which contain SLAAC - usually trustable. */
     if (c.IsSiteLocal6() && c.IsSlaac() ) {
@@ -63,7 +63,7 @@ Eui::Eui64::lookupSlaac(Ip::Address &c)
 
 // return binary representation of the EUI
 bool
-Eui::Eui64::lookupNdp(Ip::Address &c)
+Eui::Eui64::lookupNdp(const Ip::Address &c)
 {
 #if 0 /* no actual lookup coded yet */
 
index 5f9e56d3f6313a87422cebd3b027079d40fab5bb..db6da3ccaffb956e5af7bfcf3d97651aa79463ec 100644 (file)
@@ -73,13 +73,13 @@ public:
     bool encode(char *buf, const int len);
 
     // lookup an EUI-64 address via IPv6 SLAAC or NDP
-    bool lookup(Ip::Address &c);
+    bool lookup(const Ip::Address &c);
 
     // lookup an EUI-64 address via IPv6 NDP
-    bool lookupNdp(Ip::Address &c);
+    bool lookupNdp(const Ip::Address &c);
 
     // lookup an EUI-64 address via decoding the IPv6 address SLAAC data
-    bool lookupSlaac(Ip::Address &c);
+    bool lookupSlaac(const Ip::Address &c);
 
 private:
     unsigned char eui[SZ_EUI64_BUF];
index cb608e796c50c45a92560b8ce776ca9a38b3b8d1..e0bada008fee0cb19042e5c29b415f629832a7a2 100644 (file)
@@ -155,17 +155,12 @@ public:
 
     Comm::ConnectionPointer conn; ///< channel descriptor
 
-    /** Current listening socket handler. delete on shutdown or abort.
-     * FTP stores a copy of the FD in the channel descriptor.
-     * Use close() to properly close the channel.
-     */
-    Comm::ConnAcceptor *listener;
-
     /** A temporary handle to the connection being listened on.
-     *  Data channel listeners die before handing the result conn over. If the conn is
-     *  a third-party attack we may need to resume listening for the genuine connect.
+     * Closing this will also close the waiting Data channel acceptor.
+     * If a data connection has already been accepted but is still waiting in the event queue
+     * the callback will still happen and needs to be handled (usually dropped).
      */
-    Comm::ConnectionPointer listen_conn;
+    Comm::ConnectionPointer listenConn;
 
 private:
     AsyncCall::Pointer closer; /// Comm close handler callback
@@ -251,7 +246,12 @@ public:
     void completedListing(void);
     void dataComplete();
     void dataRead(const CommIoCbParams &io);
-    void switchTimeoutToDataChannel(); ///< ignore timeout on CTRL channel. set read timeout on DATA channel.
+
+    /// ignore timeout on CTRL channel. set read timeout on DATA channel.
+    void switchTimeoutToDataChannel();
+    /// create a data channel acceptor and start listening.
+    void listenForDataChannel(const Comm::ConnectionPointer &conn, const char *note);
+
     int checkAuth(const HttpHeader * req_hdr);
     void checkUrlpath();
     void buildTitleUrl();
@@ -458,10 +458,9 @@ FtpStateData::ctrlClosed(const CommCloseCbParams &io)
 void
 FtpStateData::dataClosed(const CommCloseCbParams &io)
 {
-    if (data.listener) {
-        data.listener->unsubscribe(); // listener job will self destruct.
-        data.listener = NULL;
-        data.listen_conn = NULL;
+    if (data.listenConn != NULL) {
+        data.listenConn->close();
+        data.listenConn = NULL;
         data.conn = NULL;
     }
     data.clear();
@@ -623,10 +622,22 @@ FtpStateData::switchTimeoutToDataChannel()
     commSetTimeout(ctrl.conn->fd, -1, nullCall);
 
     typedef CommCbMemFunT<FtpStateData, CommTimeoutCbParams> TimeoutDialer;
-    AsyncCall::Pointer timeoutCall =  JobCallback(9, 5, TimeoutDialer, this, FtpStateData::ftpTimeout);
+    AsyncCall::Pointer timeoutCall = JobCallback(9, 5, TimeoutDialer, this, FtpStateData::ftpTimeout);
     commSetTimeout(data.conn->fd, Config.Timeout.read, timeoutCall);
 }
 
+void
+FtpStateData::listenForDataChannel(const Comm::ConnectionPointer &conn, const char *note)
+{
+    data.listenConn = conn;
+
+    typedef CommCbMemFunT<FtpStateData, CommAcceptCbParams> AcceptDialer;
+    typedef AsyncCallT<AcceptDialer> AcceptCall;
+    RefCount<AcceptCall> call = (AcceptCall*)JobCallback(11, 5, AcceptDialer, this, FtpStateData::ftpAcceptDataConnection);
+    Subscription::Pointer sub = new CallSubscription<AcceptCall>(call);
+    AsyncJob::Start(new Comm::ConnAcceptor(data.listenConn, note, sub));
+}
+
 void
 FtpStateData::ftpTimeout(const CommTimeoutCbParams &io)
 {
@@ -2707,14 +2718,14 @@ ftpOpenListenSocket(FtpStateData * ftpState, int fallback)
 {
     /// Close old data channels, if any. We may open a new one below.
     ftpState->data.close();
+    ftpState->data.host = NULL;
 
     /*
      * Set up a listen socket on the same local address as the
      * control connection.
      */
-
-    Comm::ConnectionPointer conn = new Comm::Connection;
-    conn->local = ftpState->ctrl.conn->local;
+    ftpState->data.listenConn = new Comm::Connection;
+    ftpState->data.listenConn->local = ftpState->ctrl.conn->local;
 
     /*
      * REUSEADDR is needed in fallback mode, since the same port is
@@ -2724,28 +2735,13 @@ ftpOpenListenSocket(FtpStateData * ftpState, int fallback)
         int on = 1;
         setsockopt(ftpState->ctrl.conn->fd, SOL_SOCKET, SO_REUSEADDR, (char *) &on, sizeof(on));
         ftpState->ctrl.conn->flags |= COMM_REUSEADDR;
-        conn->flags |= COMM_REUSEADDR;
+        ftpState->data.listenConn->flags |= COMM_REUSEADDR;
     } else {
         /* if not running in fallback mode a new port needs to be retrieved */
-        conn->local.SetPort(0);
+        ftpState->data.listenConn->local.SetPort(0);
     }
 
-    typedef CommCbMemFunT<FtpStateData, CommAcceptCbParams> acceptDialer;
-    AsyncCall::Pointer acceptCall = JobCallback(11, 5, acceptDialer, ftpState, FtpStateData::ftpAcceptDataConnection);
-    ftpState->data.listener = new Comm::ConnAcceptor(conn, false, ftpState->entry->url());
-    ftpState->data.listener->subscribe(acceptCall);
-
-    if (ftpState->data.listener->errcode != 0) {
-        conn->close();
-    } else {
-
-        if (!fallback)
-            conn->local.SetPort(comm_local_port(conn->fd));
-        ftpState->data.host = NULL;
-        AsyncJob::Start(ftpState->data.listener);
-    }
-
-    ftpState->data.listen_conn = conn;
+    ftpState->listenForDataChannel(ftpState->data.listenConn, ftpState->entry->url());
 }
 
 /// \ingroup ServerProtocolFTPInternal
@@ -2765,8 +2761,8 @@ ftpSendPORT(FtpStateData * ftpState)
     ftpState->flags.pasv_supported = 0;
     ftpOpenListenSocket(ftpState, 0);
 
-    if (!Comm::IsConnOpen(ftpState->data.listen_conn)) {
-        if ( ftpState->data.listen_conn != NULL && !ftpState->data.listen_conn->local.IsIPv4() ) {
+    if (!Comm::IsConnOpen(ftpState->data.listenConn)) {
+        if ( ftpState->data.listenConn != NULL && !ftpState->data.listenConn->local.IsIPv4() ) {
             /* non-IPv4 CANNOT send PORT command.                       */
             /* we got here by attempting and failing an EPRT            */
             /* using the same reply code should simulate a PORT failure */
@@ -2783,7 +2779,7 @@ ftpSendPORT(FtpStateData * ftpState)
     // source them from the listen_conn->local
 
     struct addrinfo *AI = NULL;
-    ftpState->data.listen_conn->local.GetAddrInfo(AI, AF_INET);
+    ftpState->data.listenConn->local.GetAddrInfo(AI, AF_INET);
     unsigned char *addrptr = (unsigned char *) &((struct sockaddr_in*)AI->ai_addr)->sin_addr;
     unsigned char *portptr = (unsigned char *) &((struct sockaddr_in*)AI->ai_addr)->sin_port;
     snprintf(cbuf, CTRL_BUFLEN, "PORT %d,%d,%d,%d,%d,%d\r\n",
@@ -2792,7 +2788,7 @@ ftpSendPORT(FtpStateData * ftpState)
     ftpState->writeCommand(cbuf);
     ftpState->state = SENT_PORT;
 
-    ftpState->data.listen_conn->local.FreeAddrInfo(AI);
+    ftpState->data.listenConn->local.FreeAddrInfo(AI);
 }
 
 /// \ingroup ServerProtocolFTPInternal
@@ -2826,7 +2822,7 @@ ftpSendEPRT(FtpStateData * ftpState)
     ftpState->flags.pasv_supported = 0;
 
     ftpOpenListenSocket(ftpState, 0);
-    if (!Comm::IsConnOpen(ftpState->data.listen_conn)) {
+    if (!Comm::IsConnOpen(ftpState->data.listenConn)) {
         /* XXX Need to set error message */
         ftpFail(ftpState);
         return;
@@ -2835,9 +2831,9 @@ ftpSendEPRT(FtpStateData * ftpState)
     /* RFC 2428 defines EPRT as IPv6 equivalent to IPv4 PORT command. */
     /* Which can be used by EITHER protocol. */
     snprintf(cbuf, CTRL_BUFLEN, "EPRT |%d|%s|%d|\r\n",
-             ( ftpState->data.listen_conn->local.IsIPv6() ? 2 : 1 ),
-             ftpState->data.listen_conn->local.NtoA(buf,MAX_IPSTRLEN),
-             ftpState->data.listen_conn->local.GetPort() );
+             ( ftpState->data.listenConn->local.IsIPv6() ? 2 : 1 ),
+             ftpState->data.listenConn->local.NtoA(buf,MAX_IPSTRLEN),
+             ftpState->data.listenConn->local.GetPort() );
 
     ftpState->writeCommand(cbuf);
     ftpState->state = SENT_EPRT;
@@ -2872,56 +2868,58 @@ FtpStateData::ftpAcceptDataConnection(const CommAcceptCbParams &io)
     char ntoapeer[MAX_IPSTRLEN];
     debugs(9, 3, "ftpAcceptDataConnection");
 
-    // one connection accepted. the handler has stopped listening. drop our local pointer to it.
-    data.listener = NULL;
-
     if (EBIT_TEST(entry->flags, ENTRY_ABORTED)) {
         abortTransaction("entry aborted when accepting data conn");
+        data.listenConn->close();
+        data.listenConn = NULL;
         return;
     }
 
     if (io.flag != COMM_OK) {
-        debugs(9, DBG_IMPORTANT, "ftpHandleDataAccept: FD " << io.details->fd << ": " << xstrerr(io.xerrno));
-        /** \todo XXX Need to set error message */
+        data.listenConn->close();
+        data.listenConn = NULL;
+        debugs(9, DBG_IMPORTANT, "FTP AcceptDataConnection: " << io.conn << ": " << xstrerr(io.xerrno));
+        /** \todo Need to send error message on control channel*/
         ftpFail(this);
         return;
     }
 
+    /* data listening conn is no longer even open. abort. */
+    if (!Comm::IsConnOpen(data.listenConn)) {
+        data.listenConn = NULL; // ensure that it's cleared and not just closed.
+        return;
+    }
+
     /** \par
      * When squid.conf ftp_sanitycheck is enabled, check the new connection is actually being
      * made by the remote client which is connected to the FTP control socket.
+     * Or the one which we were told to listen for by control channel messages (may differ under NAT).
      * This prevents third-party hacks, but also third-party load balancing handshakes.
      */
     if (Config.Ftp.sanitycheck) {
-        io.details->remote.NtoA(ntoapeer,MAX_IPSTRLEN);
+        io.conn->remote.NtoA(ntoapeer,MAX_IPSTRLEN);
 
         // accept if either our data or ctrl connection is talking to this remote peer.
-        if (data.listen_conn->remote != io.details->remote && ctrl.conn->remote != io.details->remote) {
+        if (data.listenConn->remote != io.conn->remote && ctrl.conn->remote != io.conn->remote) {
             debugs(9, DBG_IMPORTANT,
                    "FTP data connection from unexpected server (" <<
-                   io.details->remote << "), expecting " <<
-                   data.listen_conn->remote << " or " << ctrl.conn->remote);
-
-            /* drop the bad connection (io) by ignoring. */
+                   io.conn->remote << "), expecting " <<
+                   data.listenConn->remote << " or " << ctrl.conn->remote);
 
-            /* we are ony accepting once, so need to reset the listener socket. */
-            typedef CommCbMemFunT<FtpStateData, CommAcceptCbParams> acceptDialer;
-            AsyncCall::Pointer acceptCall = JobCallback(11, 5, acceptDialer, this, FtpStateData::ftpAcceptDataConnection);
-            data.listener = new Comm::ConnAcceptor(data.listen_conn, false, data.host);
-            data.listener->subscribe(acceptCall);
-            AsyncJob::Start(data.listener);
+            /* drop the bad connection (io) by ignoring the attempt. */
             return;
         }
     }
 
     /** On COMM_OK start using the accepted data socket and discard the temporary listen socket. */
     data.close();
-    data.opened(io.details, dataCloser());
-    io.details->remote.NtoA(data.host,SQUIDHOSTNAMELEN);
-    data.listen_conn = NULL;
+    data.opened(io.conn, dataCloser());
+    io.conn->remote.NtoA(data.host,SQUIDHOSTNAMELEN);
+    data.listenConn->close();
+    data.listenConn = NULL;
 
     debugs(9, 3, HERE << "Connected data socket on " <<
-           "FD " << io.details->fd << " to " << io.details->remote << " FD table says: " <<
+           io.conn << ". FD table says: " <<
            "ctrl-peer= " << fd_table[ctrl.conn->fd].ipaddr << ", " <<
            "data-peer= " << fd_table[data.conn->fd].ipaddr);
 
@@ -3010,13 +3008,7 @@ void FtpStateData::readStor()
     } else if (code == 150) {
         /* When client code is 150 with no data channel, Accept data channel. */
         debugs(9, 3, "ftpReadStor: accepting data channel");
-        typedef CommCbMemFunT<FtpStateData, CommAcceptCbParams> acceptDialer;
-        AsyncCall::Pointer acceptCall = JobCallback(11, 5,
-                                        acceptDialer, this, FtpStateData::ftpAcceptDataConnection);
-
-        data.listener = new Comm::ConnAcceptor(data.conn, false, data.host);
-        data.listener->subscribe(acceptCall);
-        AsyncJob::Start(data.listener);
+        listenForDataChannel(data.conn, data.host);
     } else {
         debugs(9, DBG_IMPORTANT, HERE << "Unexpected reply code "<< std::setfill('0') << std::setw(3) << code);
         ftpFail(this);
@@ -3146,13 +3138,7 @@ ftpReadList(FtpStateData * ftpState)
         ftpState->switchTimeoutToDataChannel();
 
         /* Accept data channel */
-        typedef CommCbMemFunT<FtpStateData, CommAcceptCbParams> acceptDialer;
-        AsyncCall::Pointer acceptCall = JobCallback(11, 5,
-                                        acceptDialer, ftpState, FtpStateData::ftpAcceptDataConnection);
-
-        ftpState->data.listener = new Comm::ConnAcceptor(ftpState->data.conn, false, ftpState->data.host);
-        ftpState->data.listener->subscribe(acceptCall);
-        AsyncJob::Start(ftpState->data.listener);
+        ftpState->listenForDataChannel(ftpState->data.conn, ftpState->data.host);
         return;
     } else if (!ftpState->flags.tried_nlst && code > 300) {
         ftpSendNlst(ftpState);
@@ -3194,12 +3180,7 @@ ftpReadRetr(FtpStateData * ftpState)
     } else if (code == 150) {
         /* Accept data channel */
         ftpState->switchTimeoutToDataChannel();
-        typedef CommCbMemFunT<FtpStateData, CommAcceptCbParams> acceptDialer;
-        AsyncCall::Pointer acceptCall = JobCallback(11, 5,
-                                        acceptDialer, ftpState, FtpStateData::ftpAcceptDataConnection);
-        ftpState->data.listener = new Comm::ConnAcceptor(ftpState->data.conn, false, ftpState->data.host);
-        ftpState->data.listener->subscribe(acceptCall);
-        AsyncJob::Start(ftpState->data.listener);
+        ftpState->listenForDataChannel(ftpState->data.conn, ftpState->data.host);
     } else if (code >= 300) {
         if (!ftpState->flags.try_slash_hack) {
             /* Try this as a directory missing trailing slash... */
@@ -3859,18 +3840,17 @@ void
 FtpChannel::close()
 {
     // channels with active listeners will be closed when the listener handler dies.
-    if (listener) {
-        listener->unsubscribe(); /// listener job will self-destruct.
-        listener = NULL;
+    if (listenConn != NULL) {
+        listenConn->close();
+        listenConn = NULL;
         comm_remove_close_handler(conn->fd, closer);
         closer = NULL;
-        conn = NULL;
     } else if (Comm::IsConnOpen(conn)) {
         comm_remove_close_handler(conn->fd, closer);
         closer = NULL;
         conn->close(); // we do not expect to be called back
-        conn = NULL;
     }
+    conn = NULL;
 }
 
 void
index e47c13ddbb336bef895b55fe54b2417747451265..3fd597621d1761c26a1437f5fecd6f9a9e2ff2ac 100644 (file)
@@ -219,7 +219,7 @@ CBDATA_TYPE(IdentStateData);
  * start a TCP connection to the peer host on port 113
  */
 void
-Ident::Start(Comm::ConnectionPointer &conn, IDCB * callback, void *data)
+Ident::Start(const Comm::ConnectionPointer &conn, IDCB * callback, void *data)
 {
     IdentStateData *state;
     char key1[IDENT_KEY_SZ];
index 15ed12842a861d0b68e5b43a5ad0e3ae7d0c8c42..88db7ad0f1d11341b9483331f9f6fd657f138765 100644 (file)
@@ -27,7 +27,7 @@ namespace Ident
  * Self-registers with a global ident lookup manager,
  * will call Ident::Init() itself if the manager has not been initialized already.
  */
-void Start(Comm::ConnectionPointer &conn, IDCB * callback, void *cbdata);
+void Start(const Comm::ConnectionPointer &conn, IDCB * callback, void *cbdata);
 
 /**
  \ingroup IdentAPI