From c15372337db5c6e65ab48db95a500c25c81c2991 Mon Sep 17 00:00:00 2001 From: Bert Hajee Date: Tue, 15 Mar 2022 11:19:44 +0100 Subject: [PATCH] Add support for certificate_content and private_key_content parameters The current implementation only allows you to pass a file name to the certificate and private_key parameters. When you are fetching certificates from vault or another secure store, you'll first have to save them to a file. This is very innconveniant. This PR add's the parameters certificate_content and private_key_content. These parameters are mutually exclusive from their file counterparts. With this change, you can now fetch a certificate and/or a password from vault (through a hiera lookup for example) and use it directly on the type. Because these values can be sensitive, both of the new parameters support passing the value as a sensitive data type. --- README.md | 37 +++++++--- lib/puppet/provider/java_ks/keytool.rb | 29 +++++++- lib/puppet/type/java_ks.rb | 32 +++++++-- spec/acceptance/content_spec.rb | 57 +++++++++++++++ .../puppet/provider/java_ks/keytool_spec.rb | 71 +++++++++++++------ spec/unit/puppet/type/java_ks_spec.rb | 24 +++++++ 6 files changed, 210 insertions(+), 40 deletions(-) create mode 100644 spec/acceptance/content_spec.rb diff --git a/README.md b/README.md index c45db274..16555fa1 100644 --- a/README.md +++ b/README.md @@ -4,17 +4,18 @@ #### Table of Contents -1. [Overview](#overview) -2. [Module Description](#module-description) - * [Beginning with the module](#beginning-with-the-module) -3. [Setup](#setup) -4. [Usage](#usage) - * [Certificates](#certificates) - * [Namevars](#namevars) - * [Windows task](#windows-task) -5. [Reference](#reference) -6. [Limitations](#limitations) -7. [Development](#development) +- [java_ks](#java_ks) + - [Table of Contents](#table-of-contents) + - [Overview](#overview) + - [Module Description](#module-description) + - [Setup](#setup) + - [Beginning with the module](#beginning-with-the-module) + - [Usage](#usage) + - [Certificates](#certificates) + - [Namevars](#namevars) + - [Reference](#reference) + - [Limitations](#limitations) + - [Development](#development) ## Overview @@ -63,6 +64,20 @@ java_ks { 'broker.example.com:/etc/activemq/broker.ks': } ``` +For use cases where you hire to fetch the certificate data from a secure store, like vault, you can use the `_content` attributes. Here is an example: + +```puppet +java_ks { 'broker.example.com:/etc/activemq/broker.ks': + ensure => latest, + certificate_content => $certificate_data_fetched_from_secure_store, + private_key_content => $private_key_data_fetched_from_secure_store + password => 'albatros', + password_fail_reset => true, +} +``` + +We recommend using the data type `Senstive` for the attributes `certificate_content` and `private_key_content`. But These attributes also support a regular `String` data type. The `_content` attributes are mutual exclusive with their file-based variants. + You can also use Hiera by passing params to the java_ks::config class: ```yaml diff --git a/lib/puppet/provider/java_ks/keytool.rb b/lib/puppet/provider/java_ks/keytool.rb index 3773af32..4a93e4c2 100644 --- a/lib/puppet/provider/java_ks/keytool.rb +++ b/lib/puppet/provider/java_ks/keytool.rb @@ -280,11 +280,36 @@ def update end def certificate - @resource[:certificate] + return @resource[:certificate] if @resource[:certificate] + + # When no certificate file is specified, we infer the usage of + # certificate content and create a tempfile containing this value. + # we leave it to to the tempfile to clean it up after the pupet run exists. + file = Tempfile.new('certificate') + # Check if the specified value is a Sensitive data type. If so, unwrap it and use + # the value. + content = @resource[:certificate_content].respond_to?(:unwrap) ? @resource[:certificate_content].unwrap : @resource[:certificate_content] + file.write(content) + file.close + file.path end def private_key - @resource[:private_key] + return @resource[:private_key] if @resource[:private_key] + if @resource[:private_key_content] + + + # When no private key file is specified, we infer the usage of + # private key content and create a tempfile containing this value. + # we leave it to to the tempfile to clean it up after the pupet run exists. + file = Tempfile.new('private_key') + # Check if the specified value is a Sensitive data type. If so, unwrap it and use + # the value. + content = @resource[:private_key_content].respond_to?(:unwrap) ? @resource[:private_key_content].unwrap : @resource[:private_key_content] + file.write(content) + file.close + file.path + end end def private_key_type diff --git a/lib/puppet/type/java_ks.rb b/lib/puppet/type/java_ks.rb index 7935eaf7..fd156eec 100644 --- a/lib/puppet/type/java_ks.rb +++ b/lib/puppet/type/java_ks.rb @@ -66,10 +66,13 @@ def insync?(is) end newparam(:certificate) do - desc 'A server certificate, followed by zero or more intermediate certificate authorities. - All certificates will be placed in the keystore. This will autorequire the specified file.' + desc 'A file containing a server certificate, followed by zero or more intermediate certificate authorities. + All certificates will be placed in the keystore. This will autorequire the specified file.' + end - isrequired + newparam(:certificate_content) do + desc 'A string containing a server certificate, followed by zero or more intermediate certificate authorities. + All certificates will be placed in the keystore.' end newparam(:storetype) do @@ -82,7 +85,16 @@ def insync?(is) newparam(:private_key) do desc 'If you want an application to be a server and encrypt traffic, you will need a private key. Private key entries in a keystore must be - accompanied by a signed certificate for the keytool provider. This will autorequire the specified file.' + accompanied by a signed certificate for the keytool provider. This parameter + allows you to specify the file name containing the private key. This will autorequire + the specified file.' + end + + newparam(:private_key_content) do + desc 'If you want an application to be a server and encrypt traffic, + you will need a private key. Private key entries in a keystore must be + accompanied by a signed certificate for the keytool provider. This parameter allows you to specify the content + of the private key.' end newparam(:private_key_type) do @@ -228,6 +240,18 @@ def self.title_patterns end validate do + unless value(:certificate) || value(:certificate_content) + raise Puppet::Error, "You must pass one of 'certificate' or 'certificate_content'" + end + + if value(:certificate) && value(:certificate_content) + raise Puppet::Error, "You must pass either 'certificate' or 'certificate_content', not both." + end + + if value(:private_key) && value(:private_key_content) + raise Puppet::Error, "You must pass either 'private_key' or 'private_key_content', not both." + end + if value(:password) && value(:password_file) raise Puppet::Error, "You must pass either 'password' or 'password_file', not both." end diff --git a/spec/acceptance/content_spec.rb b/spec/acceptance/content_spec.rb new file mode 100644 index 00000000..ce4a2eb7 --- /dev/null +++ b/spec/acceptance/content_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper_acceptance' + +RSpec.shared_examples 'a private key creator' do |sensitive| + it 'creates a private key' do + pp = if sensitive + <<-MANIFEST + java_ks { 'broker.example.com:#{temp_dir}private_key.ts': + ensure => #{@ensure_ks}, + certificate_content => "#{ca_content}", + private_key_content => "#{priv_key_content}", + password => 'puppet', + path => #{@resource_path}, + } + MANIFEST + else + <<-MANIFEST + java_ks { 'broker.example.com:#{temp_dir}private_key.ts': + ensure => #{@ensure_ks}, + certificate_content => Sensitive("#{ca_content}"), + private_key_content => Sensitive("#{priv_key_content}"), + password => 'puppet', + path => #{@resource_path}, + } + MANIFEST + end + idempotent_apply(pp) + end + + expectations = [ + %r{Alias name: broker\.example\.com}, + %r{Entry type: (keyEntry|PrivateKeyEntry)}, + %r{CN=Test CA}, + ] + it 'verifies the private key' do + run_shell(keytool_command("-list -v -keystore #{temp_dir}private_key.ts -storepass puppet"), expect_failures: true) do |r| + expectations.each do |expect| + expect(r.stdout).to match(expect) + end + end + end +end + +describe 'using certificate_content and private_key_content' do + include_context 'common variables' + let(:ca_content) { File.read('spec/acceptance/certs/ca.pem') } + let(:priv_key_content) { File.read('spec/acceptance/certs/privkey.pem') } + + context 'Using data type String' do + it_behaves_like 'a private key creator', false + end + + context 'Using data type Sensitive' do + it_behaves_like 'a private key creator', true + end +end diff --git a/spec/unit/puppet/provider/java_ks/keytool_spec.rb b/spec/unit/puppet/provider/java_ks/keytool_spec.rb index a466095e..78c86cfd 100755 --- a/spec/unit/puppet/provider/java_ks/keytool_spec.rb +++ b/spec/unit/puppet/provider/java_ks/keytool_spec.rb @@ -46,6 +46,7 @@ write: true, flush: true, close!: true, + close: true, path: "#{temp_dir}testing.stuff") allow(Tempfile).to receive(:new).and_return(tempfile) end @@ -97,29 +98,53 @@ describe 'when importing a private key and certifcate' do describe '#to_pkcs12' do - it 'converts a certificate to a pkcs12 file' do - sleep 0.1 # due to https://github.com/mitchellh/vagrant/issues/5056 - testing_key = OpenSSL::PKey::RSA.new 1024 - testing_ca = OpenSSL::X509::Certificate.new - testing_ca.serial = 1 - testing_ca.public_key = testing_key.public_key - testing_subj = '/CN=Test CA/ST=Denial/L=Springfield/O=Dis/CN=www.example.com' - testing_ca.subject = OpenSSL::X509::Name.parse testing_subj - testing_ca.issuer = testing_ca.subject - testing_ca.not_before = Time.now - testing_ca.not_after = testing_ca.not_before + 360 - testing_ca.sign(testing_key, OpenSSL::Digest::SHA256.new) - - allow(provider).to receive(:password).and_return(resource[:password]) - allow(File).to receive(:read).with(resource[:private_key]).and_return('private key') - allow(File).to receive(:read).with(resource[:certificate], hash_including(encoding: 'ISO-8859-1')).and_return(testing_ca.to_pem) - expect(OpenSSL::PKey::RSA).to receive(:new).with('private key', 'puppet').and_return('priv_obj') - expect(OpenSSL::X509::Certificate).to receive(:new).with(testing_ca.to_pem.chomp).and_return('cert_obj') - - pkcs_double = BogusPkcs.new - expect(pkcs_double).to receive(:to_der) - expect(OpenSSL::PKCS12).to receive(:create).with(resource[:password], resource[:name], 'priv_obj', 'cert_obj', []).and_return(pkcs_double) - provider.to_pkcs12("#{temp_dir}testing.stuff") + sleep 0.1 # due to https://github.com/mitchellh/vagrant/issues/5056 + testing_key = OpenSSL::PKey::RSA.new 1024 + testing_ca = OpenSSL::X509::Certificate.new + testing_ca.serial = 1 + testing_ca.public_key = testing_key.public_key + testing_subj = '/CN=Test CA/ST=Denial/L=Springfield/O=Dis/CN=www.example.com' + testing_ca.subject = OpenSSL::X509::Name.parse testing_subj + testing_ca.issuer = testing_ca.subject + testing_ca.not_before = Time.now + testing_ca.not_after = testing_ca.not_before + 360 + testing_ca.sign(testing_key, OpenSSL::Digest::SHA256.new) + + context "Using the file based parameters for certificate and private_key" do + it 'converts a certificate to a pkcs12 file' do + allow(provider).to receive(:password).and_return(resource[:password]) + allow(File).to receive(:read).with(resource[:private_key]).and_return('private key') + allow(File).to receive(:read).with(resource[:certificate], hash_including(encoding: 'ISO-8859-1')).and_return(testing_ca.to_pem) + expect(OpenSSL::PKey::RSA).to receive(:new).with('private key', 'puppet').and_return('priv_obj') + expect(OpenSSL::X509::Certificate).to receive(:new).with(testing_ca.to_pem.chomp).and_return('cert_obj') + + pkcs_double = BogusPkcs.new + expect(pkcs_double).to receive(:to_der) + expect(OpenSSL::PKCS12).to receive(:create).with(resource[:password], resource[:name], 'priv_obj', 'cert_obj', []).and_return(pkcs_double) + provider.to_pkcs12("#{temp_dir}testing.stuff") + end + end + + context "Using content based parameters for certificate and private_key" do + let(:params) { + global_params.tap {|h| [:certificate, :private_key].each {|k| h.delete(k)}}.merge( + :private_key_content => 'private_key', + :certificate_content => testing_ca.to_pem, + ) + } + + it 'converts a certificate to a pkcs12 file' do + allow(provider).to receive(:password).and_return(resource[:password]) + allow(File).to receive(:read).with('/tmp/testing.stuff').ordered.and_return('private key') + allow(File).to receive(:read).with('/tmp/testing.stuff', hash_including(encoding: 'ISO-8859-1')).ordered.and_return(testing_ca.to_pem) + expect(OpenSSL::PKey::RSA).to receive(:new).with('private key', 'puppet').and_return('priv_obj') + expect(OpenSSL::X509::Certificate).to receive(:new).with(testing_ca.to_pem.chomp).and_return('cert_obj') + + pkcs_double = BogusPkcs.new + expect(pkcs_double).to receive(:to_der) + expect(OpenSSL::PKCS12).to receive(:create).with(resource[:password], resource[:name], 'priv_obj', 'cert_obj', []).and_return(pkcs_double) + provider.to_pkcs12("#{temp_dir}testing.stuff") + end end end diff --git a/spec/unit/puppet/type/java_ks_spec.rb b/spec/unit/puppet/type/java_ks_spec.rb index da52dba2..c91bc5c0 100644 --- a/spec/unit/puppet/type/java_ks_spec.rb +++ b/spec/unit/puppet/type/java_ks_spec.rb @@ -117,6 +117,30 @@ }.to raise_error(Puppet::Error) end + it 'fails if both :certificate and :certificate_content are provided' do + jks = jks_resource.dup + jks[:certificate_content] = 'certificate_content' + expect { + described_class.new(jks) + }.to raise_error(Puppet::Error, %r{You must pass either}) + end + + it 'fails if neither :certificate or :certificate_content is provided' do + jks = jks_resource.dup + jks.delete(:certificate) + expect { + described_class.new(jks) + }.to raise_error(Puppet::Error, %r{You must pass one of}) + end + + it 'fails if both :private_key and :private_key_content are provided' do + jks = jks_resource.dup + jks[:private_key_content] = 'private_content' + expect { + described_class.new(jks) + }.to raise_error(Puppet::Error, %r{You must pass either}) + end + it 'fails if both :password and :password_file are provided' do jks = jks_resource.dup jks[:password_file] = '/path/to/password_file'