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 <[email protected]>

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

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

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

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'

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'

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

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

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

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

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

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

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"

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