From 6eb7a2da64aba9a942b27dd0c8153cf72078a9d8 Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Fri, 2 Oct 2020 17:58:54 +0200 Subject: [PATCH] core / mvc: add new config changed event using syshook structure o in order for this to work properly we need to change when a config backup is made, previously we performed a backup before the fact, now we backup afterwards. which means the top level always represents the current change (and can thus be signaled to an event handler). After upgrade one might lose a single backup file due to this change, but that should be a small price to pay for progress. o config backup count was defined incorrect (60 instead of 100 according to the gui) o the syslog-ng event structure is using the existing configd handler and filters relevant events within a small time frame (which prevents flooding configd) Since the event is loosely coupled, the risk for releasing this into an existing environment should be rather low. For https://github.com/opnsense/core/issues/4388 sponsored by : Modirum (https://www.modirum.com/) --- .../mvc/app/library/OPNsense/Core/Config.php | 43 ++++++------ .../system/trigger_config_changed_events.py | 66 +++++++++++++++++++ .../conf/actions.d/actions_system.conf | 6 ++ .../templates/OPNsense/Syslog/+TARGETS | 1 + .../Syslog/syslog-ng-config-events.conf | 13 ++++ 5 files changed, 109 insertions(+), 20 deletions(-) create mode 100755 src/opnsense/scripts/system/trigger_config_changed_events.py create mode 100644 src/opnsense/service/templates/OPNsense/Syslog/syslog-ng-config-events.conf diff --git a/src/opnsense/mvc/app/library/OPNsense/Core/Config.php b/src/opnsense/mvc/app/library/OPNsense/Core/Config.php index 0ed4628c3..add38b5d6 100644 --- a/src/opnsense/mvc/app/library/OPNsense/Core/Config.php +++ b/src/opnsense/mvc/app/library/OPNsense/Core/Config.php @@ -455,25 +455,29 @@ class Config extends Singleton } /** - * backup current (running) config - * @return float timestamp + * backup current config + * @return string target filename */ public function backup() { $timestamp = microtime(true); $target_dir = dirname($this->config_file) . "/backup/"; - $target_filename = "config-" . $timestamp . ".xml"; if (!file_exists($target_dir)) { // create backup directory if it is missing mkdir($target_dir); } - // The new target backup filename shouldn't exists, because of the use of microtime. - // But if for some reason a script keeps calling this backup very often, it should not crash. - if (!file_exists($target_dir . $target_filename)) { - copy($this->config_file, $target_dir . $target_filename); + if (file_exists($target_dir . "config-" . $timestamp . ".xml")) { + // The new target backup filename shouldn't exists, because of the use of microtime. + // in the unlikely event that we can process events too fast for microtime(), suffix with a more + // precise tiestamp to ensure we can't miss a backup + $target_filename = "config-" . $timestamp . "_" . hrtime()[1] . ".xml"; + } else { + $target_filename = "config-" . $timestamp . ".xml"; } - return $timestamp; + copy($this->config_file, $target_dir . $target_filename); + + return $target_dir . $target_filename; } /** @@ -555,7 +559,7 @@ class Config extends Singleton ) { return intval($this->simplexml->system->backupcount); } else { - return 60; + return 100; } } @@ -601,25 +605,24 @@ class Config extends Singleton { $this->checkvalid(); - if ($backup) { - $timestamp = $this->backup(); - } else { - $timestamp = microtime(true); - } - // update revision information ROOT.revision tag, align timestamp to backup output - $this->updateRevision($revision, null, $timestamp); - - // serialize to text - $xml_text = $this->__toString(); + $this->updateRevision($revision, null, microtime(true)); if ($this->config_file_handle !== null) { if (flock($this->config_file_handle, LOCK_EX)) { fseek($this->config_file_handle, 0); ftruncate($this->config_file_handle, 0); - fwrite($this->config_file_handle, $xml_text); + fwrite($this->config_file_handle, (string)$this); // flush, unlock, but keep the handle open fflush($this->config_file_handle); + $backup_filename = $backup ? $this->backup() : null; + if ($backup_filename) { + // use syslog to trigger a new configd event, which should signal a syshook config (in batch). + // Althought we include the backup filename, the event handler is responsible to determine the + // last processed event itself. (it's merely added for debug purposes) + $logger = new Syslog("config", array('option' => LOG_PID, 'facility' => LOG_LOCAL5)); + $logger->info("config-event: new_config " . $backup_filename); + } flock($this->config_file_handle, LOCK_UN); } else { throw new ConfigException("Unable to lock config"); diff --git a/src/opnsense/scripts/system/trigger_config_changed_events.py b/src/opnsense/scripts/system/trigger_config_changed_events.py new file mode 100755 index 000000000..5ab4bae95 --- /dev/null +++ b/src/opnsense/scripts/system/trigger_config_changed_events.py @@ -0,0 +1,66 @@ +#!/usr/local/bin/python3 + +""" + Copyright (c) 2020 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. + +""" +import decimal +import fcntl +import glob +import os +import subprocess +import sys +import ujson + + +try: + fm = 'r+' if os.path.isfile('/conf/event_config_changed.json') else 'w+' + status_fhandle = open('/conf/event_config_changed.json', fm) + fcntl.flock(status_fhandle, fcntl.LOCK_EX | fcntl.LOCK_NB) +except IOError: + # already running, exit status 99, it should be safe to skip an event when config changes happen too frequently + sys.exit(99) + +status_fhandle.seek(0) +try: + metadata = ujson.loads(status_fhandle.read()) + # ujson treats decimals as floats, round these numbers to avoid re-triggering the previous handled event + metadata['last_proccessed_stamp'] = round(decimal.Decimal(metadata['last_proccessed_stamp']), 4) +except ValueError: + metadata = {'last_proccessed_stamp': 0} + +for filename in sorted(glob.glob('/conf/backup/config-*.xml')): + ts=filename.split('-')[-1].split('.xml')[0].replace('_', '') + if ts.count('.') <= 1 and ts.replace('.', '').isdigit(): + # only process valid config backups containing a timestamp + ts_num = decimal.Decimal(ts) + if ts_num > metadata['last_proccessed_stamp']: + subprocess.run(["/usr/local/etc/rc.syshook", "config", filename]) + metadata['last_proccessed_stamp'] = ts_num + +# write metadata and exit +status_fhandle.seek(0) +status_fhandle.truncate() +status_fhandle.write(ujson.dumps(metadata)) diff --git a/src/opnsense/service/conf/actions.d/actions_system.conf b/src/opnsense/service/conf/actions.d/actions_system.conf index f0277b68e..f75258f19 100644 --- a/src/opnsense/service/conf/actions.d/actions_system.conf +++ b/src/opnsense/service/conf/actions.d/actions_system.conf @@ -42,6 +42,12 @@ type:script message:Performing remote backup description:Remote backup +[event.config_changed] +parameters: +command:/usr/local/opnsense/scripts/system/trigger_config_changed_events.py +type:script +message:trigger config changed event + [reboot] command:/usr/local/etc/rc.reboot parameters: diff --git a/src/opnsense/service/templates/OPNsense/Syslog/+TARGETS b/src/opnsense/service/templates/OPNsense/Syslog/+TARGETS index 19115d463..40a96ee25 100644 --- a/src/opnsense/service/templates/OPNsense/Syslog/+TARGETS +++ b/src/opnsense/service/templates/OPNsense/Syslog/+TARGETS @@ -5,3 +5,4 @@ syslog-ng-legacy.conf:/usr/local/etc/syslog-ng.conf.d/legacy.conf syslog-ng-destinations.conf:/usr/local/etc/syslog-ng.conf.d/syslog-ng-destinations.conf syslog-ng-local.conf:/usr/local/etc/syslog-ng.conf.d/syslog-ng-local.conf syslog-ng-lockout.conf:/usr/local/etc/syslog-ng.conf.d/syslog-ng-lockout.conf +syslog-ng-config-events.conf:/usr/local/etc/syslog-ng.conf.d/syslog-ng-config-events.conf diff --git a/src/opnsense/service/templates/OPNsense/Syslog/syslog-ng-config-events.conf b/src/opnsense/service/templates/OPNsense/Syslog/syslog-ng-config-events.conf new file mode 100644 index 000000000..64e38d647 --- /dev/null +++ b/src/opnsense/service/templates/OPNsense/Syslog/syslog-ng-config-events.conf @@ -0,0 +1,13 @@ +destination d_config_changed_event { + program("/usr/local/sbin/configctl -e -t 0.5 system event config_changed"); +}; + +filter f_config_changed_event { + program("config") and level("info") and message(".*config-event: new_config*"); +}; + +log { + source(s_all); + filter(f_config_changed_event); + destination(d_config_changed_event); +};