From 182becf2614f5c004bac66b768f3c7b2bd94c595 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 12 Sep 2024 15:45:56 +0200 Subject: [PATCH 1/2] Add a test for custom server certificates in tar file This asserts that the default CA and server CA are the same in one scenario and differ in the other. --- spec/acceptance/foreman_proxy_content_spec.rb | 81 ++++++++++++++----- 1 file changed, 61 insertions(+), 20 deletions(-) diff --git a/spec/acceptance/foreman_proxy_content_spec.rb b/spec/acceptance/foreman_proxy_content_spec.rb index 037f7afc..6ddfb192 100644 --- a/spec/acceptance/foreman_proxy_content_spec.rb +++ b/spec/acceptance/foreman_proxy_content_spec.rb @@ -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, @@ -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 From 975bdb9611f58a5dbbf2ac8d3d764329507581f6 Mon Sep 17 00:00:00 2001 From: "Eric D. Helms" Date: Thu, 12 Sep 2024 21:45:27 -0400 Subject: [PATCH 2/2] Fixes #37817: Only copy server CA in build root if generate is true Fixes: 433dadc5ec41 ("Copy the server CA certificate with file resource") --- manifests/ca.pp | 34 +++++------ spec/acceptance/certs_spec.rb | 111 ++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 17 deletions(-) diff --git a/manifests/ca.pp b/manifests/ca.pp index 05dc00cd..29a46d2f 100644 --- a/manifests/ca.pp +++ b/manifests/ca.pp @@ -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, diff --git a/spec/acceptance/certs_spec.rb b/spec/acceptance/certs_spec.rb index 9d48bf30..a2e1533f 100644 --- a/spec/acceptance/certs_spec.rb +++ b/spec/acceptance/certs_spec.rb @@ -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, + 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