diff --git a/Cargo.lock b/Cargo.lock index 5d4f7b3..ce27b6a 100755 --- a/Cargo.lock +++ b/Cargo.lock @@ -481,7 +481,7 @@ checksum = "ac6b926516df9c60bfa16e107b21086399f8285a44ca9711344b9e553c5146e2" [[package]] name = "depot" -version = "0.60.0" +version = "1.0.0" dependencies = [ "anyhow", "ar", @@ -1417,9 +1417,9 @@ dependencies = [ [[package]] name = "md5" -version = "0.8.0" +version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae960838283323069879657ca3de837e9f7bbb4c7bf6ea7f1b290d5e9476d2e0" +checksum = "7ebb8d8732c6a6df3d8f032a82911cfc747e00efb95cc46e8d0acd5b5b88570c" [[package]] name = "memchr" diff --git a/Cargo.toml b/Cargo.toml index a049978..3f79335 100755 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "depot" -version = "0.60.0" +version = "1.0.0" edition = "2024" [lints.rust] @@ -42,7 +42,7 @@ xz2 = "0.1.7" zip = "8.5.0" zstd = { version = "0.13.3", features = ["zstdmt"] } inquire = "0.9.4" -md5 = "0.8.0" +md5 = "0.8.1" suppaftp = "10.0.0" minisign = "0.9.1" petgraph = "0.8.3" diff --git a/src/commands.rs b/src/commands.rs index 6539239..8d53a50 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -9,7 +9,7 @@ use crate::{ }; use anyhow::{Context, Result}; use git2::Direction; -use indicatif::{ProgressBar, ProgressDrawTarget, ProgressStyle}; +use indicatif::{MultiProgress, ProgressBar, ProgressDrawTarget, ProgressStyle}; use std::cmp::Ordering; use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::fs; @@ -23,6 +23,8 @@ use std::time::Duration; use url::Url; use walkdir::WalkDir; +const MAX_PARALLEL_DOWNLOADS: usize = 8; + use build_cmd::support::{ automatic_tests_disabled_for_outputs, build_lib32_companion_package, clean_build_source_dirs, clean_build_workspace, effective_lib32_only, ensure_requested_development_package_installed, @@ -1426,26 +1428,24 @@ fn install_planned_packages_to_rootfs_with_pre_removed( Ok(()) } -fn run_parallel_verification(items: &[T], progress: &ProgressBar, verify: F) -> Result<()> +fn run_parallel_tasks(items: &[T], worker_count: usize, task: F) -> Result> where T: Sync, - F: Fn(&T) -> Result<()> + Sync, + U: Send, + F: Fn(usize, &T) -> Result + Sync, { if items.is_empty() { - return Ok(()); + return Ok(Vec::new()); } - let worker_count = std::thread::available_parallelism() - .map(|count| count.get()) - .unwrap_or(1) - .min(items.len()); + let worker_count = worker_count.max(1).min(items.len()); let next_index = AtomicUsize::new(0); let (sender, receiver) = mpsc::channel(); - std::thread::scope(|scope| -> Result<()> { + std::thread::scope(|scope| -> Result> { for _ in 0..worker_count { let sender = sender.clone(); - let verify = &verify; + let task = &task; let next_index = &next_index; scope.spawn(move || { loop { @@ -1453,7 +1453,7 @@ where if index >= items.len() { break; } - let result = verify(&items[index]); + let result = task(index, &items[index]); if sender.send((index, result)).is_err() { break; } @@ -1462,22 +1462,37 @@ where } drop(sender); - let mut results: Vec>> = (0..items.len()).map(|_| None).collect(); + let mut results: Vec>> = (0..items.len()).map(|_| None).collect(); for _ in 0..items.len() { let (index, result) = receiver .recv() - .context("Verification worker exited before reporting a result")?; + .context("Parallel worker exited before reporting a result")?; results[index] = Some(result); - progress.inc(1); } - for result in results { - result.expect("every verification item must report a result")?; - } - Ok(()) + results + .into_iter() + .map(|result| result.expect("every parallel item must report a result")) + .collect() }) } +fn run_parallel_verification(items: &[T], progress: &ProgressBar, verify: F) -> Result<()> +where + T: Sync, + F: Fn(&T) -> Result<()> + Sync, +{ + let worker_count = std::thread::available_parallelism() + .map(|count| count.get()) + .unwrap_or(1); + run_parallel_tasks(items, worker_count, |_, item| { + let result = verify(item); + progress.inc(1); + result + })?; + Ok(()) +} + #[cfg(test)] fn install_package_outputs_to_rootfs( pkg_spec: &package::PackageSpec, @@ -1775,8 +1790,11 @@ fn execute_install_plan_with_child_commands( let mut binary_archives: HashMap<(String, String), db::repo::BinaryRepoCachedArchive> = HashMap::new(); let mut binary_phase_items = Vec::new(); + let mut seen_binary_archives = HashSet::new(); for step in &actionable_steps { - if let planner::PlanOrigin::Binary { repo_name, record } = &step.origin { + if let planner::PlanOrigin::Binary { repo_name, record } = &step.origin + && seen_binary_archives.insert((repo_name.clone(), record.filename.clone())) + { binary_phase_items.push(BinaryPhaseItem { repo_name: repo_name.clone(), record: (**record).clone(), @@ -1790,53 +1808,72 @@ fn execute_install_plan_with_child_commands( binary_phase_items.len() )); let use_tty_progress = std::io::stderr().is_terminal(); - for item in &binary_phase_items { - let label = format!( - "{}-{}-{}", - item.record.name, - item.record.version, - binary_arch_from_filename(&item.record.filename) - ); - let pb = ProgressBar::new(item.record.size.max(1)); - pb.set_draw_target(if use_tty_progress { - ProgressDrawTarget::stderr() - } else { - ProgressDrawTarget::hidden() - }); - pb.set_style( - ProgressStyle::default_bar() - .template("{prefix:.bold} [{bar:40.cyan/blue}] {eta}") - .unwrap_or_else(|_| ProgressStyle::default_bar()) - .progress_chars("#>-"), - ); - pb.set_prefix(label); - - let repo_cfg = config - .binary_repos - .get(&item.repo_name) - .with_context(|| format!("Binary repo '{}' not found in config", item.repo_name))?; - let mut progress_cb = |downloaded: u64, total: Option| { - if let Some(t) = total - && t > 0 - { - pb.set_length(t); - } - pb.set_position(downloaded); - }; - let cached = db::repo::cache_binary_package_archive_with_progress( - &item.repo_name, - repo_cfg, - &item.record, - &config.package_cache_dir, - Some(&mut progress_cb), - ) - .with_context(|| { - format!( - "Failed to cache binary package '{}' from repo '{}'", - item.record.filename, item.repo_name - ) - })?; - pb.finish_and_clear(); + let download_progress = MultiProgress::with_draw_target(if use_tty_progress { + ProgressDrawTarget::stderr() + } else { + ProgressDrawTarget::hidden() + }); + let download_bars = binary_phase_items + .iter() + .map(|item| { + let label = format!( + "{}-{}-{}", + item.record.name, + item.record.version, + binary_arch_from_filename(&item.record.filename) + ); + let pb = download_progress.add(ProgressBar::new(item.record.size.max(1))); + pb.set_style( + ProgressStyle::default_bar() + .template("{prefix:.bold} [{bar:40.cyan/blue}] {eta}") + .unwrap_or_else(|_| ProgressStyle::default_bar()) + .progress_chars("#>-"), + ); + pb.set_prefix(label); + pb + }) + .collect::>(); + let download_client = db::repo::binary_package_http_client()?; + let download_results = run_parallel_tasks( + &binary_phase_items, + MAX_PARALLEL_DOWNLOADS, + |index, item| { + let pb = &download_bars[index]; + let mut progress_cb = |downloaded: u64, total: Option| { + if let Some(t) = total + && t > 0 + { + pb.set_length(t); + } + pb.set_position(downloaded); + }; + let result = (|| { + let repo_cfg = config.binary_repos.get(&item.repo_name).with_context(|| { + format!("Binary repo '{}' not found in config", item.repo_name) + })?; + db::repo::cache_binary_package_archive_with_client_and_progress( + &item.repo_name, + repo_cfg, + &item.record, + &config.package_cache_dir, + &download_client, + Some(&mut progress_cb), + ) + .with_context(|| { + format!( + "Failed to cache binary package '{}' from repo '{}'", + item.record.filename, item.repo_name + ) + }) + })(); + pb.finish_and_clear(); + result + }, + ); + download_progress + .clear() + .context("Failed to clear binary download progress")?; + for (item, cached) in binary_phase_items.iter().zip(download_results?) { binary_archives.insert( (item.repo_name.clone(), item.record.filename.clone()), cached, @@ -1844,59 +1881,34 @@ fn execute_install_plan_with_child_commands( } ui::info(format!( - "Verifying checksums for {} binary package(s)...", + "Verifying checksums and detached signatures for {} binary package(s)...", binary_phase_items.len() )); - let checksum_pb = ProgressBar::new(binary_phase_items.len() as u64); - checksum_pb.set_draw_target(if use_tty_progress { + let integrity_pb = ProgressBar::new(binary_phase_items.len() as u64); + integrity_pb.set_draw_target(if use_tty_progress { ProgressDrawTarget::stderr() } else { ProgressDrawTarget::hidden() }); - checksum_pb.set_style( + integrity_pb.set_style( ProgressStyle::default_bar() .template("{prefix:.bold} [{bar:40.cyan/blue}] {pos}/{len} {eta}") .unwrap_or_else(|_| ProgressStyle::default_bar()) .progress_chars("#>-"), ); - checksum_pb.set_prefix("checksums"); - run_parallel_verification(&binary_phase_items, &checksum_pb, |item| { - let cached = binary_archives + integrity_pb.set_prefix("integrity"); + let has_detached_signatures = binary_phase_items.iter().any(|item| { + binary_archives .get(&(item.repo_name.clone(), item.record.filename.clone())) - .with_context(|| { - format!( - "Cached archive missing for {} from repo '{}'", - item.record.filename, item.repo_name - ) - })?; - db::repo::verify_binary_package_archive_checksums(&cached.package_path, &item.record) - .with_context(|| { - format!( - "Checksum verification failed for {} from repo '{}'", - item.record.filename, item.repo_name - ) - }) - })?; - checksum_pb.finish_and_clear(); - - ui::info(format!( - "Verifying detached signatures for {} binary package(s)...", - binary_phase_items.len() - )); - let signature_pb = ProgressBar::new(binary_phase_items.len() as u64); - signature_pb.set_draw_target(if use_tty_progress { - ProgressDrawTarget::stderr() - } else { - ProgressDrawTarget::hidden() + .is_some_and(|cached| cached.signature_path.exists()) }); - signature_pb.set_style( - ProgressStyle::default_bar() - .template("{prefix:.bold} [{bar:40.cyan/blue}] {pos}/{len} {eta}") - .unwrap_or_else(|_| ProgressStyle::default_bar()) - .progress_chars("#>-"), - ); - signature_pb.set_prefix("signatures"); - run_parallel_verification(&binary_phase_items, &signature_pb, |item| { + let trusted_keys = if has_detached_signatures { + signing::load_trusted_public_keys(rootfs) + .context("Failed to load trusted Minisign public keys")? + } else { + Vec::new() + }; + run_parallel_verification(&binary_phase_items, &integrity_pb, |item| { let repo_cfg = config .binary_repos .get(&item.repo_name) @@ -1909,21 +1921,22 @@ fn execute_install_plan_with_child_commands( item.record.filename, item.repo_name ) })?; - db::repo::verify_binary_package_archive_signature( + db::repo::verify_binary_package_archive_integrity_with_trusted_keys( &item.repo_name, repo_cfg, - rootfs, + &item.record, &cached.package_path, &cached.signature_path, + &trusted_keys, ) .with_context(|| { format!( - "Detached signature verification failed for {} from repo '{}'", + "Integrity verification failed for {} from repo '{}'", item.record.filename, item.repo_name ) }) })?; - signature_pb.finish_and_clear(); + integrity_pb.finish_and_clear(); } if should_delegate_live_rootfs_installs(rootfs) { diff --git a/src/commands/tests.rs b/src/commands/tests.rs index b18ff1d..cf19b44 100644 --- a/src/commands/tests.rs +++ b/src/commands/tests.rs @@ -7,7 +7,7 @@ use crate::test_support::TestEnv; use git2::{Oid, Repository}; use std::path::Path; use std::sync::{ - Mutex, MutexGuard, + Barrier, Mutex, MutexGuard, atomic::{AtomicUsize, Ordering as AtomicOrdering}, }; @@ -107,6 +107,20 @@ fn parallel_verification_processes_every_item() -> Result<()> { Ok(()) } +#[test] +fn parallel_tasks_run_concurrently_and_preserve_input_order() -> Result<()> { + let items = vec![3_u8, 1, 4, 2]; + let barrier = Barrier::new(items.len()); + + let results = run_parallel_tasks(&items, items.len(), |_, item| { + barrier.wait(); + Ok(item * 2) + })?; + + assert_eq!(results, vec![6, 2, 8, 4]); + Ok(()) +} + #[test] fn install_post_extract_env_uses_selected_non_live_rootfs() -> Result<()> { let _guard = assume_yes_test_lock(); diff --git a/src/db/repo.rs b/src/db/repo.rs index 186e8db..83d2f03 100644 --- a/src/db/repo.rs +++ b/src/db/repo.rs @@ -6,7 +6,7 @@ use rusqlite::{Connection, params}; use sha2::{Digest, Sha256, Sha512}; use std::collections::{BTreeSet, HashMap}; use std::fs; -use std::io::{Read, Write}; +use std::io::{Read, Seek, SeekFrom, Write}; use std::path::{Path, PathBuf}; use std::sync::{ Mutex, OnceLock, @@ -988,26 +988,11 @@ fn verify_with_any_trusted_public_key( input: &Path, sig_path: &Path, ) -> Result { - let keys = crate::signing::list_trusted_public_keys(rootfs)?; + let keys = crate::signing::load_trusted_public_keys(rootfs)?; if keys.is_empty() { anyhow::bail!("No trusted minisign public keys found in rootfs or host"); } - - let mut last_failure: Option<(PathBuf, anyhow::Error)> = None; - for key_path in keys { - match crate::signing::verify_zst_file_detached_with_public_key(input, sig_path, &key_path) { - Ok(()) => return Ok(key_path), - Err(err) => last_failure = Some((key_path, err)), - } - } - - let (key_path, err) = last_failure.expect("non-empty key list must produce a failure"); - Err(err).with_context(|| { - format!( - "Detached signature verification failed with all trusted public keys (last tried {})", - key_path.display() - ) - }) + crate::signing::verify_zst_file_detached_with_trusted_keys(input, sig_path, &keys) } fn sanitize_filename_component(input: &str) -> String { @@ -2199,16 +2184,7 @@ fn verify_binary_package_record_checksums( path: &Path, rec: &BinaryRepoPackageRecord, ) -> Result<()> { - use sha2::{Digest, Sha512}; - - let expected = rec.sha512.trim().to_ascii_lowercase(); - if expected.is_empty() { - anyhow::bail!( - "Missing SHA-512 checksum for {} from repo '{}'", - path.display(), - rec.repo_name - ); - } + let expected = expected_binary_package_sha512(path, rec)?; let mut file = fs::File::open(path).with_context(|| format!("Failed to open {}", path.display()))?; @@ -2222,7 +2198,33 @@ fn verify_binary_package_record_checksums( hasher.update(&buf[..n]); } - if crate::hex::encode_lower(hasher.finalize()) != expected { + verify_binary_package_sha512_digest( + path, + rec, + &expected, + &crate::hex::encode_lower(hasher.finalize()), + ) +} + +fn expected_binary_package_sha512(path: &Path, rec: &BinaryRepoPackageRecord) -> Result { + let expected = rec.sha512.trim().to_ascii_lowercase(); + if expected.is_empty() { + anyhow::bail!( + "Missing SHA-512 checksum for {} from repo '{}'", + path.display(), + rec.repo_name + ); + } + Ok(expected) +} + +fn verify_binary_package_sha512_digest( + path: &Path, + rec: &BinaryRepoPackageRecord, + expected: &str, + actual: &str, +) -> Result<()> { + if actual != expected { anyhow::bail!( "SHA-512 mismatch for {} from repo '{}'", path.display(), @@ -2232,6 +2234,48 @@ fn verify_binary_package_record_checksums( Ok(()) } +struct Sha512Reader { + inner: R, + hasher: Sha512, +} + +impl Sha512Reader { + fn new(inner: R) -> Self { + Self { + inner, + hasher: Sha512::new(), + } + } + + fn finalize_hex(self) -> String { + crate::hex::encode_lower(self.hasher.finalize()) + } +} + +impl Read for Sha512Reader { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + let read = self.inner.read(buf)?; + if read > 0 { + self.hasher.update(&buf[..read]); + } + Ok(read) + } +} + +impl Seek for Sha512Reader { + fn seek(&mut self, position: SeekFrom) -> std::io::Result { + if position != SeekFrom::Start(0) { + return Err(std::io::Error::new( + std::io::ErrorKind::Unsupported, + "checksum reader only supports rewinding to the start", + )); + } + let position = self.inner.seek(position)?; + self.hasher = Sha512::new(); + Ok(position) + } +} + fn download_binary_package_archive( client: &reqwest::blocking::Client, pkg_url: &str, @@ -2312,12 +2356,12 @@ fn fetch_binary_package_signature( Ok(found) } -fn verify_binary_package_signature( +fn verify_binary_package_signature_with_trusted_keys( repo_name: &str, repo: &crate::config::BinaryRepo, - rootfs: &Path, pkg_path: &Path, sig_path: &Path, + trusted_keys: &[crate::signing::TrustedPublicKey], ) -> Result<()> { if !sig_path.exists() { if repo.allow_unsigned { @@ -2329,7 +2373,6 @@ fn verify_binary_package_signature( ); } - let trusted_keys = crate::signing::list_trusted_public_keys(rootfs)?; if trusted_keys.is_empty() { if repo.allow_unsigned { crate::log_warn!( @@ -2344,13 +2387,17 @@ fn verify_binary_package_signature( ); } - let _verified_key = verify_with_any_trusted_public_key(rootfs, pkg_path, sig_path) - .with_context(|| { - format!( - "Failed to verify detached package signature for {}", - pkg_path.display() - ) - })?; + let _verified_key = crate::signing::verify_zst_file_detached_with_trusted_keys( + pkg_path, + sig_path, + trusted_keys, + ) + .with_context(|| { + format!( + "Failed to verify detached package signature for {}", + pkg_path.display() + ) + })?; Ok(()) } @@ -2373,6 +2420,31 @@ pub fn cache_binary_package_archive_with_progress( repo: &crate::config::BinaryRepo, rec: &BinaryRepoPackageRecord, package_cache_dir: &Path, + progress_cb: Option<&mut dyn FnMut(u64, Option)>, +) -> Result { + let client = binary_package_http_client()?; + cache_binary_package_archive_with_client_and_progress( + repo_name, + repo, + rec, + package_cache_dir, + &client, + progress_cb, + ) +} + +pub(crate) fn binary_package_http_client() -> Result { + reqwest::blocking::Client::builder() + .build() + .context("Failed to build HTTP client for binary package fetch") +} + +pub(crate) fn cache_binary_package_archive_with_client_and_progress( + repo_name: &str, + repo: &crate::config::BinaryRepo, + rec: &BinaryRepoPackageRecord, + package_cache_dir: &Path, + client: &reqwest::blocking::Client, mut progress_cb: Option<&mut dyn FnMut(u64, Option)>, ) -> Result { let machine_arch = std::env::consts::ARCH; @@ -2393,12 +2465,8 @@ pub fn cache_binary_package_archive_with_progress( let pkg_url = join_repo_url(base_url, &rec.filename)?; let sig_url = join_repo_url(base_url, &format!("{}.sig", rec.filename))?; - let client = reqwest::blocking::Client::builder() - .build() - .context("Failed to build HTTP client for binary package fetch")?; - let package_downloaded = if !package_path.exists() { - download_binary_package_archive(&client, &pkg_url, &tmp_path, &mut progress_cb)?; + download_binary_package_archive(client, &pkg_url, &tmp_path, &mut progress_cb)?; fs::rename(&tmp_path, &package_path).with_context(|| { format!( "Failed to move {} to {}", @@ -2419,7 +2487,7 @@ pub fn cache_binary_package_archive_with_progress( if package_downloaded || !signature_path.exists() { let sig_downloaded = - fetch_binary_package_signature(repo_name, repo, &client, &sig_url, &tmp_sig_path)?; + fetch_binary_package_signature(repo_name, repo, client, &sig_url, &tmp_sig_path)?; if sig_downloaded { fs::rename(&tmp_sig_path, &signature_path).with_context(|| { format!( @@ -2448,15 +2516,44 @@ pub fn verify_binary_package_archive_checksums( verify_binary_package_record_checksums(archive_path, rec) } -/// Verify a cached/downloaded package archive against its detached signature. -pub fn verify_binary_package_archive_signature( +pub(crate) fn verify_binary_package_archive_integrity_with_trusted_keys( repo_name: &str, repo: &crate::config::BinaryRepo, - rootfs: &Path, + record: &BinaryRepoPackageRecord, package_path: &Path, signature_path: &Path, + trusted_keys: &[crate::signing::TrustedPublicKey], ) -> Result<()> { - verify_binary_package_signature(repo_name, repo, rootfs, package_path, signature_path) + if signature_path.exists() && !trusted_keys.is_empty() { + let expected = expected_binary_package_sha512(package_path, record)?; + let file = fs::File::open(package_path) + .with_context(|| format!("Failed to open {}", package_path.display()))?; + let mut reader = Sha512Reader::new(file); + let _verified_key = crate::signing::verify_reader_detached_with_trusted_keys( + &mut reader, + package_path, + signature_path, + trusted_keys, + ) + .with_context(|| { + format!( + "Failed to verify detached package signature for {}", + package_path.display() + ) + })?; + let actual = reader.finalize_hex(); + verify_binary_package_sha512_digest(package_path, record, &expected, &actual)?; + return Ok(()); + } + + verify_binary_package_archive_checksums(package_path, record)?; + verify_binary_package_signature_with_trusted_keys( + repo_name, + repo, + package_path, + signature_path, + trusted_keys, + ) } /// Download a binary package archive and verify it against detached signatures @@ -2469,22 +2566,22 @@ pub fn fetch_binary_package_archive( package_cache_dir: &Path, ) -> Result { let cached = cache_binary_package_archive(repo_name, repo, rec, package_cache_dir)?; - verify_binary_package_archive_checksums(&cached.package_path, rec).with_context(|| { - format!( - "Binary package failed checksum verification: {}", - cached.package_path.display() - ) - })?; - verify_binary_package_archive_signature( + let trusted_keys = if cached.signature_path.exists() { + crate::signing::load_trusted_public_keys(rootfs)? + } else { + Vec::new() + }; + verify_binary_package_archive_integrity_with_trusted_keys( repo_name, repo, - rootfs, + rec, &cached.package_path, &cached.signature_path, + &trusted_keys, ) .with_context(|| { format!( - "Binary package failed signature verification: {}", + "Binary package failed integrity verification: {}", cached.package_path.display() ) })?; @@ -3239,7 +3336,7 @@ revision = 1 } #[test] - fn test_cache_binary_package_archive_supports_phased_verification() { + fn test_cache_binary_package_archive_supports_combined_integrity_verification() { let rootfs = tempfile::tempdir().unwrap(); let repo_dir = tempfile::tempdir().unwrap(); let cache_dir = tempfile::tempdir().unwrap(); @@ -3286,14 +3383,29 @@ revision = 1 verify_binary_package_archive_checksums(&cached.package_path, &rec) .expect("checksum verification should succeed"); - verify_binary_package_archive_signature( + let trusted_keys = crate::signing::load_trusted_public_keys(rootfs.path()).unwrap(); + verify_binary_package_archive_integrity_with_trusted_keys( "repo", &repo_cfg, - rootfs.path(), + &rec, &cached.package_path, &cached.signature_path, + &trusted_keys, ) - .expect("signature verification should succeed"); + .expect("combined integrity verification should succeed"); + + let mut wrong_record = rec.clone(); + wrong_record.sha512 = crate::hex::encode_lower(Sha512::digest(b"wrong payload")); + let error = verify_binary_package_archive_integrity_with_trusted_keys( + "repo", + &repo_cfg, + &wrong_record, + &cached.package_path, + &cached.signature_path, + &trusted_keys, + ) + .expect_err("combined verification must reject a checksum mismatch"); + assert!(error.to_string().contains("SHA-512 mismatch")); } #[test] diff --git a/src/signing.rs b/src/signing.rs index 1f1179e..5926817 100644 --- a/src/signing.rs +++ b/src/signing.rs @@ -4,7 +4,7 @@ use anyhow::{Context, Result}; use inquire::Password; use minisign::{PublicKey, SecretKey, SecretKeyBox, SignatureBox}; use std::fs; -use std::io::IsTerminal; +use std::io::{IsTerminal, Read, Seek}; use std::path::{Path, PathBuf}; const PUBLIC_KEYS_DIR_REL: &str = "usr/share/depot/keys/public"; @@ -164,6 +164,24 @@ pub fn list_trusted_public_keys(rootfs: &Path) -> Result> { list_public_key_files_in_roots(rootfs, Path::new("/")) } +#[derive(Clone)] +pub(crate) struct TrustedPublicKey { + pub(crate) path: PathBuf, + pub(crate) key: PublicKey, +} + +pub(crate) fn load_trusted_public_keys(rootfs: &Path) -> Result> { + list_trusted_public_keys(rootfs)? + .into_iter() + .map(|path| { + let key = PublicKey::from_file(&path).with_context(|| { + format!("Failed to load minisign public key: {}", path.display()) + })?; + Ok(TrustedPublicKey { path, key }) + }) + .collect() +} + fn detached_sig_path(input: &Path) -> PathBuf { PathBuf::from(format!("{}.sig", input.display())) } @@ -388,6 +406,70 @@ pub fn verify_zst_file_detached_with_public_key( verify_detached_with_key_paths(input, sig_path, &keys) } +pub(crate) fn verify_zst_file_detached_with_trusted_keys( + input: &Path, + sig_path: &Path, + trusted_keys: &[TrustedPublicKey], +) -> Result { + if !input.exists() { + anyhow::bail!("File not found: {}", input.display()); + } + if !sig_path.exists() { + anyhow::bail!("Detached signature not found: {}", sig_path.display()); + } + if !is_verify_supported_zst_file(input) { + anyhow::bail!( + "Verification currently only supports .zst and .zst.tmp files: {}", + input.display() + ); + } + + let mut file = + fs::File::open(input).with_context(|| format!("Failed to open {}", input.display()))?; + verify_reader_detached_with_trusted_keys(&mut file, input, sig_path, trusted_keys) +} + +pub(crate) fn verify_reader_detached_with_trusted_keys( + reader: &mut R, + input: &Path, + sig_path: &Path, + trusted_keys: &[TrustedPublicKey], +) -> Result { + let signature = SignatureBox::from_file(sig_path) + .with_context(|| format!("Failed to load detached signature: {}", sig_path.display()))?; + let matching_keys = trusted_keys + .iter() + .filter(|trusted| trusted.key.keynum() == signature.keynum()) + .collect::>(); + if matching_keys.is_empty() { + anyhow::bail!( + "No trusted minisign public key matches signature key ID {} for {}", + crate::hex::encode_lower(signature.keynum()), + input.display() + ); + } + + let mut last_failure = None; + for trusted in matching_keys { + reader + .rewind() + .with_context(|| format!("Failed to rewind {}", input.display()))?; + match minisign::verify(&trusted.key, &signature, &mut *reader, true, false, false) { + Ok(()) => return Ok(trusted.path.clone()), + Err(err) => last_failure = Some((trusted.path.clone(), err)), + } + } + + let (key_path, err) = last_failure.expect("at least one matching trusted key was tried"); + Err(err).with_context(|| { + format!( + "Detached signature verification failed for {} with trusted key {}", + input.display(), + key_path.display() + ) + }) +} + /// Sign one or more `.zst` files with detached minisign signatures written to `.sig`. pub fn sign_zst_files_detached(rootfs: &Path, inputs: &[PathBuf]) -> Result> { let signable_inputs = collect_signable_inputs(inputs, true)?; @@ -584,6 +666,43 @@ mod tests { Ok(()) } + #[test] + fn trusted_key_verification_selects_signature_key_id() -> Result<()> { + let rootfs = tempfile::tempdir()?; + let file = rootfs.path().join("artifact.tar.zst"); + fs::write(&file, b"signed payload")?; + + let wrong_pair = KeyPair::generate_unencrypted_keypair()?; + let signing_pair = KeyPair::generate_unencrypted_keypair()?; + let signature = minisign::sign( + Some(&signing_pair.pk), + &signing_pair.sk, + fs::File::open(&file)?, + None, + Some("test signature"), + )?; + let sig_path = detached_sig_path(&file); + fs::write(&sig_path, signature.to_bytes())?; + + let wrong_path = rootfs.path().join("wrong.pub"); + let signing_path = rootfs.path().join("signing.pub"); + let trusted_keys = vec![ + TrustedPublicKey { + path: wrong_path, + key: wrong_pair.pk, + }, + TrustedPublicKey { + path: signing_path.clone(), + key: signing_pair.pk, + }, + ]; + + let verified_path = + verify_zst_file_detached_with_trusted_keys(&file, &sig_path, &trusted_keys)?; + assert_eq!(verified_path, signing_path); + Ok(()) + } + #[test] fn sign_zst_files_detached_signs_multiple_files() -> Result<()> { let rootfs = tempfile::tempdir()?;