Fix long standing bug in determining remote IP

The check was for is_behind_cache without the (), meaning it always
returned true, which in turn meant we trusted all x-forwarded-for
headers. It was pretty hard to get them into the system, and
we didn't actually use it for anything other than locking survey
submissions, so it's not a big problem.

However, the basic logic was also wrong, as it assumes that all
SSL connections terminate directly at the backend server, which is
not necessarily true anymore.

The new version of the function will trust an X-Forwarded-For as
long as it's set on one of our frontend servers, regardless of if
it's an encrypted connection or not.

1 files changed, 8 insertions, 13 deletions

diff --git a/pgweb/util/misc.py b/pgweb/util/misc.py
index 46de7a42..321b4748 100644
--- a/pgweb/util/misc.py
+++ b/pgweb/util/misc.py
@@ -38,22 +38,17 @@ def is_behind_cache(request):
 def get_client_ip(request):
 	"""
 	Get the IP of the client. If the client is served through our Varnish caches,
-	make sure we get the *actual* client IP, and not the IP of the Varnish cache.
+	or behind one of our SSL proxies, make sure to get the *actual* client IP,
+	and not the IP of the cache/proxy.
 	"""
-	if is_behind_cache:
-		# When we are served behind a cache, our varnish is (should) always be configured
-		# to set the X-Forwarded-For header. It will also remove any previous such header,
-		# so we can always trust that header if it's there.
-		try:
+	if request.META.has_key('HTTP_X_FORWARDED_FOR'):
+		# There is a x-forwarded-for header, so trust it but only if the actual connection
+		# is coming in from one of our frontends.
+		if request.META['REMOTE_ADDR'] in settings.FRONTEND_SERVERS:
 			return request.META['HTTP_X_FORWARDED_FOR']
-		except:
-			# In case something failed, we'll return the remote address. This is most likely
-			# the varnish server itself, but it's better than aborting the request.
-			return request.META['REMOTE_ADDR']
-	else:
-		return request.META['REMOTE_ADDR']
-
 
+	# Else fall back and return the actual IP of the connection
+	return request.META['REMOTE_ADDR']
 
 
 def varnish_purge(url):