From 200436b06ea7ee4b91f59eaee61a5a9263c4bf0f Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Mon, 16 Jun 2025 18:00:13 +0200 Subject: [PATCH 1/8] security/acme-client: fix acme.sh is always called with "--days 1", refs #4711 --- security/acme-client/pkg-descr | 5 +++++ .../mvc/app/library/OPNsense/AcmeClient/LeCertificate.php | 3 ++- .../app/library/OPNsense/AcmeClient/LeValidation/Base.php | 4 ++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/security/acme-client/pkg-descr b/security/acme-client/pkg-descr index 9a3c8d013..0e8663411 100644 --- a/security/acme-client/pkg-descr +++ b/security/acme-client/pkg-descr @@ -8,6 +8,11 @@ WWW: https://github.com/acmesh-official/acme.sh Plugin Changelog ================ +4.10 + +Fixed: +* acme.sh is always called with "--days 1" (#4711) + 4.9 Added: diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php index 518a2c0f7..edc3c2356 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php @@ -642,7 +642,8 @@ class LeCertificate extends LeCommon // Configure validation object $val->setNames($this->config->name, $this->config->altNames, $this->config->aliasmode, $this->config->domainalias, $this->config->challengealias); - $val->setRenewal((int)$this->config->renewInterval); + $renewInterval = (string)$this->config->renewInterval; + $val->setRenewal((int)$renewInterval); $val->setForce($this->force); $val->setOcsp((string)$this->config->ocsp == 1 ? true : false); // strip prefix from key value diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/Base.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/Base.php index b28effbb6..1946ab3b9 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/Base.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/Base.php @@ -1,7 +1,7 @@ * All rights reserved. @@ -286,6 +286,6 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon */ public function setRenewal(int $interval = 60) { - $this->acme_args[] = LeUtils::execSafe('--days %s', (string)$interval); + $this->acme_args[] = LeUtils::execSafe('--days %s', $interval); } } From 18d2c753d42744dee8537b311d3a1406c38084b0 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Mon, 16 Jun 2025 18:02:45 +0200 Subject: [PATCH 2/8] security/acme-client: bump version --- security/acme-client/Makefile | 2 +- security/acme-client/pkg-descr | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/security/acme-client/Makefile b/security/acme-client/Makefile index 809895dc0..a81e74fb7 100644 --- a/security/acme-client/Makefile +++ b/security/acme-client/Makefile @@ -1,5 +1,5 @@ PLUGIN_NAME= acme-client -PLUGIN_VERSION= 4.9 +PLUGIN_VERSION= 4.10 PLUGIN_COMMENT= ACME Client PLUGIN_MAINTAINER= opnsense@moov.de PLUGIN_DEPENDS= acme.sh py${PLUGIN_PYTHON}-dns-lexicon diff --git a/security/acme-client/pkg-descr b/security/acme-client/pkg-descr index 0e8663411..ff4b768bb 100644 --- a/security/acme-client/pkg-descr +++ b/security/acme-client/pkg-descr @@ -10,6 +10,9 @@ Plugin Changelog 4.10 +Added: +* new automation to reload www/caddy (#4692) + Fixed: * acme.sh is always called with "--days 1" (#4711) From 5458836c03132b9b9e1b4c0c1c030619a29c0e26 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Mon, 16 Jun 2025 18:20:47 +0200 Subject: [PATCH 3/8] security/acme-client: avoid startup error, refs #4743 --- security/acme-client/pkg-descr | 1 + .../src/opnsense/scripts/OPNsense/AcmeClient/setup.sh | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/security/acme-client/pkg-descr b/security/acme-client/pkg-descr index ff4b768bb..4c69017f3 100644 --- a/security/acme-client/pkg-descr +++ b/security/acme-client/pkg-descr @@ -15,6 +15,7 @@ Added: Fixed: * acme.sh is always called with "--days 1" (#4711) +* avoid startup error: "rmdir... Not a directory" (#4743) 4.9 diff --git a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/setup.sh b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/setup.sh index 8a6352652..567364b40 100755 --- a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/setup.sh +++ b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/setup.sh @@ -20,7 +20,10 @@ chmod 750 ${ACME_BASE} ${ACME_BASE}/* # This should guard against manual misconfiguration. for link in ${ACME_LINKS}; do # First remove any existing file/directory. - if [ -f "${ACME_BASE}/home/${link}" ]; then + if [ -L "${ACME_BASE}/home/${link}" ]; then + # Already a symlink, skip this enty. + continue + elif [ -f "${ACME_BASE}/home/${link}" ]; then rm ${ACME_BASE}/home/${link} elif [ -d "${ACME_BASE}/home/${link}" ]; then rmdir ${ACME_BASE}/home/${link} From 82cff66d04290962b3ca2c3d0aa93cf75a21c823 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Tue, 17 Jun 2025 12:00:48 +0200 Subject: [PATCH 4/8] security/acme-client: automatically resolve cron job mismatch, fixes #4627 --- security/acme-client/pkg-descr | 4 + .../AcmeClient/Api/SettingsController.php | 99 +++++++++++++------ 2 files changed, 74 insertions(+), 29 deletions(-) diff --git a/security/acme-client/pkg-descr b/security/acme-client/pkg-descr index 4c69017f3..4650976da 100644 --- a/security/acme-client/pkg-descr +++ b/security/acme-client/pkg-descr @@ -13,9 +13,13 @@ Plugin Changelog Added: * new automation to reload www/caddy (#4692) +Changed: +* automatically resolve cron job mismatch (#4627) + Fixed: * acme.sh is always called with "--days 1" (#4711) * avoid startup error: "rmdir... Not a directory" (#4743) +* fails to create/update cron job on UUID mismatch (#4627) 4.9 diff --git a/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/Api/SettingsController.php b/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/Api/SettingsController.php index 695239e5b..5251a32f7 100644 --- a/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/Api/SettingsController.php +++ b/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/Api/SettingsController.php @@ -1,7 +1,7 @@ getModel(); $backend = new Backend(); - // Setup cronjob if AcmeClient and AutoRenewal is enabled. + // Setup cron job if AcmeClient and AutoRenewal is enabled. if ( - (string)$mdlAcme->settings->UpdateCron == "" and (string)$mdlAcme->settings->autoRenewal == "1" and (string)$mdlAcme->settings->enabled == "1" ) { - $mdlCron = new Cron(); - // NOTE: Only configd actions are valid commands for cronjobs - // and they *must* provide a description that is not empty. - $cron_uuid = $mdlCron->newDailyJob( - "AcmeClient", - "acmeclient cron-auto-renew", - "AcmeClient Cronjob for Certificate AutoRenewal", - "*", - "1" - ); - $mdlAcme->settings->UpdateCron = $cron_uuid; + // Check if a cron job UUID is available. + $cron_uuid = (string)$mdlAcme->settings->UpdateCron; + $cron_found = 0; + if ($cron_uuid != "") { + // Try to get cron job data from system config. + $cron_job = (new Cron())->getNodeByReference('jobs.job.' . $cron_uuid); + if ($cron_job != null) { + // Cron job found, no changes required. + $cron_found = 1; + $this->getLogger()->notice("AcmeClient: successfully validated cron job"); + } else { + // Cron job NOT found. This should not happen, try to fix + // this automatically. + $this->getLogger()->error("AcmeClient: cron job with stored UUID not found in system config: ${cron_uuid}"); - // Save updated configuration. - if ($mdlCron->performValidation()->count() == 0) { - $mdlCron->serializeToConfig(); - // save data to config, do not validate because the current in memory model doesn't know about the - // cron item just created. - $mdlAcme->serializeToConfig($validateFullModel = false, $disable_validation = true); - Config::getInstance()->save(); - // Refresh the crontab - $backend->configdRun('template reload OPNsense/Cron'); - // (res)start daemon - $backend->configdRun("cron restart"); - $result['result'] = "new"; - $result['uuid'] = $cron_uuid; - } else { - $result['result'] = "unable to add cron"; + // Search for existing AcmeClient cron job. + foreach ((new Cron())->getNodeByReference('jobs.job')->iterateItems() as $cron) { + $_uuid = $cron->getAttributes()["uuid"]; + $_origin = (string)$cron->origin; + if ($_origin == 'AcmeClient') { + // Found a matching AcmeClient cron job. + $cron_found = 1; + $this->getLogger()->notice("AcmeClient: found existing AcmeClient cron job, fixing inconsistency in config (new UUID: ${_uuid})"); + // Update UUID in Acme Client config. + $mdlAcme->settings->UpdateCron = $_uuid; + // Save updated configuration. + // TODO: need to disable validation? + $mdlAcme->serializeToConfig(); + Config::getInstance()->save(); + $result['result'] = "new"; + $result['uuid'] = $_uuid; + break; + } + } + } + } + + // No matching cron job found. Create a new one. + if ($cron_found == 0) { + $this->getLogger()->notice("AcmeClient: no cron job for AutoRenewal found, creating a new one"); + $mdlCron = new Cron(); + // NOTE: Only configd actions are valid commands for cronjobs + // and they *must* provide a description that is not empty. + $cron_uuid = $mdlCron->newDailyJob( + "AcmeClient", + "acmeclient cron-auto-renew", + "AcmeClient Cronjob for Certificate AutoRenewal", + "*", + "1" + ); + $mdlAcme->settings->UpdateCron = $cron_uuid; + + // Save updated configuration. + if ($mdlCron->performValidation()->count() == 0) { + $mdlCron->serializeToConfig(); + // save data to config, do not validate because the current in memory model doesn't know about the + // cron item just created. + $mdlAcme->serializeToConfig($validateFullModel = false, $disable_validation = true); + Config::getInstance()->save(); + // Refresh the crontab + $backend->configdRun('template reload OPNsense/Cron'); + // (res)start daemon + $backend->configdRun("cron restart"); + $result['result'] = "new"; + $result['uuid'] = $cron_uuid; + } else { + $result['result'] = "unable to add cron"; + } } // Delete cronjob if AcmeClient or AutoRenewal is disabled. } elseif ( @@ -99,6 +139,7 @@ class SettingsController extends ApiMutableModelControllerBase ((string)$mdlAcme->settings->autoRenewal == "0" or (string)$mdlAcme->settings->enabled == "0") ) { + $this->getLogger()->notice("AcmeClient: plugin or AutoRenewal is disabled, removing existing cron job"); // Get UUID, clean existin entry $cron_uuid = (string)$mdlAcme->settings->UpdateCron; $mdlAcme->settings->UpdateCron = ""; From 417ad699e24a538acf107b56bb6f463f257e425d Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Tue, 17 Jun 2025 14:44:16 +0200 Subject: [PATCH 5/8] security/acme-client: add support for Websupport.sk DNS API, closes #4540 --- security/acme-client/pkg-descr | 1 + .../AcmeClient/forms/dialogValidation.xml | 15 +++++++ .../AcmeClient/LeValidation/DnsWebsupport.php | 44 +++++++++++++++++++ .../models/OPNsense/AcmeClient/AcmeClient.xml | 7 +++ 4 files changed, 67 insertions(+) create mode 100644 security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/DnsWebsupport.php diff --git a/security/acme-client/pkg-descr b/security/acme-client/pkg-descr index 4650976da..7bf1c46d5 100644 --- a/security/acme-client/pkg-descr +++ b/security/acme-client/pkg-descr @@ -12,6 +12,7 @@ Plugin Changelog Added: * new automation to reload www/caddy (#4692) +* add support for Websupport.sk DNS API (#4540) Changed: * automatically resolve cron job mismatch (#4627) diff --git a/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/forms/dialogValidation.xml b/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/forms/dialogValidation.xml index b0f4e09a2..d71044036 100644 --- a/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/forms/dialogValidation.xml +++ b/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/forms/dialogValidation.xml @@ -1779,6 +1779,21 @@ password + + + header + + + + validation.dns_websupport_api_key + + text + + + validation.dns_websupport_api_secret + + password + header diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/DnsWebsupport.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/DnsWebsupport.php new file mode 100644 index 000000000..83393e3eb --- /dev/null +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/DnsWebsupport.php @@ -0,0 +1,44 @@ +acme_env['WS_ApiKey'] = (string)$this->config->dns_websupport_api_key; + $this->acme_env['WS_ApiSecret'] = (string)$this->config->dns_websupport_api_secret; + } +} diff --git a/security/acme-client/src/opnsense/mvc/app/models/OPNsense/AcmeClient/AcmeClient.xml b/security/acme-client/src/opnsense/mvc/app/models/OPNsense/AcmeClient/AcmeClient.xml index b1efacf4a..4a3a31dcf 100644 --- a/security/acme-client/src/opnsense/mvc/app/models/OPNsense/AcmeClient/AcmeClient.xml +++ b/security/acme-client/src/opnsense/mvc/app/models/OPNsense/AcmeClient/AcmeClient.xml @@ -525,6 +525,7 @@ Variomedia.de Vscale Vultr + Websupport.sk World4You Yandex PDD Zilore @@ -1247,6 +1248,12 @@ N + + N + + + N + N From 4215dae9d51a59c10dac39dd144998161b0091e9 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Tue, 1 Jul 2025 16:02:47 +0200 Subject: [PATCH 6/8] security/acme-client: fix account config, closes #4622 --- security/acme-client/pkg-descr | 2 ++ .../library/OPNsense/AcmeClient/LeAccount.php | 29 ++++++++++--------- .../OPNsense/AcmeClient/LeCertificate.php | 2 ++ 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/security/acme-client/pkg-descr b/security/acme-client/pkg-descr index 7bf1c46d5..d9d83c57b 100644 --- a/security/acme-client/pkg-descr +++ b/security/acme-client/pkg-descr @@ -15,9 +15,11 @@ Added: * add support for Websupport.sk DNS API (#4540) Changed: +* automatically fix account config if CERT_HOME is set (#4622) * automatically resolve cron job mismatch (#4627) Fixed: +* deploy hooks may use the old CERT_HOME (#4622) * acme.sh is always called with "--days 1" (#4711) * avoid startup error: "rmdir... Not a directory" (#4743) * fails to create/update cron job on UUID mismatch (#4627) diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAccount.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAccount.php index e643696f2..0bf429dba 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAccount.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAccount.php @@ -1,7 +1,7 @@ fixConfig(); - // Update account status. LeUtils::log('account registration successful for ' . $this->config->name); $this->setStatus(200); @@ -234,6 +231,9 @@ class LeAccount extends LeCommon LeUtils::log_debug('account already registered: ' . (string)$this->config->name, $this->debug); } + // Always check (and fix) account config + $this->fixConfig(); + return true; } @@ -251,18 +251,21 @@ class LeAccount extends LeCommon // Parse config file and remove property $account_conf = parse_ini_file($account_conf_file); if (isset($account_conf['CERT_HOME'])) { + LeUtils::log('fixing invalid account config (CERT_HOME): ' . $this->config->name); unset($account_conf['CERT_HOME']); - } - // Convert array back to ini file format - $new_account_conf = array(); - foreach ($account_conf as $key => $value) { - $new_account_conf[] = "{$key}='{$value}'"; - } + // Convert array back to ini file format + $new_account_conf = array(); + foreach ($account_conf as $key => $value) { + $new_account_conf[] = "{$key}='{$value}'"; + } - // Write changes back to file - file_put_contents($account_conf_file, implode("\n", $new_account_conf) . "\n"); - chmod($account_conf_file, 0600); + // Write changes back to file + file_put_contents($account_conf_file, implode("\n", $new_account_conf) . "\n"); + chmod($account_conf_file, 0600); + } else { + LeUtils::log('account config is valid (CERT_HOME): ' . $this->config->name); + } } } } diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php index edc3c2356..ff4361337 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php @@ -618,6 +618,8 @@ class LeCertificate extends LeCommon $this->loadConfig(self::CONFIG_PATH, $this->uuid); } LeUtils::log('account is registered: ' . (string)$account->config->name); + // Always check (and fix) account config + $account->fixConfig(); return true; } From f28fd30af5fa6b73b4d37c1158b2e4785bb6de11 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Tue, 1 Jul 2025 17:44:33 +0200 Subject: [PATCH 7/8] security/acme-client: add SFTP option to (not) preserve mod time, refs #3862 --- security/acme-client/pkg-descr | 2 ++ .../AcmeClient/forms/dialogAction.xml | 6 +++++ .../OPNsense/AcmeClient/SftpUploader.php | 26 +++++++++++++++---- .../models/OPNsense/AcmeClient/AcmeClient.xml | 6 ++++- 4 files changed, 34 insertions(+), 6 deletions(-) diff --git a/security/acme-client/pkg-descr b/security/acme-client/pkg-descr index d9d83c57b..8f7f21ace 100644 --- a/security/acme-client/pkg-descr +++ b/security/acme-client/pkg-descr @@ -13,10 +13,12 @@ Plugin Changelog Added: * new automation to reload www/caddy (#4692) * add support for Websupport.sk DNS API (#4540) +* add SFTP option: Preserve Modification Time (#3862) Changed: * automatically fix account config if CERT_HOME is set (#4622) * automatically resolve cron job mismatch (#4627) +* change default SFTP options to NOT preserve modification time (#3862) Fixed: * deploy hooks may use the old CERT_HOME (#4622) diff --git a/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/forms/dialogAction.xml b/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/forms/dialogAction.xml index a2fb434eb..292fd9c23 100644 --- a/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/forms/dialogAction.xml +++ b/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/forms/dialogAction.xml @@ -68,6 +68,12 @@ The path can be absolute or relative to home and must exist. Leave blank to not change path after login. + + action.sftp_modtime + + checkbox + Preserves modification times from the source file. Note that this is not supported by all SFTP servers and may cause the upload to fail, e.g. on VMware + action.sftp_chmod diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpUploader.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpUploader.php index 353495880..d7b5df79a 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpUploader.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpUploader.php @@ -1,6 +1,7 @@ $local_file, "target" => $remote_file, "mode" => $chmod, - "group" => $chgrp + "group" => $chgrp, + "modtime" => $modtime ]; return $local_file; @@ -94,9 +99,10 @@ class SftpUploader * @param int $content_last_modified the unix timestamp when the content was last modified (is preserved when chmod is also specified). * @param bool $chmod the 4 digit unix permission to apply or false to leave it unchanged. * @param bool $chgrp the numeric group on the remote server to apply or false to leave it unchanged. + * @param bool $modtime a boolean to indicate that the modification times should be preserved. * @return string the name of the remote file. */ - public function addContent(string $content, string $remote_file = "", $content_last_modified = 0, $chmod = false, $chgrp = false): string + public function addContent(string $content, string $remote_file = "", $content_last_modified = 0, $chmod = false, $chgrp = false, $modtime = false): string { $local_file = $this->temporaryFile(); Utils::requireThat($local_file, "Failed creating temporary file for '$remote_file'"); @@ -113,7 +119,7 @@ class SftpUploader $remote_file = basename($local_file); } - $local_file = $this->addFile($local_file, $remote_file, $chmod, $chgrp); + $local_file = $this->addFile($local_file, $remote_file, $chmod, $chgrp, $modtime); $this->pending_files[$local_file]["delete_source"] = true; return $remote_file; @@ -252,16 +258,24 @@ class SftpUploader $chmod = $file["mode"] ?? ""; $chmod = preg_match('/^0\d{3}$/', $chmod) ? (string)$chmod : false; + // Preserving the modification time is not supported by all SFTP servers. + $preserve = $file["modtime"]; + if ($preserve !== false) { + LeUtils::log("Sftp upload will try to preserve file modification time"); + } else { + LeUtils::log("Sftp upload will not preserve file modification time"); + } // Initial upload when permissions are properly set. $should_upload_with_permission_change = $chmod !== false && isset($remote_files[$remote_filename]); + // Upload file. if (!$remote_is_readonly) { $preserve_times_and_mod = $chmod !== false; - if ($error = $this->sftp->put($local_file, $remote_filename, $preserve_times_and_mod)->lastError()) { + if ($error = $this->sftp->put($local_file, $remote_filename, $preserve)->lastError()) { if ($error["permission_denied"] !== true) { $should_upload_with_permission_change = false; } @@ -281,11 +295,13 @@ class SftpUploader if ($should_upload_with_permission_change && $this->isFileOwnedByConnection($remote_file, $connection)) { Utils::log()->info("Trying to upload file '{$local_file}' to '{$file["target"]}' with adjusted permissions"); + // Change file permission to make it writable. if ($error = $this->sftp->chmod($remote_filename, '0600')->lastError()) { Utils::log()->error("Failed changing permission to '0600' for '{$file["target"]}'. ", $error); $this->sftp->clearError(); } + // Try again to upload file. if ($error = $this->sftp->put($local_file, $remote_filename)->lastError()) { Utils::log()->error("Failed uploading file (with adjusted permissions) '{$local_file}' to '{$file["target"]}'", $error); return self::UPLOAD_ERROR_NO_PERMISSION; diff --git a/security/acme-client/src/opnsense/mvc/app/models/OPNsense/AcmeClient/AcmeClient.xml b/security/acme-client/src/opnsense/mvc/app/models/OPNsense/AcmeClient/AcmeClient.xml index 4a3a31dcf..8d4b88475 100644 --- a/security/acme-client/src/opnsense/mvc/app/models/OPNsense/AcmeClient/AcmeClient.xml +++ b/security/acme-client/src/opnsense/mvc/app/models/OPNsense/AcmeClient/AcmeClient.xml @@ -1,6 +1,6 @@ //OPNsense/AcmeClient - 4.2.0 + 4.3.0 A secure ACME Client plugin @@ -1410,6 +1410,10 @@ /^0[0-9]{3}$/u A unix permission, 4 digits (e.g. 0400). + + N + 0 + N /^(?![\/\\])[\w\d_\-@.\/{}%]{1,255}(?<![\/\\])$/ui From e3717e63888680c3765bd245a9e48cf1a0618052 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Wed, 2 Jul 2025 14:17:18 +0200 Subject: [PATCH 8/8] security/acme-client: assorted logging enhancements --- security/acme-client/pkg-descr | 6 ++ .../library/OPNsense/AcmeClient/LeAccount.php | 18 ++--- .../OPNsense/AcmeClient/LeAutomation/Base.php | 6 +- .../OPNsense/AcmeClient/LeCertificate.php | 6 +- .../library/OPNsense/AcmeClient/LeUtils.php | 16 +++- .../OPNsense/AcmeClient/LeValidation/Base.php | 2 +- .../library/OPNsense/AcmeClient/Process.php | 9 ++- .../library/OPNsense/AcmeClient/SSHKeys.php | 31 ++++---- .../OPNsense/AcmeClient/SftpClient.php | 15 ++-- .../OPNsense/AcmeClient/SftpUploader.php | 41 +++++----- .../app/library/OPNsense/AcmeClient/Utils.php | 79 ++----------------- .../OPNsense/AcmeClient/run_remote_ssh.php | 22 +++--- .../OPNsense/AcmeClient/upload_sftp.php | 39 ++++----- 13 files changed, 129 insertions(+), 161 deletions(-) diff --git a/security/acme-client/pkg-descr b/security/acme-client/pkg-descr index 8f7f21ace..05a12c7b7 100644 --- a/security/acme-client/pkg-descr +++ b/security/acme-client/pkg-descr @@ -19,6 +19,9 @@ Changed: * automatically fix account config if CERT_HOME is set (#4622) * automatically resolve cron job mismatch (#4627) * change default SFTP options to NOT preserve modification time (#3862) +* migrate SFTP/SSH operations to AcmeClient logging +* add detailed SFTP/SSH logging when debug logging is enabled +* add new log messages and improve existing ones Fixed: * deploy hooks may use the old CERT_HOME (#4622) @@ -26,6 +29,9 @@ Fixed: * avoid startup error: "rmdir... Not a directory" (#4743) * fails to create/update cron job on UUID mismatch (#4627) +Removed: +* remove stdout (CLI) logging from SFTP library + 4.9 Added: diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAccount.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAccount.php index 0bf429dba..4eef9200a 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAccount.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAccount.php @@ -94,17 +94,17 @@ class LeAccount extends LeCommon // Check if account key already exists both in filesystem and in config if (!is_file($account_key_file) || empty((string)$this->config->key)) { - LeUtils::log_debug('creating account key for ' . (string)$this->config->name, $this->debug); + LeUtils::log_debug('creating account key for ' . (string)$this->config->name); // Check if we have an account key in our configuration if (!empty((string)$this->config->key)) { - LeUtils::log_debug('exporting existing account key to filesystem for ' . (string)$this->config->name, $this->debug); + LeUtils::log_debug('exporting existing account key to filesystem for ' . (string)$this->config->name); // Write key to disk file_put_contents($account_key_file, (string)base64_decode((string)$this->config->key)); chmod($account_key_file, 0600); return true; } else { - LeUtils::log_debug('generating a new account key for ' . (string)$this->config->name, $this->debug); + LeUtils::log_debug('generating a new account key for ' . (string)$this->config->name); // Preparation to run acme client $proc_env = $this->acme_env; // add env variables @@ -116,7 +116,7 @@ class LeAccount extends LeCommon . implode(' ', $this->acme_args) . ' ' . LeUtils::execSafe('--accountkeylength %s', self::ACME_ACCOUNT_KEY_LENGTH) . ' ' . LeUtils::execSafe('--accountconf %s', $account_conf_file); - LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd, $this->debug); + LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd); // Run acme.sh command $result = LeUtils::run_shell_command($acmecmd, $proc_env); @@ -156,7 +156,7 @@ class LeAccount extends LeCommon LeUtils::log_error('failed to save account key for ' . (string)$this->config->name); return false; } - LeUtils::log_debug('successfully created account key for ' . (string)$this->config->name, $this->debug); + LeUtils::log_debug('successfully created account key for ' . (string)$this->config->name); return true; } } @@ -194,11 +194,11 @@ class LeAccount extends LeCommon // Check if account is already registered if (!($this->isRegistered())) { - LeUtils::log_debug('starting account registration for ' . (string)$this->config->name, $this->debug); + LeUtils::log_debug('starting account registration for ' . (string)$this->config->name); // Check if ACME External Account Binding (EAB) is enabled if (!empty((string)$this->config->eab_kid) && !empty((string)$this->config->eab_hmac)) { - LeUtils::log_debug('enabling ACME EAB for this account', $this->debug); + LeUtils::log_debug('enabling ACME EAB for this account'); $this->acme_args[] = LeUtils::execSafe('--eab-kid %s', $this->config->eab_kid); $this->acme_args[] = LeUtils::execSafe('--eab-hmac-key %s', $this->config->eab_hmac); } @@ -212,7 +212,7 @@ class LeAccount extends LeCommon . '--registeraccount ' . implode(' ', $this->acme_args) . ' ' . LeUtils::execSafe('--accountconf %s', $this->account_conf_file); - LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd, $this->debug); + LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd); // Run acme.sh command $result = LeUtils::run_shell_command($acmecmd, $proc_env); @@ -228,7 +228,7 @@ class LeAccount extends LeCommon LeUtils::log('account registration successful for ' . $this->config->name); $this->setStatus(200); } else { - LeUtils::log_debug('account already registered: ' . (string)$this->config->name, $this->debug); + LeUtils::log_debug('account already registered: ' . (string)$this->config->name); } // Always check (and fix) account config diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAutomation/Base.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAutomation/Base.php index 138d64d81..0571a8e08 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAutomation/Base.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeAutomation/Base.php @@ -1,7 +1,7 @@ * All rights reserved. @@ -131,7 +131,7 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon . implode(' ', $this->acme_args); // Run acme.sh command - LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd, $this->debug); + LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd); $result = LeUtils::run_shell_command($acmecmd, $proc_env); // acme.sh records the last used deploy hook and would automatically @@ -156,7 +156,7 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon if (!file_put_contents($filename, $contents)) { LeUtils::log_error('clearing recorded deploy hook from acme.sh failed (' . $filename . ')'); } else { - LeUtils::log_debug('cleared recorded deploy deploy hook from acme.sh (' . $filename . ')', $this->debug); + LeUtils::log_debug('cleared recorded deploy deploy hook from acme.sh (' . $filename . ')'); } } } diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php index ff4361337..e45f92ded 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeCertificate.php @@ -358,7 +358,7 @@ class LeCertificate extends LeCommon $configdir = (string)sprintf(self::ACME_CONFIG_DIR, (string)$this->config->id); foreach (array($certdir, $keydir, $configdir) as $dir) { if (!is_dir($dir)) { - LeUtils::log_debug("creating directory: {$dir}", $this->debug); + LeUtils::log_debug("creating directory: {$dir}"); mkdir($dir, 0700, true); } } @@ -467,7 +467,7 @@ class LeCertificate extends LeCommon . '--remove ' . implode(' ', $this->acme_args) . ' ' . LeUtils::execSafe('--domain %s', (string)$this->config->name); - LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd, $this->debug); + LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd); // Run acme.sh command $result = LeUtils::run_shell_command($acmecmd, $proc_env); @@ -542,7 +542,7 @@ class LeCertificate extends LeCommon . implode(' ', $this->acme_args) . ' ' . LeUtils::execSafe('--domain %s', (string)$this->config->name) . ' ' . LeUtils::execSafe('--accountconf %s', $account_conf_file); - LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd, $this->debug); + LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd); // Run acme.sh command $result = LeUtils::run_shell_command($acmecmd, $proc_env); diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeUtils.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeUtils.php index 6dfd33eff..ec51a0177 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeUtils.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeUtils.php @@ -32,6 +32,7 @@ namespace OPNsense\AcmeClient; use OPNsense\Core\Config; +use OPNsense\AcmeClient\AcmeClient; /** * Helper functions for LeAcme @@ -81,8 +82,13 @@ class LeUtils /** * log additional debug output */ - public static function log_debug($msg, bool $debug = false) + public static function log_debug($msg) { + $log_config = (new AcmeClient())->getNodeByReference('settings.logLevel'); + if (strpos($log_config, "debug") !== false) { + $debug = true; + } + if ($debug) { syslog(LOG_NOTICE, "AcmeClient: {$msg}"); } @@ -91,9 +97,13 @@ class LeUtils /** * log error messages */ - public static function log_error($msg) + public static function log_error($msg, $error = null) { - syslog(LOG_ERR, "AcmeClient: {$msg}"); + syslog(LOG_ERR, + $error + ? ("AcmeClient: $msg; Trace: " . json_encode($error, JSON_UNESCAPED_SLASHES)) + : "AcmeClient: $msg" + ); } /** diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/Base.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/Base.php index 1946ab3b9..6163abadd 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/Base.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/LeValidation/Base.php @@ -167,7 +167,7 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon . "--{$acme_action} " . implode(' ', $this->acme_args) . ' ' . LeUtils::execSafe('--accountconf %s', $account_conf_file); - LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd, $this->debug); + LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd); // Run acme.sh command $result = LeUtils::run_shell_command($acmecmd, $proc_env); diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/Process.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/Process.php index 59ed4808f..0a0b150fe 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/Process.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/Process.php @@ -1,6 +1,7 @@ error("Terminating process: " . json_encode(proc_get_status($handle))); + LeUtils::log_error("Terminating process: " . json_encode(proc_get_status($handle))); @proc_terminate($handle); } } @@ -102,7 +105,7 @@ class Process self::manageOpenedProcess($this->handle); } else { - Utils::log()->error("Failed opening '$cmd' in '$cwd'"); + LeUtils::log_error("Failed opening '$cmd' in '$cwd'"); } } @@ -181,7 +184,7 @@ class Process { // Read up-to 10k remaining lines from STDOUT/ERR to release locks before closing. for ($i = 0; ($line = $this->get(0)) && $i < 10000; $i++) { - Utils::log()->error("WARN: process: $line"); + LeUtils::log_error("WARN: process: $line"); } if ($this->isRunning()) { diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SSHKeys.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SSHKeys.php index 3d91cef69..ac70602d2 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SSHKeys.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SSHKeys.php @@ -1,6 +1,7 @@ info("No host key specified, using existing known_hosts entry for '$host'"); + LeUtils::log_debug("No host key specified, using existing known_hosts entry for '$host'"); $host_key = $known_by_host["key_info"]; } else { - Utils::log()->info("No host key specified and existing entry for '$host' cannot be used as isn't matching port $port."); + LeUtils::log_debug("No host key specified and existing entry for '$host' cannot be used as isn't matching port $port."); } } @@ -165,7 +168,7 @@ class SSHKeys $is_key_known = true; } elseif ($known_by_key) { if (strcasecmp(trim($host), trim($known_by_key["host"])) != 0) { - Utils::log()->info("Host key is in known_hosts but hostname differs. Changing '$host' to '{$known_by_key["host"]}'."); + LeUtils::log_debug("Host key is in known_hosts but hostname differs. Changing '$host' to '{$known_by_key["host"]}'."); $host = $known_by_key["host"]; } $is_key_known = true; @@ -196,15 +199,15 @@ class SSHKeys if (!empty($matching_remote_host_keys)) { if ($known_by_host && $known_by_host_matches_port) { - Utils::log()->info("Removing known_hosts entry with differing key for '{$known_by_host["host_query"]}' as it is in the way."); + LeUtils::log_debug("Removing known_hosts entry with differing key for '{$known_by_host["host_query"]}' as it is in the way."); $this->removeKnownHost($known_by_host["host_query"]); } foreach ($matching_remote_host_keys as $key) { - Utils::log()->info("Adding known_hosts entry: " . json_encode($key["key_info"], JSON_UNESCAPED_SLASHES)); + LeUtils::log_debug("Adding known_hosts entry: " . json_encode($key["key_info"], JSON_UNESCAPED_SLASHES)); $ok = file_put_contents($this->knownHostsFile(), $key["host_key"] . PHP_EOL, FILE_APPEND); if (!$ok) { - Utils::log()->error("Failed adding known_hosts entry {$key["host_key"]}"); + LeUtils::log_error("Failed adding SSH known_hosts entry {$key["host_key"]}"); } } @@ -331,12 +334,12 @@ class SSHKeys ? "" : PHP_EOL . "ssh-keyscan: " . join(PHP_EOL . "ssh-keyscan: ", $lines); - Utils::log()->error("Failed querying host keys ($key_type) for [$names] port $port. Exit code: {$p->exitCode} (error-marker: $marker) $output"); + LeUtils::log_error("Failed querying SSH host keys ($key_type) for [$names] port $port. Exit code: {$p->exitCode} (error-marker: $marker) $output"); } } if (empty($keys)) { - Utils::log()->info("Couldn't fetch public host key ($key_type) from {$host}:{$port}"); + LeUtils::log_debug("Couldn't fetch public host key ($key_type) from {$host}:{$port}"); if (!is_array($error) || empty($error)) { $error = ["connection_refused" => true]; @@ -384,13 +387,13 @@ class SSHKeys ? "" : PHP_EOL . join(PHP_EOL, $lines); - Utils::log()->error("Failed querying known hosts for $name_or_ip ($host). Exit code: {$p->exitCode} $output"); + LeUtils::log_error("Failed querying SSH known hosts for $name_or_ip ($host). Exit code: {$p->exitCode} $output"); } } } if (empty($keys)) { - Utils::log()->info("Didn't find $host in known_hosts"); + LeUtils::log_debug("Could not find $host in known_hosts"); } return $keys; @@ -408,7 +411,7 @@ class SSHKeys if ($p = Process::open(["ssh-keygen", "-R", $host, "-f", $this->knownHostsFile()])) { $ok = $p->close() === 0; if (!$ok) { - Utils::log()->error("Failed removing known hosts for $host. Return code was: {$p->exitCode}"); + LeUtils::log_error("Failed removing SSH known hosts for $host. Return code was: {$p->exitCode}"); } } @@ -433,11 +436,11 @@ class SSHKeys "key_length" => $matches[1] ]; } else { - Utils::log()->error("Unsupported hash type: $hash"); + LeUtils::log_error("Unsupported hash type: $hash"); } } - Utils::log()->error("Failed getting hash for host_key"); + LeUtils::log_error("Failed getting hash for host_key"); return false; } @@ -472,7 +475,7 @@ class SSHKeys if ($p = Process::open($generate_key)) { while (($line = $p->get(10)) !== false) { - Utils::log()->info("SSH keygen: $line"); + LeUtils::log_debug("SSH keygen: $line"); } Utils::requireThat( diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpClient.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpClient.php index 1804596af..ff3982dc6 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpClient.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpClient.php @@ -1,6 +1,7 @@ failed_status = ["invalid_parameters" => true]; - Utils::log()->error("Failed connecting to '$host'. Hostname or username is missing."); + LeUtils::log_error("Failed connecting to '$host'. Hostname or username is missing."); return false; } $trust = $this->ssh_keys->trustHost($host, $host_key, $port); if ($trust["ok"] !== true) { - Utils::log()->error("Failed establishing trust in '$host'; Cause: {$trust["error"]}"); + LeUtils::log_error("Failed establishing trust in '$host'; Cause: {$trust["error"]}"); unset($trust["ok"]); $this->failed_status = array_merge($trust, ["host_not_trusted" => true]); return false; @@ -103,7 +106,7 @@ class SftpClient "-oPreferredAuthentications=publickey" ); } else { - Utils::log()->error("Failed adding client identity ($identity). Connect will likely fail."); + LeUtils::log_error("Failed adding client identity ($identity). Connect will likely fail."); } // Adding the host @@ -113,7 +116,7 @@ class SftpClient if ($this->process = Process::open($cmd)) { $this->processAvailableInput(self::CONNECT_REPLY_TIMEOUT, 1, null, 0.75); if (($error = $this->lastError()) || !$this->process->isRunning()) { - Utils::log()->error("Failed connecting to '$host' (user: '$username')", $error); + LeUtils::log_error("Failed connecting to '$host' (user: '$username')", $error); return false; } $this->connection_info = ["host" => $host, "port" => $port, "user" => $username]; @@ -151,7 +154,7 @@ class SftpClient $consumed = ($lines_consumer && $lines_consumer($line) === true); if (!$consumed) { - Utils::log()->info("SFTP: " . rtrim($line)); + LeUtils::log_debug("SFTP: " . rtrim($line)); } if (!$lines_consumer || $consumed) { @@ -286,7 +289,7 @@ class SftpClient . (empty($remote_file) ? "" : " " . escapeshellarg($remote_file))); $this->processAvailableInput(self::COMMAND_REPLY_TIMEOUT, 2); } else { - Utils::log()->info("put: File $local_file doesn't exist."); + LeUtils::log_debug("put: File $local_file doesn't exist."); $this->failed_status = ["file_not_found" => true, "error" => $local_file]; } diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpUploader.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpUploader.php index d7b5df79a..bfd4b1885 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpUploader.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpUploader.php @@ -159,7 +159,7 @@ class SftpUploader ->lastError(); if ($error) { - Utils::log()->error("Cannot continue since changing to initial remote path '{$this->pending_base_path}' failed", $error); + LeUtils::log_error("Cannot continue with SFTP upload since changing to initial remote path '{$this->pending_base_path}' failed", $error); return self::UPLOAD_ERROR; } } @@ -189,7 +189,7 @@ class SftpUploader try { $connection = $this->sftp->connected(); if (!$connection) { - Utils::log()->error("The sftp client is not connected, upload stopped."); + LeUtils::log_error("The SFTP client is not connected, upload stopped."); return self::UPLOAD_ERROR; } @@ -211,7 +211,7 @@ class SftpUploader foreach ($dir_names as $dir) { if ($error = $this->sftp->cd($dir)->lastError()) { if ($error["file_not_found"]) { - Utils::log()->info("Creating remote directory: $dir"); + LeUtils::log_debug("SFTP creating remote directory: $dir"); $this->sftp->clearError() ->mkdir($dir) ->cd($dir); @@ -222,7 +222,7 @@ class SftpUploader } if ($error = $this->sftp->lastError()) { - Utils::log()->error("Failed to cd into '$target_dir'.", $error); + LeUtils::log_error("SFTP failed to cd into '$target_dir'", $error); return self::UPLOAD_ERROR; } @@ -234,7 +234,7 @@ class SftpUploader if (empty($remote_files)) { $remote_files = $this->sftp->clearError()->ls(); if ($error = $this->sftp->lastError()) { - Utils::log()->error("Failed listing remote files.", $error); + LeUtils::log_error("SFTP failed listing remote files", $error); return self::UPLOAD_ERROR; } } @@ -245,10 +245,11 @@ class SftpUploader $remote_file = $remote_files[$remote_filename] ?? ["type" => "-", "owner" => $username]; $remote_is_file = $remote_file["type"] === "-"; $remote_is_readonly = preg_match('/^[^wW]+$/', $remote_file["permissions"] ?? ""); + LeUtils::log_debug("SFTP current remote file permissions: '{$remote_file["permissions"]}'"); // Check if a folder/socket/symlink, etc is in the way if (!$remote_is_file) { - Utils::log()->error("Failed uploading file '{$local_file}' as there is a non-file in the way at '{$file["target"]}'"); + LeUtils::log_error("SFTP failed uploading file '{$local_file}' as there is a non-file in the way at '{$file["target"]}'"); return self::UPLOAD_ERROR_NO_OVERWRITE; } @@ -261,9 +262,9 @@ class SftpUploader // Preserving the modification time is not supported by all SFTP servers. $preserve = $file["modtime"]; if ($preserve !== false) { - LeUtils::log("Sftp upload will try to preserve file modification time"); + LeUtils::log("SFTP upload will try to preserve file modification time for '{$file["target"]}'"); } else { - LeUtils::log("Sftp upload will not preserve file modification time"); + LeUtils::log("SFTP upload will not preserve file modification time for '{$file["target"]}'"); } // Initial upload when permissions are properly set. @@ -273,6 +274,7 @@ class SftpUploader // Upload file. if (!$remote_is_readonly) { + LeUtils::log("Uploading file '{$local_file}' to '{$file["target"]}'"); $preserve_times_and_mod = $chmod !== false; if ($error = $this->sftp->put($local_file, $remote_filename, $preserve)->lastError()) { @@ -283,7 +285,7 @@ class SftpUploader if ($should_upload_with_permission_change) { $this->sftp->clearError(); } else { - Utils::log()->error("Failed uploading file '{$local_file}' to '{$file["target"]}'", $error); + LeUtils::log_error("SFTP failed uploading file '{$local_file}' to '{$file["target"]}'", $error); return self::UPLOAD_ERROR_NO_PERMISSION; } } else { @@ -293,21 +295,21 @@ class SftpUploader // Second attempt when initial failed or was skipped due to write protection (only possible if we have chmod defined to reset permissions later) if ($should_upload_with_permission_change && $this->isFileOwnedByConnection($remote_file, $connection)) { - Utils::log()->info("Trying to upload file '{$local_file}' to '{$file["target"]}' with adjusted permissions"); + LeUtils::log("Trying to upload file '{$local_file}' to '{$file["target"]}' with adjusted permissions"); // Change file permission to make it writable. if ($error = $this->sftp->chmod($remote_filename, '0600')->lastError()) { - Utils::log()->error("Failed changing permission to '0600' for '{$file["target"]}'. ", $error); + LeUtils::log_error("SFTP failed changing permission to '0600' for '{$file["target"]}'", $error); $this->sftp->clearError(); } // Try again to upload file. - if ($error = $this->sftp->put($local_file, $remote_filename)->lastError()) { - Utils::log()->error("Failed uploading file (with adjusted permissions) '{$local_file}' to '{$file["target"]}'", $error); + if ($error = $this->sftp->put($local_file, $remote_filename, $preserve)->lastError()) { + LeUtils::log_error("SFTP failed uploading file (with adjusted permissions) '{$local_file}' to '{$file["target"]}'", $error); return self::UPLOAD_ERROR_NO_PERMISSION; } } elseif ($remote_is_readonly) { - Utils::log()->error("Failed uploading file '{$local_file}' to '{$file["target"]}'. Existing file is write protected."); + LeUtils::log_error("SFTP failed uploading file '{$local_file}' to '{$file["target"]}'. Existing file is write protected."); return self::UPLOAD_ERROR_NO_PERMISSION; } @@ -315,14 +317,14 @@ class SftpUploader // Applying chmod / chgrp if requested. if ($chmod) { if ($error = $this->sftp->chmod($remote_filename, $chmod)->lastError()) { - Utils::log()->error("Failed chmod ($chmod) for '{$file["target"]}'", $error); + LeUtils::log_error("SFTP failed chmod ($chmod) for '{$file["target"]}'", $error); return self::UPLOAD_ERROR_CHMOD_FAILED; } } if ($chgrp) { if ($error = $this->sftp->chgrp($remote_filename, $chgrp)->lastError()) { - Utils::log()->error("Failed chgrp ($chgrp) for '{$file["target"]}'", $error); + LeUtils::log_error("SFTP failed chgrp ($chgrp) for '{$file["target"]}'", $error); return self::UPLOAD_ERROR_CHGRP_FAILED; } } @@ -359,8 +361,9 @@ class SftpUploader $this->sftp->clearError(); - if ($error = $this->sftp->put($local_test_file, $remote_test_file)->lastError()) { - Utils::log()->error("Failed uploading test file to detect ownership. Next uploads may fail as well.", $error); + // Perform test upload without preserving file modification time (extra safekeeping). + if ($error = $this->sftp->put($local_test_file, $remote_test_file, false)->lastError()) { + LeUtils::log_error("Failed uploading SFTP test file to detect ownership. Next uploads may fail as well", $error); } else { // Get owner of the test file $file_info = $this->sftp->ls()[$remote_test_file] ?? ["owner" => -1]; @@ -416,7 +419,7 @@ class SftpUploader } if ($count > 0) { - Utils::log()->info("Removed $count files in shutdown hook instead of object destruction."); + LeUtils::log_debug("Removed $count files in shutdown hook instead of object destruction."); } $shared_temporary_files = []; diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/Utils.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/Utils.php index b5842d58f..a06e7b806 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/Utils.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/Utils.php @@ -1,6 +1,7 @@ error("FATAL: $message"); + LeUtils::log_error("FATAL: $message"); throw new \AssertionError($message); } return $expression; @@ -193,7 +133,6 @@ class Utils { static $options = [ "-h, --help Print commandline help", - "--log Enable log to stdout (instead of syslog)", "--automation-id Read options from the action specified by id or uuid", "--no-error Always exit with 0 (original exit codes are still logged)", ]; @@ -241,7 +180,7 @@ class Utils { global $argv; $command = self::getSelectedCLICommand($commands); - $options = ["help", "log", "no-error"]; + $options = ["help", "no-error"]; $has_automation_id = preg_match('/--automation-id=\S+/', join(" ", $argv)); if ($has_automation_id) { @@ -255,10 +194,6 @@ class Utils if (isset($options["h"]) || isset($options["help"])) { $help(); } else { - if (isset($options["log"])) { - self::log(true)->info("Logging to stdout enabled"); - } - $options = array_filter($options, function ($value) { return !is_string($value) || (!empty($value = trim($value)) && $value !== "__default_value"); @@ -268,7 +203,7 @@ class Utils if (is_array($config = $optionsByActionId($options["automation-id"]))) { $options = array_merge($config, $options); } else { - self::log()->error("No usable config found for automation-id {$options["automation-id"]}"); + LeUtils::log_error("No usable config found for automation-id {$options["automation-id"]}"); exit(1); } } @@ -277,7 +212,7 @@ class Utils $code = $runner($options); if ($code != $exit_success) { - self::log()->error("Command execution failed, exit code $code. Last input was: " . json_encode($options, JSON_UNESCAPED_SLASHES)); + LeUtils::log_error("Command execution failed, exit code $code. Last input was: " . json_encode($options, JSON_UNESCAPED_SLASHES)); } exit(isset($options["no-error"]) ? $exit_success : $code); @@ -290,7 +225,7 @@ class Utils $help(); } else { $cmd = join(" ", $argv); - self::log()->error("Parsing of '$cmd' failed at argument '{$argv[$index]}'"); + LeUtils::log_error("Parsing of '$cmd' failed at argument '{$argv[$index]}'"); } exit(1); } diff --git a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/run_remote_ssh.php b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/run_remote_ssh.php index 019c44d81..5db3dd072 100755 --- a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/run_remote_ssh.php +++ b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/run_remote_ssh.php @@ -2,6 +2,7 @@ error("Failed getting identity. See log output for details."); + LeUtils::log_error("SSH failed getting identity. See log output for details."); } return EXITCODE_ERROR; } @@ -156,14 +158,14 @@ function commandTestConnection(array &$options): int function commandRunRemote(array &$options): int { if (empty($options["run"])) { - Utils::log()->error("SSH: Command is empty, nothing to do."); + LeUtils::log_error("SSH: Command is empty, nothing to do."); return EXITCODE_ERROR; } $lines = runRemoteCommand($options, $error); if (!$error) { $host = $options["host"] . (($port = ($options["port"] ?? false)) ? ":$port" : ""); - Utils::log()->info("SSH [$host]> {$options["run"]}:" . PHP_EOL . join(PHP_EOL, $lines)); + LeUtils::log_debug("SSH [$host]> {$options["run"]}:" . PHP_EOL . join(PHP_EOL, $lines)); return EXITCODE_SUCCESS; } @@ -244,7 +246,7 @@ function runRemoteCommand(array $options, &$error): ?array "exit_code" => $exit_code ]); $error["connect_failed"] = $exit_code == 255; - Utils::log()->error("SSH failed with '$exit_code': $cl", $error); + LeUtils::log_error("SSH failed with '$exit_code': $cl", $error); } return $result; @@ -253,7 +255,7 @@ function runRemoteCommand(array $options, &$error): ?array function buildSSHArguments(SSHKeys $ssh_keys, $host, $username, $identity_type = "", $host_key = "", $port = SSHKeys::DEFAULT_PORT): array { if (empty(trim($host)) || empty(trim($username))) { - Utils::log()->error("Failed connecting to '$host'. Hostname or username is missing."); + LeUtils::log_error("Failed connecting to '$host'. Hostname or username is missing."); return [false, ["invalid_parameters" => true]]; } @@ -263,7 +265,7 @@ function buildSSHArguments(SSHKeys $ssh_keys, $host, $username, $identity_type = $trust = $ssh_keys->trustHost($host, $host_key, $port); if ($trust["ok"] !== true) { - Utils::log()->error("Failed establishing trust in '$host'; Cause: {$trust["error"]}"); + LeUtils::log_error("Failed establishing trust in '$host'; Cause: {$trust["error"]}"); unset($trust["ok"]); return [false, array_merge($trust, ["host_not_trusted" => true])]; } else { @@ -288,7 +290,7 @@ function buildSSHArguments(SSHKeys $ssh_keys, $host, $username, $identity_type = "-oPreferredAuthentications=publickey" ); } else { - Utils::log()->error("Failed adding client identity ($identity). Connect will likely fail."); + LeUtils::log_error("Failed adding SSH client identity ($identity). Connect will likely fail."); } // Adding the host @@ -304,7 +306,7 @@ function help() function getOptionsById($automation_id) { - Utils::log()->info("Reading options from automation: $automation_id"); + LeUtils::log_debug("Reading options from automation: $automation_id"); if (is_object($action = Utils::getAutomationActionById($automation_id))) { if ($action->enabled && "configd_remote_ssh" === (string)$action->type) { @@ -317,10 +319,10 @@ function getOptionsById($automation_id) "run" => trim((string)$action->remote_ssh_command), ]; } else { - Utils::log()->error("Ignoring disabled or invalid automation '$automation_id'"); + LeUtils::log_error("Ignoring disabled or invalid automation '$automation_id'"); } } else { - Utils::log()->error("No upload automation found with uuid = '$automation_id'"); + LeUtils::log_error("No upload automation found with uuid = '$automation_id'"); } return false; diff --git a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/upload_sftp.php b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/upload_sftp.php index 2f42e46e5..b3510b909 100755 --- a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/upload_sftp.php +++ b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/upload_sftp.php @@ -142,6 +142,7 @@ use OPNsense\AcmeClient\SftpUploader; use OPNsense\AcmeClient\SftpClient; use OPNsense\AcmeClient\SSHKeys; use OPNsense\AcmeClient\Utils; +use OPNsense\AcmeClient\LeUtils; // Implementing logic function commandShowIdentity(array &$options): int @@ -162,7 +163,7 @@ function commandShowIdentity(array &$options): int echo file_get_contents($id_file); return EXITCODE_SUCCESS; } else { - Utils::log()->error("Failed getting identity. See log output for details."); + LeUtils::log_error("SFTP failed getting identity. See log output for details."); } return EXITCODE_ERROR; } @@ -214,7 +215,7 @@ function commandTestConnection(array &$options): int if ($remove_file) { if ($error = $sftp->clearError()->rm($filename)->lastError(3)) { - Utils::log()->error("Failed removing upload test file '$filename'", $error); + LeUtils::log_error("SFTP failed removing upload test file '$filename'", $error); } } @@ -261,7 +262,7 @@ function commandUpload(array &$options): int } elseif (isset($options["host"])) { return uploadCertificatesToHost($options); } else { - Utils::log()->error("No work to do, neither --host nor --certificates is present."); + LeUtils::log_error("No work to do, neither --host nor --certificates is present."); return EXITCODE_ERROR_NOTHING_TO_UPLOAD; } } @@ -270,7 +271,7 @@ function uploadCertificatesToHost(array $options): int { $sftp = connectWithServer($options, $error); if ($sftp === null) { - Utils::log()->error("Aborting after connect failure."); + LeUtils::log_error("SFTP aborting after connect failure."); return ($error["connect_failed"] ?? false) ? EXITCODE_ERROR : EXITCODE_ERROR_NO_PERMISSION; @@ -289,7 +290,7 @@ function uploadCertificatesToHost(array $options): int $result = $uploader->upload(); if ($result != SftpUploader::UPLOAD_SUCCESS) { - Utils::log()->error("Failed on " . json_encode($uploader->current(), JSON_UNESCAPED_SLASHES)); + LeUtils::log_error("SFTP failed on " . json_encode($uploader->current(), JSON_UNESCAPED_SLASHES)); switch ($result) { case SftpUploader::UPLOAD_ERROR_NO_PERMISSION: @@ -336,7 +337,7 @@ function connectWithServer(array $options, &$error): ?SftpClient if ($err = $sftp->cd($remote_path)->lastError()) { $error = $err; $error["change_home_dir_failed"] = true; - Utils::log()->error("Failed cd into '{$remote_path}'", $err); + LeUtils::log_error("SFTP failed cd into '{$remote_path}'", $err); return null; } } @@ -352,7 +353,7 @@ function help() function getOptionsById($automation_id, $silent = false) { if (!$silent) { - Utils::log()->info("Reading options from automation: $automation_id"); + LeUtils::log_debug("Reading options from automation: $automation_id"); } if (is_object($action = Utils::getAutomationActionById($automation_id))) { @@ -367,6 +368,7 @@ function getOptionsById($automation_id, $silent = false) "chgrp" => trim((string)$action->sftp_chgrp), "chmod" => trim((string)$action->sftp_chmod), "chmod-key" => trim((string)$action->sftp_chmod_key), + "modtime" => trim((string)$action->sftp_modtime), "cert-name" => trim((string)$action->sftp_filename_cert), "key-name" => trim((string)$action->sftp_filename_key), "ca-name" => trim((string)$action->sftp_filename_ca), @@ -374,10 +376,10 @@ function getOptionsById($automation_id, $silent = false) "certificates" => "", // defaults to all (= empty), may be overridden via CLI ]; } elseif (!$silent) { - Utils::log()->error("Ignoring disabled or invalid automation '$automation_id'"); + LeUtils::log_error("SFTP ignoring disabled or invalid automation '$automation_id'"); } } else { - Utils::log()->error("No upload automation found with uuid = '$automation_id'"); + LeUtils::log_error("No SFTP upload automation found with uuid = '$automation_id'"); } return false; @@ -388,19 +390,20 @@ function addFilesToUpload(array $options, SftpUploader &$uploader) $chmod = isset($options["chmod"]) ? ($options["chmod"] ?: DEFAULT_CERT_MODE) : false; $chmod_key = isset($options["chmod-key"]) ? ($options["chmod-key"] ?: DEFAULT_KEY_MODE) : false; $chgrp = ($options["chgrp"] ?? "") ?: false; + $modtime = ($options["modtime"] ?? "") ?: false; if (isset($options["certificates"])) { $cert_ids = preg_split('/[,;\s]+/', $options["certificates"] ?: "", 0, PREG_SPLIT_NO_EMPTY); foreach (findCertificates($cert_ids) as $cert) { if (!isset($cert["content"])) { - Utils::log()->error("Ignoring upload for cert '{$cert["name"]}', since it is not available in trust storage."); + LeUtils::log_error("Ignoring SFTP upload for cert '{$cert["name"]}', since it is not available in trust storage."); continue; } foreach ($cert["content"] as $name => $content) { if (empty($content)) { - Utils::log()->error("Content for '{$name}.pem' in cert '{$cert["name"]}' is empty, skipping it."); + LeUtils::log_error("Content for '{$name}.pem' in cert '{$cert["name"]}' is empty, skipping SFTP upload."); continue; } @@ -441,27 +444,27 @@ function addFilesToUpload(array $options, SftpUploader &$uploader) ? $chmod_key : $chmod; - $uploader->addContent($content, $target_path, $cert["updated"], $mod, $chgrp); + $uploader->addContent($content, $target_path, $cert["updated"], $mod, $chgrp, $modtime); } else { - Utils::log()->error("Cannot add '{$name}.pem' since the upload path '$target_path' is invalid."); + LeUtils::log_error("Cannot add '{$name}.pem' to SFTP upload since the upload path '$target_path' is invalid."); } } } if (empty($uploader->pending())) { - Utils::log()->error("Didn't find any certificates to upload (cert-ids: " . (empty($cert_ids) ? "*all*" : join(", ", $cert_ids)) . ")."); + LeUtils::log_error("Could not find any certificates for SFTP upload (cert-ids: " . (empty($cert_ids) ? "*all*" : join(", ", $cert_ids)) . ")."); } } elseif (isset($options["files"])) { $files = preg_split('/[,;\s]+/', $options["files"] ?: "", 0, PREG_SPLIT_NO_EMPTY); foreach ($files as $file) { - $uploader->addFile($file, "", $chmod, $chgrp); + $uploader->addFile($file, "", $chmod, $chgrp, $modtime); } if (empty($uploader->pending())) { - Utils::log()->error("Didn't files to upload (files: " . join(", ", $files) . ")."); + LeUtils::log_error("Could not find files for SFTP upload (files: " . join(", ", $files) . ")."); } } else { - Utils::log()->error("Neither '--certificates' nor '--files' was specified. Have nothing to upload."); + LeUtils::log_error("Neither '--certificates' nor '--files' was specified. Have nothing to upload."); } } @@ -489,7 +492,7 @@ function findCertificates(array $certificate_ids_or_names, $load_content = true) ) { if ($cert->enabled == 0) { if (!empty($certificate_ids_or_names)) { - Utils::log()->error("Certificate '{$name}' (id: $id) is disabled, skipping it."); + LeUtils::log_error("Certificate '{$name}' (id: $id) is disabled, skipping SFTP upload."); } continue;