Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #35270 - Enable boot image download #837

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion config/settings.d/tftp.yml.example
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
33 changes: 33 additions & 0 deletions lib/proxy/archive_extract.rb
Original file line number Diff line number Diff line change
@@ -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
29 changes: 28 additions & 1 deletion lib/proxy/util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/smart_proxy_for_testing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
1 change: 1 addition & 0 deletions lib/smart_proxy_main.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
5 changes: 5 additions & 0 deletions modules/tftp/http_config.ru
Original file line number Diff line number Diff line change
@@ -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
64 changes: 64 additions & 0 deletions modules/tftp/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
12 changes: 12 additions & 0 deletions modules/tftp/tftp_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 12 additions & 1 deletion modules/tftp/tftp_plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
17 changes: 17 additions & 0 deletions modules/tftp/tftp_system_image_api.rb
Original file line number Diff line number Diff line change
@@ -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
20 changes: 18 additions & 2 deletions test/tftp/integration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

class TftpApiFeaturesTest < SmartProxyRootApiTestCase
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'

Expand All @@ -16,7 +16,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
6 changes: 6 additions & 0 deletions test/tftp/tftp_api_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
69 changes: 69 additions & 0 deletions test/tftp/tftp_system_image_api_test.rb
Original file line number Diff line number Diff line change
@@ -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