Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #25293 - Puma support for smart proxy #623

Closed
wants to merge 1 commit into from
Closed
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
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ Lint/UnusedBlockArgument:
Lint/UnusedMethodArgument:
Enabled: false

Naming/FileName:
Enabled: false

Metrics:
Enabled: false

Expand Down
3 changes: 3 additions & 0 deletions bundler.d/puma.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
group :puma do
gem 'puma', '~> 4.1', :require => 'puma'
end
25 changes: 16 additions & 9 deletions config/settings.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@
#: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]
# Enabled ciphers for SSL. The complete list of cipher suite
# names and list format can be found at:
# https://www.openssl.org/docs/man1.0.2/man1/ciphers.html
# Accepts both array and colon-separated (OpenSSL) string.
# The default value (when commented out) is defined in:
# /usr/share/foreman-proxy/lib/proxy/settings/global.rb
#:ssl_enabled_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']
#:ssl_enabled_ciphers: "ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256"
ekohl marked this conversation as resolved.
Show resolved Hide resolved

# 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'
# SSLv3 and TLS 1.0 and 1.1 are disabled in smart-proxy. To disable additional
# versions of TLS, use this setting. Specify versions in an array, e.g. ['1.2'].
#:tls_disabled_versions: []

# Hosts which the proxy accepts connections from
Expand Down Expand Up @@ -49,7 +51,7 @@
#:daemon_pid: /var/run/foreman-proxy/foreman-proxy.pid

# host and ports configuration
# an array of interfaces to bind ports to (possible values: *, localhost, 0.0.0.0)
# an array of interfaces to bind ports to (possible values: *, localhost, 0.0.0.0, ::1, ::)
#:bind_host: ['*']
# http is disabled by default. To enable, uncomment 'http_port' setting
#:http_port: 8000
Expand All @@ -58,6 +60,11 @@
# default values for https_port is 8443
#:https_port: 8443

# http server type configuration
lzap marked this conversation as resolved.
Show resolved Hide resolved
# A string that indicates the server type (possible values: 'webrick', 'puma').
# Default value is 'puma'
#:http_server_type: 'puma'
ekohl marked this conversation as resolved.
Show resolved Hide resolved

# Log configuration
# Uncomment and modify if you want to change the location of the log file or use STDOUT or SYSLOG values
#:log_file: /var/log/foreman-proxy/proxy.log
Expand Down
237 changes: 172 additions & 65 deletions lib/launcher.rb
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
require 'proxy/log'
require 'proxy/util'
require 'proxy/sd_notify_all'
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
require 'rack'
require 'webrick'
iNecas marked this conversation as resolved.
Show resolved Hide resolved
ekohl marked this conversation as resolved.
Show resolved Hide resolved

module Proxy
class Launcher
include ::Proxy::Log
include ::Proxy::Util

attr_reader :settings

def initialize(settings = SETTINGS)
def initialize(settings = Proxy::SETTINGS)
@settings = settings
if @settings.http_server_type == "puma"
require 'puma'
require 'rack/handler/puma'
require 'puma-patch'
end
@servers = []
end

def pid_path
Expand Down Expand Up @@ -50,7 +57,7 @@ def http_app(http_port, plugins = http_plugins)

{
:app => app,
:server => :webrick,
:server => settings.http_server_type.to_sym,
:DoNotListen => true,
:Port => http_port, # only being used to correctly log http port being used
:Logger => ::Proxy::LogBuffer::TraceDecorator.instance,
Expand All @@ -70,41 +77,69 @@ 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)

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
ssl_enabled_ciphers = if settings.ssl_enabled_ciphers.is_a?(String)
settings.ssl_enabled_ciphers.split(':')
else
settings.ssl_enabled_ciphers
end

{
app_details = {
:app => app,
:server => :webrick,
:server => settings.http_server_type,
:DoNotListen => true,
:Port => https_port, # only being used to correctly log https port being used
:Logger => ::Proxy::LogBuffer::Decorator.instance,
:ServerSoftware => "foreman-proxy/#{Proxy::VERSION}",
:SSLEnable => true,
:SSLVerifyClient => OpenSSL::SSL::VERIFY_PEER,
: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,
:SSLCiphers => ssl_enabled_ciphers,
:daemonize => false,
}

case settings.http_server_type
when "webrick"
ssl_options = OpenSSL::SSL::SSLContext::DEFAULT_PARAMS[:options]
ssl_options |= OpenSSL::SSL::OP_CIPHER_SERVER_PREFERENCE if defined?(OpenSSL::SSL::OP_CIPHER_SERVER_PREFERENCE)
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)

if settings.tls_disabled_versions
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
end

app_details[:SSLEnable] = true
app_details[:SSLVerifyClient] = OpenSSL::SSL::VERIFY_PEER
app_details[:SSLCACertificateFile] = settings.ssl_ca_file
app_details[:SSLPrivateKey] = load_ssl_private_key(settings.ssl_private_key)
app_details[:SSLCertificate] = load_ssl_certificate(settings.ssl_certificate)
app_details[:SSLOptions] = ssl_options
when "puma"
# https://github.com/puma/puma#binding-tcp--sockets
app_details[:SSLArgs] = {
:ca => settings.ssl_ca_file,
:key => settings.ssl_private_key,
:cert => settings.ssl_certificate,
:verify_mode => 'peer',
}
app_details[:SSLArgs][:no_tlsv1] = "true"
app_details[:SSLArgs][:no_tlsv1_1] = "true"
# no additional TLS versions via tls_disabled_versions can be currently disabled for puma
if settings.ssl_enabled_ciphers
app_details[:SSLArgs][:ssl_cipher_list] = ssl_enabled_ciphers.join(':')
end
else
raise "Unknown http_server_type: #{settings.http_server_type}"
end
app_details
end

def load_ssl_private_key(path)
Expand Down Expand Up @@ -152,13 +187,87 @@ def write_pid
retry
end

def webrick_server(app, addresses, port)
def add_puma_server_callback(sd_notify)
events = ::Puma::Events.new(::Proxy::LogBuffer::Decorator.instance, ::Proxy::LogBuffer::Decorator.instance)
events.register(:state) do |status|
if status == :running
sd_notify.ready_all { sd_notify.status("Started all #{sd_notify.total} threads, ready", logger) }
sd_notify.status("Started, #{sd_notify.pending} threads to go", logger) if sd_notify.pending > 0
end
end
events
end

def format_ip_for_url(address)
addr = IPAddr.new(address)
addr.ipv6? ? "[#{addr}]" : addr.to_s
rescue IPAddr::InvalidAddressError
address
end

def add_puma_server(app, address, port, conn_type, sd_notify)
address = format_ip_for_url(address)
logger.debug "Launching Puma listener at #{address} port #{port}"
if conn_type == :ssl
host = "ssl://#{address}:#{port}/?#{hash_to_query_string(app[:SSLArgs])}"
else
host = address
end
logger.debug "Host URL: #{host}"
# the following lines are from lib/rack/handler/puma.rb#run
options = {Verbose: true, Port: port, Host: host}
conf = Rack::Handler::Puma.config(app[:app], options)
# install callback to notify systemd
events = add_puma_server_callback(sd_notify)
launcher = ::Puma::Launcher.new(conf, :events => events)
@servers << launcher
launcher.run
end

def add_webrick_server_callback(app, sd_notify)
app[:StartCallback] = lambda do
sd_notify.ready_all { sd_notify.status("Started all #{sd_notify.total} threads, ready", logger) }
sd_notify.status("Started, #{sd_notify.pending} threads to go", logger) if sd_notify.pending > 0
end
end

def add_webrick_server(app, addresses, port, sd_notify)
# install callback to notify systemd
add_webrick_server_callback(app, sd_notify)
# initialize the server
server = ::WEBrick::HTTPServer.new(app)
addresses.each { |a| server.listen(a, port) }
server.mount "/", Rack::Handler::WEBrick, app[:app]
addresses.each do |address|
logger.debug "Launching Webrick listener at #{address} port #{port}"
ekohl marked this conversation as resolved.
Show resolved Hide resolved
server.listen(address, port)
end
server.mount '/', Rack::Handler::WEBrick, app[:app]
server
end

def ipv6_enabled?
File.exist?('/proc/net/if_inet6') || (RUBY_PLATFORM =~ /cygwin|mswin|mingw|bccwin|wince|emx/)
end

def add_threaded_server(server_name, conn_type, app, addresses, port, sd_notify)
result = []
case server_name
when "webrick"
result << Thread.new do
@servers << add_webrick_server(app, addresses, port, sd_notify).start
end
when "puma"
addresses.flatten.each do |address|
# Puma listens both on IPv4 and IPv6 on '::', there is no way to make Puma
Copy link
Member

Choose a reason for hiding this comment

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

In EL7 systems with Ruby 2.0.0 where IPv6 was disabled in the kernel I've seen it fail to listen on ::. That's why we have a workaround in the installer. Could you verify this won't be an issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed it does not work, however I can hardly do anything about it in the code because the exception is "Bad URI":

2019-08-14T13:22:10  [E] Unable to start server on [::] port 8448: bad URI(is not URI?): "tcp://[::]:8448"

I guess on non-IPv6 systems users must not use '*'.

Copy link
Member

Choose a reason for hiding this comment

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

This surprises me, but I've verified that on Fedora 31 with Ruby 2.6.5-p114 that binding to * only binds to 0.0.0.0 and thus breaking IPv6. https://stackoverflow.com/questions/20657650/socket-listener-for-ipv6-and-ipv4 suggests you should bind to both for cross platform support. On Linux it can also be changed via the net.ipv6.bindv6only sysctl in which case :: only binds to IPv6. Any idea how Windows behaves?

# to listen only on IPv6.
address = '::' if address == '*' && ipv6_enabled?
result << Thread.new do
add_puma_server(app, address, port, conn_type, sd_notify)
end
end
end
result
end

def launch
raise Exception.new("Both http and https are disabled, unable to start.") unless http_enabled? || https_enabled?

Expand All @@ -172,14 +281,35 @@ def launch

http_app = http_app(settings.http_port)
https_app = https_app(settings.https_port)
install_webrick_callback!(http_app, https_app)

t1 = Thread.new { webrick_server(https_app, settings.bind_host, settings.https_port).start } unless https_app.nil?
t2 = Thread.new { webrick_server(http_app, settings.bind_host, settings.http_port).start } unless http_app.nil?
hosts = settings.bind_host.is_a?(Array) ? settings.bind_host.size : 1
expected = [http_app, https_app].compact.size * hosts
logger.debug "Expected number of instances to launch: #{expected}"
sd_notify = Proxy::SdNotifyAll.new(expected)
sd_notify.status("Starting #{expected} threads", logger)

http_server_name = settings.http_server_type
https_server_name = settings.http_server_type
threads = []
if https_app
threads += add_threaded_server(https_server_name,
:ssl,
https_app,
settings.bind_host,
settings.https_port,
sd_notify)
end

Proxy::SignalHandler.install_traps
if http_app
threads += add_threaded_server(http_server_name,
:tcp,
http_app,
settings.bind_host,
settings.http_port,
sd_notify)
end

(t1 || t2).join
Proxy::SignalHandler.install_traps(@servers)
threads.each(&:join)
rescue SignalException => e
logger.debug("Caught #{e}. Exiting")
raise
Expand All @@ -191,28 +321,5 @@ def launch
puts "Errors detected on startup, see log for details. Exiting: #{e}"
exit(1)
end

def install_webrick_callback!(*apps)
apps.compact!

# track how many webrick apps are still starting up
@pending_webrick = apps.size
@pending_webrick_lock = Mutex.new

apps.each do |app|
# add a callback to each server, decrementing the pending counter
app[:StartCallback] = lambda do
@pending_webrick_lock.synchronize do
@pending_webrick -= 1
launched(apps) if @pending_webrick.zero?
end
end
end
end

def launched(apps)
logger.info("Smart proxy has launched on #{apps.size} socket(s), waiting for requests")
SdNotify.ready
end
end
end
19 changes: 17 additions & 2 deletions lib/proxy/helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ def log_halt(code = nil, exception_or_msg = nil, custom_msg = nil)
end

# read the HTTPS client certificate from the environment and extract its CN
def https_cert_cn
certificate_raw = request.env['SSL_CLIENT_CERT'].to_s
def https_cert_cn(request)
certificate_raw = ssl_client_cert(request)
log_halt 403, 'could not read client cert from environment' if certificate_raw.empty?

begin
Expand Down Expand Up @@ -106,4 +106,19 @@ def remote_fqdn(forward_verify = true)
fqdn
end
end

def ssl_client_cert(request)
if request.env.key?('SSL_CLIENT_CERT')
Copy link
Member

Choose a reason for hiding this comment

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

IMHO this should look at the server in use and only test for that specific variable.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see any security threat, this way it can be a simple helper.

Copy link
Member

Choose a reason for hiding this comment

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

When using Puma and providing a header named SSL_CLIENT_CERT, can you guarantee it won't end up in the env? I'd rather be safe than sorry and only check for what is known to be expected.

request.env['SSL_CLIENT_CERT'].to_s
Copy link
Member

Choose a reason for hiding this comment

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

When is an env var not a string?

Copy link
Member

Choose a reason for hiding this comment

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

When it's variable ;) Not apologizing for my bad joke :P

Copy link
Member

Choose a reason for hiding this comment

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

I later noticed that the .to_s was already there in the previous code as well.

elsif request.env.key?('puma.peercert')
request.env['puma.peercert'].to_s
else
''
end
end

def https?(request)
# test env variable for puma and also webrick
request.env['HTTPS'].to_s == 'https' || request.env['HTTPS'].to_s =~ /yes|on|1/
end
end
Loading