From b62720fee3dfa2cc40328c0884b6fe2d32cb6e30 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Mon, 30 Sep 2024 17:50:04 +0200 Subject: [PATCH] Reapply "Send passwords via environment variables" This reverts commit 41513a9ac1c302033edd774fb1bd8f8af8c78f69. The previously implementation contained bugs and this is a proper fix. --- lib/puppet/provider/x509_cert/openssl.rb | 12 +- lib/puppet/provider/x509_request/openssl.rb | 12 +- manifests/export/pem_cert.pp | 16 ++- manifests/export/pem_key.pp | 25 ++-- manifests/export/pkcs12.pp | 27 ++-- spec/defines/openssl_export_pem_cert_spec.rb | 3 +- .../puppet/provider/x509_cert/openssl_spec.rb | 125 +++++++++++------- .../provider/x509_request/openssl_spec.rb | 43 ++++-- 8 files changed, 171 insertions(+), 92 deletions(-) diff --git a/lib/puppet/provider/x509_cert/openssl.rb b/lib/puppet/provider/x509_cert/openssl.rb index 3fa3b19..f5f9339 100644 --- 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 index f25b2a7..05a7c8c 100644 --- a/lib/puppet/provider/x509_request/openssl.rb +++ b/lib/puppet/provider/x509_request/openssl.rb @@ -28,6 +28,7 @@ def exists? end def create + env = {} options = [ 'req', '-new', '-key', resource[:private_key], @@ -35,10 +36,15 @@ def create '-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 index 826fa44..d19132f 100644 --- 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 index efbb9e7..390b8b1 100644 --- 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 index ea3bace..ccef3f3 100644 --- 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 index d5cf277..f04c649 100644 --- 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 index 74902c6..4b94ce7 100644 --- 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 index ce5a466..57d113e 100644 --- a/spec/unit/puppet/provider/x509_request/openssl_spec.rb +++ b/spec/unit/puppet/provider/x509_request/openssl_spec.rb @@ -4,7 +4,6 @@ require 'pathname' require 'puppet/type/x509_request' -provider_class = Puppet::Type.type(:x509_request).provider(:openssl) describe 'The openssl provider for the x509_request type' do let(:path) { '/tmp/foo.csr' } let(:pathname) { Pathname.new(path) } @@ -27,12 +26,20 @@ end it 'creates a certificate with the proper options' do - expect(provider_class).to receive(:openssl).with([ - 'req', '-new', - '-key', '/tmp/foo.key', - '-config', '/tmp/foo.cnf', - '-out', '/tmp/foo.csr' - ]) + expect(resource.provider).to receive(:execute).with( + [ + '/usr/bin/openssl', + 'req', '-new', + '-key', '/tmp/foo.key', + '-config', '/tmp/foo.cnf', + '-out', '/tmp/foo.csr' + ], + { + combine: true, + custom_environment: {}, + failonfail: true, + } + ) resource.provider.create end end @@ -40,13 +47,21 @@ 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', '-new', - '-key', '/tmp/foo.key', - '-config', '/tmp/foo.cnf', - '-out', '/tmp/foo.csr', - '-passin', 'pass:2x6${', - ]) + expect(resource.provider).to receive(:execute).with( + [ + '/usr/bin/openssl', + 'req', '-new', + '-key', '/tmp/foo.key', + '-config', '/tmp/foo.cnf', + '-out', '/tmp/foo.csr', + '-passin', 'env:CERTIFICATE_PASSIN', + ], + { + combine: true, + custom_environment: { 'CERTIFICATE_PASSIN' => '2x6${' }, + failonfail: true, + } + ) resource.provider.create end end