From 86c1087dd660c3bf50107641e3e1f4df8a9b3ab1 Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Wed, 29 Nov 2023 17:19:47 +0100 Subject: [PATCH] configd - implement optional trustmodel and add extended logging, closes https://github.com/opnsense/core/issues/6647 Use socket.LOCAL_PEERCRED to fetch the callers credentials so we are able to log system (shell) users calling our configuration engine. Messages are send to our Audit log using severity informational (action succeeded) or error (not allowed or unknown action), this needs a small change in our syslog template to exclude the audit messages (included in this commit). While here, also add a general overwrite for settings that should apply for all actions, as this would ease applying future default restrictions for all actions. Action defaults can be set in configd.conf using the following construct: [action_defaults] allowed_groups = wheel To require group membership, the `allowed_groups` option is added to the action, when set, the connected user should be a member to at least one of the mentioned groups. For example, to require wheel membership for a call "echo", the configuration might look like this: [echo] command:echo parameters:%s type:script_output allowed_groups = wheel Finally, remove the simulation mode for the configd service as this is less useful nowadays. --- src/opnsense/service/configd.py | 29 ++--- src/opnsense/service/modules/__init__.py | 7 ++ src/opnsense/service/modules/actions/base.py | 25 ++++ .../service/modules/processhandler.py | 112 +++++++++--------- src/opnsense/service/modules/session.py | 72 +++++++++++ .../OPNsense/Syslog/local/configd.conf | 2 +- src/opnsense/service/tests/core.py | 22 ++-- 7 files changed, 190 insertions(+), 79 deletions(-) create mode 100644 src/opnsense/service/modules/session.py diff --git a/src/opnsense/service/configd.py b/src/opnsense/service/configd.py index 57a035392..944a1333f 100755 --- a/src/opnsense/service/configd.py +++ b/src/opnsense/service/configd.py @@ -72,7 +72,7 @@ def validate_config(cnf): sys.exit(0) -def main(cnf, simulate=False, single_threaded=False): +def main(cnf, single_threaded=False): """ configd startup :param cnf: config handle :param simulate: simulate only @@ -86,17 +86,18 @@ def main(cnf, simulate=False, single_threaded=False): for envKey in cnf.items('environment'): config_environment[envKey[0]] = envKey[1] + action_defaults = dict() + if cnf.has_section('action_defaults'): + for envKey in cnf.items('action_defaults'): + action_defaults[envKey[0]] = envKey[1] + # run process coordinator ( on console or as daemon ) - # if command-line arguments contain "emulate", start in emulation mode - if simulate: - proc_handler = modules.processhandler.Handler(socket_filename=cnf.get('main', 'socket_filename'), - config_path='%s/conf' % program_path, - config_environment=config_environment, - simulation_mode=True) - else: - proc_handler = modules.processhandler.Handler(socket_filename=cnf.get('main', 'socket_filename'), - config_path='%s/conf' % program_path, - config_environment=config_environment) + proc_handler = modules.processhandler.Handler( + socket_filename=cnf.get('main', 'socket_filename'), + config_path='%s/conf' % program_path, + config_environment=config_environment, + action_defaults=action_defaults, + ) proc_handler.single_threaded = single_threaded proc_handler.run() @@ -135,11 +136,7 @@ if len(sys.argv) > 1 and 'console' in sys.argv[1:]: profile = cProfile.Profile(subcalls=True) profile.enable() try: - if len(sys.argv) > 1 and 'simulate' in sys.argv[1:]: - print('simulate calls.') - main(cnf=this_config, simulate=True, single_threaded=True) - else: - main(cnf=this_config, single_threaded=True) + main(cnf=this_config, single_threaded=True) except KeyboardInterrupt: pass except: diff --git a/src/opnsense/service/modules/__init__.py b/src/opnsense/service/modules/__init__.py index 1ea2e8d09..1e0e40618 100644 --- a/src/opnsense/service/modules/__init__.py +++ b/src/opnsense/service/modules/__init__.py @@ -60,3 +60,10 @@ def syslog_info(message): def syslog_error(message): emit_syslog(syslog.LOG_ERR, message) + + +def syslog_auth_info(message): + emit_syslog(syslog.LOG_AUTH | syslog.LOG_INFO, message) + +def syslog_auth_error(message): + emit_syslog(syslog.LOG_AUTH | syslog.LOG_ERR, message) diff --git a/src/opnsense/service/modules/actions/base.py b/src/opnsense/service/modules/actions/base.py index a03cdcbf1..07c859f11 100644 --- a/src/opnsense/service/modules/actions/base.py +++ b/src/opnsense/service/modules/actions/base.py @@ -24,6 +24,7 @@ POSSIBILITY OF SUCH DAMAGE. """ from .. import syslog_notice +from ..session import xucred class BaseAction: @@ -41,6 +42,30 @@ class BaseAction: self.parameters = action_parameters.get('parameters', None) self.message = action_parameters.get('message', None) self.description = action_parameters.get('description', '') + self.allowed_groups = set() + for item in action_parameters.get('allowed_groups', '').split(','): + if item: + self.allowed_groups.add(item) + self.full_command = action_parameters.get('__full_command', '') + + def is_allowed(self, session : xucred = None): + """ Check if action is allowed for the session provided. + An action config may optionally supply allowed_groups (or generic in configd.conf) as constraint for + the call in question. + :param session: xucred session object + :return: bool + """ + print(session.get_groups()) + print(self.allowed_groups) + memberOf = session.get_groups() if isinstance(session, xucred) else [] + return len(self.allowed_groups) == 0 or len(self.allowed_groups & memberOf) > 0 + + def requires(self): + """ + :return: list of requirements for logging purposes + """ + return ','.join(self.allowed_groups) + def _cmd_builder(self, parameters): """ basic (shell) script command builder, uses action command, expected parameter phrase and given parameters diff --git a/src/opnsense/service/modules/processhandler.py b/src/opnsense/service/modules/processhandler.py index 47910184c..24c5d54e4 100644 --- a/src/opnsense/service/modules/processhandler.py +++ b/src/opnsense/service/modules/processhandler.py @@ -28,19 +28,20 @@ function: unix domain socket process worker process """ +import copy +import configparser +import glob import os -import subprocess +import shlex import socket import traceback import threading -import configparser -import glob import time import uuid -import shlex +from .session import get_session_context from .actions import ActionFactory from .actions.base import BaseAction -from . import syslog_error, syslog_info, syslog_notice, singleton +from . import syslog_error, syslog_info, syslog_notice, syslog_auth_info, syslog_auth_error, singleton class Handler(object): @@ -55,18 +56,21 @@ class Handler(object): <- send back result string """ - def __init__(self, socket_filename, config_path, config_environment=None, simulation_mode=False): + def __init__(self, socket_filename, config_path, config_environment=None, action_defaults=None): """ Constructor :param socket_filename: filename of unix domain socket to use - :param config_path: location of configuration files - :param simulation_mode: emulation mode, do not start actual (script) commands + :param config_path: location of action configuration files + :param config_environment: env to use in shell commands + :param action_defaults: default properties for action objects """ if config_environment is None: config_environment = {} + if action_defaults is None: + action_defaults = {} self.socket_filename = socket_filename self.config_path = config_path - self.simulation_mode = simulation_mode self.config_environment = config_environment + self.action_defaults = action_defaults self.single_threaded = False def run(self): @@ -77,7 +81,11 @@ class Handler(object): while True: # noinspection PyBroadException try: - act_handler = ActionHandler(config_path=self.config_path, config_environment=self.config_environment) + act_handler = ActionHandler( + config_path=self.config_path, + config_environment=self.config_environment, + action_defaults=self.action_defaults + ) try: os.unlink(self.socket_filename) except OSError: @@ -91,12 +99,12 @@ class Handler(object): while True: # wait for a connection to arrive connection, client_address = sock.accept() + # spawn a client connection cmd_thread = HandlerClient( connection=connection, client_address=client_address, - action_handler=act_handler, - simulation_mode=self.simulation_mode + action_handler=act_handler ) if self.single_threaded: # run single threaded @@ -128,20 +136,19 @@ class HandlerClient(threading.Thread): """ Handle commands via specified socket connection """ - def __init__(self, connection, client_address, action_handler, simulation_mode=False): + def __init__(self, connection, client_address, action_handler): """ :param connection: socket connection object :param client_address: client address ( from socket accept ) :param action_handler: action handler object - :param simulation_mode: Emulation mode, do not start actual (script) commands :return: None """ threading.Thread.__init__(self) self.connection = connection self.client_address = client_address self.action_handler = action_handler - self.simulation_mode = simulation_mode self.message_uuid = uuid.uuid4() + self.session = get_session_context(connection) def run(self): """ handle single action ( read data, execute command, send response ) @@ -149,9 +156,6 @@ class HandlerClient(threading.Thread): :return: None """ result = '' - exec_command = '' - exec_action = '' - exec_params = '' exec_in_background = False # noinspection PyBroadException try: @@ -176,11 +180,7 @@ class HandlerClient(threading.Thread): self.connection.close() # execute requested action - if self.simulation_mode: - self.action_handler.show_action(data_parts, self.message_uuid) - result = 'OK' - else: - result = self.action_handler.execute(data_parts, self.message_uuid, self.connection) + result = self.action_handler.execute(data_parts, self.message_uuid, self.connection, self.session) if not exec_in_background: # send response back to client (including trailing enters) @@ -199,13 +199,13 @@ class HandlerClient(threading.Thread): # send end of stream characters if not exec_in_background: self.connection.sendall(("%c%c%c" % (chr(0), chr(0), chr(0))).encode()) - except SystemExit: - # ignore system exit related errors + except (SystemExit, BrokenPipeError): + # ignore system exit or "client left" related errors pass except Exception: print(traceback.format_exc()) - syslog_notice('unable to sendback response [%s] for [%s][%s][%s] {%s}, message was %s' % ( - result, exec_command, exec_action, exec_params, self.message_uuid, traceback.format_exc() + syslog_notice('unable to sendback response for %s, message was %s' % ( + self.message_uuid, traceback.format_exc() )) finally: if not exec_in_background: @@ -218,28 +218,27 @@ class ActionHandler(object): """ Start/stop services and functions using configuration data defined in conf/actions_.conf """ - def __init__(self, config_path=None, config_environment=None): + def __init__(self, config_path=None, config_environment=None, action_defaults=None): """ Initialize action handler to start system functions :param config_path: full path of configuration data :param config_environment: environment to use (if possible) + :param action_defaults: default properties for action objects :return: """ - if config_path is not None: - self.config_path = config_path - if config_environment is not None: - self.config_environment = config_environment - - # try to load data on initial start - if not hasattr(self, 'action_map'): - self.action_map = {} - self.load_config() + self.config_path = config_path + self.config_environment = config_environment if config_environment else {} + self.action_defaults = action_defaults if action_defaults else {} + self.action_map = {} + self.load_config() def load_config(self): """ load action configuration from config files into local dictionary :return: None """ + if self.config_path is None: + return action_factory = ActionFactory() for config_filename in glob.glob('%s/actions_*.conf' % self.config_path) \ + glob.glob('%s/actions.d/actions_*.conf' % self.config_path): @@ -257,8 +256,10 @@ class ActionHandler(object): syslog_error('exception occurred while reading "%s": %s' % (config_filename, traceback.format_exc(0))) for section in cnf.sections(): - # map configuration data on object - conf = {} + # map configuration data on object, start with default action config and add __full_command for + # easy reference. + conf = copy.deepcopy(self.action_defaults) + conf['__full_command'] = "%s.%s" % (topic_name, section) for act_prop in cnf.items(section): conf[act_prop[0]] = act_prop[1] action_obj = action_factory.get(environment=self.config_environment, conf=conf) @@ -321,28 +322,29 @@ class ActionHandler(object): return None, [] - def execute(self, action, message_uuid, connection=None): + def execute(self, action, message_uuid, connection, session): """ execute configuration defined action :param action: list of commands and parameters :param message_uuid: message unique id + :param connection: socket connection (in case we need to stream data back) + :param session: this session context (used for access management) :return: OK on success, else error code """ + full_command = '.'.join(action) action_obj, action_params = self.find_action(action) if action_obj is not None: - return action_obj.execute(action_params, message_uuid, connection) + is_allowed = action_obj.is_allowed(session) + if is_allowed: + syslog_auth_info("action allowed %s for user %s" % (action_obj.full_command,session.get_user())) + return action_obj.execute(action_params, message_uuid, connection) + else: + syslog_auth_error("action denied %s for user %s (requires : %s)" % ( + action_obj.full_command, + session.get_user(), + action_obj.requires()) + ) + return 'Action not allowed or missing\n' - return 'Action not found\n' - - def show_action(self, action, message_uuid): - """ debug/simulation mode: show action information - :param action: list of commands and parameters - :param message_uuid: message unique id - :return: None - """ - action_obj, parameters = self.find_action(action) - if action_obj is not None: - print('---------------------------------------------------------------------') - print('execute %s ' % ' '.join(action)) - print('action object %s (%s) %s' % (action_obj, action_obj.command, message_uuid)) - print('---------------------------------------------------------------------') + syslog_auth_error("action %s not found for user %s" % (full_command, session.get_user())) + return 'Action not allowed or missing\n' diff --git a/src/opnsense/service/modules/session.py b/src/opnsense/service/modules/session.py new file mode 100644 index 000000000..f0e090ab9 --- /dev/null +++ b/src/opnsense/service/modules/session.py @@ -0,0 +1,72 @@ + +""" + Copyright (c) 2014-2023 Ad Schellevis + All rights reserved. + + Redistribution and use in source and binary forms, with or without + modification, are permitted provided that the following conditions are met: + + 1. Redistributions of source code must retain the above copyright notice, + this list of conditions and the following disclaimer. + + 2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + + THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, + INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY + AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, + OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + POSSIBILITY OF SUCH DAMAGE. + + -------------------------------------------------------------------------------------- + package : configd + function: session handling and authorisation +""" +import struct +import socket +import pwd +import grp + +class xucred: + def __init__(self, connection): + # xucred structure defined in : https://man.freebsd.org/cgi/man.cgi?query=unix&sektion=4 + # XU_NGROUPS is 16 + xucred_fmt = '2ih16iP' + tmp = connection.getsockopt(0, socket.LOCAL_PEERCRED, struct.calcsize(xucred_fmt)) + tmp = tuple(struct.unpack(xucred_fmt, tmp)) + self.cr_version = tmp[0] + self.cr_uid = tmp[1] + self.cr_ngroups = tmp[2] + self.cr_groups = tmp[3:18] + self.cr_pid = tmp[19] + self._user = None + self._groups = set() + tmp = pwd.getpwuid(self.cr_uid) + if tmp: + self._user = tmp.pw_name + for idx,item in enumerate(self.cr_groups): + if idx < self.cr_ngroups: + tmp = grp.getgrgid(item) + if tmp: + self._groups.add(tmp.gr_name) + + def get_groups(self): + return self._groups + + def get_user(self): + return self._user + + +def get_session_context(connection): + """ + :param instr: string with optional tags [field.$$] + :return: xucred + """ + + return xucred(connection) diff --git a/src/opnsense/service/templates/OPNsense/Syslog/local/configd.conf b/src/opnsense/service/templates/OPNsense/Syslog/local/configd.conf index e64000098..a41bbdcb1 100644 --- a/src/opnsense/service/templates/OPNsense/Syslog/local/configd.conf +++ b/src/opnsense/service/templates/OPNsense/Syslog/local/configd.conf @@ -2,5 +2,5 @@ # Local syslog-ng configuration filter definition [configd/backend]. ################################################################### filter f_local_configd { - program("configd.py") or program("api"); + (program("configd.py") or program("api")) and not facility(auth); }; diff --git a/src/opnsense/service/tests/core.py b/src/opnsense/service/tests/core.py index d55d2dcb6..64e2de25d 100644 --- a/src/opnsense/service/tests/core.py +++ b/src/opnsense/service/tests/core.py @@ -27,6 +27,7 @@ package : configd """ +import struct import unittest import json from modules import processhandler @@ -81,6 +82,12 @@ class DummySocket(object): def shutdown(self, mode): pass + def getsockopt(*args, **kwargs): + # return dummy xucred structure data + tmp = ('2ih16iP', 0, 0, 3, 0, 0, 5, 1999, 2002, 2012, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1317) + return struct.pack(*tmp) + + class TestCoreMethods(unittest.TestCase): def setUp(self): @@ -106,8 +113,7 @@ class TestCoreMethods(unittest.TestCase): self.dummysock.setTestData('xxxxxx\n') cmd_thread = processhandler.HandlerClient(connection=self.dummysock, client_address=None, - action_handler=self.act_handler, - simulation_mode=False) + action_handler=self.act_handler) cmd_thread.run() self.assertEqual(self.dummysock.getReceived()[-4:], '\n%c%c%c' % (chr(0), chr(0), chr(0)), "Invalid sequence") @@ -118,10 +124,13 @@ class TestCoreMethods(unittest.TestCase): self.dummysock.setTestData('xxxxxx\n') cmd_thread = processhandler.HandlerClient(connection=self.dummysock, client_address=None, - action_handler=self.act_handler, - simulation_mode=False) + action_handler=self.act_handler) cmd_thread.run() - self.assertEqual(self.dummysock.getReceived().split('\n')[0], 'Action not found', 'Invalid response') + self.assertEqual( + self.dummysock.getReceived().split('\n')[0], + 'Action not allowed or missing', + 'Invalid response' + ) def test_configd_actions(self): """ request configd command list @@ -130,8 +139,7 @@ class TestCoreMethods(unittest.TestCase): self.dummysock.setTestData('configd actions json\n') cmd_thread = processhandler.HandlerClient(connection=self.dummysock, client_address=None, - action_handler=self.act_handler, - simulation_mode=False) + action_handler=self.act_handler) cmd_thread.run() response = json.loads(self.dummysock.getReceived()[:-4]) self.assertGreater(len(response), 10, 'number of configd commands very suspicious')