MVC/Config - Prevent config restore when writer has flushed or partly wrtiten the file. closes https://github.com/opnsense/core/issues/6565

This should lower the chances of accidental restores when reader processes are active very frequently triggering a restore on a faulty read.
It should be possible to read the configuration while the config is exclusively locked, as these operations may take much more time than the actual write process takes.
After this commit the reader first tries to read unconditionally and if this fails, waits until a lock can be acquired.

To increase visibilty of restore invents, send these to the audit log in stead of the general system log.
This commit is contained in:
Ad Schellevis 2023-05-16 20:45:53 +02:00
parent 2da3787b38
commit bebf3a2a7c

View File

@ -1,7 +1,7 @@
<?php
/*
* Copyright (C) 2015-2021 Deciso B.V.
* Copyright (C) 2015-2023 Deciso B.V.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@ -31,6 +31,7 @@ namespace OPNsense\Core;
use Phalcon\Di\FactoryDefault;
use OPNsense\Phalcon\Logger\Logger;
use Phalcon\Logger\Adapter\Syslog;
use Phalcon\Logger\Formatter\Line;
/**
* Class Config provides access to systems config xml
@ -316,13 +317,12 @@ class Config extends Singleton
$this->simplexml = null;
// there was an issue with loading the config, try to restore the last backup
$backups = $this->getBackups();
$adapter = new Syslog('audit', ['option' => LOG_PID,'facility' => LOG_LOCAL5]);
$adapter->setFormatter(new Line('%message%'));
$logger = new Logger(
'messages',
[
'main' => new Syslog("config", array(
'option' => LOG_PID,
'facility' => LOG_LOCAL4
))
'main' => $adapter
]
);
if (count($backups) > 0) {
@ -331,6 +331,7 @@ class Config extends Singleton
foreach ($backups as $backup) {
try {
$this->restoreBackup($backup);
$logger->error("restored " . $backup);
return;
} catch (ConfigException $e) {
$logger->error("failed restoring " . $backup);
@ -363,23 +364,33 @@ class Config extends Singleton
*/
private function loadFromStream($fp)
{
fseek($fp, 0);
$xml = stream_get_contents($fp);
if (trim($xml) == '') {
throw new ConfigException('empty file');
/**
* load data from stream in shared mode unless no valid xml data is returned
* (in which case the writer holds a lock and we should wait for it [LOCK_EX])
*/
foreach ([LOCK_SH|LOCK_NB, LOCK_EX] as $idx => $mode) {
flock($fp, $mode);
fseek($fp, 0);
$xml = trim(stream_get_contents($fp));
set_error_handler(
function () {
// reset simplexml pointer on parse error.
$result = null;
}
);
$result = simplexml_load_string($xml);
restore_error_handler();
flock($fp, LOCK_UN);
if ($result != null) {
break; // successful load
}
}
set_error_handler(
function () {
// reset simplexml pointer on parse error.
$result = null;
}
);
$result = simplexml_load_string($xml);
restore_error_handler();
if ($result == null) {
if (empty($xml)) {
throw new ConfigException('empty file');
}
throw new ConfigException("invalid config xml");
} else {
return $result;
@ -681,13 +692,12 @@ class Config extends Singleton
// use syslog to trigger a new configd event, which should signal a syshook config (in batch).
// Although we include the backup filename, the event handler is responsible to determine the
// last processed event itself. (it's merely added for debug purposes)
$adapter = new Syslog('config', ['option' => LOG_PID,'facility' => LOG_LOCAL5]);
$adapter->setFormatter(new Line('%message%'));
$logger = new Logger(
'messages',
[
'main' => new Syslog("config", array(
'option' => LOG_PID,
'facility' => LOG_LOCAL5
))
'main' => $adapter
]
);
$logger->info("config-event: new_config " . $backup_filename);