Re: [pve-devel] [PATCH v6 1/4] add proxmox-pgp subcrate, move POM verifier code to it

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

On October 30, 2025 2:28 pm, Nicolas Frey wrote:
> Add new micro-crate `proxmox-pgp` for use in `proxmox-apt` to verify
> repository signatures. Code is derived from `proxmox-offline-mirror`,
> removing direct `stderr` output in favor of added context to the
> returned `anyhow::Error`.
> 
> 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      | 194 +++++++++++++++++++++++++++++++
>  8 files changed, 288 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"

a bit torn here, but I guess the current interface is small enough that
we can extend without breakage ;)

> +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

missing some features here

> +sequoia-openpgp = "2"
> +
> +proxmox-api-macro.workspace = true
> +proxmox-schema.workspace = true

and here

cargo is a bit annoying here - if you build the whole workspace in one
go, features enabled by other crates in the workspace that share
dependencies will hide issues here. so you need to do a standalone
`cargo check/test/build -p foo`, possibly with and without features of
foo enabled. doing a final `make foo-deb` before patch submission also
doesn't hurt ;)

if you get stuck with such details, feel free to ping me!

> 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..c2eadbc5
> --- /dev/null
> +++ b/proxmox-pgp/src/verifier.rs
> @@ -0,0 +1,194 @@
> +use std::io;
> +
> +use anyhow::{bail, format_err, Error};
> +use sequoia_openpgp::cert::CertParser;
> +use sequoia_openpgp::parse::stream::{
> +    DetachedVerifierBuilder, MessageLayer, MessageStructure, VerificationError, VerificationHelper,
> +    VerifierBuilder,
> +};
> +use sequoia_openpgp::parse::{PacketParser, PacketParserResult, Parse};
> +use sequoia_openpgp::policy::StandardPolicy;
> +use sequoia_openpgp::types::HashAlgorithm;
> +use sequoia_openpgp::{Cert, KeyHandle};
> +use serde::{Deserialize, Serialize};
> +
> +use proxmox_api_macro::api;
> +use proxmox_schema::Updater;
> +
> +#[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 CertWrapper<'a> {
> +    cert: &'a Cert,
> +}
> +
> +impl VerificationHelper for CertWrapper<'_> {
> +    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.
> +
> +        // 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();
> +
> +        let MessageLayer::SignatureGroup { results } = layer else {
> +            bail!("Unexpected message structure")
> +        };
> +
> +        for result in results {
> +            if let Err(e) = result {
> +                errors.push(e);
> +            } else {
> +                // good signature, early return
> +                return Ok(());
> +            }
> +        }
> +
> +        let mut context = String::new();
> +        let errlen = errors.len();
> +
> +        if errlen > 1 {
> +            context.push_str(&format!("\nEncountered {errlen} errors:"));
> +        }
> +
> +        for (n, err) in errors.iter().enumerate() {
> +            if errlen > 1 {
> +                context.push_str(&format!("\nSignature #{n}: {err}"));
> +            } else {
> +                context.push_str(&format!("\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
> +                        context.push_str("Caused by:");
> +                        for (n, e) in cause.enumerate() {
> +                            context.push_str(&format!("\t{n}: {e}"));
> +                        }
> +                    }
> +                }
> +                VerificationError::MissingKey { .. }
> +                | VerificationError::UnknownSignature { .. } => {} // doesn't contain a cause
> +                _ => {} // we already print the error above in full
> +            };
> +        }
> +
> +        Err(anyhow::anyhow!("No valid signature found.").context(context))
> +    }
> +}
> +
> +/// 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 = CertWrapper { 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)?;
> +            if !verifier.message_processed() {
> +                bail!("Failed to verify message!");
> +            }
> +            Ok(verified)
> +        }
> +    };
> +
> +    let mut packet_parser = PacketParser::from_bytes(key)?;
> +
> +    // parse all packets to see whether this is a simple certificate or a keyring
> +    while let PacketParserResult::Some(pp) = packet_parser {
> +        packet_parser = pp.recurse()?.1;
> +    }
> +
> +    if let PacketParserResult::EOF(eof) = packet_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 packet_parser = PacketParser::from_bytes(key)?;
> +
> +            return CertParser::from(packet_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
> 
> 
> 


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