diff --git a/lib/proxy/helpers.rb b/lib/proxy/helpers.rb index 7f7d6038b..046ba8012 100644 --- a/lib/proxy/helpers.rb +++ b/lib/proxy/helpers.rb @@ -28,34 +28,17 @@ def log_halt(code = nil, exception_or_msg = nil, custom_msg = nil) halt code, message 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 - log_halt 403, 'could not read client cert from environment' if certificate_raw.empty? - - begin - certificate = OpenSSL::X509::Certificate.new certificate_raw - if certificate.subject && certificate.subject.to_s =~ /CN=([^\s\/,]+)/i - $1 - else - log_halt 403, 'could not read CN from the client certificate' - end - rescue OpenSSL::X509::CertificateError => e - log_halt 403, "could not parse the client certificate\n\n#{e.message}" - end - end - # parses the body as json and returns a hash of the body # returns empty hash if there is a json parse error, the body is empty or is not a hash # request.env["CONTENT_TYPE"] must contain application/json in order for the json to be parsed - def parse_json_body + def parse_json_body(request) json_data = {} # if the user has explicitly set the content_type then there must be something worth decoding # we use a regex because it might contain something else like: application/json;charset=utf-8 # by default the content type will probably be set to "application/x-www-form-urlencoded" unless the # user changed it. If the user doesn't specify the content type we just ignore the body since a form # will be parsed into the request.params object for us by sinatra - if request.env["CONTENT_TYPE"] =~ /application\/json/ + if request.media_type == 'application/json' begin body_parameters = request.body.read json_data = JSON.parse(body_parameters) @@ -77,33 +60,4 @@ def dns_resolv(*args) def resolv(*args) ::Proxy::LoggingResolv.new(Resolv.new(*args)) end - - # reverse lookup an IP address while verifying it via forward resolv - def remote_fqdn(forward_verify = true) - ip = request.env['REMOTE_ADDR'] - log_halt 403, 'could not get remote address from environment' if ip.empty? - - begin - dns = resolv - fqdn = dns.getname(ip) - rescue Resolv::ResolvError => e - log_halt 403, "unable to resolve hostname for ip address #{ip}\n\n#{e.message}" - end - - if forward_verify - begin - forward = dns.getaddresses(fqdn) - rescue Resolv::ResolvError => e - log_halt 403, "could not forward verify the remote hostname - #{fqdn} (#{ip})\n\n#{e.message}" - end - - if forward.include?(ip) - fqdn - else - log_halt 403, "untrusted client has no matching forward DNS lookup - #{fqdn} (#{ip})" - end - else - fqdn - end - end end diff --git a/lib/proxy/middleware/authorization.rb b/lib/proxy/middleware/authorization.rb new file mode 100644 index 000000000..8acae6109 --- /dev/null +++ b/lib/proxy/middleware/authorization.rb @@ -0,0 +1,110 @@ +module Proxy + module Middleware + class Authorization + include ::Proxy::Log + + def initialize(app) + @app = app + end + + def call(env) + if https?(env) + certificate_raw = https_client_cert_raw(env) + return unauthorized if certificate_raw.empty? + + if trusted_hosts? + begin + certificate = OpenSSL::X509::Certificate.new(certificate_raw) + rescue OpenSSL::X509::CertificateError => e + logger.warn("Could not parse the client certificate: #{e.message}") + return unauthorized + end + + fqdn = get_cn_from_certificate(certificate) + unless fqdn + logger.warn('Could not read CN from the client certificate') + return unauthorized + end + + return denied(fqdn) unless trusted_host?(fqdn) + end + elsif trusted_hosts? + return denied(fqdn) unless trusted_host?(remote_fqdn) + end + + @app.call(env) + end + + private + + def settings + Proxy::SETTINGS + end + + def unauthorized + [401, {}, ['Unauthorized']] + end + + def denied(fqdn) + path = request.path_info # TODO + logger.warn("Untrusted client #{fqdn} attempted to access #{path}. Check :trusted_hosts: in settings.yml") + [403, {}, ['Denied']] + end + + def https?(env) + ['yes', 'on', 1].include?(env['HTTPS'].to_s) + end + + def https_client_cert_raw(env) + env['SSL_CLIENT_CERT'].to_s + end + + def trusted_hosts? + settings.trusted_hosts + end + + def trusted_host?(fqdn) + logger.debug "verifying remote client #{fqdn} against trusted_hosts #{trusted_hosts}" + trusted_hosts.include?(fqdn.downcase) + end + + # reverse lookup an IP address while verifying it via forward resolv + def remote_fqdn + ip = env['REMOTE_ADDR'] + log_halt 403, 'could not get remote address from environment' if ip.empty? + + begin + dns = resolv + fqdn = dns.getname(ip) + rescue Resolv::ResolvError => e + log_halt 403, "unable to resolve hostname for ip address #{ip}\n\n#{e.message}" + end + + if settings.forward_verify + begin + forward = dns.getaddresses(fqdn) + rescue Resolv::ResolvError => e + log_halt 403, "could not forward verify the remote hostname - #{fqdn} (#{ip})\n\n#{e.message}" + end + + if forward.include?(ip) + fqdn + else + log_halt 403, "untrusted client has no matching forward DNS lookup - #{fqdn} (#{ip})" + end + else + fqdn + end + end + + def get_cn_from_certificate(certificate) + return unless certificate&.subject + + cn = certificate.subject.to_a.find { |oid| oid == 'CN' } + return unless cn + + cn[2] + end + end + end +end diff --git a/lib/sinatra/authorization.rb b/lib/sinatra/authorization.rb index 7be503e8c..e3eb14b21 100644 --- a/lib/sinatra/authorization.rb +++ b/lib/sinatra/authorization.rb @@ -30,24 +30,19 @@ def do_authorize_with_trusted_hosts # HTTP: test the reverse DNS entry of the remote IP trusted_hosts = Proxy::SETTINGS.trusted_hosts if trusted_hosts - logger.debug "verifying remote client #{request.env['REMOTE_ADDR']} against trusted_hosts #{trusted_hosts}" + fqdn = (https?(request) ? https_cert_cn(request) : remote_fqdn(Proxy::SETTINGS.forward_verify)).downcase - if ['yes', 'on', 1].include? request.env['HTTPS'].to_s - fqdn = https_cert_cn - else - fqdn = remote_fqdn(Proxy::SETTINGS.forward_verify) - end - fqdn = fqdn.downcase + logger.debug "verifying remote client #{fqdn} against trusted_hosts #{trusted_hosts}" - unless Proxy::SETTINGS.trusted_hosts.include?(fqdn) + unless trusted_hosts.include?(fqdn) log_halt 403, "Untrusted client #{fqdn} attempted to access #{request.path_info}. Check :trusted_hosts: in settings.yml" end end end def do_authorize_with_ssl_client - if ['yes', 'on', '1'].include? request.env['HTTPS'].to_s - if request.env['SSL_CLIENT_CERT'].to_s.empty? + if https?(request) + if https_client_cert_raw(request).empty? log_halt 403, "No client SSL certificate supplied" end else @@ -60,6 +55,84 @@ def do_authorize_any do_authorize_with_trusted_hosts do_authorize_with_ssl_client end + + private + + def https?(request) + ['yes', 'on', 1].include?(request.env['HTTPS'].to_s) + end + + def https_client_cert_raw(request) + request.env['SSL_CLIENT_CERT'].to_s + end + + # read the HTTPS client certificate from the environment and extract its CN + def https_cert_cn(request) + log_halt 403, 'No HTTPS environment' unless https?(request) + + certificate_raw = https_client_cert_raw(request) + certificate = parse_openssl_cert(certificate_raw) + log_halt 403, 'could not read client cert from environment' unless certificate + + cn = get_cn_from_certificate(certificate) + log_halt 403, 'could not read CN from the client certificate' unless certificate + + cn + rescue OpenSSL::X509::CertificateError => e + log_halt 403, "could not parse the client certificate\n\n#{e.message}" + end + + # reverse lookup an IP address while verifying it via forward resolv + def remote_fqdn(forward_verify = true) + ip = request.env['REMOTE_ADDR'] + log_halt 403, 'could not get remote address from environment' if ip.empty? + + begin + dns = resolv + fqdn = dns.getname(ip) + rescue Resolv::ResolvError => e + log_halt 403, "unable to resolve hostname for ip address #{ip}\n\n#{e.message}" + end + + if forward_verify + begin + forward = dns.getaddresses(fqdn) + rescue Resolv::ResolvError => e + log_halt 403, "could not forward verify the remote hostname - #{fqdn} (#{ip})\n\n#{e.message}" + end + + if forward.include?(ip) + fqdn + else + log_halt 403, "untrusted client has no matching forward DNS lookup - #{fqdn} (#{ip})" + end + else + fqdn + end + end + + def parse_openssl_cert(certificate_raw) + return if certificate_raw.nil? || certificate_raw.empty? + + OpenSSL::X509::Certificate.new(certificate_raw) + end + + def get_cn_from_certificate(certificate) + return unless certificate&.subject + + cn = certificate.subject.to_a.find { |oid| oid == 'CN' } + return unless cn + + cn[2] + end + end + + def authorize! + include Helpers + + before do + do_authorize_with_any + end end def authorize_with_trusted_hosts diff --git a/lib/smart_proxy_main.rb b/lib/smart_proxy_main.rb index 6428cda65..2309af3e3 100644 --- a/lib/smart_proxy_main.rb +++ b/lib/smart_proxy_main.rb @@ -16,6 +16,7 @@ require 'proxy/http_download' require 'proxy/helpers' require 'proxy/memory_store' +require 'proxy/middleware/authorization' require 'proxy/plugin_validators' require 'proxy/pluggable' require 'proxy/plugins' diff --git a/modules/bmc/bmc_api.rb b/modules/bmc/bmc_api.rb index cbcc48d79..c2f095735 100644 --- a/modules/bmc/bmc_api.rb +++ b/modules/bmc/bmc_api.rb @@ -4,8 +4,6 @@ module Proxy::BMC class Api < ::Sinatra::Base helpers ::Proxy::Helpers - authorize_with_trusted_hosts - authorize_with_ssl_client # All GET requests will only read ipmi data, no changes # All PUT requests will update information on the bmc device @@ -449,7 +447,7 @@ def bmc_setup # also if the user decides to do http://127.0.0.1/bmc/192.168.1.6/test?bmc_provider=freeipmi as well as pass in # a json encode body with the parameters, all of these items will be merged together def body_parameters - @body_parameters ||= parse_json_body.merge(params) + @body_parameters ||= parse_json_body(request).merge(params) end def auth diff --git a/modules/dhcp/dhcp_api.rb b/modules/dhcp/dhcp_api.rb index 01dec9791..da8c5b73c 100644 --- a/modules/dhcp/dhcp_api.rb +++ b/modules/dhcp/dhcp_api.rb @@ -2,8 +2,6 @@ class Proxy::DhcpApi < ::Sinatra::Base extend Proxy::DHCP::DependencyInjection helpers ::Proxy::Helpers - authorize_with_trusted_hosts - authorize_with_ssl_client use Rack::MethodOverride inject_attr :dhcp_provider, :server diff --git a/modules/dhcp/http_config.ru b/modules/dhcp/http_config.ru index 0418f5fda..5691f0a1e 100644 --- a/modules/dhcp/http_config.ru +++ b/modules/dhcp/http_config.ru @@ -1,5 +1,6 @@ require 'dhcp/dhcp_api' map "/dhcp" do + use Proxy::Middleware::Authorization run Proxy::DhcpApi end diff --git a/modules/dns/dns_api.rb b/modules/dns/dns_api.rb index 426cfbe34..835b51545 100644 --- a/modules/dns/dns_api.rb +++ b/modules/dns/dns_api.rb @@ -6,8 +6,6 @@ class Api < ::Sinatra::Base inject_attr :dns_provider, :server helpers ::Proxy::Helpers - authorize_with_trusted_hosts - authorize_with_ssl_client post "/?" do fqdn = params[:fqdn] diff --git a/modules/dns/http_config.ru b/modules/dns/http_config.ru index 3d617e0f5..0f08bbc85 100644 --- a/modules/dns/http_config.ru +++ b/modules/dns/http_config.ru @@ -1,5 +1,6 @@ require 'dns/dns_api' map "/dns" do + use Proxy::Middleware::Authorization run Proxy::Dns::Api end diff --git a/modules/facts/facts_api.rb b/modules/facts/facts_api.rb index 52f5c7ca6..8f9b690ff 100644 --- a/modules/facts/facts_api.rb +++ b/modules/facts/facts_api.rb @@ -1,7 +1,5 @@ class Proxy::FactsApi < Sinatra::Base helpers ::Proxy::Helpers - authorize_with_trusted_hosts - authorize_with_ssl_client get "/?" do content_type :json diff --git a/modules/facts/http_config.ru b/modules/facts/http_config.ru index ab16e35a3..e9b161128 100644 --- a/modules/facts/http_config.ru +++ b/modules/facts/http_config.ru @@ -1,5 +1,6 @@ require 'facts/facts_api' map "/facts" do + use Proxy::Middleware::Authorization run Proxy::FactsApi end diff --git a/modules/logs/http_config.ru b/modules/logs/http_config.ru index 63584dca4..ebd6f207b 100644 --- a/modules/logs/http_config.ru +++ b/modules/logs/http_config.ru @@ -1,5 +1,6 @@ require 'logs/logs_api' map "/logs" do + use Proxy::Middleware::Authorization run Proxy::LogsApi end diff --git a/modules/logs/logs_api.rb b/modules/logs/logs_api.rb index 1e8251619..799375c2c 100644 --- a/modules/logs/logs_api.rb +++ b/modules/logs/logs_api.rb @@ -3,8 +3,6 @@ class Proxy::LogsApi < Sinatra::Base helpers ::Proxy::Helpers - authorize_with_trusted_hosts - authorize_with_ssl_client get "/" do content_type :json diff --git a/modules/puppet_proxy/http_config.ru b/modules/puppet_proxy/http_config.ru index d6fa24a5e..98e00e649 100644 --- a/modules/puppet_proxy/http_config.ru +++ b/modules/puppet_proxy/http_config.ru @@ -1,5 +1,6 @@ require 'puppet_proxy/puppet_api' map "/puppet" do + use Proxy::Middleware::Authorization run Proxy::Puppet::Api end diff --git a/modules/puppet_proxy/puppet_api.rb b/modules/puppet_proxy/puppet_api.rb index 77cc927ac..f962bf18e 100644 --- a/modules/puppet_proxy/puppet_api.rb +++ b/modules/puppet_proxy/puppet_api.rb @@ -2,9 +2,6 @@ class Proxy::Puppet::Api < ::Sinatra::Base extend Proxy::Puppet::DependencyInjection helpers ::Proxy::Helpers - authorize_with_trusted_hosts - authorize_with_ssl_client - inject_attr :class_retriever_impl, :class_retriever inject_attr :environment_retriever_impl, :environment_retriever diff --git a/modules/puppetca/http_config.ru b/modules/puppetca/http_config.ru index e8aabf118..81ea04607 100644 --- a/modules/puppetca/http_config.ru +++ b/modules/puppetca/http_config.ru @@ -1,5 +1,6 @@ require 'puppetca/puppetca_api' map "/puppet/ca" do + use Proxy::Middleware::Authorization run Proxy::PuppetCa::Api end diff --git a/modules/puppetca/puppetca_api.rb b/modules/puppetca/puppetca_api.rb index e99969949..81b737319 100644 --- a/modules/puppetca/puppetca_api.rb +++ b/modules/puppetca/puppetca_api.rb @@ -5,8 +5,6 @@ class Api < ::Sinatra::Base inject_attr :autosigner, :autosigner helpers ::Proxy::Helpers - authorize_with_trusted_hosts - authorize_with_ssl_client get "/?" do content_type :json diff --git a/modules/realm/http_config.ru b/modules/realm/http_config.ru index ff4cabe2d..f6bff9683 100644 --- a/modules/realm/http_config.ru +++ b/modules/realm/http_config.ru @@ -1,5 +1,6 @@ require 'realm/realm_api' map "/realm" do + use Proxy::Middleware::Authorization run Proxy::Realm::Api end diff --git a/modules/realm/realm_api.rb b/modules/realm/realm_api.rb index 39d6d183f..a41ffc0d5 100644 --- a/modules/realm/realm_api.rb +++ b/modules/realm/realm_api.rb @@ -3,8 +3,6 @@ class Api < Sinatra::Base extend Proxy::Realm::DependencyInjection helpers ::Proxy::Helpers - authorize_with_trusted_hosts - authorize_with_ssl_client inject_attr :realm_provider_impl, :realm_provider diff --git a/modules/root/http_config.ru b/modules/root/http_config.ru index 09b3e5b3a..d46430d83 100644 --- a/modules/root/http_config.ru +++ b/modules/root/http_config.ru @@ -6,5 +6,6 @@ map "/" do end map "/v2" do + use Proxy::Middleware::Authorization run Proxy::RootV2Api end diff --git a/modules/root/root_v2_api.rb b/modules/root/root_v2_api.rb index 69a35c765..98955c71c 100644 --- a/modules/root/root_v2_api.rb +++ b/modules/root/root_v2_api.rb @@ -1,9 +1,6 @@ class Proxy::RootV2Api < Sinatra::Base helpers ::Proxy::Helpers - authorize_with_trusted_hosts - authorize_with_ssl_client - get "/features" do enabled_plugins = ::Proxy::Plugins.instance.select do |plugin| plugin[:name] != :foreman_proxy && \ diff --git a/modules/tftp/http_config.ru b/modules/tftp/http_config.ru index 73a415d77..b1521342f 100644 --- a/modules/tftp/http_config.ru +++ b/modules/tftp/http_config.ru @@ -1,5 +1,6 @@ require 'tftp/tftp_api' map "/tftp" do + use Proxy::Middleware::Authorization run Proxy::TFTP::Api end diff --git a/modules/tftp/tftp_api.rb b/modules/tftp/tftp_api.rb index 1bc6276c7..f2a91d6a8 100644 --- a/modules/tftp/tftp_api.rb +++ b/modules/tftp/tftp_api.rb @@ -6,8 +6,6 @@ class Api < ::Sinatra::Base include ::Proxy::Log include ::Proxy::Validations helpers ::Proxy::Helpers - authorize_with_trusted_hosts - authorize_with_ssl_client VARIANTS = ["Syslinux", "Pxelinux", "Pxegrub", "Pxegrub2", "Ztp", "Poap", "Ipxe"].freeze helpers do diff --git a/smart_proxy.gemspec b/smart_proxy.gemspec index c7df536c3..c2db753b0 100644 --- a/smart_proxy.gemspec +++ b/smart_proxy.gemspec @@ -15,7 +15,7 @@ Gem::Specification.new do |s| s.required_ruby_version = '>= 2.5' s.add_dependency 'json' s.add_dependency 'logging' - s.add_dependency 'rack', '>= 1.3' + s.add_dependency 'rack', '~> 1.6' s.add_dependency 'sd_notify', '~> 0.1' s.add_dependency 'sinatra' s.description = <<~EOF diff --git a/test/proxy/helpers_test.rb b/test/proxy/helpers_test.rb new file mode 100644 index 000000000..035bf0258 --- /dev/null +++ b/test/proxy/helpers_test.rb @@ -0,0 +1,26 @@ +require 'test_helper' + +require 'proxy/helpers' + +class HelperTester + include Proxy::Helpers +end + +class ProxyHelpersTest < Test::Unit::TestCase + def build_request(content_type, input) + env = Rack::MockRequest.env_for('http://example.com/', method: 'POST', 'CONTENT_TYPE' => content_type, input: input) + Rack::Request.new(env) + end + + def test_parse_json_body_different_media_type + request = build_request('text/plain', 'Hello World') + result = HelperTester.new.parse_json_body(request) + assert_equal({}, result) + end + + def test_parse_json_body_json_media_type + request = build_request('application/json', '{"hello": "world"}') + result = HelperTester.new.parse_json_body(request) + assert_equal({'hello' => 'world'}, result) + end +end