Skip to content

Commit

Permalink
Improve auditing for middleware methods
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
anodos325 committed Aug 7, 2024
1 parent 292788c commit cf28ec8
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 48 deletions.
44 changes: 44 additions & 0 deletions src/middlewared/middlewared/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -94,3 +131,10 @@ class FakeApplication:

def fake_app():
return FakeApplication()


def dump_credentials(credentials):
return {
"credentials": credentials.class_name(),
"credentials_data": credentials.dump(),
}
5 changes: 3 additions & 2 deletions src/middlewared/middlewared/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down
48 changes: 2 additions & 46 deletions src/middlewared/middlewared/plugins/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
31 changes: 31 additions & 0 deletions src/middlewared/middlewared/pytest/unit/utils/test_audit.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
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)
TOKEN_API_KEY_SESSION = SimpleNamespace(root_credentials=API_KEY_SESSION, is_user_session=False)
NODE_SESSION = TrueNasNodeSessionManagerCredentials()


@pytest.mark.parametrize('cred,expected', [
(None, '.TRUENAS_INTERNAL'),
(USER_SESSION, 'bob'),
(API_KEY_SESSION, '.TRUENAS_API_KEY:MY_KEY'),
(TOKEN_USER_SESSION, 'bob'),
(TOKEN_API_KEY_SESSION, '.TRUENAS_API_KEY:MY_KEY'),
(NODE_SESSION, '.TRUENAS_NODE')
])
def test_privilege_has_webui_access(cred, expected):
assert audit_username_from_session(cred) == expected
33 changes: 33 additions & 0 deletions src/middlewared/middlewared/utils/audit.py
Original file line number Diff line number Diff line change
@@ -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
INTERNAL_SESSION = '.TRUENAS_INTERNAL'
API_KEY_PREFIX = '.TRUENAS_API_KEY:'
NODE_SESSION = '.TRUENAS_NODE'
UNKNOWN_SESSION = '.UNKNOWN'


def audit_username_from_session(cred) -> str:
if cred is None:
return INTERNAL_SESSION

# 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

0 comments on commit cf28ec8

Please sign in to comment.