security/acme-client: prevent shell command injection, as discussed in #2028

This commit is contained in:
Frank Wall
2020-10-04 22:43:01 +02:00
parent 8f84e74c8d
commit 3284a2b895
3 changed files with 20 additions and 20 deletions
@@ -57,7 +57,7 @@ class LeAccount extends LeCommon
$this->setEnvironment();
// Store acme filenames
$this->acme_args[] = '--home ' . self::ACME_HOME_DIR;
$this->acme_args[] = exec_safe('--home %s', self::ACME_HOME_DIR);
}
/**
@@ -118,8 +118,8 @@ class LeAccount extends LeCommon
$acmecmd = '/usr/local/sbin/acme.sh '
. '--createAccountKey '
. implode(' ', $this->acme_args) . ' '
. '--accountkeylength ' . self::ACME_ACCOUNT_KEY_LENGTH . ' '
. "--accountconf ${account_conf_file}";
. exec_safe('--accountkeylength %s', self::ACME_ACCOUNT_KEY_LENGTH) . ' '
. exec_safe('--accountconf %s', $account_conf_file);
LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd, $this->debug);
$proc = proc_open($acmecmd, $proc_desc, $proc_pipes, null, $proc_env);
@@ -226,7 +226,7 @@ class LeAccount extends LeCommon
$acmecmd = '/usr/local/sbin/acme.sh '
. '--registeraccount '
. implode(' ', $this->acme_args) . ' '
. '--accountconf ' . $this->account_conf_file;
. exec_safe('--accountconf %s', $this->account_conf_file);
LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd, $this->debug);
$proc = proc_open($acmecmd, $proc_desc, $proc_pipes, null, $proc_env);
@@ -88,11 +88,11 @@ class LeCertificate extends LeCommon
$this->cert_fullchain_file = (string)sprintf(self::ACME_FULLCHAIN_FILE, $this->config->id);
// Store acme filenames
$this->acme_args[] = '--home ' . self::ACME_HOME_DIR;
$this->acme_args[] = '--certpath ' . $this->cert_file;
$this->acme_args[] = '--keypath ' . $this->cert_key_file;
$this->acme_args[] = '--capath ' . $this->cert_chain_file;
$this->acme_args[] = '--fullchainpath ' . $this->cert_fullchain_file;
$this->acme_args[] = exec_safe('--home %s', self::ACME_HOME_DIR);
$this->acme_args[] = exec_safe('--certpath %s', $this->cert_file);
$this->acme_args[] = exec_safe('--keypath %s', $this->cert_key_file);
$this->acme_args[] = exec_safe('--capath %s', $this->cert_chain_file);
$this->acme_args[] = exec_safe('--fullchainpath %s', $this->cert_fullchain_file);
}
/**
@@ -529,7 +529,7 @@ class LeCertificate extends LeCommon
. '--revoke '
. implode(' ', $this->acme_args) . ' '
. exec_safe('--domain %s', (string)$this->config->name) . ' '
. "--accountconf ${account_conf_file}";
. exec_safe('--accountconf %s', $account_conf_file);
LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd, $this->debug);
$proc = proc_open($acmecmd, $proc_desc, $proc_pipes, null, $proc_env);
@@ -82,8 +82,8 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon
// Store acme hook
switch ((string)$this->config->method) {
case 'dns01':
$this->acme_args[] = '--dns ' . (string)$this->config->dns_service;
$this->acme_args[] = '--dnssleep ' . (string)$this->config->dns_sleep;
$this->acme_args[] = exec_safe('--dns %s', (string)$this->config->dns_service);
$this->acme_args[] = exec_safe('--dnssleep %s', (string)$this->config->dns_sleep);
break;
case 'http01':
$this->acme_args[] = '--webroot /var/etc/acme-client/challenges';
@@ -91,11 +91,11 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon
}
// Store acme filenames
$this->acme_args[] = '--home ' . self::ACME_HOME_DIR;
$this->acme_args[] = '--certpath ' . sprintf(self::ACME_CERT_FILE, $this->cert_id);
$this->acme_args[] = '--keypath ' . sprintf(self::ACME_KEY_FILE, $this->cert_id);
$this->acme_args[] = '--capath ' . sprintf(self::ACME_CHAIN_FILE, $this->cert_id);
$this->acme_args[] = '--fullchainpath ' . sprintf(self::ACME_FULLCHAIN_FILE, $this->cert_id);
$this->acme_args[] = exec_safe('--home %s', self::ACME_HOME_DIR);
$this->acme_args[] = exec_safe('--certpath %s', sprintf(self::ACME_CERT_FILE, $this->cert_id));
$this->acme_args[] = exec_safe('--keypath %s', sprintf(self::ACME_KEY_FILE, $this->cert_id));
$this->acme_args[] = exec_safe('--capath %s', sprintf(self::ACME_CHAIN_FILE, $this->cert_id));
$this->acme_args[] = exec_safe('--fullchainpath %s', sprintf(self::ACME_FULLCHAIN_FILE, $this->cert_id));
return true;
}
@@ -165,7 +165,7 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon
$acmecmd = '/usr/local/sbin/acme.sh '
. "--${acme_action} "
. implode(' ', $this->acme_args) . ' '
. "--accountconf ${account_conf_file}";
. exec_safe('--accountconf %s', $account_conf_file);
LeUtils::log_debug('running acme.sh command: ' . (string)$acmecmd, $this->debug);
$proc = proc_open($acmecmd, $proc_desc, $proc_pipes, null, $proc_env);
@@ -215,7 +215,7 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon
$key_length = $length;
}
$this->acme_args[] = '--keylength ' . $key_length;
$this->acme_args[] = exec_safe('--keylength %s', $key_length);
$this->cert_keylength = $length;
}
@@ -296,6 +296,6 @@ abstract class Base extends \OPNsense\AcmeClient\LeCommon
*/
public function setRenewal(int $interval = 60)
{
$this->acme_args[] = '--days ' . (string)$interval;
$this->acme_args[] = exec_safe('--days %s', (string)$interval);
}
}