Skip to content

Commit

Permalink
acme, use escapeshellarg() for shell commands, provide more sensible …
Browse files Browse the repository at this point in the history
…warnings, use fa-plus instead of fa-level-down icon
  • Loading branch information
PiBa-NL committed Jan 10, 2017
1 parent cb886a0 commit f885d77
Show file tree
Hide file tree
Showing 11 changed files with 55 additions and 81 deletions.
2 changes: 2 additions & 0 deletions security/pfSense-pkg-acme/Makefile
Expand Up @@ -12,6 +12,8 @@ COMMENT= pfSense package acme

USE_PHP= ftp

RUN_DEPENDS= samba-nsupdate:dns/samba-nsupdate

CONFLICTS=

NO_BUILD= yes
Expand Down
4 changes: 2 additions & 2 deletions security/pfSense-pkg-acme/files/usr/local/pkg/acme/acme.inc
Expand Up @@ -196,15 +196,15 @@ $acme_domain_validation_method['dns_me'] = array(name => "DNS-DNSMadeEasy",
'description' =>"Fill in the API Secret"
)
));
/*$acme_domain_validation_method['dns_nsupdate'] = array(name => "DNS-NSupdate",
$acme_domain_validation_method['dns_nsupdate'] = array(name => "DNS-NSupdate",
'fields' => array(
'NSUPDATE_SERVER' => array('name'=>"NSUPDATE_SERVER",'columnheader'=>"Key",'type'=>"textbox",
'description' =>"Fill in the API Key"
),
'NSUPDATE_KEY' => array('name'=>"NSUPDATE_KEY",'columnheader'=>"Id",'type'=>"textarea",
'description' =>"Fill in the API Id"
)
));needs a file reference..*/
));
$acme_domain_validation_method['dns_ovh'] = array(name => "DNS-ovh / kimsufi / soyoustart / runabove",
'fields' => array(
'OVH_AK' => array('name'=>"OVH_AK",'columnheader'=>"Application Key",'type'=>"textbox",
Expand Down
Expand Up @@ -32,7 +32,7 @@ $acme_icons = array(
"icon" => "icon_down.gif",
"iconsize" => 17),
'add' => array(
"faicon" => "fa-level-down",
"faicon" => "fa-plus",
"icon" => "icon_plus.gif",
"iconsize" => 17),
'delete' => array(
Expand Down
Expand Up @@ -332,9 +332,9 @@ EOT
}
$result .= "</tbody>
</table>
<a onclick='javascript:addRowTo(\"{$tablename}\"); return false;' href='#'>
".acmeicon('add','add another entry')."
</a>
<button class='btn btn-success addbtn' type='button' value='add another entry'
onclick='javascript:addRowTo(\"{$tablename}\"); return false;'
><i class='fa fa-plus icon-embed-btn'></i>Add</button>
</div></div>";
return $result;
}
Expand Down
38 changes: 22 additions & 16 deletions security/pfSense-pkg-acme/files/usr/local/pkg/acme/acme_sh.inc
Expand Up @@ -80,19 +80,24 @@ class acme_sh {
function generateAccountKey() {
unlink_if_exists("{$this->path_account}/account.key");
$this->debug = false;
$this->execacmesh("--home {$this->acmeconf} --createAccountKey --accountkeylength 4096 --accountconf {$this->accountconfig}");
$this->execacmesh(""
. " --home " . escapeshellarg($this->acmeconf)
. " --createAccountKey"
. " --accountkeylength 4096"
. " --accountconf " . escapeshellarg($this->accountconfig)
);
$privateKey = file_get_contents("{$this->path_account}/account.key");
return $privateKey;
}

function registeraccount($key) {
file_put_contents("{$this->path_account}/account.key", $key);
$result = $this->execacmesh(""
. " --home {$this->acmeconf}"
. " --home " . escapeshellarg($this->acmeconf)
. " --registeraccount"
. " --accountconf {$this->accountconfig}"
. " --accountconf " . escapeshellarg($this->accountconfig)
. " --log-level 3"
. " --log {$this->acmeconf}acme_issuecert.log");
. " --log " . escapeshellarg($this->acmeconf."acme_issuecert.log"));
return $result == 0;
}

Expand All @@ -104,15 +109,16 @@ class acme_sh {
}
$certpath = "{$this->acmeconf}{$domain}{$pathadd}";
safe_mkdir($certpath);

unlink_if_exists("{$certpath}/{$domain}.key");
$this->execacmesh(""
. " --home {$this->acmeconf}"
. " --accountconf {$this->accountconfig}"
. " --createDomainKey -d $domain"
. " --keylength $keylength"
. " --home " . escapeshellarg($this->acmeconf)
. " --accountconf " . escapeshellarg($this->accountconfig)
. " --createDomainKey -d " . escapeshellarg($domain)
. " --keylength " . escapeshellarg($keylength)
. " --log-level 3"
. " --log {$this->acmeconf}acme_createdomainkey.log");
. " --log " . escapeshellarg($this->acmeconf."acme_createdomainkey.log")
);
$privateKey = file_get_contents("{$certpath}/{$domain}.key");
return $privateKey;
}
Expand All @@ -125,7 +131,7 @@ class acme_sh {
if ($api == "dns_manual") {
$api = "";
}
$cmdparameters = " --dns {$api}";
$cmdparameters = " --dns " . escapeshellarg($api);
} else {
$cmdparameters = " --webroot pfSenseacme";
}
Expand Down Expand Up @@ -166,17 +172,17 @@ EOF;
file_put_contents("{$certpath}/{$domainstosign[0]}.key", $certificatepsk);
$domainstr = "";
foreach($domainstosign as $domain) {
$domainstr .= " -d {$domain}";
$domainstr .= " -d " . escapeshellarg($domain);
}
$this->execacmesh(""
. " --{$action} {$domainstr}"
. " --home {$this->acmeconf}"
. " --accountconf {$this->accountconfig}"
. " --home " . escapeshellarg($this->acmeconf)
. " --accountconf " . escapeshellarg($this->accountconfig)
. " --force"
. " --reloadCmd {$this->acmeconf}reloadcmd.sh"
. " --reloadCmd " . escapeshellarg("{$this->acmeconf}reloadcmd.sh")
. $cmdparameters
. " --log-level 3"
. " --log {$this->acmeconf}acme_issuecert.log"
. " --log " . escapeshellarg("{$this->acmeconf}acme_issuecert.log")
//. " > {$this->acmeconf}issue.log 2>&1"
, $envvariables);
$cer = "{$certpath}/{$domainstosign[0]}.cer";
Expand Down
Expand Up @@ -195,7 +195,7 @@
</div>
<nav class="action-buttons">
<a href="acme_accountkeys_edit.php" role="button" class="btn btn-sm btn-success" title="<?=gettext('Add backend to the end of the list')?>">
<i class="fa fa-level-down icon-embed-btn"></i>
<i class="fa fa-plus icon-embed-btn"></i>
<?=gettext("Add");?>
</a>
<button name="del_x" type="submit" class="btn btn-danger btn-sm" value="<?=gettext("Delete selected backends"); ?>" title="<?=gettext('Delete selected backends')?>">
Expand Down
Expand Up @@ -206,59 +206,18 @@ function customdrawcell_actions($object, $item, $itemvalue, $editable, $itemname
}
}

/*if (preg_match("/[^a-zA-Z0-9\.\-_]/", $_POST['name'])) {
$input_errors[] = "The field 'Name' contains invalid characters.";
}
if ($_POST['checkinter'] !== "" && !is_numeric($_POST['checkinter'])) {
$input_errors[] = "The field 'Check frequency' value is not a number.";
}
if ($_POST['connection_timeout'] !== "" && !is_numeric($_POST['connection_timeout'])) {
$input_errors[] = "The field 'Connection timeout' value is not a number.";
}
if ($_POST['server_timeout'] !== "" && !is_numeric($_POST['server_timeout'])) {
$input_errors[] = "The field 'Server timeout' value is not a number.";
}
if ($_POST['retries'] !== "" && !is_numeric($_POST['retries'])) {
$input_errors[] = "The field 'Retries' value is not a number.";
}
// the colon ":" is invalid in the username, other than that pretty much any character can be used.
if (preg_match("/[^a-zA-Z0-9!-\/;-~ ]/", $_POST['stats_username'])) {
$input_errors[] = "The field 'Stats Username' contains invalid characters.";
}
// the colon ":" can also be used in the password
if (preg_match("/[^a-zA-Z0-9!-~ ]/", $_POST['stats_password'])) {
$input_errors[] = "The field 'Stats Password' contains invalid characters.";
}
if (preg_match("/[^a-zA-Z0-9\-_]/", $_POST['stats_node'])) {
$input_errors[] = "The field 'Stats Node' contains invalid characters. Should be a string with digits(0-9), letters(A-Z, a-z), hyphen(-) or underscode(_)";
}*/

/* Ensure that our pool names are unique */
/* Ensure that our account key names are unique */
for ($i=0; isset($config['installedpackages']['acme']['accountkeys']['item'][$i]); $i++) {
if (($_POST['name'] == $config['installedpackages']['acme']['accountkeys']['item'][$i]['name']) && ($i != $id)) {
$input_errors[] = "This pool name has already been used. Pool names must be unique.";
}
}
$a_domains = $domainslist->acme_htmllist_get_values();
foreach($a_domains as $server){
$domain_name = $server['name'];
if (!is_hostname($domain_name)) {
$input_errors[] = "The field 'Domainname' does not contain a valid hostname.";
$input_errors[] = "This name has already been used. Names must be unique.";
}
}
$a_actions = $actionslist->acme_htmllist_get_values();

$accountkey = array();
if(isset($id) && $a_accountkeys[$id]) {
$accountkey = $a_accountkeys[$id];
}

// echo "newname id:$id";

if (!empty($accountkey['name']) && ($accountkey['name'] != $_POST['name'])) {
//old $accountkey['name'] can be empty if a new or cloned item is saved, nothing should be renamed then
// name changed:
Expand All @@ -272,7 +231,7 @@ function customdrawcell_actions($object, $item, $itemvalue, $editable, $itemname
}

if($accountkey['name'] != "") {
$changedesc .= " modified pool: '{$accountkey['name']}'";
$changedesc .= " modified account key: '{$accountkey['name']}'";
}
$accountkey['a_domainlist']['item'] = $a_domains;
$accountkey['a_actionlist']['item'] = $a_actions;
Expand Down Expand Up @@ -342,13 +301,13 @@ function customdrawcell_actions($object, $item, $itemvalue, $editable, $itemname
$section->addInput(new \Form_StaticText(
'',
"<a id='btncreatekey' class='btn btn-sm btn-primary'>"
. "<i id='btncreatekeyicon' class='fa fa-check'></i> Create new account key</a>"
. "<i id='btncreatekeyicon' class='fa fa-plus'></i> Create new account key</a>"
));

$section->addInput(new \Form_StaticText(
'Acme account registration',
"<a id='btnregisterkey' class='btn btn-sm btn-primary'>"
. "<i id='btnregisterkeyicon' class='fa fa-check'></i> Register acme account key</a>"
. "<i id='btnregisterkeyicon' class='fa fa-key'></i> Register acme account key</a>"
))->setHelp("Before using a accountkey it must first be registered at the chosen CA.");

$form->add($section);
Expand Down Expand Up @@ -422,7 +381,7 @@ function createkey() {
}
events.push(function() {
$('#btnregisterkey').click(function() {
$("#btnregisterkeyicon").removeClass("fa-check").addClass("fa-cog fa-spin");
$("#btnregisterkeyicon").removeClass("fa-key").addClass("fa-cog fa-spin");
var key = $("#accountkey").val();
var caname = $("#acmeserver").val();
ajaxRequest = $.ajax({
Expand All @@ -432,11 +391,10 @@ function createkey() {
$("#btnregisterkeyicon").removeClass("fa-cog fa-spin").addClass("fa-check");
}
});

});

$('#btncreatekey').click(function() {
$("#btncreatekeyicon").removeClass("fa-check").addClass("fa-cog fa-spin");
$("#btncreatekeyicon").removeClass("fa-plus").addClass("fa-cog fa-spin");
var caname = $("#acmeserver").val();
ajaxRequest = $.ajax({
type: "post",
Expand Down
Expand Up @@ -265,7 +265,7 @@
</div>
<nav class="action-buttons">
<a href="acme_certificates_edit.php" role="button" class="btn btn-sm btn-success" title="<?=gettext('Add backend to the end of the list')?>">
<i class="fa fa-level-down icon-embed-btn"></i>
<i class="fa fa-plus icon-embed-btn"></i>
<?=gettext("Add");?>
</a>
<button name="del_x" type="submit" class="btn btn-danger btn-sm" value="<?=gettext("Delete selected backends"); ?>" title="<?=gettext('Delete selected backends')?>">
Expand Down
Expand Up @@ -193,15 +193,15 @@ function customdrawcell_actions($object, $item, $itemvalue, $editable, $itemname
}
}

/* Ensure that our pool names are unique */
/* Ensure that our certificate names are unique */
for ($i=0; isset($config['installedpackages']['acme']['certificates']['item'][$i]); $i++) {
if (($_POST['name'] == $config['installedpackages']['acme']['certificates']['item'][$i]['name']) && ($i != $id)) {
$input_errors[] = "This pool name has already been used. Pool names must be unique.";
$input_errors[] = "This name has already been used. Names must be unique.";
}
}
$a_domains = $domainslist->acme_htmllist_get_values();
foreach($a_domains as $server){
$domain_name = $server['name'];
$domain_name = $server['name'];
if (!is_hostname($domain_name)) {
$input_errors[] = "The field 'Domainname' does not contain a valid hostname.";
}
Expand All @@ -227,7 +227,7 @@ function customdrawcell_actions($object, $item, $itemvalue, $editable, $itemname
}

if($certificate['name'] != "") {
$changedesc .= " modified pool: '{$certificate['name']}'";
$changedesc .= " modified certificate: '{$certificate['name']}'";
}
$certificate['a_domainlist']['item'] = $a_domains;
$certificate['a_actionlist']['item'] = $a_actions;
Expand Down Expand Up @@ -313,7 +313,7 @@ function updatevisibility()

$section = new \Form_Section('Edit Certificate options');
$section->addInput(new \Form_Input('name', 'Name', 'text', $pconfig['name']
))->setHelp('The name set here will also be used to create or overwrite a certificate that might already exist with this name in the pfSense certificate m.');
))->setHelp('The name set here will also be used to create or overwrite a certificate that might already exist with this name in the pfSense Certificate Manager.');
$section->addInput(new \Form_Input('desc', 'Description', 'text', $pconfig['desc']));
$activedisable = array();
$activedisable['active'] = "Active";
Expand Down Expand Up @@ -364,6 +364,15 @@ function updatevisibility()

$form->add($section);

if (!is_array($a_accountkeys) || count($a_accountkeys) == 0) {
$form = new \Form;
$section = new \Form_Section('Edit Certificate options');
$section->addInput(new \Form_StaticText(
'Accountkey required',
"A account key should be created and registered before configuring certificates."
));
$form->add($section);
}
print $form;
?>
<?php if (isset($id) && $a_certificates[$id]): ?>
Expand Down
Expand Up @@ -72,7 +72,7 @@
$section->addInput(new \Form_Checkbox(
'enable',
'',
'Enable Acme client renewal job',
'Enable Acme client renewal job. This will configure cron to renew certificates once a day at 3:16. Keeping track of the last succesfull renewal and the number of days set after to renew again. When renewal happens a service can be restarted or a shell script run to load the new certificate for services that need it, if needed this needs to be configured as a action under the certificate aettings.',
$pconfig['enable']
));

Expand Down
1 change: 0 additions & 1 deletion security/pfSense-pkg-acme/pkg-plist
Expand Up @@ -4,7 +4,6 @@ pkg/acme/acme_gui.inc
pkg/acme/acme_htmllist.inc
pkg/acme/acme_serverconnectors.inc
pkg/acme/acme_utils.inc
pkg/acme/acme.inc
pkg/acme/acme.sh
pkg/acme/acme_command.sh
pkg/acme/acme_sh.inc
Expand Down

0 comments on commit f885d77

Please sign in to comment.