From bdf9379b22405946b281280796b39e75df6bc594 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Sun, 5 Jan 2025 23:23:24 +0100 Subject: [PATCH] security/acme-client: migrate cert+CA import/update to Trust MVC --- security/acme-client/pkg-descr | 2 + .../OPNsense/AcmeClient/LeCertificate.php | 171 ++++++++---------- .../library/OPNsense/AcmeClient/LeUtils.php | 82 +-------- .../scripts/OPNsense/AcmeClient/lecert.php | 3 +- .../OPNsense/AcmeClient/run_remote_ssh.php | 1 - .../OPNsense/AcmeClient/upload_sftp.php | 17 +- 6 files changed, 91 insertions(+), 185 deletions(-) diff --git a/security/acme-client/pkg-descr b/security/acme-client/pkg-descr index ff97285f0..1536c9c9c 100644 --- a/security/acme-client/pkg-descr +++ b/security/acme-client/pkg-descr @@ -18,10 +18,12 @@ Added: Changed: * Convert Synology deploy hook variables to uppercase (#4286) +* Migrate to MVC Trust storage Fixed: * SFTP/SSH automation results in fatal PHP error (#4363) * Typo in INWX password field name +* Certs not fully functional after import into Trust storage (#4401) 4.6 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 9df9e8808..9a5056fbe 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 @@ -1,7 +1,7 @@ cert_chain_file); if ($ca_content != false) { - $ca_subject = cert_get_subject($ca_content, false); - $ca_serial = cert_get_serial($ca_content, false); - $ca_cn = LeUtils::local_cert_get_cn($ca_content, false); - $ca_issuer = cert_get_issuer($ca_content, false); - $ca_purpose = cert_get_purpose($ca_content, false); + $ca_details = CertStore::parseX509($ca_content); + $ca_subject = $ca_details['name']; + $ca_serial = $ca_details['serialNumber']; + $ca_cn = $ca_details['commonname']; + $ca_issuer = implode(",", $ca_details['issuer']); } else { LeUtils::log_error('unable to read CA certificate content from file'); Config::getInstance()->unlock(); return false; } - // Prepare CA for import in Cert Manager + // Prepare CA + $caModel = new Ca(); $ca = array(); - $ca['crt'] = base64_encode($ca_content); $ca['refid'] = uniqid(); + $ca['descr'] = (string)$ca_cn . ' (ACME Client)'; $ca_found = false; // Check if CA was previously imported - foreach (Config::getInstance()->object()->ca as $cacrt) { - $cacrt_subject = cert_get_subject($cacrt->crt, true); - $cacrt_issuer = cert_get_issuer($cacrt->crt, true); + foreach ($caModel->ca->iterateItems() as $cacrt) { + $cacrt_content = base64_decode((string)$cacrt->crt); + $cacrt_details = CertStore::parseX509($cacrt_content); + $cacrt_subject = $cacrt_details['name']; + $cacrt_issuer = implode(",", $cacrt_details['issuer']); if (($ca_subject === $cacrt_subject) and ($ca_issuer === $cacrt_issuer)) { // Use old refid instead of generating a new one $ca['refid'] = (string)$cacrt->refid; + // Update existing CA + $cacrt->descr = $ca['descr']; $ca_found = true; break; } } - // Collect required CA information - $ca_cn = LeUtils::local_cert_get_cn($ca_content, false); - $ca['descr'] = (string)$ca_cn . ' (ACME Client)'; - - // Prepare CA for import - LeUtils::local_ca_import($ca, $ca_content); - - // Check if CA was found in config - if ($ca_found == true) { - // Update existing CA - foreach (Config::getInstance()->object()->ca as $cacrt) { - if ((string)$cacrt->refid == $ca['refid']) { - $cacrt->crt = $ca['crt']; - $cacrt->descr = $ca['descr']; - break; - } - } - } else { - // Create new CA - LeUtils::log("importing ACME CA: {$ca_cn}"); - $newca = Config::getInstance()->object()->addChild('ca'); + // Create new CA + if ($ca_found == false) { + LeUtils::log("imported ACME CA: {$ca_cn} ({$ca['refid']})"); + $newca = $caModel->ca->Add(); foreach (array_keys($ca) as $cacfg) { - $newca->addChild($cacfg, (string)$ca[$cacfg]); + $newca->$cacfg = (string)$ca[$cacfg]; } + $newca->crt = base64_encode($ca_content); } + // Serialize to config and save + $caModel->serializeToConfig(); + Config::getInstance()->save(); + /** * Step 2: import certificate */ @@ -205,11 +200,11 @@ class LeCertificate extends LeCommon // Read contents from certificate file $cert_content = @file_get_contents($this->cert_file); if ($cert_content != false) { - $cert_subject = cert_get_subject($cert_content, false); - $cert_serial = cert_get_serial($cert_content, false); - $cert_cn = LeUtils::local_cert_get_cn($cert_content, false); - $cert_issuer = cert_get_issuer($cert_content, false); - $cert_purpose = cert_get_purpose($cert_content, false); + $cert_details = CertStore::parseX509($cert_content); + $cert_subject = $cert_details['name']; + $cert_serial = $cert_details['serialNumber']; + $cert_cn = $cert_details['commonname']; + $cert_issuer = implode(",", $cert_details['issuer']); } else { LeUtils::log_error('unable to read certificate content from file'); Config::getInstance()->unlock(); @@ -217,34 +212,6 @@ class LeCertificate extends LeCommon return false; } - // Prepare certificate for import in Cert Manager - $cert = array(); - $cert_refid = uniqid(); - $cert['refid'] = $cert_refid; - $cert['caref'] = (string)$ca['refid']; - $import_log_message = 'imported'; - $cert_found = false; - - // Check if cert was previously imported - if (!empty((string)$this->config->certRefId)) { - // Check if the previously imported certificate can still be found - foreach (Config::getInstance()->object()->cert as $cfgCert) { - // Check if IDs match - if ((string)$this->config->certRefId == (string)$cfgCert->refid) { - $cert_found = true; - break; - } - } - // Existing cert? - if ($cert_found) { - // Use old refid instead of generating a new one - $cert_refid = (string)$this->config->certRefId; - $import_log_message = 'updated'; - } - } else { - // Not found. Just import as new cert. - } - // Read private key $key_content = @file_get_contents($this->cert_key_file); if ($key_content == false) { @@ -254,28 +221,39 @@ class LeCertificate extends LeCommon return false; } - // Collect required cert information - $cert_cn = LeUtils::local_cert_get_cn($cert_content, false); - $cert['descr'] = (string)$cert_cn . ' (ACME Client)'; - $cert['refid'] = $cert_refid; - - // Prepare certificate for import - cert_import($cert, $cert_content, $key_content); - - // Overwrite caref in order to use the correct CA (GH #2550). - // This is required because cert_import() uses lookup_ca_by_subject() - // to find a matching CA. If multiple CAs are using the same name, the - // first CA wins, but it may still be the wrong CA. + // Prepare certificate + $certModel = new Cert(); + $cert = array(); + $cert['refid'] = uniqid(); $cert['caref'] = (string)$ca['refid']; + $cert['descr'] = (string)$cert_cn . ' (ACME Client)'; + $import_log_message = 'imported'; + $cert_found = false; + + // Check if cert was previously imported. + // Otherwise just import as new cert. + if (!empty((string)$this->config->certRefId)) { + // Check if the previously imported certificate can still be found + foreach ($certModel->cert->iterateItems() as $cfgCert) { + // Check if IDs match + if ((string)$this->config->certRefId == (string)$cfgCert->refid) { + // Use old refid instead of generating a new one + $cert['refid'] = (string)$cfgCert->refid; + $import_log_message = 'updated'; + $cert_found = true; + break; + } + } + } // Check if cert was found in config if ($cert_found == true) { // Update existing cert - foreach (Config::getInstance()->object()->cert as $cfgCert) { + foreach ($certModel->cert->iterateItems() as $cfgCert) { if ((string)$cfgCert->refid == $cert['refid']) { - $cfgCert->crt = $cert['crt']; - $cfgCert->prv = $cert['prv']; $cfgCert->descr = $cert['descr']; + $cfgCert->crt = base64_encode($cert_content); + $cfgCert->prv = base64_encode($key_content); // Update CA ref, because it may be signed by a different CA. $cfgCert->caref = $cert['caref']; break; @@ -283,27 +261,32 @@ class LeCertificate extends LeCommon } } else { // Create new cert - $newcert = Config::getInstance()->object()->addChild('cert'); + $newcert = $certModel->cert->Add(); foreach (array_keys($cert) as $certcfg) { - $newcert->addChild($certcfg, (string)$cert[$certcfg]); + $newcert->$certcfg = (string)$cert[$certcfg]; } + $newcert->crt = base64_encode($cert_content); + $newcert->prv = base64_encode($key_content); } - LeUtils::log("{$import_log_message} ACME X.509 certificate: {$cert_cn}"); + LeUtils::log("{$import_log_message} ACME X.509 certificate: {$cert_cn} ({$cert['refid']})"); + + // Serialize to config and save + // Skip validation because the current in-memory model may not + // know about the CA item that was just created. + $certModel->serializeToConfig(false,true); + Config::getInstance()->save(); /** * Step 3: update configuration */ - // Add refid to certObj - $this->config->certRefId = $cert_refid; - // Set update/create time + // Update Acme cert config + $this->config->certRefId = $cert['refid']; $this->config->lastUpdate = time(); // Serialize to config and save $this->model->serializeToConfig(); Config::getInstance()->save(); - - // Reload to get most recent config Config::getInstance()->forceReload(); $this->loadConfig(self::CONFIG_PATH, $this->uuid); @@ -402,12 +385,12 @@ class LeCertificate extends LeCommon return false; } - // Run referenced automations. - $this->runAutomations(); - // Update cert status. $this->setStatus(200); + // Run referenced automations. + $this->runAutomations(); + return true; } 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 7514ed656..6dfd33eff 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 @@ -1,7 +1,7 @@ * Copyright (C) 2008 Shrew Soft Inc. @@ -70,86 +70,6 @@ class LeUtils return vsprintf($format, $args); } - // Copied from system_camanager.php. - public static function local_ca_import(&$ca, $str, $key = "", $serial = 0) - { - // Get config object. - $config = Config::getInstance()->object(); - - $ca['crt'] = base64_encode($str); - if (!empty($key)) { - $ca['prv'] = base64_encode($key); - } - if (!empty($serial)) { - $ca['serial'] = $serial; - } - $subject = cert_get_subject($str, false); - $issuer = cert_get_issuer($str, false); - - // Find my issuer unless self-signed - if ($issuer != $subject) { - $issuer_crt =& lookup_ca_by_subject($issuer); - if ($issuer_crt) { - $ca['caref'] = $issuer_crt['refid']; - } - } - - /* Correct if child certificate was loaded first */ - if (is_array($config['ca'])) { - foreach ($config['ca'] as & $oca) { - $issuer = cert_get_issuer($oca['crt']); - if ($ca['refid'] != $oca['refid'] && $issuer == $subject) { - $oca['caref'] = $ca['refid']; - } - } - } - if (is_array($config['cert'])) { - foreach ($config['cert'] as & $cert) { - $issuer = cert_get_issuer($cert['crt']); - if ($issuer == $subject) { - $cert['caref'] = $ca['refid']; - } - } - } - return true; - } - - // copied from certs.inc - public static function local_cert_get_cn($crt, $decode = true) - { - $sub = self::local_cert_get_subject_array($crt, $decode); - if (is_array($sub)) { - foreach ($sub as $s) { - if (strtoupper($s['a']) == "CN") { - return $s['v']; - } - } - } - return ""; - } - - // copied from certs.inc - public static function local_cert_get_subject_array($str_crt, $decode = true) - { - if ($decode) { - $str_crt = base64_decode($str_crt); - } - $inf_crt = openssl_x509_parse($str_crt); - $components = $inf_crt['subject']; - - if (!is_array($components)) { - return; - } - - $subject_array = array(); - - foreach ($components as $a => $v) { - $subject_array[] = array('a' => $a, 'v' => $v); - } - - return $subject_array; - } - /** * log runtime information */ diff --git a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/lecert.php b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/lecert.php index 73bb87ee2..0079a3a8f 100755 --- a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/lecert.php +++ b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/lecert.php @@ -2,7 +2,7 @@ object(); - foreach ($config->cert as $cert) { + $certModel = new Cert(); + foreach ($certModel->cert->iterateItems() as $cert) { $refid = (string)$cert->refid; $item = []; if (in_array($refid, $cert_refids)) { - $item["cert"] = str_replace(["\n\n", "\r"], ["\n", ""], base64_decode($cert->crt)); - $item["key"] = str_replace(["\n\n", "\r"], ["\n", ""], base64_decode($cert->prv)); + $_tmp = CertStore::getCertificate($refid); + $item["cert"] = $_tmp["crt"]; + $item["key"] = $_tmp["prv"]; // check if a CA is linked if (!empty((string)$cert->caref)) { - $cert = (array)$cert; - $item["ca"] = ca_chain($cert); + $item["ca"] = $_tmp["ca"];; // combine files to export a fullchain.pem $item["fullchain"] = $item["cert"] . $item["ca"]; }