Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NAS-129926 / 25.04 / Log when private methods are called via websocket connection #15160

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/freenas/usr/local/sbin/hactl
Original file line number Diff line number Diff line change
Expand Up @@ -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}')

Expand Down
1 change: 1 addition & 0 deletions src/middlewared/middlewared/api/base/server/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions src/middlewared/middlewared/api/base/server/method.py
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
25 changes: 25 additions & 0 deletions src/middlewared/middlewared/api/base/server/ws_handler/rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,34 @@ async def process_message(self, app: RpcWebSocketApp, message: Any):
if method is None:
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.get("params", [])))

def _can_call_private_methods(self, app: RpcWebSocketApp):
if app.origin.uid == 33:
# Proxied HexOS calls
return False

if app.origin.loginuid() is None:
# 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):
try:
async with app.softhardsemaphore:
Expand Down
1 change: 1 addition & 0 deletions src/middlewared/middlewared/api/v25_04_0/core.py
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
2 changes: 1 addition & 1 deletion src/middlewared/middlewared/plugins/failover_/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
3 changes: 2 additions & 1 deletion src/middlewared/middlewared/plugins/jbof/redfish/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
5 changes: 2 additions & 3 deletions src/middlewared/middlewared/plugins/zettarepl.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,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):
Expand All @@ -219,7 +218,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:
Expand Down
2 changes: 2 additions & 0 deletions src/middlewared/middlewared/service/core_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,8 @@ def _cli_args_descriptions(self, doc, names):
@api_method(CoreSetOptionsArgs, CoreSetOptionsResult, 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"]

Expand Down
4 changes: 2 additions & 2 deletions src/middlewared/middlewared/test/integration/utils/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
cl = Client(uri, private_methods=True, py_exceptions=True, log_py_exceptions=True)
try:
resp = cl.call('auth.login_ex', {
'mechanism': 'PASSWORD_PLAIN',
Expand Down Expand Up @@ -166,7 +166,7 @@ def client(*, auth=undefined, auth_required=True, py_exceptions=True, log_py_exc

uri = host_websocket_uri(host_ip)
try:
with Client(uri, py_exceptions=py_exceptions, log_py_exceptions=log_py_exceptions) as c:
with Client(uri, private_methods=True, py_exceptions=py_exceptions, log_py_exceptions=log_py_exceptions) as c:
if auth is not None:
auth_req = {
"mechanism": "PASSWORD_PLAIN",
Expand Down
40 changes: 40 additions & 0 deletions src/middlewared/middlewared/utils/origin.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from dataclasses import dataclass
import re
from socket import AF_INET, AF_INET6, AF_UNIX, SO_PEERCRED, SOL_SOCKET
from struct import calcsize, unpack

Expand Down Expand Up @@ -97,6 +98,45 @@ def is_ha_connection(self) -> bool:
self.rem_addr and self.rem_addr in HA_HEARTBEAT_IPS
)

def loginuid(self) -> int | None:
if self.pid is None:
return None

try:
with open(f"/proc/{self.pid}/loginuid") as f:
loginuid = int(f.read())
except (FileNotFoundError, ValueError):
return None

if loginuid == 4294967295:
return None

return loginuid

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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a large file to read into memory and then do a regex search on. I also don't like this approach. I'd rather remove entries from middlewared.mako (cron) and schedule them as @periodic

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
Expand Down
7 changes: 4 additions & 3 deletions src/middlewared/middlewared/worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,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:
Expand Down Expand Up @@ -82,7 +83,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)


Expand Down Expand Up @@ -121,7 +122,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'))

Expand Down
Loading