From c3b9653d8e896502b5b15084cdfc9b23605e9388 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Thu, 11 Jul 2019 21:43:36 +0200 Subject: [PATCH 1/4] security/acme-client: remove acme.sh cert config, refs #1380 --- .../AcmeClient/Api/CertificatesController.php | 10 ++++ .../OPNsense/AcmeClient/certhelper.php | 49 +++++++++++++++++++ .../conf/actions.d/actions_acmeclient.conf | 6 +++ 3 files changed, 65 insertions(+) diff --git a/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/Api/CertificatesController.php b/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/Api/CertificatesController.php index 88df62cdc..1e0a76664 100644 --- a/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/Api/CertificatesController.php +++ b/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/Api/CertificatesController.php @@ -62,6 +62,16 @@ class CertificatesController extends ApiMutableModelControllerBase public function delAction($uuid) { + # Remove the cert from list of certs known to acme.sh. + $mdlAcme = new AcmeClient(); + if ($uuid != null) { + $node = $mdlAcme->getNodeByReference('certificates.certificate.' . $uuid); + if ($node != null) { + $cert_id = $node->id; + $backend = new Backend(); + $response = $backend->configdRun("acmeclient remove-cert {$cert_id}"); + } + } return $this->delBase('certificates.certificate', $uuid); } diff --git a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php index 78f63dbad..b0f9d8345 100755 --- a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php +++ b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php @@ -86,6 +86,11 @@ switch ($options["a"]) { $result = cert_action_validator($options["c"]); echo json_encode(array('status'=>$result)); break; + case 'remove': + //$result = revoke_cert($options["c"]); + $result = cert_action_validator($options["c"]); + echo json_encode(array('status'=>$result)); + exit(1); case 'revoke': //$result = revoke_cert($options["c"]); $result = cert_action_validator($options["c"]); @@ -179,6 +184,19 @@ function cert_action_validator($opt_cert_id) } } + // Cert is being removed from the GUI, delete all traces. + if ($options["a"] == "remove") { + // Start acme client to remove the certificate + $rev_result = remove_cert($certObj); + if (!$rev_result) { + log_error("AcmeClient: successfully removed acme.sh certificate configuration for " . (string)$certObj->name); + return(0); // Success! + } else { + log_error("AcmeClient: failed to remove acme.sh certificate configuration for " . (string)$certObj->name); + return(1); + } + } + // Make sure we found the configured validation method if ($ref_found == true) { // Was a revocation requested? @@ -951,6 +969,37 @@ function revoke_cert($certObj, $valObj, $acctObj) return($result); } +// Remove a cert from list of certs known to acme.sh. +function remove_cert($certObj) +{ + // Prepare optional parameters for acme-client + $acme_args = eval_optional_acme_args(); + + // Generate certificate filenames + $cert_id = (string)$certObj->id; + + // Check if EC certificate is used, if yes add the --ecc parameter to acme client + $key_length = (string) $certObj->keyLength; + $ecc_param = " "; + if ($key_length == 'key_ec256' || $key_length == 'key_ec384') { + $ecc_param = "--ecc"; + } + + // Run acme client + // NOTE: We "export" certificates to our own directory, so we don't have to deal + // with domain names in filesystem, but instead can use the ID of our certObj. + $acmecmd = "/usr/local/sbin/acme.sh " + . implode(" ", $acme_args) . " " + . "--remove " + . "--domain " . (string)$certObj->name . " " + . "--home /var/etc/acme-client/home " + . $ecc_param; + $result = mwexec($acmecmd); + + // Simply return acme clients exit code + return($result); +} + function import_certificate($certObj, $modelObj) { global $config; diff --git a/security/acme-client/src/opnsense/service/conf/actions.d/actions_acmeclient.conf b/security/acme-client/src/opnsense/service/conf/actions.d/actions_acmeclient.conf index eddddd48a..c6c1fafba 100644 --- a/security/acme-client/src/opnsense/service/conf/actions.d/actions_acmeclient.conf +++ b/security/acme-client/src/opnsense/service/conf/actions.d/actions_acmeclient.conf @@ -53,6 +53,12 @@ parameters:%s type:script message:revoking a certificate +[remove-cert] +command:/usr/local/opnsense/scripts/OPNsense/AcmeClient/certhelper.php -a remove -c +parameters:%s +type:script +message:removing a certificate + [sign-all-certs] command:/usr/sbin/daemon -f /usr/local/opnsense/scripts/OPNsense/AcmeClient/certhelper.php -a sign -A parameters: From 742b0fecc65bdaa137c0791a636507f4735f5bca Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Thu, 11 Jul 2019 22:05:28 +0200 Subject: [PATCH 2/4] security/acme-client: small cleanup --- .../src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php index b0f9d8345..d572c0c1c 100755 --- a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php +++ b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php @@ -77,22 +77,18 @@ if (isset($options["C"])) { // Run the specified action switch ($options["a"]) { case 'sign': - //$result = sign_or_renew_cert($options["c"]); $result = cert_action_validator($options["c"]); echo json_encode(array('status'=>$result)); break; case 'renew': - //$result = sign_or_renew_cert($options["c"]); $result = cert_action_validator($options["c"]); echo json_encode(array('status'=>$result)); break; case 'remove': - //$result = revoke_cert($options["c"]); $result = cert_action_validator($options["c"]); echo json_encode(array('status'=>$result)); exit(1); case 'revoke': - //$result = revoke_cert($options["c"]); $result = cert_action_validator($options["c"]); echo json_encode(array('status'=>$result)); exit(1); @@ -963,8 +959,6 @@ function revoke_cert($certObj, $valObj, $acctObj) . $ecc_param; $result = mwexec($acmecmd); - // TODO: maybe clear lastUpdate value? - // Simply return acme clients exit code return($result); } From 74c158a71ce1882af97e1bc0383e5f0435fb77c6 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Thu, 11 Jul 2019 22:08:48 +0200 Subject: [PATCH 3/4] security/acme-client: bump version --- security/acme-client/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/acme-client/Makefile b/security/acme-client/Makefile index 0a6e2f62f..9f906ec38 100644 --- a/security/acme-client/Makefile +++ b/security/acme-client/Makefile @@ -1,5 +1,5 @@ PLUGIN_NAME= acme-client -PLUGIN_VERSION= 1.23 +PLUGIN_VERSION= 1.24 PLUGIN_COMMENT= Let's Encrypt client PLUGIN_MAINTAINER= opnsense@moov.de PLUGIN_DEPENDS= acme.sh From 5a1aefe495a570e8f7c5f66b6c5e27ad7d4eda37 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Thu, 11 Jul 2019 23:33:49 +0200 Subject: [PATCH 4/4] security/acme-client: add private key reset, closes #990 --- .../AcmeClient/Api/CertificatesController.php | 20 ++++++++ .../OPNsense/AcmeClient/certificates.volt | 30 ++++++++++-- .../OPNsense/AcmeClient/certhelper.php | 47 +++++++++++++++++-- .../conf/actions.d/actions_acmeclient.conf | 6 +++ 4 files changed, 94 insertions(+), 9 deletions(-) diff --git a/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/Api/CertificatesController.php b/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/Api/CertificatesController.php index 1e0a76664..99cad52cb 100644 --- a/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/Api/CertificatesController.php +++ b/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/Api/CertificatesController.php @@ -109,6 +109,26 @@ class CertificatesController extends ApiMutableModelControllerBase return $result; } + /** + * remove private key from certificate by uuid + * @param $uuid item unique id + * @return array status + */ + public function removekeyAction($uuid) + { + $result = array("result"=>"failed"); + $mdlAcme = new AcmeClient(); + if ($uuid != null) { + $node = $mdlAcme->getNodeByReference('certificates.certificate.' . $uuid); + if ($node != null) { + $cert_id = $node->id; + $backend = new Backend(); + $response = $backend->configdRun("acmeclient remove-key {$cert_id}"); + } + } + return $result; + } + /** * revoke certificate by uuid * @param $uuid item unique id diff --git a/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/certificates.volt b/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/certificates.volt index 35d2bae83..fc47d510f 100644 --- a/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/certificates.volt +++ b/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/certificates.volt @@ -47,6 +47,7 @@ POSSIBILITY OF SUCH DAMAGE. toggle:'/api/acmeclient/certificates/toggle/', sign:'/api/acmeclient/certificates/sign/', revoke:'/api/acmeclient/certificates/revoke/', + removekey:'/api/acmeclient/certificates/removekey/', }; var gridopt = { @@ -61,7 +62,8 @@ POSSIBILITY OF SUCH DAMAGE. "" + "" + "" + - ""; + "" + + ""; }, "rowtoggle": function (column, row) { if (parseInt(row[column.id], 2) == 1) { @@ -314,7 +316,7 @@ POSSIBILITY OF SUCH DAMAGE. }); // sign cert - // TODO: this should block other sign/revoke actions + // TODO: this should block other acme.sh actions grid_certificates.find(".command-sign").on("click", function(e) { if (gridParams['sign'] != undefined) { @@ -336,7 +338,7 @@ POSSIBILITY OF SUCH DAMAGE. }); // revoke cert - // TODO: this should block other sign/revoke actions + // TODO: this should block other acme.sh actions grid_certificates.find(".command-revoke").on("click", function(e) { if (gridParams['revoke'] != undefined) { @@ -355,6 +357,26 @@ POSSIBILITY OF SUCH DAMAGE. } }); + // remove private key + // TODO: this should block other acme.sh actions + grid_certificates.find(".command-removekey").on("click", function(e) + { + if (gridParams['removekey'] != undefined) { + var uuid=$(this).data("row-id"); + stdDialogConfirm('{{ lang._('Confirmation Required') }}', + '{{ lang._('Really remove the private key?%s%sThe certificate will be completely reset. This is useful when the private key has been compromised or when you have changed the key options and want to regenerate the private key.%sNote that you have to revalidate the certificate afterwards in order to create a new private key and a matching certificate.') | format('
', '
', '
') }}', + '{{ lang._('Yes') }}', '{{ lang._('Cancel') }}', function() { + ajaxCall(url=gridParams['removekey'] + uuid, + sendData={},callback=function(data,status){ + // reload grid after sign + $("#"+gridId).bootgrid("reload"); + }); + }, 'danger'); + } else { + console.log("[grid] action removekey missing") + } + }); + }); /*********************************************************************** @@ -363,7 +385,7 @@ POSSIBILITY OF SUCH DAMAGE. /** * Sign or renew ALL certificates - * TODO: this should block other sign/revoke actions + * TODO: this should block other acme.sh actions */ $("#signallcertsAct").click(function(){ //$("#signallcertsAct_progress").addClass("fa fa-spinner fa-pulse"); diff --git a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php index d572c0c1c..ae5ac9d26 100755 --- a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php +++ b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php @@ -87,11 +87,15 @@ switch ($options["a"]) { case 'remove': $result = cert_action_validator($options["c"]); echo json_encode(array('status'=>$result)); - exit(1); + break; + case 'removekey': + $result = cert_action_validator($options["c"]); + echo json_encode(array('status'=>$result)); + break; case 'revoke': $result = cert_action_validator($options["c"]); echo json_encode(array('status'=>$result)); - exit(1); + break; default: echo "ERROR: invalid argument specified\n"; log_error("invalid argument specified"); @@ -127,10 +131,14 @@ function cert_action_validator($opt_cert_id) if (isset($options["A"]) or ((string)$cert_id == (string)$opt_cert_id)) { // Ignore disabled certificates if ($certObj->enabled == 0) { + // Always ignore disabled certs when working on ALL certs. if (isset($options["A"])) { continue; // skip to next item } - return(1); // Cert is disabled, skip it. + // Allow only "revoke", "remove" and "removekey" for disabled certs. + if (!in_array($options["a"], ['remove','removekey','revoke'])) { + return(1); // Cert is disabled, skip it. + } } // Extract Account from referenced obj @@ -193,6 +201,24 @@ function cert_action_validator($opt_cert_id) } } + // Remove private key + // NOTE: Although the user requested to remove the private key, + // we simply perform a full cert removal because without the + // matching private key the cert is useless. + if ($options["a"] == "removekey") { + // Start acme client to remove the certificate + $rev_result = remove_cert($certObj); + if (!$rev_result) { + log_error("AcmeClient: successfully removed the private key and reset certificate " . (string)$certObj->name); + // Reset certificate state, treat it like a new certificate. + log_cert_acme_status($certObj, $modelObj, '100'); + return(0); // Success! + } else { + log_error("AcmeClient: failed to remove the private key and reset certificate " . (string)$certObj->name); + return(1); + } + } + // Make sure we found the configured validation method if ($ref_found == true) { // Was a revocation requested? @@ -980,8 +1006,6 @@ function remove_cert($certObj) } // Run acme client - // NOTE: We "export" certificates to our own directory, so we don't have to deal - // with domain names in filesystem, but instead can use the ID of our certObj. $acmecmd = "/usr/local/sbin/acme.sh " . implode(" ", $acme_args) . " " . "--remove " @@ -990,6 +1014,19 @@ function remove_cert($certObj) . $ecc_param; $result = mwexec($acmecmd); + $cert_files = [ + "/var/etc/acme-client/keys/${cert_id}/private.key", + "/var/etc/acme-client/certs/${cert_id}/cert.pem", + "/var/etc/acme-client/certs/${cert_id}/chain.pem", + "/var/etc/acme-client/certs/${cert_id}/fullchain.pem", + ]; + + foreach ($cert_files as $_file) { + if (file_exists($_file)) { + unlink($_file); + } + } + // Simply return acme clients exit code return($result); } diff --git a/security/acme-client/src/opnsense/service/conf/actions.d/actions_acmeclient.conf b/security/acme-client/src/opnsense/service/conf/actions.d/actions_acmeclient.conf index c6c1fafba..99858b1aa 100644 --- a/security/acme-client/src/opnsense/service/conf/actions.d/actions_acmeclient.conf +++ b/security/acme-client/src/opnsense/service/conf/actions.d/actions_acmeclient.conf @@ -59,6 +59,12 @@ parameters:%s type:script message:removing a certificate +[remove-key] +command:/usr/local/opnsense/scripts/OPNsense/AcmeClient/certhelper.php -a removekey -c +parameters:%s +type:script +message:removing a certificate private key + [sign-all-certs] command:/usr/sbin/daemon -f /usr/local/opnsense/scripts/OPNsense/AcmeClient/certhelper.php -a sign -A parameters: