From 6353109a95357456610bcc77ca7ea7ce6e0a0323 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Mon, 20 Mar 2017 11:59:08 +0100 Subject: [PATCH 01/10] security/acme-client: don't make sensitive data world-readable --- .../src/opnsense/scripts/OPNsense/AcmeClient/setup.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 8215a8262..583e3f9d6 100755 --- a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/setup.sh +++ b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/setup.sh @@ -5,7 +5,7 @@ ACME_DIRS="/var/etc/acme-client /var/etc/acme-client/certs /var/etc/acme-client/ for directory in ${ACME_DIRS}; do mkdir -p ${directory} chown -R root:wheel ${directory} - chmod -R 755 ${directory} + chmod -R 750 ${directory} done exit 0 From 80243f294f8be7784644e82558def7351fff54fa Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Mon, 20 Mar 2017 17:02:56 +0100 Subject: [PATCH 02/10] security/acme-client: properly import CA certificates, closes #84 --- .../OPNsense/AcmeClient/certhelper.php | 118 ++++++++++++++++-- 1 file changed, 108 insertions(+), 10 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 3acb96297..d148eaed6 100755 --- a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php +++ b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php @@ -2,7 +2,7 @@ id; $cert_filename = "/var/etc/acme-client/certs/${cert_id}/cert.pem"; + $cert_chain_filename = "/var/etc/acme-client/certs/${cert_id}/chain.pem"; $cert_fullchain_filename = "/var/etc/acme-client/certs/${cert_id}/fullchain.pem"; $key_filename = "/var/etc/acme-client/keys/${cert_id}/private.key"; // Check if certificate files can be found clearstatcache(); // don't let the cache fool us - foreach (array($cert_filename, $key_filename, $cert_fullchain_filename) as $file) { + foreach (array($cert_filename, $key_filename, $cert_chain_filename, $cert_fullchain_filename) as $file) { if (is_file($file)) { // certificate file found } else { @@ -791,6 +792,63 @@ function import_certificate($certObj, $modelObj) } } + /* + * Step 1: import CA + */ + + // Read contents from CA file + $ca_content = @file_get_contents($cert_chain_filename); + if ($ca_content != false) { + $ca_subject = cert_get_subject($ca_content, false); + $ca_serial = cert_get_serial($ca_content, false); + $ca_cn = local_cert_get_cn($ca_content, false); + $ca_issuer = cert_get_issuer($ca_content, false); + $ca_purpose = cert_get_purpose($ca_content, false); + } else { + log_error("AcmeClient: unable to read CA certificate content from file"); + return(1); + } + + // Prepare CA for import in Cert Manager + $ca = array(); + $ca['crt'] = base64_encode($ca_content); + $ca['refid'] = uniqid(); + $ca_found = false; + + // Check if CA was previously imported + $cacnt = 0; + foreach ($config['ca'] as $cacrt) { + $cacrt_subject = cert_get_subject($cacrt['crt'], true); + $cacrt_issuer = cert_get_issuer($cacrt['crt'], true); + if (($ca_subject == $cacrt_subject) and ($ca_issuer == $cacrt_issuer)) { + // Use old refid instead of generating a new one + $ca['refid'] = (string)$cacrt['refid']; + $ca_found = true; + break; + } + $cacnt++; + } + + // Collect required CA information + $ca_cn = local_cert_get_cn($ca_content, false); + $ca['descr'] = (string)$ca_cn . ' (Let\'s Encrypt)'; + + // Prepare CA for import + local_ca_import($ca, $ca_content); + + // Update existing CA? + if ($ca_found == true) { + $config['ca'][$cacnt] = $ca; + } else { + // Create new CA item + $config['ca'][] = $ca; + log_error("AcmeClient: importing Let's Encrypt CA: ${ca_cn}"); + } + + /* + * Step 2: import certificate + */ + // Read contents from certificate file $cert_content = @file_get_contents($cert_filename); if ($cert_content != false) { @@ -809,6 +867,7 @@ function import_certificate($certObj, $modelObj) $cert = array(); $cert_refid = uniqid(); $cert['refid'] = $cert_refid; + $cert['caref'] = (string)$ca['refid']; $import_log_message = 'Imported'; $cert_found = false; @@ -842,20 +901,13 @@ function import_certificate($certObj, $modelObj) return(1); } - // Read cert fullchain - $cert_fullchain_content = @file_get_contents($cert_fullchain_filename); - if ($cert_fullchain_content == false) { - log_error("AcmeClient: unable to read full certificate chain from file: ${cert_fullchain_filename}"); - return(1); - } - // Collect required cert information $cert_cn = local_cert_get_cn($cert_content, false); $cert['descr'] = (string)$cert_cn . ' (Let\'s Encrypt)'; $cert['refid'] = $cert_refid; // Prepare certificate for import - cert_import($cert, $cert_fullchain_content, $key_content); + cert_import($cert, $cert_content, $key_content); // Update existing certificate? if ($cert_found == true) { @@ -874,6 +926,10 @@ function import_certificate($certObj, $modelObj) $config['cert'][] = $cert; } + /* + * Step 3: update configuration + */ + // Write changes to config // TODO: Legacy code, should be replaced with code from OPNsense framework write_config("${import_log_message} Let's Encrypt SSL certificate: ${cert_cn}"); @@ -1101,6 +1157,48 @@ function local_cert_get_cn($crt, $decode = true) return ""; } +// taken from system_camanager.php +function local_ca_import(& $ca, $str, $key="", $serial=0) { + global $config; + + $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; +} + function base64url_encode($str) { return rtrim(strtr(base64_encode($str), '+/', '-_'), '='); From f3db0b507d51517588452fb31a02ef0978cf7d6d Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Tue, 21 Mar 2017 14:00:23 +0100 Subject: [PATCH 03/10] security/acme-client: hide http service entries when not selected --- .../AcmeClient/forms/dialogValidation.xml | 18 ++---------------- .../views/OPNsense/AcmeClient/validations.volt | 11 ++++++++++- 2 files changed, 12 insertions(+), 17 deletions(-) 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 f9dd582a2..9b1fa7702 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 @@ -37,7 +37,7 @@ header - + validation.http_opn_autodiscovery @@ -63,7 +63,7 @@ header - + validation.http_haproxyInject @@ -79,20 +79,6 @@ true Choose the local HAProxy frontends. They will automatically be configured to redirect acme challenges to the internal acme client. The HAProxy service will automatically be restarted if a certificate was renewed. - header diff --git a/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/validations.volt b/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/validations.volt index b192c9477..d42a0f1ce 100644 --- a/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/validations.volt +++ b/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/validations.volt @@ -51,16 +51,25 @@ POSSIBILITY OF SUCH DAMAGE. // hook into on-show event for dialog to extend layout. $('#DialogValidation').on('shown.bs.modal', function (e) { $("#validation\\.dns_service").change(function(){ - var service_id = 'table_' + $(this).val() ; + var service_id = 'table_' + $(this).val(); $(".table_dns").hide(); if ($("#validation\\.method").val() == 'dns01') { $("."+service_id).show(); } }); + $("#validation\\.http_service").change(function(){ + var service_id = 'table_http_' + $(this).val(); + $(".table_http").hide(); + if ($("#validation\\.method").val() == 'http01') { + $("."+service_id).show(); + } else { + } + }); $("#validation\\.method").change(function(){ $(".method_table").hide(); $(".method_table_"+$(this).val()).show(); $("#validation\\.dns_service").change(); + $("#validation\\.http_service").change(); }); $("#validation\\.method").change(); }) From 664ec700dc85717d0757357945df65b17f9d9cb3 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Tue, 21 Mar 2017 14:04:56 +0100 Subject: [PATCH 04/10] security/acme-client: remove prefixes from validation name --- .../AcmeClient/forms/dialogValidation.xml | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) 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 9b1fa7702..30bceb613 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 @@ -35,7 +35,7 @@ - + header @@ -61,7 +61,7 @@ Enter IP addresses here. Finish each with TAB. - + header @@ -97,7 +97,7 @@ - + header @@ -108,7 +108,7 @@ - + header @@ -125,7 +125,7 @@ - + header @@ -142,7 +142,7 @@ - + header @@ -159,7 +159,7 @@ - + header @@ -176,7 +176,7 @@ - + header @@ -194,7 +194,7 @@ - + header @@ -212,7 +212,7 @@ - + header @@ -229,7 +229,7 @@ - + header @@ -247,7 +247,7 @@ - + header @@ -259,7 +259,7 @@ - + header @@ -276,7 +276,7 @@ - + header @@ -305,7 +305,7 @@ - + header @@ -328,7 +328,7 @@ - + header @@ -340,7 +340,7 @@ - + header @@ -357,7 +357,7 @@ - + header @@ -374,7 +374,7 @@ - + header @@ -391,7 +391,7 @@ - + header @@ -420,7 +420,7 @@ acme.sh documentation for further information.]]> - + header From f65be494844fd30e76ad76272c050761ab9f3cd1 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Tue, 21 Mar 2017 15:05:24 +0100 Subject: [PATCH 05/10] security/acme-client: hide params for restart actions when not selected --- .../OPNsense/AcmeClient/forms/dialogAction.xml | 10 +++++++++- .../app/views/OPNsense/AcmeClient/actions.volt | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) 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 51254439c..4dcfa42b4 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 @@ -24,19 +24,27 @@ Pre-defined commands for this restart action. - + header + action.configd dropdown Select a pre-defined system command which should be run for this action. + + + + + header + action.custom textbox Specify a custom commands which should be run for this action. + diff --git a/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/actions.volt b/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/actions.volt index fbb8f54c5..034406368 100644 --- a/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/actions.volt +++ b/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/actions.volt @@ -48,6 +48,22 @@ POSSIBILITY OF SUCH DAMAGE. } ); + // hook into on-show event for dialog to extend layout. + $('#DialogAction').on('shown.bs.modal', function (e) { + $("#action\\.type").change(function(){ + var service_id = 'table_optional_' + $(this).val(); + $(".table_optional").hide(); + if (($("#action\\.type").val() == 'configd') || ($("#action\\.type").val() == 'custom')) { + $("."+service_id).show(); + } else { + } + }); + $("#action\\.type").change(function(){ + $(".method_table").hide(); + $(".method_table_"+$(this).val()).show(); + }); + $("#action\\.type").change(); + }) }); From 5b4bdb0fa68d4278f5c5e291457555a885ef2d60 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Tue, 21 Mar 2017 15:07:41 +0100 Subject: [PATCH 06/10] security/acme-client: bump version --- security/acme-client/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/security/acme-client/Makefile b/security/acme-client/Makefile index 8d1ada4d0..972358733 100644 --- a/security/acme-client/Makefile +++ b/security/acme-client/Makefile @@ -1,6 +1,5 @@ PLUGIN_NAME= acme-client -PLUGIN_VERSION= 1.2 -PLUGIN_REVISION= 1 +PLUGIN_VERSION= 1.3 PLUGIN_COMMENT= Let's Encrypt client PLUGIN_MAINTAINER= opnsense@moov.de From 09de341f2c8f4aa6b8665eaa6bf711f62ee459a1 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Tue, 21 Mar 2017 15:10:35 +0100 Subject: [PATCH 07/10] security/acme-client: remove useless else clause --- .../src/opnsense/mvc/app/views/OPNsense/AcmeClient/actions.volt | 1 - 1 file changed, 1 deletion(-) diff --git a/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/actions.volt b/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/actions.volt index 034406368..68def86c4 100644 --- a/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/actions.volt +++ b/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/actions.volt @@ -55,7 +55,6 @@ POSSIBILITY OF SUCH DAMAGE. $(".table_optional").hide(); if (($("#action\\.type").val() == 'configd') || ($("#action\\.type").val() == 'custom')) { $("."+service_id).show(); - } else { } }); $("#action\\.type").change(function(){ From 6fcc7f8a5ae706279553601e606ee6d6da2b6766 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Tue, 21 Mar 2017 15:36:35 +0100 Subject: [PATCH 08/10] security/acme-client: simplify JS code --- .../opnsense/mvc/app/views/OPNsense/AcmeClient/actions.volt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/actions.volt b/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/actions.volt index 68def86c4..2eef0c115 100644 --- a/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/actions.volt +++ b/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/actions.volt @@ -53,9 +53,7 @@ POSSIBILITY OF SUCH DAMAGE. $("#action\\.type").change(function(){ var service_id = 'table_optional_' + $(this).val(); $(".table_optional").hide(); - if (($("#action\\.type").val() == 'configd') || ($("#action\\.type").val() == 'custom')) { - $("."+service_id).show(); - } + $("."+service_id).show(); }); $("#action\\.type").change(function(){ $(".method_table").hide(); From f86e71051bd11246bbb01c15319e557ab4f4ea56 Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Tue, 21 Mar 2017 16:46:29 +0100 Subject: [PATCH 09/10] security/acme-client: avoid log message on missing restart action (fixes df2a727) --- .../src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php | 4 ++-- 1 file changed, 2 insertions(+), 2 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 d148eaed6..324b8f9b4 100755 --- a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php +++ b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php @@ -971,11 +971,11 @@ function run_restart_actions($certlist, $modelObj) continue; } // Extract restart actions - $_actions = explode(',', $certObj->restartActions); - if (empty($_actions)) { + if (empty((string)$certObj->restartActions)) { // No restart actions configured. continue; } + $_actions = explode(',', $certObj->restartActions); // Walk through all linked restart actions. foreach ($_actions as $_action) { // Extract restart action From 9999529561fc1edf078a03a0cf3f41d8e640a02f Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Tue, 21 Mar 2017 18:06:08 +0100 Subject: [PATCH 10/10] security/acme-client: make the timeout for (custom) restart commands configurable --- .../controllers/OPNsense/AcmeClient/forms/settings.xml | 7 +++++++ .../mvc/app/models/OPNsense/AcmeClient/AcmeClient.xml | 6 ++++++ .../opnsense/scripts/OPNsense/AcmeClient/certhelper.php | 9 +++++++-- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/forms/settings.xml b/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/forms/settings.xml index 65afd3a4f..8f5debb9d 100644 --- a/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/forms/settings.xml +++ b/security/acme-client/src/opnsense/mvc/app/controllers/OPNsense/AcmeClient/forms/settings.xml @@ -30,4 +30,11 @@ true + + acmeclient.settings.restartTimeout + + text + + true + 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 5edec0877..65e463196 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 @@ -43,6 +43,12 @@ 65535 Y + + 600 + 10 + 86400 + Y + 0 N 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 324b8f9b4..c1fbd1767 100755 --- a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php +++ b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/certhelper.php @@ -957,6 +957,7 @@ function run_restart_actions($certlist, $modelObj) { global $config; $return = 0; + $configObj = Config::getInstance()->object(); // NOTE: Do NOT run any restart action twice, collect duplicates first. $restart_actions = array(); @@ -1041,8 +1042,12 @@ function run_restart_actions($certlist, $modelObj) $proc_stderr = ''; $result = ''; // exit code (or '99' in case of timeout) - // TODO: Make the timeout configurable. - $timeout = '600'; + // Timeout for custom restart actions. + if (!empty((string)$configObj->OPNsense->AcmeClient->settings->restartTimeout)) { + $timeout = (string)$configObj->OPNsense->AcmeClient->settings->restartTimeout; + } else { + $timeout = '600'; + } $starttime = time(); $proc_cmd = (string)$action->custom;