As the initial use case for the first boot feature request [0] was for running shell scripts, the auto installer retrieved the binary as a `String`. Unfortunately, this tries to interpret binary data as UTF-8 and will transform 'invalid' characters to replacement characters [1].
This causes the auto-installer to create an invalid binary when fetching from an URL, which will likely cause a segmentation fault and the binary to be never removed by the systemd target, and error with a `stream did not contain valid UTF-8` when fetching from the ISO image. To allow binary executables to be used as a first boot executable, this commit changes the fetching from being read as a `String` to being read as a vector of bytes `Vec<u8>` to not interfere with the content of a binary executable. [0] https://bugzilla.proxmox.com/show_bug.cgi?id=5579 [1] https://doc.rust-lang.org/src/alloc/string.rs.html#635 Signed-off-by: Daniel Kral <d.k...@proxmox.com> --- To verify the former, I've created two simple test executables to verify that both were executed as expected: ``` #include <syslog.h> int main() { int logprio = LOG_USER | LOG_INFO; syslog(logprio, "Hello from first-boot!"); return 0; } ``` ``` #!/bin/bash logger <<EOF Hello from the first-boot script! EOF ``` As described in the commit message, with an unpatched auto-installer, the binary will fail to run when fetched from an URL or fail the installation when fetched from the ISO image. The shell script works as expected. After applying the patch when booting the install ISO in debug mode, the auto-installer correctly writes and execute the binary when fetched from an URL (both HTTP and HTTPS) and the same works when fetched from ISO. I've also tested two common error scenarios: When the HTTP is offline, there's an human-readable error that the connection was refused (e.g. host is online, cannot establish a connection). When the HTTP response is missing the "Content-Length" header, it will be explicitly stated in the error message. .../src/bin/proxmox-auto-installer.rs | 12 ++++++--- proxmox-installer-common/src/http.rs | 25 +++++++++++++++---- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs index ece9a94..408fc0e 100644 --- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs +++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs @@ -38,14 +38,18 @@ fn setup_first_boot_executable(first_boot: &FirstBootHookInfo) -> Result<()> { FirstBootHookSourceMode::FromUrl => { if let Some(url) = &first_boot.url { info!("Fetching first-boot hook from {url} .."); - Some(http::get(url, first_boot.cert_fingerprint.as_deref())?) + Some(http::get_as_bytes( + url, + first_boot.cert_fingerprint.as_deref(), + FIRST_BOOT_EXEC_MAX_SIZE, + )?) } else { bail!("first-boot hook source set to URL, but none specified!"); } } - FirstBootHookSourceMode::FromIso => Some(fs::read_to_string(format!( - "/cdrom/{FIRST_BOOT_EXEC_NAME}" - ))?), + FirstBootHookSourceMode::FromIso => { + Some(fs::read(format!("/cdrom/{FIRST_BOOT_EXEC_NAME}"))?) + } }; if let Some(content) = content { diff --git a/proxmox-installer-common/src/http.rs b/proxmox-installer-common/src/http.rs index f4afe14..a748266 100644 --- a/proxmox-installer-common/src/http.rs +++ b/proxmox-installer-common/src/http.rs @@ -1,6 +1,7 @@ -use anyhow::Result; +use anyhow::{bail, Result}; use rustls::ClientConfig; use sha2::{Digest, Sha256}; +use std::io::Read; use std::sync::Arc; use ureq::{Agent, AgentBuilder}; @@ -53,12 +54,26 @@ fn build_agent(fingerprint: Option<&str>) -> Result<Agent> { /// # Arguments /// * `url` - URL to fetch /// * `fingerprint` - SHA256 cert fingerprint if certificate pinning should be used. Optional. -pub fn get(url: &str, fingerprint: Option<&str>) -> Result<String> { - Ok(build_agent(fingerprint)? +/// * `max_size` - Maximum amount of bytes that will be read. +pub fn get_as_bytes(url: &str, fingerprint: Option<&str>, max_size: usize) -> Result<Vec<u8>> { + let mut result: Vec<u8> = Vec::new(); + + let response = build_agent(fingerprint)? .get(url) .timeout(std::time::Duration::from_secs(60)) - .call()? - .into_string()?) + .call()?; + + match response + .into_reader() + .take(max_size as u64) + .read_to_end(&mut result) + { + Err(ref err) if err.kind() == std::io::ErrorKind::UnexpectedEof => { + bail!("Unexpected end of line. Does the HTTP server provide a Content-Length header?") + } + Err(err) => bail!("Error while reading GET request: {err}"), + Ok(_) => Ok(result), + } } /// Issues a POST request with the payload (JSON). Optionally a SHA256 fingerprint can be used to -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
