Skip to content

Fixes #37817: Only copy server CA in build root if generate is true #463

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 13, 2024
Merged

Fixes #37817: Only copy server CA in build root if generate is true #463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
182becf
Add a test for custom server certificates in tar file
ekohl Sep 12, 2024
975bdb9
Fixes #37817: Only copy server CA in build root if generate is true
ehelms Sep 13, 2024
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
34 changes: 17 additions & 17 deletions manifests/ca.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,25 +45,25 @@
build_dir => $certs::ssl_build_dir,
}

if $certs::server_ca_cert {
file { $server_ca_path:
ensure => file,
source => $certs::server_ca_cert,
owner => 'root',
group => 'root',
mode => '0644',
}
} else {
file { $server_ca_path:
ensure => file,
source => "${certs::ssl_build_dir}/${default_ca_name}.crt",
owner => 'root',
group => 'root',
mode => '0644',
if $generate {
if $certs::server_ca_cert {
file { $server_ca_path:
ensure => file,
source => $certs::server_ca_cert,
owner => 'root',
group => 'root',
mode => '0644',
}
} else {
file { $server_ca_path:
ensure => file,
source => "${certs::ssl_build_dir}/${default_ca_name}.crt",
owner => 'root',
group => 'root',
mode => '0644',
}
}
}

if $generate {
file { "${certs::ssl_build_dir}/KATELLO-TRUSTED-SSL-CERT":
ensure => link,
target => $server_ca_path,
Expand Down
111 changes: 111 additions & 0 deletions spec/acceptance/certs_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,4 +151,115 @@ class { 'certs':
its(:keylength) { should be >= 2048 }
end
end

context 'with tar file' do
before(:context) do
['crt', 'key'].each do |ext|
source_path = "fixtures/example.partial.solutions.#{ext}"
dest_path = "/server.#{ext}"
scp_to(hosts, source_path, dest_path)
end
end

context 'with default ca' do
before(:context) do
manifest = <<~PUPPET
class { 'certs':
generate => true,
deploy => false,
}

class { 'certs::foreman_proxy_content':
foreman_proxy_fqdn => 'foreman-proxy.example.com',
certs_tar => '/root/foreman-proxy.example.com.tar.gz',
}
PUPPET

apply_manifest(manifest, catch_failures: true)

on default, 'rm -rf /root/ssl-build'
end

describe 'deploy certificates' do
manifest = <<-PUPPET
class { 'certs':
tar_file => '/root/foreman-proxy.example.com.tar.gz',
}
PUPPET
# tar extraction is not idempotent
it { apply_manifest(manifest, catch_failures: true) }
end

describe 'default and server ca certs match' do
it { expect(file('/etc/pki/katello/certs/katello-default-ca.crt').content).to eq(file('/etc/pki/katello/certs/katello-server-ca.crt').content) }
end

describe x509_certificate('/etc/pki/katello/certs/katello-default-ca.crt') do
it { should be_certificate }
it { should be_valid }
it { should have_purpose 'SSL server CA' }
its(:issuer) { should match_without_whitespace(/C = US, ST = North Carolina, L = Raleigh, O = Katello, OU = SomeOrgUnit, CN = #{fact('fqdn')}/) }
its(:subject) { should match_without_whitespace(/C = US, ST = North Carolina, L = Raleigh, O = Katello, OU = SomeOrgUnit, CN = #{fact('fqdn')}/) }
its(:keylength) { should be >= 4096 }
end
end

context 'with custom certificates' do
before(:context) do
manifest = <<~PUPPET
class { 'certs':
server_cert => '/server.crt',
server_key => '/server.key',
server_ca_cert => '/server-ca.crt',
generate => true,
deploy => false,
}

class { 'certs::foreman_proxy_content':
foreman_proxy_fqdn => 'foreman-proxy.example.com',
certs_tar => '/root/foreman-proxy.example.com.tar.gz',
}
PUPPET

apply_manifest(manifest, catch_failures: true)

on default, 'rm -rf /root/ssl-build'
end

describe 'deploy certificates' do
manifest = <<-PUPPET
class { 'certs':
generate => false,
Copy link
Member

@ekohl ekohl Sep 13, 2024

Choose a reason for hiding this comment

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

Note to self: this bit I missed in my PR.

tar_file => '/root/foreman-proxy.example.com.tar.gz',
}
PUPPET
# tar extraction is not idempotent
it { apply_manifest(manifest, catch_failures: true) }
end

describe 'default and server ca certs match' do
it { expect(file('/etc/pki/katello/certs/katello-default-ca.crt').content).not_to eq(file('/etc/pki/katello/certs/katello-server-ca.crt').content) }
end

describe x509_certificate('/etc/pki/katello/certs/katello-default-ca.crt') do
it { should be_certificate }
it { should be_valid }
it { should have_purpose 'SSL server CA' }
its(:issuer) { should match_without_whitespace(/C = US, ST = North Carolina, L = Raleigh, O = Katello, OU = SomeOrgUnit, CN = #{fact('fqdn')}/) }
its(:subject) { should match_without_whitespace(/C = US, ST = North Carolina, L = Raleigh, O = Katello, OU = SomeOrgUnit, CN = #{fact('fqdn')}/) }
its(:keylength) { should be >= 4096 }
end

describe x509_certificate('/etc/pki/katello/certs/katello-server-ca.crt') do
it { should be_certificate }
it { should be_valid }
it { should have_purpose 'SSL server CA' }
# These don't match since we only configure it with the intermediate
# and not the actual root
its(:issuer) { should match_without_whitespace(/CN = Fake LE Root X1/) }
its(:subject) { should match_without_whitespace(/CN = Fake LE Intermediate X1/) }
its(:keylength) { should be >= 2048 }
end
end
end
end
81 changes: 61 additions & 20 deletions spec/acceptance/foreman_proxy_content_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,26 @@
on default, 'rm -rf /root/ssl-build'
end

context 'with default parameters' do
before(:context) do
apply_manifest('include certs', catch_failures: true)
let(:expected_files_in_tar) do
[
'ssl-build/katello-default-ca.crt',
'ssl-build/katello-server-ca.crt',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-apache.crt',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-foreman-client.crt',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-foreman-proxy-client.crt',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-foreman-proxy.crt',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-puppet-client.crt',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-apache.key',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-foreman-client.key',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-foreman-proxy-client.key',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-foreman-proxy.key',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-puppet-client.key',
]
end

pp = <<-PUPPET
context 'with default CA' do
before(:context) do
manifest = <<~PUPPET
class { 'certs':
generate => true,
deploy => false,
Expand All @@ -21,29 +36,55 @@ class { 'certs::foreman_proxy_content':
}
PUPPET

apply_manifest(pp, catch_failures: true)
apply_manifest(manifest, catch_failures: true)
end

describe tar('/root/foreman-proxy.example.com.tar.gz') do
it { should exist }
its(:contents) { should match_array(expected_files_in_tar) }
end

describe 'default and server ca certs match' do
it { expect(file('/root/ssl-build/katello-default-ca.crt').content).to eq(file('/root/ssl-build/katello-server-ca.crt').content) }
end
end

context 'with server certificates' do
before(:context) do
certs = {
'fixtures/example.partial.solutions.crt' => '/server.crt',
'fixtures/example.partial.solutions.key' => '/server.key',
'fixtures/example.partial.solutions-chain.pem' => '/server-ca.crt',
}
certs.each do |source_path, dest_path|
scp_to(hosts, source_path, dest_path)
end

manifest = <<~PUPPET
class { 'certs':
server_cert => '/server.crt',
server_key => '/server.key',
server_ca_cert => '/server-ca.crt',
generate => true,
deploy => false,
}

class { 'certs::foreman_proxy_content':
foreman_proxy_fqdn => 'foreman-proxy.example.com',
certs_tar => '/root/foreman-proxy.example.com.tar.gz',
}
PUPPET

let(:expected_files_in_tar) do
[
'ssl-build/katello-default-ca.crt',
'ssl-build/katello-server-ca.crt',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-apache.crt',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-foreman-client.crt',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-foreman-proxy-client.crt',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-foreman-proxy.crt',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-puppet-client.crt',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-apache.key',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-foreman-client.key',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-foreman-proxy-client.key',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-foreman-proxy.key',
'ssl-build/foreman-proxy.example.com/foreman-proxy.example.com-puppet-client.key',
]
apply_manifest(manifest, catch_failures: true)
end

describe tar('/root/foreman-proxy.example.com.tar.gz') do
it { should exist }
its(:contents) { should match_array(expected_files_in_tar) }
end

describe 'default and server ca certs differ' do
it { expect(file('/root/ssl-build/katello-default-ca.crt').content).not_to eq(file('/root/ssl-build/katello-server-ca.crt').content) }
end
end
end