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

* [pve-devel] [PATCH v4] fix #5207: apt: check signage of repos with gpgv for POM
@ 2025-10-20 14:45 Nicolas Frey
  2025-10-21 12:41 ` Thomas Lamprecht
  0 siblings, 1 reply; 3+ messages in thread
From: Nicolas Frey @ 2025-10-20 14:45 UTC (permalink / raw)
  To: pve-devel

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);
+    }
+}
-- 
2.47.3


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


^ permalink raw reply	[flat|nested] 3+ messages in thread