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
2026-08-25 14:53:37 +00:00 · 2021-07-05 09:43:56 +02:00
parent b7a11232eb
commit 62aa1333cd
7 changed files with 123 additions and 42 deletions
@@ -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``.

@@ -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``.

@@ -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}

@@ -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
@@ -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)

@@ -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:
@@ -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',