From ef1d1552bf5a82eeafa1e64f033983eed0aab363 Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Sun, 26 Nov 2023 14:31:50 +0100 Subject: [PATCH] auth - improve config revision auditability, closes https://github.com/opnsense/core/issues/7033 This commit contains the following changes to improve revision visibility. * add username and api token for external (non-gui) callers. * offer the ability to merge revision information into configuration saves. (getRevisionContext / setRevisionContext) * merge session attributes starting with xrevision_ into a revision item, for example. xrevision_impersonated_by would be recorded as impersonated_by * add "impersonated_by" to audit log when specified (for future use) * remove revision attributes before adding, this prevents attributes sticking around. --- .../OPNsense/Base/ApiControllerBase.php | 7 +- .../mvc/app/library/OPNsense/Core/Config.php | 122 +++++++++++------- 2 files changed, 83 insertions(+), 46 deletions(-) diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiControllerBase.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiControllerBase.php index c4e27e3b0..bc23fbb75 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiControllerBase.php +++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiControllerBase.php @@ -29,6 +29,7 @@ namespace OPNsense\Base; use OPNsense\Core\ACL; +use OPNsense\Core\Config; use OPNsense\Auth\AuthenticationFactory; /** @@ -267,7 +268,11 @@ class ApiControllerBase extends ControllerRoot // link username on successful login $this->logged_in_user = $authResult['username']; - + // pass revision context to config object + Config::getInstance()->setRevisionContext([ + 'username' => $authResult['username'], + 'user_apitoken' => $apiKey + ]); return true; } } diff --git a/src/opnsense/mvc/app/library/OPNsense/Core/Config.php b/src/opnsense/mvc/app/library/OPNsense/Core/Config.php index c19782f02..ae3ffd49c 100644 --- a/src/opnsense/mvc/app/library/OPNsense/Core/Config.php +++ b/src/opnsense/mvc/app/library/OPNsense/Core/Config.php @@ -69,6 +69,11 @@ class Config extends Singleton */ private $statusIsValid = false; + /** + * @var array list of revision relevant data + */ + private $revisionContext = []; + /** * @var float current modification time of our known config */ @@ -465,76 +470,103 @@ class Config extends Singleton return $dom->saveXML(); } + /** + * @return array revision key/values + */ + public function getRevisionContext() + { + $revision = $this->revisionContext; + if (!empty($_SESSION["Username"])) { + $revision['username'] = $_SESSION["Username"]; + } elseif (!isset($revision['username'])) { + $revision['username'] = "(system)" ; + } + if (!empty($_SERVER['REMOTE_ADDR']) && strpos($revision['username'], '@') === false) { + $revision['username'] .= "@" . $_SERVER['REMOTE_ADDR']; + } + $revision['description'] = sprintf( + gettext('%s made changes'), + !empty($_SERVER['REQUEST_URI']) ? $_SERVER['REQUEST_URI'] : $_SERVER['SCRIPT_NAME'] + ); + // append session revision tags when supplied (keys start with xrevision_) + if (!empty($_SESSION) && is_array($_SESSION)) { + foreach ($_SESSION as $key => $value) { + if (stripos($key, 'xrevision_') === 0 && !isset($revision[substr($key,10)])) { + $revision[substr($key,10)] = $value; + } + } + + } + $revision['time'] = empty($timestamp) ? microtime(true) : $timestamp; + + return $revision; + } + + /** + * set revision payload + * @param array revision payload + */ + public function setRevisionContext($ctx) + { + if (is_array($ctx)) { + $this->revisionContext = $ctx; + return true; + } + return false; + } + /** * update config revision information (ROOT.revision tag) * @param array|null $revision revision tag (associative array) * @param \SimpleXMLElement|null pass trough xml node + * @return array revision data */ private function updateRevision($revision, $node = null, $timestamp = null) { - // if revision info is not provided, create a default. + /* If revision info is not provided, create one. $revision is used for recursion */ if (!is_array($revision)) { - $revision = array(); - // try to fetch userinfo from session - if (!empty($_SESSION["Username"])) { - $revision['username'] = $_SESSION["Username"]; - } else { - $revision['username'] = "(system)"; - } - if (!empty($_SERVER['REMOTE_ADDR'])) { - $revision['username'] .= "@" . $_SERVER['REMOTE_ADDR']; - } - if (!empty($_SERVER['REQUEST_URI'])) { - // when update revision is called from a controller, log the endpoint uri - $revision['description'] = sprintf(gettext('%s made changes'), $_SERVER['REQUEST_URI']); - } else { - // called from a script, log script name and path - $revision['description'] = sprintf(gettext('%s made changes'), $_SERVER['SCRIPT_NAME']); - } + $revision = $this->getRevisionContext(); } - - // always set timestamp - $revision['time'] = empty($timestamp) ? microtime(true) : $timestamp; - if ($node == null) { - if (isset($this->simplexml->revision)) { - $node = $this->simplexml->revision; + if (!isset($this->simplexml->revision)) { + $target = $this->simplexml->addChild("revision"); } else { - $node = $this->simplexml->addChild("revision"); + $target = $this->simplexml->revision; + foreach (iterator_to_array($target->children()) as $child) { + unset($target->{$child->getName()}); + } } + } else { + $target = $node; } - foreach ($revision as $revKey => $revItem) { - if (isset($node->{$revKey})) { - // key already in revision object - $childNode = $node->{$revKey}; + + array_walk($revision, function($value, $key) use (&$target) { + $node = $target->addChild($key); + if (is_array($value)) { + $this->updateRevision($value, $node); } else { - $childNode = $node->addChild($revKey); + $node[0] = $value; } - if (is_array($revItem)) { - $this->updateRevision($revItem, $childNode); - } else { - $childNode[0] = $revItem; - } - } + }); + + return $revision; } /** * send config change to audit log including the context we currently know of. + * @param string $backup_filename new backup filename + * @param array $revision revision adata used */ - private function auditLogChange($backup_filename, $revision = null) + private function auditLogChange($backup_filename, $revision) { openlog("audit", LOG_ODELAY, LOG_AUTH); - $append_message = ""; - if (is_array($revision) && !empty($revision['description'])) { - $append_message = sprintf(" [%s]", $revision['description']); - } syslog(LOG_NOTICE, sprintf( "user %s%s changed configuration to %s in %s%s", - !empty($_SESSION["Username"]) ? $_SESSION["Username"] : "(system)", - !empty($_SERVER['REMOTE_ADDR']) ? "@" . $_SERVER['REMOTE_ADDR'] : "", + $revision['username'], + !empty($revision['impersonated_by']) ? sprintf(" (%s)", $revision['impersonated_by']) : '', $backup_filename, !empty($_SERVER['REQUEST_URI']) ? $_SERVER['REQUEST_URI'] : $_SERVER['SCRIPT_NAME'], - $append_message + $revision['description'] ?? '' )); closelog(); } @@ -716,7 +748,7 @@ class Config extends Singleton $this->checkvalid(); $time = microtime(true); // update revision information ROOT.revision tag, align timestamp to backup output - $this->updateRevision($revision, null, $time); + $revision = $this->updateRevision($revision, null, $time); if ($this->config_file_handle !== null) { if (flock($this->config_file_handle, LOCK_EX)) {