From 9c8047b68094dba0a4c75ef435f5f4ae96558a18 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 23:23:55 +0000 Subject: [PATCH] apt proxies: keep pool and by-hash files immutable only once verified An apt proxy stored every pool file and by-hash copy as immutable, even when it had checked nothing: a package no Packages index lists (Ubuntu's _all packages), one fetched while the index was unavailable, and MD5Sum, SHA1 and SHA512 by-hash copies. A wrong copy was then served for good. - The plugin marks a pool file immutable only when the upstream's Packages index gave it a size and SHA-256, and a by-hash copy only when its name is a digest of its hash type. FileLocation carries SHA-512, SHA-1 and MD5 too, and the host checks every one it gets. - A re-validation whose cached copy contradicts the index's digest fetches unconditionally. - Invalidate cache releases an apt proxy's immutable files: they are re-validated without validators on their next download, checked anew and replaced when wrong. - DELETE /admin/repositories/{name}/files evicts a file of a package format's proxy; the repository page gets a "Cached files" panel for apt proxies. Fixes #760 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012e6DJYJ9Bfma4cFLf9LWgh --- crates/db/src/raw_dao.rs | 24 +++ crates/formats/apt/src/layout.rs | 80 +++++++-- crates/formats/apt/src/proxy.rs | 99 ++++++++++- crates/plugin-api/src/types.rs | 18 +- crates/server/src/admin/proxy_cache.rs | 21 ++- crates/server/src/files/proxy.rs | 66 ++++++- crates/server/src/files/web.rs | 10 +- crates/server/tests/apt.rs | 171 +++++++++++++++++++ docs/apt.md | 21 ++- docs/changes/2.2.0/760.fixed.md | 13 ++ docs/format-plugins.md | 2 +- web/src/api/repositories.ts | 16 ++ web/src/components/CachedFilesPanel.test.tsx | 77 +++++++++ web/src/components/CachedFilesPanel.tsx | 63 +++++++ web/src/components/ProxyCachePanel.tsx | 7 +- web/src/queries/useRepositories.ts | 8 + web/src/routes/admin/repositories/Edit.tsx | 4 + 17 files changed, 656 insertions(+), 44 deletions(-) create mode 100644 docs/changes/2.2.0/760.fixed.md create mode 100644 web/src/components/CachedFilesPanel.test.tsx create mode 100644 web/src/components/CachedFilesPanel.tsx diff --git a/crates/db/src/raw_dao.rs b/crates/db/src/raw_dao.rs index 7444dad6..4d702ea6 100644 --- a/crates/db/src/raw_dao.rs +++ b/crates/db/src/raw_dao.rs @@ -488,6 +488,30 @@ where Ok(result.rows_affected()) } +/// Proxies: make the cached files of `repository_id` that were kept as +/// immutable (an apt proxy's packages and `by-hash` copies) ordinary cached +/// files again, due for re-validation and without the upstream's +/// validators, so each is fetched from the upstream again on its next +/// download and checked anew. The cached copy is served while the upstream +/// cannot be reached. Returns how many there were. +pub async fn release_immutable<'e, E>(executor: E, repository_id: i64) -> sqlx::Result +where + E: PgExecutor<'e>, +{ + let result = sqlx::query( + r#" + UPDATE raw_file + SET immutable = false, checked_at = NULL, + upstream_etag = NULL, upstream_last_modified = NULL + WHERE repository_id = $1 AND immutable + "#, + ) + .bind(repository_id) + .execute(executor) + .await?; + Ok(result.rows_affected()) +} + /// Proxies: make the cached file at `path` due for re-validation. Returns /// whether it was fresh. pub async fn expire_file_check<'e, E>( diff --git a/crates/formats/apt/src/layout.rs b/crates/formats/apt/src/layout.rs index 49c395a9..2f4bc45d 100644 --- a/crates/formats/apt/src/layout.rs +++ b/crates/formats/apt/src/layout.rs @@ -194,13 +194,49 @@ pub fn is_by_hash(rest: &str) -> bool { && hash.bytes().all(|b| b.is_ascii_hexdigit()) } -/// The SHA-256 a `by-hash/SHA256/` path names. -pub fn by_hash_sha256(rest: &str) -> Option { - let hash = rest - .strip_prefix("by-hash/SHA256/") - .or_else(|| rest.rsplit_once("/by-hash/SHA256/").map(|(_, hash)| hash))?; - (hash.len() == 64 && hash.bytes().all(|b| b.is_ascii_hexdigit())) - .then(|| hash.to_ascii_lowercase()) +/// A hash type of `by-hash` paths, as apt names it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum HashType { + Md5, + Sha1, + Sha256, + Sha512, +} + +impl HashType { + fn named(name: &str) -> Option { + match name { + "MD5Sum" => Some(HashType::Md5), + "SHA1" => Some(HashType::Sha1), + "SHA256" => Some(HashType::Sha256), + "SHA512" => Some(HashType::Sha512), + _ => None, + } + } + + /// The length of its digest in hex digits. + fn hex_len(self) -> usize { + match self { + HashType::Md5 => 32, + HashType::Sha1 => 40, + HashType::Sha256 => 64, + HashType::Sha512 => 128, + } + } +} + +/// The hash type and digest (lowercase hex) a `by-hash//` path +/// names; `None` when the digest is not one of its type. +pub fn by_hash_digest(rest: &str) -> Option<(HashType, String)> { + let mut segments = rest.rsplit('/'); + let (Some(hash), Some(algorithm), Some("by-hash")) = + (segments.next(), segments.next(), segments.next()) + else { + return None; + }; + let kind = HashType::named(algorithm)?; + (hash.len() == kind.hex_len() && hash.bytes().all(|b| b.is_ascii_hexdigit())) + .then(|| (kind, hash.to_ascii_lowercase())) } /// Whether a pool path is where a hosted package of `component` and `name` @@ -361,12 +397,36 @@ mod tests { "main/binary-amd64/by-hash/SHA256/{hash}" ))); assert_eq!( - by_hash_sha256(&format!("main/binary-amd64/by-hash/SHA256/{hash}")), - Some(hash.clone()) + by_hash_digest(&format!("main/binary-amd64/by-hash/SHA256/{hash}")), + Some((HashType::Sha256, hash.clone())) ); assert!(!is_by_hash("main/binary-amd64/Packages")); assert!(!is_by_hash("by-hash/SHA256/xyz")); - assert_eq!(by_hash_sha256("main/by-hash/SHA256/abc"), None); + assert_eq!(by_hash_digest("main/by-hash/SHA256/abc"), None); + // Every hash type apt asks for, each with its digest's length. + for (name, kind, len) in [ + ("MD5Sum", HashType::Md5, 32), + ("SHA1", HashType::Sha1, 40), + ("SHA512", HashType::Sha512, 128), + ] { + let hex = "AB".repeat(len / 2); + assert_eq!( + by_hash_digest(&format!("main/i18n/by-hash/{name}/{hex}")), + Some((kind, hex.to_ascii_lowercase())) + ); + assert_eq!( + by_hash_digest(&format!("main/i18n/by-hash/{name}/{}", "a".repeat(len - 2))), + None + ); + } + assert_eq!( + by_hash_digest(&format!("by-hash/SHA384/{}", "a".repeat(96))), + None + ); + assert_eq!( + by_hash_digest(&format!("by-hash/SHA256/{hash}")).unwrap().0, + HashType::Sha256 + ); } #[test] diff --git a/crates/formats/apt/src/proxy.rs b/crates/formats/apt/src/proxy.rs index 32ac5d48..707f2975 100644 --- a/crates/formats/apt/src/proxy.rs +++ b/crates/formats/apt/src/proxy.rs @@ -3,8 +3,10 @@ //! verify the upstream's signatures with the upstream's keys. //! //! * `dists//…` (the distribution the proxy is for): the -//! indexes, re-validated once the repository's metadata TTL has passed; -//! their `by-hash` copies never change. +//! indexes, re-validated once the repository's metadata TTL has passed. +//! A `by-hash//` copy (`MD5Sum`, `SHA1`, `SHA256` or +//! `SHA512`) is checked against the digest its path names, and never +//! changes once it matched. //! * `pool/…`: packages, which never change at their path. Before a //! package is cached the host checks it against the size and SHA-256 the //! upstream's `Packages` index of its component and architecture gives it @@ -13,6 +15,12 @@ //! last before the name's prefix: `main` for the Debian security //! archive's `pool/updates/main/o/openssl/…`, which //! `dists//main/binary-/Packages` lists. +//! +//! Only a file so checked is kept as immutable. One the host could not +//! check (no index lists it, the index could not be fetched, Ubuntu's +//! `_all` packages without a `binary-all` index) is cached as an index is, +//! and re-validated once the metadata TTL has passed: a wrong copy is not +//! kept for good. use std::io::{BufRead, BufReader, Read}; @@ -20,7 +28,7 @@ use forge_plugin_api::{ Fetch, FetchResult, Fetched, FileLocation, Freshness, Remote, UpstreamAnswer, UpstreamStep, }; -use crate::layout::{self, Target}; +use crate::layout::{self, HashType, Target}; /// The most an index may inflate to while it is searched. const MAX_INDEX_BYTES: u64 = 1024 * 1024 * 1024; @@ -55,11 +63,18 @@ pub fn file(remote: &Remote, path: &str, fetched: &[FetchResult]) -> UpstreamSte let mut location = FileLocation::at(url); match layout::parse(path) { Ok(Target::Dists { rest, .. }) => { - location.immutable = layout::is_by_hash(&rest); - location.sha256 = layout::by_hash_sha256(&rest); + if let Some((kind, digest)) = layout::by_hash_digest(&rest) { + let field = match kind { + HashType::Md5 => &mut location.md5, + HashType::Sha1 => &mut location.sha1, + HashType::Sha256 => &mut location.sha256, + HashType::Sha512 => &mut location.sha512, + }; + *field = Some(digest); + location.immutable = true; + } } Ok(Target::Pool(pool)) => { - location.immutable = true; if let (Some(deb), Some(distribution)) = (layout::deb_name(&pool.file), remote.option("distribution")) { @@ -70,6 +85,8 @@ pub fn file(remote: &Remote, path: &str, fetched: &[FetchResult]) -> UpstreamSte match listed(remote, &dir, path, fetched) { Lookup::Fetch(fetch) => return UpstreamStep::Fetch(vec![fetch]), Lookup::Found { size, sha256 } => { + // Kept for good only once both are checked. + location.immutable = size.is_some() && sha256.is_some(); location.size = size; location.sha256 = sha256; } @@ -349,6 +366,38 @@ mod tests { }; assert!(at.immutable); assert_eq!(at.sha256, Some(hash)); + // Every hash type apt asks for is checked against its digest (#760). + for (name, len) in [("MD5Sum", 32), ("SHA1", 40), ("SHA512", 128)] { + let hash = "e".repeat(len); + let UpstreamStep::Done(UpstreamAnswer::FileAt(at)) = file( + &remote(), + &format!("dists/bookworm/main/binary-amd64/by-hash/{name}/{hash}"), + &[], + ) else { + panic!() + }; + assert!(at.immutable, "{name}"); + let given = match name { + "MD5Sum" => &at.md5, + "SHA1" => &at.sha1, + _ => &at.sha512, + }; + assert_eq!(given.as_deref(), Some(hash.as_str()), "{name}"); + assert_eq!(at.sha256, None); + } + // A copy whose name is no digest of its type is not kept for good. + let UpstreamStep::Done(UpstreamAnswer::FileAt(at)) = file( + &remote(), + &format!( + "dists/bookworm/main/binary-amd64/by-hash/SHA512/{}", + "e".repeat(64) + ), + &[], + ) else { + panic!() + }; + assert!(!at.immutable); + assert!(!at.has_digest()); } #[test] @@ -387,12 +436,48 @@ mod tests { assert!(at.immutable); assert_eq!(at.size, Some(42)); assert!(at.sha256.is_some()); - // An index that is down leaves the package to apt's own check. + // An index that is down leaves the package to apt's own check, and + // the copy is re-validated, not kept for good (#760). let down = result(&fetches[0].url, Fetched::Failed("503".into())); let UpstreamStep::Done(UpstreamAnswer::FileAt(at)) = file(&remote(), path, &[down]) else { panic!() }; assert_eq!((at.size, at.sha256), (None, None)); + assert!(!at.immutable); + // So does an index that does not list the package, or lists it + // without a usable digest. + let index = result(&fetches[0].url, Fetched::Found(INDEX.as_bytes().to_vec())); + for path in ["pool/main/x/x/x_1_amd64.deb", "pool/main/a/a/a_1_amd64.deb"] { + let UpstreamStep::Done(UpstreamAnswer::FileAt(at)) = + file(&remote(), path, std::slice::from_ref(&index)) + else { + panic!() + }; + assert!(!at.immutable, "{path}"); + } + // No index anywhere (Ubuntu's `binary-all`). + let gone: Vec<_> = ["Packages.gz", "Packages.xz", "Packages"] + .iter() + .map(|name| { + result( + &format!( + "http://deb.example.test/debian/dists/bookworm/main/binary-amd64/{name}" + ), + Fetched::NotFound, + ) + }) + .collect(); + let UpstreamStep::Done(UpstreamAnswer::FileAt(at)) = file(&remote(), path, &gone) else { + panic!() + }; + assert!(!at.immutable); + // A package the plugin cannot place in an index. + let UpstreamStep::Done(UpstreamAnswer::FileAt(at)) = + file(&remote(), "pool/main/h/hello/README", &[]) + else { + panic!() + }; + assert!(!at.immutable); } #[test] diff --git a/crates/plugin-api/src/types.rs b/crates/plugin-api/src/types.rs index d7c97e6e..704e9df2 100644 --- a/crates/plugin-api/src/types.rs +++ b/crates/plugin-api/src/types.rs @@ -1453,7 +1453,9 @@ pub struct FileLocation { /// On an origin a [`Fetch`] may reach. pub url: String, /// The file never changes at its path (a package, a content-addressed - /// copy): the host keeps its copy without re-validating it. + /// copy): the host keeps its copy without re-validating it. A plugin + /// sets it only with a digest below that the content must match, so + /// what is kept is the file the upstream's index (or the path) names. pub immutable: bool, /// The size the upstream's index gives the file: the host caches no /// content of another size, and answers 502. @@ -1461,6 +1463,12 @@ pub struct FileLocation { /// The SHA-256 (lowercase hex) the upstream's index gives the file: /// the host caches no content of another digest, and answers 502. pub sha256: Option, + /// The SHA-512 (lowercase hex), checked as [`Self::sha256`] is. + pub sha512: Option, + /// The SHA-1 (lowercase hex), checked as [`Self::sha256`] is. + pub sha1: Option, + /// The MD5 (lowercase hex), checked as [`Self::sha256`] is. + pub md5: Option, } impl FileLocation { @@ -1471,8 +1479,16 @@ impl FileLocation { immutable: false, size: None, sha256: None, + sha512: None, + sha1: None, + md5: None, } } + + /// Whether the location names a digest the content must match. + pub fn has_digest(&self) -> bool { + self.sha256.is_some() || self.sha512.is_some() || self.sha1.is_some() || self.md5.is_some() + } } /// What a repository's indexes are made of ([`FormatPlugin::index`]). diff --git a/crates/server/src/admin/proxy_cache.rs b/crates/server/src/admin/proxy_cache.rs index c22e3d08..de3372d3 100644 --- a/crates/server/src/admin/proxy_cache.rs +++ b/crates/server/src/admin/proxy_cache.rs @@ -23,12 +23,16 @@ //! while the upstream is unreachable; //! * a docker proxy's cached tags are re-validated on their next pull; //! * a raw proxy's cached files are re-validated on their next download, and -//! so are the upstream's indexes an apt proxy keeps (its packages and -//! `by-hash` copies never change). +//! so are the upstream's files an apt proxy keeps. A whole-repository +//! invalidation also releases the apt proxy's immutable files (packages +//! and `by-hash` copies): each is fetched from the upstream again on its +//! next download, checked anew, and replaced when it is not the file the +//! upstream has (the cached copy is served while the upstream is down). //! //! What stays: downloaded packages, blobs and digest-addressed manifests -//! (removing those is retention's job), the versions the upstream listed and -//! their package-delay clock. +//! (removing those is retention's job, or for an apt proxy's files +//! `DELETE /admin/repositories/{name}/files?path=…`), the versions the +//! upstream listed and their package-delay clock. use axum::extract::{Path, State}; use axum::http::StatusCode; @@ -66,7 +70,7 @@ pub struct Invalidated { pub not_found_cleared: u64, /// Tags due for re-validation (docker proxies). pub tags_expired: u64, - /// Cached files due for re-validation (raw proxies, and apt's indexes). + /// Cached files due for re-validation (raw proxies, and apt's files). #[serde(skip_serializing_if = "Option::is_none")] pub files_expired: Option, /// Stored upstream documents deleted (package formats' proxies: npm @@ -188,9 +192,10 @@ async fn forget( proxy_dao::clear_not_found(&mut tx, repo.id, package).await?; if package.is_none() { proxy_dao::expire_upstream_index(&mut tx, repo.id).await?; - // The upstream's files a package format's proxy keeps (apt's - // indexes); the ones that never change stay as they are. - let expired = raw_dao::expire_checks(&mut *tx, repo.id, "").await?; + // The upstream's files a package format's proxy keeps (apt's), + // those kept as immutable too: a wrong one is not kept for good. + let released = raw_dao::release_immutable(&mut *tx, repo.id).await?; + let expired = released + raw_dao::expire_checks(&mut *tx, repo.id, "").await?; if expired > 0 { invalidated.files_expired = Some(expired); } diff --git a/crates/server/src/files/proxy.rs b/crates/server/src/files/proxy.rs index 36c95bcc..ee60ead6 100644 --- a/crates/server/src/files/proxy.rs +++ b/crates/server/src/files/proxy.rs @@ -219,12 +219,14 @@ impl Files<'_> { })?; let stored = &ingested.stored; forge_telemetry::proxy::upstream_bytes(&repo.name, stored.size); - let wrong_size = location.size.is_some_and(|size| size != stored.size); - let wrong_digest = location - .sha256 - .as_deref() - .is_some_and(|sha256| !sha256.eq_ignore_ascii_case(&stored.sha256)); - if wrong_size || wrong_digest { + let digests = Digests { + size: stored.size, + sha256: &stored.sha256, + sha512_base64: &stored.sha512_base64, + sha1: &ingested.sha1, + md5: &ingested.md5, + }; + if !digests.match_location(location) { tracing::warn!( repo = %repo.name, %path, @@ -232,6 +234,9 @@ impl Files<'_> { sha256 = %stored.sha256, expected_size = ?location.size, expected_sha256 = ?location.sha256, + expected_sha512 = ?location.sha512, + expected_sha1 = ?location.sha1, + expected_md5 = ?location.md5, "file proxy: the upstream's file is not the one its index names; not cached" ); return Err(FileError::BadGateway( @@ -333,8 +338,11 @@ impl Files<'_> { raw_dao::mark_checked(&self.state.pool, file.id, None, None).await?; return Ok(file.clone()); }; + // A copy that is not the file the upstream's index names now is + // fetched again whatever its validators say. + let conditional = Digests::of(file).is_some_and(|d| d.match_location(&location)); let response = self - .upstream_get(repo, &file.path, &location.url, Some(file)) + .upstream_get(repo, &file.path, &location.url, conditional.then_some(file)) .await?; let status = response.status(); if status == StatusCode::NOT_MODIFIED { @@ -432,6 +440,50 @@ impl Files<'_> { } } +/// What content is: its size and digests (hex, SHA-512 in base64 as the +/// store keeps it). +struct Digests<'a> { + size: u64, + sha256: &'a str, + sha512_base64: &'a str, + sha1: &'a str, + md5: &'a str, +} + +impl<'a> Digests<'a> { + /// A cached file's. + fn of(file: &'a RawFile) -> Option { + Some(Digests { + size: u64::try_from(file.size).ok()?, + sha256: &file.blob_sha256, + sha512_base64: &file.sha512_base64, + sha1: &file.sha1, + md5: &file.md5, + }) + } + + /// Whether the content is the size and has every digest `location` + /// gives it. + fn match_location(&self, location: &FileLocation) -> bool { + let hex = |expected: &Option, actual: &str| { + expected + .as_deref() + .is_none_or(|expected| expected.eq_ignore_ascii_case(actual)) + }; + let sha512 = location.sha512.as_deref().is_none_or(|expected| { + use base64::Engine as _; + base64::engine::general_purpose::STANDARD + .decode(self.sha512_base64) + .is_ok_and(|bytes| expected.eq_ignore_ascii_case(&hex::encode(bytes))) + }); + location.size.is_none_or(|size| size == self.size) + && hex(&location.sha256, self.sha256) + && hex(&location.sha1, self.sha1) + && hex(&location.md5, self.md5) + && sha512 + } +} + /// Whether a cached file is due for re-validation after `max_age` seconds. fn is_due(file: &RawFile, max_age: u64) -> bool { let max_age = chrono::Duration::seconds(i64::try_from(max_age).unwrap_or(i64::MAX / 1000)); diff --git a/crates/server/src/files/web.rs b/crates/server/src/files/web.rs index 2b1a801f..eacea5e2 100644 --- a/crates/server/src/files/web.rs +++ b/crates/server/src/files/web.rs @@ -18,7 +18,8 @@ //! * `DELETE /admin/repositories/{name}/files?path=` (admin API) — //! delete a file, or with a path ending in `/` (or empty: everything) a //! directory and all files below it, in hosted and proxy repositories -//! (a proxy's cached copies). Answers `{"deleted": }`. +//! (a proxy's cached copies; for a package format's proxy, the upstream's +//! files it keeps, as an apt proxy's). Answers `{"deleted": }`. use axum::extract::{Path, Query, Request, State}; use axum::http::StatusCode; @@ -197,10 +198,13 @@ pub async fn admin_delete( Ok(None) => return (StatusCode::NOT_FOUND, "repository not found").into_response(), Err(e) => return crate::admin::db_error(e), }; - if !state.formats.stores_files(&repo.format) { + // A package format's proxy may keep the upstream's files (apt's), which + // an administrator evicts here; a hosted repository's are its indexes. + let proxy_files = repo.kind() == RepoKind::Proxy && !state.formats.stores_images(&repo.format); + if !state.formats.stores_files(&repo.format) && !proxy_files { return ( StatusCode::BAD_REQUEST, - "files are deleted from repositories of files (raw)", + "files are deleted from repositories of files (raw) and from proxies", ) .into_response(); } diff --git a/crates/server/tests/apt.rs b/crates/server/tests/apt.rs index aabd0b3e..15706b1a 100644 --- a/crates/server/tests/apt.rs +++ b/crates/server/tests/apt.rs @@ -1116,3 +1116,174 @@ async fn proxies_serve_the_security_archives_pool() { assert_eq!(status, StatusCode::OK); assert_eq!(body, hello); } + +/// The `Packages` index of `bookworm/main/binary-amd64` listing `deb` as +/// `hello` 2.10-3. +fn hello_index(deb: &[u8]) -> Vec { + format!( + "Package: hello\nVersion: 2.10-3\nFilename: pool/main/h/hello/hello_2.10-3_amd64.deb\nSize: {}\nSHA256: {}\n", + deb.len(), + sha256(deb) + ) + .into_bytes() +} + +/// A mock mirror with `files` (paths below `/debian/`) and an apt proxy of +/// it whose metadata TTL is a second. +async fn proxy_of(srv: &TestServer, files: Vec<(String, Vec)>) -> (Arc, Apt, String) { + let state = Arc::new(Mirror::default()); + let url = mirror(state.clone()).await; + state.files.lock().unwrap().extend( + files + .into_iter() + .map(|(path, body)| (format!("debian/{path}"), body)), + ); + let name = format!("aptverified{}", unique()); + create( + srv, + json!({"name": name, "kind": "proxy", "format": "apt", "remote_url": format!("{url}/debian/"), + "metadata_ttl_seconds": 1, "options": {"distribution": "bookworm"}}), + ) + .await; + let apt = Apt::new(srv, &name).await; + (state, apt, name) +} + +const HELLO: &str = "pool/main/h/hello/hello_2.10-3_amd64.deb"; + +/// A `by-hash` copy is checked against its digest for every hash type apt +/// asks for, not only `SHA256` (#760). +#[tokio::test] +async fn proxies_check_by_hash_copies_of_every_hash_type() { + let srv = common::start().await; + let content = b"Translation-en".to_vec(); + let digests = [ + ("MD5Sum", hex::encode(md5::Md5::digest(&content))), + ("SHA1", hex::encode(sha1::Sha1::digest(&content))), + ("SHA512", hex::encode(sha2::Sha512::digest(&content))), + ]; + let mut files: Vec<_> = digests + .iter() + .map(|(kind, hex)| { + ( + format!("dists/bookworm/main/i18n/by-hash/{kind}/{hex}"), + content.clone(), + ) + }) + .collect(); + // Copies whose content is not their digest. + for (kind, hex) in &digests { + files.push(( + format!( + "dists/bookworm/main/i18n/by-hash/{kind}/{}", + "1".repeat(hex.len()) + ), + b"forged".to_vec(), + )); + } + let (_, apt, _) = proxy_of(&srv, files).await; + for (kind, hex) in &digests { + let path = format!("dists/bookworm/main/i18n/by-hash/{kind}/{hex}"); + assert_eq!( + apt.get(&path).await, + (StatusCode::OK, content.clone()), + "{kind}" + ); + let forged = format!( + "dists/bookworm/main/i18n/by-hash/{kind}/{}", + "1".repeat(hex.len()) + ); + assert_eq!(apt.get(&forged).await.0, StatusCode::BAD_GATEWAY, "{kind}"); + } +} + +/// A package the proxy could not check (no index lists it, as Ubuntu's +/// `_all` packages) is not kept for good: a new copy at its path replaces it +/// once the metadata TTL has passed (#760). +#[tokio::test] +async fn proxies_revalidate_packages_they_could_not_check() { + let srv = common::start().await; + let unlisted = "pool/main/h/hello/hello-doc_2.10-3_all.deb"; + let doc = deb("hello-doc", "2.10-3", "all", "doc"); + let (state, apt, _) = proxy_of(&srv, vec![(unlisted.into(), doc.clone())]).await; + assert_eq!(apt.get(unlisted).await, (StatusCode::OK, doc)); + let doc2 = deb("hello-doc", "2.10-3", "all", "docs"); + state + .files + .lock() + .unwrap() + .insert(format!("debian/{unlisted}"), doc2.clone()); + tokio::time::sleep(std::time::Duration::from_millis(1100)).await; + assert_eq!(apt.get(unlisted).await, (StatusCode::OK, doc2)); +} + +/// A checked package is kept for good, and an administrator evicts it: +/// **Invalidate cache** has it fetched again and checked anew, and a delete +/// of its path removes it (#760). +#[tokio::test] +async fn invalidate_cache_and_deletes_evict_checked_packages() { + let srv = common::start().await; + let hello = deb("hello", "2.10-3", "amd64", "devel"); + let (state, apt, name) = proxy_of( + &srv, + vec![ + ( + "dists/bookworm/main/binary-amd64/Packages".into(), + hello_index(&hello), + ), + (HELLO.into(), hello.clone()), + ], + ) + .await; + let upstream = format!("debian/{HELLO}"); + let seen = || state.seen.lock().unwrap()[&upstream]; + assert_eq!(apt.get(HELLO).await, (StatusCode::OK, hello.clone())); + // The upstream re-uploads the package under its path, and its index + // names the new one. + let hello2 = deb("hello", "2.10-3", "amd64", "utils"); + { + let mut files = state.files.lock().unwrap(); + files.insert(upstream.clone(), hello2.clone()); + files.insert( + "debian/dists/bookworm/main/binary-amd64/Packages".into(), + hello_index(&hello2), + ); + } + tokio::time::sleep(std::time::Duration::from_millis(1100)).await; + // Kept for good: not asked for again. + assert_eq!(apt.get(HELLO).await, (StatusCode::OK, hello.clone())); + assert_eq!(seen(), 1); + + let (status, text) = admin( + &srv, + reqwest::Method::POST, + &format!("/admin/repositories/{name}/proxy-cache/invalidate"), + None, + ) + .await; + assert_eq!(status, StatusCode::OK, "{text}"); + let invalidated: Value = serde_json::from_str(&text).unwrap(); + assert!( + invalidated["files_expired"].as_u64().unwrap() >= 1, + "{text}" + ); + assert_eq!(apt.get(HELLO).await, (StatusCode::OK, hello2.clone())); + assert_eq!(seen(), 2); + // The new copy was checked, and is kept for good again. + tokio::time::sleep(std::time::Duration::from_millis(1100)).await; + assert_eq!(apt.get(HELLO).await, (StatusCode::OK, hello2.clone())); + assert_eq!(seen(), 2); + + // A delete of the path evicts it at once. + let (status, text) = admin( + &srv, + reqwest::Method::DELETE, + &format!("/admin/repositories/{name}/files?path={HELLO}"), + None, + ) + .await; + assert_eq!(status, StatusCode::OK, "{text}"); + assert_eq!(serde_json::from_str::(&text).unwrap()["deleted"], 1); + assert_eq!(apt.get(HELLO).await, (StatusCode::OK, hello2)); + assert_eq!(seen(), 3); +} diff --git a/docs/apt.md b/docs/apt.md index 51c3f7f3..b3840ac4 100644 --- a/docs/apt.md +++ b/docs/apt.md @@ -230,19 +230,30 @@ deb [signed-by=/usr/share/keyrings/debian-archive-keyring.gpg] https://forge.exa - `InRelease`, `Release` and the other indexes are re-validated with the upstream once the repository's metadata TTL (`metadata_ttl_seconds`, 10 - minutes by default) has passed; `by-hash/` copies and pool files never - change, and are cached on their first download for good. + minutes by default) has passed. - A pool file is checked against the size and SHA-256 the upstream's `Packages` index of its component and architecture gives it (the distribution's `Packages.gz`, else `.xz`, else the plain one), when the index lists it: a file that does not match is not cached, and answers - `502`. A `by-hash` copy is checked against its hash. + `502`. A `by-hash` copy (`MD5Sum`, `SHA1`, `SHA256` or `SHA512`) is + checked against the digest its path names, the same way. +- A file so checked never changes, and is cached on its first download for + good. One that could not be checked (no index lists it, as Ubuntu's + `_all` packages, which no `binary-all` index lists; or the index could + not be fetched) is re-validated once the metadata TTL has passed, as an + index is. The `Packages` index itself is not checked against `InRelease`; + apt checks every file against the signed indexes. - While the upstream is down, what is cached is served; a file the upstream did not have is remembered for the not-found TTL. Upstream credentials are optional (`upstream_username`, `upstream_password`). - *Invalidate cache* on the repository's page (or - `POST /admin/repositories/{name}/proxy-cache/invalidate`) has the cached - indexes re-validated on their next download. + `POST /admin/repositories/{name}/proxy-cache/invalidate`) has every + cached file re-validated on its next download. A file kept for good is + fetched from the upstream again and checked anew, and replaced when it is + not the file the upstream has; the cached copy is served while the + upstream is down. *Delete cached file* on the same page (or + `DELETE /admin/repositories/{name}/files?path=`, a path ending in + `/` for a directory) evicts a file at once. There is no package delay for apt proxies: the indexes are the upstream's, signed, and cannot leave a version out. diff --git a/docs/changes/2.2.0/760.fixed.md b/docs/changes/2.2.0/760.fixed.md new file mode 100644 index 00000000..2508c756 --- /dev/null +++ b/docs/changes/2.2.0/760.fixed.md @@ -0,0 +1,13 @@ +An apt proxy kept every pool file and `by-hash` copy for good, never +re-validated, even when it could not check it: a package no `Packages` index +lists (Ubuntu's `_all` packages), one cached while the index could not be +fetched, and `MD5Sum`, `SHA1` and `SHA512` `by-hash` copies, of which only +`SHA256` ones were checked. A wrong copy, from an on-path attacker on a plain +`http://` upstream or a package re-uploaded under its path, then gave every +client "Hash Sum mismatch" until the proxy was recreated. A file is now kept +for good only once its size and digest were checked (a `by-hash` copy against +the digest its path names, for every hash type), and re-validated after the +metadata TTL otherwise. **Invalidate cache** has the files kept for good +fetched again and checked anew, and an administrator can evict one with +**Delete cached file** on the repository's page +(`DELETE /admin/repositories/{name}/files?path=`). diff --git a/docs/format-plugins.md b/docs/format-plugins.md index af34fe34..556a5d4e 100644 --- a/docs/format-plugins.md +++ b/docs/format-plugins.md @@ -481,7 +481,7 @@ what they say: | `manifest(id, version)` | The manifest, verified to be that version's, without downloading the package | | `metadata(versions)`, `dating(versions)` | Per version: listing state, publish time, a manifest summary for a version not downloaded yet, and (`dating`) when it was created upstream, which the package delay counts from | | `search(query)` | A page of the upstream's search results | -| `file(path)` | The file's URL, or none when the upstream cannot have it. The host downloads it (following redirects; credentials go to `remote_url`'s origin only), caches it, and re-validates it with a conditional GET once the repository's `max_age_seconds` (a package format: its metadata TTL) has passed (raw: `/`). Answered `file-at`, it says more: that the file never changes (`immutable`: never re-validated, as apt's packages and `by-hash` copies), and the size and SHA-256 the upstream's index gives it, which the host checks before it caches the file (502 when it does not match) | +| `file(path)` | The file's URL, or none when the upstream cannot have it. The host downloads it (following redirects; credentials go to `remote_url`'s origin only), caches it, and re-validates it with a conditional GET once the repository's `max_age_seconds` (a package format: its metadata TTL) has passed (raw: `/`). Answered `file-at`, it says more: that the file never changes (`immutable`: never re-validated, as apt's packages and `by-hash` copies once their digest is known; a plugin says so only with a digest to check), and the size and the SHA-256, SHA-512, SHA-1 or MD5 the upstream's index (or the path) gives it, which the host checks before it caches the file (502 when it does not match) | | `tags(id)` | The tags the upstream gives the package. The host keeps the last ones read, for when the upstream is down | Each operation is a conversation: `upstream` either answers or asks for diff --git a/web/src/api/repositories.ts b/web/src/api/repositories.ts index 8f05e337..c1a13fea 100644 --- a/web/src/api/repositories.ts +++ b/web/src/api/repositories.ts @@ -467,6 +467,22 @@ export function invalidateProxyCache( ) } +/** + * `DELETE /admin/repositories/{name}/files?path=` — evict a file a + * proxy cached from its upstream (a path ending in `/`: a directory and + * every file below it). The next download fetches it from the upstream + * again. + */ +export function deleteCachedFiles( + name: string, + path: string, +): Promise<{ deleted: number }> { + return httpClient.delete<{ deleted: number }>( + `/admin/repositories/${encodeURIComponent(name)}/files`, + { query: { path } }, + ) +} + /** `GET /admin/repositories/{name}/delay-releases` — a proxy's early releases, newest first. */ export function fetchDelayReleases( name: string, diff --git a/web/src/components/CachedFilesPanel.test.tsx b/web/src/components/CachedFilesPanel.test.tsx new file mode 100644 index 00000000..ed4f99d5 --- /dev/null +++ b/web/src/components/CachedFilesPanel.test.tsx @@ -0,0 +1,77 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { QueryClientProvider } from '@tanstack/react-query' + +import { makeQueryClient } from '~/lib/queryClient' +import { clearSession, setSession } from '~/lib/auth' +import type { RepoView } from '~/api/repositories' +import { CachedFilesPanel } from './CachedFilesPanel' + +/** + * The cached files panel of an apt proxy: **Delete cached file** evicts the + * path it names through the admin API (#760). + */ + +const repo = { name: 'debian', format: 'apt', kind: 'proxy' } as RepoView +const fetchMock = vi.fn() + +function renderPanel() { + render( + + + , + ) +} + +describe('CachedFilesPanel', () => { + beforeEach(() => { + vi.stubGlobal('fetch', fetchMock) + fetchMock.mockReset() + setSession('admin-csrf') + }) + afterEach(() => { + clearSession() + vi.unstubAllGlobals() + }) + + it('deletes the cached file it names', async () => { + const user = userEvent.setup() + fetchMock.mockResolvedValue( + new Response(JSON.stringify({ deleted: 1 }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ) + renderPanel() + + const button = screen.getByRole('button', { name: 'Delete cached file' }) + expect(button).toBeDisabled() + await user.type( + screen.getByLabelText(/File or directory/), + 'pool/main/h/hello/hello_2.10-3_amd64.deb', + ) + await user.click(button) + + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)) + const [input, init] = fetchMock.mock.calls[0] + const url = new URL(String(input), 'http://localhost') + expect((init as RequestInit).method).toBe('DELETE') + expect(url.pathname).toBe('/admin/repositories/debian/files') + expect(url.searchParams.get('path')).toBe('pool/main/h/hello/hello_2.10-3_amd64.deb') + expect( + await screen.findByText(/Deleted pool\/main\/h\/hello\/hello_2.10-3_amd64.deb/), + ).toBeInTheDocument() + }) + + it('shows why a delete failed', async () => { + const user = userEvent.setup() + fetchMock.mockResolvedValue(new Response('file not found', { status: 404 })) + renderPanel() + + await user.type(screen.getByLabelText(/File or directory/), 'pool/main/x/x/x_1_amd64.deb') + await user.click(screen.getByRole('button', { name: 'Delete cached file' })) + + expect(await screen.findByText('Could not delete the cached file')).toBeInTheDocument() + }) +}) diff --git a/web/src/components/CachedFilesPanel.tsx b/web/src/components/CachedFilesPanel.tsx new file mode 100644 index 00000000..8c42490e --- /dev/null +++ b/web/src/components/CachedFilesPanel.tsx @@ -0,0 +1,63 @@ +import { useState } from 'react' + +import type { RepoView } from '~/api/repositories' +import { useDeleteCachedFiles } from '~/queries/useRepositories' +import { Button, Notice, Panel, TextField } from '~/components/ui' + +/** + * Evicting a file an apt proxy cached from its upstream + * (`DELETE /admin/repositories/{name}/files?path=…`): a package or a + * `by-hash` copy is kept for good once it was checked, and this deletes it + * so the next download fetches it from the upstream again. A path ending in + * `/` evicts a directory. A panel of the edit screen's side column. + */ +export function CachedFilesPanel({ repo }: { repo: RepoView }) { + const remove = useDeleteCachedFiles(repo.name) + const [path, setPath] = useState('') + + function handleSubmit(e: React.FormEvent) { + e.preventDefault() + const target = path.trim().replace(/^\/+/, '') + if (target) remove.mutate(target) + } + + const done = remove.isSuccess ? remove.data : null + return ( + + setPath(e.target.value)} + placeholder="pool/main/h/hello/hello_2.10-3_amd64.deb" + description="A path ending in / deletes a directory and every file below it." + /> +
+ +
+ {done && ( + + {done.deleted === 1 + ? `Deleted ${remove.variables}: the next download fetches it again.` + : `Deleted ${done.deleted} cached files under ${remove.variables}.`} + + )} + {remove.isError && ( + + {remove.error.body || remove.error.message} + + )} + + ) +} diff --git a/web/src/components/ProxyCachePanel.tsx b/web/src/components/ProxyCachePanel.tsx index 589d1207..04116528 100644 --- a/web/src/components/ProxyCachePanel.tsx +++ b/web/src/components/ProxyCachePanel.tsx @@ -23,6 +23,7 @@ export function ProxyCachePanel({ repo }: { repo: RepoView }) { const [pkg, setPkg] = useState('') const docker = repo.format === 'docker' const raw = repo.format === 'raw' + const apt = repo.format === 'apt' function handleSubmit(e: React.FormEvent) { e.preventDefault() @@ -46,7 +47,9 @@ export function ProxyCachePanel({ repo }: { repo: RepoView }) { : 'The proxy keeps the upstream’s listings and documents for every instance and serves them without asking for the metadata TTL, and remembers a package the upstream did not have for the not-found TTL.'}{' '} {raw ? 'Invalidate the cache to ask the upstream again, for example after a file changed there. Cached files stay, and are re-validated on their next download.' - : 'Invalidate the cache to ask the upstream again, for example after a package was published there. Downloaded packages stay cached.'} + : apt + ? 'Invalidate the whole cache to ask the upstream again for every cached file: the indexes, and the packages and by-hash copies kept for good, which are fetched again and checked anew on their next download.' + : 'Invalidate the cache to ask the upstream again, for example after a package was published there. Downloaded packages stay cached.'} } > @@ -73,7 +76,7 @@ export function ProxyCachePanel({ repo }: { repo: RepoView }) { ? `${plural(done.tags_expired, 'tag is', 'tags are')} re-validated on the next pull.` : raw ? `${plural(done.files_expired ?? 0, 'cached file is', 'cached files are')} re-validated on the next download; “not found” answers are forgotten.` - : `${plural(done.not_found_cleared, '“not found” answer', '“not found” answers')} forgotten, ${plural(done.documents_dropped, 'stored upstream document', 'stored upstream documents')} deleted; listings are fetched again.`} + : `${plural(done.not_found_cleared, '“not found” answer', '“not found” answers')} forgotten, ${plural(done.documents_dropped, 'stored upstream document', 'stored upstream documents')} deleted; listings are fetched again.${done.files_expired ? ` ${plural(done.files_expired, 'cached file is', 'cached files are')} re-validated on the next download.` : ''}`} )} {invalidate.isError && ( diff --git a/web/src/queries/useRepositories.ts b/web/src/queries/useRepositories.ts index ae95aa70..d5353037 100644 --- a/web/src/queries/useRepositories.ts +++ b/web/src/queries/useRepositories.ts @@ -26,6 +26,7 @@ import { fetchTrustedPublishers, formatKeys, invalidateProxyCache, + deleteCachedFiles, releaseFromDelay, removeMember, repoKeys, @@ -155,6 +156,13 @@ export function useInvalidateProxyCache(name: string) { }) } +/** Evict a file (or a directory: a path ending in `/`) a proxy cached. */ +export function useDeleteCachedFiles(name: string) { + return useMutation<{ deleted: number }, ApiError, string>({ + mutationFn: (path) => deleteCachedFiles(name, path), + }) +} + /** A proxy's versions released from the package delay early. */ export function useDelayReleases(name: string, enabled = true) { return useQuery({ diff --git a/web/src/routes/admin/repositories/Edit.tsx b/web/src/routes/admin/repositories/Edit.tsx index 2b073a2e..5f72befd 100644 --- a/web/src/routes/admin/repositories/Edit.tsx +++ b/web/src/routes/admin/repositories/Edit.tsx @@ -26,6 +26,7 @@ import { } from '~/components/ui' import { ErrorState } from '~/components/ErrorState' import { ProxyCachePanel } from '~/components/ProxyCachePanel' +import { CachedFilesPanel } from '~/components/CachedFilesPanel' import { DelayReleasesPanel } from '~/components/DelayReleasesPanel' import { useSelectors } from '~/queries/useRbac' import { SigningKeysPanel } from '~/components/SigningKeysPanel' @@ -175,6 +176,9 @@ function EditRepository() { {current.kind === 'proxy' && ( )} + {current.kind === 'proxy' && current.format === 'apt' && ( + + )} {delayed && }