Fixes #36979 - Remove cdn_ssl_version setting

This was originally added to allow downgrading the CDN connection SSL version for compatibility with much older proxy servers. That should be less of a concern now. We do still set a value of TLS v1.2 for the min_version, but only because ruby/openssl#709 prevents using the system-wide crypto policy for now. In the future, that can be removed as well, restoring control to the user at the OS level. (cherry picked from commit 78fcba9)
qcjames53 · Mar 19, 2024 · 815a5c7 · 815a5c7
1 parent 78c2b3b
commit 815a5c7
Show file tree

Hide file tree

Showing 4 changed files with 4 additions and 53 deletions.
diff --git a/app/lib/katello/resources/cdn.rb b/app/lib/katello/resources/cdn.rb
@@ -1,8 +1,6 @@
 module Katello
   module Resources
     module CDN
-      SUPPORTED_SSL_VERSIONS = ['SSLv23', 'TLSv1'].freeze
-
       class Utils
         # takes releasever from contentUrl (e.g. 6Server, 6.0, 6.1)
         # returns hash e.g. {:major => 6, :minor => "6.1"}
@@ -24,11 +22,6 @@ 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")
-          end
-
           options.reverse_merge!(:verify_ssl => 9)
           options.assert_valid_keys(:ssl_client_key,
                                     :ssl_client_cert,
@@ -106,12 +99,10 @@ def http_downloader
             net.cert_store = @cert_store
           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'
-          # 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
+          # NOTE: This is only here due to https://github.com/ruby/openssl/issues/709, otherwise the
+          # system-wide crypto policy could be used. Enforcing TLS version >= 1.2  will prevent using
+          # very old infrastructure for now, but that was considered better than having an insecure default.
+          net.min_version = OpenSSL::SSL::TLS1_2_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
@@ -373,13 +373,6 @@ def katello_template_setting_values(name)
         collection: proc { http_proxy_select },
         include_blank: N_("no global default")
 
-      setting 'cdn_ssl_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) }
-
       setting 'katello_default_provision',
         type: :string,
         default: 'Kickstart default',

diff --git a/test/lib/resources/cdn_test.rb b/test/lib/resources/cdn_test.rb
@@ -1,22 +1,5 @@
 require 'katello_test_helper'
 class CdnResourceTest < ActiveSupport::TestCase
-  def test_http_downloader_v2
-    Setting[:cdn_ssl_version] = 'SSLv23'
-    cdn_resource = Katello::Resources::CDN::CdnResource.new('http://foo.com')
-    assert_equal cdn_resource.http_downloader.ssl_version, 'SSLv23'
-  end
-
-  def test_http_downloader_tls
-    Setting[:cdn_ssl_version] = 'TLSv1'
-    cdn_resource = Katello::Resources::CDN::CdnResource.new('http://foo.com')
-    assert_equal cdn_resource.http_downloader.ssl_version, 'TLSv1'
-  end
-
-  def test_http_downloader_no_version
-    cdn_resource = Katello::Resources::CDN::CdnResource.new('http://foo.com')
-    assert_nil cdn_resource.http_downloader.ssl_version
-  end
-
   def test_http_proxy_no_cacert
     proxy = FactoryBot.create(:http_proxy, :url => 'http://foo.com:1000',
                               :username => 'admin',
@@ -28,13 +11,6 @@ def test_http_proxy_no_cacert
     Katello::Resources::CDN::CdnResource.new('http://foo.com', ssl_ca_file: "lol")
   end
 
-  def test_http_downloader_bad_param
-    Setting[:cdn_ssl_version] = 'Foo'
-    assert_raise RuntimeError do
-      Katello::Resources::CDN::CdnResource.new('http://foo.com')
-    end
-  end
-
   def test_custom_cdn_auth
     organization = taxonomies(:empty_organization)
     credential = FactoryBot.create(:katello_content_credential, organization: organization)

diff --git 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']