From 3dd77dbd7696345970d866ab254e324e2ea95666 Mon Sep 17 00:00:00 2001 From: Frantisek Tobias Date: Thu, 15 Aug 2024 13:41:44 +0200 Subject: [PATCH 1/9] datamodel: file permission checks: Created new types to check if files can be opened --- .../datamodel/types/__init__.py | 4 +- .../datamodel/types/files.py | 43 +++++++++++++++++++ manager/knot_resolver_manager/server.py | 7 +++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/manager/knot_resolver_manager/datamodel/types/__init__.py b/manager/knot_resolver_manager/datamodel/types/__init__.py index 350cf2133..a87c5c7c1 100644 --- a/manager/knot_resolver_manager/datamodel/types/__init__.py +++ b/manager/knot_resolver_manager/datamodel/types/__init__.py @@ -1,5 +1,5 @@ from .enums import DNSRecordTypeEnum, PolicyActionEnum, PolicyFlagEnum -from .files import AbsoluteDir, Dir, File, FilePath +from .files import AbsoluteDir, Dir, File, FilePath, WritableFile, ReadableFile from .generic_types import ListOrItem from .types import ( DomainName, @@ -60,6 +60,8 @@ "SizeUnit", "TimeUnit", "AbsoluteDir", + "ReadableFile", + "WritableFile", "File", "FilePath", "Dir", diff --git a/manager/knot_resolver_manager/datamodel/types/files.py b/manager/knot_resolver_manager/datamodel/types/files.py index 49b51f713..ec2fcca54 100644 --- a/manager/knot_resolver_manager/datamodel/types/files.py +++ b/manager/knot_resolver_manager/datamodel/types/files.py @@ -1,3 +1,5 @@ +from logging import debug +from os import close from pathlib import Path from typing import Any, Dict, Tuple, Type, TypeVar @@ -135,3 +137,44 @@ def __init__( raise ValueError(f"path '{self._value}' does not point inside an existing directory") if self.strict_validation and self._value.is_dir(): raise ValueError(f"path '{self._value}' points to a directory when we expected a file") + + +class ReadableFile(UncheckedPath): + """ + File, that is enforced to be: + - readable by kresd + """ + def __init__( + self, source_value: Any, parents: Tuple["UncheckedPath", ...] = tuple(), object_path: str = "/" + ) -> None: + super().__init__(source_value, parents=parents, object_path=object_path) + try: + f = open(self._value, "r") + except IOError as e: + if e.args == (13, 'permission denied'): + raise ValueError(f"file'{self._value}' isn't readable") + raise ValueError(f"Unexpected error '{e}'") + + f.close() + + +class WritableFile(UncheckedPath): + """ + File, that is enforced to be: + - writable by kresd + """ + def __init__( + self, source_value: Any, parents: Tuple["UncheckedPath", ...] = tuple(), object_path: str = "/" + ) -> None: + print(type(self)) + super().__init__(source_value, parents=parents, object_path=object_path) + try: + f = open(self._value, "w") + except IOError as e: + if e.args == (13, 'permission denied'): + raise ValueError(f"file'{self._value}' isn't readable") + raise ValueError(f"Unexpected error '{e}'") + + f.close() + + diff --git a/manager/knot_resolver_manager/server.py b/manager/knot_resolver_manager/server.py index b27cadb33..b5ebd6c2f 100644 --- a/manager/knot_resolver_manager/server.py +++ b/manager/knot_resolver_manager/server.py @@ -18,6 +18,7 @@ from aiohttp.web_runner import AppRunner, TCPSite, UnixSite from typing_extensions import Literal +from knot_resolver_manager.datamodel.types.files import ReadableFile, WritableFile import knot_resolver_manager.utils.custom_atexit as atexit from knot_resolver_manager import log, statistics from knot_resolver_manager.compat import asyncio as asyncio_compat @@ -508,6 +509,7 @@ async def start_server(config: Path = DEFAULT_MANAGER_CONFIG_FILE) -> int: # This function is quite long, but it describes how manager runs. So let's silence pylint # pylint: disable=too-many-statements + ReadableFile(config) start_time = time() working_directory_on_startup = os.getcwd() manager: Optional[KresManager] = None @@ -586,6 +588,11 @@ async def start_server(config: Path = DEFAULT_MANAGER_CONFIG_FILE) -> int: logger.error(e) return 1 + except PermissionError as e: + logger.error(f"Reading of the configuration file failed: {e}") + # logger.error("Insufficient permissions") + return 1 + except BaseException: logger.error("Uncaught generic exception during manager inicialization...", exc_info=True) return 1 From 5cbc876119b1f9c5ae1e760fc22ae7b31554e6f9 Mon Sep 17 00:00:00 2001 From: Frantisek Tobias Date: Tue, 20 Aug 2024 09:32:38 +0200 Subject: [PATCH 2/9] datamodel: file permission checks: #814 created function to check that kresd_user() can access the files and directories --- manager/knot_resolver_manager/constants.py | 8 ++ .../datamodel/types/__init__.py | 4 +- .../datamodel/types/files.py | 74 +++++++++++++------ manager/knot_resolver_manager/server.py | 3 - python/knot_resolver.py.in | 2 + python/meson.build | 2 + 6 files changed, 65 insertions(+), 28 deletions(-) diff --git a/manager/knot_resolver_manager/constants.py b/manager/knot_resolver_manager/constants.py index 90ceed9f8..9253c2db1 100644 --- a/manager/knot_resolver_manager/constants.py +++ b/manager/knot_resolver_manager/constants.py @@ -35,6 +35,14 @@ def kres_gc_executable() -> Path: return knot_resolver.sbin_dir / "kres-cache-gc" +def kresd_user(): + return None if knot_resolver is None else knot_resolver.user + + +def kresd_group(): + return None if knot_resolver is None else knot_resolver.group + + def kresd_cache_dir(config: "KresConfig") -> Path: return config.cache.storage.to_path() diff --git a/manager/knot_resolver_manager/datamodel/types/__init__.py b/manager/knot_resolver_manager/datamodel/types/__init__.py index a87c5c7c1..52ab1cf8c 100644 --- a/manager/knot_resolver_manager/datamodel/types/__init__.py +++ b/manager/knot_resolver_manager/datamodel/types/__init__.py @@ -1,5 +1,5 @@ from .enums import DNSRecordTypeEnum, PolicyActionEnum, PolicyFlagEnum -from .files import AbsoluteDir, Dir, File, FilePath, WritableFile, ReadableFile +from .files import AbsoluteDir, Dir, File, FilePath, WritableDir, ReadableFile from .generic_types import ListOrItem from .types import ( DomainName, @@ -61,7 +61,7 @@ "TimeUnit", "AbsoluteDir", "ReadableFile", - "WritableFile", + "WritableDir", "File", "FilePath", "Dir", diff --git a/manager/knot_resolver_manager/datamodel/types/files.py b/manager/knot_resolver_manager/datamodel/types/files.py index ec2fcca54..df95a886b 100644 --- a/manager/knot_resolver_manager/datamodel/types/files.py +++ b/manager/knot_resolver_manager/datamodel/types/files.py @@ -1,8 +1,11 @@ -from logging import debug -from os import close from pathlib import Path from typing import Any, Dict, Tuple, Type, TypeVar +import os +import stat +from pwd import getpwnam +from grp import getgrnam +from knot_resolver_manager.constants import kresd_user, kresd_group from knot_resolver_manager.datamodel.globals import get_resolve_root, get_strict_validation from knot_resolver_manager.utils.modeling.base_value_type import BaseValueType @@ -135,11 +138,49 @@ def __init__( p = self._value.parent if self.strict_validation and (not p.exists() or not p.is_dir()): raise ValueError(f"path '{self._value}' does not point inside an existing directory") + + # WARNING: is_dir() fails for knot-resolver owned paths when using kresctl to validate config if self.strict_validation and self._value.is_dir(): raise ValueError(f"path '{self._value}' points to a directory when we expected a file") -class ReadableFile(UncheckedPath): +READ_MODE = 0 +WRITE_MODE = 1 +EXECUTE_MODE = 2 + +def kresd_accesible(dest_path: Path, perm_mode: int) -> bool: + chflags = [ + [stat.S_IRUSR, stat.S_IRGRP, stat.S_IROTH], + [stat.S_IWUSR, stat.S_IWGRP, stat.S_IWOTH], + [stat.S_IXUSR, stat.S_IXGRP, stat.S_IXOTH], + ] + + username = kresd_user() + groupname = kresd_group() + + if username is None or groupname is None: + return True + + user_uid = getpwnam(username).pw_uid + user_gid = getgrnam(groupname).gr_gid + + dest_stat = os.stat(dest_path) + dest_uid = dest_stat.st_uid + dest_gid = dest_stat.st_gid + + _mode = dest_stat.st_mode + + if user_uid == dest_uid: + return bool(_mode & chflags[perm_mode][0]) + + b_groups = os.getgrouplist(os.getlogin(), user_gid) + if user_gid == dest_gid or dest_gid in b_groups: + return bool(_mode & chflags[perm_mode][1]) + + return bool(_mode & chflags[perm_mode][2]) + + +class ReadableFile(File): """ File, that is enforced to be: - readable by kresd @@ -148,33 +189,20 @@ def __init__( self, source_value: Any, parents: Tuple["UncheckedPath", ...] = tuple(), object_path: str = "/" ) -> None: super().__init__(source_value, parents=parents, object_path=object_path) - try: - f = open(self._value, "r") - except IOError as e: - if e.args == (13, 'permission denied'): - raise ValueError(f"file'{self._value}' isn't readable") - raise ValueError(f"Unexpected error '{e}'") - f.close() + if self.strict_validation and not kresd_accesible(self._value, READ_MODE): + raise ValueError(f"{kresd_user()}:{kresd_group()} has insuficient permissions to read \"{self._value}\"") -class WritableFile(UncheckedPath): +class WritableDir(Dir): """ - File, that is enforced to be: - - writable by kresd + Dif, that is enforced to be: + - writable to by kresd """ def __init__( self, source_value: Any, parents: Tuple["UncheckedPath", ...] = tuple(), object_path: str = "/" ) -> None: - print(type(self)) super().__init__(source_value, parents=parents, object_path=object_path) - try: - f = open(self._value, "w") - except IOError as e: - if e.args == (13, 'permission denied'): - raise ValueError(f"file'{self._value}' isn't readable") - raise ValueError(f"Unexpected error '{e}'") - - f.close() - + if self.strict_validation and not kresd_accesible(self._value, WRITE_MODE): + raise ValueError(f"{kresd_user()}:{kresd_group()} has insuficient permissions to write to \"{self._value}\"") diff --git a/manager/knot_resolver_manager/server.py b/manager/knot_resolver_manager/server.py index b5ebd6c2f..b7cd02122 100644 --- a/manager/knot_resolver_manager/server.py +++ b/manager/knot_resolver_manager/server.py @@ -17,8 +17,6 @@ from aiohttp.web_response import json_response from aiohttp.web_runner import AppRunner, TCPSite, UnixSite from typing_extensions import Literal - -from knot_resolver_manager.datamodel.types.files import ReadableFile, WritableFile import knot_resolver_manager.utils.custom_atexit as atexit from knot_resolver_manager import log, statistics from knot_resolver_manager.compat import asyncio as asyncio_compat @@ -509,7 +507,6 @@ async def start_server(config: Path = DEFAULT_MANAGER_CONFIG_FILE) -> int: # This function is quite long, but it describes how manager runs. So let's silence pylint # pylint: disable=too-many-statements - ReadableFile(config) start_time = time() working_directory_on_startup = os.getcwd() manager: Optional[KresManager] = None diff --git a/python/knot_resolver.py.in b/python/knot_resolver.py.in index 262f7a840..e6b2accbd 100644 --- a/python/knot_resolver.py.in +++ b/python/knot_resolver.py.in @@ -8,3 +8,5 @@ etc_dir = Path("@etc_dir@") run_dir = Path("@run_dir@") lib_dir = Path("@lib_dir@") modules_dir = Path("@modules_dir@") +user = "@user@" +group = "@group@" diff --git a/python/meson.build b/python/meson.build index e209df542..c6b2a27ed 100644 --- a/python/meson.build +++ b/python/meson.build @@ -9,6 +9,8 @@ python_config.set('etc_dir', etc_dir) python_config.set('run_dir', run_dir) python_config.set('lib_dir', lib_dir) python_config.set('modules_dir', modules_dir) +python_config.set('user', user) +python_config.set('group', group) configure_file( input: 'knot_resolver.py.in', From 3bcb08e6d4534c085d9fade405f138eb2f34e23c Mon Sep 17 00:00:00 2001 From: Frantisek Tobias Date: Tue, 20 Aug 2024 10:09:03 +0200 Subject: [PATCH 3/9] datamodel: file permission checks: remove left over code, fix linter formatting ignored for now since the code that is to be formated will likely be moved elsewhere --- manager/knot_resolver_manager/datamodel/types/files.py | 1 + manager/knot_resolver_manager/server.py | 5 ----- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/manager/knot_resolver_manager/datamodel/types/files.py b/manager/knot_resolver_manager/datamodel/types/files.py index df95a886b..bcb439a05 100644 --- a/manager/knot_resolver_manager/datamodel/types/files.py +++ b/manager/knot_resolver_manager/datamodel/types/files.py @@ -148,6 +148,7 @@ def __init__( WRITE_MODE = 1 EXECUTE_MODE = 2 + def kresd_accesible(dest_path: Path, perm_mode: int) -> bool: chflags = [ [stat.S_IRUSR, stat.S_IRGRP, stat.S_IROTH], diff --git a/manager/knot_resolver_manager/server.py b/manager/knot_resolver_manager/server.py index b7cd02122..d05ac7b4a 100644 --- a/manager/knot_resolver_manager/server.py +++ b/manager/knot_resolver_manager/server.py @@ -585,11 +585,6 @@ async def start_server(config: Path = DEFAULT_MANAGER_CONFIG_FILE) -> int: logger.error(e) return 1 - except PermissionError as e: - logger.error(f"Reading of the configuration file failed: {e}") - # logger.error("Insufficient permissions") - return 1 - except BaseException: logger.error("Uncaught generic exception during manager inicialization...", exc_info=True) return 1 From 1a3cb860afefa790e5842e5c7936d0730db3e362 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mr=C3=A1zek?= Date: Tue, 20 Aug 2024 12:04:19 +0200 Subject: [PATCH 4/9] scripts/poe-tasks/configure: add user and group --- scripts/poe-tasks/configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/poe-tasks/configure b/scripts/poe-tasks/configure index d205a6dec..ea2bdeb3e 100755 --- a/scripts/poe-tasks/configure +++ b/scripts/poe-tasks/configure @@ -8,6 +8,6 @@ reconfigure='' if [ -f .build_kresd/ninja.build ]; then reconfigure='--reconfigure' fi -meson setup .build_kresd "$reconfigure" --prefix=$(realpath .install_kresd) "$@" +meson setup .build_kresd "$reconfigure" --prefix=$(realpath .install_kresd) -Duser=$USER -Dgroup=$(id -gn) "$@" build_kresd From cdd96e9ad3c2593da2a56f1f07276b0b6fb5fd71 Mon Sep 17 00:00:00 2001 From: Frantisek Tobias Date: Wed, 21 Aug 2024 11:44:14 +0200 Subject: [PATCH 5/9] datamodel: file permission checks: format files --- .../datamodel/types/__init__.py | 2 +- .../datamodel/types/files.py | 15 ++++++++------- manager/knot_resolver_manager/server.py | 1 + 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/manager/knot_resolver_manager/datamodel/types/__init__.py b/manager/knot_resolver_manager/datamodel/types/__init__.py index 52ab1cf8c..26675da3f 100644 --- a/manager/knot_resolver_manager/datamodel/types/__init__.py +++ b/manager/knot_resolver_manager/datamodel/types/__init__.py @@ -1,5 +1,5 @@ from .enums import DNSRecordTypeEnum, PolicyActionEnum, PolicyFlagEnum -from .files import AbsoluteDir, Dir, File, FilePath, WritableDir, ReadableFile +from .files import AbsoluteDir, Dir, File, FilePath, ReadableFile, WritableDir from .generic_types import ListOrItem from .types import ( DomainName, diff --git a/manager/knot_resolver_manager/datamodel/types/files.py b/manager/knot_resolver_manager/datamodel/types/files.py index bcb439a05..98b9d86e4 100644 --- a/manager/knot_resolver_manager/datamodel/types/files.py +++ b/manager/knot_resolver_manager/datamodel/types/files.py @@ -1,11 +1,11 @@ -from pathlib import Path -from typing import Any, Dict, Tuple, Type, TypeVar import os import stat -from pwd import getpwnam from grp import getgrnam +from pathlib import Path +from pwd import getpwnam +from typing import Any, Dict, Tuple, Type, TypeVar -from knot_resolver_manager.constants import kresd_user, kresd_group +from knot_resolver_manager.constants import kresd_group, kresd_user from knot_resolver_manager.datamodel.globals import get_resolve_root, get_strict_validation from knot_resolver_manager.utils.modeling.base_value_type import BaseValueType @@ -139,7 +139,6 @@ def __init__( if self.strict_validation and (not p.exists() or not p.is_dir()): raise ValueError(f"path '{self._value}' does not point inside an existing directory") - # WARNING: is_dir() fails for knot-resolver owned paths when using kresctl to validate config if self.strict_validation and self._value.is_dir(): raise ValueError(f"path '{self._value}' points to a directory when we expected a file") @@ -186,13 +185,14 @@ class ReadableFile(File): File, that is enforced to be: - readable by kresd """ + def __init__( self, source_value: Any, parents: Tuple["UncheckedPath", ...] = tuple(), object_path: str = "/" ) -> None: super().__init__(source_value, parents=parents, object_path=object_path) if self.strict_validation and not kresd_accesible(self._value, READ_MODE): - raise ValueError(f"{kresd_user()}:{kresd_group()} has insuficient permissions to read \"{self._value}\"") + raise ValueError(f'{kresd_user()}:{kresd_group()} has insuficient permissions to read "{self._value}"') class WritableDir(Dir): @@ -200,10 +200,11 @@ class WritableDir(Dir): Dif, that is enforced to be: - writable to by kresd """ + def __init__( self, source_value: Any, parents: Tuple["UncheckedPath", ...] = tuple(), object_path: str = "/" ) -> None: super().__init__(source_value, parents=parents, object_path=object_path) if self.strict_validation and not kresd_accesible(self._value, WRITE_MODE): - raise ValueError(f"{kresd_user()}:{kresd_group()} has insuficient permissions to write to \"{self._value}\"") + raise ValueError(f'{kresd_user()}:{kresd_group()} has insuficient permissions to write to "{self._value}"') diff --git a/manager/knot_resolver_manager/server.py b/manager/knot_resolver_manager/server.py index d05ac7b4a..b27cadb33 100644 --- a/manager/knot_resolver_manager/server.py +++ b/manager/knot_resolver_manager/server.py @@ -17,6 +17,7 @@ from aiohttp.web_response import json_response from aiohttp.web_runner import AppRunner, TCPSite, UnixSite from typing_extensions import Literal + import knot_resolver_manager.utils.custom_atexit as atexit from knot_resolver_manager import log, statistics from knot_resolver_manager.compat import asyncio as asyncio_compat From dc8eef312717afa850404ebee5f45340b055e7a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mr=C3=A1zek?= Date: Mon, 2 Sep 2024 17:51:25 +0200 Subject: [PATCH 6/9] datamodel: types: files: enum for permission mode added --- .../datamodel/types/files.py | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/manager/knot_resolver_manager/datamodel/types/files.py b/manager/knot_resolver_manager/datamodel/types/files.py index 98b9d86e4..41959ea4c 100644 --- a/manager/knot_resolver_manager/datamodel/types/files.py +++ b/manager/knot_resolver_manager/datamodel/types/files.py @@ -1,5 +1,6 @@ import os import stat +from enum import Enum from grp import getgrnam from pathlib import Path from pwd import getpwnam @@ -143,17 +144,18 @@ def __init__( raise ValueError(f"path '{self._value}' points to a directory when we expected a file") -READ_MODE = 0 -WRITE_MODE = 1 -EXECUTE_MODE = 2 +class _PermissionMode(Enum): + READ = 0 + WRITE = 1 + EXECUTE = 2 -def kresd_accesible(dest_path: Path, perm_mode: int) -> bool: - chflags = [ - [stat.S_IRUSR, stat.S_IRGRP, stat.S_IROTH], - [stat.S_IWUSR, stat.S_IWGRP, stat.S_IWOTH], - [stat.S_IXUSR, stat.S_IXGRP, stat.S_IXOTH], - ] +def _kresd_accessible(dest_path: Path, perm_mode: _PermissionMode) -> bool: + chflags = { + _PermissionMode.READ: [stat.S_IRUSR, stat.S_IRGRP, stat.S_IROTH], + _PermissionMode.WRITE: [stat.S_IWUSR, stat.S_IWGRP, stat.S_IWOTH], + _PermissionMode.EXECUTE: [stat.S_IXUSR, stat.S_IXGRP, stat.S_IXOTH], + } username = kresd_user() groupname = kresd_group() @@ -182,7 +184,8 @@ def kresd_accesible(dest_path: Path, perm_mode: int) -> bool: class ReadableFile(File): """ - File, that is enforced to be: + Path, that is enforced to be: + - an existing file - readable by kresd """ @@ -191,14 +194,15 @@ def __init__( ) -> None: super().__init__(source_value, parents=parents, object_path=object_path) - if self.strict_validation and not kresd_accesible(self._value, READ_MODE): - raise ValueError(f'{kresd_user()}:{kresd_group()} has insuficient permissions to read "{self._value}"') + if self.strict_validation and not _kresd_accessible(self._value, _PermissionMode.READ): + raise ValueError(f"{kresd_user()}:{kresd_group()} has insufficient permissions to read '{self._value}'") class WritableDir(Dir): """ - Dif, that is enforced to be: - - writable to by kresd + Path, that is enforced to be: + - an existing directory + - writable by kresd """ def __init__( @@ -206,5 +210,5 @@ def __init__( ) -> None: super().__init__(source_value, parents=parents, object_path=object_path) - if self.strict_validation and not kresd_accesible(self._value, WRITE_MODE): - raise ValueError(f'{kresd_user()}:{kresd_group()} has insuficient permissions to write to "{self._value}"') + if self.strict_validation and not _kresd_accessible(self._value, _PermissionMode.WRITE): + raise ValueError(f"{kresd_user()}:{kresd_group()} has insufficient permissions to write to '{self._value}'") From 4b4567f33736872b70d336a6bba19b0bb3b05870 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mr=C3=A1zek?= Date: Mon, 2 Sep 2024 21:16:31 +0200 Subject: [PATCH 7/9] datamodel: types: files: WritableFilePath added --- .../datamodel/types/__init__.py | 3 +- .../datamodel/types/files.py | 72 +++++++++++++------ 2 files changed, 52 insertions(+), 23 deletions(-) diff --git a/manager/knot_resolver_manager/datamodel/types/__init__.py b/manager/knot_resolver_manager/datamodel/types/__init__.py index 26675da3f..a3d7db3e6 100644 --- a/manager/knot_resolver_manager/datamodel/types/__init__.py +++ b/manager/knot_resolver_manager/datamodel/types/__init__.py @@ -1,5 +1,5 @@ from .enums import DNSRecordTypeEnum, PolicyActionEnum, PolicyFlagEnum -from .files import AbsoluteDir, Dir, File, FilePath, ReadableFile, WritableDir +from .files import AbsoluteDir, Dir, File, FilePath, ReadableFile, WritableDir, WritableFilePath from .generic_types import ListOrItem from .types import ( DomainName, @@ -62,6 +62,7 @@ "AbsoluteDir", "ReadableFile", "WritableDir", + "WritableFilePath", "File", "FilePath", "Dir", diff --git a/manager/knot_resolver_manager/datamodel/types/files.py b/manager/knot_resolver_manager/datamodel/types/files.py index 41959ea4c..a8158e6e4 100644 --- a/manager/knot_resolver_manager/datamodel/types/files.py +++ b/manager/knot_resolver_manager/datamodel/types/files.py @@ -1,6 +1,6 @@ import os import stat -from enum import Enum +from enum import auto, Flag from grp import getgrnam from pathlib import Path from pwd import getpwnam @@ -144,13 +144,13 @@ def __init__( raise ValueError(f"path '{self._value}' points to a directory when we expected a file") -class _PermissionMode(Enum): - READ = 0 - WRITE = 1 - EXECUTE = 2 +class _PermissionMode(Flag): + READ = auto() + WRITE = auto() + EXECUTE = auto() -def _kresd_accessible(dest_path: Path, perm_mode: _PermissionMode) -> bool: +def _kres_accessible(dest_path: Path, perm_mode: _PermissionMode) -> bool: chflags = { _PermissionMode.READ: [stat.S_IRUSR, stat.S_IRGRP, stat.S_IROTH], _PermissionMode.WRITE: [stat.S_IWUSR, stat.S_IWGRP, stat.S_IWOTH], @@ -169,24 +169,27 @@ def _kresd_accessible(dest_path: Path, perm_mode: _PermissionMode) -> bool: dest_stat = os.stat(dest_path) dest_uid = dest_stat.st_uid dest_gid = dest_stat.st_gid - - _mode = dest_stat.st_mode - - if user_uid == dest_uid: - return bool(_mode & chflags[perm_mode][0]) - - b_groups = os.getgrouplist(os.getlogin(), user_gid) - if user_gid == dest_gid or dest_gid in b_groups: - return bool(_mode & chflags[perm_mode][1]) - - return bool(_mode & chflags[perm_mode][2]) + dest_mode = dest_stat.st_mode + + 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) + 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]) + + for perm in perm_mode: + if not accessible(perm): + return False + return True class ReadableFile(File): """ Path, that is enforced to be: - an existing file - - readable by kresd + - readable by knot-resolver processes """ def __init__( @@ -194,7 +197,7 @@ def __init__( ) -> None: super().__init__(source_value, parents=parents, object_path=object_path) - if self.strict_validation and not _kresd_accessible(self._value, _PermissionMode.READ): + if self.strict_validation and not _kres_accessible(self._value, _PermissionMode.READ): raise ValueError(f"{kresd_user()}:{kresd_group()} has insufficient permissions to read '{self._value}'") @@ -202,7 +205,28 @@ class WritableDir(Dir): """ Path, that is enforced to be: - an existing directory - - writable by kresd + - writable/executable by knot-resolver processes + """ + + def __init__( + self, source_value: Any, parents: Tuple["UncheckedPath", ...] = tuple(), object_path: str = "/" + ) -> None: + super().__init__(source_value, parents=parents, object_path=object_path) + + if self.strict_validation and not _kres_accessible( + self._value, _PermissionMode.WRITE | _PermissionMode.EXECUTE + ): + raise ValueError( + f"{kresd_user()}:{kresd_group()} has insufficient permissions to write/execute '{self._value}'" + ) + + +class WritableFilePath(FilePath): + """ + Path, that is enforced to be: + - parent of the last path segment is an existing directory + - it does not point to a dir + - writable/executable parent directory by knot-resolver processes """ def __init__( @@ -210,5 +234,9 @@ def __init__( ) -> None: super().__init__(source_value, parents=parents, object_path=object_path) - if self.strict_validation and not _kresd_accessible(self._value, _PermissionMode.WRITE): - raise ValueError(f"{kresd_user()}:{kresd_group()} has insufficient permissions to write to '{self._value}'") + if self.strict_validation and not _kres_accessible( + self._value.parent, _PermissionMode.WRITE | _PermissionMode.EXECUTE + ): + raise ValueError( + f"{kresd_user()}:{kresd_group()} has insufficient permissions to write/execute'{self._value.parent}'" + ) From 0c3e5f36c5a6f5194b12da7d3c8d6503d84f1578 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mr=C3=A1zek?= Date: Mon, 2 Sep 2024 21:18:05 +0200 Subject: [PATCH 8/9] datamodel: use permission types in config --- .../datamodel/cache_schema.py | 8 ++++---- .../datamodel/config_schema.py | 10 +++++----- .../datamodel/dnssec_schema.py | 4 ++-- .../datamodel/forward_schema.py | 4 ++-- .../datamodel/local_data_schema.py | 10 +++++----- .../datamodel/logging_schema.py | 4 ++-- .../knot_resolver_manager/datamodel/lua_schema.py | 4 ++-- .../datamodel/management_schema.py | 4 ++-- .../datamodel/network_schema.py | 14 +++++++------- .../knot_resolver_manager/datamodel/rpz_schema.py | 4 ++-- .../datamodel/static_hints_schema.py | 6 +++--- .../datamodel/webmgmt_schema.py | 8 ++++---- 12 files changed, 40 insertions(+), 40 deletions(-) diff --git a/manager/knot_resolver_manager/datamodel/cache_schema.py b/manager/knot_resolver_manager/datamodel/cache_schema.py index ac30f0d01..4ed9fc3a8 100644 --- a/manager/knot_resolver_manager/datamodel/cache_schema.py +++ b/manager/knot_resolver_manager/datamodel/cache_schema.py @@ -4,16 +4,16 @@ from knot_resolver_manager.datamodel.templates import template_from_str from knot_resolver_manager.datamodel.types import ( - Dir, DNSRecordTypeEnum, DomainName, EscapedStr, - File, IntNonNegative, IntPositive, Percent, + ReadableFile, SizeUnit, TimeUnit, + WritableDir, ) from knot_resolver_manager.utils.modeling import ConfigSchema from knot_resolver_manager.utils.modeling.base_schema import lazy_default @@ -51,7 +51,7 @@ class PrefillSchema(ConfigSchema): origin: DomainName url: EscapedStr refresh_interval: TimeUnit = TimeUnit("1d") - ca_file: Optional[File] = None + ca_file: Optional[ReadableFile] = None def _validate(self) -> None: if str(self.origin) != ".": @@ -125,7 +125,7 @@ class CacheSchema(ConfigSchema): prefetch: These options help keep the cache hot by prefetching expiring records or learning usage patterns and repetitive queries. """ - storage: Dir = lazy_default(Dir, "/var/cache/knot-resolver") + storage: WritableDir = lazy_default(WritableDir, "/var/cache/knot-resolver") size_max: SizeUnit = SizeUnit("100M") garbage_collector: Union[GarbageCollectorSchema, Literal[False]] = GarbageCollectorSchema() ttl_min: TimeUnit = TimeUnit("5s") diff --git a/manager/knot_resolver_manager/datamodel/config_schema.py b/manager/knot_resolver_manager/datamodel/config_schema.py index 353712310..c8398569e 100644 --- a/manager/knot_resolver_manager/datamodel/config_schema.py +++ b/manager/knot_resolver_manager/datamodel/config_schema.py @@ -18,7 +18,7 @@ from knot_resolver_manager.datamodel.network_schema import NetworkSchema from knot_resolver_manager.datamodel.options_schema import OptionsSchema from knot_resolver_manager.datamodel.templates import POLICY_CONFIG_TEMPLATE, WORKER_CONFIG_TEMPLATE -from knot_resolver_manager.datamodel.types import Dir, EscapedStr, IntPositive +from knot_resolver_manager.datamodel.types import EscapedStr, IntPositive, WritableDir from knot_resolver_manager.datamodel.view_schema import ViewSchema from knot_resolver_manager.datamodel.webmgmt_schema import WebmgmtSchema from knot_resolver_manager.utils.modeling import ConfigSchema @@ -114,7 +114,7 @@ class Raw(ConfigSchema): version: int = 1 nsid: Optional[EscapedStr] = None hostname: Optional[EscapedStr] = None - rundir: Dir = lazy_default(Dir, _DEFAULT_RUNDIR) + rundir: WritableDir = lazy_default(WritableDir, _DEFAULT_RUNDIR) workers: Union[Literal["auto"], IntPositive] = IntPositive(1) max_workers: IntPositive = IntPositive(_default_max_worker_count()) management: ManagementSchema = lazy_default(ManagementSchema, {"unix-socket": DEFAULT_MANAGER_API_SOCK}) @@ -135,7 +135,7 @@ class Raw(ConfigSchema): nsid: Optional[EscapedStr] hostname: EscapedStr - rundir: Dir + rundir: WritableDir workers: IntPositive max_workers: IntPositive management: ManagementSchema @@ -231,7 +231,7 @@ def render_lua_policy(self) -> str: return POLICY_CONFIG_TEMPLATE.render(cfg=self, cwd=os.getcwd()) -def get_rundir_without_validation(data: Dict[str, Any]) -> Dir: +def get_rundir_without_validation(data: Dict[str, Any]) -> WritableDir: """ Without fully parsing, try to get a rundir from a raw config data, otherwise use default. Attempts a dir validation to produce a good error message. @@ -239,4 +239,4 @@ def get_rundir_without_validation(data: Dict[str, Any]) -> Dir: Used for initial manager startup. """ - return Dir(data["rundir"] if "rundir" in data else _DEFAULT_RUNDIR, object_path="/rundir") + return WritableDir(data["rundir"] if "rundir" in data else _DEFAULT_RUNDIR, object_path="/rundir") diff --git a/manager/knot_resolver_manager/datamodel/dnssec_schema.py b/manager/knot_resolver_manager/datamodel/dnssec_schema.py index 5e274c9a9..e51500e18 100644 --- a/manager/knot_resolver_manager/datamodel/dnssec_schema.py +++ b/manager/knot_resolver_manager/datamodel/dnssec_schema.py @@ -1,6 +1,6 @@ from typing import List, Optional -from knot_resolver_manager.datamodel.types import DomainName, EscapedStr, File, IntNonNegative, TimeUnit +from knot_resolver_manager.datamodel.types import DomainName, EscapedStr, IntNonNegative, ReadableFile, TimeUnit from knot_resolver_manager.utils.modeling import ConfigSchema @@ -14,7 +14,7 @@ class TrustAnchorFileSchema(ConfigSchema): """ - file: File + file: ReadableFile read_only: bool = False diff --git a/manager/knot_resolver_manager/datamodel/forward_schema.py b/manager/knot_resolver_manager/datamodel/forward_schema.py index ee5206c27..52a05f36d 100644 --- a/manager/knot_resolver_manager/datamodel/forward_schema.py +++ b/manager/knot_resolver_manager/datamodel/forward_schema.py @@ -2,7 +2,7 @@ from typing_extensions import Literal -from knot_resolver_manager.datamodel.types import DomainName, File, IPAddressOptionalPort, ListOrItem, PinSha256 +from knot_resolver_manager.datamodel.types import DomainName, IPAddressOptionalPort, ListOrItem, PinSha256, ReadableFile from knot_resolver_manager.utils.modeling import ConfigSchema @@ -22,7 +22,7 @@ class ForwardServerSchema(ConfigSchema): transport: Optional[Literal["tls"]] = None pin_sha256: Optional[ListOrItem[PinSha256]] = None hostname: Optional[DomainName] = None - ca_file: Optional[File] = None + ca_file: Optional[ReadableFile] = None def _validate(self) -> None: if self.pin_sha256 and (self.hostname or self.ca_file): diff --git a/manager/knot_resolver_manager/datamodel/local_data_schema.py b/manager/knot_resolver_manager/datamodel/local_data_schema.py index e891601ce..fafa7ebe2 100644 --- a/manager/knot_resolver_manager/datamodel/local_data_schema.py +++ b/manager/knot_resolver_manager/datamodel/local_data_schema.py @@ -5,10 +5,10 @@ from knot_resolver_manager.datamodel.types import ( DomainName, EscapedStr, - File, IDPattern, IPAddress, ListOrItem, + ReadableFile, TimeUnit, ) from knot_resolver_manager.utils.modeling import ConfigSchema @@ -32,7 +32,7 @@ class RuleSchema(ConfigSchema): name: Optional[ListOrItem[DomainName]] = None subtree: Optional[Literal["empty", "nxdomain", "redirect"]] = None address: Optional[ListOrItem[IPAddress]] = None - file: Optional[ListOrItem[File]] = None + file: Optional[ListOrItem[ReadableFile]] = None records: Optional[EscapedStr] = None tags: Optional[List[IDPattern]] = None ttl: Optional[TimeUnit] = None @@ -64,7 +64,7 @@ class RPZSchema(ConfigSchema): tags: Tags to link with other policy rules. """ - file: File + file: ReadableFile tags: Optional[List[IDPattern]] = None @@ -87,9 +87,9 @@ class LocalDataSchema(ConfigSchema): ttl: Optional[TimeUnit] = None nodata: bool = True root_fallback_addresses: Optional[Dict[DomainName, ListOrItem[IPAddress]]] = None - root_fallback_addresses_files: Optional[List[File]] = None + root_fallback_addresses_files: Optional[List[ReadableFile]] = None addresses: Optional[Dict[DomainName, ListOrItem[IPAddress]]] = None - addresses_files: Optional[List[File]] = None + addresses_files: Optional[List[ReadableFile]] = None records: Optional[EscapedStr] = None rules: Optional[List[RuleSchema]] = None rpz: Optional[List[RPZSchema]] = None diff --git a/manager/knot_resolver_manager/datamodel/logging_schema.py b/manager/knot_resolver_manager/datamodel/logging_schema.py index d2b7b7e7a..601cd4a54 100644 --- a/manager/knot_resolver_manager/datamodel/logging_schema.py +++ b/manager/knot_resolver_manager/datamodel/logging_schema.py @@ -3,7 +3,7 @@ from typing_extensions import Literal -from knot_resolver_manager.datamodel.types import FilePath, TimeUnit +from knot_resolver_manager.datamodel.types import TimeUnit, WritableFilePath from knot_resolver_manager.utils.modeling import ConfigSchema from knot_resolver_manager.utils.modeling.base_schema import is_obj_type_valid @@ -84,7 +84,7 @@ class DnstapSchema(ConfigSchema): log_tcp_rtt: Log TCP RTT (Round-trip time). """ - unix_socket: FilePath + unix_socket: WritableFilePath log_queries: bool = True log_responses: bool = True log_tcp_rtt: bool = True diff --git a/manager/knot_resolver_manager/datamodel/lua_schema.py b/manager/knot_resolver_manager/datamodel/lua_schema.py index cf49b7124..079333ae8 100644 --- a/manager/knot_resolver_manager/datamodel/lua_schema.py +++ b/manager/knot_resolver_manager/datamodel/lua_schema.py @@ -1,6 +1,6 @@ from typing import Optional -from knot_resolver_manager.datamodel.types import File +from knot_resolver_manager.datamodel.types import ReadableFile from knot_resolver_manager.utils.modeling import ConfigSchema @@ -16,7 +16,7 @@ class LuaSchema(ConfigSchema): script_only: bool = False script: Optional[str] = None - script_file: Optional[File] = None + script_file: Optional[ReadableFile] = None def _validate(self) -> None: if self.script and self.script_file: diff --git a/manager/knot_resolver_manager/datamodel/management_schema.py b/manager/knot_resolver_manager/datamodel/management_schema.py index 09daa3ff3..44f8f3e83 100644 --- a/manager/knot_resolver_manager/datamodel/management_schema.py +++ b/manager/knot_resolver_manager/datamodel/management_schema.py @@ -1,6 +1,6 @@ from typing import Optional -from knot_resolver_manager.datamodel.types import FilePath, IPAddressPort +from knot_resolver_manager.datamodel.types import WritableFilePath, IPAddressPort from knot_resolver_manager.utils.modeling import ConfigSchema @@ -13,7 +13,7 @@ class ManagementSchema(ConfigSchema): interface: IP address and port number to listen to. """ - unix_socket: Optional[FilePath] = None + unix_socket: Optional[WritableFilePath] = None interface: Optional[IPAddressPort] = None def _validate(self) -> None: diff --git a/manager/knot_resolver_manager/datamodel/network_schema.py b/manager/knot_resolver_manager/datamodel/network_schema.py index 289104b82..b9a35090f 100644 --- a/manager/knot_resolver_manager/datamodel/network_schema.py +++ b/manager/knot_resolver_manager/datamodel/network_schema.py @@ -4,8 +4,7 @@ from knot_resolver_manager.datamodel.types import ( EscapedStr32B, - File, - FilePath, + WritableFilePath, Int0_512, Int0_65535, InterfaceOptionalPort, @@ -16,6 +15,7 @@ IPv6Address, ListOrItem, PortNumber, + ReadableFile, SizeUnit, ) from knot_resolver_manager.utils.modeling import ConfigSchema @@ -62,10 +62,10 @@ class TLSSchema(ConfigSchema): padding: EDNS(0) padding of queries and answers sent over an encrypted channel. """ - cert_file: Optional[File] = None - key_file: Optional[File] = None + cert_file: Optional[ReadableFile] = None + key_file: Optional[ReadableFile] = None sticket_secret: Optional[EscapedStr32B] = None - sticket_secret_file: Optional[File] = None + sticket_secret_file: Optional[ReadableFile] = None auto_discovery: bool = False padding: Union[bool, Int0_512] = True @@ -88,7 +88,7 @@ class Raw(ConfigSchema): """ interface: Optional[ListOrItem[InterfaceOptionalPort]] = None - unix_socket: Optional[ListOrItem[FilePath]] = None + unix_socket: Optional[ListOrItem[WritableFilePath]] = None port: Optional[PortNumber] = None kind: KindEnum = "dns" freebind: bool = False @@ -96,7 +96,7 @@ class Raw(ConfigSchema): _LAYER = Raw interface: Optional[ListOrItem[InterfaceOptionalPort]] - unix_socket: Optional[ListOrItem[FilePath]] + unix_socket: Optional[ListOrItem[WritableFilePath]] port: Optional[PortNumber] kind: KindEnum freebind: bool diff --git a/manager/knot_resolver_manager/datamodel/rpz_schema.py b/manager/knot_resolver_manager/datamodel/rpz_schema.py index 633e34a5b..bf98bd0ca 100644 --- a/manager/knot_resolver_manager/datamodel/rpz_schema.py +++ b/manager/knot_resolver_manager/datamodel/rpz_schema.py @@ -1,6 +1,6 @@ from typing import List, Optional -from knot_resolver_manager.datamodel.types import File, PolicyActionEnum, PolicyFlagEnum +from knot_resolver_manager.datamodel.types import PolicyActionEnum, PolicyFlagEnum, ReadableFile from knot_resolver_manager.utils.modeling import ConfigSchema @@ -18,7 +18,7 @@ class RPZSchema(ConfigSchema): """ action: PolicyActionEnum - file: File + file: ReadableFile watch: bool = True views: Optional[List[str]] = None options: Optional[List[PolicyFlagEnum]] = None diff --git a/manager/knot_resolver_manager/datamodel/static_hints_schema.py b/manager/knot_resolver_manager/datamodel/static_hints_schema.py index 7d39fcf40..89db49bbb 100644 --- a/manager/knot_resolver_manager/datamodel/static_hints_schema.py +++ b/manager/knot_resolver_manager/datamodel/static_hints_schema.py @@ -1,6 +1,6 @@ from typing import Dict, List, Optional -from knot_resolver_manager.datamodel.types import DomainName, File, IPAddress, TimeUnit +from knot_resolver_manager.datamodel.types import DomainName, IPAddress, ReadableFile, TimeUnit from knot_resolver_manager.utils.modeling import ConfigSchema @@ -22,6 +22,6 @@ class StaticHintsSchema(ConfigSchema): nodata: bool = True etc_hosts: bool = False root_hints: Optional[Dict[DomainName, List[IPAddress]]] = None - root_hints_file: Optional[File] = None + root_hints_file: Optional[ReadableFile] = None hints: Optional[Dict[DomainName, List[IPAddress]]] = None - hints_files: Optional[List[File]] = None + hints_files: Optional[List[ReadableFile]] = None diff --git a/manager/knot_resolver_manager/datamodel/webmgmt_schema.py b/manager/knot_resolver_manager/datamodel/webmgmt_schema.py index 41cc33877..2e75c3b74 100644 --- a/manager/knot_resolver_manager/datamodel/webmgmt_schema.py +++ b/manager/knot_resolver_manager/datamodel/webmgmt_schema.py @@ -1,6 +1,6 @@ from typing import Optional -from knot_resolver_manager.datamodel.types import File, FilePath, InterfacePort +from knot_resolver_manager.datamodel.types import WritableFilePath, InterfacePort, ReadableFile from knot_resolver_manager.utils.modeling import ConfigSchema @@ -16,11 +16,11 @@ class WebmgmtSchema(ConfigSchema): key_file: Path to certificate key. """ - unix_socket: Optional[FilePath] = None + unix_socket: Optional[WritableFilePath] = None interface: Optional[InterfacePort] = None tls: bool = False - cert_file: Optional[File] = None - key_file: Optional[File] = None + cert_file: Optional[ReadableFile] = None + key_file: Optional[ReadableFile] = None def _validate(self) -> None: if bool(self.unix_socket) == bool(self.interface): From 244c4ae13e87cd55ada24dd880e08ff38fc0d807 Mon Sep 17 00:00:00 2001 From: Frantisek Tobias Date: Wed, 4 Sep 2024 13:51:56 +0200 Subject: [PATCH 9/9] datamodel: types: fix object iteration compatibility for Python < 3.11 --- manager/knot_resolver_manager/datamodel/types/files.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/manager/knot_resolver_manager/datamodel/types/files.py b/manager/knot_resolver_manager/datamodel/types/files.py index a8158e6e4..4c6e7186f 100644 --- a/manager/knot_resolver_manager/datamodel/types/files.py +++ b/manager/knot_resolver_manager/datamodel/types/files.py @@ -179,9 +179,12 @@ def accessible(perm: _PermissionMode) -> bool: return bool(dest_mode & chflags[perm][1]) return bool(dest_mode & chflags[perm][2]) - for perm in perm_mode: - if not accessible(perm): - return False + # __iter__ for class enum.Flag added in python3.11 + # 'for perm in perm_mode:' failes for <=python3.11 + for perm in _PermissionMode: + if perm in perm_mode: + if not accessible(perm): + return False return True