Reapply "Send passwords via environment variables"

ekohl · ekohl · commit b62720fee3df · 2024-10-01T14:33:53.000+02:00
This reverts commit 41513a9. The previously implementation contained bugs and this is a proper fix.
diff --git a/lib/puppet/provider/x509_cert/openssl.rb b/lib/puppet/provider/x509_cert/openssl.rb
@@ -57,6 +57,8 @@ def exists?
   end
 
   def create
+    env = {}
+
     if resource[:csr]
       options = [
         'x509',
@@ -92,9 +94,15 @@ def create
 
     password = resource[:cakey_password] || resource[:password]
 
-    options += ['-passin', "pass:#{password}"] if password
+    if password
+      options += ['-passin', 'env:CERTIFICATE_PASSIN']
+      env['CERTIFICATE_PASSIN'] = password
+    end
     options += ['-extensions', 'v3_req'] if resource[:req_ext] != :false
-    openssl options
+
+    # openssl(options) doesn't work because it's impossible to pass an env
+    # https://github.com/puppetlabs/puppet/issues/9493
+    execute([command('openssl')] + options, { failonfail: true, combine: true, custom_environment: env })
   end
 
   def destroy
diff --git a/lib/puppet/provider/x509_request/openssl.rb b/lib/puppet/provider/x509_request/openssl.rb
@@ -28,17 +28,23 @@ def exists?
   end
 
   def create
+    env = {}
     options = [
       'req', '-new',
       '-key', resource[:private_key],
       '-config', resource[:template],
       '-out', resource[:path]
     ]
 
-    options += ['-passin', "pass:#{resource[:password]}"] if resource[:password]
-    options += ['-nodes'] unless resource[:encrypted]
+    if resource[:password]
+      options += ['-passin', 'env:CERTIFICATE_PASSIN']
+      env['CERTIFICATE_PASSIN'] = resource[:password]
+    end
+    options << '-nodes' unless resource[:encrypted]
 
-    openssl options
+    # openssl(options) doesn't work because it's impossible to pass an env
+    # https://github.com/puppetlabs/puppet/issues/9493
+    execute([command('openssl')] + options, { failonfail: true, combine: true, custom_environment: env })
   end
 
   def destroy
diff --git a/manifests/export/pem_cert.pp b/manifests/export/pem_cert.pp
@@ -44,9 +44,12 @@
     $in_cert    = $pfx_cert
   }
 
-  $passin_opt = $in_pass ? {
-    undef   => [],
-    default => ['-nokeys', '-passin', "pass:${in_pass}"],
+  if $in_pass {
+    $passin_opt = ['-nokeys', '-passin', 'env:CERTIFICATE_PASSIN']
+    $passin_env = ["CERTIFICATE_PASSIN=${in_pass}"]
+  } else {
+    $passin_opt = []
+    $passin_env = []
   }
 
   if $ensure == 'present' {
@@ -62,9 +65,10 @@
     }
 
     exec { "Export ${in_cert} to ${pem_cert}":
-      command => $cmd,
-      path    => $facts['path'],
-      *       => $exec_params,
+      command     => $cmd,
+      environment => $passin_env,
+      path        => $facts['path'],
+      *           => $exec_params,
     }
   } else {
     file { $pem_cert:
diff --git a/manifests/export/pem_key.pp b/manifests/export/pem_key.pp
@@ -25,14 +25,20 @@
   Optional[String]           $out_pass  = undef,
 ) {
   if $ensure == 'present' {
-    $passin_opt = $in_pass ? {
-      undef   => [],
-      default => ['-passin', "pass:${in_pass}"],
+    if $in_pass {
+      $passin_opt = ['-nokeys', '-passin', 'env:CERTIFICATE_PASSIN']
+      $passin_env = ["CERTIFICATE_PASSIN=${in_pass}"]
+    } else {
+      $passin_opt = []
+      $passin_env = []
     }
 
-    $passout_opt = $out_pass ? {
-      undef   => ['-nodes'],
-      default => ['-passout', "pass:${out_pass}"],
+    if $out_pass {
+      $passout_opt = ['-nokeys', '-passout', 'env:CERTIFICATE_PASSOUT']
+      $passout_env = ["CERTIFICATE_PASSOUT=${out_pass}"]
+    } else {
+      $passout_opt = []
+      $passout_env = []
     }
 
     $cmd = [
@@ -52,9 +58,10 @@
     }
 
     exec { "Export ${pfx_cert} to ${pem_key}":
-      command => $cmd,
-      path    => $facts['path'],
-      *       => $exec_params,
+      command     => $cmd,
+      environment => $passin_env + $passout_env,
+      path        => $facts['path'],
+      *           => $exec_params,
     }
   } else {
     file { $pem_key:
diff --git a/manifests/export/pkcs12.pp b/manifests/export/pkcs12.pp
@@ -33,14 +33,20 @@
   $full_path = "${basedir}/${name}.p12"
 
   if $ensure == 'present' {
-    $pass_opt = $in_pass ? {
-      undef   => [],
-      default => ['-passin', "pass:${in_pass}"],
+    if $in_pass {
+      $passin_opt = ['-nokeys', '-passin', 'env:CERTIFICATE_PASSIN']
+      $passin_env = ["CERTIFICATE_PASSIN=${in_pass}"]
+    } else {
+      $passin_opt = []
+      $passin_env = []
     }
 
-    $passout_opt = $out_pass ? {
-      undef   => [],
-      default => ['-passout', "pass:${out_pass}"],
+    if $out_pass {
+      $passout_opt = ['-nokeys', '-passout', 'env:CERTIFICATE_PASSOUT']
+      $passout_env = ["CERTIFICATE_PASSOUT=${out_pass}"]
+    } else {
+      $passout_opt = []
+      $passout_env = []
     }
 
     $chain_opt = $chaincert ? {
@@ -55,7 +61,7 @@
       '-out', $full_path,
       '-name', $name,
       '-nodes', '-noiter',
-    ] + $chain_opt + $pass_opt + $passout_opt
+    ] + $chain_opt + $passin_opt + $passout_opt
 
     if $dynamic {
       $exec_params = {
@@ -67,9 +73,10 @@
     }
 
     exec { "Export ${name} to ${full_path}":
-      command => $cmd,
-      path    => $facts['path'],
-      *       => $exec_params,
+      command     => $cmd,
+      environment => $passin_env + $passout_env,
+      path        => $facts['path'],
+      *           => $exec_params,
     }
   } else {
     file { $full_path:
diff --git a/spec/defines/openssl_export_pem_cert_spec.rb b/spec/defines/openssl_export_pem_cert_spec.rb
@@ -79,7 +79,8 @@
 
     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', 'env:CERTIFICATE_PASSIN'],
+        environment: ['CERTIFICATE_PASSIN=5r$}^'],
         creates: '/etc/ssl/certs/foo.pem',
         path: '/usr/bin:/bin:/usr/sbin:/sbin'
       )
diff --git a/spec/unit/puppet/provider/x509_cert/openssl_spec.rb b/spec/unit/puppet/provider/x509_cert/openssl_spec.rb
@@ -5,7 +5,6 @@
 require 'pathname'
 require 'puppet/type/x509_cert'
 
-provider_class = Puppet::Type.type(:x509_cert).provider(:openssl)
 describe 'The openssl provider for the x509_cert type' do
   let(:path) { '/tmp/foo.crt' }
   let(:pathname) { Pathname.new(path) }
@@ -31,33 +30,49 @@
     end
 
     it 'creates a certificate with the proper options' do
-      expect(provider_class).to receive(:openssl).with([
-                                                         'req',
-                                                         '-config', '/tmp/foo.cnf',
-                                                         '-new',
-                                                         '-x509',
-                                                         '-days', 3650,
-                                                         '-key', '/tmp/foo.key',
-                                                         '-out', '/tmp/foo.crt',
-                                                         '-extensions', 'v3_req',
-                                                       ])
+      expect(resource.provider).to receive(:execute).with(
+        [
+          '/usr/bin/openssl',
+          'req',
+          '-config', '/tmp/foo.cnf',
+          '-new',
+          '-x509',
+          '-days', 3650,
+          '-key', '/tmp/foo.key',
+          '-out', '/tmp/foo.crt',
+          '-extensions', 'v3_req',
+        ],
+        {
+          combine: true,
+          custom_environment: {},
+          failonfail: true,
+        }
+      )
       resource.provider.create
     end
 
     context 'when using password' do
       it 'creates a certificate with the proper options' do
         resource[:password] = '2x6${'
-        expect(provider_class).to receive(:openssl).with([
-                                                           'req',
-                                                           '-config', '/tmp/foo.cnf',
-                                                           '-new',
-                                                           '-x509',
-                                                           '-days', 3650,
-                                                           '-key', '/tmp/foo.key',
-                                                           '-out', '/tmp/foo.crt',
-                                                           '-passin', 'pass:2x6${',
-                                                           '-extensions', 'v3_req',
-                                                         ])
+        expect(resource.provider).to receive(:execute).with(
+          [
+            '/usr/bin/openssl',
+            'req',
+            '-config', '/tmp/foo.cnf',
+            '-new',
+            '-x509',
+            '-days', 3650,
+            '-key', '/tmp/foo.key',
+            '-out', '/tmp/foo.crt',
+            '-passin', 'env:CERTIFICATE_PASSIN',
+            '-extensions', 'v3_req',
+          ],
+          {
+            combine: true,
+            custom_environment: { 'CERTIFICATE_PASSIN' => '2x6${' },
+            failonfail: true,
+          }
+        )
         resource.provider.create
       end
     end
@@ -68,18 +83,26 @@
       resource[:csr] = '/tmp/foo.csr'
       resource[:ca]  = '/tmp/foo-ca.crt'
       resource[:cakey] = '/tmp/foo-ca.key'
-      expect(provider_class).to receive(:openssl).with([
-                                                         'x509',
-                                                         '-req',
-                                                         '-days', 3650,
-                                                         '-in', '/tmp/foo.csr',
-                                                         '-out', '/tmp/foo.crt',
-                                                         '-extfile', '/tmp/foo.cnf',
-                                                         '-CAcreateserial',
-                                                         '-CA', '/tmp/foo-ca.crt',
-                                                         '-CAkey', '/tmp/foo-ca.key',
-                                                         '-extensions', 'v3_req',
-                                                       ])
+      expect(resource.provider).to receive(:execute).with(
+        [
+          '/usr/bin/openssl',
+          'x509',
+          '-req',
+          '-days', 3650,
+          '-in', '/tmp/foo.csr',
+          '-out', '/tmp/foo.crt',
+          '-extfile', '/tmp/foo.cnf',
+          '-CAcreateserial',
+          '-CA', '/tmp/foo-ca.crt',
+          '-CAkey', '/tmp/foo-ca.key',
+          '-extensions', 'v3_req',
+        ],
+        {
+          combine: true,
+          custom_environment: {},
+          failonfail: true,
+        }
+      )
       resource.provider.create
     end
   end
@@ -90,19 +113,27 @@
       resource[:ca]  = '/tmp/foo-ca.crt'
       resource[:cakey] = '/tmp/foo-ca.key'
       resource[:cakey_password] = '5i;6%'
-      expect(provider_class).to receive(:openssl).with([
-                                                         'x509',
-                                                         '-req',
-                                                         '-days', 3650,
-                                                         '-in', '/tmp/foo.csr',
-                                                         '-out', '/tmp/foo.crt',
-                                                         '-extfile', '/tmp/foo.cnf',
-                                                         '-CAcreateserial',
-                                                         '-CA', '/tmp/foo-ca.crt',
-                                                         '-CAkey', '/tmp/foo-ca.key',
-                                                         '-passin', 'pass:5i;6%',
-                                                         '-extensions', 'v3_req',
-                                                       ])
+      expect(resource.provider).to receive(:execute).with(
+        [
+          '/usr/bin/openssl',
+          'x509',
+          '-req',
+          '-days', 3650,
+          '-in', '/tmp/foo.csr',
+          '-out', '/tmp/foo.crt',
+          '-extfile', '/tmp/foo.cnf',
+          '-CAcreateserial',
+          '-CA', '/tmp/foo-ca.crt',
+          '-CAkey', '/tmp/foo-ca.key',
+          '-passin', 'env:CERTIFICATE_PASSIN',
+          '-extensions', 'v3_req',
+        ],
+        {
+          combine: true,
+          custom_environment: { 'CERTIFICATE_PASSIN' => '5i;6%' },
+          failonfail: true,
+        }
+      )
       resource.provider.create
     end
   end
diff --git a/spec/unit/puppet/provider/x509_request/openssl_spec.rb b/spec/unit/puppet/provider/x509_request/openssl_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,8 @@`
`79`	`79`
`80`	`80`	`it {`
`81`	`81`	`is_expected.to contain_exec('Export /etc/ssl/certs/foo.pfx to /etc/ssl/certs/foo.pem').with(`
`82`		`- command: ['openssl', 'pkcs12', '-in', '/etc/ssl/certs/foo.pfx', '-out', '/etc/ssl/certs/foo.pem', '-nokeys', '-passin', 'pass:5r$}^'],`
	`82`	`+ command: ['openssl', 'pkcs12', '-in', '/etc/ssl/certs/foo.pfx', '-out', '/etc/ssl/certs/foo.pem', '-nokeys', '-passin', 'env:CERTIFICATE_PASSIN'],`
	`83`	`+ environment: ['CERTIFICATE_PASSIN=5r$}^'],`
`83`	`84`	`creates: '/etc/ssl/certs/foo.pem',`
`84`	`85`	`path: '/usr/bin:/bin:/usr/sbin:/sbin'`
`85`	`86`	`)`