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

Add support for certificate_content and private_key_content parameters #385

Merged
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
15 changes: 15 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,21 @@ java_ks { 'broker.example.com:/etc/activemq/broker.ks':
}
```

For use cases where you want 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
Expand Down
29 changes: 27 additions & 2 deletions lib/puppet/provider/java_ks/keytool.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or... anytime after this method exits leading to a race condition. See #425

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
Expand Down
32 changes: 28 additions & 4 deletions lib/puppet/type/java_ks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
57 changes: 57 additions & 0 deletions spec/acceptance/content_spec.rb
Original file line number Diff line number Diff line change
@@ -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
71 changes: 48 additions & 23 deletions spec/unit/puppet/provider/java_ks/keytool_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
24 changes: 24 additions & 0 deletions spec/unit/puppet/type/java_ks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down