From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id 371C91FF164
for <inbox@lore.proxmox.com>; Fri, 6 Dec 2024 14:25:37 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 336726440;
Fri, 6 Dec 2024 14:25:37 +0100 (CET)
From: Daniel Kral <d.kral@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Fri, 6 Dec 2024 14:24:55 +0100
Message-Id: <20241206132455.145355-1-d.kral@proxmox.com>
X-Mailer: git-send-email 2.39.5
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.004 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [proxmox-auto-installer.rs, proxmox.com, rust-lang.org, http.rs]
Subject: [pve-devel] [RFC PATCH installer] auto: allow a binary executable
as the first boot executable
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
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.kral@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