Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions config/settings.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@
#:ssl_ca_file: ssl/certs/ca.pem
#:ssl_private_key: ssl/private_keys/fqdn.key

# Use this option only if you need to disable certain cipher suites.
# Note: we use the OpenSSL suite name, such as "RC4-MD5".
# The complete list of cipher suite names can be found at:
# https://www.openssl.org/docs/manmaster/man1/ciphers.html#CIPHER-SUITE-NAMES
#:ssl_disabled_ciphers: [CIPHER-SUITE-1, CIPHER-SUITE-2]
# TLS cipher string passed directly to OpenSSL. Accepts any valid OpenSSL cipher
# string. When unset, foreman-proxy auto-detects: 'PROFILE=SYSTEM' is used if
# /etc/crypto-policies/back-ends/opensslcnf.config, otherwise 'HIGH' is used
# as a secure default.
#:tls_ciphers: PROFILE=SYSTEM

# Use this option only if you need to strictly specify TLS versions to be
# disabled. SSLv3 and TLS v1.0 are always disabled and cannot be configured.
# Specify versions like: '1.1', or '1.2'
#:tls_disabled_versions: []
# Minimum TLS protocol version to accept. When unset, the minimum version is
# determined by the system's OpenSSL configuration (crypto-policies).
# Valid values: '1.0', '1.1', '1.2', '1.3'.
#:tls_min_version: '1.2'

# Hosts which the proxy accepts connections from
# commenting the following lines would mean every verified SSL connection allowed
Expand Down
136 changes: 110 additions & 26 deletions lib/launcher.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
require 'openssl'
require 'proxy/log'
require 'proxy/settings'
require 'proxy/signal_handler'
require 'proxy/log_buffer/trace_decorator'
require 'sd_notify'

CIPHERS = ['ECDHE-RSA-AES128-GCM-SHA256', 'ECDHE-RSA-AES256-GCM-SHA384',
'AES128-GCM-SHA256', 'AES256-GCM-SHA384', 'AES128-SHA256',
'AES256-SHA256', 'AES128-SHA', 'AES256-SHA'].freeze
CRYPTO_POLICIES_CONFIG = '/etc/crypto-policies/back-ends/opensslcnf.config'.freeze
TLS_MIN_VERSION_MAP = {
'1.0' => OpenSSL::SSL::TLS1_VERSION,
'1.1' => OpenSSL::SSL::TLS1_1_VERSION,
'1.2' => OpenSSL::SSL::TLS1_2_VERSION,
'1.3' => OpenSSL::SSL::TLS1_3_VERSION,
}.freeze

module Proxy
class Launcher
Expand Down Expand Up @@ -66,26 +71,8 @@ def https_app(https_port, plugins = https_plugins)
plugins.each { |p| instance_eval(p.https_rackup) }
end

ssl_options = OpenSSL::SSL::SSLContext::DEFAULT_PARAMS[:options]
ssl_options |= OpenSSL::SSL::OP_CIPHER_SERVER_PREFERENCE if defined?(OpenSSL::SSL::OP_CIPHER_SERVER_PREFERENCE)
# This is required to disable SSLv3 on Ruby 1.8.7
ssl_options |= OpenSSL::SSL::OP_NO_SSLv2 if defined?(OpenSSL::SSL::OP_NO_SSLv2)
ssl_options |= OpenSSL::SSL::OP_NO_SSLv3 if defined?(OpenSSL::SSL::OP_NO_SSLv3)
ssl_options |= OpenSSL::SSL::OP_NO_TLSv1 if defined?(OpenSSL::SSL::OP_NO_TLSv1)
ssl_options |= OpenSSL::SSL::OP_NO_TLSv1_1 if defined?(OpenSSL::SSL::OP_NO_TLSv1_1)
# Disable client initiated renegotiation
ssl_options |= OpenSSL::SSL::OP_NO_RENEGOTIATION if defined?(OpenSSL::SSL::OP_NO_RENEGOTIATION)

Proxy::SETTINGS.tls_disabled_versions&.each do |version|
constant = OpenSSL::SSL.const_get("OP_NO_TLSv#{version.to_s.tr('.', '_')}") rescue nil

if constant
logger.info "TLSv#{version} will be disabled."
ssl_options |= constant
else
logger.warn "TLSv#{version} was not found."
end
end
tls_ciphers = resolve_tls_ciphers
cipher_list, ciphersuites = validate_tls_ciphers!(tls_ciphers)

https_settings = {
:app => app,
Expand All @@ -96,12 +83,93 @@ def https_app(https_port, plugins = https_plugins)
:SSLPrivateKey => load_ssl_private_key(settings.ssl_private_key),
:SSLCertificate => load_ssl_certificate(settings.ssl_certificate),
:SSLCACertificateFile => settings.ssl_ca_file,
:SSLOptions => ssl_options,
:SSLCiphers => CIPHERS - Proxy::SETTINGS.ssl_disabled_ciphers,
:SSLOptions => build_ssl_options,
:SSLCiphers => cipher_list,
:SSLCiphersuites => ciphersuites,
:SSLMinVersion => resolve_tls_min_version,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we set crypto-policy to DEFAULT:TLS13-ONLY (TLS13-ONLY being a custom subpolicy) but in the config we set tls_min_version to 1.2, this would keep the correct cipher set but would degrade TLS min version to 1.2 (in conflict with crypto-policies).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chmm, in theory we could go back to the old approach of disabling individual versions which should interact with crypto policies in a slightly better way

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a bit of an edge case. I'd prefer to have 2 use cases:

  • crypto-policies (RPM-based installs)
  • explicitly configured (where crypto-policies is unavailable, like Debian, FreeBSD and Windows)
    The case where you configure 1 option, but not the other is IMHO up to the user who does that.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can keep min_tls version in repo and remove it in the foreman-packaging rpm patch so that rpm builds rely only on crypto policies. Would that work @adamruzicka @ekohl ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to have as few patches in packaging as possible. How about making PROFILE=SYSTEM special and refusing to start if tls_ciphers == 'PROFILE=SYSTEM' and tls_min_version is set?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I can do that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, and please add a test for it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can keep min_tls version in repo and remove it in the foreman-packaging rpm patch so that rpm builds rely only on crypto policies.

If we're confident enough in the auto-detection then we wouldn't need any packaging patch. I'd be OK with that. Emitting a log message on startup that can be asserted in automated tests is probably enough.

I'd prefer to have as few patches in packaging as possible. How about making PROFILE=SYSTEM special and refusing to start if tls_ciphers == 'PROFILE=SYSTEM' and tls_min_version is set?

Is it really a problem? If the user configures it that way, what will break? If foremanctl doesn't provide you any option to set that then IMHO it's really up to the user. Perhaps emit a log warning?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to have as few patches in packaging as possible. How about making PROFILE=SYSTEM special and refusing to start if tls_ciphers == 'PROFILE=SYSTEM' and tls_min_version is set?

Applied.

}
base_app_settings.merge(https_settings)
end

def build_ssl_options
ssl_options = OpenSSL::SSL::SSLContext::DEFAULT_PARAMS[:options]
ssl_options |= OpenSSL::SSL::OP_CIPHER_SERVER_PREFERENCE
# Disable client initiated renegotiation
ssl_options |= OpenSSL::SSL::OP_NO_RENEGOTIATION

ssl_options
end

def resolve_tls_min_version
return nil unless settings.tls_min_version

min_version = settings.tls_min_version.to_s
unless TLS_MIN_VERSION_MAP.key?(min_version)
raise "Invalid tls_min_version '#{min_version}'. Valid values: #{TLS_MIN_VERSION_MAP.keys.join(', ')}"
end

logger.info "Setting minimum TLS version to #{min_version}."
TLS_MIN_VERSION_MAP[min_version]
end

def resolve_tls_ciphers
configured = settings.tls_ciphers
raise "Invalid tls_ciphers value '#{configured}': must be a String" if !configured.nil? && !configured.is_a?(String)
return nil if configured&.empty?
return configured unless configured.nil?

if File.exist?(CRYPTO_POLICIES_CONFIG)
logger.info "Crypto-policies detected, using PROFILE=SYSTEM for TLS ciphers."
'PROFILE=SYSTEM'
else
logger.debug "No crypto-policies detected, using HIGH cipher string as default."
'HIGH'
end
end

def validate_tls_ciphers!(ciphers)
return [nil, nil] if ciphers.nil?

if ciphers == 'PROFILE=SYSTEM' && !settings.tls_min_version.nil? && settings.tls_min_version != ''
logger.warn "tls_min_version is configured together with tls_ciphers 'PROFILE=SYSTEM'. " \
"The system crypto policy minimum TLS version may be overridden by this setting."
end

cipher_list = begin
OpenSSL::SSL::SSLContext.new.ciphers = ciphers
ciphers
rescue OpenSSL::SSL::SSLError
nil
end

if OpenSSL::SSL::SSLContext.method_defined?(:ciphersuites=)
ciphersuites = begin
OpenSSL::SSL::SSLContext.new.ciphersuites = ciphers
ciphers
rescue OpenSSL::SSL::SSLError
nil
end
elsif settings.tls_ciphers
logger.warn "tls_ciphers is configured but this Ruby/OpenSSL build does not support " \
"OpenSSL::SSL::SSLContext#ciphersuites=. TLS 1.3 connections will not be " \
"restricted by tls_ciphers."
end

# If the cipher list and ciphersuites are not valid, return the original string to let the SSL server fail at startup
return [ciphers, nil] unless cipher_list || ciphersuites

if cipher_list.nil? && ciphersuites && settings.tls_min_version != '1.3'
logger.warn "tls_ciphers '#{ciphers}' is only valid for TLS 1.3. " \
"Set tls_min_version to '1.3' to prevent TLS 1.2 and lower connections with unrestricted ciphers."
end

if settings.tls_ciphers && settings.tls_min_version == '1.3' && ciphersuites.nil?
raise "tls_ciphers '#{ciphers}' is not valid for TLS 1.3 but tls_min_version is 1.3."
end

[cipher_list, ciphersuites]
end

def load_ssl_private_key(path)
OpenSSL::PKey.read(File.read(path))
rescue Exception => e
Expand All @@ -118,8 +186,24 @@ def load_ssl_certificate(path)

def webrick_server(app, addresses, port)
server = ::WEBrick::HTTPServer.new(app)
addresses.each { |a| server.listen(a, port) }
begin
addresses.each { |a| server.listen(a, port) }
rescue ::OpenSSL::SSL::SSLError => e
raise "Invalid tls_ciphers value '#{app[:SSLCiphers]}': #{e.message}"
end
server.mount "/", Rack::Handler::WEBrick, app[:app]

# WEBrick 1.9.x does not support :SSLMinVersion in its config hash, so we
# apply min_version= directly on the SSL context after WEBrick creates it.
# This patch should be removed once WEBrick adds support for :SSLMinVersion.
#
# :SSLCiphers is passed to SSL_CTX_set_cipher_list(), covering TLS 1.0–1.2.
# :SSLCiphersuites is applied here via SSL_CTX_set_ciphersuites() for TLS 1.3.
if app[:SSLEnable]
server.ssl_context.min_version = app[:SSLMinVersion] if app[:SSLMinVersion]
server.ssl_context.ciphersuites = app[:SSLCiphersuites] if app[:SSLCiphersuites]
end
Comment thread
adamruzicka marked this conversation as resolved.

server
end

Expand Down
4 changes: 2 additions & 2 deletions lib/proxy/settings/global.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ class Global < ::OpenStruct
:bind_host => ["*"],
:log_buffer => 2000,
:log_buffer_errors => 1000,
:ssl_disabled_ciphers => [],
:tls_disabled_versions => [],
:tls_ciphers => nil,
:tls_min_version => nil,
:dns_resolv_timeouts => [5, 8, 13], # Ruby default is [5, 20, 40] which is a bit too much for us
}

Expand Down
174 changes: 174 additions & 0 deletions test/launcher_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,177 @@ def test_launched_with_sdnotify
@launcher.launched([:app1, :app2])
end
end

class LauncherTlsCiphersTest < Test::Unit::TestCase
def setup
@launcher = Proxy::Launcher.new
end

def launcher_with(settings_hash)
settings = Proxy::Settings::Global.new(settings_hash)
Proxy::Launcher.new(settings)
end

def test_resolve_tls_ciphers_returns_configured_value
launcher = launcher_with(tls_ciphers: 'HIGH:!aNULL')
assert_equal 'HIGH:!aNULL', launcher.resolve_tls_ciphers
end

def test_resolve_tls_ciphers_returns_nil_for_empty_string
launcher = launcher_with(tls_ciphers: '')
assert_nil launcher.resolve_tls_ciphers
end

def test_resolve_tls_ciphers_raises_on_non_string
launcher = launcher_with(tls_ciphers: 12)
error = assert_raises(RuntimeError) { launcher.resolve_tls_ciphers }
assert_match(/Invalid tls_ciphers value/, error.message)
assert_match(/must be a String/, error.message)
end

def test_resolve_tls_ciphers_autodetects_profile_system_when_crypto_policies_present
launcher = launcher_with({})
File.expects(:exist?).with(CRYPTO_POLICIES_CONFIG).returns(true)
launcher.logger.stubs(:info)
assert_equal 'PROFILE=SYSTEM', launcher.resolve_tls_ciphers
end

def test_resolve_tls_ciphers_defaults_to_high_when_no_crypto_policies
launcher = launcher_with({})
File.expects(:exist?).with(CRYPTO_POLICIES_CONFIG).returns(false)
launcher.logger.stubs(:debug)
assert_equal 'HIGH', launcher.resolve_tls_ciphers
end

def test_validate_tls_ciphers_returns_nil_pair_for_nil
assert_equal [nil, nil], @launcher.validate_tls_ciphers!(nil)
end

def test_validate_tls_ciphers_returns_tls12_only_for_tls12_cipher_string
tls12, tls13 = @launcher.validate_tls_ciphers!('HIGH')
assert_equal 'HIGH', tls12
assert_nil tls13
end

def test_validate_tls_ciphers_warns_when_profile_system_and_tls_min_version_set
launcher = launcher_with(tls_min_version: '1.2')
launcher.logger.expects(:warn).with(regexp_matches(/PROFILE=SYSTEM/))
launcher.validate_tls_ciphers!('PROFILE=SYSTEM')
end

def test_validate_tls_ciphers_raises_when_tls12_only_cipher_string_and_min_version_tls13
launcher = launcher_with(tls_ciphers: 'HIGH', tls_min_version: '1.3')
error = assert_raises(RuntimeError) { launcher.validate_tls_ciphers!('HIGH') }
assert_match(/1\.3/, error.message)
end

def test_validate_tls_ciphers_does_not_raise_when_min_version_tls13_and_tls_ciphers_unset
launcher = launcher_with(tls_min_version: '1.3')
File.stubs(:exist?).with(CRYPTO_POLICIES_CONFIG).returns(false)
launcher.logger.stubs(:debug)
assert_nothing_raised { launcher.validate_tls_ciphers!(launcher.resolve_tls_ciphers) }
end

def test_validate_tls_ciphers_warns_when_ciphersuites_method_absent_and_tls_ciphers_set
launcher = launcher_with(tls_ciphers: 'HIGH')
OpenSSL::SSL::SSLContext.stubs(:method_defined?).with(:ciphersuites=).returns(false)
launcher.logger.expects(:warn).with(regexp_matches(/ciphersuites=/))
cipher_list, ciphersuites = launcher.validate_tls_ciphers!('HIGH')
assert_equal 'HIGH', cipher_list
assert_nil ciphersuites
end

def test_validate_tls_ciphers_does_not_warn_when_ciphersuites_method_absent_and_tls_ciphers_unset
launcher = launcher_with({})
OpenSSL::SSL::SSLContext.stubs(:method_defined?).with(:ciphersuites=).returns(false)
launcher.logger.expects(:warn).never
cipher_list, ciphersuites = launcher.validate_tls_ciphers!('HIGH')
assert_equal 'HIGH', cipher_list
assert_nil ciphersuites
end
end

class LauncherTlsMinVersionTest < Test::Unit::TestCase
def test_resolve_tls_min_version_raises_on_invalid_version
settings = Proxy::Settings::Global.new(tls_min_version: '1.4')
launcher = Proxy::Launcher.new(settings)
error = assert_raises(RuntimeError) { launcher.resolve_tls_min_version }
assert_match(/Invalid tls_min_version/, error.message)
assert_match(/1\.4/, error.message)
end

def test_resolve_tls_min_version_returns_constant_on_valid_version
settings = Proxy::Settings::Global.new(tls_min_version: '1.3')
launcher = Proxy::Launcher.new(settings)
launcher.logger.stubs(:info)
assert_equal OpenSSL::SSL::TLS1_3_VERSION, launcher.resolve_tls_min_version
end

def test_resolve_tls_min_version_returns_nil_when_not_configured
settings = Proxy::Settings::Global.new({})
launcher = Proxy::Launcher.new(settings)
assert_nil launcher.resolve_tls_min_version
end
end

class LauncherWebrickSslTest < Test::Unit::TestCase
def setup
cert, key = WEBrick::Utils.create_self_signed_cert(2048, [['CN', 'test']], 'test')
@ssl_app = {
:SSLEnable => true,
:SSLPrivateKey => key,
:SSLCertificate => cert,
:DoNotListen => true,
:Logger => WEBrick::Log.new(::File::NULL),
:AccessLog => [],
}
@launcher = Proxy::Launcher.new
::WEBrick::HTTPServer.any_instance.stubs(:mount)
end

def test_webrick_server_applies_ssl_min_version
@ssl_app[:SSLMinVersion] = OpenSSL::SSL::TLS1_3_VERSION
server = @launcher.webrick_server(@ssl_app, [], 8443)
assert_equal OpenSSL::SSL::TLS1_3_VERSION,
server.ssl_context.instance_variable_get(:@min_proto_version)
end

def test_webrick_server_does_not_apply_ssl_min_version_when_absent
server = @launcher.webrick_server(@ssl_app, [], 8443)
assert_nil server.ssl_context.instance_variable_get(:@min_proto_version)
end

def test_webrick_server_applies_ciphersuites_when_ssl_ciphersuites_set
@ssl_app[:SSLCiphersuites] = 'PROFILE=SYSTEM'
OpenSSL::SSL::SSLContext.any_instance.expects(:ciphersuites=).with('PROFILE=SYSTEM')
@launcher.webrick_server(@ssl_app, [], 8443)
end

def test_webrick_server_skips_ciphersuites_when_ssl_ciphersuites_absent
OpenSSL::SSL::SSLContext.any_instance.expects(:ciphersuites=).never
@launcher.webrick_server(@ssl_app, [], 8443)
end
end

class LauncherSslCipherTest < Test::Unit::TestCase
SSL_FIXTURES = File.expand_path(File.join(__dir__, 'fixtures', 'ssl')).freeze

def launcher_with_cipher(cipher)
settings = Proxy::Settings::Global.new(
tls_ciphers: cipher,
ssl_private_key: File.join(SSL_FIXTURES, 'private_keys', 'server.example.com.pem'),
ssl_certificate: File.join(SSL_FIXTURES, 'certs', 'server.example.com.pem'),
ssl_ca_file: File.join(SSL_FIXTURES, 'certs', 'ca.pem')
)
Proxy::Launcher.new(settings)
end

def test_server_raises_on_invalid_cipher_string
launcher = launcher_with_cipher('NOT_A_VALID_CIPHER_STRING!!!')
app = launcher.https_app(0, [])
error = assert_raises(RuntimeError) do
launcher.webrick_server(app.merge(AccessLog: [Logger.new('/dev/null')]), ['localhost'], 0)
end
assert_match(/NOT_A_VALID_CIPHER_STRING!!!/, error.message)
end
end
Loading