NAS-129926 / 25.10 / Log when private methods are called via websocke…

…t connection (#15160) * Log when private methods are called via websocket connection * Update src/middlewared/middlewared/api/base/server/ws_handler/rpc.py Co-authored-by: Caleb St. John <[email protected]> * Do not log private method calls initialized by systemd * Allow `cron` to call private methods * Address review * Address review * Fix `core.get_services` --------- Co-authored-by: Caleb St. John <[email protected]>
truenas · Jan 28, 2025 · 9138421 · 9138421
1 parent 55852f6
commit 9138421
Show file tree

Hide file tree

Showing 13 changed files with 81 additions and 13 deletions.
diff --git 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/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
@@ -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
@@ -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,11 +300,37 @@ 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 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
+            )
 
         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
+
+        if ppids := app.origin.ppids():
+            try:
+                with open("/run/crond.pid") as f:
+                    cron_pid = int(f.read())
+            except (FileNotFoundError, ValueError):
+                return False
+
+            return cron_pid in ppids
+
+        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/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
@@ -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
@@ -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
@@ -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
@@ -861,6 +861,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
@@ -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/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):

diff --git a/src/middlewared/middlewared/utils/origin.py b/src/middlewared/middlewared/utils/origin.py
@@ -148,6 +148,37 @@ 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:
+                    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 pid is not None:
+                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

diff --git 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'))
Original file line number	Diff line number	Diff line change
Expand Up		@@ -23,6 +23,7 @@ class CorePingResult(BaseModel):


		class CoreSetOptionsOptions(BaseModel, metaclass=ForUpdateMetaclass):
		private_methods: bool
		py_exceptions: bool


Expand Down