From 771908d313ee9c255adfb5e4fdba4d6797c18409 Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Thu, 7 Mar 2019 13:50:38 +0000 Subject: [PATCH] Bug 4928: Cannot convert non-IPv4 to IPv4 (#379) ... when reaching client_ip_max_connections The client_ip_max_connections limit is checked before the TCP dst-IP is located for the newly received TCP connection. This leaves Squid unable to fetch the NFMARK or similar details later on (they do not exist for [::]). Move client_ip_max_connections test later in the TCP accept process to ensure dst-IP is known when the error is produced. --- src/comm/TcpAcceptor.cc | 82 ++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 43 deletions(-) diff --git a/src/comm/TcpAcceptor.cc b/src/comm/TcpAcceptor.cc index d4b576d..936aa30 100644 --- a/src/comm/TcpAcceptor.cc +++ b/src/comm/TcpAcceptor.cc @@ -282,7 +282,16 @@ Comm::TcpAcceptor::acceptOne() ConnectionPointer newConnDetails = new Connection(); const Comm::Flag flag = oldAccept(newConnDetails); - if (flag == Comm::COMM_ERROR) { + /* Check for errors */ + if (!newConnDetails->isOpen()) { + + if (flag == Comm::NOMESSAGE) { + /* register interest again */ + debugs(5, 5, HERE << "try later: " << conn << " handler Subscription: " << theCallSub); + SetSelect(conn->fd, COMM_SELECT_READ, doAccept, this, 0); + return; + } + // A non-recoverable error; notify the caller */ debugs(5, 5, HERE << "non-recoverable error:" << status() << " handler Subscription: " << theCallSub); if (intendedForUserConnections()) @@ -292,16 +301,12 @@ Comm::TcpAcceptor::acceptOne() return; } - if (flag == Comm::NOMESSAGE) { - /* register interest again */ - debugs(5, 5, "try later: " << conn << " handler Subscription: " << theCallSub); - } else { - debugs(5, 5, "Listener: " << conn << - " accepted new connection " << newConnDetails << - " handler Subscription: " << theCallSub); - notify(flag, newConnDetails); - } + newConnDetails->nfmark = Ip::Qos::getNfmarkFromConnection(newConnDetails, Ip::Qos::dirAccepted); + debugs(5, 5, HERE << "Listener: " << conn << + " accepted new connection " << newConnDetails << + " handler Subscription: " << theCallSub); + notify(flag, newConnDetails); SetSelect(conn->fd, COMM_SELECT_READ, doAccept, this, 0); } @@ -341,8 +346,8 @@ Comm::TcpAcceptor::notify(const Comm::Flag flag, const Comm::ConnectionPointer & * * \retval Comm::OK success. details parameter filled. * \retval Comm::NOMESSAGE attempted accept() but nothing useful came in. - * Or this client has too many connections already. * \retval Comm::COMM_ERROR an outright failure occurred. + * Or this client has too many connections already. */ Comm::Flag Comm::TcpAcceptor::oldAccept(Comm::ConnectionPointer &details) @@ -383,6 +388,15 @@ Comm::TcpAcceptor::oldAccept(Comm::ConnectionPointer &details) details->remote = *gai; + if ( Config.client_ip_max_connections >= 0) { + if (clientdbEstablished(details->remote, 0) > Config.client_ip_max_connections) { + debugs(50, DBG_IMPORTANT, "WARNING: " << details->remote << " attempting more than " << Config.client_ip_max_connections << " connections."); + Ip::Address::FreeAddr(gai); + PROF_stop(comm_accept); + return Comm::COMM_ERROR; + } + } + // lookup the local-end details of this new connection Ip::Address::InitAddr(gai); details->local.setEmpty(); @@ -396,6 +410,23 @@ Comm::TcpAcceptor::oldAccept(Comm::ConnectionPointer &details) details->local = *gai; Ip::Address::FreeAddr(gai); + /* fdstat update */ + fdd_table[sock].close_file = NULL; + fdd_table[sock].close_line = 0; + + fde *F = &fd_table[sock]; + details->remote.toStr(F->ipaddr,MAX_IPSTRLEN); + F->remote_port = details->remote.port(); + F->local_addr = details->local; + F->sock_family = details->local.isIPv6()?AF_INET6:AF_INET; + + // set socket flags + commSetCloseOnExec(sock); + commSetNonBlocking(sock); + + /* IFF the socket is (tproxy) transparent, pass the flag down to allow spoofing */ + F->flags.transparent = fd_table[conn->fd].flags.transparent; // XXX: can we remove this line yet? + // Perform NAT or TPROXY operations to retrieve the real client/dest IP addresses if (conn->flags&(COMM_TRANSPARENT|COMM_INTERCEPTION) && !Ip::Interceptor.Lookup(details, conn)) { debugs(50, DBG_IMPORTANT, "ERROR: NAT/TPROXY lookup failed to locate original IPs on " << details); @@ -414,33 +445,6 @@ Comm::TcpAcceptor::oldAccept(Comm::ConnectionPointer &details) } #endif - details->nfmark = Ip::Qos::getNfmarkFromConnection(details, Ip::Qos::dirAccepted); - - if (Config.client_ip_max_connections >= 0) { - if (clientdbEstablished(details->remote, 0) > Config.client_ip_max_connections) { - debugs(50, DBG_IMPORTANT, "WARNING: " << details->remote << " attempting more than " << Config.client_ip_max_connections << " connections."); - PROF_stop(comm_accept); - return Comm::NOMESSAGE; - } - } - - /* fdstat update */ - fdd_table[sock].close_file = NULL; - fdd_table[sock].close_line = 0; - - fde *F = &fd_table[sock]; - details->remote.toStr(F->ipaddr,MAX_IPSTRLEN); - F->remote_port = details->remote.port(); - F->local_addr = details->local; - F->sock_family = details->local.isIPv6()?AF_INET6:AF_INET; - - // set socket flags - commSetCloseOnExec(sock); - commSetNonBlocking(sock); - - /* IFF the socket is (tproxy) transparent, pass the flag down to allow spoofing */ - F->flags.transparent = fd_table[conn->fd].flags.transparent; // XXX: can we remove this line yet? - PROF_stop(comm_accept); return Comm::OK; }