From b0b8102feb5814fb10f2fcd4f3617fbee91d90a8 Mon Sep 17 00:00:00 2001 From: William Bradford Clark Date: Tue, 12 Dec 2023 14:24:49 -0500 Subject: [PATCH] Fixes #36979 - Change cdn_ssl_version setting to cdn_min_tls_version This uses the min_version attribute from Net::HTTP instead of ssl_version, which in turn uses OpenSSL::SSL::SSLContext#min_version and not the older OpenSSL::SSL::SSLContent#ssl_version. As a result the there is no longer a blanket SSLv23 setting and the data format for specifying TLS versions has changed. It also sets a default value of the new setting for TLSv1.2, which is reasonable because older versions of the standard are deprecated. Users requiring a deprecated protocol version due to their network infrastructure can lower this value as needed. --- app/lib/katello/resources/cdn.rb | 21 ++++++++++++------ lib/katello/plugin.rb | 10 ++++----- test/lib/resources/cdn_test.rb | 37 ++++++++++++++++++++++++-------- test/models/setting_test.rb | 9 -------- 4 files changed, 47 insertions(+), 30 deletions(-) diff --git a/app/lib/katello/resources/cdn.rb b/app/lib/katello/resources/cdn.rb index c91f61124f8..e8dec19dad2 100644 --- a/app/lib/katello/resources/cdn.rb +++ b/app/lib/katello/resources/cdn.rb @@ -1,7 +1,14 @@ module Katello module Resources module CDN - SUPPORTED_SSL_VERSIONS = ['SSLv23', 'TLSv1'].freeze + SUPPORTED_TLS_VERSIONS_MAPPING = { + 'TLSv1' => OpenSSL::SSL::TLS1_VERSION, + 'TLSv1.1' => OpenSSL::SSL::TLS1_1_VERSION, + 'TLSv1.2' => OpenSSL::SSL::TLS1_2_VERSION, + 'TLSv1.3' => (OpenSSL::SSL::TLS1_3_VERSION if defined?(OpenSSL::SSL::TLS1_3_VERSION)) + }.compact.freeze + + SUPPORTED_TLS_VERSIONS = SUPPORTED_TLS_VERSIONS_MAPPING.keys.freeze class Utils # takes releasever from contentUrl (e.g. 6Server, 6.0, 6.1) @@ -24,9 +31,9 @@ def substitutor end def initialize(url, options = {}) - @ssl_version = Setting[:cdn_ssl_version] - if @ssl_version && !SUPPORTED_SSL_VERSIONS.include?(@ssl_version) - fail("Invalid SSL version specified. Check the 'CDN SSL Version' setting") + @cdn_min_tls_version = Setting[:cdn_min_tls_version] + if @cdn_min_tls_version && !SUPPORTED_TLS_VERSIONS.include?(@cdn_min_tls_version) + fail("Invalid TLS min_version specified. Check the 'CDN Minimum Allowed TLS Version' setting") end options.reverse_merge!(:verify_ssl => 9) @@ -107,11 +114,11 @@ def http_downloader end # NOTE: This was added because some proxies dont support SSLv23 and do not handle TLS 1.2 - # Valid values in ruby 1.9.3 are 'SSLv23' or 'TLSV1' + # Valid values in ruby 2.7.8 are :TLS1, :TLS1_1, :TLS1_2, :TLS1_3 # Run the following command in rails console to figure out other # valid constants in other ruby versions - # "OpenSSL::SSL::SSLContext::METHODS" - net.ssl_version = @ssl_version + # "OpenSSL::SSL.constants.select { |sym| sym.to_s.include?('VERSION') }" + net.mind_version = SUPPORTED_TLS_VERSIONS_MAPPING[@cdn_min_tls_version] if (@options[:verify_ssl] == false) || (@options[:verify_ssl] == OpenSSL::SSL::VERIFY_NONE) net.verify_mode = OpenSSL::SSL::VERIFY_NONE diff --git a/lib/katello/plugin.rb b/lib/katello/plugin.rb index 2b7e14b5bda..a6277c925a8 100644 --- a/lib/katello/plugin.rb +++ b/lib/katello/plugin.rb @@ -377,12 +377,12 @@ def katello_template_setting_values(name) collection: proc { http_proxy_select }, include_blank: N_("no global default") - setting 'cdn_ssl_version', + setting 'cdn_min_tls_version', type: :string, - default: nil, - full_name: N_('CDN SSL version'), - description: N_("SSL version used to communicate with the CDN"), - collection: proc { hashify_parameters(Katello::Resources::CDN::SUPPORTED_SSL_VERSIONS) } + default: 'TLSv1.2', + full_name: N_('CDN minimum allowed TLS version'), + description: N_("Minimum allowed TLS version used to communicate with the CDN. WARNING: Older versions of the TLS standard than TLSv1.2 are deprecated and should only be enabled when required for backwards compatibility with HTTP proxy servers."), + collection: proc { hashify_parameters(Katello::Resources::CDN::SUPPORTED_TLS_VERSIONS) } setting 'katello_default_provision', type: :string, diff --git a/test/lib/resources/cdn_test.rb b/test/lib/resources/cdn_test.rb index 219803e6de0..72904e422ef 100644 --- a/test/lib/resources/cdn_test.rb +++ b/test/lib/resources/cdn_test.rb @@ -1,20 +1,39 @@ require 'katello_test_helper' class CdnResourceTest < ActiveSupport::TestCase - def test_http_downloader_v2 - Setting[:cdn_ssl_version] = 'SSLv23' + def test_http_downloader_tlsv1 + Setting[:cdn_min_tls_version] = 'TLSv1' cdn_resource = Katello::Resources::CDN::CdnResource.new('http://foo.com') - assert_equal cdn_resource.http_downloader.ssl_version, 'SSLv23' + assert_equal cdn_resource.http_downloader.min_version, 769 end - def test_http_downloader_tls - Setting[:cdn_ssl_version] = 'TLSv1' + def test_http_downloader_tlsv11 + Setting[:cdn_min_tls_version] = 'TLSv1.1' cdn_resource = Katello::Resources::CDN::CdnResource.new('http://foo.com') - assert_equal cdn_resource.http_downloader.ssl_version, 'TLSv1' + assert_equal cdn_resource.http_downloader.min_version, 770 end - def test_http_downloader_no_version + def test_http_downloader_tlsv12 + Setting[:cdn_min_tls_version] = 'TLSv1.2' cdn_resource = Katello::Resources::CDN::CdnResource.new('http://foo.com') - assert_nil cdn_resource.http_downloader.ssl_version + assert_equal cdn_resource.http_downloader.min_version, 771 + end + + def test_http_downloader_tlsv13 + # TLSv1.3 is unavailable in some environments, for example the EL7 builders at + # ci.theforeman.org. This check should be removable when those are upgraded to + # EL8 or later. See https://github.com/theforeman/foreman-infra/issues/1706 + unless defined?(OpenSSL::SSL::TLS1_3_VERSION) + skip "TLSv1.3 is unavailable in the current environment" + end + + Setting[:cdn_min_tls_version] = 'TLSv1.3' + cdn_resource = Katello::Resources::CDN::CdnResource.new('http://foo.com') + assert_equal cdn_resource.http_downloader.min_version, 772 + end + + def test_http_downloader_default_version_tlsv12 + cdn_resource = Katello::Resources::CDN::CdnResource.new('http://foo.com') + assert_equal cdn_resource.http_downloader.min_version, 771 end def test_http_proxy_no_cacert @@ -29,7 +48,7 @@ def test_http_proxy_no_cacert end def test_http_downloader_bad_param - Setting[:cdn_ssl_version] = 'Foo' + Setting[:cdn_min_tls_version] = 'Foo' assert_raise RuntimeError do Katello::Resources::CDN::CdnResource.new('http://foo.com') end diff --git a/test/models/setting_test.rb b/test/models/setting_test.rb index a80266e7938..69871700a38 100644 --- a/test/models/setting_test.rb +++ b/test/models/setting_test.rb @@ -17,15 +17,6 @@ def test_foreman_proxy_content_auto_sync_setting assert setting.valid? end - def test_cdn_ssl_setting - # TODO: assert an error raised by the SettingRegistry - # setting = Foreman.settings.set_user_value('cdn_ssl_version', nil) - # assert setting.valid? - - setting = Foreman.settings.set_user_value('cdn_ssl_version', 'SSLv23') - assert setting.valid? - end - def test_recalculate_errata_status ForemanTasks.expects(:async_task).with(::Actions::Katello::Host::RecalculateErrataStatus) Setting['errata_status_installable'] = !Setting['errata_status_installable']