From cd5fb034715103ace40c08557ec0dd079e4f15bf Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 6 Aug 2019 20:25:24 +0200 Subject: [PATCH 1/5] Add support for blacklisting hosts to the HTTP runner by adding new "hosts_blacklist" runner attribute. --- .../http_runner/http_runner/http_runner.py | 38 ++++++++++++- .../http_runner/http_runner/runner.yaml | 7 +++ .../tests/unit/test_http_runner.py | 57 +++++++++++++++++++ 3 files changed, 100 insertions(+), 2 deletions(-) diff --git a/contrib/runners/http_runner/http_runner/http_runner.py b/contrib/runners/http_runner/http_runner/http_runner.py index 6915840814..3fc5abec5a 100644 --- a/contrib/runners/http_runner/http_runner/http_runner.py +++ b/contrib/runners/http_runner/http_runner/http_runner.py @@ -13,6 +13,7 @@ # limitations under the License. from __future__ import absolute_import + import ast import copy import json @@ -21,6 +22,7 @@ import requests from requests.auth import HTTPBasicAuth from oslo_config import cfg +from six.moves.urllib import parse as urlparse # pylint: disable=import-error from st2common.runners.base import ActionRunner from st2common.runners.base import get_metadata as get_runner_metadata @@ -55,6 +57,7 @@ RUNNER_VERIFY_SSL_CERT = 'verify_ssl_cert' RUNNER_USERNAME = 'username' RUNNER_PASSWORD = 'password' +RUNNER_HOSTS_BLACKLIST = 'hosts_blacklist' # Lookup constants for action params ACTION_AUTH = 'auth' @@ -93,6 +96,7 @@ def pre_run(self): self._http_proxy = self.runner_parameters.get(RUNNER_HTTP_PROXY, None) self._https_proxy = self.runner_parameters.get(RUNNER_HTTPS_PROXY, None) self._verify_ssl_cert = self.runner_parameters.get(RUNNER_VERIFY_SSL_CERT, None) + self._hosts_blacklist = self.runner_parameters.get(RUNNER_HOSTS_BLACKLIST, []) def run(self, action_parameters): client = self._get_http_client(action_parameters) @@ -147,7 +151,8 @@ def _get_http_client(self, action_parameters): headers=headers, cookies=self._cookies, auth=auth, timeout=timeout, allow_redirects=self._allow_redirects, proxies=proxies, files=files, verify=self._verify_ssl_cert, - username=self._username, password=self._password) + username=self._username, password=self._password, + hosts_blacklist=self._hosts_blacklist) @staticmethod def _get_result_status(status_code): @@ -158,7 +163,8 @@ def _get_result_status(status_code): class HTTPClient(object): def __init__(self, url=None, method=None, body='', params=None, headers=None, cookies=None, auth=None, timeout=60, allow_redirects=False, proxies=None, - files=None, verify=False, username=None, password=None): + files=None, verify=False, username=None, password=None, + hosts_blacklist=None): if url is None: raise Exception('URL must be specified.') @@ -188,12 +194,19 @@ def __init__(self, url=None, method=None, body='', params=None, headers=None, co self.verify = verify self.username = username self.password = password + self.hosts_blacklist = hosts_blacklist or [] def run(self): results = {} resp = None json_content = self._is_json_content() + # Check if the provided URL is blacklisted + is_url_blacklisted = self._is_url_blacklisted(url=self.url) + + if is_url_blacklisted: + raise ValueError('URL "%s" is blacklisted' % (self.url)) + try: if json_content: # cast params (body) to dict @@ -301,6 +314,27 @@ def _cast_object(self, value): else: return value + def _is_url_blacklisted(self, url): + """ + Verify if the provided URL is blacklisted via hosts_blacklist runner parameter. + """ + if not self.hosts_blacklist: + # Blacklist is empty + return False + + parsed = urlparse.urlparse(url) + + # Remove the port and [] + host = parsed.netloc.replace('[', '').replace(']', '') + + if parsed.port is not None: + host = host.replace(':%s' % (parsed.port), '') + + if host in self.hosts_blacklist: + return True + + return False + def get_runner(): return HttpRunner(str(uuid.uuid4())) diff --git a/contrib/runners/http_runner/http_runner/runner.yaml b/contrib/runners/http_runner/http_runner/runner.yaml index 85f1e4c1c5..a194fd2be0 100644 --- a/contrib/runners/http_runner/http_runner/runner.yaml +++ b/contrib/runners/http_runner/http_runner/runner.yaml @@ -36,6 +36,13 @@ CA bundle which comes from Mozilla. Verification using a custom CA bundle is not yet supported. Set to False to skip verification. type: boolean + hosts_blacklist: + description: Optional list of hosts (network locations) to blacklist (e.g. example.com, + 127.0.0.1, etc.) + required: false + type: array + items: + type: string output_key: body output_schema: status_code: diff --git a/contrib/runners/http_runner/tests/unit/test_http_runner.py b/contrib/runners/http_runner/tests/unit/test_http_runner.py index 7f02d987d1..9dc25d8c74 100644 --- a/contrib/runners/http_runner/tests/unit/test_http_runner.py +++ b/contrib/runners/http_runner/tests/unit/test_http_runner.py @@ -15,6 +15,8 @@ from __future__ import absolute_import +import re + import six import mock import unittest2 @@ -212,3 +214,58 @@ def test_http_unicode_body_data(self, mock_requests): expected_data = body self.assertEqual(call_kwargs['data'], expected_data) + + @mock.patch('http_runner.http_runner.requests') + def test_blacklisted_url_netloc(self, mock_requests): + # Black list is empty + self.assertEqual(mock_requests.request.call_count, 0) + + url = 'http://www.example.com' + client = HTTPClient(url=url, method='GET') + client.run() + + self.assertEqual(mock_requests.request.call_count, 1) + + # Blacklist is set + hosts_blacklist = [ + 'example.com', + '127.0.0.1', + '::1', + '2001:0db8:85a3:0000:0000:8a2e:0370:7334' + ] + + # Blacklisted urls + urls = [ + 'https://example.com', + 'http://example.com', + 'http://example.com:81', + 'http://example.com:80', + 'http://example.com:9000', + 'http://[::1]:80/', + 'http://[::1]', + 'http://[::1]:9000', + 'http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]', + 'https://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:8000' + ] + + for url in urls: + expected_msg = r'URL "%s" is blacklisted' % (re.escape(url)) + client = HTTPClient(url=url, method='GET', hosts_blacklist=hosts_blacklist) + self.assertRaisesRegexp(ValueError, expected_msg, client.run) + + # Non blacklisted URLs + urls = [ + 'https://example2.com', + 'http://example3.com', + 'http://example4.com:81' + ] + + for url in urls: + mock_requests.request.reset_mock() + + self.assertEqual(mock_requests.request.call_count, 0) + + client = HTTPClient(url=url, method='GET') + client.run() + + self.assertEqual(mock_requests.request.call_count, 1) From 07a5fac5db6cd5d0f5f26ae6e48634084bbf059e Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 7 Aug 2019 09:44:51 +0200 Subject: [PATCH 2/5] Rename existing runner parameter from "hosts_blacklist" to "url_hosts_blacklist" and also add support for whitelist approach using "url_hosts_whitelist" runner parameter. --- .../http_runner/http_runner/http_runner.py | 60 ++++++++++++++---- .../http_runner/http_runner/runner.yaml | 13 +++- .../tests/unit/test_http_runner.py | 63 +++++++++++++++++-- 3 files changed, 118 insertions(+), 18 deletions(-) diff --git a/contrib/runners/http_runner/http_runner/http_runner.py b/contrib/runners/http_runner/http_runner/http_runner.py index 3fc5abec5a..b37e92c60e 100644 --- a/contrib/runners/http_runner/http_runner/http_runner.py +++ b/contrib/runners/http_runner/http_runner/http_runner.py @@ -57,7 +57,8 @@ RUNNER_VERIFY_SSL_CERT = 'verify_ssl_cert' RUNNER_USERNAME = 'username' RUNNER_PASSWORD = 'password' -RUNNER_HOSTS_BLACKLIST = 'hosts_blacklist' +RUNNER_URL_HOSTS_BLACKLIST = 'url_hosts_blacklist' +RUNNER_URL_HOSTS_WHITELIST = 'url_hosts_whitelist' # Lookup constants for action params ACTION_AUTH = 'auth' @@ -96,11 +97,17 @@ def pre_run(self): self._http_proxy = self.runner_parameters.get(RUNNER_HTTP_PROXY, None) self._https_proxy = self.runner_parameters.get(RUNNER_HTTPS_PROXY, None) self._verify_ssl_cert = self.runner_parameters.get(RUNNER_VERIFY_SSL_CERT, None) - self._hosts_blacklist = self.runner_parameters.get(RUNNER_HOSTS_BLACKLIST, []) + self._url_hosts_blacklist = self.runner_parameters.get(RUNNER_URL_HOSTS_BLACKLIST, []) + self._url_hosts_whitelist = self.runner_parameters.get(RUNNER_URL_HOSTS_WHITELIST, []) def run(self, action_parameters): client = self._get_http_client(action_parameters) + if self._url_hosts_blacklist and self._url_hosts_whitelist: + msg = ('"url_hosts_blacklist" and "url_hosts_whitelist" parameters are mutually ' + 'exclusive. Only one should be provided.') + raise ValueError(msg) + try: result = client.run() except requests.exceptions.Timeout as e: @@ -152,7 +159,8 @@ def _get_http_client(self, action_parameters): timeout=timeout, allow_redirects=self._allow_redirects, proxies=proxies, files=files, verify=self._verify_ssl_cert, username=self._username, password=self._password, - hosts_blacklist=self._hosts_blacklist) + url_hosts_blacklist=self._url_hosts_blacklist, + url_hosts_whitelist=self._url_hosts_whitelist) @staticmethod def _get_result_status(status_code): @@ -164,7 +172,7 @@ class HTTPClient(object): def __init__(self, url=None, method=None, body='', params=None, headers=None, cookies=None, auth=None, timeout=60, allow_redirects=False, proxies=None, files=None, verify=False, username=None, password=None, - hosts_blacklist=None): + url_hosts_blacklist=None, url_hosts_whitelist=None): if url is None: raise Exception('URL must be specified.') @@ -194,7 +202,8 @@ def __init__(self, url=None, method=None, body='', params=None, headers=None, co self.verify = verify self.username = username self.password = password - self.hosts_blacklist = hosts_blacklist or [] + self.url_hosts_blacklist = url_hosts_blacklist or [] + self.url_hosts_whitelist = url_hosts_whitelist or [] def run(self): results = {} @@ -207,6 +216,11 @@ def run(self): if is_url_blacklisted: raise ValueError('URL "%s" is blacklisted' % (self.url)) + is_url_whitelisted = self._is_url_whitelisted(url=self.url) + + if not is_url_whitelisted: + raise ValueError('URL "%s" is not whitelisted' % (self.url)) + try: if json_content: # cast params (body) to dict @@ -316,24 +330,46 @@ def _cast_object(self, value): def _is_url_blacklisted(self, url): """ - Verify if the provided URL is blacklisted via hosts_blacklist runner parameter. + Verify if the provided URL is blacklisted via url_hosts_blacklist runner parameter. """ - if not self.hosts_blacklist: + if not self.url_hosts_blacklist: # Blacklist is empty return False + host = self._get_host_from_url(url=url) + + if host in self.url_hosts_blacklist: + return True + + return False + + def _is_url_whitelisted(self, url): + """ + Verify if the provided URL is whitelisted via url_hosts_whitelist runner parameter. + """ + if not self.url_hosts_whitelist: + return True + + host = self._get_host_from_url(url=url) + + if host in self.url_hosts_whitelist: + return True + + return False + + def _get_host_from_url(self, url): + """ + Return sanitized host (netloc) value from the provided url. + """ parsed = urlparse.urlparse(url) - # Remove the port and [] + # Remove port and [] host = parsed.netloc.replace('[', '').replace(']', '') if parsed.port is not None: host = host.replace(':%s' % (parsed.port), '') - if host in self.hosts_blacklist: - return True - - return False + return host def get_runner(): diff --git a/contrib/runners/http_runner/http_runner/runner.yaml b/contrib/runners/http_runner/http_runner/runner.yaml index a194fd2be0..168f68bbb2 100644 --- a/contrib/runners/http_runner/http_runner/runner.yaml +++ b/contrib/runners/http_runner/http_runner/runner.yaml @@ -36,9 +36,18 @@ CA bundle which comes from Mozilla. Verification using a custom CA bundle is not yet supported. Set to False to skip verification. type: boolean - hosts_blacklist: + url_hosts_blacklist: description: Optional list of hosts (network locations) to blacklist (e.g. example.com, - 127.0.0.1, etc.) + 127.0.0.1, ::1, etc.). If action will try to access that endpoint, an exception will be + thrown and action will be marked as failed. + required: false + type: array + items: + type: string + url_hosts_whitelist: + description: Optional list of hosts (network locations) to whitelist (e.g. example.com, + 127.0.0.1, ::1, etc.). If specified, actions will only be able to hit hosts on this + whitelist. required: false type: array items: diff --git a/contrib/runners/http_runner/tests/unit/test_http_runner.py b/contrib/runners/http_runner/tests/unit/test_http_runner.py index 9dc25d8c74..0cbc9f8c84 100644 --- a/contrib/runners/http_runner/tests/unit/test_http_runner.py +++ b/contrib/runners/http_runner/tests/unit/test_http_runner.py @@ -216,7 +216,7 @@ def test_http_unicode_body_data(self, mock_requests): self.assertEqual(call_kwargs['data'], expected_data) @mock.patch('http_runner.http_runner.requests') - def test_blacklisted_url_netloc(self, mock_requests): + def test_blacklisted_url_url_hosts_blacklist_runner_parameter(self, mock_requests): # Black list is empty self.assertEqual(mock_requests.request.call_count, 0) @@ -227,7 +227,7 @@ def test_blacklisted_url_netloc(self, mock_requests): self.assertEqual(mock_requests.request.call_count, 1) # Blacklist is set - hosts_blacklist = [ + url_hosts_blacklist = [ 'example.com', '127.0.0.1', '::1', @@ -250,7 +250,7 @@ def test_blacklisted_url_netloc(self, mock_requests): for url in urls: expected_msg = r'URL "%s" is blacklisted' % (re.escape(url)) - client = HTTPClient(url=url, method='GET', hosts_blacklist=hosts_blacklist) + client = HTTPClient(url=url, method='GET', url_hosts_blacklist=url_hosts_blacklist) self.assertRaisesRegexp(ValueError, expected_msg, client.run) # Non blacklisted URLs @@ -265,7 +265,62 @@ def test_blacklisted_url_netloc(self, mock_requests): self.assertEqual(mock_requests.request.call_count, 0) - client = HTTPClient(url=url, method='GET') + client = HTTPClient(url=url, method='GET', url_hosts_blacklist=url_hosts_blacklist) + client.run() + + self.assertEqual(mock_requests.request.call_count, 1) + + @mock.patch('http_runner.http_runner.requests') + def test_whitelisted_url_url_hosts_whitelist_runner_parameter(self, mock_requests): + # Whitelist is empty + self.assertEqual(mock_requests.request.call_count, 0) + + url = 'http://www.example.com' + client = HTTPClient(url=url, method='GET') + client.run() + + self.assertEqual(mock_requests.request.call_count, 1) + + # Whitelist is set + url_hosts_whitelist = [ + 'example.com', + '127.0.0.1', + '::1', + '2001:0db8:85a3:0000:0000:8a2e:0370:7334' + ] + + # Non whitelisted urls + urls = [ + 'https://www.google.com', + 'https://www.example2.com', + 'http://127.0.0.2' + ] + + for url in urls: + expected_msg = r'URL "%s" is not whitelisted' % (re.escape(url)) + client = HTTPClient(url=url, method='GET', url_hosts_whitelist=url_hosts_whitelist) + self.assertRaisesRegexp(ValueError, expected_msg, client.run) + + # Whitelisted URLS + urls = [ + 'https://example.com', + 'http://example.com', + 'http://example.com:81', + 'http://example.com:80', + 'http://example.com:9000', + 'http://[::1]:80/', + 'http://[::1]', + 'http://[::1]:9000', + 'http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]', + 'https://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:8000' + ] + + for url in urls: + mock_requests.request.reset_mock() + + self.assertEqual(mock_requests.request.call_count, 0) + + client = HTTPClient(url=url, method='GET', url_hosts_whitelist=url_hosts_whitelist) client.run() self.assertEqual(mock_requests.request.call_count, 1) From c074677c99b969855717fbfce2f5669b03cc1407 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 7 Aug 2019 09:57:51 +0200 Subject: [PATCH 3/5] Add changelog entry. --- CHANGELOG.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index adb08ce0b3..c02b0a25a8 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -4,6 +4,13 @@ Changelog in development -------------- +Added +~~~~~ + +* Add support for blacklisting / whitelisting hosts to the HTTP runner by adding new + ``url_hosts_blacklist`` and ``url_hosts_whitelist`` runner attribute. (new feature) + #4757 + Changed ~~~~~~~ From c07f6d6fc1c316ec3e29696e5e89c1f0bbc09210 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 8 Aug 2019 14:00:46 +0200 Subject: [PATCH 4/5] Add a test case which checks behavior when mutually exclusive arguments are provided. --- contrib/runners/http_runner/http_runner/http_runner.py | 5 +++++ .../runners/http_runner/tests/unit/test_http_runner.py | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/contrib/runners/http_runner/http_runner/http_runner.py b/contrib/runners/http_runner/http_runner/http_runner.py index b37e92c60e..af5edbdf11 100644 --- a/contrib/runners/http_runner/http_runner/http_runner.py +++ b/contrib/runners/http_runner/http_runner/http_runner.py @@ -205,6 +205,11 @@ def __init__(self, url=None, method=None, body='', params=None, headers=None, co self.url_hosts_blacklist = url_hosts_blacklist or [] self.url_hosts_whitelist = url_hosts_whitelist or [] + if self.url_hosts_blacklist and self.url_hosts_whitelist: + msg = ('"url_hosts_blacklist" and "url_hosts_whitelist" parameters are mutually ' + 'exclusive. Only one should be provided.') + raise ValueError(msg) + def run(self): results = {} resp = None diff --git a/contrib/runners/http_runner/tests/unit/test_http_runner.py b/contrib/runners/http_runner/tests/unit/test_http_runner.py index 0cbc9f8c84..7d3f26d48f 100644 --- a/contrib/runners/http_runner/tests/unit/test_http_runner.py +++ b/contrib/runners/http_runner/tests/unit/test_http_runner.py @@ -324,3 +324,11 @@ def test_whitelisted_url_url_hosts_whitelist_runner_parameter(self, mock_request client.run() self.assertEqual(mock_requests.request.call_count, 1) + + def test_url_host_blacklist_and_url_host_blacklist_params_are_mutually_exclusive(self): + url = 'http://www.example.com' + + expected_msg = (r'"url_hosts_blacklist" and "url_hosts_whitelist" parameters are mutually ' + 'exclusive.') + self.assertRaisesRegexp(ValueError, expected_msg, HTTPClient, url=url, method='GET', + url_hosts_blacklist=[url], url_hosts_whitelist=[url]) From 25b43cbae43ddfee5e145089d5b0399e320b0164 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 8 Aug 2019 15:29:46 +0200 Subject: [PATCH 5/5] Add some additional test cases for HttpRunner class itself. Previously we don't had test cases for HTTPClient class, but not the actual runner. --- .../tests/unit/test_http_runner.py | 52 ++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/contrib/runners/http_runner/tests/unit/test_http_runner.py b/contrib/runners/http_runner/tests/unit/test_http_runner.py index 7d3f26d48f..fa12ce27a6 100644 --- a/contrib/runners/http_runner/tests/unit/test_http_runner.py +++ b/contrib/runners/http_runner/tests/unit/test_http_runner.py @@ -21,9 +21,17 @@ import mock import unittest2 +from st2common.constants.action import LIVEACTION_STATUS_SUCCEEDED from http_runner.http_runner import HTTPClient +from http_runner.http_runner import HttpRunner + import st2tests.config as tests_config +__all__ = [ + 'HTTPClientTestCase', + 'HTTPRunnerTestCase' +] + if six.PY2: EXPECTED_DATA = '' @@ -35,7 +43,7 @@ class MockResult(object): close = mock.Mock() -class HTTPRunnerTestCase(unittest2.TestCase): +class HTTPClientTestCase(unittest2.TestCase): @classmethod def setUpClass(cls): tests_config.parse_args() @@ -332,3 +340,45 @@ def test_url_host_blacklist_and_url_host_blacklist_params_are_mutually_exclusive 'exclusive.') self.assertRaisesRegexp(ValueError, expected_msg, HTTPClient, url=url, method='GET', url_hosts_blacklist=[url], url_hosts_whitelist=[url]) + + +class HTTPRunnerTestCase(unittest2.TestCase): + @mock.patch('http_runner.http_runner.requests') + def test_get_success(self, mock_requests): + mock_result = MockResult() + + # Unknown content type, body should be returned raw + mock_result.text = 'foo bar ponies' + mock_result.headers = {'Content-Type': 'text/html'} + mock_result.status_code = 200 + + mock_requests.request.return_value = mock_result + + runner_parameters = { + 'url': 'http://www.example.com', + 'method': 'GET' + } + runner = HttpRunner('id') + runner.runner_parameters = runner_parameters + runner.pre_run() + + status, result, _ = runner.run({}) + self.assertEqual(status, LIVEACTION_STATUS_SUCCEEDED) + self.assertEqual(result['body'], 'foo bar ponies') + self.assertEqual(result['status_code'], 200) + self.assertEqual(result['parsed'], False) + + def test_url_host_blacklist_and_url_host_blacklist_params_are_mutually_exclusive(self): + runner_parameters = { + 'url': 'http://www.example.com', + 'method': 'GET', + 'url_hosts_blacklist': ['http://127.0.0.1'], + 'url_hosts_whitelist': ['http://127.0.0.1'], + } + runner = HttpRunner('id') + runner.runner_parameters = runner_parameters + runner.pre_run() + + expected_msg = (r'"url_hosts_blacklist" and "url_hosts_whitelist" parameters are mutually ' + 'exclusive.') + self.assertRaisesRegexp(ValueError, expected_msg, runner.run, {})