From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id E3A531FF165 for ; Thu, 23 Oct 2025 12:41:59 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4646B7227; Thu, 23 Oct 2025 12:42:27 +0200 (CEST) Message-ID: Date: Thu, 23 Oct 2025 12:41:52 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: pve-devel@lists.proxmox.com References: <20251020144553.360147-1-n.frey@proxmox.com> Content-Language: en-US From: Nicolas Frey In-Reply-To: <20251020144553.360147-1-n.frey@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1761216104422 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.863 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH v4] fix #5207: apt: check signage of repos with gpgv for POM X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Superseded-by: https://lore.proxmox.com/pve-devel/20251023103953.305810-1-n.frey@proxmox.com/T/#t On 10/20/25 4:45 PM, Nicolas Frey wrote: > If POM is set up to mirror the PVE repository and only this repository > is added on a PVE host, the `Repositories` panel will show an `Error` > status with the message: > > `No Proxmox VE repository is enabled, you do not get any updates!` > > This is because the current implementation only checks if the uri of > the repo matches that of one of the standard repos. > > This patch aims to fix this issue by verifying it through signage > info via gpgv. The InRelease file is used to check whether the > package is of Proxmox Origin. The cached file at `/var/lib/apt/lists/` > is used if present, otherwise it is fetched from the repos URI, which > solves the issue of needing to update the cached files, which would > need an apt update. This way the feedback is immediate even for POM > configured repos. > > Added tests to ensure common file paths for POM would be encoded > correctly. > > Changes since v3: > * Moved found_uri_or_signed to function and to the end of bool chain > to prevent redundant signage checks to improve performance > * Added fallback to the cached InRelease file to get it from repos URI > > Changes since v2: > * correct the mapping in `gpg_signed` > > Changes since v1: > * rewrite test so it compiles > > Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=5207 > Signed-off-by: Nicolas Frey > --- > proxmox-apt/Cargo.toml | 1 + > proxmox-apt/src/api.rs | 5 +- > proxmox-apt/src/repositories/mod.rs | 6 +- > proxmox-apt/src/repositories/repository.rs | 105 +++++++++++++++++++-- > 4 files changed, 105 insertions(+), 12 deletions(-) > > diff --git a/proxmox-apt/Cargo.toml b/proxmox-apt/Cargo.toml > index e5beb4e6..ae78e31c 100644 > --- a/proxmox-apt/Cargo.toml > +++ b/proxmox-apt/Cargo.toml > @@ -17,6 +17,7 @@ hex.workspace = true > openssl.workspace = true > serde = { workspace = true, features = ["derive"] } > serde_json.workspace = true > +proxmox-http = { workspace = true, features = [ "client-sync" ] } > > rfc822-like = "0.2.1" > > diff --git a/proxmox-apt/src/api.rs b/proxmox-apt/src/api.rs > index 14dfb53b..b5fa10f4 100644 > --- a/proxmox-apt/src/api.rs > +++ b/proxmox-apt/src/api.rs > @@ -8,6 +8,7 @@ use proxmox_apt_api_types::{ > APTRepositoryHandle, > }; > use proxmox_config_digest::ConfigDigest; > +use proxmox_http::client::sync::Client; > > use crate::repositories::{APTRepositoryFileImpl, APTRepositoryImpl}; > > @@ -61,10 +62,12 @@ pub fn add_repository_handle( > > let suite = crate::repositories::get_current_release_codename()?; > > + let client = Client::default(); > + > // check if it's already configured first > for file in files.iter_mut() { > for repo in file.repositories.iter_mut() { > - if repo.is_referenced_repository(handle, "pbs", &suite.to_string()) { > + if repo.is_referenced_repository(handle, "pbs", &suite.to_string(), &client) { > if repo.enabled { > return Ok(()); > } > diff --git a/proxmox-apt/src/repositories/mod.rs b/proxmox-apt/src/repositories/mod.rs > index 2b52c774..4fa043fc 100644 > --- a/proxmox-apt/src/repositories/mod.rs > +++ b/proxmox-apt/src/repositories/mod.rs > @@ -10,6 +10,7 @@ use proxmox_apt_api_types::{ > APTStandardRepository, > }; > use proxmox_config_digest::ConfigDigest; > +use proxmox_http::client::sync::Client; > pub use repository::APTRepositoryImpl; > > mod file; > @@ -100,6 +101,8 @@ pub fn standard_repositories( > ]); > } > > + let client = Client::default(); > + > for file in files.iter() { > for repo in file.repositories.iter() { > for entry in result.iter_mut() { > @@ -107,7 +110,8 @@ pub fn standard_repositories( > continue; > } > > - if repo.is_referenced_repository(entry.handle, product, &suite.to_string()) { > + if repo.is_referenced_repository(entry.handle, product, &suite.to_string(), &client) > + { > entry.status = Some(repo.enabled); > } > } > diff --git a/proxmox-apt/src/repositories/repository.rs b/proxmox-apt/src/repositories/repository.rs > index 24e7943b..47cf50e3 100644 > --- a/proxmox-apt/src/repositories/repository.rs > +++ b/proxmox-apt/src/repositories/repository.rs > @@ -1,7 +1,10 @@ > use std::io::{BufRead, BufReader, Write}; > use std::path::{Path, PathBuf}; > +use std::process::{Command, Stdio}; > > use anyhow::{bail, format_err, Error}; > +use proxmox_http::client::sync::Client; > +use proxmox_http::HttpClient; > > use crate::repositories::standard::APTRepositoryHandleImpl; > use proxmox_apt_api_types::{ > @@ -25,6 +28,7 @@ pub trait APTRepositoryImpl { > handle: APTRepositoryHandle, > product: &str, > suite: &str, > + client: &Client, > ) -> bool; > > /// Guess the origin from the repository's URIs. > @@ -121,22 +125,26 @@ impl APTRepositoryImpl for APTRepository { > handle: APTRepositoryHandle, > product: &str, > suite: &str, > + client: &Client, > ) -> bool { > - let (package_type, handle_uris, component, _key) = handle.info(product); > - > - let mut found_uri = false; > - > - for uri in self.uris.iter() { > - let uri = uri.trim_end_matches('/'); > - > - found_uri = found_uri || handle_uris.iter().any(|handle_uri| handle_uri == uri); > - } > + let (package_type, handle_uris, component, key) = handle.info(product); > + > + let found_uri_or_signed = || { > + let mut found = false; > + for uri in self.uris.iter() { > + let uri = uri.trim_end_matches('/'); > + found = found > + || handle_uris.iter().any(|handle_uri| handle_uri == uri) > + || gpg_signed(&client, uri, suite, key); > + } > + found > + }; > > self.types.contains(&package_type) > - && found_uri > // using contains would require a &String > && self.suites.iter().any(|self_suite| self_suite == suite) > && self.components.contains(&component) > + && found_uri_or_signed() > } > > fn origin_from_uris(&self) -> Option { > @@ -389,8 +397,85 @@ fn write_stanza(repo: &APTRepository, w: &mut dyn Write) -> Result<(), Error> { > Ok(()) > } > > +fn gpg_signed(client: &Client, uri: &str, suite: &str, key: &str) -> bool { > + let path = release_filename(Path::new("/var/lib/apt/lists"), uri, suite, false); > + > + // check cached file path first > + let data = if path.exists() { > + let Ok(d) = std::fs::read(path) else { > + return false; > + }; > + d > + } else if uri.starts_with("file://") { > + let Ok(d) = std::fs::read(&format!( > + "{}/dists/{}/InRelease", > + &uri["file://".len()..], > + suite > + )) else { > + return false; > + }; > + d > + } else { > + match client.get( > + &format!("{}/dists/{}/InRelease", uri.trim_end_matches('/'), suite), > + None, > + ) { > + Ok(resp) if resp.status().is_success() => resp.into_body(), > + _ => return false, > + } > + }; > + > + let Ok(mut gpgv) = Command::new("gpgv") > + .args(["--keyring", key, "-"]) > + .stdin(Stdio::piped()) > + .stdout(Stdio::null()) > + .stderr(Stdio::null()) > + .spawn() > + else { > + return false; > + }; > + > + if let Some(mut stdin) = gpgv.stdin.take() { > + if stdin.write_all(&data).is_err() { > + let _ = gpgv.kill(); > + return false; > + } > + } > + > + gpgv.wait().is_ok_and(|status| status.success()) > +} > + > #[test] > fn test_uri_to_filename() { > let filename = uri_to_filename("https://some_host/some/path"); > assert_eq!(filename, "some%5fhost_some_path".to_string()); > } > + > +#[test] > +fn test_release_filename() { > + let data = [ > + // testcase for proxmox offline mirror (mounted) > + ( > + Path::new("/var/lib/apt/lists"), > + "file:///mnt/mirror/pve-no-subscription/2025-10-16T08:07:41Z", > + "trixie", > + false, > + // expected > + "/var/lib/apt/lists/_mnt_mirror_pve-no-subscription_2025-10-16T08:07:41Z_dists_trixie_InRelease" > + ), > + // testcase for proxmox offline mirror (local http server) > + ( > + Path::new("/var/lib/apt/lists"), > + "http://proxmox-offline-mirror.domain.example/pve-subscription/2025-10-16T08:07:41Z", > + "trixie", > + false, > + // expected > + "/var/lib/apt/lists/proxmox-offline-mirror.domain.example_pve-subscription_2025-10-16T08:07:41Z_dists_trixie_InRelease" > + ), > + ]; > + > + for d in data { > + let filename = release_filename(d.0, d.1, d.2, d.3).display().to_string(); > + assert_eq!(filename, d.4); > + } > +} _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel