Refactored Ip::Intercept::Lookup() (#1062)

The TPROXY lookup part was misleading -- the method only checked that
TPROXY was enabled by the configuration and did not change the accepted
connection settings.

Also do not assume that a listening port can have both COMM_TRANSPARENT
and COMM_INTERCEPTION flags set. Squid treats TPROXY and NAT modes as
mutually exclusive and rejects such configurations.

Also forbid invalid port configurations with tproxy mode where TPROXY is
prohibited by ./configure.

Also removed unused Ip::Intercept::StopInterception().

diff --git a/src/comm/TcpAcceptor.cc b/src/comm/TcpAcceptor.cc
index 42f73c5397361603e4b3c96f8728bf2d181929bb..0faa640a8c2149d247c9a77138887574035d42ef 100644 (file)
--- a/src/comm/TcpAcceptor.cc
+++ b/src/comm/TcpAcceptor.cc
@@ -388,10 +388,19 @@ Comm::TcpAcceptor::oldAccept(Comm::ConnectionPointer &details)
     details->local = *gai;
     Ip::Address::FreeAddr(gai);
 
-    // 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);
-        return Comm::NOMESSAGE;
+    if (conn->flags & COMM_TRANSPARENT) { // the real client/dest IP address must be already available via getsockname()
+        details->flags |= COMM_TRANSPARENT;
+        if (!Ip::Interceptor.TransparentActive()) {
+            debugs(50, DBG_IMPORTANT, "ERROR: Cannot use transparent " << details << " because TPROXY mode became inactive");
+            // TODO: consider returning Comm::COMM_ERROR instead
+            return Comm::NOMESSAGE;
+        }
+    } else if (conn->flags & COMM_INTERCEPTION) { // request the real client/dest IP address from NAT
+        details->flags |= COMM_INTERCEPTION;
+        if (!Ip::Interceptor.LookupNat(*details)) {
+            debugs(50, DBG_IMPORTANT, "ERROR: NAT lookup failed to locate original IPs on " << details);
+            return Comm::NOMESSAGE;
+        }
     }
 
 #if USE_SQUID_EUI

diff --git a/src/ip/Intercept.cc b/src/ip/Intercept.cc
index 160978f4af92f5b692b770924fa35b8739718eea..08b00faf28ba29ab98e21824cdcaece688d0fbdc 100644 (file)
--- a/src/ip/Intercept.cc
+++ b/src/ip/Intercept.cc
@@ -119,15 +119,6 @@ Ip::Intercept::StopTransparency(const char *str)
     }
 }
 
-void
-Ip::Intercept::StopInterception(const char *str)
-{
-    if (interceptActive_) {
-        debugs(89, DBG_IMPORTANT, "Stopping IP interception: " << str);
-        interceptActive_ = 0;
-    }
-}
-
 bool
 Ip::Intercept::NetfilterInterception(const Comm::ConnectionPointer &newConn)
 {
@@ -157,21 +148,32 @@ Ip::Intercept::NetfilterInterception(const Comm::ConnectionPointer &newConn)
     return false;
 }
 
-bool
-Ip::Intercept::TproxyTransparent(const Comm::ConnectionPointer &newConn)
+void
+Ip::Intercept::StartTransparency()
 {
+    // --enable-linux-netfilter
+    // --enable-pf-transparent
+    // --enable-ipfw-transparent
 #if (LINUX_NETFILTER && defined(IP_TRANSPARENT)) || \
     (PF_TRANSPARENT && defined(SO_BINDANY)) || \
     (IPFW_TRANSPARENT && defined(IP_BINDANY))
+    transparentActive_ = 1;
+#else
+    throw TextException("requires TPROXY feature to be enabled by ./configure", Here());
+#endif
+}
 
-    /* Trust the user configured properly. If not no harm done.
-     * We will simply attempt a bind outgoing on our own IP.
-     */
-    debugs(89, 5, "address TPROXY: " << newConn);
-    return true;
+void
+Ip::Intercept::StartInterception()
+{
+    // --enable-linux-netfilter
+    // --enable-ipfw-transparent
+    // --enable-ipf-transparent
+    // --enable-pf-transparent
+#if IPF_TRANSPARENT || LINUX_NETFILTER || IPFW_TRANSPARENT || PF_TRANSPARENT
+    interceptActive_ = 1;
 #else
-    (void)newConn;
-    return false;
+    throw TextException("requires NAT Interception feature to be enabled by ./configure", Here());
 #endif
 }
 
@@ -377,40 +379,14 @@ Ip::Intercept::PfInterception(const Comm::ConnectionPointer &newConn)
 }
 
 bool
-Ip::Intercept::Lookup(const Comm::ConnectionPointer &newConn, const Comm::ConnectionPointer &listenConn)
+Ip::Intercept::LookupNat(const Comm::Connection &aConn)
 {
-    /* --enable-linux-netfilter    */
-    /* --enable-ipfw-transparent   */
-    /* --enable-ipf-transparent    */
-    /* --enable-pf-transparent     */
-#if IPF_TRANSPARENT || LINUX_NETFILTER || IPFW_TRANSPARENT || PF_TRANSPARENT
-
-    debugs(89, 5, "address BEGIN: me/client= " << newConn->local << ", destination/me= " << newConn->remote);
-
-    newConn->flags |= (listenConn->flags & (COMM_TRANSPARENT|COMM_INTERCEPTION));
-
-    /* NP: try TPROXY first, its much quieter than NAT when non-matching */
-    if (transparentActive_ && listenConn->flags&COMM_TRANSPARENT) {
-        if (TproxyTransparent(newConn)) return true;
-    }
+    debugs(89, 5, "address BEGIN: me/client= " << aConn.local << ", destination/me= " << aConn.remote);
+    assert(interceptActive_);
 
-    if (interceptActive_ && listenConn->flags&COMM_INTERCEPTION) {
-        /* NAT methods that use sock-opts to return client address */
-        if (NetfilterInterception(newConn)) return true;
-        if (IpfwInterception(newConn)) return true;
-
-        /* NAT methods that use ioctl to return client address AND destination address */
-        if (PfInterception(newConn)) return true;
-        if (IpfInterception(newConn)) return true;
-    }
-
-#else /* none of the transparent options configured */
-    (void)newConn;
-    (void)listenConn;
-    debugs(89, DBG_IMPORTANT, "WARNING: transparent proxying not supported");
-#endif
-
-    return false;
+    Comm::ConnectionPointer newConn = &aConn;
+    return NetfilterInterception(newConn) || IpfwInterception(newConn) || // use sock-opts to return client address
+           PfInterception(newConn) || IpfInterception(newConn); // use ioctl to return client address AND destination address
 }
 
 bool

diff --git a/src/ip/Intercept.h b/src/ip/Intercept.h
index 807b3642671c9bb04e0aededdef6d6a10f7ccf1d..7005487d219369ad3009a9bea1d840640cdf4991 100644 (file)
--- a/src/ip/Intercept.h
+++ b/src/ip/Intercept.h
@@ -30,8 +30,8 @@ public:
     Intercept() : transparentActive_(0), interceptActive_(0) {}
     ~Intercept() {};
 
-    /** Perform NAT lookups */
-    bool Lookup(const Comm::ConnectionPointer &newConn, const Comm::ConnectionPointer &listenConn);
+    /// perform NAT lookups for the local address of the given connection
+    bool LookupNat(const Comm::Connection &);
 
     /**
      * Test system networking calls for TPROXY support.
@@ -55,7 +55,7 @@ public:
      * This function should be called during parsing of the squid.conf
      * When any option requiring full-transparency is encountered.
      */
-    inline void StartTransparency() { transparentActive_=1; };
+    void StartTransparency();
 
     /** \par
      * Turn off fully Transparent-Proxy activities on all new connections.
@@ -76,27 +76,10 @@ public:
      * This function should be called during parsing of the squid.conf
      * When any option requiring interception / NAT handling is encountered.
      */
-    inline void StartInterception() { interceptActive_=1; };
-
-    /** \par
-     * Turn off IP-Interception-Proxy activities on all new connections.
-     * Existing transactions and connections are unaffected and will run
-     * to their natural completion.
-     \param str    Reason for stopping. Will be logged to cache.log
-     */
-    inline void StopInterception(const char *str);
+    void StartInterception();
 
 private:
 
-    /**
-     * perform Lookups on fully-transparent interception targets (TPROXY).
-     * Supports Netfilter, PF and IPFW.
-     *
-     * \param newConn  Details known, to be updated where relevant.
-     * \return         Whether successfully located the new address.
-     */
-    bool TproxyTransparent(const Comm::ConnectionPointer &newConn);
-
     /**
      * perform Lookups on Netfilter interception targets (REDIRECT, DNAT).
      *