From c35deebe71414463dd65eaad30d41916410648fe Mon Sep 17 00:00:00 2001 From: James Cunningham Date: Mon, 6 Jun 2016 10:45:52 -0700 Subject: [PATCH 1/3] Add a period to the IP checker to ensure it only finds IPv4. --- src/sentry/middleware/proxy.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sentry/middleware/proxy.py b/src/sentry/middleware/proxy.py index 7a598d64e950df..6bb57e44bf7313 100644 --- a/src/sentry/middleware/proxy.py +++ b/src/sentry/middleware/proxy.py @@ -11,7 +11,8 @@ def process_request(self, request): # HTTP_X_FORWARDED_FOR can be a comma-separated list of IPs. # Take just the first one. real_ip = real_ip.split(",")[0] - if ':' in real_ip: + if '.' in real_ip and ':' in real_ip: + # Strip the port number off of an IPv4 FORWARDED_FOR entry. real_ip = real_ip.split(':', 1)[0] request.META['REMOTE_ADDR'] = real_ip From 09117211bc1a2e518334ecf0708beab8395f40f8 Mon Sep 17 00:00:00 2001 From: James Cunningham Date: Mon, 6 Jun 2016 10:46:07 -0700 Subject: [PATCH 2/3] Add a test case for both IP versions. --- tests/sentry/middleware/test_proxy.py | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/sentry/middleware/test_proxy.py b/tests/sentry/middleware/test_proxy.py index b22f517ad73461..e2a68ff902c61b 100644 --- a/tests/sentry/middleware/test_proxy.py +++ b/tests/sentry/middleware/test_proxy.py @@ -1,10 +1,11 @@ from __future__ import absolute_import from exam import fixture -from django.http import HttpResponse, StreamingHttpResponse +from django.http import HttpRequest, HttpResponse, StreamingHttpResponse from sentry.testutils import TestCase -from sentry.middleware.proxy import ContentLengthHeaderMiddleware +from sentry.middleware.proxy import ( + ContentLengthHeaderMiddleware, SetRemoteAddrFromForwardedFor) class ContentLengthHeaderMiddlewareTest(TestCase): @@ -19,3 +20,19 @@ def test_streaming(self): response = self.middleware.process_response(None, StreamingHttpResponse()) assert 'Transfer-Encoding' not in response assert 'Content-Length' not in response + + +class SetRemoteAddrFromForwardedForTestCase(TestCase): + middleware = fixture(SetRemoteAddrFromForwardedFor) + + def test_ipv4(self): + request = HttpRequest() + request.META['HTTP_X_FORWARDED_FOR'] = '8.8.8.8:80,8.8.4.4' + self.middleware.process_request(request) + assert request.META['REMOTE_ADDR'] == '8.8.8.8' + + def test_ipv6(self): + request = HttpRequest() + request.META['HTTP_X_FORWARDED_FOR'] = '2001:4860:4860::8888,2001:4860:4860::8844' + self.middleware.process_request(request) + assert request.META['REMOTE_ADDR'] == '2001:4860:4860::8888' From d5c21a9244091ba04f864bb35e7d3a135af5fd3c Mon Sep 17 00:00:00 2001 From: James Cunningham Date: Mon, 6 Jun 2016 10:58:46 -0700 Subject: [PATCH 3/3] IPv4 is still the most popular but it's 2016. :sadface: --- src/sentry/middleware/proxy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/middleware/proxy.py b/src/sentry/middleware/proxy.py index 6bb57e44bf7313..f16a1503e903a5 100644 --- a/src/sentry/middleware/proxy.py +++ b/src/sentry/middleware/proxy.py @@ -11,7 +11,7 @@ def process_request(self, request): # HTTP_X_FORWARDED_FOR can be a comma-separated list of IPs. # Take just the first one. real_ip = real_ip.split(",")[0] - if '.' in real_ip and ':' in real_ip: + if ':' in real_ip and '.' in real_ip: # Strip the port number off of an IPv4 FORWARDED_FOR entry. real_ip = real_ip.split(':', 1)[0] request.META['REMOTE_ADDR'] = real_ip