Skip to content

Commit 63f27ff

Browse files
committed
fix(vm): decompress gzip rootfs tar archives during staging
`--from` accepts `.tar.gz` and `.tgz`, but the driver staged whatever bytes it was given and the guest image-prep VM extracts the staged file with a plain `tar -xpf`. Compressed sources therefore depended on the guest tar auto-detecting gzip, and the prepared disk was sized from the compressed length, which is far too small for the expanded rootfs. Staging now detects gzip from the archive's magic bytes -- the driver only ever sees a gateway-issued staging path, never the caller's file name -- and writes an uncompressed tar. The digest still covers the source bytes, so the "archive changed while staging" check is unaffected, and expansion is bounded by `rootfs_tar_max_bytes` so a compression bomb cannot fill the host disk. `extract_rootfs_archive_to` sniffs gzip as well, so the host-side extraction path matches. Adds unit coverage for gzip staging, bounded expansion, and gzip extraction, plus an e2e sandbox created from a gzip-compressed export. Signed-off-by: Philippe Martin <phmartin@redhat.com>
1 parent 7b8be28 commit 63f27ff

5 files changed

Lines changed: 308 additions & 64 deletions

File tree

crates/openshell-driver-vm/src/driver.rs

Lines changed: 182 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use bollard::Docker;
1818
use bollard::errors::Error as BollardError;
1919
use bollard::models::ContainerCreateBody;
2020
use bollard::query_parameters::{CreateContainerOptionsBuilder, RemoveContainerOptionsBuilder};
21-
use flate2::read::GzDecoder;
21+
use flate2::read::{GzDecoder, MultiGzDecoder};
2222
use futures::{Stream, StreamExt, TryStreamExt};
2323
use nix::errno::Errno;
2424
use nix::sys::signal::{Signal, kill};
@@ -61,7 +61,7 @@ use sha2::{Digest, Sha256};
6161
use std::collections::{HashMap, HashSet};
6262
use std::fs;
6363
use std::future::Future;
64-
use std::io::{Read, Write};
64+
use std::io::{BufRead, BufReader, BufWriter, Read, Write};
6565
use std::net::{IpAddr, Ipv4Addr};
6666
#[cfg(unix)]
6767
use std::os::unix::fs::PermissionsExt;
@@ -2961,26 +2961,28 @@ impl VmDriver {
29612961
);
29622962
let copy_src = tar_path.to_path_buf();
29632963
let copy_dst = rootfs_archive.clone();
2964-
let copied_digest =
2965-
match tokio::task::spawn_blocking(move || copy_file_sha256_hex(&copy_src, &copy_dst))
2966-
.await
2967-
{
2968-
Ok(Ok(digest)) => digest,
2969-
Ok(Err(err)) => {
2970-
let _ = tokio::fs::remove_dir_all(&staging_dir).await;
2971-
cleanup_request_staging().await;
2972-
return Err(Status::internal(format!(
2973-
"failed to copy rootfs tar to staging: {err}"
2974-
)));
2975-
}
2976-
Err(err) => {
2977-
let _ = tokio::fs::remove_dir_all(&staging_dir).await;
2978-
cleanup_request_staging().await;
2979-
return Err(Status::internal(format!(
2980-
"failed to copy rootfs tar to staging: {err}"
2981-
)));
2982-
}
2983-
};
2964+
let max_bytes = self.config.rootfs_tar_max_bytes();
2965+
let copied_digest = match tokio::task::spawn_blocking(move || {
2966+
stage_rootfs_tar_archive(&copy_src, &copy_dst, max_bytes)
2967+
})
2968+
.await
2969+
{
2970+
Ok(Ok(digest)) => digest,
2971+
Ok(Err(err)) => {
2972+
let _ = tokio::fs::remove_dir_all(&staging_dir).await;
2973+
cleanup_request_staging().await;
2974+
return Err(Status::internal(format!(
2975+
"failed to copy rootfs tar to staging: {err}"
2976+
)));
2977+
}
2978+
Err(err) => {
2979+
let _ = tokio::fs::remove_dir_all(&staging_dir).await;
2980+
cleanup_request_staging().await;
2981+
return Err(Status::internal(format!(
2982+
"failed to copy rootfs tar to staging: {err}"
2983+
)));
2984+
}
2985+
};
29842986

29852987
// The archive changed between the hash pass and the copy: the prepared
29862988
// disk we are about to build would not match the identity it is cached
@@ -4814,33 +4816,100 @@ fn compute_bytes_sha256_hex(bytes: &[u8]) -> String {
48144816
format!("{:x}", hasher.finalize())
48154817
}
48164818

4817-
/// Copy `src` to `dst` and return the SHA-256 of the bytes actually written.
4819+
/// Stage the caller-supplied rootfs archive at `src` into the image cache at
4820+
/// `dst`, and return the SHA-256 of the source bytes that were read.
48184821
///
4819-
/// Hashing the copy rather than re-reading the source is what lets the caller
4820-
/// detect an archive that changed underneath it during staging: the digest
4821-
/// describes exactly the bytes that landed in the image cache.
4822-
fn copy_file_sha256_hex(src: &Path, dst: &Path) -> Result<String, String> {
4823-
let mut reader = fs::File::open(src).map_err(|err| format!("open {}: {err}", src.display()))?;
4824-
let mut writer =
4825-
fs::File::create(dst).map_err(|err| format!("create {}: {err}", dst.display()))?;
4826-
let mut hasher = Sha256::new();
4822+
/// The staged file is always an uncompressed tar. `--from` accepts `.tar.gz`
4823+
/// and `.tgz`, but the guest image-prep VM extracts the staged file with a
4824+
/// plain `tar -xpf`, and the prepared disk is sized from that file's length,
4825+
/// so leaving gzip bytes on disk would both depend on the guest tar
4826+
/// auto-detecting compression and size the disk from the compressed length.
4827+
/// Compression is detected from the magic bytes: the driver only ever sees a
4828+
/// gateway-issued staging path, never the caller's file name.
4829+
///
4830+
/// Expansion is bounded by `max_bytes` — the same limit the driver applies to
4831+
/// the archive it accepts — so a compression bomb cannot fill the host disk.
4832+
///
4833+
/// The digest covers the source bytes rather than the bytes written, which is
4834+
/// what lets the caller detect an archive that changed underneath it during
4835+
/// staging: it stays comparable with the pre-copy hash pass whether or not the
4836+
/// source was compressed.
4837+
fn stage_rootfs_tar_archive(src: &Path, dst: &Path, max_bytes: u64) -> Result<String, String> {
4838+
let file = fs::File::open(src).map_err(|err| format!("open {}: {err}", src.display()))?;
4839+
let mut reader = BufReader::new(file);
4840+
let compressed = reader
4841+
.fill_buf()
4842+
.map_err(|err| format!("read {}: {err}", src.display()))?
4843+
.starts_with(&crate::rootfs::GZIP_MAGIC);
4844+
4845+
let mut source = HashingReader::new(reader);
4846+
if compressed {
4847+
write_stream_to_file(MultiGzDecoder::new(&mut source), dst, max_bytes)?;
4848+
} else {
4849+
write_stream_to_file(&mut source, dst, max_bytes)?;
4850+
}
4851+
4852+
// A decoder stops at the end of the compressed stream, so drain whatever
4853+
// it left behind: the digest has to describe the whole source file for the
4854+
// caller's change-detection comparison to mean anything.
4855+
std::io::copy(&mut source, &mut std::io::sink())
4856+
.map_err(|err| format!("read {}: {err}", src.display()))?;
4857+
Ok(source.finish())
4858+
}
4859+
4860+
/// Reader adapter that digests every byte it yields.
4861+
struct HashingReader<R> {
4862+
inner: R,
4863+
hasher: Sha256,
4864+
}
4865+
4866+
impl<R: Read> HashingReader<R> {
4867+
fn new(inner: R) -> Self {
4868+
Self {
4869+
inner,
4870+
hasher: Sha256::new(),
4871+
}
4872+
}
4873+
4874+
fn finish(self) -> String {
4875+
format!("{:x}", self.hasher.finalize())
4876+
}
4877+
}
4878+
4879+
impl<R: Read> Read for HashingReader<R> {
4880+
fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
4881+
let read = self.inner.read(buf)?;
4882+
self.hasher.update(&buf[..read]);
4883+
Ok(read)
4884+
}
4885+
}
4886+
4887+
fn write_stream_to_file(mut reader: impl Read, dst: &Path, max_bytes: u64) -> Result<(), String> {
4888+
let mut writer = BufWriter::new(
4889+
fs::File::create(dst).map_err(|err| format!("create {}: {err}", dst.display()))?,
4890+
);
48274891
let mut buffer = vec![0_u8; 64 * 1024].into_boxed_slice();
4892+
let mut written = 0_u64;
48284893
loop {
48294894
let read = reader
48304895
.read(&mut buffer)
4831-
.map_err(|err| format!("read {}: {err}", src.display()))?;
4896+
.map_err(|err| format!("read rootfs tar: {err}"))?;
48324897
if read == 0 {
48334898
break;
48344899
}
4835-
hasher.update(&buffer[..read]);
4900+
written = written.saturating_add(u64::try_from(read).unwrap_or(u64::MAX));
4901+
if written > max_bytes {
4902+
return Err(format!(
4903+
"rootfs tar expands to more than the {max_bytes} byte limit"
4904+
));
4905+
}
48364906
writer
48374907
.write_all(&buffer[..read])
48384908
.map_err(|err| format!("write {}: {err}", dst.display()))?;
48394909
}
48404910
writer
48414911
.flush()
4842-
.map_err(|err| format!("flush {}: {err}", dst.display()))?;
4843-
Ok(format!("{:x}", hasher.finalize()))
4912+
.map_err(|err| format!("flush {}: {err}", dst.display()))
48444913
}
48454914

48464915
/// Cache identity for a rootfs tar archive, derived from its contents.
@@ -9191,16 +9260,38 @@ mod tests {
91919260
);
91929261
}
91939262

9263+
const TEST_STAGING_LIMIT: u64 = 10 * 1024 * 1024;
9264+
9265+
/// Build an uncompressed tar holding a single file.
9266+
fn tar_bytes_with_file(name: &str, contents: &[u8]) -> Vec<u8> {
9267+
let mut builder = tar::Builder::new(Vec::new());
9268+
let mut header = tar::Header::new_gnu();
9269+
header.set_size(u64::try_from(contents.len()).expect("tar entry size fits u64"));
9270+
header.set_mode(0o644);
9271+
header.set_cksum();
9272+
builder
9273+
.append_data(&mut header, name, contents)
9274+
.expect("append tar entry");
9275+
builder.into_inner().expect("finish tar")
9276+
}
9277+
9278+
fn gzip_bytes(bytes: &[u8]) -> Vec<u8> {
9279+
let mut encoder = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default());
9280+
encoder.write_all(bytes).expect("gzip payload");
9281+
encoder.finish().expect("finish gzip")
9282+
}
9283+
91949284
#[test]
9195-
fn copy_file_sha256_hex_matches_source_digest() {
9285+
fn stage_rootfs_tar_archive_matches_source_digest() {
91969286
let base = unique_temp_dir();
91979287
std::fs::create_dir_all(&base).expect("create base dir");
91989288
let src = base.join("src.tar");
91999289
let dst = base.join("dst.tar");
92009290
let payload = vec![3_u8; 200 * 1024];
92019291
std::fs::write(&src, &payload).expect("write source");
92029292

9203-
let copied = copy_file_sha256_hex(&src, &dst).expect("copy should succeed");
9293+
let copied =
9294+
stage_rootfs_tar_archive(&src, &dst, TEST_STAGING_LIMIT).expect("copy should succeed");
92049295

92059296
assert_eq!(copied, compute_file_sha256_hex(&src).expect("hash source"));
92069297
assert_eq!(copied, compute_bytes_sha256_hex(&payload));
@@ -9212,15 +9303,16 @@ mod tests {
92129303
/// different digest, which is what lets the caller reject it instead of
92139304
/// caching a disk under an identity that does not describe it.
92149305
#[test]
9215-
fn copy_file_sha256_hex_detects_content_change_between_passes() {
9306+
fn stage_rootfs_tar_archive_detects_content_change_between_passes() {
92169307
let base = unique_temp_dir();
92179308
std::fs::create_dir_all(&base).expect("create base dir");
92189309
let src = base.join("src.tar");
92199310
std::fs::write(&src, b"original").expect("write source");
92209311
let first = compute_file_sha256_hex(&src).expect("hash source");
92219312

92229313
std::fs::write(&src, b"replaced").expect("rewrite source");
9223-
let second = copy_file_sha256_hex(&src, &base.join("dst.tar")).expect("copy");
9314+
let second = stage_rootfs_tar_archive(&src, &base.join("dst.tar"), TEST_STAGING_LIMIT)
9315+
.expect("copy");
92249316

92259317
assert_ne!(
92269318
first, second,
@@ -9229,6 +9321,57 @@ mod tests {
92299321
let _ = std::fs::remove_dir_all(&base);
92309322
}
92319323

9324+
/// `--from` accepts `.tar.gz`/`.tgz`, and the guest extracts the staged
9325+
/// file as a plain tar, so staging has to decompress on the way in.
9326+
#[test]
9327+
fn stage_rootfs_tar_archive_decompresses_gzip_sources() {
9328+
let base = unique_temp_dir();
9329+
std::fs::create_dir_all(&base).expect("create base dir");
9330+
let tar = tar_bytes_with_file("etc/marker.txt", b"rootfs-tar-gzip\n");
9331+
let gzipped = gzip_bytes(&tar);
9332+
let src = base.join("src.tar.gz");
9333+
let dst = base.join("source-rootfs.tar");
9334+
std::fs::write(&src, &gzipped).expect("write source");
9335+
9336+
let digest =
9337+
stage_rootfs_tar_archive(&src, &dst, TEST_STAGING_LIMIT).expect("stage gzip archive");
9338+
9339+
assert_eq!(
9340+
digest,
9341+
compute_bytes_sha256_hex(&gzipped),
9342+
"the digest must cover the whole compressed source"
9343+
);
9344+
assert_eq!(
9345+
std::fs::read(&dst).expect("read staged archive"),
9346+
tar,
9347+
"the staged archive must be an uncompressed tar"
9348+
);
9349+
9350+
let extracted = base.join("extracted");
9351+
extract_rootfs_archive_to(&dst, &extracted).expect("extract staged archive");
9352+
assert_eq!(
9353+
std::fs::read_to_string(extracted.join("etc/marker.txt")).expect("read marker"),
9354+
"rootfs-tar-gzip\n"
9355+
);
9356+
let _ = std::fs::remove_dir_all(&base);
9357+
}
9358+
9359+
/// The configured limit bounds what the driver writes, not just what it
9360+
/// accepts, so a highly compressible archive cannot fill the host disk.
9361+
#[test]
9362+
fn stage_rootfs_tar_archive_rejects_oversized_expansion() {
9363+
let base = unique_temp_dir();
9364+
std::fs::create_dir_all(&base).expect("create base dir");
9365+
let src = base.join("bomb.tar.gz");
9366+
std::fs::write(&src, gzip_bytes(&vec![0_u8; 4 * 1024 * 1024])).expect("write source");
9367+
9368+
let err = stage_rootfs_tar_archive(&src, &base.join("dst.tar"), 64 * 1024)
9369+
.expect_err("expansion beyond the limit must be rejected");
9370+
9371+
assert!(err.contains("65536"), "unexpected error: {err}");
9372+
let _ = std::fs::remove_dir_all(&base);
9373+
}
9374+
92329375
fn test_driver_with_extensions(extensions: LifecycleExtensionRegistry) -> VmDriver {
92339376
let (events, _) = broadcast::channel(WATCH_BUFFER);
92349377
VmDriver {

crates/openshell-driver-vm/src/rootfs.rs

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,22 @@
11
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4+
use flate2::read::MultiGzDecoder;
45
use std::fs;
56
use std::fs::File;
67
#[cfg(test)]
78
use std::io::BufWriter;
8-
use std::io::{Cursor, Read, Seek, SeekFrom, Write};
9+
use std::io::{BufRead, BufReader, Cursor, Read, Seek, SeekFrom, Write};
910
use std::path::{Path, PathBuf};
1011
use std::process::Command;
1112
use std::sync::atomic::{AtomicU64, Ordering};
1213

1314
const SUPERVISOR: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/openshell-sandbox.zst"));
1415
const UMOCI: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/umoci.zst"));
1516
const ROOTFS_VARIANT_MARKER: &str = ".openshell-rootfs-variant";
17+
/// Leading bytes of a gzip stream, used to recognize `.tar.gz`/`.tgz` input
18+
/// without trusting the file name.
19+
pub const GZIP_MAGIC: [u8; 2] = [0x1f, 0x8b];
1620
const SANDBOX_GUEST_INIT_PATH: &str = "/srv/openshell-vm-sandbox-init.sh";
1721
const SANDBOX_SUPERVISOR_PATH: &str = openshell_core::driver_utils::SUPERVISOR_CONTAINER_BINARY;
1822
const SANDBOX_UMOCI_PATH: &str = openshell_core::container_paths::VM_UMOCI_PATH;
@@ -44,6 +48,12 @@ pub fn prepare_sandbox_rootfs_from_image_root(
4448
Ok(())
4549
}
4650

51+
/// Extract a rootfs tarball, transparently decompressing gzip archives.
52+
///
53+
/// Compression is detected from the magic bytes rather than the file name:
54+
/// `--from` accepts `.tar.gz` and `.tgz`, but nothing guarantees a caller's
55+
/// extension matches the bytes, and the archives this crate stages internally
56+
/// carry no extension at all.
4757
pub fn extract_rootfs_archive_to(archive_path: &Path, dest: &Path) -> Result<(), String> {
4858
if dest.exists() {
4959
fs::remove_dir_all(dest)
@@ -53,8 +63,20 @@ pub fn extract_rootfs_archive_to(archive_path: &Path, dest: &Path) -> Result<(),
5363
fs::create_dir_all(dest).map_err(|e| format!("create rootfs dir {}: {e}", dest.display()))?;
5464
let file =
5565
File::open(archive_path).map_err(|e| format!("open {}: {e}", archive_path.display()))?;
56-
let mut archive = tar::Archive::new(file);
57-
archive
66+
let mut reader = BufReader::new(file);
67+
let compressed = reader
68+
.fill_buf()
69+
.map_err(|e| format!("read {}: {e}", archive_path.display()))?
70+
.starts_with(&GZIP_MAGIC);
71+
if compressed {
72+
unpack_tar_reader(MultiGzDecoder::new(reader), dest)
73+
} else {
74+
unpack_tar_reader(reader, dest)
75+
}
76+
}
77+
78+
fn unpack_tar_reader(reader: impl Read, dest: &Path) -> Result<(), String> {
79+
tar::Archive::new(reader)
5880
.unpack(dest)
5981
.map_err(|e| format!("extract rootfs tarball into {}: {e}", dest.display()))
6082
}
@@ -1031,6 +1053,36 @@ mod tests {
10311053
let _ = fs::remove_dir_all(&dir);
10321054
}
10331055

1056+
/// `--from` accepts `.tar.gz` and `.tgz`, so extraction must recognize a
1057+
/// gzip stream instead of handing compressed bytes to the tar reader.
1058+
#[test]
1059+
fn extract_rootfs_archive_accepts_gzip_archives() {
1060+
let dir = unique_temp_dir();
1061+
let rootfs = dir.join("rootfs");
1062+
let extracted = dir.join("extracted");
1063+
let archive = dir.join("rootfs.tar");
1064+
let gz_archive = dir.join("rootfs.tar.gz");
1065+
1066+
fs::create_dir_all(rootfs.join("etc")).expect("create etc");
1067+
fs::write(rootfs.join("etc/marker.txt"), "gzip-rootfs\n").expect("write marker");
1068+
create_rootfs_archive_from_dir(&rootfs, &archive).expect("archive rootfs");
1069+
1070+
let mut encoder = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default());
1071+
encoder
1072+
.write_all(&fs::read(&archive).expect("read archive"))
1073+
.expect("gzip archive");
1074+
fs::write(&gz_archive, encoder.finish().expect("finish gzip")).expect("write gzip archive");
1075+
1076+
extract_rootfs_archive_to(&gz_archive, &extracted).expect("extract gzip rootfs");
1077+
1078+
assert_eq!(
1079+
fs::read_to_string(extracted.join("etc/marker.txt")).expect("read extracted marker"),
1080+
"gzip-rootfs\n"
1081+
);
1082+
1083+
let _ = fs::remove_dir_all(&dir);
1084+
}
1085+
10341086
#[cfg(unix)]
10351087
#[test]
10361088
fn create_rootfs_archive_preserves_broken_symlinks() {

0 commit comments

Comments
 (0)