From 6eaec885515fb62a05a8fc736c9beddd0e39301e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mr=C3=A1zek?= Date: Mon, 23 Sep 2024 17:57:26 +0200 Subject: [PATCH 1/3] datamodel: types: files: improvements - compare intended uid with current working uid - check permissions for current user and group - use os.getuid() and pwd.getpwuid() instead of os.getlogin() #919 --- python/knot_resolver/datamodel/types/files.py | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/python/knot_resolver/datamodel/types/files.py b/python/knot_resolver/datamodel/types/files.py index c29627296..21f6d3ad8 100644 --- a/python/knot_resolver/datamodel/types/files.py +++ b/python/knot_resolver/datamodel/types/files.py @@ -1,15 +1,18 @@ +import logging import os import stat from enum import Flag, auto from grp import getgrnam from pathlib import Path -from pwd import getpwnam +from pwd import getpwnam, getpwuid from typing import Any, Dict, Tuple, Type, TypeVar from knot_resolver.constants import GROUP, USER from knot_resolver.datamodel.globals import get_resolve_root, get_strict_validation from knot_resolver.utils.modeling.base_value_type import BaseValueType +logger = logging.Logger(__name__) + class UncheckedPath(BaseValueType): """ @@ -157,9 +160,24 @@ def _kres_accessible(dest_path: Path, perm_mode: _PermissionMode) -> bool: _PermissionMode.EXECUTE: [stat.S_IXUSR, stat.S_IXGRP, stat.S_IXOTH], } + # process working user id + pwuid = os.getuid() + pwgid = os.getgid() + + # defaults user_uid = getpwnam(USER).pw_uid user_gid = getgrnam(GROUP).gr_gid + # if current user do not match intended user + # log warning message and check permissions for current user running the manager + if pwuid != user_uid: + logger.warning( + f"Knot Resolver does not run under the intended '{USER}' user, '{getpwuid(pwuid).pw_name}' instead." + " This may or may not affect the configuration validation and the proper functioning of the resolver." + ) + user_uid = pwuid + user_gid = pwgid + dest_stat = os.stat(dest_path) dest_uid = dest_stat.st_uid dest_gid = dest_stat.st_gid @@ -168,13 +186,13 @@ def _kres_accessible(dest_path: Path, perm_mode: _PermissionMode) -> bool: def accessible(perm: _PermissionMode) -> bool: if user_uid == dest_uid: return bool(dest_mode & chflags[perm][0]) - b_groups = os.getgrouplist(os.getlogin(), user_gid) + b_groups = os.getgrouplist(getpwuid(pwuid).pw_name, user_gid) if user_gid == dest_gid or dest_gid in b_groups: return bool(dest_mode & chflags[perm][1]) return bool(dest_mode & chflags[perm][2]) # __iter__ for class enum.Flag added in python3.11 - # 'for perm in perm_mode:' failes for <=python3.11 + # 'for perm in perm_mode:' fails for <=python3.11 for perm in _PermissionMode: if perm in perm_mode: if not accessible(perm): From b7f38aa6e937714561aed9c82531fdc2a3665a7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mr=C3=A1zek?= Date: Tue, 24 Sep 2024 10:08:37 +0200 Subject: [PATCH 2/3] python: manager: check the current user against the default constant when the server starts --- python/knot_resolver/manager/server.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/python/knot_resolver/manager/server.py b/python/knot_resolver/manager/server.py index 972b167fb..059e45de0 100644 --- a/python/knot_resolver/manager/server.py +++ b/python/knot_resolver/manager/server.py @@ -8,6 +8,7 @@ from functools import partial from http import HTTPStatus from pathlib import Path +from pwd import getpwuid from time import time from typing import Any, Dict, List, Literal, Optional, Set, Union, cast @@ -17,7 +18,7 @@ from aiohttp.web_response import json_response from aiohttp.web_runner import AppRunner, TCPSite, UnixSite -from knot_resolver.constants import CONFIG_FILE +from knot_resolver.constants import CONFIG_FILE, USER from knot_resolver.controller import get_best_controller_implementation from knot_resolver.controller.exceptions import SubprocessControllerExecException from knot_resolver.controller.registered_workers import command_single_registered_worker @@ -517,6 +518,14 @@ async def start_server(config: Path = CONFIG_FILE) -> int: # Block signals during initialization to force their processing once everything is ready signal.pthread_sigmask(signal.SIG_BLOCK, Server.all_handled_signals()) + # Check if we are running under the intended user, if not, log a warning message + pw_username = getpwuid(os.getuid()).pw_name + if pw_username != USER: + logger.warning( + f"Knot Resolver does not run as the default '{USER}' user, but as '{pw_username}' instead." + " This may or may not affect the configuration validation and the proper functioning of the resolver." + ) + # before starting server, initialize the subprocess controller, config store, etc. Any errors during inicialization # are fatal try: From e28a72081c52ace225cf1164d267f35cf57f3929 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mr=C3=A1zek?= Date: Tue, 24 Sep 2024 10:40:45 +0200 Subject: [PATCH 3/3] python: datamodel: added permissions_default to global validation context It is used to change the check of dirs/files permissions against the default constant user:group or the current user the process is running as. --- python/knot_resolver/datamodel/globals.py | 10 +++++- python/knot_resolver/datamodel/types/files.py | 32 ++++++------------- python/knot_resolver/manager/server.py | 6 ++-- 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/python/knot_resolver/datamodel/globals.py b/python/knot_resolver/datamodel/globals.py index 88f95c2a4..044cf475b 100644 --- a/python/knot_resolver/datamodel/globals.py +++ b/python/knot_resolver/datamodel/globals.py @@ -24,10 +24,14 @@ class Context: resolve_root: Optional[Path] strict_validation: bool + permissions_default: bool - def __init__(self, resolve_root: Optional[Path], strict_validation: bool = True) -> None: + def __init__( + self, resolve_root: Optional[Path], strict_validation: bool = True, permissions_default: bool = True + ) -> None: self.resolve_root = resolve_root self.strict_validation = strict_validation + self.permissions_default = permissions_default _global_context: Context = Context(None) @@ -59,3 +63,7 @@ def get_resolve_root() -> Path: def get_strict_validation() -> bool: return _global_context.strict_validation + + +def get_permissions_default() -> bool: + return _global_context.permissions_default diff --git a/python/knot_resolver/datamodel/types/files.py b/python/knot_resolver/datamodel/types/files.py index 21f6d3ad8..2d22d075a 100644 --- a/python/knot_resolver/datamodel/types/files.py +++ b/python/knot_resolver/datamodel/types/files.py @@ -1,4 +1,3 @@ -import logging import os import stat from enum import Flag, auto @@ -8,11 +7,9 @@ from typing import Any, Dict, Tuple, Type, TypeVar from knot_resolver.constants import GROUP, USER -from knot_resolver.datamodel.globals import get_resolve_root, get_strict_validation +from knot_resolver.datamodel.globals import get_permissions_default, get_resolve_root, get_strict_validation from knot_resolver.utils.modeling.base_value_type import BaseValueType -logger = logging.Logger(__name__) - class UncheckedPath(BaseValueType): """ @@ -160,23 +157,14 @@ def _kres_accessible(dest_path: Path, perm_mode: _PermissionMode) -> bool: _PermissionMode.EXECUTE: [stat.S_IXUSR, stat.S_IXGRP, stat.S_IXOTH], } - # process working user id - pwuid = os.getuid() - pwgid = os.getgid() - - # defaults - user_uid = getpwnam(USER).pw_uid - user_gid = getgrnam(GROUP).gr_gid - - # if current user do not match intended user - # log warning message and check permissions for current user running the manager - if pwuid != user_uid: - logger.warning( - f"Knot Resolver does not run under the intended '{USER}' user, '{getpwuid(pwuid).pw_name}' instead." - " This may or may not affect the configuration validation and the proper functioning of the resolver." - ) - user_uid = pwuid - user_gid = pwgid + if get_permissions_default(): + user_uid = getpwnam(USER).pw_uid + user_gid = getgrnam(GROUP).gr_gid + username = USER + else: + user_uid = os.getuid() + user_gid = os.getgid() + username = getpwuid(user_uid).pw_name dest_stat = os.stat(dest_path) dest_uid = dest_stat.st_uid @@ -186,7 +174,7 @@ def _kres_accessible(dest_path: Path, perm_mode: _PermissionMode) -> bool: def accessible(perm: _PermissionMode) -> bool: if user_uid == dest_uid: return bool(dest_mode & chflags[perm][0]) - b_groups = os.getgrouplist(getpwuid(pwuid).pw_name, user_gid) + b_groups = os.getgrouplist(username, user_gid) if user_gid == dest_gid or dest_gid in b_groups: return bool(dest_mode & chflags[perm][1]) return bool(dest_mode & chflags[perm][2]) diff --git a/python/knot_resolver/manager/server.py b/python/knot_resolver/manager/server.py index 059e45de0..18a2aebc2 100644 --- a/python/knot_resolver/manager/server.py +++ b/python/knot_resolver/manager/server.py @@ -536,8 +536,10 @@ async def start_server(config: Path = CONFIG_FILE) -> int: config_raw = await _load_raw_config(config) # before processing any configuration, set validation context - # - resolve_root = root against which all relative paths will be resolved - set_global_validation_context(Context(config.parent, True)) + # - resolve_root: root against which all relative paths will be resolved + # - strict_validation: check for path existence during configuration validation + # - permissions_default: validate dirs/files rwx permissions against default user:group in constants + set_global_validation_context(Context(config.parent, True, False)) # We want to change cwd as soon as possible. Some parts of the codebase are using os.getcwd() to get the # working directory.