public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Nicolas Frey <n.frey@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH v4] fix #5207: apt: check signage of repos with gpgv for POM
Date: Mon, 20 Oct 2025 16:45:53 +0200	[thread overview]
Message-ID: <20251020144553.360147-1-n.frey@proxmox.com> (raw)

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


             reply	other threads:[~2025-10-20 14:46 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-20 14:45 Nicolas Frey [this message]
2025-10-21 12:41 ` Thomas Lamprecht
2025-10-21 13:49   ` Nicolas Frey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251020144553.360147-1-n.frey@proxmox.com \
    --to=n.frey@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal