Skip to content

Commit

Permalink
Fixes #36979 - Change cdn_ssl_version setting to cdn_min_tls_version
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
wbclark committed Dec 12, 2023
1 parent d068f0d commit 3b49686
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 28 deletions.
19 changes: 13 additions & 6 deletions app/lib/katello/resources/cdn.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module Katello
module Resources
module CDN
SUPPORTED_SSL_VERSIONS = ['SSLv23', 'TLSv1'].freeze
SUPPORTED_TLS_VERSIONS = ['TLSv1', 'TLSv1.1', 'TLSv1.2', 'TLSv1.3'].freeze

class Utils
# takes releasever from contentUrl (e.g. 6Server, 6.0, 6.1)
Expand All @@ -24,8 +24,8 @@ def substitutor
end

def initialize(url, options = {})
@ssl_version = Setting[:cdn_ssl_version]
if @ssl_version && !SUPPORTED_SSL_VERSIONS.include?(@ssl_version)
@cdn_min_tls_version = Setting[:cdn_min_tls_version]
if @cdn_min_tls_version && !SUPPORTED_TLS_VERSIONS.include?(@cdn_min_tls_version)
fail("Invalid SSL version specified. Check the 'CDN SSL Version' setting")
end

Expand Down Expand Up @@ -107,11 +107,18 @@ 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') }"
if @cdn_min_tls_version
net.min_version = OpenSSL::SSL.const_get(
@cdn_min_tls_version.gsub('TLSv', 'TLS')
.gsub('.', '_')
.concat('_VERSION')
.to_sym
)
end

if (@options[:verify_ssl] == false) || (@options[:verify_ssl] == OpenSSL::SSL::VERIFY_NONE)
net.verify_mode = OpenSSL::SSL::VERIFY_NONE
Expand Down
10 changes: 5 additions & 5 deletions lib/katello/plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
28 changes: 20 additions & 8 deletions test/lib/resources/cdn_test.rb
Original file line number Diff line number Diff line change
@@ -1,20 +1,32 @@
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'
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_tlsv12
Setting[:cdn_min_tls_version] = 'TLSv1'
cdn_resource = Katello::Resources::CDN::CdnResource.new('http://foo.com')
assert_equal cdn_resource.http_downloader.min_version, 771
end

def test_http_downloader_tlsv13
Setting[:cdn_min_tls_version] = 'TLSv1'
cdn_resource = Katello::Resources::CDN::CdnResource.new('http://foo.com')
assert_equal cdn_resource.http_downloader.min_version, 772
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
assert_nil cdn_resource.http_downloader.min_version
end

def test_http_proxy_no_cacert
Expand All @@ -29,7 +41,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
Expand Down
9 changes: 0 additions & 9 deletions test/models/setting_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down

0 comments on commit 3b49686

Please sign in to comment.