diff --git a/knot2/crates/knot-fixtures/src/lib.rs b/knot2/crates/knot-fixtures/src/lib.rs index 2c0e565e..35386d1f 100644 --- a/knot2/crates/knot-fixtures/src/lib.rs +++ b/knot2/crates/knot-fixtures/src/lib.rs @@ -96,6 +96,12 @@ pub fn commit(work: &Path, file: &str, contents: &str, message: &str) { must(work, &["commit", "-q", "-m", message]); } +pub fn contains(haystack: &[u8], needle: &[u8]) -> bool { + haystack + .windows(needle.len()) + .any(|window| window == needle) +} + #[cfg(test)] mod tests { use super::*; diff --git a/knot2/crates/knot-git/src/repo.rs b/knot2/crates/knot-git/src/repo.rs index b5ba8f35..2a47a285 100644 --- a/knot2/crates/knot-git/src/repo.rs +++ b/knot2/crates/knot-git/src/repo.rs @@ -245,6 +245,22 @@ impl Layout { } } +// `worktree_stream` builds a `gix_filter::Pipeline` +// out of whatever config it has loaded, +// so a filter command defined in any of those sources +// runs against the tree being archived. +// The repository's own config is then the only source gix reads, +// and mr knot wrote that file when it created the repo under the scan path. +// We pin the trust because gix otherwise works it out from who owns the git dir, +// and it reduces the trust when someone else owns that dir, +// at which point it applies a 16MiB object allocation limit +// and treats the repository's own config sections as untrusted. +// Under isolation no safe.directory entry can restore `Full` trust, +// since gix honors that key from only system/global config. +fn isolated_open_options() -> gix::open::Options { + gix::open::Options::isolated().with(gix::sec::Trust::Full) +} + pub(crate) fn init_bare_with_format( path: &Path, format: ObjectFormat, @@ -257,7 +273,7 @@ pub(crate) fn init_bare_with_format( object_hash, ..Default::default() }, - gix::open::Options::default(), + isolated_open_options(), ) .map(Into::into) .map_err(|error| error.to_string()) @@ -270,7 +286,7 @@ fn staging_path(parent: &Path) -> PathBuf { } fn init_bare_idempotent(path: PathBuf) -> Result { - if let Ok(git) = gix::open(&path) { + if let Ok(git) = gix::open_opts(&path, isolated_open_options()) { return Ok(assembled(git, path)); } let parent = path.parent().ok_or_else(|| GitError::Create { @@ -283,9 +299,9 @@ fn init_bare_idempotent(path: PathBuf) -> Result { })?; let staging = staging_path(parent); let _ = std::fs::remove_dir_all(&staging); - gix::init_bare(&staging).map_err(|error| GitError::Create { + init_bare_with_format(&staging, ObjectFormat::SHA1).map_err(|message| GitError::Create { path: staging.clone(), - message: error.to_string(), + message, })?; match std::fs::rename(&staging, &path) { Ok(()) => Repo::open(path), @@ -442,10 +458,11 @@ pub(crate) fn fsync_if_present(path: &Path) -> Result<(), GitError> { impl Repo { pub fn open(path: impl Into) -> Result { let path = path.into(); - let git = gix::open(&path).map_err(|error| GitError::Open { - path: path.clone(), - message: error.to_string(), - })?; + let git = + gix::open_opts(&path, isolated_open_options()).map_err(|error| GitError::Open { + path: path.clone(), + message: error.to_string(), + })?; Ok(assembled(git, path)) } @@ -1086,6 +1103,85 @@ mod tests { (dir, layout, did) } + #[test] + fn every_repo_opens_against_its_own_config_and_nothing_ambient() { + let permissions = isolated_open_options().permissions; + let config = permissions.config; + assert!( + !config.system + && !config.git + && !config.user + && !config.env + && !config.includes + && !config.git_binary, + "a filter driver in ambient config would execute when worktree_stream archives a \ + pushed tree, so we read only the repository's own config: {config:?}" + ); + assert!( + !permissions.attributes.system + && !permissions.attributes.git + && !permissions.attributes.git_binary, + "only the archived tree's own .gitattributes may set filter= on a path: {:?}", + permissions.attributes + ); + let denied = |permission| matches!(permission, gix::sec::Permission::Deny); + let env = permissions.env; + assert!( + denied(env.xdg_config_home) + && denied(env.home) + && denied(env.git_prefix) + && denied(env.ssh_prefix) + && denied(env.identity) + && denied(env.objects) + && denied(env.http_transport), + "gix resolves GIT_CONFIG_KEY_n, HOME and XDG_CONFIG_HOME into config that can define a \ + filter driver: {env:?}" + ); + assert!( + permissions.is_isolated(), + "every permission must match the set gix itself calls isolated, including any field a \ + gix upgrade adds that the three checks above don't name: {permissions:?}" + ); + } + + #[cfg(unix)] + #[test] + fn the_knot_keeps_full_trust_on_a_git_dir_it_no_longer_owns() { + const NOBODY: u32 = 65534; + let (_dir, layout, did) = repo(); + layout.create(&did).unwrap(); + let path = layout.repo_path(&did).unwrap(); + let unreachable_uid = |kind| { + matches!( + kind, + std::io::ErrorKind::PermissionDenied | std::io::ErrorKind::InvalidInput + ) + }; + match std::os::unix::fs::chown(&path, Some(NOBODY), None) { + Err(error) if unreachable_uid(error.kind()) => { + eprintln!( + "skipping the foreign-owner trust check: chown to {NOBODY} needs root and a \ + uid mapping reaching that far" + ); + return; + } + outcome => outcome.unwrap(), + } + assert_eq!( + layout.open(&did).unwrap().git().git_dir_trust(), + gix::sec::Trust::Full, + "reduced trust applies a 16MiB limit to every object allocation" + ); + assert_eq!( + gix::open_opts(&path, gix::open::Options::isolated()) + .unwrap() + .git_dir_trust(), + gix::sec::Trust::Reduced, + "gix raised the trust of a git dir owned by another user with no safe.directory entry \ + in reach" + ); + } + #[test] fn layout_paths_shard_and_stay_within_scan() { let layout = Layout::new("/srv/git"); diff --git a/knot2/crates/knot-git/tests/common/mod.rs b/knot2/crates/knot-git/tests/common/mod.rs index 3f61963c..ef6a6ffd 100644 --- a/knot2/crates/knot-git/tests/common/mod.rs +++ b/knot2/crates/knot-git/tests/common/mod.rs @@ -4,7 +4,7 @@ use knot_git::Layout; use knot_types::RepoDid; pub use knot_fixtures::{ - available as git_available, commit as commit_file, must as git_ok, run as git, + available as git_available, commit as commit_file, contains, must as git_ok, run as git, }; pub fn seeded() -> (tempfile::TempDir, tempfile::TempDir, Layout, RepoDid) { diff --git a/knot2/crates/knot-git/tests/config_isolation.rs b/knot2/crates/knot-git/tests/config_isolation.rs new file mode 100644 index 00000000..0c0aa2c8 --- /dev/null +++ b/knot2/crates/knot-git/tests/config_isolation.rs @@ -0,0 +1,102 @@ +use std::path::Path; +use std::sync::atomic::AtomicBool; + +use knot_git::ArchiveFormat; +use knot_types::Oid; + +mod common; +use common::{commit_file, contains, git_ok as git, seeded}; + +fn archive_without_isolation(bare_path: &Path, head: Oid) -> Vec { + let permissive = gix::open_opts(bare_path, gix::open::Options::default()).unwrap(); + let tree = permissive + .find_object(head.object_id()) + .unwrap() + .peel_to_tree() + .unwrap(); + let (stream, _index) = permissive.worktree_stream(tree.id).unwrap(); + let mut out = std::io::Cursor::new(Vec::new()); + permissive + .worktree_archive( + stream, + &mut out, + gix::progress::Discard, + &AtomicBool::new(false), + gix_archive::Options { + format: gix_archive::Format::Tar, + tree_prefix: None, + modification_time: 0, + }, + ) + .unwrap(); + out.into_inner() +} + +#[test] +fn a_filter_driver_pulled_in_by_an_include_never_runs_for_a_served_archive() { + let (_scan, work_dir, layout, did) = seeded(); + let work = work_dir.path(); + let bare_path = layout.repo_path(&did).unwrap(); + std::fs::write(work.join(".gitattributes"), "payload.txt filter=knotpwn\n").unwrap(); + commit_file(work, "payload.txt", "kelp\n", "seed"); + git(work, &["push", "-q", bare_path.to_str().unwrap(), "main"]); + let head = Oid::from_hex(&git(work, &["rev-parse", "HEAD"])).unwrap(); + + let ambient = tempfile::tempdir().unwrap(); + let driver = ambient.path().join("driver.cfg"); + std::fs::write( + &driver, + "[filter \"knotpwn\"]\n\tsmudge = sed s/kelp/pwned/\n\trequired = true\n", + ) + .unwrap(); + let config_path = bare_path.join("config"); + let local = std::fs::read_to_string(&config_path).unwrap(); + std::fs::write( + &config_path, + format!("{local}[include]\n\tpath = {}\n", driver.display()), + ) + .unwrap(); + + let via_git = knot_fixtures::command(&bare_path) + .args(["archive", "--format=tar", "main"]) + .output() + .unwrap(); + assert!( + via_git.status.success(), + "git archive failed:\n{}", + String::from_utf8_lossy(&via_git.stderr) + ); + assert!( + contains(&via_git.stdout, b"pwned"), + "git archived with the include in place and its output has no pwned in it, so the driver \ + never ran" + ); + assert!( + contains(&archive_without_isolation(&bare_path, head), b"pwned"), + "gix archived the same repo under default permissions and its output has no pwned in it, \ + so worktree_stream no longer applies filters" + ); + + let bare = layout.open(&did).unwrap(); + let tree = bare.peel_to_tree(head).unwrap(); + let mut out = std::io::Cursor::new(Vec::new()); + bare.write_archive(tree, ArchiveFormat::Tar, None, &mut out) + .unwrap(); + let served = out.into_inner(); + + assert!( + std::fs::read_to_string(&config_path) + .unwrap() + .contains("[include]"), + "opening the repo rewrote its config and removed the include, so gix never read the \ + driver definition for the archive below" + ); + assert!( + contains(&served, b"kelp"), + "the served archive contains the blob as it was pushed" + ); + assert!( + !contains(&served, b"pwned"), + "the knot ran a filter driver defined by config outside the repository" + ); +} diff --git a/knot2/crates/knot-git/tests/reads.rs b/knot2/crates/knot-git/tests/reads.rs index e4eac12b..44b91479 100644 --- a/knot2/crates/knot-git/tests/reads.rs +++ b/knot2/crates/knot-git/tests/reads.rs @@ -8,7 +8,7 @@ fn rp(path: &str) -> RepoPath { } mod common; -use common::{commit_file, git_ok as git}; +use common::{commit_file, contains, git_ok as git}; #[test] fn typed_reads_over_a_seeded_repo() { @@ -641,9 +641,8 @@ fn archives_round_trip_through_tar() { let mut decoder = flate2::read::GzDecoder::new(compressed.as_slice()); let mut tar = Vec::new(); std::io::Read::read_to_end(&mut decoder, &mut tar).unwrap(); - let needle = b"squid-main/src/lib.rs"; assert!( - tar.windows(needle.len()).any(|window| window == needle), + contains(&tar, b"squid-main/src/lib.rs"), "tar contains prefixed entries" ); } diff --git a/knot2/crates/knot-pack/tests/common/mod.rs b/knot2/crates/knot-pack/tests/common/mod.rs index 1d365762..985373a1 100644 --- a/knot2/crates/knot-pack/tests/common/mod.rs +++ b/knot2/crates/knot-pack/tests/common/mod.rs @@ -12,7 +12,7 @@ use knot_git::{Layout, Repo}; use knot_pack::{RepoLookup, RepoResolver, RepoTarget}; use knot_types::{ObjectFormat, RepoDid}; -pub use knot_fixtures::{commit, must, run as git}; +pub use knot_fixtures::{commit, contains, must, run as git}; pub fn pkt(payload: &[u8]) -> Vec { let mut out = format!("{:04x}", payload.len() + 4).into_bytes(); diff --git a/knot2/crates/knot-pack/tests/git_client.rs b/knot2/crates/knot-pack/tests/git_client.rs index d6c150eb..6a3d22c1 100644 --- a/knot2/crates/knot-pack/tests/git_client.rs +++ b/knot2/crates/knot-pack/tests/git_client.rs @@ -9,7 +9,7 @@ use knot_pack::{RepoLookup, RepoResolver, RepoTarget}; use knot_types::{OwnerDid, RefName, RepoDid, RepoRkey}; mod common; -use common::{commit, git, must, pkt, serve_dids, spawn, unsideband}; +use common::{commit, contains, git, must, pkt, serve_dids, spawn, unsideband}; fn seed_repo(work: &Path, bare: &str, file: &str, contents: &str) { std::fs::create_dir_all(work).unwrap(); @@ -471,8 +471,7 @@ async fn http_upload_archive_serves_a_framed_tar_and_guards_refuse_cob_raw_oids_ "archive response opens with the ACK pkt-line" ); assert!( - body.windows("README.md".len()) - .any(|window| window == b"README.md"), + contains(&body, b"README.md"), "framed archive contains README.md entry" ); @@ -523,8 +522,7 @@ async fn http_upload_archive_serves_a_framed_tar_and_guards_refuse_cob_raw_oids_ "archiving cob-only tree must be refused" ); assert!( - !cob.windows("README.md".len()) - .any(|window| window == b"README.md"), + !contains(&cob, b"README.md"), "refused archive mustn't leak the hidden tree's contents" ); } @@ -834,7 +832,7 @@ async fn http_boundary_encoding_and_streaming() { ); let collected = streamed.into_body().collect().await.unwrap().to_bytes(); assert!( - collected.windows(4).any(|window| window == b"PACK"), + contains(&collected, b"PACK"), "streamed response must contain a real PACK" ); } diff --git a/knot2/crates/knot-pack/tests/h3_conformance.rs b/knot2/crates/knot-pack/tests/h3_conformance.rs index 6b23d34d..719dfd4b 100644 --- a/knot2/crates/knot-pack/tests/h3_conformance.rs +++ b/knot2/crates/knot-pack/tests/h3_conformance.rs @@ -23,7 +23,7 @@ use tokio::task::JoinHandle; use tokio_util::sync::CancellationToken; mod common; -use common::{must, pack_objects, receive_request}; +use common::{contains, must, pack_objects, receive_request}; type Captured = (Method, Uri, HeaderMap, Bytes); @@ -373,9 +373,7 @@ fn fetch_request(log: &Arc>>) -> Captured { .find(|(method, uri, _, body)| { method == Method::POST && uri.path().ends_with("/git-upload-pack") - && body - .windows(b"command=fetch".len()) - .any(|window| window == b"command=fetch") + && contains(body, b"command=fetch") }) .cloned() .unwrap_or_else(|| { diff --git a/knot2/crates/knot-pack/tests/hardening.rs b/knot2/crates/knot-pack/tests/hardening.rs index 89708645..d8cfd7f8 100644 --- a/knot2/crates/knot-pack/tests/hardening.rs +++ b/knot2/crates/knot-pack/tests/hardening.rs @@ -7,8 +7,8 @@ use knot_types::{ObjectCount, ObjectFormat, Oid, RefName, RepoDid}; mod common; use common::{ - commit, delta_bomb_pack, index_into_bare, must, pack_objects, pack_objects_tuned, pkt, - receive_request, seeded, unsideband, + commit, contains, delta_bomb_pack, index_into_bare, must, pack_objects, pack_objects_tuned, + pkt, receive_request, seeded, unsideband, }; fn generous() -> PackLimits { @@ -679,7 +679,7 @@ fn v2_fetch_negotiation_acks_readies_waits_and_ignores_unknowns() { "must open packfile section after ready" ); assert!( - bytes.windows(4).any(|window| window == b"PACK"), + contains(&bytes, b"PACK"), "side-band payload must contain a real PACK" ); @@ -714,7 +714,7 @@ fn v2_fetch_negotiation_acks_readies_waits_and_ignores_unknowns() { "once done arrives server opens the pack:\n{finished_text}" ); assert!( - finished.windows(4).any(|window| window == b"PACK"), + contains(&finished, b"PACK"), "follow-up round must contain a real PACK" ); diff --git a/knot2/crates/knot-ssh/tests/ssh_push.rs b/knot2/crates/knot-ssh/tests/ssh_push.rs index f50326e5..a7899473 100644 --- a/knot2/crates/knot-ssh/tests/ssh_push.rs +++ b/knot2/crates/knot-ssh/tests/ssh_push.rs @@ -1360,7 +1360,7 @@ async fn git_archive_remote_over_ssh_streams_a_tar_of_the_tree() { let tar = std::fs::read(&out_tar).unwrap(); assert!( - tar.windows(b"README.md".len()).any(|w| w == b"README.md"), + knot_fixtures::contains(&tar, b"README.md"), "archived tar must contain the README.md entry" ); }