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

[Nicolas Frey]

On 11/7/25 11:11 AM, Fabian Grünbichler wrote:
> 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 at 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..
> 

Makes a lot of sense, will change in v7, thanks!

>> +        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 at lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>
>>
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>