From 62aa1333cde1d0f1f3eb6d8094228909c3695221 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 5 Jul 2021 09:43:56 +0200 Subject: [PATCH] Implemented allowlist for REST API (#1959) If configured, only IPs that matching rules would be allowed to call unsafe endpoints. In addition to that, it is possible to automatically include IPs of members of the cluster to the list. If neither of the above is configured the old behavior is retained. Partially address https://github.com/zalando/patroni/issues/1734 --- docs/ENVIRONMENT.rst | 2 ++ docs/SETTINGS.rst | 2 ++ patroni/api.py | 75 +++++++++++++++++++++++++++++++++++--------- patroni/config.py | 60 +++++++++++++++++++++++------------ setup.py | 9 +++--- tests/test_api.py | 15 +++++++-- tests/test_config.py | 2 ++ 7 files changed, 123 insertions(+), 42 deletions(-) diff --git a/docs/ENVIRONMENT.rst b/docs/ENVIRONMENT.rst index 618497aa..7c837a6b 100644 --- a/docs/ENVIRONMENT.rst +++ b/docs/ENVIRONMENT.rst @@ -166,6 +166,8 @@ REST API - **PATRONI\_RESTAPI\_CAFILE**: Specifies the file with the CA_BUNDLE with certificates of trusted CAs to use while verifying client certs. - **PATRONI\_RESTAPI\_CIPHERS**: (optional) Specifies the permitted cipher suites (e.g. "ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES128-GCM-SHA256:!SSLv1:!SSLv2:!SSLv3:!TLSv1:!TLSv1.1") - **PATRONI\_RESTAPI\_VERIFY\_CLIENT**: ``none`` (default), ``optional`` or ``required``. When ``none`` REST API will not check client certificates. When ``required`` client certificates are required for all REST API calls. When ``optional`` client certificates are required for all unsafe REST API endpoints. When ``required`` is used, then client authentication succeeds, if the certificate signature verification succeeds. For ``optional`` the client cert will only be checked for ``PUT``, ``POST``, ``PATCH``, and ``DELETE`` requests. +- **PATRONI\_RESTAPI\_ALLOWLIST**: (optional): Specifies the set of hosts that are allowed to call unsafe REST API endpoints. The single element could be a host name, an IP address or a network address using CIDR notation. By default ``allow all`` is used. In case if ``allowlist`` or ``allowlist_include_members`` are set, anything that is not included is rejected. +- **PATRONI\_RESTAPI\_ALLOWLIST\_INCLUDE\_MEMBERS**: (optional): If set to ``true`` it allows accessing unsafe REST API endpoints from other cluster members registered in DCS (IP address or hostname is taken from the members ``api_url``). Be careful, it might happen that OS will use a different IP for outgoing connections. - **PATRONI\_RESTAPI\_HTTP\_EXTRA\_HEADERS**: (optional) HTTP headers let the REST API server pass additional information with an HTTP response. - **PATRONI\_RESTAPI\_HTTPS\_EXTRA\_HEADERS**: (optional) HTTPS headers let the REST API server pass additional information with an HTTP response when TLS is enabled. This will also pass additional information set in ``http_extra_headers``. diff --git a/docs/SETTINGS.rst b/docs/SETTINGS.rst index 664a62c8..64428f38 100644 --- a/docs/SETTINGS.rst +++ b/docs/SETTINGS.rst @@ -327,6 +327,8 @@ REST API - **cafile**: (optional): Specifies the file with the CA_BUNDLE with certificates of trusted CAs to use while verifying client certs. - **ciphers**: (optional): Specifies the permitted cipher suites (e.g. "ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES128-GCM-SHA256:!SSLv1:!SSLv2:!SSLv3:!TLSv1:!TLSv1.1") - **verify\_client**: (optional): ``none`` (default), ``optional`` or ``required``. When ``none`` REST API will not check client certificates. When ``required`` client certificates are required for all REST API calls. When ``optional`` client certificates are required for all unsafe REST API endpoints. When ``required`` is used, then client authentication succeeds, if the certificate signature verification succeeds. For ``optional`` the client cert will only be checked for ``PUT``, ``POST``, ``PATCH``, and ``DELETE`` requests. + - **allowlist**: (optional): Specifies the set of hosts that are allowed to call unsafe REST API endpoints. The single element could be a host name, an IP address or a network address using CIDR notation. By default ``allow all`` is used. In case if ``allowlist`` or ``allowlist_include_members`` are set, anything that is not included is rejected. + - **allowlist\_include\_members**: (optional): If set to ``true`` it allows accessing unsafe REST API endpoints from other cluster members registered in DCS (IP address or hostname is taken from the members ``api_url``). Be careful, it might happen that OS will use a different IP for outgoing connections. - **http\_extra\_headers**: (optional): HTTP headers let the REST API server pass additional information with an HTTP response. - **https\_extra\_headers**: (optional): HTTPS headers let the REST API server pass additional information with an HTTP response when TLS is enabled. This will also pass additional information set in ``http_extra_headers``. diff --git a/patroni/api.py b/patroni/api.py index 2a34c242..e5e2baa9 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -12,6 +12,7 @@ import six import socket import sys +from ipaddress import ip_address, ip_network as _ip_network from six.moves.BaseHTTPServer import BaseHTTPRequestHandler, HTTPServer from six.moves.socketserver import ThreadingMixIn from six.moves.urllib_parse import urlparse, parse_qs @@ -25,6 +26,10 @@ from .utils import deep_compare, enable_keepalive, parse_bool, patch_config, Ret logger = logging.getLogger(__name__) +def ip_network(value): + return _ip_network(value.decode('utf-8') if six.PY2 else value, False) + + class RestApiHandler(BaseHTTPRequestHandler): def _write_status_code_only(self, status_code): @@ -47,17 +52,17 @@ class RestApiHandler(BaseHTTPRequestHandler): def _write_json_response(self, status_code, response): self._write_response(status_code, json.dumps(response, default=str), content_type='application/json') - def check_auth(func): - """Decorator function to check authorization header or client certificates + def check_access(func): + """Decorator function to check the source ip, authorization header. or client certificates Usage example: - @check_auth + @check_access def do_PUT_foo(): pass """ def wrapper(self, *args, **kwargs): - if self.server.check_auth(self): + if self.server.check_access(self): return func(self, *args, **kwargs) return wrapper @@ -299,7 +304,7 @@ class RestApiHandler(BaseHTTPRequestHandler): logger.exception('Bad request') self.send_error(400) - @check_auth + @check_access def do_PATCH_config(self): request = self._read_json_content() if request: @@ -314,7 +319,7 @@ class RestApiHandler(BaseHTTPRequestHandler): self.server.patroni.ha.wakeup() self._write_json_response(200, data) - @check_auth + @check_access def do_PUT_config(self): request = self._read_json_content() if request: @@ -325,7 +330,7 @@ class RestApiHandler(BaseHTTPRequestHandler): return self.send_error(502) self._write_json_response(200, request) - @check_auth + @check_access def do_POST_reload(self): self.server.patroni.sighup_handler() self._write_response(202, 'reload scheduled') @@ -351,7 +356,7 @@ class RestApiHandler(BaseHTTPRequestHandler): status_code = 422 return (status_code, error, scheduled_at) - @check_auth + @check_access def do_POST_restart(self): status_code = 500 data = 'restart failed' @@ -412,7 +417,7 @@ class RestApiHandler(BaseHTTPRequestHandler): status_code = 409 self._write_response(status_code, data) - @check_auth + @check_access def do_DELETE_restart(self): if self.server.patroni.ha.delete_future_restart(): data = "scheduled restart deleted" @@ -422,7 +427,7 @@ class RestApiHandler(BaseHTTPRequestHandler): code = 404 self._write_response(code, data) - @check_auth + @check_access def do_DELETE_switchover(self): failover = self.server.patroni.dcs.get_cluster().failover if failover and failover.scheduled_at: @@ -436,7 +441,7 @@ class RestApiHandler(BaseHTTPRequestHandler): code = 404 self._write_response(code, data) - @check_auth + @check_access def do_POST_reinitialize(self): request = self._read_json_content(body_is_optional=True) @@ -493,7 +498,7 @@ class RestApiHandler(BaseHTTPRequestHandler): return None return action + ' is not possible: no good candidates have been found' - @check_auth + @check_access def do_POST_failover(self, action='failover'): request = self._read_json_content() (status_code, data) = (400, '') @@ -641,7 +646,6 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): self.patroni = patroni self.__listen = None self.__ssl_options = None - self.http_extra_headers = {} self.reload_config(config) self.daemon = True self.__ssl_serial_number = None @@ -675,7 +679,35 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): if not auth_header.startswith('Basic ') or not self.check_basic_auth_key(auth_header[6:]): return 'not authenticated' - def check_auth(self, rh): + @staticmethod + def __resolve_ips(host, port): + try: + for _, _, _, _, sa in socket.getaddrinfo(host, port, 0, socket.SOCK_STREAM, socket.IPPROTO_TCP): + yield ip_network(sa[0]) + except Exception as e: + logger.error('Failed to resolve %s: %r', host, e) + + def __members_ips(self): + cluster = self.patroni.dcs.cluster + if self.__allowlist_include_members and cluster: + for member in cluster.members: + if member.api_url: + try: + r = urlparse(member.api_url) + host = r.hostname + port = r.port or (443 if r.scheme == 'https' else 80) + for ip in self.__resolve_ips(host, port): + yield ip + except Exception as e: + logger.debug('Failed to parse url %s: %r', member.api_url, e) + + def check_access(self, rh): + if self.__allowlist or self.__allowlist_include_members: + incoming_ip = rh.client_address[0] + incoming_ip = ip_address(incoming_ip.decode('utf-8') if six.PY2 else incoming_ip) + if not any(incoming_ip in net for net in self.__allowlist + tuple(self.__members_ips())): + return rh._write_response(403, 'Access is denied') + if not hasattr(rh.request, 'getpeercert') or not rh.request.getpeercert(): # valid client cert isn't present if self.__protocol == 'https' and self.__ssl_options.get('verify_client') in ('required', 'optional'): return rh._write_response(403, 'client certificate required') @@ -799,10 +831,25 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): self.__ssl_serial_number = on_disk_cert_serial_number return True + def _build_allowlist(self, value): + if isinstance(value, list): + for v in value: + if '/' in v: # netmask + try: + yield ip_network(v) + except Exception as e: + logger.error('Invalid value "%s" in the allowlist: %r', v, e) + else: # ip or hostname, try to resolve it + for ip in self.__resolve_ips(v, 8080): + yield ip + def reload_config(self, config): if 'listen' not in config: # changing config in runtime raise ValueError('Can not find "restapi.listen" config') + self.__allowlist = tuple(self._build_allowlist(config.get('allowlist'))) + self.__allowlist_include_members = config.get('allowlist_include_members') + ssl_options = {n: config[n] for n in ('certfile', 'keyfile', 'keyfile_password', 'cafile', 'ciphers') if n in config} diff --git a/patroni/config.py b/patroni/config.py index 6c6c73b0..ce5b840e 100644 --- a/patroni/config.py +++ b/patroni/config.py @@ -268,11 +268,42 @@ class Config(object): _set_section_values('restapi', ['listen', 'connect_address', 'certfile', 'keyfile', 'keyfile_password', 'cafile', 'ciphers', 'verify_client', 'http_extra_headers', - 'https_extra_headers']) + 'https_extra_headers', 'allowlist', 'allowlist_include_members']) _set_section_values('ctl', ['insecure', 'cacert', 'certfile', 'keyfile']) _set_section_values('postgresql', ['listen', 'connect_address', 'config_dir', 'data_dir', 'pgpass', 'bin_dir']) _set_section_values('log', ['level', 'traceback_level', 'format', 'dateformat', 'max_queue_size', 'dir', 'file_size', 'file_num', 'loggers']) + _set_section_values('raft', ['data_dir', 'self_addr', 'partner_addrs', 'password', 'bind_addr']) + + for first, second in (('restapi', 'allowlist_include_members'), ('ctl', 'insecure')): + value = ret.get(first, {}).pop(second, None) + if value: + value = parse_bool(value) + if value is not None: + ret[first][second] = value + + for second in ('max_queue_size', 'file_size', 'file_num'): + value = ret.get('log', {}).pop(second, None) + if value: + value = parse_int(value) + if value is not None: + ret['log'][second] = value + + def _parse_list(value): + if not (value.strip().startswith('-') or '[' in value): + value = '[{0}]'.format(value) + try: + return yaml.safe_load(value) + except Exception: + logger.exception('Exception when parsing list %s', value) + return None + + for first, second in (('raft', 'partner_addrs'), ('restapi', 'allowlist')): + value = ret.get(first, {}).pop(second, None) + if value: + value = _parse_list(value) + if value: + ret[first][second] = value def _parse_dict(value): if not value.strip().startswith('{'): @@ -283,11 +314,13 @@ class Config(object): logger.exception('Exception when parsing dict %s', value) return None - value = ret.get('log', {}).pop('loggers', None) - if value: - value = _parse_dict(value) - if value: - ret['log']['loggers'] = value + for first, params in (('restapi', ('http_extra_headers', 'https_extra_headers')), ('log', ('loggers',))): + for second in params: + value = ret.get(first, {}).pop(second, None) + if value: + value = _parse_dict(value) + if value: + ret[first][second] = value def _get_auth(name, params=None): ret = {} @@ -310,19 +343,6 @@ class Config(object): if authentication: ret['postgresql']['authentication'] = authentication - def _parse_list(value): - if not (value.strip().startswith('-') or '[' in value): - value = '[{0}]'.format(value) - try: - return yaml.safe_load(value) - except Exception: - logger.exception('Exception when parsing list %s', value) - return None - - _set_section_values('raft', ['data_dir', 'self_addr', 'partner_addrs', 'password', 'bind_addr']) - if 'raft' in ret and 'partner_addrs' in ret['raft']: - ret['raft']['partner_addrs'] = _parse_list(ret['raft']['partner_addrs']) - for param in list(os.environ.keys()): if param.startswith(PATRONI_ENV_PREFIX): # PATRONI_(ETCD|CONSUL|ZOOKEEPER|EXHIBITOR|...)_(HOSTS?|PORT|..) @@ -339,7 +359,7 @@ class Config(object): value = value and _parse_list(value) elif suffix == 'LABELS': value = _parse_dict(value) - elif suffix in ('USE_PROXIES', 'REGISTER_SERVICE', 'USE_ENDPOINTS', 'BYPASS_API_SERVICE'): + elif suffix in ('USE_PROXIES', 'REGISTER_SERVICE', 'USE_ENDPOINTS', 'BYPASS_API_SERVICE', 'VERIFY'): value = parse_bool(value) if value: ret[name.lower()][suffix.lower()] = value diff --git a/setup.py b/setup.py index 7302858a..8cc5940a 100644 --- a/setup.py +++ b/setup.py @@ -24,7 +24,7 @@ KEYWORDS = 'etcd governor patroni postgresql postgres ha haproxy confd' +\ EXTRAS_REQUIRE = {'aws': ['boto'], 'etcd': ['python-etcd'], 'etcd3': ['python-etcd'], 'consul': ['python-consul'], 'exhibitor': ['kazoo'], 'zookeeper': ['kazoo'], - 'kubernetes': ['ipaddress'], 'raft': ['pysyncobj', 'cryptography']} + 'kubernetes': [], 'raft': ['pysyncobj', 'cryptography']} COVERAGE_XML = True COVERAGE_HTML = False @@ -175,13 +175,12 @@ def setup_package(version): for e, deps in EXTRAS_REQUIRE.items(): for i, v in enumerate(deps): if r.startswith(v): - if e != 'kubernetes' or sys.version_info < (3, 0, 0): - deps[i] = r - else: - deps = [] + deps[i] = r EXTRAS_REQUIRE[e] = deps extra = True break + if extra: + break if not extra: install_requires.append(r) diff --git a/tests/test_api.py b/tests/test_api.py index 4b4d8ebf..34a6224e 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -538,7 +538,9 @@ class TestRestApiServer(unittest.TestCase): @patch.object(BaseHTTPServer.HTTPServer, '__init__', Mock()) def setUp(self): self.srv = MockRestApiServer(Mock(), '', {'listen': '*:8008', 'certfile': 'a', 'verify_client': 'required', - 'ciphers': '!SSLv1:!SSLv2:!SSLv3:!TLSv1:!TLSv1.1'}) + 'ciphers': '!SSLv1:!SSLv2:!SSLv3:!TLSv1:!TLSv1.1', + 'allowlist': ['127.0.0.1', '::1/128', '::1/zxc'], + 'allowlist_include_members': True}) @patch.object(BaseHTTPServer.HTTPServer, '__init__', Mock()) def test_reload_config(self): @@ -549,10 +551,17 @@ class TestRestApiServer(unittest.TestCase): with patch.object(socket.socket, 'setsockopt', Mock(side_effect=socket.error)): self.srv.reload_config({'listen': ':8008'}) - def test_check_auth(self): + @patch.object(MockPatroni, 'dcs') + def test_check_access(self, mock_dcs): + mock_dcs.cluster = get_cluster_initialized_without_leader() + mock_dcs.cluster.members[1].data['api_url'] = 'http://127.0.0.1z:8011/patroni' + mock_dcs.cluster.members.append(Member(0, 'bad-api-url', 30, {'api_url': 123})) mock_rh = Mock() + mock_rh.client_address = ('127.0.0.2',) + self.assertIsNot(self.srv.check_access(mock_rh), True) + mock_rh.client_address = ('127.0.0.1',) mock_rh.request.getpeercert.return_value = None - self.assertIsNot(self.srv.check_auth(mock_rh), True) + self.assertIsNot(self.srv.check_access(mock_rh), True) def test_handle_error(self): try: diff --git a/tests/test_config.py b/tests/test_config.py index b656da26..dddae174 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -30,12 +30,14 @@ class TestConfig(unittest.TestCase): 'PATRONI_SCOPE': 'batman2', 'PATRONI_LOGLEVEL': 'ERROR', 'PATRONI_LOG_LOGGERS': 'patroni.postmaster: WARNING, urllib3: DEBUG', + 'PATRONI_LOG_FILE_NUM': '5', 'PATRONI_RESTAPI_USERNAME': 'username', 'PATRONI_RESTAPI_PASSWORD': 'password', 'PATRONI_RESTAPI_LISTEN': '0.0.0.0:8008', 'PATRONI_RESTAPI_CONNECT_ADDRESS': '127.0.0.1:8008', 'PATRONI_RESTAPI_CERTFILE': '/certfile', 'PATRONI_RESTAPI_KEYFILE': '/keyfile', + 'PATRONI_RESTAPI_ALLOWLIST_INCLUDE_MEMBERS': 'on', 'PATRONI_POSTGRESQL_LISTEN': '0.0.0.0:5432', 'PATRONI_POSTGRESQL_CONNECT_ADDRESS': '127.0.0.1:5432', 'PATRONI_POSTGRESQL_DATA_DIR': 'data/postgres0',