From 7f1e777d0a18ae9239d39023f31a73241a4a74de Mon Sep 17 00:00:00 2001 From: Kamil Szubrycht Date: Wed, 14 Oct 2020 13:33:40 +0200 Subject: [PATCH] Fixes #31049 - Introduce server CA file setting --- app/models/setting.rb | 2 +- app/models/setting/auth.rb | 1 + lib/foreman/renderer/scope/macros/base.rb | 32 +++++--- test/fixtures/settings.yml | 11 ++- .../certificates/example2.com.crt | 33 +++++++++ .../renderer/renderers_shared_tests.rb | 74 ++++++++++++++++--- 6 files changed, 129 insertions(+), 24 deletions(-) create mode 100644 test/static_fixtures/certificates/example2.com.crt diff --git a/app/models/setting.rb b/app/models/setting.rb index 672231afd261..de64d2aca8d6 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -15,7 +15,7 @@ class Setting < ApplicationRecord FROZEN_ATTRS = %w{name category} NONZERO_ATTRS = %w{puppet_interval idle_timeout entries_per_page outofsync_interval} BLANK_ATTRS = %w{ host_owner trusted_hosts login_delegation_logout_url root_pass default_location default_organization websockets_ssl_key websockets_ssl_cert oauth_consumer_key oauth_consumer_secret login_text oidc_audience oidc_issuer oidc_algorithm - smtp_address smtp_domain smtp_user_name smtp_password smtp_openssl_verify_mode smtp_authentication sendmail_arguments sendmail_location http_proxy http_proxy_except_list default_locale default_timezone ssl_certificate ssl_ca_file ssl_priv_key default_pxe_item_global default_pxe_item_local oidc_jwks_url instance_title } + smtp_address smtp_domain smtp_user_name smtp_password smtp_openssl_verify_mode smtp_authentication sendmail_arguments sendmail_location http_proxy http_proxy_except_list default_locale default_timezone ssl_certificate ssl_ca_file server_ca_file ssl_priv_key default_pxe_item_global default_pxe_item_local oidc_jwks_url instance_title } ARRAY_HOSTNAMES = %w{trusted_hosts} URI_ATTRS = %w{foreman_url unattended_url} URI_BLANK_ATTRS = %w{login_delegation_logout_url} diff --git a/app/models/setting/auth.rb b/app/models/setting/auth.rb index 6b495baeb38e..5cb6b2f3a949 100644 --- a/app/models/setting/auth.rb +++ b/app/models/setting/auth.rb @@ -15,6 +15,7 @@ def self.default_settings set('ssl_client_dn_env', N_('Environment variable containing the subject DN from a client SSL certificate'), 'SSL_CLIENT_S_DN', N_('SSL client DN env')), set('ssl_client_verify_env', N_('Environment variable containing the verification status of a client SSL certificate'), 'SSL_CLIENT_VERIFY', N_('SSL client verify env')), set('ssl_client_cert_env', N_("Environment variable containing a client's SSL certificate"), 'SSL_CLIENT_CERT', N_('SSL client cert env')), + set('server_ca_file', N_("SSL CA file that will be used in templates (to verify the connection to Foreman)"), nil, N_('Server CA file')), set('websockets_ssl_key', N_("Private key file that Foreman will use to encrypt websockets "), nil, N_('Websockets SSL key')), set('websockets_ssl_cert', N_("Certificate that Foreman will use to encrypt websockets "), nil, N_('Websockets SSL certificate')), # websockets_encrypt depends on key/cert when true, so initialize it last diff --git a/lib/foreman/renderer/scope/macros/base.rb b/lib/foreman/renderer/scope/macros/base.rb index 265a125894f4..42f0d007cb24 100644 --- a/lib/foreman/renderer/scope/macros/base.rb +++ b/lib/foreman/renderer/scope/macros/base.rb @@ -335,22 +335,36 @@ def gem_version_compare(first, second) end apipie :method, "Returns the TLS certificate(s) needed to verify a connection to Foreman" do - desc 'Currently it relies on "SSL CA file" authentication setting, which normally points to the file containing the + desc 'Currently it relies on "SSL CA file" and "Server CA file" authentication settings, which normally points to the file containing the CA certificate for Smart Proxies. However in the default deployment, this certificate happens to be the same.' example "SSL_CA_CERT=$(mktemp) cat > $SSL_CA_CERT < CA_CONTENT curl --cacert $SSL_CA_CERT https://foreman.example.com" - end - def foreman_server_ca_cert - raise UndefinedSetting.new(setting: 'SSL CA file') if Setting[:ssl_ca_file].blank? - begin - File.read(Setting[:ssl_ca_file]) - rescue => e - msg = N_("%s, check the 'SSL CA file' in Settings > Authentication") % e.message - raise Foreman::Exception.new(msg) end + + def foreman_server_ca_cert(server_ca_file_enabled: true, ssl_ca_file_enabled: true) + setting_values = [] + setting_values << Setting[:server_ca_file] if server_ca_file_enabled + setting_values << Setting[:ssl_ca_file] if ssl_ca_file_enabled + + raise UndefinedSetting.new(setting: '"Server CA file" or "SSL CA file"') if setting_values.compact.empty? + + files_content = setting_values.uniq.compact.map do |setting_value| + File.read(setting_value) + rescue StandardError => e + Foreman::Logging.logger('templates').warn("Failed to read CA file: #{e}") + + nil + end + + result = files_content.compact.join("\n") + + msg = N_("SSL CA file not found, check the 'Server CA file' and 'SSL CA file' in Settings > Authentication") + raise Foreman::Exception.new(msg) unless result.present? + + result end private diff --git a/test/fixtures/settings.yml b/test/fixtures/settings.yml index c3a58dd8f070..089963b343e7 100644 --- a/test/fixtures/settings.yml +++ b/test/fixtures/settings.yml @@ -22,17 +22,17 @@ attributes4: description: Enable safe mode config templates rendinging(recommended) attributes5: name: ssl_certificate - category: Setting::Provisioning + category: Setting::Auth default: /var/lib/puppet/ssl/certs/some.host.fqdn description: SSL Certificate path that foreman would use to communicate with its proxies attributes6: name: ssl_ca_file - category: Setting::Provisioning + category: Setting::Auth default: /var/lib/puppet/ssl/certs/ca.pem description: SSL CA file that foreman would use to communicate with its proxies attributes7: name: ssl_priv_key - category: Setting::Provisioning + category: Setting::Auth default: /var/lib/puppet/ssl/private_keys/super.some.host.fqdn.pem description: SSL Private Key file that foreman would use to communicate with its proxies attributes8: @@ -428,3 +428,8 @@ attribute95: category: Setting::Provisioning default: 'Linux host initial configuration' description: "Default host initial configuration template" +attribute96: + name: server_ca_file + category: Setting::Auth + default: /var/lib/puppet/ssl/certs/ca.pem + description: SSL CA file that will be used in templates (to verify the connection to Foreman) diff --git a/test/static_fixtures/certificates/example2.com.crt b/test/static_fixtures/certificates/example2.com.crt new file mode 100644 index 000000000000..1954e82f2395 --- /dev/null +++ b/test/static_fixtures/certificates/example2.com.crt @@ -0,0 +1,33 @@ +-----BEGIN CERTIFICATE----- +MIIFrTCCA5WgAwIBAgIJAOzdk0zTsuBPMA0GCSqGSIb3DQEBCwUAMF0xCzAJBgNV +BAYTAlhYMRAwDgYDVQQIEwdFeGFtcGxlMRAwDgYDVQQHEwdFeGFtcGxlMRQwEgYD +VQQKEwtFeGFtcGUgSW5jLjEUMBIGA1UEAxMLZXhhbXBsZS5jb20wHhcNMTcxMDA2 +MTMxMzMyWhcNMTgxMDA2MTMxMzMyWjBdMQswCQYDVQQGEwJYWDEQMA4GA1UECBMH +RXhhbXBsZTEQMA4GA1UEBxMHRXhhbXBsZTEUMBIGA1UEChMLRXhhbXBlIEluYy4x +FDASBgNVBAMTC2V4YW1wbGUuY29tMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIIC +CgKCAgEAslMkcsSL4hhnTLBJ/ZD3qldyD+7qTE9cB5dk9MH0Vj2GEezPnnUYZ0Cu +v0029+ZbwzOb7If8XzGxGU5tnqItZLfihrM3SKqshgJFaCjWEuLXfwRzHF2+jSk8 +mkG69Z0JtewIeQ7SqUReeZb4cV47Hag6gO5LVTnFXlHjGPrz98SH+Ho80XtIdmk0 +6fCzVXUZY+0EtXIul00LwnqsNc8UupuRpNC3VIG6ZCq5snwQ9Va1u+RP77nJlGQz +bKh5/yYaMd9WU3LDREkY0kSzJzcqzzQzEJr1JAY1zmtm5+NLdU2LPZjMAU6UiuJ4 +ABNdHwlvUWQ0SyPDqH2pnaytFlHgAH3G6brOj51Jpp86hOa/3iJ+M6gGtbNcYshd +LYTpr+kl34qri0677VQtpM3uVwM5om4rGrCGbhuse8DtMdOAv4jAvF79SrLGK5k+ +sCWxtPXGIJgt+AXS3HJev3lRGWjNm5yYL6fTwgjbWceh05hBGOro/fDFPyAOpCek +KPqOhu/MpJz7x+48rBny0GIl/CsMqojQ8spFH1Xp9dKoV886ZdzDKMlvDYfSHrj4 +9A9aIOT3+W5VCsPMgSIrr0MFhv3bkIEGleo3IioHeLxIylW4FLl0D7AdXQeiqe99 +Y+Jf+w0FNoVlmykpcQ7qXmQTzHiG7VB+o3wOWaP3K7Sj0jpIjoUCAwEAAaNwMG4w +CQYDVR0TBAIwADALBgNVHQ8EBAMCBeAwVAYDVR0RBE0wS4IPd3d3LmV4YW1wbGUu +Y29tgg93d3cuZXhhbXBsZS5uZXSCD3d3dy5leGFtcGxlLm9yZ4cEwKgBAYcQIAEN +uAAAAAAAAAAAAAAAATANBgkqhkiG9w0BAQsFAAOCAgEAiirO6rvirqxB5nfziEdd +Tn9BltiBDKg38MspsWSfSGJNHdmbZkBKhoJ89p9oewa8yOMqM3jrUjg2hn0J0nZG +t4dXix3Y7jHswaqr15mJEoLFGflkiOIOVJc1efxlSweXmyC+x6O5gAarmWHb+T6B +qxL/Z72N/Tua3foFQzlzR6lQ/++QglBHeBADtmZg6uPMPmWJ6iUA1OLpzW8cg4/0 +Q2UxKsD3WaN+37JbEJnNuuamCpCXoyHIpzxDccUKzmjAWOxUHuYuiOvSc55w4Ww3 +GXMmRgu6ElVxL938LgJfTnHTBZoN3TjAlWXTRMtLV+cUlUJhBA7wnDkZK9mfXvk8 +d7XCFq9WsRQ0B7+4raMt8fn7M20ckz5J9mnt3d1I8FQRluyTKOfzs7L++OP/AagD +ZCUrI5IrxroWqrf6f65t0HyqNDJy3ZKK2drsaf+emR3X3GVeeWVV0RzhUaIu6NJS +br7erBLX9J5s6ZrAf6LbbeKtF2x9AGF8SUYCCUNVAHQxf/fTBKQAtrB7BxOmZiY1 +fplNEWaMnDChPWlzdzmH+MsCAA5025+Sr7Eb/CS9wKZDV5z6FtqkQKPeZ/eHjtJV +SylVI2XfadJwxM4gj6Jcq1L8LxURS/NpTinoXDd3xfkZYy3WrMNAsP6Cz6GvcU/n +SBq3Hxpl4HptdDg+JyI1RIg= +-----END CERTIFICATE----- diff --git a/test/unit/foreman/renderer/renderers_shared_tests.rb b/test/unit/foreman/renderer/renderers_shared_tests.rb index cdec28a1c739..5cb3db5e9804 100644 --- a/test/unit/foreman/renderer/renderers_shared_tests.rb +++ b/test/unit/foreman/renderer/renderers_shared_tests.rb @@ -205,18 +205,70 @@ module RenderersSharedTests assert_equal(renderer.render(source, @scope), '-1') end - test "foreman_server_ca_cert - existing file" do - cert_path = Rails.root.join('test/static_fixtures/certificates/example.com.crt') - Setting[:ssl_ca_file] = cert_path - source = OpenStruct.new(content: '<%= foreman_server_ca_cert %>') - assert_equal(renderer.render(source, @scope), File.read(cert_path)) - end + describe '#foreman_server_ca_cert' do + subject { renderer.render(source, @scope) } - test "foreman_server_ca_cert - not existing file" do - Setting[:ssl_ca_file] = 'not-existing-file' - source = OpenStruct.new(content: '<%= foreman_server_ca_cert %>') - error = assert_raise Foreman::Exception do - renderer.render(source, @scope) + let(:source) { OpenStruct.new(content: '<%= foreman_server_ca_cert %>') } + let(:cert_path) { Rails.root.join('test/static_fixtures/certificates/example.com.crt') } + let(:cert_2_path) { Rails.root.join('test/static_fixtures/certificates/example2.com.crt') } + let(:cert_file_content) { File.read(cert_path) } + let(:cert_2_file_content) { File.read(cert_2_path) } + + test "load server_ca_file" do + Setting[:server_ca_file] = cert_path + Setting[:ssl_ca_file] = 'not-existing-file' + + assert_equal subject, cert_file_content + end + + test "load ssl_ca_file" do + Setting[:server_ca_file] = 'not-existing-file' + Setting[:ssl_ca_file] = cert_path + + assert_equal subject, cert_file_content + end + + test "load server_ca_file and ssl_ca_file" do + Setting[:server_ca_file] = cert_path + Setting[:ssl_ca_file] = cert_2_path + + expected = "#{cert_file_content}\n#{cert_2_file_content}" + assert_equal subject, expected + end + + test "do not load any files and raise exception" do + Setting[:server_ca_file] = 'not-existing-file' + Setting[:ssl_ca_file] = 'not-existing-file' + + assert_raise Foreman::Exception do + subject + end + end + + context 'when server_ca_file is disabled' do + let(:source) { OpenStruct.new(content: '<%= foreman_server_ca_cert(server_ca_file_enabled: false) %>') } + + test "do not load server_ca_file and raise exception" do + Setting[:server_ca_file] = cert_path + Setting[:ssl_ca_file] = 'not-existing-file' + + assert_raise Foreman::Exception do + subject + end + end + end + + context 'when ssl_ca_file is disabled' do + let(:source) { OpenStruct.new(content: '<%= foreman_server_ca_cert(ssl_ca_file_enabled: false) %>') } + + test "do not load ssl_ca_file and raise exception" do + Setting[:server_ca_file] = 'not-existing-file' + Setting[:ssl_ca_file] = cert_path + + assert_raise Foreman::Exception do + subject + end + end end assert_includes error.message, '[Foreman::Exception]: No such file or directory'