Collector #2321: skip trusted proxies when extracting the client ip from X-Forwarded-For

doc/CHANGES.txt

@@ -8,6 +8,9 @@ Zope Changes
 
     Bugs fixed
 
+      - Collector #2321: Skip trusted proxies when extracting the client IP
+        address from the request.
+
       - Collector #2318: Allow override of zopectl's control socket in
         zope.conf
 

lib/python/ZPublisher/HTTPRequest.py

@@ -272,7 +272,15 @@ class HTTPRequest(BaseRequest):
             if environ.has_key('HTTP_X_FORWARDED_FOR') and self._client_addr in trusted_proxies:
                 # REMOTE_ADDR is one of our trusted local proxies. Not really very remote at all.
                 # The proxy can tell us the IP of the real remote client in the forwarded-for header
-                self._client_addr = environ['HTTP_X_FORWARDED_FOR'].split(',')[-1].strip()
+                # Skip the proxy-address itself though
+                forwarded_for = [
+                    e.strip()
+                    for e in environ['HTTP_X_FORWARDED_FOR'].split(',')]
+                forwarded_for.reverse()
+                for entry in forwarded_for:
+                    if entry not in trusted_proxies:
+                        self._client_addr = entry
+                        break
         else:
             self._client_addr = ''
 

lib/python/ZPublisher/tests/testHTTPRequest.py

@@ -740,6 +740,38 @@ class RequestTests( unittest.TestCase ):
         self.assertEqual(getDebug(request), '1')
         self.assert_(isinstance(getDebugFromZope3(request), DebugFlags))
 
+    def testTrustedProxies(self):
+        TEST_ENVIRON = {
+            'REQUEST_METHOD': 'GET',
+            'SERVER_NAME': 'localhost',
+            'SERVER_PORT': '80',
+            'REMOTE_ADDR': '127.0.0.1',
+            'HTTP_X_FORWARDED_FOR': '10.1.20.30, 192.168.1.100',
+            }
+        from StringIO import StringIO
+        from ZPublisher.HTTPRequest import HTTPRequest, trusted_proxies
+        s = StringIO('')
+
+        env = TEST_ENVIRON.copy()
+        request = HTTPRequest(s, env, None)
+        self.assertEqual(request.getClientAddr(), '127.0.0.1')
+
+        trusted_proxies.append('127.0.0.1')
+        request = HTTPRequest(s, env, None)
+        self.assertEqual(request.getClientAddr(), '192.168.1.100')
+
+        trusted_proxies[0] = '192.168.1.100' 
+        env = TEST_ENVIRON.copy()
+        env['REMOTE_ADDR'] = '192.168.1.100'
+        request = HTTPRequest(s, env, None)
+        self.assertEqual(request.getClientAddr(), '10.1.20.30')
+
+        env = TEST_ENVIRON.copy()
+        del env['REMOTE_ADDR']
+        request = HTTPRequest(s, env, None)
+        self.assertEqual(request.getClientAddr(), '')
+
+
 def test_suite():
     suite = unittest.TestSuite()
     suite.addTest(unittest.makeSuite(AuthCredentialsTestsa, 'test'))