Re: [pve-devel] [PATCH v4] fix #5207: apt: check signage of repos with gpgv for POM

[Nicolas Frey <n.frey@proxmox.com>]

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 <n.frey@proxmox.com>
> ---
>  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<String> {
> @@ -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