Merge pull request #190 from ekohl/use-openssl-pkey-read

Use OpenSSL::PKey.read to read private keys

REFERENCE.md

@@ -1318,7 +1318,6 @@ Default value: `present`
 
 The following parameters are available in the `x509_cert` type.
 
-* [`authentication`](#-x509_cert--authentication)
 * [`ca`](#-x509_cert--ca)
 * [`cakey`](#-x509_cert--cakey)
 * [`cakey_password`](#-x509_cert--cakey_password)
@@ -1332,14 +1331,6 @@ The following parameters are available in the `x509_cert` type.
 * [`req_ext`](#-x509_cert--req_ext)
 * [`template`](#-x509_cert--template)
 
-##### <a name="-x509_cert--authentication"></a>`authentication`
-
-Valid values: `rsa`, `dsa`, `ec`
-
-The authentication algorithm: 'rsa', 'dsa or ec'
-
-Default value: `rsa`
-
 ##### <a name="-x509_cert--ca"></a>`ca`
 
 The optional ca certificate filepath
@@ -1421,7 +1412,6 @@ Default value: `present`
 
 The following parameters are available in the `x509_request` type.
 
-* [`authentication`](#-x509_request--authentication)
 * [`encrypted`](#-x509_request--encrypted)
 * [`force`](#-x509_request--force)
 * [`password`](#-x509_request--password)
@@ -1430,14 +1420,6 @@ The following parameters are available in the `x509_request` type.
 * [`provider`](#-x509_request--provider)
 * [`template`](#-x509_request--template)
 
-##### <a name="-x509_request--authentication"></a>`authentication`
-
-Valid values: `rsa`, `dsa`, `ec`
-
-The authentication algorithm: 'rsa', 'dsa' or ec
-
-Default value: `rsa`
-
 ##### <a name="-x509_request--encrypted"></a>`encrypted`
 
 Valid values: `true`, `false`

lib/puppet/provider/x509_cert/openssl.rb

@@ -8,17 +8,7 @@
 
   def self.private_key(resource)
     file = File.read(resource[:private_key])
-    case resource[:authentication]
-    when :dsa
-      OpenSSL::PKey::DSA.new(file, resource[:password])
-    when :rsa
-      OpenSSL::PKey::RSA.new(file, resource[:password])
-    when :ec
-      OpenSSL::PKey::EC.new(file, resource[:password])
-    else
-      raise Puppet::Error,
-            "Unknown authentication type '#{resource[:authentication]}'"
-    end
+    OpenSSL::PKey.read(file, resource[:password])
   end
 
   def self.check_private_key(resource)

lib/puppet/provider/x509_request/openssl.rb

@@ -8,17 +8,7 @@
 
   def self.private_key(resource)
     file = File.read(resource[:private_key])
-    case resource[:authentication]
-    when :dsa
-      OpenSSL::PKey::DSA.new(file, resource[:password])
-    when :rsa
-      OpenSSL::PKey::RSA.new(file, resource[:password])
-    when :ec
-      OpenSSL::PKey::EC.new(file, resource[:password])
-    else
-      raise Puppet::Error,
-            "Unknown authentication type '#{resource[:authentication]}'"
-    end
+    OpenSSL::PKey.read(file, resource[:password])
   end
 
   def self.check_private_key(resource)

lib/puppet/type/x509_cert.rb

@@ -60,12 +60,6 @@
     end
   end
 
-  newparam(:authentication) do
-    desc "The authentication algorithm: 'rsa', 'dsa or ec'"
-    newvalues :rsa, :dsa, :ec
-    defaultto :rsa
-  end
-
   newparam(:csr) do
     desc 'The optional certificate signing request path'
   end

lib/puppet/type/x509_request.rb

@@ -48,12 +48,6 @@
     end
   end
 
-  newparam(:authentication) do
-    desc "The authentication algorithm: 'rsa', 'dsa' or ec"
-    newvalues :rsa, :dsa, :ec
-    defaultto :rsa
-  end
-
   newparam(:encrypted, boolean: true) do
     desc 'Whether to generate the key unencrypted. This is needed by some applications like OpenLDAP'
     newvalues(:true, :false)

spec/unit/puppet/provider/cert_file/posix_spec.rb

@@ -25,11 +25,13 @@
   end
 
   let(:path) { '/tmp/test.pem' }
+  let(:pathname) { instance_double(Pathname) }
   let(:source) { 'http://example.org/cert.der' }
   let(:resource) { Puppet::Type::Cert_file.new(path: path, source: source) }
 
   it 'exists? returns false on arbitraty path' do
-    allow_any_instance_of(Pathname).to receive(:exist?).and_return(false) # rubocop:disable RSpec/AnyInstance
+    allow(Pathname).to receive(:new).with(path).and_return(pathname)
+    allow(pathname).to receive(:exist?).and_return(false)
     expect(resource.provider.exists?).to be(false)
   end
 

spec/unit/puppet/provider/ssl_pkey/openssl_spec.rb

@@ -25,15 +25,15 @@
   context 'when creating a key with defaults' do
     it 'creates an rsa key' do
       allow(OpenSSL::PKey::RSA).to receive(:new).with(2048).and_return(key)
-      allow(File).to receive(:open).with('/tmp/foo.key', 'w')
+      expect(File).to receive(:write).with('/tmp/foo.key', kind_of(String))
       resource.provider.create
     end
 
     context 'when setting size' do
       it 'creates with given size' do
         resource[:size] = 1024
         allow(OpenSSL::PKey::RSA).to receive(:new).with(1024).and_return(key)
-        allow(File).to receive(:open).with('/tmp/foo.key', 'w')
+        expect(File).to receive(:write).with('/tmp/foo.key', kind_of(String))
         resource.provider.create
       end
     end
@@ -43,7 +43,7 @@
         resource[:password] = '2x$5{'
         allow(OpenSSL::PKey::RSA).to receive(:new).with(2048).and_return(key)
         allow(OpenSSL::Cipher).to receive(:new).with('des3')
-        allow(File).to receive(:open).with('/tmp/foo.key', 'w')
+        expect(File).to receive(:write).with('/tmp/foo.key', kind_of(String))
         resource.provider.create
       end
     end
@@ -53,7 +53,7 @@
     it 'creates a dsa key' do
       resource[:authentication] = :rsa
       allow(OpenSSL::PKey::RSA).to receive(:new).with(2048).and_return(key)
-      allow(File).to receive(:open).with('/tmp/foo.key', 'w')
+      expect(File).to receive(:write).with('/tmp/foo.key', kind_of(String))
       resource.provider.create
     end
 
@@ -62,7 +62,7 @@
         resource[:authentication] = :rsa
         resource[:size] = 1024
         allow(OpenSSL::PKey::RSA).to receive(:new).with(1024).and_return(key)
-        allow(File).to receive(:open).with('/tmp/foo.key', 'w')
+        expect(File).to receive(:write).with('/tmp/foo.key', kind_of(String))
         resource.provider.create
       end
     end
@@ -73,7 +73,7 @@
         resource[:password] = '2x$5{'
         allow(OpenSSL::PKey::RSA).to receive(:new).with(2048).and_return(key)
         allow(OpenSSL::Cipher).to receive(:new).with('des3')
-        allow(File).to receive(:open).with('/tmp/foo.key', 'w')
+        expect(File).to receive(:write).with('/tmp/foo.key', kind_of(String))
         resource.provider.create
       end
     end
@@ -83,7 +83,7 @@
     it 'creates a dsa key' do
       resource[:authentication] = :dsa
       allow(OpenSSL::PKey::DSA).to receive(:new).with(2048).and_return(key)
-      allow(File).to receive(:open).with('/tmp/foo.key', 'w')
+      expect(File).to receive(:write).with('/tmp/foo.key', kind_of(String))
       resource.provider.create
     end
 
@@ -92,7 +92,7 @@
         resource[:authentication] = :dsa
         resource[:size] = 1024
         allow(OpenSSL::PKey::DSA).to receive(:new).with(1024).and_return(key)
-        allow(File).to receive(:open).with('/tmp/foo.key', 'w')
+        expect(File).to receive(:write).with('/tmp/foo.key', kind_of(String))
         resource.provider.create
       end
     end
@@ -103,7 +103,7 @@
         resource[:password] = '2x$5{'
         allow(OpenSSL::PKey::DSA).to receive(:new).with(2048).and_return(key)
         allow(OpenSSL::Cipher).to receive(:new).with('des3')
-        allow(File).to receive(:open).with('/tmp/foo.key', 'w')
+        expect(File).to receive(:write).with('/tmp/foo.key', kind_of(String))
         resource.provider.create
       end
     end
@@ -115,7 +115,7 @@
     it 'creates an ec key' do
       resource[:authentication] = :ec
       allow(OpenSSL::PKey::EC).to receive(:new).with('secp384r1').and_return(key)
-      allow(File).to receive(:open).with('/tmp/foo.key', 'w')
+      expect(File).to receive(:write).with('/tmp/foo.key', kind_of(String))
       resource.provider.create
     end
 
@@ -124,7 +124,7 @@
         resource[:authentication] = :ec
         resource[:curve] = 'prime239v1'
         allow(OpenSSL::PKey::EC).to receive(:new).with('prime239v1').and_return(key)
-        allow(File).to receive(:open).with('/tmp/foo.key', 'w')
+        expect(File).to receive(:write).with('/tmp/foo.key', kind_of(String))
         resource.provider.create
       end
     end
@@ -135,7 +135,7 @@
         resource[:password] = '2x$5{'
         allow(OpenSSL::PKey::EC).to receive(:new).with('secp384r1').and_return(key)
         allow(OpenSSL::Cipher).to receive(:new).with('des3')
-        allow(File).to receive(:open).with('/tmp/foo.key', 'w')
+        expect(File).to receive(:write).with('/tmp/foo.key', kind_of(String))
         resource.provider.create
       end
     end

spec/unit/puppet/provider/x509_cert/openssl_spec.rb

@@ -8,20 +8,25 @@
 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) }
   let(:resource) { Puppet::Type::X509_cert.new(path: path) }
   let(:cert) { OpenSSL::X509::Certificate.new }
 
   context 'when not forcing key' do
     it 'exists? should return true if certificate exists and is synced' do
       allow(File).to receive(:read)
-      allow_any_instance_of(Pathname).to receive(:exist?).and_return(true) # rubocop:disable RSpec/AnyInstance
+      allow(Pathname).to receive(:new).and_call_original
+      allow(Pathname).to receive(:new).with(path).and_return(pathname)
+      expect(pathname).to receive(:exist?).and_return(true)
       c = OpenSSL::X509::Certificate.new # Fake certificate for mocking
       allow(OpenSSL::X509::Certificate).to receive(:new).and_return(c)
       expect(resource.provider.exists?).to be(true)
     end
 
     it 'exists? should return false if certificate does not exist' do
-      allow_any_instance_of(Pathname).to receive(:exist?).and_return(false) # rubocop:disable RSpec/AnyInstance
+      allow(Pathname).to receive(:new).and_call_original
+      allow(Pathname).to receive(:new).with(path).and_return(pathname)
+      expect(pathname).to receive(:exist?).and_return(false)
       expect(resource.provider.exists?).to be(false)
     end
 
@@ -105,33 +110,40 @@
   context 'when forcing key' do
     it 'exists? should return true if certificate exists and is synced' do
       resource[:force] = true
-      allow(File).to receive(:read)
-      allow_any_instance_of(Pathname).to receive(:exist?).and_return(true) # rubocop:disable RSpec/AnyInstance
-      allow(OpenSSL::X509::Certificate).to receive(:new).and_return(cert)
-      allow(OpenSSL::PKey::RSA).to receive(:new)
+      expect(File).to receive(:read).with('/tmp/foo.crt').twice.and_return('cert')
+      expect(File).to receive(:read).with('/tmp/foo.key').and_return('pkey')
+      expect(Pathname).to receive(:new).with(path).and_return(pathname)
+      expect(pathname).to receive(:exist?).and_return(true)
+      expect(OpenSSL::X509::Certificate).to receive(:new).with('cert').twice.and_return(cert)
+      expect(OpenSSL::PKey).to receive(:read).with('pkey', nil)
       expect(cert).to receive(:check_private_key).and_return(true)
       expect(resource.provider.exists?).to be(true)
     end
 
     it 'exists? should return false if certificate exists and is not synced' do
       resource[:force] = true
-      allow(File).to receive(:read)
-      allow_any_instance_of(Pathname).to receive(:exist?).and_return(true) # rubocop:disable RSpec/AnyInstance
-      allow(OpenSSL::X509::Certificate).to receive(:new).and_return(cert)
-      allow(OpenSSL::PKey::RSA).to receive(:new)
+      expect(File).to receive(:read).with('/tmp/foo.crt').and_return('cert')
+      expect(File).to receive(:read).with('/tmp/foo.key').and_return('pkey')
+      expect(Pathname).to receive(:new).with(path).and_return(pathname)
+      expect(pathname).to receive(:exist?).and_return(true)
+      expect(OpenSSL::X509::Certificate).to receive(:new).with('cert').and_return(cert)
+      expect(OpenSSL::PKey).to receive(:read).with('pkey', nil)
       expect(cert).to receive(:check_private_key).and_return(false)
       expect(resource.provider.exists?).to be(false)
     end
 
     it 'exists? should return false if certificate does not exist' do
       resource[:force] = true
-      allow_any_instance_of(Pathname).to receive(:exist?).and_return(false) # rubocop:disable RSpec/AnyInstance
+      expect(Pathname).to receive(:new).with(path).and_return(pathname)
+      expect(pathname).to receive(:exist?).and_return(false)
       expect(resource.provider.exists?).to be(false)
     end
   end
 
   it 'deletes files' do
-    allow_any_instance_of(Pathname).to receive(:delete) # rubocop:disable RSpec/AnyInstance
+    allow(Pathname).to receive(:new).and_call_original
+    allow(Pathname).to receive(:new).with(path).and_return(pathname)
+    expect(pathname).to receive(:delete)
     resource.provider.destroy
   end
 end

spec/unit/puppet/provider/x509_request/openssl_spec.rb

@@ -7,17 +7,22 @@
 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) }
   let(:resource) { Puppet::Type::X509_request.new(path: path) }
   let(:cert) { OpenSSL::X509::Request.new }
 
   context 'when not forcing key' do
     it 'exists? should return true if csr exists' do
-      allow_any_instance_of(Pathname).to receive(:exist?).and_return(true) # rubocop:disable RSpec/AnyInstance
+      allow(Pathname).to receive(:new).and_call_original
+      allow(Pathname).to receive(:new).with(path).and_return(pathname)
+      expect(pathname).to receive(:exist?).and_return(true)
       expect(resource.provider.exists?).to be(true)
     end
 
     it 'exists? should return false if csr exists' do
-      allow_any_instance_of(Pathname).to receive(:exist?).and_return(false) # rubocop:disable RSpec/AnyInstance
+      allow(Pathname).to receive(:new).and_call_original
+      allow(Pathname).to receive(:new).with(path).and_return(pathname)
+      expect(pathname).to receive(:exist?).and_return(false)
       expect(resource.provider.exists?).to be(false)
     end
 
@@ -50,32 +55,37 @@
     it 'exists? should return true if certificate exists and is synced' do
       resource[:force] = true
       allow(File).to receive(:read)
-      allow_any_instance_of(Pathname).to receive(:exist?).and_return(true) # rubocop:disable RSpec/AnyInstance
+      expect(Pathname).to receive(:new).with(path).and_return(pathname)
+      expect(pathname).to receive(:exist?).and_return(true)
       allow(OpenSSL::X509::Request).to receive(:new).and_return(cert)
-      allow(OpenSSL::PKey::RSA).to receive(:new)
+      expect(OpenSSL::PKey).to receive(:read)
       expect(cert).to receive(:verify).and_return(true)
       expect(resource.provider.exists?).to be(true)
     end
 
     it 'exists? should return false if certificate exists and is not synced' do
       resource[:force] = true
       allow(File).to receive(:read)
-      allow_any_instance_of(Pathname).to receive(:exist?).and_return(true) # rubocop:disable RSpec/AnyInstance
+      expect(Pathname).to receive(:new).with(path).and_return(pathname)
+      expect(pathname).to receive(:exist?).and_return(true)
       allow(OpenSSL::X509::Request).to receive(:new).and_return(cert)
-      allow(OpenSSL::PKey::RSA).to receive(:new)
+      expect(OpenSSL::PKey).to receive(:read)
       expect(cert).to receive(:verify).and_return(false)
       expect(resource.provider.exists?).to be(false)
     end
 
     it 'exists? should return false if certificate does not exist' do
       resource[:force] = true
-      allow_any_instance_of(Pathname).to receive(:exist?).and_return(false) # rubocop:disable RSpec/AnyInstance
+      expect(Pathname).to receive(:new).with(path).and_return(pathname)
+      expect(pathname).to receive(:exist?).and_return(false)
       expect(resource.provider.exists?).to be(false)
     end
   end
 
   it 'deletes files' do
-    allow_any_instance_of(Pathname).to receive(:delete) # rubocop:disable RSpec/AnyInstance
+    allow(Pathname).to receive(:new).and_call_original
+    allow(Pathname).to receive(:new).with(path).and_return(pathname)
+    expect(pathname).to receive(:delete)
     resource.provider.destroy
   end
 end