From 509451d8ce0a7550bd50a1a1c9b6550e0084c969 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Ne=C4=8Das?= Date: Fri, 14 Sep 2018 12:50:59 +0200 Subject: [PATCH] Fixes #25001 - CVE-2018-14643 - ensure auth --- lib/smart_proxy_dynflow/api.rb | 39 +++++++++++++++- test/api_test/api_test.rb | 84 +++++++++++++++++++++++++++++----- 2 files changed, 110 insertions(+), 13 deletions(-) diff --git a/lib/smart_proxy_dynflow/api.rb b/lib/smart_proxy_dynflow/api.rb index cf9c5ca..653d9a1 100644 --- a/lib/smart_proxy_dynflow/api.rb +++ b/lib/smart_proxy_dynflow/api.rb @@ -6,18 +6,53 @@ module Proxy class Dynflow class Api < ::Sinatra::Base helpers ::Proxy::Helpers + helpers ::Proxy::Log helpers ::Proxy::Dynflow::Helpers before do - logger = Proxy::LogBuffer::Decorator.instance content_type :json if request.env['HTTP_AUTHORIZATION'] && request.env['PATH_INFO'].end_with?('/done') # Halt running before callbacks if a token is provided and the request is notifying about task being done return + else + do_authorize_with_ssl_client + do_authorize_with_trusted_hosts end end - helpers Sinatra::Authorization + + # TODO: move this to foreman-proxy to reduce code duplicities + def do_authorize_with_trusted_hosts + # When :trusted_hosts is given, we check the client against the list + # HTTPS: test the certificate CN + # HTTP: test the reverse DNS entry of the remote IP + trusted_hosts = Proxy::SETTINGS.trusted_hosts + if trusted_hosts + if [ 'yes', 'on', 1 ].include? request.env['HTTPS'].to_s + fqdn = https_cert_cn + source = 'SSL_CLIENT_CERT' + else + fqdn = remote_fqdn(Proxy::SETTINGS.forward_verify) + source = 'REMOTE_ADDR' + end + fqdn = fqdn.downcase + logger.debug "verifying remote client #{fqdn} (based on #{source}) against trusted_hosts #{trusted_hosts}" + + unless Proxy::SETTINGS.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? + log_halt 403, "No client SSL certificate supplied" + end + else + logger.debug('require_ssl_client_verification: skipping, non-HTTPS request') + end + end post "/*" do relay_request diff --git a/test/api_test/api_test.rb b/test/api_test/api_test.rb index 7bc8280..e7b506e 100644 --- a/test/api_test/api_test.rb +++ b/test/api_test/api_test.rb @@ -14,27 +14,34 @@ def hostname 'somehost.somedomain.org:9000' end - def request_factory(kind, path) + def request_factory(kind, path, env = {}) body = mock() body.stubs(:read).returns("") - env = { + env = env.merge( 'REQUEST_METHOD' => kind, 'rack.request.query_hash' => {}, - 'HTTP_HOST' => hostname - } - OpenStruct.new(:env => env, :body => body, :path => '/dynflow' + path) + 'HTTP_HOST' => hostname, + 'PATH_INFO' => "/dynflow#{path}" + ) + Sinatra::Request.new(env).tap do |r| + r.stubs(:body).returns(body) + end end let(:new_request) { Net::HTTP::Get.new 'example.org' } - it 'relays GET requests' do + def mock_core_service(method, path, response) factory = mock() - factory.expects(:create_get).with('/tasks/count', {}).returns(new_request) + factory.expects(method).with { |p| p == path }.returns(new_request) Proxy::Dynflow::Callback::Core.any_instance.expects(:request_factory).returns(factory) Proxy::Dynflow::Callback::Core.any_instance - .expects(:send_request).with(new_request) - .returns(OpenStruct.new(:code => 200, :body => {'count' => 0})) - Sinatra::Base.any_instance.expects(:request).times(4).returns(request_factory('GET', '/tasks/count')) + .expects(:send_request).with(new_request) + .returns(OpenStruct.new(response)) + end + + it 'relays GET requests' do + mock_core_service(:create_get, '/tasks/count', :code => 200, :body => {'count' => 0}) + Proxy::Dynflow::Api.any_instance.stubs(:request).returns(request_factory('GET', '/tasks/count')) get '/tasks/count' new_request['X-Forwarded-For'].must_equal hostname end @@ -46,9 +53,64 @@ def request_factory(kind, path) Proxy::Dynflow::Callback::Core.any_instance .expects(:send_request).with(new_request) .returns(OpenStruct.new(:code => 200, :body => {'count' => 0})) - Sinatra::Base.any_instance.expects(:request).times(4).returns(request_factory('POST', '/tasks/12345/cancel')) + Proxy::Dynflow::Api.any_instance.stubs(:request).returns(request_factory('POST', '/tasks/12345/cancel')) post '/tasks/12345/cancel', {} new_request['X-Forwarded-For'].must_equal hostname end + + it 'refuses unauthorized http connections (using remote_fqdn)' do + Proxy::Dynflow::Api.any_instance.stubs(:request).returns(request_factory('POST', '/tasks')) + Proxy::Dynflow::Api.any_instance.stubs(:remote_fqdn).returns('unauthorized_host.example.com') + Proxy::SETTINGS.stubs(:trusted_hosts).returns(["mytrustedhost.example.com"]) + post '/tasks' + assert(last_response.forbidden?, 'The request should be forbidden') + end + + it 'accepts authorized http connections (using remote_fqdn)' do + mock_core_service(:create_post, '/tasks', :code => 200, :body => {}) + Proxy::Dynflow::Api.any_instance.stubs(:request).returns(request_factory('POST', '/tasks')) + Proxy::Dynflow::Api.any_instance.stubs(:remote_fqdn).returns('mytrustedhost.example.com') + Proxy::SETTINGS.stubs(:trusted_hosts).returns(["mytrustedhost.example.com"]) + post '/tasks' + assert(last_response.ok?, 'The response should be ok') + end + + it 'refuses unauthorized https connections (using https_cert_cn)' do + Proxy::Dynflow::Api.any_instance.stubs(:request). + returns(request_factory('POST', '/tasks', 'HTTPS' => 'yes','SSL_CLIENT_CERT' => 'mytrustedcert')) + Proxy::Dynflow::Api.any_instance.stubs(:https_cert_cn).returns('unauthorized_host.example.com') + Proxy::SETTINGS.stubs(:trusted_hosts).returns(["mytrustedhost.example.com"]) + post '/tasks' + assert(last_response.forbidden?, 'The request should be forbidden') + end + + it 'accepts unauthorized https connections (using https_cert_cn)' do + mock_core_service(:create_post, '/tasks', :code => 200, :body => {}) + Proxy::Dynflow::Api.any_instance.stubs(:request). + returns(request_factory('POST', '/tasks', 'HTTPS' => 'yes', 'SSL_CLIENT_CERT' => 'mytrustedcert')) + Proxy::Dynflow::Api.any_instance.stubs(:https_cert_cn).returns('mytrustedhost.example.com') + Proxy::SETTINGS.stubs(:trusted_hosts).returns(["mytrustedhost.example.com"]) + post '/tasks' + assert(last_response.ok?, 'The response should be ok') + end + + it 'refuses unauthorized https connections (when client cert is not supplied)' do + Proxy::Dynflow::Api.any_instance.stubs(:request). + returns(request_factory('POST', '/tasks', 'HTTPS' => 'yes')) + Proxy::Dynflow::Api.any_instance.stubs(:https_cert_cn).returns('mytrustedhost.example.com') + Proxy::SETTINGS.stubs(:trusted_hosts).returns(["mytrustedhost.example.com"]) + post '/tasks' + assert(last_response.forbidden?, 'The request should be forbidden') + end + + it 'passes the done requests to the core service, when authorization keys are provided' do + mock_core_service(:create_post, '/tasks/123/done', :code => 200, :body => {}) + Proxy::Dynflow::Api.any_instance.stubs(:request). + returns(request_factory('POST', '/tasks/123/done', 'HTTPS' => 'yes', 'HTTP_AUTHORIZATION' => 'Basic ValidToken')) + Proxy::Dynflow::Api.any_instance.stubs(:https_cert_cn).returns('mytrustedhost.example.com') + Proxy::SETTINGS.stubs(:trusted_hosts).returns(["mytrustedhost.example.com"]) + post '/tasks' + assert(last_response.ok?, 'The response should be ok') + end end end