Fixes #39765 - TFTP archive extraction feature

config/settings.d/tftp.yml.example

@@ -10,6 +10,14 @@
 # like initrd and vmlinuz. Default value 10 seconds
 #:tftp_connect_timeout: 10
 
+# Maximum time in seconds for each TFTP curl transfer attempt. Curl retries
+# transient failures up to three times, and this timeout applies per attempt.
+#:tftp_download_max_time: 3600
+
 # Defines the default certificate action for certificate checking.
 # When false, the argument --no-check-certificate will be used.
 #:verify_server_cert: true
+
+# Perform an HTTP HEAD request before downloading TFTP artifacts.
+# Disable this for HTTP servers that do not support HEAD. Default value is true.
+#:tftp_http_download_preflight: true

extra/download_test.sh

@@ -0,0 +1,78 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+PROXY_URL="http://127.0.0.1:8000"
+
+# CentOS 10 Stream
+curl --fail --silent --show-error \
+  --header 'Content-Type: application/json' \
+  --data '{
+  "extract": {
+    "source": "https://mirror.stream.centos.org/10-stream/BaseOS/x86_64/os/images/boot.iso",
+    "destination": "bootloader-universe/pxegrub2/centos/10/x86_64/boot.iso",
+    "type": "iso",
+    "files": {
+      "bootloader-universe/pxegrub2/centos/10/x86_64/grubx64.efi": "EFI/BOOT/grubx64.efi",
+      "bootloader-universe/pxegrub2/centos/10/x86_64/shimx64.efi": "EFI/BOOT/BOOTX64.EFI"
+    },
+    "symlinks": {
+      "bootloader-universe/pxegrub2/centos/10/x86_64/boot.efi": "bootloader-universe/pxegrub2/centos/10/x86_64/grubx64.efi",
+      "bootloader-universe/pxegrub2/centos/10/x86_64/boot-sb.efi": "bootloader-universe/pxegrub2/centos/10/x86_64/shimx64.efi"
+    }
+  }
+}' \
+  "${PROXY_URL}/tftp/fetch_and_process"
+
+# Debian stable
+curl --fail --silent --show-error \
+  --header 'Content-Type: application/json' \
+  --data '{
+  "extract": {
+    "source": "https://deb.debian.org/debian/dists/bookworm/main/installer-amd64/current/images/netboot/netboot.tar.gz",
+    "destination": "bootloader-universe/pxegrub2/debian/bookworm/amd64/netboot.tar.gz",
+    "type": "tgz",
+    "files": {
+      "bootloader-universe/pxegrub2/debian/bookworm/amd64/linux": "debian-installer/amd64/linux",
+      "bootloader-universe/pxegrub2/debian/bookworm/amd64/initrd.gz": "debian-installer/amd64/initrd.gz",
+      "bootloader-universe/pxegrub2/debian/bookworm/amd64/grubx64.efi": "debian-installer/amd64/grubx64.efi",
+      "bootloader-universe/pxegrub2/debian/bookworm/amd64/shimx64.efi": "debian-installer/amd64/bootnetx64.efi"
+    },
+    "symlinks": {
+      "bootloader-universe/pxegrub2/debian/bookworm/amd64/boot.efi": "bootloader-universe/pxegrub2/debian/bookworm/amd64/grubx64.efi",
+      "bootloader-universe/pxegrub2/debian/bookworm/amd64/boot-sb.efi": "bootloader-universe/pxegrub2/debian/bookworm/amd64/shimx64.efi"
+    }
+  }
+}' \
+  "${PROXY_URL}/tftp/fetch_and_process"
+
+# Ubuntu stable
+curl --fail --silent --show-error \
+  --header 'Content-Type: application/json' \
+  --data '{
+  "extract": {
+    "source": "https://releases.ubuntu.com/26.04/ubuntu-26.04-netboot-amd64.tar.gz",
+    "destination": "bootloader-universe/pxegrub2/ubuntu/26.04/amd64/netboot.tar.gz",
+    "type": "tgz",
+    "files": {
+      "bootloader-universe/pxegrub2/ubuntu/26.04/amd64/linux": "amd64/linux",
+      "bootloader-universe/pxegrub2/ubuntu/26.04/amd64/initrd.gz": "amd64/initrd",
+      "bootloader-universe/pxegrub2/ubuntu/26.04/amd64/grubx64.efi": "amd64/grubx64.efi",
+      "bootloader-universe/pxegrub2/ubuntu/26.04/amd64/shimx64.efi": "amd64/bootx64.efi"
+    },
+    "symlinks": {
+      "bootloader-universe/pxegrub2/ubuntu/26.04/amd64/boot.efi": "bootloader-universe/pxegrub2/ubuntu/26.04/amd64/grubx64.efi",
+      "bootloader-universe/pxegrub2/ubuntu/26.04/amd64/boot-sb.efi": "bootloader-universe/pxegrub2/ubuntu/26.04/amd64/shimx64.efi"
+    }
+  }
+}' \
+  "${PROXY_URL}/tftp/fetch_and_process"
+
+# Ubuntu ISO is also needed
+curl --fail --silent --show-error \
+  --header 'Content-Type: application/json' \
+  --data '{
+  "source": "https://releases.ubuntu.com/26.04/ubuntu-26.04-live-server-amd64.iso",
+  "destination": "bootloader-universe/pxegrub2/ubuntu/26.04/amd64/boot.iso"
+}' \
+  "${PROXY_URL}/tftp/fetch_and_process"

lib/proxy/http_download.rb

@@ -1,11 +1,27 @@
 require 'proxy/file_lock'
+require 'net/http'
+require 'openssl'
+require 'uri'
 
 module Proxy
   class HttpDownload < Proxy::Util::CommandTask
     include Util
     DEFAULT_CONNECT_TIMEOUT = 10
+    DEFAULT_MAX_TIME = 3600
+    MAX_PREFLIGHT_REDIRECTS = 5
 
-    def initialize(src, dst, connect_timeout: DEFAULT_CONNECT_TIMEOUT, verify_server_cert: false)
+    class PreflightError < StandardError
+      attr_reader :status_code
+
+      def initialize(message, status_code: 502)
+        @status_code = status_code
+        super(message)
+      end
+    end
+
+    def initialize(src, dst, connect_timeout: DEFAULT_CONNECT_TIMEOUT, max_time: DEFAULT_MAX_TIME,
+                   verify_server_cert: false, preflight: false, conditional: true)
+      @preflight = [src, connect_timeout, verify_server_cert] if preflight
       @dst = dst
       args = [which('curl')]
 
@@ -23,12 +39,12 @@ def initialize(src, dst, connect_timeout: DEFAULT_CONNECT_TIMEOUT, verify_server
       args += ["--retry", "3"]
       # with a short delay
       args += ["--retry-delay", "10"]
-      # but exit after one hour
-      args += ["--max-time", "3600"]
+      # Limit each transfer attempt; curl resets this limit when retrying.
+      args += ["--max-time", max_time.to_s]
       # keep last changed file attribute
       args << "--remote-time"
-      # only download newer files
-      args += ["--time-cond", dst.to_s]
+      # only download newer files when the destination's source is unchanged
+      args += ["--time-cond", dst.to_s] if conditional
       # print stats in the end
       args += [
         "--write-out",
@@ -44,16 +60,90 @@ def initialize(src, dst, connect_timeout: DEFAULT_CONNECT_TIMEOUT, verify_server
       super(args)
     end
 
+    # curl follows redirects for the download. Follow the same HTTP(S) chain
+    # here so a successful preflight describes the resource curl will request.
+    def preflight_http(src, connect_timeout, verify_server_cert)
+      uri = URI.parse(src.to_s)
+      return unless %w[http https].include?(uri.scheme)
+
+      (MAX_PREFLIGHT_REDIRECTS + 1).times do
+        if uri.host.nil?
+          raise PreflightError.new("Invalid HTTP download URL: #{uri}", status_code: 400)
+        end
+
+        response = preflight_request(uri, connect_timeout, verify_server_cert)
+        return if response.is_a?(Net::HTTPSuccess)
+
+        unless response.is_a?(Net::HTTPRedirection)
+          begin
+            response.value
+          rescue Net::HTTPError => e
+            raise PreflightError, "HTTP HEAD preflight failed for #{uri}: #{response.code} #{response.message}", cause: e
+          end
+        end
+
+        location = response['location']
+        if location.nil? || location.empty?
+          raise PreflightError, "HTTP HEAD preflight redirect (#{response.code} #{response.message}) has no location: #{uri}"
+        end
+
+        begin
+          uri = URI.join(uri.to_s, location)
+        rescue URI::InvalidURIError => e
+          raise PreflightError, "HTTP HEAD preflight failed for redirect from #{uri}: #{e.message}", cause: e
+        end
+        unless %w[http https].include?(uri.scheme)
+          raise PreflightError, "HTTP HEAD preflight redirect (#{response.code} #{response.message}) uses unsupported protocol: #{uri.scheme}"
+        end
+      end
+
+      raise PreflightError, "HTTP HEAD preflight exceeded #{MAX_PREFLIGHT_REDIRECTS} redirects for #{src}"
+    rescue URI::InvalidURIError => e
+      raise PreflightError.new("HTTP HEAD preflight failed for #{src}: #{e.message}", status_code: 400), cause: e
+    rescue SocketError, SystemCallError, Timeout::Error, IOError, OpenSSL::SSL::SSLError, Net::HTTPBadResponse => e
+      status_code = e.is_a?(Timeout::Error) ? 504 : 502
+      raise PreflightError.new("HTTP HEAD preflight failed for #{uri || src}: #{e.message}", status_code: status_code), cause: e
+    end
+
+    def preflight_request(uri, connect_timeout, verify_server_cert)
+      proxy = uri.find_proxy
+      http = if proxy
+               Net::HTTP.new(uri.host, uri.port, proxy.host, proxy.port, proxy.user, proxy.password)
+             else
+               Net::HTTP.new(uri.host, uri.port, nil)
+             end
+      http.use_ssl = uri.scheme == 'https'
+      http.open_timeout = connect_timeout
+      http.read_timeout = connect_timeout
+      http.verify_mode = OpenSSL::SSL::VERIFY_NONE unless verify_server_cert
+      http.start { |connection| connection.head(uri.request_uri) }
+    end
+
+    private :preflight_http, :preflight_request
+
     def start
       lock = Proxy::FileLock.try_locking(File.join(File.dirname(@dst), ".#{File.basename(@dst)}.lock"))
       if lock.nil?
         false
       else
-        super do
+        super(before_start: method(:run_preflight)) do
           Proxy::FileLock.unlock(lock)
-          File.unlink(lock)
+        ensure
+          yield if block_given?
         end
       end
     end
+
+    private
+
+    def run_preflight
+      return true unless @preflight
+
+      preflight_http(*@preflight)
+      true
+    rescue StandardError => e
+      logger.error "HTTP HEAD preflight failed for #{@preflight.first}: #{e.message}"
+      false
+    end
   end
 end

lib/proxy/util.rb

@@ -18,25 +18,33 @@ def initialize(command, input = nil)
       @input = input
     end
 
-    def start(&ensured_block)
+    def start(before_start: nil, &ensured_block)
       # run the task in its own thread
-      @task = Thread.new(@command, @input) do |cmd, input|
+      @task = Thread.new(@command, @input, before_start) do |cmd, input, task_before_start|
         status = nil
-        Open3.popen3(*cmd) do |stdin, stdout, stderr, thr|
-          cmdline_string = Shellwords.escape(cmd.is_a?(Array) ? cmd.join(' ') : cmd)
-          logger.info "[#{thr.pid}] Started task #{cmdline_string}"
-          stdin.write(input) if input
-          stdin.close
-          stdout.each do |line|
-            logger.debug "[#{thr.pid}] #{line}"
+        unless task_before_start && task_before_start.call == false
+          Open3.popen3(*cmd) do |stdin, stdout, stderr, thr|
+            cmdline_string = Shellwords.escape(cmd.is_a?(Array) ? cmd.join(' ') : cmd)
+            logger.info "[#{thr.pid}] Started task #{cmdline_string}"
+            stdin.write(input) if input
+            stdin.close
+            stdout.each do |line|
+              logger.debug "[#{thr.pid}] #{line}"
+            end
+            stderr.each do |line|
+              logger.warn "[#{thr.pid}] #{line}"
+            end
+            # call thr.value to wait for a Process::Status object.
+            status = thr.value
           end
-          stderr.each do |line|
-            logger.warn "[#{thr.pid}] #{line}"
-          end
-          # call thr.value to wait for a Process::Status object.
-          status = thr.value
         end
-        status ? status.exitstatus : $CHILD_STATUS
+        if status
+          status.exitstatus
+        elsif task_before_start
+          1
+        else
+          $CHILD_STATUS
+        end
       ensure
         yield if block_given?
       end

modules/tftp/dir_lock.rb

@@ -0,0 +1,112 @@
+module Proxy::TFTP
+  class BootFilesInProgress < StandardError
+    def status_code
+      409
+    end
+  end
+
+  # Coordinates writes and reads of TFTP paths so conflicting operations can
+  # fail fast instead of waiting on work in progress.
+  class DirLock
+    def initialize
+      @mutex = Mutex.new
+      @active = {}
+    end
+
+    # Call defer_release when handing the reservation to async work. That work
+    # must call release on completion. Otherwise, release happens on block exit.
+    def with_write(paths)
+      registered = false
+      deferred = false
+      released = false
+      release_mutex = Mutex.new
+      release = proc do
+        should_release = release_mutex.synchronize do
+          next false if released
+          released = true
+        end
+        release_write(paths) if should_release
+      end
+      defer_release = proc { deferred = true }
+
+      paths = reserve_write(paths)
+      registered = true
+      yield release, defer_release
+    ensure
+      release.call if registered && !deferred
+    end
+
+    def with_read(paths)
+      reserved_paths = reserve_read(paths)
+      yield
+    ensure
+      release_read(reserved_paths) if reserved_paths
+    end
+
+    def active?(path)
+      @mutex.synchronize do
+        @active.any? { |active_path, entry| entry[:writer] && paths_conflict?(active_path, path.to_s) }
+      end
+    end
+
+    private
+
+    def reserve_write(paths)
+      paths = normalize_paths(paths)
+      @mutex.synchronize do
+        busy_path = paths.find do |path|
+          @active.keys.find { |active_path| paths_conflict?(active_path, path) }
+        end
+        raise BootFilesInProgress, "TFTP files are already being updated: #{busy_path}" if busy_path
+
+        paths.each { |path| @active[path] = { writer: true, readers: 0 } }
+      end
+      paths
+    end
+
+    def release_write(paths)
+      @mutex.synchronize do
+        paths.each do |path|
+          entry = @active[path.to_s]
+          @active.delete(path.to_s) if entry && entry[:writer]
+        end
+      end
+    end
+
+    def reserve_read(paths)
+      paths = normalize_paths(paths)
+      @mutex.synchronize do
+        busy_path = paths.find do |path|
+          @active.any? { |active_path, entry| entry[:writer] && paths_conflict?(active_path, path) }
+        end
+        raise BootFilesInProgress, "Bootloader universe files are currently being downloaded or extracted: #{busy_path}" if busy_path
+
+        paths.each do |path|
+          entry = @active[path] ||= { writer: false, readers: 0 }
+          entry[:readers] += 1
+        end
+      end
+      paths
+    end
+
+    def release_read(paths)
+      @mutex.synchronize do
+        paths.each do |path|
+          entry = @active[path.to_s]
+          next unless entry && entry[:readers].positive?
+
+          entry[:readers] -= 1
+          @active.delete(path.to_s) if entry[:readers].zero? && !entry[:writer]
+        end
+      end
+    end
+
+    def normalize_paths(paths)
+      paths.map(&:to_s).uniq
+    end
+
+    def paths_conflict?(first, second)
+      first == second || first.start_with?("#{second}#{File::SEPARATOR}") || second.start_with?("#{first}#{File::SEPARATOR}")
+    end
+  end
+end