-
Notifications
You must be signed in to change notification settings - Fork 494
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.10 / Log when private methods are called via websocket connection #15160
base: master
Are you sure you want to change the base?
Changes from all commits
862996a
0755b0f
fa65cf8
7e34e26
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment is misleading. This will represent HexOS based connections but it will also represent nginx based connections. 33 is the |
||
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: | ||
|
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 | ||
|
||
|
@@ -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: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @yocalebo which entries? All of them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @yocalebo ping There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, all of the entries in cron. Putting them in cron is making it harder on us from a security standpoint and so I'd rather just shift to periodic tasks inside middleware. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Periodic tasks in middleware are not the same. We'll need a whole new subsystem to execute things at certain time (basically, re-implement cron internally). Does this belong to the scope of the ticket and do we want this for 25.04? |
||
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 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.