all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH proxmox v5 0/4] fix #5207: apt: check signage of repos with proxmox-pgp
@ 2025-10-23 10:39 Nicolas Frey
  2025-10-23 10:39 ` [pve-devel] [PATCH proxmox v5 1/4] add proxmox-pgp subcrate, move POM verifier code to it Nicolas Frey
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Nicolas Frey @ 2025-10-23 10:39 UTC (permalink / raw)
  To: pve-devel

This patch series moves in pgp verification code from POM into its
own micro-crate `proxmox-pgp` to reuse it to verify a package is of
Proxmox Origin, which fixes #5207.

If this patch series is applied, then `proxmox-offline-mirror` should
use the `proxmox-pgp` crate.

The last patch again adds in the local file fallback in case that the
URI starts with `file://` for (IMO) better UX. I'm fine with this 
being dropped if it's not desired, though.

Changes since v4 (thanks @Thomas for feedback):
* added `proxmox-pgp` micro-crate and moved code from POM
* removed reliance on gpgv in favor of now available `verify_signature`
    function in `proxmox-pgp`
* removed http(s) fallback for cached InRelease file
* split up initial patch into smaller commits

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

Nicolas Frey (4):
  add proxmox-pgp subcrate, move POM verifier code to it
  fix #5207: apt: check signage of repos with  proxmox-pgp
  apt: add tests for POM release filenames
  apt: check for local POM InRelease as fallback

 Cargo.toml                                 |   2 +
 proxmox-apt/Cargo.toml                     |   1 +
 proxmox-apt/src/repositories/repository.rs |  94 ++++++++--
 proxmox-pgp/Cargo.toml                     |  17 ++
 proxmox-pgp/debian/changelog               |   5 +
 proxmox-pgp/debian/control                 |  40 +++++
 proxmox-pgp/debian/copyright               |  18 ++
 proxmox-pgp/debian/debcargo.toml           |   7 +
 proxmox-pgp/src/lib.rs                     |   5 +
 proxmox-pgp/src/verifier.rs                | 200 +++++++++++++++++++++
 10 files changed, 379 insertions(+), 10 deletions(-)
 create mode 100644 proxmox-pgp/Cargo.toml
 create mode 100644 proxmox-pgp/debian/changelog
 create mode 100644 proxmox-pgp/debian/control
 create mode 100644 proxmox-pgp/debian/copyright
 create mode 100644 proxmox-pgp/debian/debcargo.toml
 create mode 100644 proxmox-pgp/src/lib.rs
 create mode 100644 proxmox-pgp/src/verifier.rs

-- 
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] 6+ messages in thread

* [pve-devel] [PATCH proxmox v5 1/4] add proxmox-pgp subcrate, move POM verifier code to it
  2025-10-23 10:39 [pve-devel] [PATCH proxmox v5 0/4] fix #5207: apt: check signage of repos with proxmox-pgp Nicolas Frey
@ 2025-10-23 10:39 ` Nicolas Frey
  2025-10-23 10:39 ` [pve-devel] [PATCH proxmox v5 2/4] fix #5207: apt: check signage of repos with proxmox-pgp Nicolas Frey
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Nicolas Frey @ 2025-10-23 10:39 UTC (permalink / raw)
  To: pve-devel; +Cc: Thomas Lamprecht

Add new micro-crate `proxmox-pgp` for use in `proxmox-apt` to verify
repository signatures. Code is derived from `proxmox-offline-mirror`.
If this patch is applied, then `proxmox-offline-mirror` should use
the `proxmox-pgp` crate.

For the original development history, see commits related to [0].

[0] https://git.proxmox.com/?p=proxmox-offline-mirror.git;a=blob;f=src/helpers/verifier.rs;h=15195a6ec3413ad3c016fffc38f9eee3cefc6827;hb=cd56fab

Suggested-by: Thomas Lamprecht <t.lamprech@proxmox.com>
Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 Cargo.toml                       |   2 +
 proxmox-pgp/Cargo.toml           |  17 +++
 proxmox-pgp/debian/changelog     |   5 +
 proxmox-pgp/debian/control       |  40 +++++++
 proxmox-pgp/debian/copyright     |  18 +++
 proxmox-pgp/debian/debcargo.toml |   7 ++
 proxmox-pgp/src/lib.rs           |   5 +
 proxmox-pgp/src/verifier.rs      | 200 +++++++++++++++++++++++++++++++
 8 files changed, 294 insertions(+)
 create mode 100644 proxmox-pgp/Cargo.toml
 create mode 100644 proxmox-pgp/debian/changelog
 create mode 100644 proxmox-pgp/debian/control
 create mode 100644 proxmox-pgp/debian/copyright
 create mode 100644 proxmox-pgp/debian/debcargo.toml
 create mode 100644 proxmox-pgp/src/lib.rs
 create mode 100644 proxmox-pgp/src/verifier.rs

diff --git a/Cargo.toml b/Cargo.toml
index 8091bf70..7d35c9fd 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -28,6 +28,7 @@ members = [
     "proxmox-network-types",
     "proxmox-notify",
     "proxmox-openid",
+    "proxmox-pgp",
     "proxmox-product-config",
     "proxmox-resource-scheduling",
     "proxmox-rest-server",
@@ -154,6 +155,7 @@ proxmox-lang = { version = "1.5", path = "proxmox-lang" }
 proxmox-log = { version = "1.0.0", path = "proxmox-log" }
 proxmox-login = { version = "1.0.0", path = "proxmox-login" }
 proxmox-network-types = { version = "0.1.0", path = "proxmox-network-types" }
+proxmox-pgp = { version = "1.0.0", path = "proxmox-pgp" }
 proxmox-product-config = { version = "1.0.0", path = "proxmox-product-config" }
 proxmox-config-digest = { version = "1.0.0", path = "proxmox-config-digest" }
 proxmox-rest-server = { version = "1.0.0", path = "proxmox-rest-server" }
diff --git a/proxmox-pgp/Cargo.toml b/proxmox-pgp/Cargo.toml
new file mode 100644
index 00000000..66cac444
--- /dev/null
+++ b/proxmox-pgp/Cargo.toml
@@ -0,0 +1,17 @@
+[package]
+name = "proxmox-pgp"
+version = "1.0.0"
+authors.workspace = true
+edition.workspace = true
+license.workspace = true
+repository.workspace = true
+exclude.workspace = true
+description = "Proxmox wrapper for verifying pgp signatures using sequoia-openpgp"
+
+[dependencies]
+anyhow.workspace = true
+serde.workspace = true
+sequoia-openpgp = "2"
+
+proxmox-api-macro.workspace = true
+proxmox-schema.workspace = true
diff --git a/proxmox-pgp/debian/changelog b/proxmox-pgp/debian/changelog
new file mode 100644
index 00000000..3a5a947c
--- /dev/null
+++ b/proxmox-pgp/debian/changelog
@@ -0,0 +1,5 @@
+rust-proxmox-pgp (1.0.0-1) stable; urgency=medium
+
+  * initial split out of proxmox offline mirror.
+
+ -- Proxmox Support Team <support@proxmox.com>  Wed, 22 Oct 2025 14:06:52 +0200
diff --git a/proxmox-pgp/debian/control b/proxmox-pgp/debian/control
new file mode 100644
index 00000000..23412bab
--- /dev/null
+++ b/proxmox-pgp/debian/control
@@ -0,0 +1,40 @@
+Source: rust-proxmox-pgp
+Section: rust
+Priority: optional
+Build-Depends: debhelper-compat (= 13),
+ dh-sequence-cargo
+Build-Depends-Arch: cargo:native <!nocheck>,
+ rustc:native <!nocheck>,
+ libstd-rust-dev <!nocheck>,
+ librust-anyhow-1+default-dev <!nocheck>,
+ librust-proxmox-api-macro-1+default-dev (>= 1.4.1-~~) <!nocheck>,
+ librust-proxmox-schema-5+default-dev <!nocheck>,
+ librust-sequoia-openpgp-2+default-dev <!nocheck>,
+ librust-serde-1+default-dev <!nocheck>
+Maintainer: Proxmox Support Team <support@proxmox.com>
+Standards-Version: 4.7.2
+Vcs-Git: git://git.proxmox.com/git/proxmox.git
+Vcs-Browser: https://git.proxmox.com/?p=proxmox.git
+Homepage: https://git.proxmox.com/?p=proxmox.git
+X-Cargo-Crate: proxmox-pgp
+
+Package: librust-proxmox-pgp-dev
+Architecture: any
+Multi-Arch: same
+Depends:
+ ${misc:Depends},
+ librust-anyhow-1+default-dev,
+ librust-proxmox-api-macro-1+default-dev (>= 1.4.1-~~),
+ librust-proxmox-schema-5+default-dev,
+ librust-sequoia-openpgp-2+default-dev,
+ librust-serde-1+default-dev
+Provides:
+ librust-proxmox-pgp+default-dev (= ${binary:Version}),
+ librust-proxmox-pgp-1-dev (= ${binary:Version}),
+ librust-proxmox-pgp-1+default-dev (= ${binary:Version}),
+ librust-proxmox-pgp-1.0-dev (= ${binary:Version}),
+ librust-proxmox-pgp-1.0+default-dev (= ${binary:Version}),
+ librust-proxmox-pgp-1.0.0-dev (= ${binary:Version}),
+ librust-proxmox-pgp-1.0.0+default-dev (= ${binary:Version})
+Description: Proxmox wrapper for verifying pgp signatures using sequoia-openpgp - Rust source code
+ Source code for Debianized Rust crate "proxmox-pgp"
diff --git a/proxmox-pgp/debian/copyright b/proxmox-pgp/debian/copyright
new file mode 100644
index 00000000..d6e3c304
--- /dev/null
+++ b/proxmox-pgp/debian/copyright
@@ -0,0 +1,18 @@
+Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
+
+Files:
+ *
+Copyright: 2025 Proxmox Server Solutions GmbH <support@proxmox.com>
+License: AGPL-3.0-or-later
+ This program is free software: you can redistribute it and/or modify it under
+ the terms of the GNU Affero General Public License as published by the Free
+ Software Foundation, either version 3 of the License, or (at your option) any
+ later version.
+ .
+ This program is distributed in the hope that it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+ details.
+ .
+ You should have received a copy of the GNU Affero General Public License along
+ with this program. If not, see <https://www.gnu.org/licenses/>.
diff --git a/proxmox-pgp/debian/debcargo.toml b/proxmox-pgp/debian/debcargo.toml
new file mode 100644
index 00000000..b7864cdb
--- /dev/null
+++ b/proxmox-pgp/debian/debcargo.toml
@@ -0,0 +1,7 @@
+overlay = "."
+crate_src_path = ".."
+maintainer = "Proxmox Support Team <support@proxmox.com>"
+
+[source]
+vcs_git = "git://git.proxmox.com/git/proxmox.git"
+vcs_browser = "https://git.proxmox.com/?p=proxmox.git"
diff --git a/proxmox-pgp/src/lib.rs b/proxmox-pgp/src/lib.rs
new file mode 100644
index 00000000..08a873fd
--- /dev/null
+++ b/proxmox-pgp/src/lib.rs
@@ -0,0 +1,5 @@
+#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
+
+mod verifier;
+
+pub use verifier::{verify_signature, WeakCryptoConfig, WeakCryptoConfigUpdater};
diff --git a/proxmox-pgp/src/verifier.rs b/proxmox-pgp/src/verifier.rs
new file mode 100644
index 00000000..84a15c4c
--- /dev/null
+++ b/proxmox-pgp/src/verifier.rs
@@ -0,0 +1,200 @@
+use anyhow::{bail, format_err, Error};
+
+use proxmox_api_macro::api;
+use proxmox_schema::Updater;
+use sequoia_openpgp::{
+    cert::CertParser,
+    parse::{
+        stream::{
+            DetachedVerifierBuilder, MessageLayer, MessageStructure, VerificationError,
+            VerificationHelper, VerifierBuilder,
+        },
+        PacketParser, PacketParserResult, Parse,
+    },
+    policy::StandardPolicy,
+    types::HashAlgorithm,
+    Cert, KeyHandle,
+};
+use serde::{Deserialize, Serialize};
+use std::io;
+
+#[api(
+    properties: {
+        "allow-sha1": {
+            type: bool,
+            default: false,
+            optional: true,
+        },
+        "min-dsa-key-size": {
+            type: u64,
+            optional: true,
+        },
+        "min-rsa-key-size": {
+            type: u64,
+            optional: true,
+        },
+    },
+)]
+#[derive(Default, Serialize, Deserialize, Updater, Clone, Debug)]
+#[serde(rename_all = "kebab-case")]
+/// Weak Cryptography Configuration
+pub struct WeakCryptoConfig {
+    /// Whether to allow SHA-1 based signatures
+    #[serde(default)]
+    pub allow_sha1: bool,
+    /// Whether to lower the key size cutoff for DSA-based signatures
+    #[serde(default)]
+    pub min_dsa_key_size: Option<u64>,
+    /// Whether to lower the key size cutoff for RSA-based signatures
+    #[serde(default)]
+    pub min_rsa_key_size: Option<u64>,
+}
+
+struct Helper<'a> {
+    cert: &'a Cert,
+}
+
+impl VerificationHelper for Helper<'_> {
+    fn get_certs(&mut self, _ids: &[KeyHandle]) -> sequoia_openpgp::Result<Vec<Cert>> {
+        // Return public keys for signature verification here.
+        Ok(vec![self.cert.clone()])
+    }
+
+    fn check(&mut self, structure: MessageStructure) -> sequoia_openpgp::Result<()> {
+        // In this function, we implement our signature verification policy.
+
+        let mut good = false;
+
+        // we don't want compression and/or encryption
+        let layers: Vec<_> = structure.iter().collect();
+        if layers.len() > 1 || layers.is_empty() {
+            bail!(
+                "unexpected GPG message structure - expected plain signed data, got {} layers!",
+                layers.len()
+            );
+        }
+        let layer = &layers[0];
+        let mut errors = Vec::new();
+        match layer {
+            MessageLayer::SignatureGroup { results } => {
+                // We possibly have multiple signatures, but not all keys, so `or` all the individual results.
+                for result in results {
+                    match result {
+                        Ok(_) => good = true,
+                        Err(e) => errors.push(e),
+                    }
+                }
+            }
+            _ => return Err(anyhow::anyhow!("Unexpected message structure")),
+        }
+
+        if good {
+            Ok(()) // Good signature.
+        } else {
+            if errors.len() > 1 {
+                eprintln!("\nEncountered {} errors:", errors.len());
+            }
+
+            for (n, err) in errors.iter().enumerate() {
+                if errors.len() > 1 {
+                    eprintln!("\nSignature #{n}: {err}");
+                } else {
+                    eprintln!("\n{err}");
+                }
+                match err {
+                    VerificationError::MalformedSignature { error, .. }
+                    | VerificationError::UnboundKey { error, .. }
+                    | VerificationError::BadKey { error, .. }
+                    | VerificationError::BadSignature { error, .. } => {
+                        let mut cause = error.chain();
+                        if cause.len() > 1 {
+                            cause.next(); // already included in `err` above
+                            eprintln!("Caused by:");
+                            for (n, e) in cause.enumerate() {
+                                eprintln!("\t{n}: {e}");
+                            }
+                        }
+                    }
+                    VerificationError::MissingKey { .. }
+                    | VerificationError::UnknownSignature { .. } => {} // doesn't contain a cause
+                    _ => {} // we already print the error above in full
+                };
+            }
+            eprintln!();
+            Err(anyhow::anyhow!("No valid signature found."))
+        }
+    }
+}
+
+/// Verifies GPG-signed `msg` was signed by `key`, returning the verified data without signature.
+pub fn verify_signature(
+    msg: &[u8],
+    key: &[u8],
+    detached_sig: Option<&[u8]>,
+    weak_crypto: &WeakCryptoConfig,
+) -> Result<Vec<u8>, Error> {
+    let mut policy = StandardPolicy::new();
+    if weak_crypto.allow_sha1 {
+        policy.accept_hash(HashAlgorithm::SHA1);
+    }
+    if let Some(min_dsa) = weak_crypto.min_dsa_key_size {
+        if min_dsa <= 1024 {
+            policy.accept_asymmetric_algo(sequoia_openpgp::policy::AsymmetricAlgorithm::DSA1024);
+        }
+    }
+    if let Some(min_rsa) = weak_crypto.min_rsa_key_size {
+        if min_rsa <= 1024 {
+            policy.accept_asymmetric_algo(sequoia_openpgp::policy::AsymmetricAlgorithm::RSA1024);
+        }
+    }
+
+    let verifier = |cert| {
+        let helper = Helper { cert: &cert };
+
+        if let Some(sig) = detached_sig {
+            let mut verifier =
+                DetachedVerifierBuilder::from_bytes(sig)?.with_policy(&policy, None, helper)?;
+            verifier.verify_bytes(msg)?;
+            Ok(msg.to_vec())
+        } else {
+            let mut verified = Vec::new();
+            let mut verifier =
+                VerifierBuilder::from_bytes(msg)?.with_policy(&policy, None, helper)?;
+            let bytes = io::copy(&mut verifier, &mut verified)?;
+            println!("{bytes} bytes verified");
+            if !verifier.message_processed() {
+                bail!("Failed to verify message!");
+            }
+            Ok(verified)
+        }
+    };
+
+    let mut packed_parser = PacketParser::from_bytes(key)?;
+
+    // parse all packets to see whether this is a simple certificate or a keyring
+    while let PacketParserResult::Some(pp) = packed_parser {
+        packed_parser = pp.recurse()?.1;
+    }
+
+    if let PacketParserResult::EOF(eof) = packed_parser {
+        // verify against a single certificate
+        if eof.is_cert().is_ok() {
+            let cert = Cert::from_bytes(key)?;
+            return verifier(cert);
+        // verify against a keyring
+        } else if eof.is_keyring().is_ok() {
+            let packed_parser = PacketParser::from_bytes(key)?;
+
+            return CertParser::from(packed_parser)
+                // flatten here as we ignore packets that aren't a certificate
+                .flatten()
+                // keep trying to verify the message until the first certificate that succeeds
+                .find_map(|c| verifier(c).ok())
+                // if no certificate verified the message, abort
+                .ok_or_else(|| format_err!("No key in keyring could verify the message!"));
+        }
+    }
+
+    // neither a keyring nor a certificate was detect, so we abort here
+    bail!("'key-path' contains neither a keyring nor a certificate, aborting!");
+}
-- 
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] 6+ messages in thread

* [pve-devel] [PATCH proxmox v5 2/4] fix #5207: apt: check signage of repos with proxmox-pgp
  2025-10-23 10:39 [pve-devel] [PATCH proxmox v5 0/4] fix #5207: apt: check signage of repos with proxmox-pgp Nicolas Frey
  2025-10-23 10:39 ` [pve-devel] [PATCH proxmox v5 1/4] add proxmox-pgp subcrate, move POM verifier code to it Nicolas Frey
@ 2025-10-23 10:39 ` Nicolas Frey
  2025-10-23 14:24   ` Nicolas Frey
  2025-10-23 10:39 ` [pve-devel] [PATCH proxmox v5 3/4] apt: add tests for POM release filenames Nicolas Frey
  2025-10-23 10:39 ` [pve-devel] [PATCH proxmox v5 4/4] apt: check for local POM InRelease as fallback Nicolas Frey
  3 siblings, 1 reply; 6+ messages in thread
From: Nicolas Frey @ 2025-10-23 10:39 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 commit aims to fix this issue by verifying it through signage
info via `proxmox-pgp`. The InRelease file cached at
`/var/lib/apt/lists/` is used to check whether the package is of
Proxmox Origin.

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


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

* [pve-devel] [PATCH proxmox v5 3/4] apt: add tests for POM release filenames
  2025-10-23 10:39 [pve-devel] [PATCH proxmox v5 0/4] fix #5207: apt: check signage of repos with proxmox-pgp Nicolas Frey
  2025-10-23 10:39 ` [pve-devel] [PATCH proxmox v5 1/4] add proxmox-pgp subcrate, move POM verifier code to it Nicolas Frey
  2025-10-23 10:39 ` [pve-devel] [PATCH proxmox v5 2/4] fix #5207: apt: check signage of repos with proxmox-pgp Nicolas Frey
@ 2025-10-23 10:39 ` Nicolas Frey
  2025-10-23 10:39 ` [pve-devel] [PATCH proxmox v5 4/4] apt: check for local POM InRelease as fallback Nicolas Frey
  3 siblings, 0 replies; 6+ messages in thread
From: Nicolas Frey @ 2025-10-23 10:39 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 proxmox-apt/src/repositories/repository.rs | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/proxmox-apt/src/repositories/repository.rs b/proxmox-apt/src/repositories/repository.rs
index 5e386665..1b3f010e 100644
--- a/proxmox-apt/src/repositories/repository.rs
+++ b/proxmox-apt/src/repositories/repository.rs
@@ -430,3 +430,32 @@ 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] 6+ messages in thread

* [pve-devel] [PATCH proxmox v5 4/4] apt: check for local POM InRelease as fallback
  2025-10-23 10:39 [pve-devel] [PATCH proxmox v5 0/4] fix #5207: apt: check signage of repos with proxmox-pgp Nicolas Frey
                   ` (2 preceding siblings ...)
  2025-10-23 10:39 ` [pve-devel] [PATCH proxmox v5 3/4] apt: add tests for POM release filenames Nicolas Frey
@ 2025-10-23 10:39 ` Nicolas Frey
  3 siblings, 0 replies; 6+ messages in thread
From: Nicolas Frey @ 2025-10-23 10:39 UTC (permalink / raw)
  To: pve-devel

Add local file fallback in case that the URI starts with `file://`,
which is useful if a POM repo is stored on a medium. This improves
UX by not necessarily needing to apt update to see immediate feedback
in the Repositories UI of pve/pbs.

If the medium is not present, the signature verification will simply
fail.

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 proxmox-apt/src/repositories/repository.rs | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/proxmox-apt/src/repositories/repository.rs b/proxmox-apt/src/repositories/repository.rs
index 1b3f010e..b8b2ecc4 100644
--- a/proxmox-apt/src/repositories/repository.rs
+++ b/proxmox-apt/src/repositories/repository.rs
@@ -396,12 +396,7 @@ fn write_stanza(repo: &APTRepository, w: &mut dyn Write) -> Result<(), Error> {
 /// 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,
-    )) {
+    let data = match read_inrelease(uri, suite) {
         Ok(d) => d,
         Err(err) => {
             log::warn!("could not read InRelease file: {err}");
@@ -425,6 +420,20 @@ fn is_signed_by_key(uri: &str, suite: &str, key_path: &str) -> bool {
     true
 }
 
+/// Reads cached/local POM InRelease file
+fn read_inrelease(uri: &str, suite: &str) -> Result<Vec<u8>, Error> {
+    let path = release_filename(Path::new("/var/lib/apt/lists"), uri, suite, false);
+
+    if path.exists() {
+        std::fs::read(&path).map_err(Into::into)
+    } else if let Some(local_path) = uri.strip_prefix("file://") {
+        let inrelease_path = format!("{}/dists/{}/InRelease", local_path, suite);
+        std::fs::read(&inrelease_path).map_err(Into::into)
+    } else {
+        bail!("Unsupported URI scheme: {}", uri);
+    }
+}
+
 #[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


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

* Re: [pve-devel] [PATCH proxmox v5 2/4] fix #5207: apt: check signage of repos with proxmox-pgp
  2025-10-23 10:39 ` [pve-devel] [PATCH proxmox v5 2/4] fix #5207: apt: check signage of repos with proxmox-pgp Nicolas Frey
@ 2025-10-23 14:24   ` Nicolas Frey
  0 siblings, 0 replies; 6+ messages in thread
From: Nicolas Frey @ 2025-10-23 14:24 UTC (permalink / raw)
  To: pve-devel

On 10/23/25 12:39 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 signage
> info via `proxmox-pgp`. The InRelease file cached at
> `/var/lib/apt/lists/` is used to check whether the package is of
> Proxmox Origin.
> 
forgot to add
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}");
> +            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}");
> +            return false;
> +        }
> +    };
> +
> +    if let Err(e) = verify_signature(&data, &key, None, &WeakCryptoConfig::default()) {
> +        log::error!("PGP signature verification failed: {e:?}");
> +        return false;
> +    }
> +
> +    true
> +}
> +
>  #[test]
>  fn test_uri_to_filename() {
>      let filename = uri_to_filename("https://some_host/some/path");



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


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

end of thread, other threads:[~2025-10-23 14:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-23 10:39 [pve-devel] [PATCH proxmox v5 0/4] fix #5207: apt: check signage of repos with proxmox-pgp Nicolas Frey
2025-10-23 10:39 ` [pve-devel] [PATCH proxmox v5 1/4] add proxmox-pgp subcrate, move POM verifier code to it Nicolas Frey
2025-10-23 10:39 ` [pve-devel] [PATCH proxmox v5 2/4] fix #5207: apt: check signage of repos with proxmox-pgp Nicolas Frey
2025-10-23 14:24   ` Nicolas Frey
2025-10-23 10:39 ` [pve-devel] [PATCH proxmox v5 3/4] apt: add tests for POM release filenames Nicolas Frey
2025-10-23 10:39 ` [pve-devel] [PATCH proxmox v5 4/4] apt: check for local POM InRelease as fallback Nicolas Frey

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal