[PATCH proxmox-offline-mirror 1/3] clippy: elide redundant lifetimes

[PATCH proxmox-offline-mirror 1/3] clippy: elide redundant lifetimes

[Nicolas Frey]

No functional changes.

for const, 'static lifetime is the default and
`mountpoint_to_path_exists` had needless lifetimes, so fix it

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 src/bin/proxmox-offline-mirror-helper.rs | 2 +-
 src/medium.rs                            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/bin/proxmox-offline-mirror-helper.rs b/src/bin/proxmox-offline-mirror-helper.rs
index 0c40338..19bf39e 100644
--- a/src/bin/proxmox-offline-mirror-helper.rs
+++ b/src/bin/proxmox-offline-mirror-helper.rs
@@ -27,7 +27,7 @@ use proxmox_offline_mirror::helpers::tty::{
 use proxmox_offline_mirror::medium::{self, MIRROR_STATE_FILE, MediumState, generate_repo_snippet};
 
 /// Converts a string slice to a Path, if it exists, otherwise returns an error result.
-fn mountpoint_to_path_exists<'a>(mountpoint: &'a str) -> Result<&'a Path, Error> {
+fn mountpoint_to_path_exists(mountpoint: &str) -> Result<&Path, Error> {
     let mountpoint = Path::new(mountpoint);
     if !mountpoint.exists() {
         bail!("Medium mountpoint doesn't exist.");
diff --git a/src/medium.rs b/src/medium.rs
index 6d9a456..3eb935d 100644
--- a/src/medium.rs
+++ b/src/medium.rs
@@ -18,7 +18,7 @@ use crate::mirror::pool;
 use crate::pool::Pool;
 use crate::types::{Diff, SNAPSHOT_REGEX, Snapshot};
 
-pub const MIRROR_STATE_FILE: &'static str = ".mirror-state";
+pub const MIRROR_STATE_FILE: &str = ".mirror-state";
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
 #[serde(rename_all = "kebab-case")]
-- 
2.47.3

Re: [PATCH proxmox-offline-mirror 1/3] clippy: elide redundant lifetimes

[Erik Fastermann]

> No functional changes.
> 
> for const, 'static lifetime is the default and
> `mountpoint_to_path_exists` had needless lifetimes, so fix it
> 
> Signed-off-by: Nicolas Frey <n.frey@proxmox.com>

LGTM

Reviewed-by: Erik Fastermann <e.fastermann@proxmox.com>

[PATCH proxmox-offline-mirror 0/3] use proxmox-pgp crate to replace verifier helper module

[Nicolas Frey]

As proxmox-pgp crate is now available, use it here and get rid of the
verifier module.

Also fixed two lifetime related clippy lints in the first patch.

Changes since v1 (thanks @Erik!):
* correctly order imports in config.rs
* add migration context to the commit message, stating
  no functional change was intended

proxmox-offline-mirror:

Nicolas Frey (3):
  clippy: elide redundant lifetimes
  use proxmox-pgp crate to replace verifier helper module
  verifier: remove module

 Cargo.toml                               |   1 +
 src/bin/proxmox-offline-mirror-helper.rs |   2 +-
 src/config.rs                            |  33 +----
 src/helpers/mod.rs                       |   3 -
 src/helpers/verifier.rs                  | 163 -----------------------
 src/medium.rs                            |   2 +-
 src/mirror.rs                            |   7 +-
 7 files changed, 7 insertions(+), 204 deletions(-)
 delete mode 100644 src/helpers/verifier.rs


Summary over all repositories:
  7 files changed, 7 insertions(+), 204 deletions(-)

--
Generated by murpp 0.12.0

[PATCH proxmox-offline-mirror 1/3] clippy: elide redundant lifetimes

[Nicolas Frey]

No functional changes.

for const, 'static lifetime is the default and
`mountpoint_to_path_exists` had needless lifetimes, so fix it

Reviewed-by: Erik Fastermann <e.fastermann@proxmox.com>
Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---

Notes:
    No changes since v1

 src/bin/proxmox-offline-mirror-helper.rs | 2 +-
 src/medium.rs                            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/bin/proxmox-offline-mirror-helper.rs b/src/bin/proxmox-offline-mirror-helper.rs
index dff41c5..8a7bd49 100644
--- a/src/bin/proxmox-offline-mirror-helper.rs
+++ b/src/bin/proxmox-offline-mirror-helper.rs
@@ -27,7 +27,7 @@ use proxmox_offline_mirror::helpers::tty::{
 use proxmox_offline_mirror::medium::{self, MIRROR_STATE_FILE, MediumState, generate_repo_snippet};
 
 /// Converts a string slice to a Path, if it exists, otherwise returns an error result.
-fn mountpoint_to_path_exists<'a>(mountpoint: &'a str) -> Result<&'a Path, Error> {
+fn mountpoint_to_path_exists(mountpoint: &str) -> Result<&Path, Error> {
     let mountpoint = Path::new(mountpoint);
     if !mountpoint.exists() {
         bail!("Medium mountpoint doesn't exist.");
diff --git a/src/medium.rs b/src/medium.rs
index 6d9a456..3eb935d 100644
--- a/src/medium.rs
+++ b/src/medium.rs
@@ -18,7 +18,7 @@ use crate::mirror::pool;
 use crate::pool::Pool;
 use crate::types::{Diff, SNAPSHOT_REGEX, Snapshot};
 
-pub const MIRROR_STATE_FILE: &'static str = ".mirror-state";
+pub const MIRROR_STATE_FILE: &str = ".mirror-state";
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
 #[serde(rename_all = "kebab-case")]
-- 
2.47.3

[PATCH proxmox-offline-mirror 2/3] use proxmox-pgp crate to replace verifier helper module

[Nicolas Frey]

No functional change intended. uses the previously ported verifier code
from the module `helpers::verifier`, which now resides in proxmox-pgp.

Reviewed-by: Erik Fastermann <e.fastermann@proxmox.com>
Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---

Notes:
    Changes since v1 (thanks @Erik!)
    * correctly order imports in config.rs
    * add migration context to the commit message, stating
      no functional change was intended

 Cargo.toml    |  1 +
 src/config.rs | 33 +--------------------------------
 src/mirror.rs |  7 +++----
 3 files changed, 5 insertions(+), 36 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index e13a0cd..7b6eb83 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -29,6 +29,7 @@ proxmox-apt-api-types = "3.0"
 proxmox-async = "0.5"
 proxmox-base64 = "1"
 proxmox-http = { version = "1", features = [ "client-sync", "client-trait" ]}
+proxmox-pgp = "1"
 proxmox-router = { version = "3", features = [ "cli" ], default-features = false }
 proxmox-schema = { version = "5", features = [ "api-macro" ] }
 proxmox-section-config = "3"
diff --git a/src/config.rs b/src/config.rs
index 0ca296e..888d48c 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -4,6 +4,7 @@ use std::sync::LazyLock;
 use anyhow::{Error, bail};
 use serde::{Deserialize, Serialize};

+use proxmox_pgp::WeakCryptoConfig;
 use proxmox_schema::{ApiStringFormat, ApiType, Updater, api};
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 use proxmox_subscription::ProductType;
@@ -47,38 +48,6 @@ pub struct SkipConfig {
     pub skip_packages: Option<Vec<String>>,
 }

-#[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>,
-}
-
 #[api(
     properties: {
         id: {
diff --git a/src/mirror.rs b/src/mirror.rs
index 9d89adf..3cf16ac 100644
--- a/src/mirror.rs
+++ b/src/mirror.rs
@@ -9,10 +9,11 @@ use globset::{Glob, GlobSet, GlobSetBuilder};
 use nix::libc;

 use proxmox_http::{HttpClient, client::sync::Client};
+use proxmox_pgp::WeakCryptoConfig;
 use proxmox_schema::{ApiType, Schema};
 use proxmox_sys::fs::file_get_contents;

-use crate::config::{MirrorConfig, SkipConfig, SubscriptionKey, WeakCryptoConfig};
+use crate::config::{MirrorConfig, SkipConfig, SubscriptionKey};
 use crate::helpers::http;
 use crate::pool::Pool;
 use crate::types::{Diff, SNAPSHOT_REGEX, Snapshot};
@@ -24,8 +25,6 @@ use proxmox_apt::deb822::{
 };
 use proxmox_apt_api_types::{APTRepository, APTRepositoryPackageType};

-use crate::helpers;
-
 fn mirror_dir(config: &MirrorConfig) -> PathBuf {
     PathBuf::from(&config.base_dir).join(&config.id)
 }
@@ -207,7 +206,7 @@ fn fetch_release(
     println!("Verifying '{name}' signature using provided repository key..");
     let content = fetched.data_ref();
     let verified =
-        helpers::verify_signature(content, &config.key, sig.as_deref(), &config.weak_crypto)?;
+        proxmox_pgp::verify_signature(content, &config.key, sig.as_deref(), &config.weak_crypto)?;
     println!("Success");

     let sha512 = Some(openssl::sha::sha512(content));
--
2.47.3

[PATCH proxmox-offline-mirror 3/3] verifier: remove module

[Nicolas Frey]

Reviewed-by: Erik Fastermann <e.fastermann@proxmox.com>
Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---

Notes:
    No changes since v1

 src/helpers/mod.rs      |   3 -
 src/helpers/verifier.rs | 163 ----------------------------------------
 2 files changed, 166 deletions(-)
 delete mode 100644 src/helpers/verifier.rs

diff --git a/src/helpers/mod.rs b/src/helpers/mod.rs
index fb084a0..906a087 100644
--- a/src/helpers/mod.rs
+++ b/src/helpers/mod.rs
@@ -1,6 +1,3 @@
 pub mod http;
 
 pub mod tty;
-
-mod verifier;
-pub(crate) use verifier::verify_signature;
diff --git a/src/helpers/verifier.rs b/src/helpers/verifier.rs
deleted file mode 100644
index e1a663e..0000000
--- a/src/helpers/verifier.rs
+++ /dev/null
@@ -1,163 +0,0 @@
-use std::io;
-
-use anyhow::{Error, bail, format_err};
-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 crate::config::WeakCryptoConfig;
-
-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(crate) 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

Re: [PATCH proxmox-offline-mirror 0/3] use proxmox-pgp crate to replace verifier helper module

[Nicolas Frey]

This should of course be a v2, sorry