Skip to content

Commit 2275694

Browse files
committed
fix(verifier): bind downloaded images end to end before caching them
- Check sha256(sha256sum.txt) against os_image_hash before parsing the manifest or hashing any file. - Require the bios, kernel and initrd that metadata.json names to be manifest entries. image_dir.join() of an absolute or `..` path, or of an unlisted name, would otherwise measure bytes os_image_hash never committed to. - Bound the downloaded archive and its extracted size at 100 MiB each; measurement images carry no rootfs.
1 parent 9db0b6f commit 2275694

1 file changed

Lines changed: 96 additions & 25 deletions

File tree

‎dstack/verifier/src/verification.rs‎

Lines changed: 96 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@ fn decode_key_provider_info(bytes: &[u8]) -> Option<dstack_types::KeyProviderInf
9595
// cached.
9696
const MEASUREMENT_CACHE_VERSION: u32 = 3;
9797

98+
/// Upper bound on both a downloaded image archive and its extracted contents.
99+
/// Measurement images carry no rootfs; released ones are under 25 MB extracted.
100+
const MAX_IMAGE_SIZE: u64 = 100 << 20;
101+
98102
#[derive(Clone, Serialize, Deserialize)]
99103
struct CachedMeasurement {
100104
version: u32,
@@ -294,6 +298,10 @@ impl CvmVerifier {
294298
Ok(measurements)
295299
}
296300

301+
fn images_dir(&self) -> PathBuf {
302+
Path::new(&self.image_cache_dir).join("images")
303+
}
304+
297305
fn image_content_digest(image_dir: &Path) -> Result<Option<Vec<u8>>> {
298306
let sha256sum_path = image_dir.join("sha256sum.txt");
299307
if !sha256sum_path.exists() {
@@ -352,13 +360,55 @@ impl CvmVerifier {
352360
Ok(entries)
353361
}
354362

363+
/// Bind an extracted image to `os_image_hash` and drop everything the
364+
/// binding does not cover.
365+
fn verify_extracted_image(extracted_dir: &Path, hex_os_image_hash: &str) -> Result<()> {
366+
// All image modes are addressed by sha256(sha256sum.txt); extra
367+
// measurement CBOR files are ordinary entries and do not define
368+
// alternate image hashes. Check it before anything else in the archive
369+
// is parsed or hashed.
370+
let files_doc = fs_err::read_to_string(extracted_dir.join("sha256sum.txt"))
371+
.context("failed to read sha256sum.txt")?;
372+
if hex::encode(Sha256::digest(&files_doc)) != hex_os_image_hash {
373+
bail!("os_image_hash does not match sha256(sha256sum.txt)");
374+
}
375+
let manifest = Self::verify_image_manifest(extracted_dir, &files_doc)?;
376+
Self::prune_unlisted_image_files(extracted_dir, &manifest)?;
377+
Self::check_measured_files_are_listed(extracted_dir, &manifest)
378+
}
379+
380+
/// `metadata.json` picks the firmware, kernel and initrd that get measured.
381+
/// The manifest binds their bytes only if they are among its entries; any
382+
/// other path -- absolute, `..`, or just unlisted -- would be measured from
383+
/// bytes `os_image_hash` never committed to.
384+
fn check_measured_files_are_listed(
385+
extracted_dir: &Path,
386+
manifest: &[sha256sum::Entry],
387+
) -> Result<()> {
388+
let listed = |name: &str| manifest.iter().any(|entry| entry.name == name);
389+
if !listed("metadata.json") {
390+
bail!("sha256sum.txt does not list metadata.json");
391+
}
392+
let image_info = fs_err::read_to_string(extracted_dir.join("metadata.json"))
393+
.context("failed to read image metadata")?;
394+
let image_info: dstack_types::ImageInfo =
395+
serde_json::from_str(&image_info).context("failed to parse image metadata")?;
396+
for name in [&image_info.bios, &image_info.kernel, &image_info.initrd] {
397+
if !listed(name) {
398+
bail!("metadata.json names {name}, which sha256sum.txt does not list");
399+
}
400+
}
401+
Ok(())
402+
}
403+
355404
fn extract_image_archive(tarball_path: &Path, extracted_dir: &Path) -> Result<()> {
356405
let file = fs_err::File::open(tarball_path).context("Failed to open image archive")?;
357406
// `MultiGzDecoder`, not `GzDecoder`: the latter stops at the first gzip
358407
// member and reports clean EOF, so a concatenated-member archive would
359408
// extract partially without any error.
360409
let decoder = flate2::read::MultiGzDecoder::new(file);
361410
let mut archive = tar::Archive::new(decoder);
411+
let mut extracted = 0u64;
362412
for entry in archive.entries().context("Failed to read image archive")? {
363413
let mut entry = entry.context("Failed to read image archive entry")?;
364414
let path = entry
@@ -374,6 +424,10 @@ impl CvmVerifier {
374424
path.display()
375425
);
376426
}
427+
extracted += entry.size();
428+
if extracted > MAX_IMAGE_SIZE {
429+
bail!("image archive extracts to more than {MAX_IMAGE_SIZE} bytes");
430+
}
377431
if !entry
378432
.unpack_in(extracted_dir)
379433
.context("Failed to extract image archive entry")?
@@ -487,9 +541,7 @@ impl CvmVerifier {
487541
let hex_os_image_hash = hex::encode(&vm_config.os_image_hash);
488542

489543
// Get image directory
490-
let image_dir = Path::new(&self.image_cache_dir)
491-
.join("images")
492-
.join(&hex_os_image_hash);
544+
let image_dir = self.images_dir().join(&hex_os_image_hash);
493545

494546
let metadata_path = image_dir.join("metadata.json");
495547
if !metadata_path.exists() {
@@ -975,7 +1027,7 @@ impl CvmVerifier {
9751027
.replace("{OS_IMAGE_HASH}", hex_os_image_hash);
9761028

9771029
// Create a temporary directory for extraction within the cache directory
978-
let cache_dir = Path::new(&self.image_cache_dir).join("images").join("tmp");
1030+
let cache_dir = self.images_dir().join("tmp");
9791031
fs_err::create_dir_all(&cache_dir).context("Failed to create cache directory")?;
9801032
let auto_delete_temp_dir = tempfile::Builder::new()
9811033
.prefix("tmp-download-")
@@ -1004,7 +1056,12 @@ impl CvmVerifier {
10041056
.await
10051057
.context("Failed to create tarball file")?;
10061058
let mut response = response;
1059+
let mut downloaded = 0u64;
10071060
while let Some(chunk) = response.chunk().await? {
1061+
downloaded += chunk.len() as u64;
1062+
if downloaded > MAX_IMAGE_SIZE {
1063+
bail!("image archive exceeds {MAX_IMAGE_SIZE} bytes");
1064+
}
10081065
file.write_all(&chunk)
10091066
.await
10101067
.context("Failed to write chunk to file")?;
@@ -1019,27 +1076,7 @@ impl CvmVerifier {
10191076
drop(file);
10201077
Self::extract_image_archive(&tarball_path, &extracted_dir)?;
10211078

1022-
let sha256sum_path = extracted_dir.join("sha256sum.txt");
1023-
let files_doc =
1024-
fs_err::read_to_string(&sha256sum_path).context("Failed to read sha256sum.txt")?;
1025-
let manifest = Self::verify_image_manifest(&extracted_dir, &files_doc)?;
1026-
1027-
// Remove the files that are not listed in sha256sum.txt
1028-
Self::prune_unlisted_image_files(&extracted_dir, &manifest)?;
1029-
1030-
// All image modes are addressed by sha256(sha256sum.txt). Extra
1031-
// measurement CBOR files are ordinary sha256sum.txt entries and do not
1032-
// define alternate image hashes.
1033-
let legacy_os_image_hash = Sha256::new_with_prefix(files_doc.as_bytes()).finalize();
1034-
if hex::encode(legacy_os_image_hash) != hex_os_image_hash {
1035-
bail!("os_image_hash does not match sha256(sha256sum.txt)");
1036-
}
1037-
1038-
// Move the extracted files to the destination directory
1039-
let metadata_path = extracted_dir.join("metadata.json");
1040-
if !metadata_path.exists() {
1041-
bail!("metadata.json not found in the extracted archive");
1042-
}
1079+
Self::verify_extracted_image(&extracted_dir, hex_os_image_hash)?;
10431080

10441081
if dst_dir.exists() {
10451082
fs_err::remove_dir_all(dst_dir).context("Failed to remove destination directory")?;
@@ -1717,6 +1754,40 @@ mod tests {
17171754
assert_ne!(key(&resized), key(&base));
17181755
}
17191756

1757+
/// `os_image_hash` is checked before the manifest is parsed, and the files
1758+
/// `metadata.json` measures must be manifest entries.
1759+
#[test]
1760+
fn extracted_image_binds_the_files_metadata_measures() {
1761+
let verify = |kernel: &str| {
1762+
let metadata = format!(
1763+
r#"{{"cmdline":"","kernel":"{kernel}","initrd":"initrd","bios":"ovmf.fd"}}"#
1764+
);
1765+
let files: &[(&str, &[u8])] = &[
1766+
("metadata.json", metadata.as_bytes()),
1767+
("bzImage", b"kernel"),
1768+
("initrd", b"initrd"),
1769+
("ovmf.fd", b"firmware"),
1770+
];
1771+
let files_doc: String = files
1772+
.iter()
1773+
.map(|(name, payload)| {
1774+
format!("{} {name}\n", hex::encode(Sha256::digest(payload)))
1775+
})
1776+
.collect();
1777+
let dir = image_dir_with_manifest(&files_doc, files);
1778+
let hash = hex::encode(Sha256::digest(&files_doc));
1779+
CvmVerifier::verify_extracted_image(dir.path(), &hash)
1780+
};
1781+
verify("bzImage").unwrap();
1782+
for kernel in ["/boot/bzImage", "../bzImage", "unlisted"] {
1783+
assert!(verify(kernel).is_err(), "accepted kernel {kernel}");
1784+
}
1785+
1786+
let dir = image_dir_with_manifest("not a manifest", &[]);
1787+
let err = CvmVerifier::verify_extracted_image(dir.path(), &"00".repeat(32)).unwrap_err();
1788+
assert!(err.to_string().contains("os_image_hash"), "{err:#}");
1789+
}
1790+
17201791
#[test]
17211792
fn image_cache_pruning_keeps_checksum_identity() {
17221793
let files_doc = format!("{} metadata.json\n", hex::encode(Sha256::digest(b"{}")));

0 commit comments

Comments
 (0)