From 9950bc6b6ed86c98b6b23fa8972025f6e29db5d5 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Mon, 19 Aug 2024 16:10:00 +0200
Subject: [PATCH] Pass openssl commands as an array

Puppet 7.9 introduced support to pass an array, which avoids using a
shell altogether. This simplifies the code because there's no more need
to escape and join options.
---
 manifests/export/pem_cert.pp                 | 20 ++++-------
 manifests/export/pem_key.pp                  | 20 +++++------
 manifests/export/pkcs12.pp                   | 37 +++++++++-----------
 metadata.json                                |  2 +-
 spec/defines/openssl_export_pem_cert_spec.rb |  8 ++---
 5 files changed, 37 insertions(+), 50 deletions(-)

diff --git a/manifests/export/pem_cert.pp b/manifests/export/pem_cert.pp
index 8b41011..826fa44 100644
--- a/manifests/export/pem_cert.pp
+++ b/manifests/export/pem_cert.pp
@@ -37,28 +37,20 @@
   }
 
   if $der_cert {
-    $sslmodule = 'x509'
+    $sslmodule = ['x509', '-inform', 'DER']
     $in_cert   = $der_cert
-    $module_opt  = '-inform DER'
   } else {
-    $sslmodule  = 'pkcs12'
+    $sslmodule  = ['pkcs12']
     $in_cert    = $pfx_cert
-    $module_opt   = ''
   }
 
   $passin_opt = $in_pass ? {
-    undef   => '',
-    default => "-nokeys -passin pass:${shellquote($in_pass)}",
+    undef   => [],
+    default => ['-nokeys', '-passin', "pass:${in_pass}"],
   }
 
   if $ensure == 'present' {
-    $cmd = [
-      "openssl ${sslmodule}",
-      $module_opt,
-      "-in ${in_cert}",
-      "-out ${pem_cert}",
-      $passin_opt,
-    ]
+    $cmd = ['openssl'] + $sslmodule + ['-in', $in_cert, '-out', $pem_cert] + $passin_opt
 
     if $dynamic {
       $exec_params = {
@@ -70,7 +62,7 @@
     }
 
     exec { "Export ${in_cert} to ${pem_cert}":
-      command => inline_template('<%= @cmd.join(" ") %>'),
+      command => $cmd,
       path    => $facts['path'],
       *       => $exec_params,
     }
diff --git a/manifests/export/pem_key.pp b/manifests/export/pem_key.pp
index eddc0fd..efbb9e7 100644
--- a/manifests/export/pem_key.pp
+++ b/manifests/export/pem_key.pp
@@ -26,23 +26,21 @@
 ) {
   if $ensure == 'present' {
     $passin_opt = $in_pass ? {
-      undef   => '',
-      default => "-passin pass:${shellquote($in_pass)}",
+      undef   => [],
+      default => ['-passin', "pass:${in_pass}"],
     }
 
     $passout_opt = $out_pass ? {
-      undef   => '-nodes',
-      default => "-passout pass:${shellquote($out_pass)}",
+      undef   => ['-nodes'],
+      default => ['-passout', "pass:${out_pass}"],
     }
 
     $cmd = [
-      'openssl pkcs12',
-      "-in ${pfx_cert}",
-      "-out ${pem_key}",
+      'openssl', 'pkcs12',
+      '-in', $pfx_cert,
+      '-out', $pem_key,
       '-nocerts',
-      $passin_opt,
-      $passout_opt,
-    ]
+    ] + $passin_opt + $passout_opt
 
     if $dynamic {
       $exec_params = {
@@ -54,7 +52,7 @@
     }
 
     exec { "Export ${pfx_cert} to ${pem_key}":
-      command => inline_template('<%= @cmd.join(" ") %>'),
+      command => $cmd,
       path    => $facts['path'],
       *       => $exec_params,
     }
diff --git a/manifests/export/pkcs12.pp b/manifests/export/pkcs12.pp
index e79bf14..ea3bace 100644
--- a/manifests/export/pkcs12.pp
+++ b/manifests/export/pkcs12.pp
@@ -30,35 +30,32 @@
   Optional[String]           $in_pass   = undef,
   Optional[String]           $out_pass  = undef,
 ) {
+  $full_path = "${basedir}/${name}.p12"
+
   if $ensure == 'present' {
     $pass_opt = $in_pass ? {
-      undef   => '',
-      default => "-passin pass:${shellquote($in_pass)}",
+      undef   => [],
+      default => ['-passin', "pass:${in_pass}"],
     }
 
     $passout_opt = $out_pass ? {
-      undef   => '',
-      default => "-passout pass:${shellquote($out_pass)}",
+      undef   => [],
+      default => ['-passout', "pass:${out_pass}"],
     }
 
     $chain_opt = $chaincert ? {
-      undef   => '',
-      default => "-chain -CAfile ${chaincert}",
+      undef   => [],
+      default => ['-chain', '-CAfile', $chaincert],
     }
 
     $cmd = [
-      'openssl pkcs12 -export',
-      "-in ${cert}",
-      "-inkey ${pkey}",
-      "-out ${basedir}/${name}.p12",
-      "-name ${name}",
-      '-nodes -noiter',
-      $chain_opt,
-      $pass_opt,
-      $passout_opt,
-    ]
-
-    $full_path = "${basedir}/${name}.p12"
+      'openssl', 'pkcs12', '-export',
+      '-in', $cert,
+      '-inkey', $pkey,
+      '-out', $full_path,
+      '-name', $name,
+      '-nodes', '-noiter',
+    ] + $chain_opt + $pass_opt + $passout_opt
 
     if $dynamic {
       $exec_params = {
@@ -70,12 +67,12 @@
     }
 
     exec { "Export ${name} to ${full_path}":
-      command => inline_template('<%= @cmd.join(" ") %>'),
+      command => $cmd,
       path    => $facts['path'],
       *       => $exec_params,
     }
   } else {
-    file { "${basedir}/${name}.p12":
+    file { $full_path:
       ensure => absent,
     }
   }
diff --git a/metadata.json b/metadata.json
index 96be615..df0d7d4 100644
--- a/metadata.json
+++ b/metadata.json
@@ -86,7 +86,7 @@
   "requirements": [
     {
       "name": "puppet",
-      "version_requirement": ">= 7.0.0 < 9.0.0"
+      "version_requirement": ">= 7.9.0 < 9.0.0"
     }
   ]
 }
diff --git a/spec/defines/openssl_export_pem_cert_spec.rb b/spec/defines/openssl_export_pem_cert_spec.rb
index a478741..b4c75d5 100644
--- a/spec/defines/openssl_export_pem_cert_spec.rb
+++ b/spec/defines/openssl_export_pem_cert_spec.rb
@@ -42,7 +42,7 @@
 
     it {
       is_expected.to contain_exec('Export /etc/ssl/certs/foo.pfx to /etc/ssl/certs/foo.pem').with(
-        command: 'openssl pkcs12  -in /etc/ssl/certs/foo.pfx -out /etc/ssl/certs/foo.pem ',
+        command: ['openssl', 'pkcs12', '-in', '/etc/ssl/certs/foo.pfx', '-out', '/etc/ssl/certs/foo.pem'],
         creates: '/etc/ssl/certs/foo.pem',
         path: '/usr/bin:/bin:/usr/sbin:/sbin'
       )
@@ -60,7 +60,7 @@
 
     it {
       is_expected.to contain_exec('Export /etc/ssl/certs/foo.pfx to /etc/ssl/certs/foo.pem').with(
-        command: 'openssl pkcs12  -in /etc/ssl/certs/foo.pfx -out /etc/ssl/certs/foo.pem ',
+        command: ['openssl', 'pkcs12', '-in', '/etc/ssl/certs/foo.pfx', '-out', '/etc/ssl/certs/foo.pem'],
         path: '/usr/bin:/bin:/usr/sbin:/sbin',
         refreshonly: true
       )
@@ -79,7 +79,7 @@
 
     it {
       is_expected.to contain_exec('Export /etc/ssl/certs/foo.pfx to /etc/ssl/certs/foo.pem').with(
-        command: "openssl pkcs12  -in /etc/ssl/certs/foo.pfx -out /etc/ssl/certs/foo.pem -nokeys -passin pass:'5r$}^'",
+        command: ['openssl', 'pkcs12', '-in', '/etc/ssl/certs/foo.pfx', '-out', '/etc/ssl/certs/foo.pem', '-nokeys', '-passin', "pass:5r$}^"],
         creates: '/etc/ssl/certs/foo.pem',
         path: '/usr/bin:/bin:/usr/sbin:/sbin'
       )
@@ -96,7 +96,7 @@
 
     it {
       is_expected.to contain_exec('Export /etc/ssl/certs/foo.der to /etc/ssl/certs/foo.pem').with(
-        command: 'openssl x509 -inform DER -in /etc/ssl/certs/foo.der -out /etc/ssl/certs/foo.pem ',
+        command: ['openssl', 'x509', '-inform', 'DER', '-in', '/etc/ssl/certs/foo.der', '-out', '/etc/ssl/certs/foo.pem'],
         creates: '/etc/ssl/certs/foo.pem',
         path: '/usr/bin:/bin:/usr/sbin:/sbin'
       )