From 4e15e9cb74f85192962d99bdebd71a3b375a41f4 Mon Sep 17 00:00:00 2001 From: Bastian Schmidt Date: Thu, 14 Jul 2022 15:33:47 +0200 Subject: [PATCH] Fixes #35270 - Support system image download * Implement fetch and extract system image * Implement class for file extraction with isoinfo * Add capability for archive extraction * Separate logging and file writing tasks * Add additional API endpoint /tftp/system_image/ Co-Authored-By: Ewoud Kohl van Wijngaarden --- config/settings.d/tftp.yml.example | 11 +++- lib/proxy/archive_extract.rb | 33 ++++++++++++ lib/proxy/util.rb | 29 ++++++++++- lib/smart_proxy_for_testing.rb | 1 + lib/smart_proxy_main.rb | 1 + modules/tftp/http_config.ru | 5 ++ modules/tftp/server.rb | 64 +++++++++++++++++++++++ modules/tftp/tftp_api.rb | 12 +++++ modules/tftp/tftp_plugin.rb | 13 ++++- modules/tftp/tftp_system_image_api.rb | 17 ++++++ test/tftp/integration_test.rb | 20 ++++++- test/tftp/tftp_api_test.rb | 6 +++ test/tftp/tftp_system_image_api_test.rb | 69 +++++++++++++++++++++++++ 13 files changed, 276 insertions(+), 5 deletions(-) create mode 100644 lib/proxy/archive_extract.rb create mode 100644 modules/tftp/tftp_system_image_api.rb create mode 100644 test/tftp/tftp_system_image_api_test.rb diff --git a/config/settings.d/tftp.yml.example b/config/settings.d/tftp.yml.example index 27688934e..356804606 100644 --- a/config/settings.d/tftp.yml.example +++ b/config/settings.d/tftp.yml.example @@ -1,5 +1,5 @@ --- -# Can be true, false, or http/https to enable just one of the protocols +# Can be true or false :enabled: false #:tftproot: /var/lib/tftpboot @@ -13,3 +13,12 @@ # Defines the default certificate action for certificate checking. # When false, the argument --no-check-certificate will be used. #:verify_server_cert: true + +# Enable/Disable Smart Proxy support for managing boot images. Allows the Smart +# Proxy to download, extract, and store system images. It becomes important when +# automating the extraction process as it is done for the Ubuntu Autoinstall +# procedure. +#:enable_system_image: true + +# Defines the default folder to provide system images. +#:system_image_root: /var/lib/foreman-proxy/tftp/system_images diff --git a/lib/proxy/archive_extract.rb b/lib/proxy/archive_extract.rb new file mode 100644 index 000000000..06d97adad --- /dev/null +++ b/lib/proxy/archive_extract.rb @@ -0,0 +1,33 @@ +module Proxy + class ArchiveExtract < Proxy::Util::CommandTask + include Util + + SHELL_COMMAND = 'isoinfo' + + def initialize(image_path, file_in_image, dst_path) + args = [ + which(SHELL_COMMAND), + # Print information from Rock Ridge extensions + '-R', + # Filename to read ISO-9660 image from + '-i', image_path.to_s, + # Extract specified file to stdout + '-x', file_in_image.to_s + ] + + super(args, nil, dst_path) + end + + def start + lock = Proxy::FileLock.try_locking(File.join(File.dirname(@output), ".#{File.basename(@output)}.lock")) + if lock.nil? + false + else + super do + Proxy::FileLock.unlock(lock) + File.unlink(lock) + end + end + end + end +end diff --git a/lib/proxy/util.rb b/lib/proxy/util.rb index e191a70dd..c4d9e314d 100644 --- a/lib/proxy/util.rb +++ b/lib/proxy/util.rb @@ -13,12 +13,19 @@ class CommandTask # stderr is redirected to proxy error log, stdout to proxy debug log # command can be either string or array (command + arguments) # input is passed into STDIN and must be string - def initialize(command, input = nil) + # output can be a string containing a file path. If this is the case, + # output is not logged but written to this file. + def initialize(command, input = nil, output = nil) @command = command @input = input + @output = output end def start(&ensured_block) + @output.nil? ? spawn_logging_thread(&ensured_block) : spawn_output_thread(&ensured_block) + end + + def spawn_logging_thread(&ensured_block) # run the task in its own thread @task = Thread.new(@command, @input) do |cmd, input| status = nil @@ -43,6 +50,26 @@ def start(&ensured_block) self end + def spawn_output_thread(&ensured_block) + # run the task in its own thread + @task = Thread.new(@command, @input, @output) do |cmd, input, file| + status = nil + Open3.pipeline_w(cmd, :out => file.to_s) do |stdin, thr| + cmdline_string = Shellwords.escape(cmd.is_a?(Array) ? cmd.join(' ') : cmd) + last_thr = thr[-1] + logger.info "[#{last_thr.pid}] Started task #{cmdline_string}" + stdin.write(input) if input + stdin.close + # call thr.value to wait for a Process::Status object. + status = last_thr.value + end + status ? status.exitstatus : $CHILD_STATUS + ensure + yield if block_given? + end + self + end + # wait for the task to finish and get the subprocess return code def join @task.value diff --git a/lib/smart_proxy_for_testing.rb b/lib/smart_proxy_for_testing.rb index c617beea3..a18a105c9 100644 --- a/lib/smart_proxy_for_testing.rb +++ b/lib/smart_proxy_for_testing.rb @@ -10,6 +10,7 @@ require 'proxy/dependency_injection' require 'proxy/util' require 'proxy/http_download' +require 'proxy/archive_extract' require 'proxy/helpers' require 'proxy/memory_store' require 'proxy/plugin_validators' diff --git a/lib/smart_proxy_main.rb b/lib/smart_proxy_main.rb index 40a84a1dc..410f168ab 100644 --- a/lib/smart_proxy_main.rb +++ b/lib/smart_proxy_main.rb @@ -14,6 +14,7 @@ require 'proxy/dependency_injection' require 'proxy/util' require 'proxy/http_download' +require 'proxy/archive_extract' require 'proxy/helpers' require 'proxy/memory_store' require 'proxy/plugin_validators' diff --git a/modules/tftp/http_config.ru b/modules/tftp/http_config.ru index 73a415d77..9e2150d93 100644 --- a/modules/tftp/http_config.ru +++ b/modules/tftp/http_config.ru @@ -1,5 +1,10 @@ require 'tftp/tftp_api' +require 'tftp/tftp_system_image_api' map "/tftp" do run Proxy::TFTP::Api end + +map "/tftp/system_image" do + run Proxy::TFTP::SystemImageApi +end diff --git a/modules/tftp/server.rb b/modules/tftp/server.rb index 98218d8bd..6cc6f2fb6 100644 --- a/modules/tftp/server.rb +++ b/modules/tftp/server.rb @@ -150,6 +150,64 @@ def pxeconfig_file(mac) end end + def self.fetch_system_image(image_dst, url, files, tftp_path) + # Build paths, verify parameter do not contain ".." (switch folder), and check existing files + image_root = Pathname.new(Proxy::TFTP::Plugin.settings.system_image_root).cleanpath + image_path = Pathname.new(File.expand_path(image_dst, image_root)).cleanpath + tftproot = Pathname.new(Proxy::TFTP::Plugin.settings.tftproot).cleanpath + raise_error_on_prohibited_path(image_root, image_path, image_dst) + file_exists = File.exist? image_path + extr_file_map = {} + files.each do |file| + extr_filename = boot_filename(tftp_path, file) + extr_file_path = Pathname.new(File.expand_path(extr_filename, tftproot)).cleanpath + raise_error_on_prohibited_path(tftproot, extr_file_path, file) + file_exists = false unless File.exist? extr_file_path + extr_file_map[file] = extr_file_path + end + + if file_exists + 200 # Return 200 if all files exist already + else + fetch_system_image_worker(url, image_path, extr_file_map) + 202 # Return 202 if download process was triggered + end + end + + def self.fetch_system_image_worker(url, image_path, extr_file_map) + lock_file = ".#{File.basename(image_path.sub_ext(''))}.lock" + # Lock + image_path.parent.mkpath + lock = Proxy::FileLock.try_locking(File.join(File.dirname(image_path), lock_file)) + if lock.nil? + raise IOError.new, "System image download and extraction is still in progress" + end + + Thread.new(lock, url, image_path, extr_file_map) do |t_lock, t_url, t_image_path, t_extr_file_map| + # Wait for download completion + download_task = choose_protocol_and_fetch(t_url, t_image_path) + if download_task.is_a?(FalseClass) + logger.error "TFTP image download error: Is another process downloading it already?" + Thread.stop + end + unless download_task.join == 0 + logger.error "TFTP image download error: Task did not complete" + Thread.stop + end + + t_extr_file_map.each do |file_in_image, extr_file| + # Create destination directory and extract file from iso + extr_file.parent.mkpath + extract_task = ::Proxy::ArchiveExtract.new(t_image_path, file_in_image, extr_file).start + logger.error "TFTP image file extraction error: #{file_in_image} => #{extr_file}" unless extract_task.join == 0 + end + ensure + # Unlock + Proxy::FileLock.unlock(t_lock) + File.unlink(t_lock) + end + end + def self.fetch_boot_file(dst, src) filename = boot_filename(dst, src) destination = Pathname.new(File.expand_path(filename, Proxy::TFTP::Plugin.settings.tftproot)).cleanpath @@ -180,4 +238,10 @@ def self.boot_filename(dst, src) # Do not append a '-' if the dst is a directory path dst.end_with?('/') ? dst + src.split("/")[-1] : dst + '-' + src.split("/")[-1] end + + def self.raise_error_on_prohibited_path(base_path, relative_path, error_parameter) + if relative_path.expand_path.relative_path_from(base_path).to_s.start_with?('..') + raise "File to extract from image contains up-directory: #{error_parameter}" + end + end end diff --git a/modules/tftp/tftp_api.rb b/modules/tftp/tftp_api.rb index 1bc6276c7..4b1379126 100644 --- a/modules/tftp/tftp_api.rb +++ b/modules/tftp/tftp_api.rb @@ -34,6 +34,18 @@ def create_default(variant) end end + post "/fetch_system_image" do + log_halt(400, "TFTP: Wrong input parameters given.") unless [params[:path], params[:url], params[:files], params[:tftp_path]].all? + + begin + Proxy::TFTP.fetch_system_image(params[:path], params[:url], params[:files], params[:tftp_path]) # differentiates between 200 and 202 + rescue IOError + log_halt(423, "TFTP: Image download process is running") + rescue => error + log_halt(500, "TFTP: Failed to fetch system file: #{error.message}") + end + end + post "/fetch_boot_file" do log_halt(400, "TFTP: Failed to fetch boot file: ") { Proxy::TFTP.fetch_boot_file(params[:prefix], params[:path]) } end diff --git a/modules/tftp/tftp_plugin.rb b/modules/tftp/tftp_plugin.rb index d4dc06450..55a8bb690 100644 --- a/modules/tftp/tftp_plugin.rb +++ b/modules/tftp/tftp_plugin.rb @@ -4,11 +4,22 @@ class Plugin < ::Proxy::Plugin rackup_path File.expand_path("http_config.ru", __dir__) + load_programmable_settings do |settings| + settings[:http_port] = ::Proxy::Settings::Plugin.http_enabled?(settings[:enabled]) ? Proxy::SETTINGS.http_port : nil + settings + end + default_settings :tftproot => '/var/lib/tftpboot', :tftp_connect_timeout => 10, - :verify_server_cert => true + :verify_server_cert => true, + :enable_system_image => true, + :system_image_root => '/var/lib/foreman-proxy/tftp/system_images' validate :verify_server_cert, boolean: true + # Expose automatic iso handling capability + capability -> { settings[:enable_system_image] ? 'system_image' : nil } + expose_setting :tftp_servername + expose_setting :http_port end end diff --git a/modules/tftp/tftp_system_image_api.rb b/modules/tftp/tftp_system_image_api.rb new file mode 100644 index 000000000..310918d3e --- /dev/null +++ b/modules/tftp/tftp_system_image_api.rb @@ -0,0 +1,17 @@ +module Proxy::TFTP + class SystemImageApi < ::Sinatra::Base + helpers ::Proxy::Helpers + + get "/*" do + file = Pathname.new(params[:splat].first).cleanpath + root = Pathname.new(Proxy::TFTP::Plugin.settings.system_image_root).expand_path.cleanpath + joined_path = File.join(root, file) + log_halt(404, "Not found") unless File.exist?(joined_path) + real_file = Pathname.new(joined_path).realpath + log_halt(403, "Invalid or empty path") unless real_file.fnmatch?("#{root}/**") + log_halt(403, "Directory listing not allowed") if File.directory?(real_file) + log_halt(503, "Not a regular file") unless File.file?(real_file) + send_file real_file + end + end +end diff --git a/test/tftp/integration_test.rb b/test/tftp/integration_test.rb index 9a626beb9..5b2034999 100644 --- a/test/tftp/integration_test.rb +++ b/test/tftp/integration_test.rb @@ -12,7 +12,7 @@ def app end def test_features - Proxy::DefaultModuleLoader.any_instance.expects(:load_configuration_file).with('tftp.yml').returns(enabled: true, tftproot: '/var/lib/tftpboot', tftp_servername: 'tftp.example.com') + Proxy::DefaultModuleLoader.any_instance.expects(:load_configuration_file).with('tftp.yml').returns(enabled: true, tftproot: '/var/lib/tftpboot', tftp_servername: 'tftp.example.com', enable_system_image: false) get '/features' @@ -23,7 +23,23 @@ def test_features assert_equal('running', mod['state'], Proxy::LogBuffer::Buffer.instance.info[:failed_modules][:tftp]) assert_equal([], mod['capabilities']) - expected_settings = { 'tftp_servername' => 'tftp.example.com' } + expected_settings = { 'http_port' => 1234, 'tftp_servername' => 'tftp.example.com' } + assert_equal(expected_settings, mod['settings']) + end + + def test_features_with_enable_system_image + Proxy::DefaultModuleLoader.any_instance.expects(:load_configuration_file).with('tftp.yml').returns(enabled: true, tftproot: '/var/lib/tftpboot', tftp_servername: 'tftp.example.com', enable_system_image: true) + + get '/features' + + response = JSON.parse(last_response.body) + + mod = response['tftp'] + refute_nil(mod) + assert_equal('running', mod['state'], Proxy::LogBuffer::Buffer.instance.info[:failed_modules][:tftp]) + assert_equal(['system_image'], mod['capabilities']) + + expected_settings = { 'http_port' => 1234, 'tftp_servername' => 'tftp.example.com' } assert_equal(expected_settings, mod['settings']) end end diff --git a/test/tftp/tftp_api_test.rb b/test/tftp/tftp_api_test.rb index 3ffcfb0cb..e935f06b8 100644 --- a/test/tftp/tftp_api_test.rb +++ b/test/tftp/tftp_api_test.rb @@ -111,6 +111,12 @@ def test_api_can_fetch_boot_file assert last_response.ok? end + def test_api_can_fetch_system_image + Proxy::TFTP.expects(:fetch_system_image).with('some/image.iso', 'http://localhost/file.iso', ['dir/first', 'dir/second'], 'os/example-asdhf').returns(true) + post "/fetch_system_image", :path => 'some/image.iso', :url => 'http://localhost/file.iso', :files => ['dir/first', 'dir/second'], :tftp_path => 'os/example-asdhf' + assert last_response.ok? + end + def test_api_can_get_servername Proxy::TFTP::Plugin.settings.stubs(:tftp_servername).returns("servername") result = get "/serverName" diff --git a/test/tftp/tftp_system_image_api_test.rb b/test/tftp/tftp_system_image_api_test.rb new file mode 100644 index 000000000..eab832d75 --- /dev/null +++ b/test/tftp/tftp_system_image_api_test.rb @@ -0,0 +1,69 @@ +require 'test_helper' +require 'tempfile' +require 'tftp/tftp_plugin' +require 'tftp/tftp_system_image_api' + +ENV['RACK_ENV'] = 'test' + +class TftpBootImageApiTest < Test::Unit::TestCase + include Rack::Test::Methods + + def app + Proxy::TFTP::SystemImageApi.new + end + + def setup + @tempdir = Dir.mktmpdir 'tftpsystemimage-test' + @osdir = Dir.mktmpdir nil, @tempdir + @osdir_base = File.basename @osdir + FileUtils.touch "#{@osdir}/valid_file" + FileUtils.ln_s "#{@osdir}/valid_file", "#{@tempdir}/valid_symlink" + FileUtils.ln_s "#{@tempdir}/does_not_exist", "#{@tempdir}/invalid_symlink" + Proxy::TFTP::Plugin.load_test_settings(enable_system_image: true, system_image_root: @tempdir) + end + + def teardown + FileUtils.rm_rf(@tempdir) if @tempdir =~ /tftpsystemimage-test/ + end + + def test_valid_file + result = get "/#{@osdir_base}/valid_file" + assert_equal 200, last_response.status + assert_equal '', result.body + end + + def test_valid_dir + result = get "/#{@osdir_base}/" + assert_equal 403, last_response.status + assert_equal 'Directory listing not allowed', result.body + end + + def test_valid_symlink + result = get "/valid_symlink" + assert_equal 200, last_response.status + assert_equal '', result.body + end + + def test_invalid_symlink + result = get "/invalid_symlink" + assert_equal 404, last_response.status + assert_equal 'Not found', result.body + end + + def test_empty_path + result = get "/" + assert_equal 403, last_response.status + assert_equal 'Invalid or empty path', result.body + end + + def test_dangerous_symlink + another_dir = Dir.mktmpdir 'tftpsystemimage-test2' + FileUtils.touch "#{another_dir}/secure_file" + FileUtils.ln_s "#{another_dir}/secure_file", "#{@tempdir}/dangerous_symlink" + result = get "/dangerous_symlink" + assert_equal 403, last_response.status + assert_equal 'Invalid or empty path', result.body + ensure + FileUtils.rm_rf(another_dir) if another_dir =~ /tftpimageboot-test/ + end +end