Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use OpenSSL::PKey.read to read private keys #190

Merged
merged 4 commits into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 0 additions & 18 deletions REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -1309,7 +1309,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)
* [`csr`](#-x509_cert--csr)
Expand All @@ -1322,14 +1321,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
Expand Down Expand Up @@ -1407,7 +1398,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)
Expand All @@ -1416,14 +1406,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`
Expand Down
12 changes: 1 addition & 11 deletions lib/puppet/provider/x509_cert/openssl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 1 addition & 11 deletions lib/puppet/provider/x509_request/openssl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 0 additions & 6 deletions lib/puppet/type/x509_cert.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 0 additions & 6 deletions lib/puppet/type/x509_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion spec/unit/puppet/provider/cert_file/posix_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
24 changes: 12 additions & 12 deletions spec/unit/puppet/provider/ssl_pkey/openssl_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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
Expand Down
36 changes: 24 additions & 12 deletions spec/unit/puppet/provider/x509_cert/openssl_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -82,33 +87,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
26 changes: 18 additions & 8 deletions spec/unit/puppet/provider/x509_request/openssl_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Loading