From 88bdd20d605e311b19420f27e2e63a54be81ee15 Mon Sep 17 00:00:00 2001 From: marysieek Date: Sun, 17 May 2020 16:18:26 +0200 Subject: [PATCH 1/6] Update readme.rst --- README.rst | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/README.rst b/README.rst index 4565224..1591c2e 100644 --- a/README.rst +++ b/README.rst @@ -44,15 +44,32 @@ import and configure the library with your Castle API secret. configuration.blacklisted = ['HTTP-X-header'] # Castle needs the original IP of the client, not the IP of your proxy or load balancer. - # we try to fetch proper ip based on X-Forwarded-For or Remote-Addr headers in that order - # but sometimes proper ip may be stored in different header or order could be different. - # SDK can extract ip automatically for you, but you must configure which ip_headers you would like to use + # The SDK will only trust the proxy chain as defined in the configuration. + # We try to fetch the client IP based on X-Forwarded-For or Remote-Addr headers in that order, + # but sometimes the client IP may be stored in a different header or order. + # The SDK can be configured to look for the client IP address in headers that you specify. + # If the specified header or X-Forwarded-For default contains a proxy chain with public IP addresses, + # then one of the following must be set + # 1. The trusted_proxies value must match the known proxy IP's + # 2. The trusted_proxy_depth value must be set to the number of known trusted proxies in the chain (see below) configuration.ip_headers = [] + # Additionally to make X-Forwarded-For and other headers work better discovering client ip address, # and not the address of a reverse proxy server, you can define trusted proxies # which will help to fetch proper ip from those headers + + # In order to extract the client IP of the X-Forwarded-For header + # and not the address of a reverse proxy server, you must define all trusted public proxies + # you can achieve this by listing all the proxies ip defined by string or regular expressions + # in trusted_proxies setting configuration.trusted_proxies = [] - # *Note: proxies list can be provided as an array of regular expressions + # or by providing number of trusted proxies used in the chain + configuration.trusted_proxy_depth = 0 + + # If there is no possibility to define options above and there is no other header which can have client ip + # then you may set trust_proxy_chain = true to trust all of the proxy IP's in X-Forwarded-For + configuration.trust_proxy_chain = false + # *Note: default always marked as trusty list is here: Castle::Configuration::TRUSTED_PROXIES Tracking From 33911a2b774e324a570448bd7ca6f81c602c9287 Mon Sep 17 00:00:00 2001 From: marysieek Date: Sun, 17 May 2020 16:55:14 +0200 Subject: [PATCH 2/6] Add new settings to configuration.py --- castle/configuration.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/castle/configuration.py b/castle/configuration.py index adb3ecd..7b864e6 100644 --- a/castle/configuration.py +++ b/castle/configuration.py @@ -50,6 +50,8 @@ def __init__(self): self.failover_strategy = FAILOVER_STRATEGY self.ip_headers = [] self.trusted_proxies = [] + self.trust_proxy_chain = False + self.trusted_proxy_depth = None def isValid(self): return self.host and self.port and self.api_secret @@ -149,6 +151,27 @@ def trusted_proxies(self, value): else: raise ConfigurationError + @property + def trust_proxy_chain(self): + return self.__trust_proxy_chain + + @trust_proxy_chain.setter + def trust_proxy_chain(self, value): + if isinstance(value, bool): + self.__trust_proxy_chain = value + else: + raise ConfigurationError + + @property + def trusted_proxies_depth(self): + return self.__trusted_proxies_depth + + @trusted_proxies_depth.setter + def trusted_proxies_depth(self, value): + if isinstance(value, (int, type(None))): + self.__trusted_proxies_depth = int(0 if value is None else value) + else: + raise ConfigurationError # pylint: disable=invalid-name configuration = Configuration() From b7d758cb6576f83ef3fa780a19fe32f48a0e6902 Mon Sep 17 00:00:00 2001 From: marysieek Date: Sun, 17 May 2020 17:42:01 +0200 Subject: [PATCH 3/6] Fix naming --- castle/configuration.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/castle/configuration.py b/castle/configuration.py index 7b864e6..b857c1c 100644 --- a/castle/configuration.py +++ b/castle/configuration.py @@ -163,13 +163,13 @@ def trust_proxy_chain(self, value): raise ConfigurationError @property - def trusted_proxies_depth(self): - return self.__trusted_proxies_depth + def trusted_proxy_depth(self): + return self.__trusted_proxy_depth - @trusted_proxies_depth.setter - def trusted_proxies_depth(self, value): + @trusted_proxy_depth.setter + def trusted_proxy_depth(self, value): if isinstance(value, (int, type(None))): - self.__trusted_proxies_depth = int(0 if value is None else value) + self.__trusted_proxy_depth = int(0 if value is None else value) else: raise ConfigurationError From 20aff6cf8f2dbfbc6beef8ea16a4b95293fc3a89 Mon Sep 17 00:00:00 2001 From: marysieek Date: Sun, 17 May 2020 18:13:06 +0200 Subject: [PATCH 4/6] Update ip extractor --- castle/extractors/ip.py | 36 +++++++++++++++++++------------ castle/test/extractors/ip_test.py | 1 - 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/castle/extractors/ip.py b/castle/extractors/ip.py index b6e6738..038e72a 100644 --- a/castle/extractors/ip.py +++ b/castle/extractors/ip.py @@ -4,7 +4,8 @@ # ordered list of ip headers for ip extraction DEFAULT = ['X-Forwarded-For', 'Remote-Addr'] - +# list of header which are used with proxy depth setting +DEPTH_RELATED = ['X-Forwarded-For'] class ExtractorsIp(object): def __init__(self, headers): @@ -14,32 +15,31 @@ def __init__(self, headers): else: self.ip_headers = DEFAULT self.proxies = configuration.trusted_proxies + TRUSTED_PROXIES + self.trust_proxy_chain = configuration.trust_proxy_chain + self.trusted_proxy_depth = configuration.trusted_proxy_depth def call(self): all_ips = [] for ip_header in self.ip_headers: ips = self._ips_from(ip_header) - filtered_ips = self._remove_proxies(ips) - - if len(filtered_ips) > 0: - return filtered_ips[-1] - - all_ips = all_ips + ips + ip_value = self._remove_proxies(ips) + if ip_value: + return ip_value + all_ips += ips - # fallback to first whatever ip + # fallback to first listed ip if len(all_ips) > 0: return all_ips[0] return None def _remove_proxies(self, ips): - result = [] + if self.trust_proxy_chain: + return next(iter(ips), None) - for ip_address in ips: - if not self._is_proxy(ip_address): - result.append(ip_address) - return result + result = [ip_address for ip_address in ips if not self._is_proxy(ip_address)] + return (result or [None])[-1] def _is_proxy(self, ip_address): for proxy_re in self.proxies: @@ -54,4 +54,12 @@ def _ips_from(self, header): if not value: return [] - return re.split(r'[,\s]+', value.strip()) + ips = re.split(r'[,\s]+', value.strip()) + + return self._limit_proxy_depth(ips, header) + + def _limit_proxy_depth(self, ips, ip_header): + if ip_header in DEPTH_RELATED: + ips = ips[:len(ips)-self.trusted_proxy_depth] + + return ips diff --git a/castle/test/extractors/ip_test.py b/castle/test/extractors/ip_test.py index bc39f81..0c3f896 100644 --- a/castle/test/extractors/ip_test.py +++ b/castle/test/extractors/ip_test.py @@ -44,7 +44,6 @@ def test_extract_ip_for_spoof_ip_attempt(self): ExtractorsIp(headers).call(), '2.2.2.3' ) -# def test_extract_ip_for_spoof_ip_attempt_when_all_trusted_proxies(self): headers = {'Client-Ip': '6.6.6.6', 'X-Forwarded-For': '6.6.6.6, 2.2.2.3, 192.168.0.7'} From 1b534bfe0a444f2bc909f74d36d7793a40cad00c Mon Sep 17 00:00:00 2001 From: marysieek Date: Sun, 17 May 2020 18:32:17 +0200 Subject: [PATCH 5/6] Add corresponding test cases --- castle/test/extractors/ip_test.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/castle/test/extractors/ip_test.py b/castle/test/extractors/ip_test.py index 0c3f896..9c9f9b6 100644 --- a/castle/test/extractors/ip_test.py +++ b/castle/test/extractors/ip_test.py @@ -7,6 +7,8 @@ class ExtractorsIpTestCase(unittest.TestCase): def tearDown(self): configuration.ip_headers = [] configuration.trusted_proxies = [] + configuration.trust_proxy_chain = False + configuration.trusted_proxy_depth = None def test_extract_ip(self): headers = {'X-Forwarded-For': '1.2.3.5'} @@ -38,6 +40,28 @@ def test_extract_ip_when_all_trusted_proxies(self): '127.0.0.1' ) + def test_extract_ip_when_trust_proxy_chain(self): + xf_header = """ + 6.6.6.6,2.2.2.3,6.6.6.5 + """ + headers = {'Remote-Addr': '6.6.6.4', 'X-Forwarded-For': xf_header} + configuration.trust_proxy_chain = True + self.assertEqual( + ExtractorsIp(headers).call(), + '6.6.6.6' + ) + + def test_extract_ip_when_trust_proxy_depth(self): + xf_header = """ + 6.6.6.6,2.2.2.3,6.6.6.5 + """ + headers = {'Remote-Addr': '6.6.6.4', 'X-Forwarded-For': xf_header} + configuration.trusted_proxy_depth = 1 + self.assertEqual( + ExtractorsIp(headers).call(), + '2.2.2.3' + ) + def test_extract_ip_for_spoof_ip_attempt(self): headers = {'Client-Ip': '6.6.6.6', 'X-Forwarded-For': '6.6.6.6, 2.2.2.3, 192.168.0.7'} self.assertEqual( From 67bfb01b3fd8e75aabe74dbb1a5b32964695071f Mon Sep 17 00:00:00 2001 From: marysieek Date: Mon, 18 May 2020 10:07:20 +0200 Subject: [PATCH 6/6] Return next item or none --- castle/extractors/ip.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/castle/extractors/ip.py b/castle/extractors/ip.py index 038e72a..d2af4f6 100644 --- a/castle/extractors/ip.py +++ b/castle/extractors/ip.py @@ -28,11 +28,7 @@ def call(self): return ip_value all_ips += ips - # fallback to first listed ip - if len(all_ips) > 0: - return all_ips[0] - - return None + return next(iter(all_ips), None) def _remove_proxies(self, ips): if self.trust_proxy_chain: