From 327263540681492d442c1096b39058bc1b9867a1 Mon Sep 17 00:00:00 2001 From: themylogin Date: Mon, 9 Dec 2024 13:53:32 +0100 Subject: [PATCH 1/7] Log when private methods are called via websocket connection --- src/middlewared/middlewared/api/base/server/app.py | 1 + src/middlewared/middlewared/api/base/server/method.py | 4 ++++ .../middlewared/api/base/server/ws_handler/rpc.py | 4 ++++ src/middlewared/middlewared/api/v25_04_0/core.py | 1 + src/middlewared/middlewared/plugins/failover_/remote.py | 2 +- src/middlewared/middlewared/plugins/jbof/redfish/client.py | 3 ++- src/middlewared/middlewared/plugins/zettarepl.py | 5 ++--- src/middlewared/middlewared/service/core_service.py | 2 ++ .../middlewared/test/integration/utils/client.py | 5 +++-- src/middlewared/middlewared/worker.py | 7 ++++--- 10 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/middlewared/middlewared/api/base/server/app.py b/src/middlewared/middlewared/api/base/server/app.py index 2a48949d696dc..c272a61c8243a 100644 --- a/src/middlewared/middlewared/api/base/server/app.py +++ b/src/middlewared/middlewared/api/base/server/app.py @@ -14,6 +14,7 @@ def __init__(self, origin: ConnectionOrigin): self.authenticated = False self.authentication_context: AuthenticationContext = AuthenticationContext() self.authenticated_credentials: SessionManagerCredentials | None = None + self.private_methods = False self.py_exceptions = False self.websocket = False self.rest = False diff --git a/src/middlewared/middlewared/api/base/server/method.py b/src/middlewared/middlewared/api/base/server/method.py index 53e344a3b29d1..7c621e5d268ae 100644 --- a/src/middlewared/middlewared/api/base/server/method.py +++ b/src/middlewared/middlewared/api/base/server/method.py @@ -22,6 +22,10 @@ def __init__(self, middleware: "Middleware", name: str): self.name = name self.serviceobj, self.methodobj = self.middleware.get_method(self.name) + @property + def private(self): + return getattr(self.methodobj, "_private", False) + async def call(self, app: "RpcWebSocketApp", params: list): """ Calls the method in the context of a given `app`. diff --git a/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py b/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py index 12c43c68fb57d..9039a2a47ec8a 100644 --- a/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py +++ b/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py @@ -299,6 +299,10 @@ async def process_message(self, app: RpcWebSocketApp, message: dict): if id_ != undefined: app.send_error(id_, JSONRPCError.METHOD_NOT_FOUND.value, "Method does not exist") return + if not app.private_methods and method.private: + # FIXME: Eventually, prohibit this + self.middleware.logger.warning("Private method %r called on a connection without private method call " + "enabled", method.name) asyncio.ensure_future( self.process_method_call(app, id_, method, message["params"]) diff --git a/src/middlewared/middlewared/api/v25_04_0/core.py b/src/middlewared/middlewared/api/v25_04_0/core.py index 4b58ffc83ad2b..2d37b03ff8920 100644 --- a/src/middlewared/middlewared/api/v25_04_0/core.py +++ b/src/middlewared/middlewared/api/v25_04_0/core.py @@ -23,6 +23,7 @@ class CorePingResult(BaseModel): class CoreSetOptionsOptions(BaseModel, metaclass=ForUpdateMetaclass): + private_methods: bool py_exceptions: bool diff --git a/src/middlewared/middlewared/plugins/failover_/remote.py b/src/middlewared/middlewared/plugins/failover_/remote.py index 2a0b35d143c17..82e0f08324b36 100644 --- a/src/middlewared/middlewared/plugins/failover_/remote.py +++ b/src/middlewared/middlewared/plugins/failover_/remote.py @@ -65,7 +65,7 @@ def connect_and_wait(self, *, legacy=False): url = f'ws://{self.remote_ip}:6000/websocket' try: - with Client(url, reserved_ports=True) as c: + with Client(url, reserved_ports=True, private_methods=True) as c: self.client = c with self._subscribe_lock: self.connected.set() diff --git a/src/middlewared/middlewared/plugins/jbof/redfish/client.py b/src/middlewared/middlewared/plugins/jbof/redfish/client.py index 8e814ae954ec3..dc4294e334c9d 100644 --- a/src/middlewared/middlewared/plugins/jbof/redfish/client.py +++ b/src/middlewared/middlewared/plugins/jbof/redfish/client.py @@ -455,7 +455,8 @@ async def cache_get(cls, uuid, jbof_query=None): if jbof_query is not None: jbofs = jbof_query else: - with Client(f'ws+unix://{MIDDLEWARE_RUN_DIR}/middlewared-internal.sock', py_exceptions=True) as c: + with Client(f'ws+unix://{MIDDLEWARE_RUN_DIR}/middlewared-internal.sock', private_methods=True, + py_exceptions=True) as c: jbofs = c.call('jbof.query', filters) for jbof in filter_list(jbofs, filters, options): diff --git a/src/middlewared/middlewared/plugins/zettarepl.py b/src/middlewared/middlewared/plugins/zettarepl.py index 3f2355cd3e042..45e119f89cfd7 100644 --- a/src/middlewared/middlewared/plugins/zettarepl.py +++ b/src/middlewared/middlewared/plugins/zettarepl.py @@ -199,9 +199,8 @@ def _observer(self, message): task_id = int(message.task_id.split("_")[-1]) if isinstance(message, PeriodicSnapshotTaskStart): - with Client() as c: + with Client(private_methods=True) as c: context = None - vm_context = None if begin_context := c.call("vmware.periodic_snapshot_task_begin", task_id): context = c.call("vmware.periodic_snapshot_task_proceed", begin_context, job=True) if vm_context := c.call("vm.periodic_snapshot_task_begin", task_id): @@ -220,7 +219,7 @@ def _observer(self, message): context = self.vmware_contexts.pop(task_id, None) vm_context = self.vm_contexts.pop(task_id, None) if context or vm_context: - with Client() as c: + with Client(private_methods=True) as c: if context: c.call("vmware.periodic_snapshot_task_end", context, job=True) if vm_context: diff --git a/src/middlewared/middlewared/service/core_service.py b/src/middlewared/middlewared/service/core_service.py index 3bd1fee40ebb8..39b426ae9d1e3 100644 --- a/src/middlewared/middlewared/service/core_service.py +++ b/src/middlewared/middlewared/service/core_service.py @@ -859,6 +859,8 @@ def _cli_args_descriptions(self, doc, names): @api_method(CoreSetOptionsArgs, CoreSetOptionsResult, authentication_required=False, rate_limit=False) @pass_app() async def set_options(self, app, options): + if "private_methods" in options: + app.private_methods = options["private_methods"] if "py_exceptions" in options: app.py_exceptions = options["py_exceptions"] diff --git a/src/middlewared/middlewared/test/integration/utils/client.py b/src/middlewared/middlewared/test/integration/utils/client.py index 67e4902ab334b..6d7242afaa266 100644 --- a/src/middlewared/middlewared/test/integration/utils/client.py +++ b/src/middlewared/middlewared/test/integration/utils/client.py @@ -116,7 +116,7 @@ def client(self) -> Client: raise RuntimeError('IP is not set') uri = host_websocket_uri(addr) - cl = Client(uri, py_exceptions=True, log_py_exceptions=True, verify_ssl=False) + cl = Client(uri, private_methods=True, py_exceptions=True, log_py_exceptions=True, verify_ssl=False) try: resp = cl.call('auth.login_ex', { 'mechanism': 'PASSWORD_PLAIN', @@ -166,7 +166,8 @@ def client(*, auth=undefined, auth_required=True, py_exceptions=True, log_py_exc uri = host_websocket_uri(host_ip, ssl) try: - with Client(uri, py_exceptions=py_exceptions, log_py_exceptions=log_py_exceptions, verify_ssl=False) as c: + with Client(uri, private_methods=True, py_exceptions=py_exceptions, log_py_exceptions=log_py_exceptions, + verify_ssl=False) as c: if auth is not None: auth_req = { "mechanism": "PASSWORD_PLAIN", diff --git a/src/middlewared/middlewared/worker.py b/src/middlewared/middlewared/worker.py index bb3d55816400d..9c7a95efa3b80 100755 --- a/src/middlewared/middlewared/worker.py +++ b/src/middlewared/middlewared/worker.py @@ -32,7 +32,8 @@ def __init__(self): def _call(self, name, serviceobj, methodobj, params=None, app=None, pipes=None, job=None): try: - with Client(f'ws+unix://{MIDDLEWARE_RUN_DIR}/middlewared-internal.sock', py_exceptions=True) as c: + with Client(f'ws+unix://{MIDDLEWARE_RUN_DIR}/middlewared-internal.sock', private_methods=True, + py_exceptions=True) as c: self.client = c job_options = getattr(methodobj, '_job', None) if job and job_options: @@ -86,7 +87,7 @@ def get_events(self): return [] def send_event(self, name, event_type, **kwargs): - with Client(py_exceptions=True) as c: + with Client(private_methods=True, py_exceptions=True) as c: return c.call('core.event_send', name, event_type, kwargs) @@ -125,7 +126,7 @@ def main_worker(*call_args): def receive_events(): - c = Client(f'ws+unix://{MIDDLEWARE_RUN_DIR}/middlewared-internal.sock', py_exceptions=True) + c = Client(f'ws+unix://{MIDDLEWARE_RUN_DIR}/middlewared-internal.sock', private_methods=True, py_exceptions=True) c.subscribe('core.environ', lambda *args, **kwargs: environ_update(kwargs['fields'])) environ_update(c.call('core.environ')) From f40bbcb62eb36fc41b0403cd4cecd903658fc8e0 Mon Sep 17 00:00:00 2001 From: themylogin Date: Wed, 11 Dec 2024 14:46:09 +0100 Subject: [PATCH 2/7] Update src/middlewared/middlewared/api/base/server/ws_handler/rpc.py Co-authored-by: Caleb St. John <30729806+yocalebo@users.noreply.github.com> --- src/middlewared/middlewared/api/base/server/ws_handler/rpc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py b/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py index 9039a2a47ec8a..f48ca19f1f354 100644 --- a/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py +++ b/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py @@ -301,7 +301,7 @@ async def process_message(self, app: RpcWebSocketApp, message: dict): return if not app.private_methods and method.private: # FIXME: Eventually, prohibit this - self.middleware.logger.warning("Private method %r called on a connection without private method call " + self.middleware.logger.warning("Private method %r called on a connection without private_methods " "enabled", method.name) asyncio.ensure_future( From e5b09710f6aa6618f9c9be00e7c1042528e9ef0b Mon Sep 17 00:00:00 2001 From: themylogin Date: Mon, 16 Dec 2024 13:31:21 +0100 Subject: [PATCH 3/7] Do not log private method calls initialized by systemd --- src/freenas/usr/local/sbin/hactl | 2 +- .../api/base/server/ws_handler/rpc.py | 16 ++++++++++++++-- src/middlewared/middlewared/utils/auth.py | 4 ++-- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/freenas/usr/local/sbin/hactl b/src/freenas/usr/local/sbin/hactl index 8856d10bfd824..0ebdb5405027c 100755 --- a/src/freenas/usr/local/sbin/hactl +++ b/src/freenas/usr/local/sbin/hactl @@ -26,7 +26,7 @@ class StatusEnum(enum.Enum): def get_client(): try: - return Client() + return Client(private_methods=True) except Exception as e: print_msg_and_exit(f'Unexpected failure enumerating websocket client: {e}') diff --git a/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py b/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py index f48ca19f1f354..d218a84c13244 100644 --- a/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py +++ b/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py @@ -17,6 +17,7 @@ from middlewared.schema import Error from middlewared.service_exception import (CallException, CallError, ValidationError, ValidationErrors, adapt_exception, get_errname) +from middlewared.utils.auth import AUID_UNSET, AUID_FAULTED from middlewared.utils.debug import get_frame_details from middlewared.utils.lang import undefined from middlewared.utils.limits import MsgSizeError, MsgSizeLimit, parse_message @@ -299,15 +300,26 @@ async def process_message(self, app: RpcWebSocketApp, message: dict): if id_ != undefined: app.send_error(id_, JSONRPCError.METHOD_NOT_FOUND.value, "Method does not exist") return - if not app.private_methods and method.private: + if not app.private_methods and method.private and not self._can_call_private_methods(app): # FIXME: Eventually, prohibit this - self.middleware.logger.warning("Private method %r called on a connection without private_methods " + self.middleware.logger.warning("Private method %r called on a connection without private_methods " "enabled", method.name) asyncio.ensure_future( self.process_method_call(app, id_, method, message["params"]) ) + def _can_call_private_methods(self, app: RpcWebSocketApp): + if app.origin.uid == 33: + # Calls made via WebSocket API + return False + + if app.origin.loginuid in [AUID_UNSET, AUID_FAULTED]: + # System-initiated calls to `midclt` + return True + + return False + async def process_method_call(self, app: RpcWebSocketApp, id_: Any, method: Method, params: list): try: async with app.softhardsemaphore: diff --git a/src/middlewared/middlewared/utils/auth.py b/src/middlewared/middlewared/utils/auth.py index 2dd8998435e63..ed730165ad721 100644 --- a/src/middlewared/middlewared/utils/auth.py +++ b/src/middlewared/middlewared/utils/auth.py @@ -6,8 +6,8 @@ LEGACY_API_KEY_USERNAME = 'LEGACY_API_KEY' MAX_OTP_ATTEMPTS = 3 -AUID_UNSET = 2 ** 32 -1 -AUID_FAULTED = 2 ** 32 -2 +AUID_UNSET = 2 ** 32 - 1 +AUID_FAULTED = 2 ** 32 - 2 class AuthMech(enum.StrEnum): From 7a4d1e452d51e989955960b117061e3add11e296 Mon Sep 17 00:00:00 2001 From: themylogin Date: Tue, 17 Dec 2024 21:55:48 +0100 Subject: [PATCH 4/7] Allow `cron` to call private methods --- .../api/base/server/ws_handler/rpc.py | 10 ++++++++ src/middlewared/middlewared/utils/origin.py | 25 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py b/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py index d218a84c13244..b3c289bd7e1f0 100644 --- a/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py +++ b/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py @@ -318,6 +318,16 @@ def _can_call_private_methods(self, app: RpcWebSocketApp): # System-initiated calls to `midclt` return True + if ppids := app.origin.ppids(): + try: + with open("/run/crond.pid") as f: + cron_pid = int(f.read()) + except (FileNotFoundError, ValueError): + return False + + if cron_pid in ppids: + return True + return False async def process_method_call(self, app: RpcWebSocketApp, id_: Any, method: Method, params: list): diff --git a/src/middlewared/middlewared/utils/origin.py b/src/middlewared/middlewared/utils/origin.py index 41664e57bb8fe..91adfc6a1c74c 100644 --- a/src/middlewared/middlewared/utils/origin.py +++ b/src/middlewared/middlewared/utils/origin.py @@ -1,5 +1,6 @@ from dataclasses import dataclass from ipaddress import ip_address +import re from socket import AF_INET, AF_INET6, AF_UNIX, SO_PEERCRED, SOL_SOCKET from struct import calcsize, unpack @@ -148,6 +149,30 @@ def secure_transport(self) -> bool: # By default assume that transport is insecure return False + def ppids(self) -> set[int]: + if self.pid is None: + return set() + + pid = self.pid + ppids = set() + while True: + try: + with open(f"/proc/{pid}/status") as f: + status = f.read() + except FileNotFoundError: + break + + if m := re.search(r"PPid:\t([0-9]+)", status): + pid = int(m.group(1)) + if pid <= 1: + break + + ppids.add(pid) + else: + break + + return ppids + def get_tcp_ip_info(sock, request) -> tuple: # All API connections are terminated by nginx reverse From fb680e45d67265f98c65667f8bed26a3c8a3c620 Mon Sep 17 00:00:00 2001 From: themylogin Date: Tue, 28 Jan 2025 11:05:44 +0100 Subject: [PATCH 5/7] Address review --- .../middlewared/api/base/server/ws_handler/rpc.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py b/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py index b3c289bd7e1f0..e82c71006a2a2 100644 --- a/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py +++ b/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py @@ -302,8 +302,10 @@ async def process_message(self, app: RpcWebSocketApp, message: dict): return if not app.private_methods and method.private and not self._can_call_private_methods(app): # FIXME: Eventually, prohibit this - self.middleware.logger.warning("Private method %r called on a connection without private_methods " - "enabled", method.name) + self.middleware.logger.warning( + "Private method %r called on a connection without private_methods enabled", + method.name + ) asyncio.ensure_future( self.process_method_call(app, id_, method, message["params"]) From a79c54cdb37ad7efa0bc7b65778dd6d3864050bc Mon Sep 17 00:00:00 2001 From: themylogin Date: Tue, 28 Jan 2025 16:10:30 +0100 Subject: [PATCH 6/7] Address review --- .../middlewared/api/base/server/ws_handler/rpc.py | 5 ++--- src/middlewared/middlewared/utils/origin.py | 14 ++++++++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py b/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py index e82c71006a2a2..aace97589bdcd 100644 --- a/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py +++ b/src/middlewared/middlewared/api/base/server/ws_handler/rpc.py @@ -316,7 +316,7 @@ def _can_call_private_methods(self, app: RpcWebSocketApp): # Calls made via WebSocket API return False - if app.origin.loginuid in [AUID_UNSET, AUID_FAULTED]: + if app.origin.loginuid in (AUID_UNSET, AUID_FAULTED): # System-initiated calls to `midclt` return True @@ -327,8 +327,7 @@ def _can_call_private_methods(self, app: RpcWebSocketApp): except (FileNotFoundError, ValueError): return False - if cron_pid in ppids: - return True + return cron_pid in ppids return False diff --git a/src/middlewared/middlewared/utils/origin.py b/src/middlewared/middlewared/utils/origin.py index 91adfc6a1c74c..932f6a0ec741a 100644 --- a/src/middlewared/middlewared/utils/origin.py +++ b/src/middlewared/middlewared/utils/origin.py @@ -1,6 +1,5 @@ from dataclasses import dataclass from ipaddress import ip_address -import re from socket import AF_INET, AF_INET6, AF_UNIX, SO_PEERCRED, SOL_SOCKET from struct import calcsize, unpack @@ -158,12 +157,19 @@ def ppids(self) -> set[int]: while True: try: with open(f"/proc/{pid}/status") as f: - status = f.read() + pid = None + for line in f: + if line.startswith("PPid:"): + try: + pid = int(line.split(":")[1].strip()) + except ValueError: + pass + + break except FileNotFoundError: break - if m := re.search(r"PPid:\t([0-9]+)", status): - pid = int(m.group(1)) + if pid is not None: if pid <= 1: break From 97fc8100b0c432652636717989d51df741685009 Mon Sep 17 00:00:00 2001 From: themylogin Date: Tue, 28 Jan 2025 17:48:12 +0100 Subject: [PATCH 7/7] Fix `core.get_services` --- src/middlewared/middlewared/service/core_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/middlewared/middlewared/service/core_service.py b/src/middlewared/middlewared/service/core_service.py index 39b426ae9d1e3..18fdc97337783 100644 --- a/src/middlewared/middlewared/service/core_service.py +++ b/src/middlewared/middlewared/service/core_service.py @@ -269,7 +269,7 @@ def get_services(self, app, target): _typ = 'service' config = {k: v for k, v in list(v._config.__dict__.items()) - if not (k in ['entry', 'process_pool', 'thread_pool'] or k.startswith('_'))} + if not (k in ['entry', 'events', 'process_pool', 'thread_pool'] or k.startswith('_'))} if config['cli_description'] is None: if v.__doc__: config['cli_description'] = inspect.getdoc(v).split("\n")[0].strip()