diff --git a/config/settings.yml.example b/config/settings.yml.example index 87f679dd7..4132affec 100644 --- a/config/settings.yml.example +++ b/config/settings.yml.example @@ -42,6 +42,21 @@ #:foreman_ssl_cert: ssl/certs/fqdn.pem #:foreman_ssl_key: ssl/private_keys/fqdn.pem +# Timeout in seconds for HTTP requests from smart-proxy to Foreman (e.g. POST /register). +# Defaults to 200s to stay above subscription-manager's default server_timeout of 180s, +# ensuring the proxy never times out before the client does. +#:foreman_request_timeout: 200 + +# Timeout in seconds for establishing TCP/TLS connections from smart-proxy to Foreman. +# Leave unset to use Ruby's default. Consider raising this if TLS handshakes (including +# future post-quantum profiles) need more time. +#:foreman_connect_timeout: 30 + +# Seconds to keep idle outbound connections to Foreman alive for reuse. +# Must be lower than the server-side KeepAliveTimeout (15s on RHEL). +# Higher values reuse more TLS connections; lower values reduce stale socket risk. +#:foreman_keep_alive_timeout: 10 + # host and ports configuration # an array of interfaces to bind ports to (possible values: *, localhost, 0.0.0.0) #:bind_host: ['*'] diff --git a/lib/proxy/request.rb b/lib/proxy/request.rb index c0fd4df9b..7e3044ccf 100644 --- a/lib/proxy/request.rb +++ b/lib/proxy/request.rb @@ -44,8 +44,99 @@ def create_post(path, body, headers = {}, query = {}) end class ForemanRequest + DEFAULT_KEEP_ALIVE_TIMEOUT = 10 + DEFAULT_READ_TIMEOUT = 200 + + RETRY_EXCEPTIONS = [ + EOFError, + Errno::ECONNRESET, + Errno::EPIPE, + IOError, + Net::OpenTimeout, + Net::ReadTimeout, + OpenSSL::SSL::SSLError, + ].freeze + + FOREMAN_MUTEX = Mutex.new + REGISTRATION_GET_MUTEX = Mutex.new + + class << self + def reset! + FOREMAN_MUTEX.synchronize do + finish_connection(:@foreman_http) + ForemanRequest.instance_variable_set(:@connection_uri, nil) + end + REGISTRATION_GET_MUTEX.synchronize do + finish_connection(:@registration_get_http) + end + end + + private + + def foreman_http + ForemanRequest.instance_variable_get(:@foreman_http) || + ForemanRequest.instance_variable_set(:@foreman_http, http_init) + end + + def registration_get_http + ForemanRequest.instance_variable_get(:@registration_get_http) || + ForemanRequest.instance_variable_set(:@registration_get_http, http_init) + end + + def finish_connection(ivar) + old = ForemanRequest.instance_variable_get(ivar) + ForemanRequest.instance_variable_set(ivar, nil) + old&.finish + rescue IOError, SystemCallError + nil + end + + def setting(name, default) + value = Proxy::SETTINGS.respond_to?(name) ? Proxy::SETTINGS.public_send(name).to_i : 0 + (value > 0) ? value : default + end + + def connection_uri + ForemanRequest.instance_variable_get(:@connection_uri) || + ForemanRequest.instance_variable_set(:@connection_uri, URI.parse(Proxy::SETTINGS.foreman_url.to_s)) + end + + def http_init + uri = connection_uri + http = Net::HTTP.new(uri.host, uri.port) + http.use_ssl = uri.scheme == 'https' + http.verify_mode = OpenSSL::SSL::VERIFY_NONE + http.keep_alive_timeout = setting(:foreman_keep_alive_timeout, DEFAULT_KEEP_ALIVE_TIMEOUT) + http.read_timeout = setting(:foreman_request_timeout, DEFAULT_READ_TIMEOUT) + timeout = setting(:foreman_connect_timeout, 0) + http.open_timeout = timeout if timeout > 0 + + if http.use_ssl? + ca_file = Proxy::SETTINGS.foreman_ssl_ca || Proxy::SETTINGS.ssl_ca_file + certificate = Proxy::SETTINGS.foreman_ssl_cert || Proxy::SETTINGS.ssl_certificate + private_key = Proxy::SETTINGS.foreman_ssl_key || Proxy::SETTINGS.ssl_private_key + + if ca_file && !ca_file.to_s.empty? + http.ca_file = ca_file + http.verify_mode = OpenSSL::SSL::VERIFY_PEER + end + + if certificate && !certificate.to_s.empty? && private_key && !private_key.to_s.empty? + http.cert = OpenSSL::X509::Certificate.new(File.read(certificate)) + http.key = OpenSSL::PKey.read(File.read(private_key), nil) + end + end + + http + end + end + def send_request(request) - http.request(request) + execute_with(FOREMAN_MUTEX, :@foreman_http, request) + end + + def send_request_direct(request) + execute_with(REGISTRATION_GET_MUTEX, :@registration_get_http, request) end def request_factory @@ -57,32 +148,26 @@ def uri end def http - @http ||= http_init + self.class.send(:foreman_http) end private - def http_init - http = Net::HTTP.new(uri.host, uri.port) - http.use_ssl = uri.scheme == 'https' - http.verify_mode = OpenSSL::SSL::VERIFY_NONE - - if http.use_ssl? - ca_file = Proxy::SETTINGS.foreman_ssl_ca || Proxy::SETTINGS.ssl_ca_file - certificate = Proxy::SETTINGS.foreman_ssl_cert || Proxy::SETTINGS.ssl_certificate - private_key = Proxy::SETTINGS.foreman_ssl_key || Proxy::SETTINGS.ssl_private_key - - if ca_file && !ca_file.to_s.empty? - http.ca_file = ca_file - http.verify_mode = OpenSSL::SSL::VERIFY_PEER - end - - if certificate && !certificate.to_s.empty? && private_key && !private_key.to_s.empty? - http.cert = OpenSSL::X509::Certificate.new(File.read(certificate)) - http.key = OpenSSL::PKey.read(File.read(private_key), nil) + def execute_with(mutex, ivar, request) + mutex.synchronize do + http = ForemanRequest.instance_variable_get(ivar) || ForemanRequest.instance_variable_set(ivar, self.class.send(:http_init)) + begin + http.start unless http.started? + http.request(request) + rescue *RETRY_EXCEPTIONS + begin + http.finish + rescue IOError, SystemCallError # rubocop:disable Lint/SuppressedException + end + ForemanRequest.instance_variable_set(ivar, self.class.send(:http_init)) + raise end end - http end end end diff --git a/modules/registration/proxy_request.rb b/modules/registration/proxy_request.rb index 6ad9e99ee..dd7443353 100644 --- a/modules/registration/proxy_request.rb +++ b/modules/registration/proxy_request.rb @@ -7,30 +7,41 @@ def global_register(request) request_params(request), headers(request) - send_request(proxy_req) + send_request_direct(proxy_req) end - # we support two way of sending data - either a JSON or url encoded data + # Retries once on transport errors. Foreman's POST /register uses + # find_or_initialize_by for the host record, making a single replay + # safe after a stale keep-alive connection failure. + REGISTER_RETRY_EXCEPTIONS = [ + EOFError, + Errno::ECONNRESET, + Errno::EPIPE, + IOError, + OpenSSL::SSL::SSLError, + ].freeze + def host_register(request) + body = request.body.read + hdrs = headers(request) + if request.content_type == 'application/x-www-form-urlencoded' - # the request has a different content type, ForemanRequestFactory sets content type to json, unless - # specified explicitly - # also request.params contain the same data that is in request.body, just parsed to hash, - # in case they are nested (e.g. host hash) we need this causes problem during CGI escaping - # therefore we only add url, everything else should be in body in this type of request - proxy_req = request_factory.create_post '/register', - request.body.read, - headers(request).merge("Content-Type" => request.content_type), - { url: register_url(request) } + content_type = request.content_type + query = { url: register_url(request) } + build_req = -> { request_factory.create_post('/register', body, hdrs.merge("Content-Type" => content_type), query) } else - # the application/json request body contains the data - JSON as a string, query contains only the URL - proxy_req = request_factory.create_post '/register', - request.body.read, - headers(request), - request_params(request) + params = request_params(request) + build_req = -> { request_factory.create_post('/register', body, hdrs, params) } end - send_request(proxy_req) + retried = false + begin + send_request(build_req.call) + rescue *REGISTER_RETRY_EXCEPTIONS + raise if retried + retried = true + retry + end end private diff --git a/test/request_test.rb b/test/request_test.rb index 81d6f53b0..26c83822c 100644 --- a/test/request_test.rb +++ b/test/request_test.rb @@ -1,8 +1,12 @@ require 'test_helper' require 'uri' require 'net/http' +require 'openssl' +require 'tempfile' require 'mocha' require 'templates/templates_plugin' +require 'templates/proxy_request' +require 'registration/proxy_request' require "proxy/util" require 'proxy/request' require 'webmock/test_unit' @@ -11,6 +15,16 @@ class RequestTest < Test::Unit::TestCase def setup @foreman_url = 'https://foreman.example.com' Proxy::SETTINGS.stubs(:foreman_url).returns(@foreman_url) + Proxy::SETTINGS.stubs(:foreman_ssl_ca).returns(nil) + Proxy::SETTINGS.stubs(:ssl_ca_file).returns(nil) + Proxy::SETTINGS.stubs(:foreman_ssl_cert).returns(nil) + Proxy::SETTINGS.stubs(:ssl_certificate).returns(nil) + Proxy::SETTINGS.stubs(:foreman_ssl_key).returns(nil) + Proxy::SETTINGS.stubs(:ssl_private_key).returns(nil) + Proxy::SETTINGS.stubs(:foreman_request_timeout).returns(nil) + Proxy::SETTINGS.stubs(:foreman_connect_timeout).returns(nil) + Proxy::SETTINGS.stubs(:foreman_keep_alive_timeout).returns(nil) + Proxy::HttpRequest::ForemanRequest.reset! @template_url = 'http://proxy.lan:8443' Proxy::Templates::Plugin.load_test_settings(:template_url => @template_url) @request = Proxy::HttpRequest::ForemanRequest.new @@ -54,6 +68,110 @@ def test_post assert_equal("body", result.body) end + def test_connection_is_shared_across_request_instances + request_a = Proxy::HttpRequest::ForemanRequest.new + request_b = Proxy::HttpRequest::ForemanRequest.new + + assert_same request_a.http, request_b.http + assert_kind_of Net::HTTP, request_a.http + end + + def test_connection_is_shared_across_foreman_request_subclasses + registration_request = Proxy::Registration::ProxyRequest.new + template_request = Proxy::Templates::ProxyRequest.new + + assert_same registration_request.http, template_request.http + end + + def test_ssl_verify_none_when_no_ca_configured + assert_equal OpenSSL::SSL::VERIFY_NONE, @request.http.verify_mode + end + + def test_ssl_verify_peer_when_ca_configured + ca_file = Tempfile.new(['foreman-ca', '.pem']) + ca_file.write("ca-content") + ca_file.flush + + Proxy::SETTINGS.stubs(:foreman_ssl_ca).returns(ca_file.path) + Proxy::HttpRequest::ForemanRequest.reset! + + connection = Proxy::HttpRequest::ForemanRequest.new.http + + assert_equal OpenSSL::SSL::VERIFY_PEER, connection.verify_mode + assert_equal ca_file.path, connection.ca_file + ensure + ca_file&.close + ca_file&.unlink + end + + def test_read_timeout_uses_default + assert_equal 200, @request.http.read_timeout + end + + def test_read_timeout_configurable + Proxy::SETTINGS.stubs(:foreman_request_timeout).returns(120) + Proxy::HttpRequest::ForemanRequest.reset! + request = Proxy::HttpRequest::ForemanRequest.new + assert_equal 120, request.http.read_timeout + end + + def test_keep_alive_timeout_uses_default + assert_equal 10, @request.http.keep_alive_timeout + end + + def test_keep_alive_timeout_configurable + Proxy::SETTINGS.stubs(:foreman_keep_alive_timeout).returns(8) + Proxy::HttpRequest::ForemanRequest.reset! + request = Proxy::HttpRequest::ForemanRequest.new + assert_equal 8, request.http.keep_alive_timeout + end + + def test_connect_timeout_not_set_by_default + default = Net::HTTP.new('example.com').open_timeout + assert_equal default, @request.http.open_timeout + end + + def test_connect_timeout_configurable + Proxy::SETTINGS.stubs(:foreman_connect_timeout).returns(30) + Proxy::HttpRequest::ForemanRequest.reset! + request = Proxy::HttpRequest::ForemanRequest.new + assert_equal 30, request.http.open_timeout + end + + def test_send_request_and_send_request_direct_use_different_connections + stub_request(:get, @foreman_url + '/path').to_return(:status => [200, 'OK'], :body => "body") + proxy_req = @request.request_factory.create_get("/path") + + @request.send_request(proxy_req) + foreman_http = Proxy::HttpRequest::ForemanRequest.instance_variable_get(:@foreman_http) + + @request.send_request_direct(proxy_req) + registration_get_http = Proxy::HttpRequest::ForemanRequest.instance_variable_get(:@registration_get_http) + + assert_not_same foreman_http, registration_get_http + end + + def test_transport_error_replaces_connection_and_raises + stub_request(:get, @foreman_url + '/path').to_raise(EOFError) + proxy_req = @request.request_factory.create_get("/path") + + assert_raises(EOFError) { @request.send_request(proxy_req) } + http_before = Proxy::HttpRequest::ForemanRequest.instance_variable_get(:@foreman_http) + + stub_request(:get, @foreman_url + '/path').to_raise(EOFError) + assert_raises(EOFError) { @request.send_request(proxy_req) } + + http_after = Proxy::HttpRequest::ForemanRequest.instance_variable_get(:@foreman_http) + assert_not_same http_before, http_after + end + + def test_send_request_direct_succeeds + stub_request(:get, @foreman_url + '/path').to_return(:status => [200, 'OK'], :body => "body") + proxy_req = @request.request_factory.create_get("/path") + result = @request.send_request_direct(proxy_req) + assert_equal("body", result.body) + end + def test_post_with_nested_params stub_request(:post, @foreman_url + '/register?activation_keys%5B%5D=ac_AlmaLinux8&location_id=2&organization_id=1&repo_data%5B%5D%5Brepo%5D=repo1&repo_data%5B%5D%5Brepo_gpg_key_url%5D=key1&repo_data%5B%5D%5Brepo%5D=repo2&repo_data%5B%5D%5Brepo_gpg_key_url%5D=key2&update_packages=false') .to_return(status: 200, body: "body", headers: {h1: "header"})