From ec17cce851c06f310ddd8e7d4b7966894ae2d274 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Wed, 7 Aug 2024 05:50:00 -0700 Subject: [PATCH] Improve auditing for middleware methods Populate `username` and `address` fields in middleware audit entries based on information from the middleware session. The UI prominently displays the `username` field which is currently hard-coded to root. This commit adds a helper function to generate a username string based on the current authenticated credentials for the session generating the audit entry. --- src/middlewared/middlewared/auth.py | 44 +++++++++++++++++ src/middlewared/middlewared/main.py | 5 +- src/middlewared/middlewared/plugins/auth.py | 48 +------------------ .../pytest/unit/utils/test_audit.py | 29 +++++++++++ src/middlewared/middlewared/utils/audit.py | 33 +++++++++++++ 5 files changed, 111 insertions(+), 48 deletions(-) create mode 100644 src/middlewared/middlewared/pytest/unit/utils/test_audit.py create mode 100644 src/middlewared/middlewared/utils/audit.py diff --git a/src/middlewared/middlewared/auth.py b/src/middlewared/middlewared/auth.py index 7dc92f4398144..89edce8e9d5fb 100644 --- a/src/middlewared/middlewared/auth.py +++ b/src/middlewared/middlewared/auth.py @@ -79,6 +79,43 @@ def dump(self): } +class TokenSessionManagerCredentials(SessionManagerCredentials): + def __init__(self, token_manager, token): + self.root_credentials = token.root_credentials() + + self.token_manager = token_manager + self.token = token + self.is_user_session = self.root_credentials.is_user_session + if self.is_user_session: + self.user = self.root_credentials.user + + self.allowlist = self.root_credentials.allowlist + + def is_valid(self): + return self.token.is_valid() + + def authorize(self, method, resource): + return self.token.parent_credentials.authorize(method, resource) + + def has_role(self, role): + return self.token.parent_credentials.has_role(role) + + def notify_used(self): + self.token.notify_used() + + def logout(self): + self.token_manager.destroy(self.token) + + def dump(self): + data = { + "parent": dump_credentials(self.token.parent_credentials), + } + if self.is_user_session: + data["username"] = self.user["username"] + + return data + + class TrueNasNodeSessionManagerCredentials(SessionManagerCredentials): def authorize(self, method, resource): return True @@ -94,3 +131,10 @@ class FakeApplication: def fake_app(): return FakeApplication() + + +def dump_credentials(credentials): + return { + "credentials": credentials.class_name(), + "credentials_data": credentials.dump(), + } diff --git a/src/middlewared/middlewared/main.py b/src/middlewared/middlewared/main.py index e6d54a6d92695..7855c1cb60ea0 100644 --- a/src/middlewared/middlewared/main.py +++ b/src/middlewared/middlewared/main.py @@ -16,6 +16,7 @@ get_errname, ) from .utils import MIDDLEWARE_RUN_DIR, sw_version +from .utils.audit import audit_username_from_session from .utils.debug import get_frame_details, get_threads_stacks from .utils.lock import SoftHardSemaphore, SoftHardSemaphoreLimit from .utils.nginx import get_remote_addr_port @@ -1547,8 +1548,8 @@ async def log_audit_message(self, app, event, event_data, success): "major": 0, "minor": 1 }, - "addr": "127.0.0.1", - "user": "root", + "addr": app.origin.repr() if isinstance(app.origin, TCPIPOrigin) else "127.0.0.1", + "user": audit_username_from_session(app.authenticated_credentials), "sess": app.session_id, "time": datetime.utcnow().strftime('%Y-%m-%d %H:%M:%S.%f'), "svc": "MIDDLEWARE", diff --git a/src/middlewared/middlewared/plugins/auth.py b/src/middlewared/middlewared/plugins/auth.py index 09a956dcf0e77..d14abc5f37b25 100644 --- a/src/middlewared/middlewared/plugins/auth.py +++ b/src/middlewared/middlewared/plugins/auth.py @@ -9,7 +9,8 @@ from middlewared.auth import (SessionManagerCredentials, UserSessionManagerCredentials, UnixSocketSessionManagerCredentials, LoginPasswordSessionManagerCredentials, - ApiKeySessionManagerCredentials, TrueNasNodeSessionManagerCredentials) + ApiKeySessionManagerCredentials, TrueNasNodeSessionManagerCredentials, + TokenSessionManagerCredentials, dump_credentials) from middlewared.schema import accepts, Any, Bool, Datetime, Dict, Int, List, Password, Patch, Ref, returns, Str from middlewared.service import ( Service, filterable, filterable_returns, filter_list, no_auth_required, no_authz_required, @@ -143,13 +144,6 @@ def _app_on_close(self, app): self.logout(app) -def dump_credentials(credentials): - return { - "credentials": credentials.class_name(), - "credentials_data": credentials.dump(), - } - - class Session: def __init__(self, manager, credentials, app): self.manager = manager @@ -166,44 +160,6 @@ def dump(self): } -class TokenSessionManagerCredentials(SessionManagerCredentials): - def __init__(self, token_manager, token): - root_credentials = token.root_credentials() - - self.token_manager = token_manager - self.token = token - self.is_user_session = root_credentials.is_user_session - if self.is_user_session: - self.user = root_credentials.user - - self.allowlist = root_credentials.allowlist - - def is_valid(self): - return self.token.is_valid() - - def authorize(self, method, resource): - return self.token.parent_credentials.authorize(method, resource) - - def has_role(self, role): - return self.token.parent_credentials.has_role(role) - - def notify_used(self): - self.token.notify_used() - - def logout(self): - self.token_manager.destroy(self.token) - - def dump(self): - data = { - "parent": dump_credentials(self.token.parent_credentials), - } - if self.is_user_session: - data["username"] = self.user["username"] - - return data - - - def is_internal_session(session): if isinstance(session.app.origin, UnixSocketOrigin) and session.app.origin.uid == 0: return True diff --git a/src/middlewared/middlewared/pytest/unit/utils/test_audit.py b/src/middlewared/middlewared/pytest/unit/utils/test_audit.py new file mode 100644 index 0000000000000..d42cfad6a445d --- /dev/null +++ b/src/middlewared/middlewared/pytest/unit/utils/test_audit.py @@ -0,0 +1,29 @@ +import pytest + +from middlewared.auth import ( + ApiKeySessionManagerCredentials, + UserSessionManagerCredentials, + TrueNasNodeSessionManagerCredentials +) + +from middlewared.utils.audit import audit_username_from_session +from types import SimpleNamespace + + +API_KEY = SimpleNamespace(api_key={'id': 1, 'name': 'MY_KEY'}) + +USER_SESSION = UserSessionManagerCredentials({'username': 'bob', 'privilege': {'allowlist': []}}) +API_KEY_SESSION = ApiKeySessionManagerCredentials(API_KEY) +TOKEN_USER_SESSION = SimpleNamespace(root_credentials=USER_SESSION, is_user_session=True, user=USER_SESSION.user) +NODE_SESSION = TrueNasNodeSessionManagerCredentials() + + +@pytest.mark.parametrize('cred,expected', [ + (None, '.UNAUTHENTICATED'), + (USER_SESSION, 'bob'), + (API_KEY_SESSION, '.TRUENAS_API_KEY:MY_KEY'), + (TOKEN_USER_SESSION, 'bob'), + (NODE_SESSION, '.TRUENAS_NODE') +]) +def test_privilege_has_webui_access(cred, expected): + assert audit_username_from_session(cred) == expected diff --git a/src/middlewared/middlewared/utils/audit.py b/src/middlewared/middlewared/utils/audit.py new file mode 100644 index 0000000000000..5117fc3f685f0 --- /dev/null +++ b/src/middlewared/middlewared/utils/audit.py @@ -0,0 +1,33 @@ +from middlewared.auth import ( + ApiKeySessionManagerCredentials, + TokenSessionManagerCredentials, + TrueNasNodeSessionManagerCredentials +) + +# Special values start with dot to ensure they cannot collide with local usernames +# created via APIs +API_KEY_PREFIX = '.TRUENAS_API_KEY:' +NODE_SESSION = '.TRUENAS_NODE' +UNAUTHENTICATED = '.UNAUTHENTICATED' +UNKNOWN_SESSION = '.UNKNOWN' + + +def audit_username_from_session(cred) -> str: + if cred is None: + return UNAUTHENTICATED + + # This works for regular user session and tokens formed on them + if cred.is_user_session: + return cred.user['username'] + + # Track back to root credential if necessary (token session) + if isinstance(cred, TokenSessionManagerCredentials): + cred = cred.root_credentials + + if isinstance(cred, ApiKeySessionManagerCredentials): + return f'{API_KEY_PREFIX}{cred.api_key.api_key["name"]}' + + elif isinstance(cred, TrueNasNodeSessionManagerCredentials): + return NODE_SESSION + + return UNKNOWN_SESSION