Re: [pve-devel] [PATCH v6 2/4] fix #5207: apt: check signature of repos with proxmox-pgp

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On October 30, 2025 2:28 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 commit aims to fix this issue by verifying it through signature
> info via `proxmox-pgp`. The InRelease file cached at
> `/var/lib/apt/lists/` is used to check whether the package is of
> Proxmox Origin.
> 
> 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/repositories/repository.rs | 56 ++++++++++++++++++----
>  2 files changed, 47 insertions(+), 10 deletions(-)
> 
> diff --git a/proxmox-apt/Cargo.toml b/proxmox-apt/Cargo.toml
> index e5beb4e6..5a8e25eb 100644
> --- a/proxmox-apt/Cargo.toml
> +++ b/proxmox-apt/Cargo.toml
> @@ -23,6 +23,7 @@ rfc822-like = "0.2.1"
>  proxmox-apt-api-types.workspace = true
>  proxmox-config-digest = { workspace = true, features = ["openssl"] }
>  proxmox-sys.workspace = true
> +proxmox-pgp.workspace = true
>  
>  apt-pkg-native = { version = "0.3.2", optional = true }
>  regex = { workspace = true, optional = true }
> diff --git a/proxmox-apt/src/repositories/repository.rs b/proxmox-apt/src/repositories/repository.rs
> index 24e7943b..5e386665 100644
> --- a/proxmox-apt/src/repositories/repository.rs
> +++ b/proxmox-apt/src/repositories/repository.rs
> @@ -2,6 +2,7 @@ use std::io::{BufRead, BufReader, Write};
>  use std::path::{Path, PathBuf};
>  
>  use anyhow::{bail, format_err, Error};
> +use proxmox_pgp::{verify_signature, WeakCryptoConfig};
>  
>  use crate::repositories::standard::APTRepositoryHandleImpl;
>  use proxmox_apt_api_types::{
> @@ -122,21 +123,24 @@ impl APTRepositoryImpl for APTRepository {
>          product: &str,
>          suite: &str,
>      ) -> 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)
> +                    || is_signed_by_key(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,6 +393,38 @@ fn write_stanza(repo: &APTRepository, w: &mut dyn Write) -> Result<(), Error> {
>      Ok(())
>  }
>  
> +/// Reads file contents of cached/local POM InRelease file from uri
> +/// and key to verify pgp signature
> +fn is_signed_by_key(uri: &str, suite: &str, key_path: &str) -> bool {
> +    let data = match std::fs::read(&release_filename(
> +        Path::new("/var/lib/apt/lists"),
> +        uri,
> +        suite,
> +        false,
> +    )) {
> +        Ok(d) => d,
> +        Err(err) => {
> +            log::warn!("could not read InRelease file: {err}");

log is an optional dependency here, but this code path is reachable
without enabling it. this fn is also rather low level for such a high
(warning) level message.

if we keep it, I'd rather downgrade it to debug or the like, and please
add the path, because {err} will most likely only contain something like
"does not exist" or "missing permissions", which is not helpful ;)

> +            return false;
> +        }
> +    };
> +
> +    let key = match std::fs::read(key_path) {
> +        Ok(k) => k,
> +        Err(err) => {
> +            log::warn!("could not read key file '{key_path}': {err}");

same here

> +            return false;
> +        }
> +    };
> +
> +    if let Err(e) = verify_signature(&data, &key, None, &WeakCryptoConfig::default()) {
> +        log::error!("PGP signature verification failed: {e:?}");

and here

for example right now, if I configure a repository with some key issue I
get the following in journal with no context:

Nov 07 11:10:34 yuna proxmox-backup-proxy[431704]: PGP signature verification failed: No key in keyring could verify the message!

which sounds super scary..

> +        return false;
> +    }
> +
> +    true
> +}
> +
>  #[test]
>  fn test_uri_to_filename() {
>      let filename = uri_to_filename("https://some_host/some/path");
> -- 
> 2.47.3
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel