Add support for certificate_content and private_key_content parameters

hajee · hajee · commit bdee3641d359 · 2022-03-15T15:52:54.000+01:00
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.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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'
