[pve-devel] [PATCH v3 many 0/5] notifications: feed system mails into proxmox_notify

[pve-devel] [PATCH v3 many 0/5] notifications: feed system mails into proxmox_notify

[Lukas Wagner]

The aim of this patch series is to adapt `proxmox-mail-forward` 
so that it forwards emails that were sent to the local root user
through the `proxmox_notify` crate.

A short summary of the status quo:
Any mail that is sent to the local `root` user is forwarded by
postfix to the `proxmox-mail-forward` binary, which receives the
mail via STDIN. `proxmox-mail-forward` looks up the email address 
configured for the `root@pam` user in /etc/{proxmox-backup,pve}/user.cfg 
and then forwards the mail to this address by calling `sendmail`

This patch series modifies `proxmox-mail-forward` in the following way:
`proxmox-mail-forward` instantiates the configuration for `proxmox_notify`
by reading `/etc/{proxmox-backup,pve}/notifications.cfg.

The forwarding behavior is the following:
  - PVE installed: Use PVE's notifications.cfg
  - PBS installed: Use PBS's notifications.cfg if present. If not,
    use an empty configuration and add a default sendmail target and
    a matcher - this is needed because notifications are not yet
    integrated in PBS. In that way, the forwarding behavior is still
    the same as before on PBS (forward to root@pam via sendmail).
  - PVE/PBS co-installed: Use PVE's config *and* PBS's config. 
    If PBS's notifications.cfg does not exist, 
    a default sendmail target will *not* be added, to avoid
    forwarding the same mail twice. 
    For co-installations we assume for now that PVE has a sensible
    matcher/target config for forwarded mails.

Required patches:
  - series: 'overhaul notification system, use matchers instead of filters' [2]
  - pve-docs: 'notifications: update docs to for matcher-based notifications' [3]
  - Also, these two patches for 'proxmox' from the SMTP target series [4] are needed:
    - 'sys: email: add `forward`'
    - 'notify: add mechanisms for email message forwarding'

Changelog:
  - v1 -> v2:
    - Rebased
    - Apply the same fix for the PVE context as in [1]
  - v2 -> v3:
    - Rebased on top of matcher-based notification system:
      This simplifies proxmox-mail-forward by a great deal, since 
      notification routing is moved into the matcher. This means 
      proxmox-mail-forward does not need to read /etc/pve/datacenter.cfg
      any more to determine the target for the notification.

[1] https://lists.proxmox.com/pipermail/pve-devel/2023-October/059294.html
[2] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059818.html
[3] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059872.html
[4] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059894.html
[5] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059899.html
[6] https://lists.proxmox.com/pipermail/pve-devel/2023-November/059900.html



proxmox:

Lukas Wagner (1):
  notify: add PVE/PBS context

 proxmox-notify/Cargo.toml            |   3 +-
 proxmox-notify/src/context.rs        |  21 -----
 proxmox-notify/src/context/common.rs |  27 ++++++
 proxmox-notify/src/context/mod.rs    |  36 ++++++++
 proxmox-notify/src/context/pbs.rs    | 130 +++++++++++++++++++++++++++
 proxmox-notify/src/context/pve.rs    |  82 +++++++++++++++++
 6 files changed, 277 insertions(+), 22 deletions(-)
 delete mode 100644 proxmox-notify/src/context.rs
 create mode 100644 proxmox-notify/src/context/common.rs
 create mode 100644 proxmox-notify/src/context/mod.rs
 create mode 100644 proxmox-notify/src/context/pbs.rs
 create mode 100644 proxmox-notify/src/context/pve.rs


proxmox-perl-rs:

Lukas Wagner (1):
  pve-rs: notify: remove notify_context for PVE

 pve-rs/Cargo.toml            |   2 +-
 pve-rs/src/lib.rs            |   7 ++-
 pve-rs/src/notify_context.rs | 117 -----------------------------------
 3 files changed, 5 insertions(+), 121 deletions(-)
 delete mode 100644 pve-rs/src/notify_context.rs


proxmox-mail-forward:

Lukas Wagner (2):
  feed forwarded mails into proxmox_notify
  update d/control

 Cargo.toml     |   6 +-
 debian/control |   6 +-
 src/main.rs    | 255 +++++++++++++++++++++++--------------------------
 3 files changed, 125 insertions(+), 142 deletions(-)


pve-docs:

Lukas Wagner (1):
  notifications: add documentation for system mail forwarding

 notifications.adoc | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)


Summary over all repositories:
  13 files changed, 423 insertions(+), 285 deletions(-)

-- 
murpp v0.4.0

[pve-devel] [PATCH v3 proxmox 1/5] notify: add PVE/PBS context

[Lukas Wagner]

This commit moves PVEContext from `proxmox-perl-rs` into the
`proxmox-notify` crate, since we now also need to access it from
`promxox-mail-forward`. The context is now hidden behind a feature
flag `pve-context`, ensuring that we only compile it when needed.

This commit adds PBSContext, since we now require it for
`proxmox-mail-forward`. Some of the code for PBSContext comes
from `proxmox-mail-forward`.

This commit also changes the global context from being stored in a
`once_cell` to a regular `Mutex`, since we now need to set/reset
the context in `proxmox-mail-forward`.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes v2 -> v3:
      - no changes

 proxmox-notify/Cargo.toml            |   3 +-
 proxmox-notify/src/context.rs        |  21 -----
 proxmox-notify/src/context/common.rs |  27 ++++++
 proxmox-notify/src/context/mod.rs    |  36 ++++++++
 proxmox-notify/src/context/pbs.rs    | 130 +++++++++++++++++++++++++++
 proxmox-notify/src/context/pve.rs    |  82 +++++++++++++++++
 6 files changed, 277 insertions(+), 22 deletions(-)
 delete mode 100644 proxmox-notify/src/context.rs
 create mode 100644 proxmox-notify/src/context/common.rs
 create mode 100644 proxmox-notify/src/context/mod.rs
 create mode 100644 proxmox-notify/src/context/pbs.rs
 create mode 100644 proxmox-notify/src/context/pve.rs

diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index f2b4db5..7a3d434 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -13,7 +13,6 @@ handlebars = { workspace = true }
 lazy_static.workspace = true
 log.workspace = true
 mail-parser = { workspace = true, optional = true }
-once_cell.workspace = true
 openssl.workspace = true
 proxmox-http = { workspace = true, features = ["client-sync"], optional = true }
 proxmox-http-error.workspace = true
@@ -32,3 +31,5 @@ default = ["sendmail", "gotify"]
 mail-forwarder = ["dep:mail-parser"]
 sendmail = ["dep:proxmox-sys"]
 gotify = ["dep:proxmox-http"]
+pve-context = ["dep:proxmox-sys"]
+pbs-context = ["dep:proxmox-sys"]
diff --git a/proxmox-notify/src/context.rs b/proxmox-notify/src/context.rs
deleted file mode 100644
index 370c7ee..0000000
--- a/proxmox-notify/src/context.rs
+++ /dev/null
@@ -1,21 +0,0 @@
-use std::fmt::Debug;
-
-use once_cell::sync::OnceCell;
-
-pub trait Context: Send + Sync + Debug {
-    fn lookup_email_for_user(&self, user: &str) -> Option<String>;
-    fn default_sendmail_author(&self) -> String;
-    fn default_sendmail_from(&self) -> String;
-    fn http_proxy_config(&self) -> Option<String>;
-}
-
-static CONTEXT: OnceCell<&'static dyn Context> = OnceCell::new();
-
-pub fn set_context(context: &'static dyn Context) {
-    CONTEXT.set(context).expect("context has already been set");
-}
-
-#[allow(unused)] // context is not used if all endpoint features are disabled
-pub(crate) fn context() -> &'static dyn Context {
-    *CONTEXT.get().expect("context has not been yet")
-}
diff --git a/proxmox-notify/src/context/common.rs b/proxmox-notify/src/context/common.rs
new file mode 100644
index 0000000..7580bd1
--- /dev/null
+++ b/proxmox-notify/src/context/common.rs
@@ -0,0 +1,27 @@
+use std::path::Path;
+
+pub(crate) fn attempt_file_read<P: AsRef<Path>>(path: P) -> Option<String> {
+    match proxmox_sys::fs::file_read_optional_string(path) {
+        Ok(contents) => contents,
+        Err(err) => {
+            log::error!("{err}");
+            None
+        }
+    }
+}
+
+pub(crate) fn lookup_datacenter_config_key(content: &str, key: &str) -> Option<String> {
+    let key_prefix = format!("{key}:");
+    normalize_for_return(
+        content
+            .lines()
+            .find_map(|line| line.strip_prefix(&key_prefix)),
+    )
+}
+
+pub(crate) fn normalize_for_return(s: Option<&str>) -> Option<String> {
+    match s?.trim() {
+        "" => None,
+        s => Some(s.to_string()),
+    }
+}
diff --git a/proxmox-notify/src/context/mod.rs b/proxmox-notify/src/context/mod.rs
new file mode 100644
index 0000000..99d86de
--- /dev/null
+++ b/proxmox-notify/src/context/mod.rs
@@ -0,0 +1,36 @@
+use std::fmt::Debug;
+use std::sync::Mutex;
+
+#[cfg(any(feature = "pve-context", feature = "pbs-context"))]
+pub mod common;
+#[cfg(feature = "pbs-context")]
+pub mod pbs;
+#[cfg(feature = "pve-context")]
+pub mod pve;
+
+/// Product-specific context
+pub trait Context: Send + Sync + Debug {
+    /// Look up a user's email address from users.cfg
+    fn lookup_email_for_user(&self, user: &str) -> Option<String>;
+    /// Default mail author for mail-based targets
+    fn default_sendmail_author(&self) -> String;
+    /// Default from address for sendmail-based targets
+    fn default_sendmail_from(&self) -> String;
+    /// Proxy configuration for the current node
+    fn http_proxy_config(&self) -> Option<String>;
+}
+
+static CONTEXT: Mutex<Option<&'static dyn Context>> = Mutex::new(None);
+
+/// Set the product-specific context
+pub fn set_context(context: &'static dyn Context) {
+    *CONTEXT.lock().unwrap() = Some(context);
+}
+
+/// Get product-specific context.
+///
+/// Panics if the context has not been set yet.
+#[allow(unused)] // context is not used if all endpoint features are disabled
+pub(crate) fn context() -> &'static dyn Context {
+    (*CONTEXT.lock().unwrap()).expect("context for proxmox-notify has not been set yet")
+}
diff --git a/proxmox-notify/src/context/pbs.rs b/proxmox-notify/src/context/pbs.rs
new file mode 100644
index 0000000..b5d3168
--- /dev/null
+++ b/proxmox-notify/src/context/pbs.rs
@@ -0,0 +1,130 @@
+use serde::Deserialize;
+
+use proxmox_schema::{ObjectSchema, Schema, StringSchema};
+use proxmox_section_config::{SectionConfig, SectionConfigPlugin};
+
+use crate::context::{common, Context};
+
+const PBS_USER_CFG_FILENAME: &str = "/etc/proxmox-backup/user.cfg";
+const PBS_NODE_CFG_FILENAME: &str = "/etc/proxmox-backup/node.cfg";
+
+// FIXME: Switch to the actual schema when possible in terms of dependency.
+// It's safe to assume that the config was written with the actual schema restrictions, so parsing
+// it with the less restrictive schema should be enough for the purpose of getting the mail address.
+const DUMMY_ID_SCHEMA: Schema = StringSchema::new("dummy ID").min_length(3).schema();
+const DUMMY_EMAIL_SCHEMA: Schema = StringSchema::new("dummy email").schema();
+const DUMMY_USER_SCHEMA: ObjectSchema = ObjectSchema {
+    description: "minimal PBS user",
+    properties: &[
+        ("userid", false, &DUMMY_ID_SCHEMA),
+        ("email", true, &DUMMY_EMAIL_SCHEMA),
+    ],
+    additional_properties: true,
+    default_key: None,
+};
+
+#[derive(Deserialize)]
+struct DummyPbsUser {
+    pub email: Option<String>,
+}
+
+/// Extract the root user's email address from the PBS user config.
+fn lookup_mail_address(content: &str, username: &str) -> Option<String> {
+    let mut config = SectionConfig::new(&DUMMY_ID_SCHEMA).allow_unknown_sections(true);
+    let user_plugin = SectionConfigPlugin::new(
+        "user".to_string(),
+        Some("userid".to_string()),
+        &DUMMY_USER_SCHEMA,
+    );
+    config.register_plugin(user_plugin);
+
+    match config.parse(PBS_USER_CFG_FILENAME, content) {
+        Ok(parsed) => {
+            parsed.sections.get(username)?;
+            match parsed.lookup::<DummyPbsUser>("user", username) {
+                Ok(user) => common::normalize_for_return(user.email.as_deref()),
+                Err(err) => {
+                    log::error!("unable to parse {PBS_USER_CFG_FILENAME}: {err}");
+                    None
+                }
+            }
+        }
+        Err(err) => {
+            log::error!("unable to parse {PBS_USER_CFG_FILENAME}: {err}");
+            None
+        }
+    }
+}
+
+#[derive(Debug)]
+pub struct PBSContext;
+
+pub static PBS_CONTEXT: PBSContext = PBSContext;
+
+impl Context for PBSContext {
+    fn lookup_email_for_user(&self, user: &str) -> Option<String> {
+        let content = common::attempt_file_read(PBS_USER_CFG_FILENAME);
+        content.and_then(|content| lookup_mail_address(&content, user))
+    }
+
+    fn default_sendmail_author(&self) -> String {
+        "Proxmox Backup Server".into()
+    }
+
+    fn default_sendmail_from(&self) -> String {
+        let content = common::attempt_file_read(PBS_NODE_CFG_FILENAME);
+        content
+            .and_then(|content| common::lookup_datacenter_config_key(&content, "email-from"))
+            .unwrap_or_else(|| String::from("root"))
+    }
+
+    fn http_proxy_config(&self) -> Option<String> {
+        let content = common::attempt_file_read(PBS_NODE_CFG_FILENAME);
+        content.and_then(|content| common::lookup_datacenter_config_key(&content, "http-proxy"))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    const USER_CONFIG: &str = "
+user: root@pam
+	email root@example.com
+
+user: test@pbs
+	enable true
+	expire 0
+    ";
+
+    #[test]
+    fn test_parse_mail() {
+        assert_eq!(
+            lookup_mail_address(USER_CONFIG, "root@pam"),
+            Some("root@example.com".to_string())
+        );
+        assert_eq!(lookup_mail_address(USER_CONFIG, "test@pbs"), None);
+    }
+
+    const NODE_CONFIG: &str = "
+default-lang: de
+email-from: root@example.com
+http-proxy: http://localhost:1234
+    ";
+
+    #[test]
+    fn test_parse_node_config() {
+        assert_eq!(
+            common::lookup_datacenter_config_key(NODE_CONFIG, "email-from"),
+            Some("root@example.com".to_string())
+        );
+        assert_eq!(
+            common::lookup_datacenter_config_key(NODE_CONFIG, "http-proxy"),
+            Some("http://localhost:1234".to_string())
+        );
+        assert_eq!(
+            common::lookup_datacenter_config_key(NODE_CONFIG, "foo"),
+            None
+        );
+    }
+}
diff --git a/proxmox-notify/src/context/pve.rs b/proxmox-notify/src/context/pve.rs
new file mode 100644
index 0000000..f263c95
--- /dev/null
+++ b/proxmox-notify/src/context/pve.rs
@@ -0,0 +1,82 @@
+use crate::context::{common, Context};
+
+fn lookup_mail_address(content: &str, user: &str) -> Option<String> {
+    common::normalize_for_return(content.lines().find_map(|line| {
+        let fields: Vec<&str> = line.split(':').collect();
+        #[allow(clippy::get_first)] // to keep expression style consistent
+        match fields.get(0)?.trim() == "user" && fields.get(1)?.trim() == user {
+            true => fields.get(6).copied(),
+            false => None,
+        }
+    }))
+}
+
+#[derive(Debug)]
+pub struct PVEContext;
+
+impl Context for PVEContext {
+    fn lookup_email_for_user(&self, user: &str) -> Option<String> {
+        let content = common::attempt_file_read("/etc/pve/user.cfg");
+        content.and_then(|content| lookup_mail_address(&content, user))
+    }
+
+    fn default_sendmail_author(&self) -> String {
+        "Proxmox VE".into()
+    }
+
+    fn default_sendmail_from(&self) -> String {
+        let content = common::attempt_file_read("/etc/pve/datacenter.cfg");
+        content
+            .and_then(|content| common::lookup_datacenter_config_key(&content, "email_from"))
+            .unwrap_or_else(|| String::from("root"))
+    }
+
+    fn http_proxy_config(&self) -> Option<String> {
+        let content = common::attempt_file_read("/etc/pve/datacenter.cfg");
+        content.and_then(|content| common::lookup_datacenter_config_key(&content, "http_proxy"))
+    }
+}
+
+pub static PVE_CONTEXT: PVEContext = PVEContext;
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    const USER_CONFIG: &str = "
+user:root@pam:1:0:::root@example.com:::
+user:test@pve:1:0:::test@example.com:::
+user:no-mail@pve:1:0::::::
+    ";
+
+    #[test]
+    fn test_parse_mail() {
+        assert_eq!(
+            lookup_mail_address(USER_CONFIG, "root@pam"),
+            Some("root@example.com".to_string())
+        );
+        assert_eq!(
+            lookup_mail_address(USER_CONFIG, "test@pve"),
+            Some("test@example.com".to_string())
+        );
+        assert_eq!(lookup_mail_address(USER_CONFIG, "no-mail@pve"), None);
+    }
+
+    const DC_CONFIG: &str = "
+email_from: user@example.com
+http_proxy: http://localhost:1234
+keyboard: en-us
+";
+    #[test]
+    fn test_parse_dc_config() {
+        assert_eq!(
+            common::lookup_datacenter_config_key(DC_CONFIG, "email_from"),
+            Some("user@example.com".to_string())
+        );
+        assert_eq!(
+            common::lookup_datacenter_config_key(DC_CONFIG, "http_proxy"),
+            Some("http://localhost:1234".to_string())
+        );
+        assert_eq!(common::lookup_datacenter_config_key(DC_CONFIG, "foo"), None);
+    }
+}
-- 
2.39.2

[pve-devel] [PATCH v3 proxmox-perl-rs 2/5] pve-rs: notify: remove notify_context for PVE

[Lukas Wagner]

The context has now been moved to `proxmox-notify` due to the fact
that we also need it in `proxmox-mail-forward` now.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes v2 -> v3:
      - No changes

 pve-rs/Cargo.toml            |   2 +-
 pve-rs/src/lib.rs            |   7 ++-
 pve-rs/src/notify_context.rs | 117 -----------------------------------
 3 files changed, 5 insertions(+), 121 deletions(-)
 delete mode 100644 pve-rs/src/notify_context.rs

diff --git a/pve-rs/Cargo.toml b/pve-rs/Cargo.toml
index e222d9d..2300c8d 100644
--- a/pve-rs/Cargo.toml
+++ b/pve-rs/Cargo.toml
@@ -36,7 +36,7 @@ perlmod = { version = "0.13", features = [ "exporter" ] }
 proxmox-apt = "0.10.6"
 proxmox-http = { version = "0.9", features = ["client-sync", "client-trait"] }
 proxmox-http-error = "0.1.0"
-proxmox-notify = "0.2"
+proxmox-notify = { version = "0.2", features = ["pve-context"] }
 proxmox-openid = "0.10"
 proxmox-resource-scheduling = "0.3.0"
 proxmox-subscription = "0.4"
diff --git a/pve-rs/src/lib.rs b/pve-rs/src/lib.rs
index d1915c9..42be39e 100644
--- a/pve-rs/src/lib.rs
+++ b/pve-rs/src/lib.rs
@@ -4,18 +4,19 @@
 pub mod common;
 
 pub mod apt;
-pub mod notify_context;
 pub mod openid;
 pub mod resource_scheduling;
 pub mod tfa;
 
 #[perlmod::package(name = "Proxmox::Lib::PVE", lib = "pve_rs")]
 mod export {
-    use crate::{common, notify_context};
+    use proxmox_notify::context::pve::PVE_CONTEXT;
+
+    use crate::common;
 
     #[export]
     pub fn init() {
         common::logger::init("PVE_LOG", "info");
-        notify_context::init();
+        proxmox_notify::context::set_context(&PVE_CONTEXT)
     }
 }
diff --git a/pve-rs/src/notify_context.rs b/pve-rs/src/notify_context.rs
deleted file mode 100644
index 3cf3e18..0000000
--- a/pve-rs/src/notify_context.rs
+++ /dev/null
@@ -1,117 +0,0 @@
-use log;
-use std::path::Path;
-
-use proxmox_notify::context::Context;
-
-// Some helpers borrowed and slightly adapted from `proxmox-mail-forward`
-
-fn normalize_for_return(s: Option<&str>) -> Option<String> {
-    match s?.trim() {
-        "" => None,
-        s => Some(s.to_string()),
-    }
-}
-
-fn attempt_file_read<P: AsRef<Path>>(path: P) -> Option<String> {
-    match proxmox_sys::fs::file_read_optional_string(path) {
-        Ok(contents) => contents,
-        Err(err) => {
-            log::error!("{err}");
-            None
-        }
-    }
-}
-
-fn lookup_mail_address(content: &str, user: &str) -> Option<String> {
-    normalize_for_return(content.lines().find_map(|line| {
-        let fields: Vec<&str> = line.split(':').collect();
-        #[allow(clippy::get_first)] // to keep expression style consistent
-        match fields.get(0)?.trim() == "user" && fields.get(1)?.trim() == user {
-            true => fields.get(6).copied(),
-            false => None,
-        }
-    }))
-}
-
-fn lookup_datacenter_config_key(content: &str, key: &str) -> Option<String> {
-    let key_prefix = format!("{key}:");
-    normalize_for_return(
-        content
-            .lines()
-            .find_map(|line| line.strip_prefix(&key_prefix)),
-    )
-}
-
-#[derive(Debug)]
-struct PVEContext;
-
-impl Context for PVEContext {
-    fn lookup_email_for_user(&self, user: &str) -> Option<String> {
-        let content = attempt_file_read("/etc/pve/user.cfg");
-        content.and_then(|content| lookup_mail_address(&content, user))
-    }
-
-    fn default_sendmail_author(&self) -> String {
-        "Proxmox VE".into()
-    }
-
-    fn default_sendmail_from(&self) -> String {
-        let content = attempt_file_read("/etc/pve/datacenter.cfg");
-        content
-            .and_then(|content| lookup_datacenter_config_key(&content, "email_from"))
-            .unwrap_or_else(|| String::from("root"))
-    }
-
-    fn http_proxy_config(&self) -> Option<String> {
-        let content = attempt_file_read("/etc/pve/datacenter.cfg");
-        content.and_then(|content| lookup_datacenter_config_key(&content, "http_proxy"))
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    const USER_CONFIG: &str = "
-user:root@pam:1:0:::root@example.com:::
-user:test@pve:1:0:::test@example.com:::
-user:no-mail@pve:1:0::::::
-    ";
-
-    #[test]
-    fn test_parse_mail() {
-        assert_eq!(
-            lookup_mail_address(USER_CONFIG, "root@pam"),
-            Some("root@example.com".to_string())
-        );
-        assert_eq!(
-            lookup_mail_address(USER_CONFIG, "test@pve"),
-            Some("test@example.com".to_string())
-        );
-        assert_eq!(lookup_mail_address(USER_CONFIG, "no-mail@pve"), None);
-    }
-
-    const DC_CONFIG: &str = "
-email_from: user@example.com
-http_proxy: http://localhost:1234
-keyboard: en-us
-";
-    #[test]
-    fn test_parse_dc_config() {
-        assert_eq!(
-            lookup_datacenter_config_key(DC_CONFIG, "email_from"),
-            Some("user@example.com".to_string())
-        );
-        assert_eq!(
-            lookup_datacenter_config_key(DC_CONFIG, "http_proxy"),
-            Some("http://localhost:1234".to_string())
-        );
-        assert_eq!(lookup_datacenter_config_key(DC_CONFIG, "foo"), None);
-    }
-}
-
-static CONTEXT: PVEContext = PVEContext;
-
-pub fn init() {
-    proxmox_notify::context::set_context(&CONTEXT)
-}
-- 
2.39.2

[pve-devel] [PATCH v3 proxmox-mail-forward 3/5] feed forwarded mails into proxmox_notify

[Lukas Wagner]

This allows us to send notifications for events from daemons that are
not under our control, e.g. zed, smartd, cron. etc...

For mail-based notification targets (sendmail, soon smtp) the mail is
forwarded as is, including all headers.
All other target types will try to parse the email to extra subject
and text body.

On PBS, where proxmox-notify is not yet fully integrated,
we simply add a default target/matcher to an empty config. That way
the behavior should be unchanged - mails will be forwarded to
root@pam.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes v2 -> v3:
      - Update to matcher-based system - no need to read
        datacenter.cfg/node.cfg any more

 Cargo.toml  |   6 +-
 src/main.rs | 255 ++++++++++++++++++++++++----------------------------
 2 files changed, 121 insertions(+), 140 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index c68e802..a562f3e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -3,6 +3,7 @@ name = "proxmox-mail-forward"
 version = "0.2.0"
 authors = [
     "Fiona Ebner <f.ebner@proxmox.com>",
+    "Lukas Wagner <l.wagner@proxmox.com>",
     "Proxmox Support Team <support@proxmox.com>",
 ]
 edition = "2021"
@@ -16,10 +17,7 @@ exclude = [ "debian" ]
 anyhow = "1.0"
 log = "0.4.17"
 nix = "0.26"
-serde = { version = "1.0", features = ["derive"] }
-#serde_json = "1.0"
 syslog = "6.0"
 
-proxmox-schema = "1.3"
-proxmox-section-config = "1.0.2"
 proxmox-sys = "0.5"
+proxmox-notify = {version = "0.2", features = ["mail-forwarder", "pve-context", "pbs-context"] }
diff --git a/src/main.rs b/src/main.rs
index f3d4193..e56bc1e 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,132 +1,124 @@
+//! A helper binary that forwards any mail passed via stdin to
+//! proxmox_notify.
+//!
+//! The binary's path is added to /root/.forward, which means that
+//! postfix will invoke it when the local root user receives an email message.
+//! The message is passed via stdin.
+//! The binary is installed with setuid permissions and will thus run as
+//! root (euid ~ root, ruid ~ nobody)
+//!
+//! The forwarding behavior is the following:
+//!   - PVE installed: Use PVE's notifications.cfg
+//!   - PBS installed: Use PBS's notifications.cfg if present. If not,
+//!     use an empty configuration and add a default sendmail target and
+//!     a matcher - this is needed because notifications are not yet
+//!     integrated in PBS.
+//!   - PVE/PBS co-installed: Use PVE's config *and* PBS's config, but if
+//!     PBS's config does not exist, a default sendmail target will *not* be
+//!     added. We assume that PVE's config contains the desired notification
+//!     behavior for system mails.
+//!
+use std::io::Read;
 use std::path::Path;
-use std::process::Command;
 
-use anyhow::{bail, format_err, Error};
-use serde::Deserialize;
+use anyhow::Error;
 
-use proxmox_schema::{ObjectSchema, Schema, StringSchema};
-use proxmox_section_config::{SectionConfig, SectionConfigPlugin};
+use proxmox_notify::context::pbs::PBS_CONTEXT;
+use proxmox_notify::context::pve::PVE_CONTEXT;
+use proxmox_notify::endpoints::sendmail::SendmailConfig;
+use proxmox_notify::matcher::MatcherConfig;
+use proxmox_notify::Config;
 use proxmox_sys::fs;
 
-const PBS_USER_CFG_FILENAME: &str = "/etc/proxmox-backup/user.cfg";
-const PBS_ROOT_USER: &str = "root@pam";
-
-// FIXME: Switch to the actual schema when possible in terms of dependency.
-// It's safe to assume that the config was written with the actual schema restrictions, so parsing
-// it with the less restrictive schema should be enough for the purpose of getting the mail address.
-const DUMMY_ID_SCHEMA: Schema = StringSchema::new("dummy ID").min_length(3).schema();
-const DUMMY_EMAIL_SCHEMA: Schema = StringSchema::new("dummy email").schema();
-const DUMMY_USER_SCHEMA: ObjectSchema = ObjectSchema {
-    description: "minimal PBS user",
-    properties: &[
-        ("userid", false, &DUMMY_ID_SCHEMA),
-        ("email", true, &DUMMY_EMAIL_SCHEMA),
-    ],
-    additional_properties: true,
-    default_key: None,
-};
-
-#[derive(Deserialize)]
-struct DummyPbsUser {
-    pub email: Option<String>,
-}
-
-const PVE_USER_CFG_FILENAME: &str = "/etc/pve/user.cfg";
-const PVE_DATACENTER_CFG_FILENAME: &str = "/etc/pve/datacenter.cfg";
-const PVE_ROOT_USER: &str = "root@pam";
+const PVE_CFG_PATH: &str = "/etc/pve";
+const PVE_PUB_NOTIFICATION_CFG_FILENAME: &str = "/etc/pve/notifications.cfg";
+const PVE_PRIV_NOTIFICATION_CFG_FILENAME: &str = "/etc/pve/priv/notifications.cfg";
 
-/// Convenience helper to get the trimmed contents of an optional &str, mapping blank ones to `None`
-/// and creating a String from it for returning.
-fn normalize_for_return(s: Option<&str>) -> Option<String> {
-    match s?.trim() {
-        "" => None,
-        s => Some(s.to_string()),
-    }
-}
+const PBS_CFG_PATH: &str = "/etc/proxmox-backup";
+const PBS_PUB_NOTIFICATION_CFG_FILENAME: &str = "/etc/proxmox-backup/notifications.cfg";
+const PBS_PRIV_NOTIFICATION_CFG_FILENAME: &str = "/etc/proxmox-backup/notifications-priv.cfg";
 
-/// Extract the root user's email address from the PBS user config.
-fn get_pbs_mail_to(content: &str) -> Option<String> {
-    let mut config = SectionConfig::new(&DUMMY_ID_SCHEMA).allow_unknown_sections(true);
-    let user_plugin = SectionConfigPlugin::new(
-        "user".to_string(),
-        Some("userid".to_string()),
-        &DUMMY_USER_SCHEMA,
-    );
-    config.register_plugin(user_plugin);
-
-    match config.parse(PBS_USER_CFG_FILENAME, content) {
-        Ok(parsed) => {
-            parsed.sections.get(PBS_ROOT_USER)?;
-            match parsed.lookup::<DummyPbsUser>("user", PBS_ROOT_USER) {
-                Ok(user) => normalize_for_return(user.email.as_deref()),
-                Err(err) => {
-                    log::error!("unable to parse {} - {}", PBS_USER_CFG_FILENAME, err);
-                    None
-                }
-            }
-        }
+/// Wrapper around `proxmox_sys::fs::file_read_optional_string` which also returns `None` upon error
+/// after logging it.
+fn attempt_file_read<P: AsRef<Path>>(path: P) -> Option<String> {
+    match fs::file_read_optional_string(path.as_ref()) {
+        Ok(contents) => contents,
         Err(err) => {
-            log::error!("unable to parse {} - {}", PBS_USER_CFG_FILENAME, err);
+            log::error!("unable to read {path:?}: {err}", path = path.as_ref());
             None
         }
     }
 }
 
-/// Extract the root user's email address from the PVE user config.
-fn get_pve_mail_to(content: &str) -> Option<String> {
-    normalize_for_return(content.lines().find_map(|line| {
-        let fields: Vec<&str> = line.split(':').collect();
-        #[allow(clippy::get_first)] // to keep expression style consistent
-        match fields.get(0)?.trim() == "user" && fields.get(1)?.trim() == PVE_ROOT_USER {
-            true => fields.get(6).copied(),
-            false => None,
-        }
-    }))
-}
+/// Read data from stdin, until EOF is encountered.
+fn read_stdin() -> Result<Vec<u8>, Error> {
+    let mut input = Vec::new();
+    let stdin = std::io::stdin();
+    let mut handle = stdin.lock();
 
-/// Extract the From-address configured in the PVE datacenter config.
-fn get_pve_mail_from(content: &str) -> Option<String> {
-    normalize_for_return(
-        content
-            .lines()
-            .find_map(|line| line.strip_prefix("email_from:")),
-    )
+    handle.read_to_end(&mut input)?;
+    Ok(input)
 }
 
-/// Executes sendmail as a child process with the specified From/To-addresses, expecting the mail
-/// contents to be passed via stdin inherited from this program.
-fn forward_mail(mail_from: String, mail_to: Vec<String>) -> Result<(), Error> {
-    if mail_to.is_empty() {
-        bail!("user 'root@pam' does not have an email address");
-    }
+fn forward_common(mail: &[u8], config: &Config) -> Result<(), Error> {
+    let real_uid = nix::unistd::getuid();
+    // The uid is passed so that `sendmail` can be called as the a correct user.
+    // (sendmail will show a warning if called from a setuid process)
+    let notification =
+        proxmox_notify::Notification::new_forwarded_mail(mail, Some(real_uid.as_raw()))?;
+
+    proxmox_notify::api::common::send(config, &notification)?;
 
-    log::info!("forward mail to <{}>", mail_to.join(","));
+    Ok(())
+}
 
-    let mut cmd = Command::new("sendmail");
-    cmd.args([
-        "-bm", "-N", "never", // never send DSN (avoid mail loops)
-        "-f", &mail_from, "--",
-    ]);
-    cmd.args(mail_to);
-    cmd.env("PATH", "/sbin:/bin:/usr/sbin:/usr/bin");
+/// Forward a mail to PVE's notification system
+fn forward_for_pve(mail: &[u8]) -> Result<(), Error> {
+    let config = attempt_file_read(PVE_PUB_NOTIFICATION_CFG_FILENAME).unwrap_or_default();
+    let priv_config = attempt_file_read(PVE_PRIV_NOTIFICATION_CFG_FILENAME).unwrap_or_default();
 
-    // with status(), child inherits stdin
-    cmd.status()
-        .map_err(|err| format_err!("command {:?} failed - {}", cmd, err))?;
+    let config = Config::new(&config, &priv_config)?;
 
-    Ok(())
+    proxmox_notify::context::set_context(&PVE_CONTEXT);
+    forward_common(mail, &config)
 }
 
-/// Wrapper around `proxmox_sys::fs::file_read_optional_string` which also returns `None` upon error
-/// after logging it.
-fn attempt_file_read<P: AsRef<Path>>(path: P) -> Option<String> {
-    match fs::file_read_optional_string(path) {
-        Ok(contents) => contents,
-        Err(err) => {
-            log::error!("{}", err);
-            None
+/// Forward a mail to PBS's notification system
+fn forward_for_pbs(mail: &[u8], has_pve: bool) -> Result<(), Error> {
+    let config = if Path::new(PBS_PUB_NOTIFICATION_CFG_FILENAME).exists() {
+        let config = attempt_file_read(PBS_PUB_NOTIFICATION_CFG_FILENAME).unwrap_or_default();
+        let priv_config = attempt_file_read(PBS_PRIV_NOTIFICATION_CFG_FILENAME).unwrap_or_default();
+
+        Config::new(&config, &priv_config)?
+    } else {
+        // TODO: This can be removed once PBS has full notification integration
+        let mut config = Config::new("", "")?;
+        if !has_pve {
+            proxmox_notify::api::sendmail::add_endpoint(
+                &mut config,
+                &SendmailConfig {
+                    name: "default-target".to_string(),
+                    mailto_user: Some(vec!["root@pam".to_string()]),
+                    ..Default::default()
+                },
+            )?;
+
+            proxmox_notify::api::matcher::add_matcher(
+                &mut config,
+                &MatcherConfig {
+                    name: "default-matcher".to_string(),
+                    target: Some(vec!["default-target".to_string()]),
+                    ..Default::default()
+                },
+            )?;
         }
-    }
+        config
+    };
+
+    proxmox_notify::context::set_context(&PBS_CONTEXT);
+    forward_common(mail, &config)?;
+
+    Ok(())
 }
 
 fn main() {
@@ -135,40 +127,31 @@ fn main() {
         log::LevelFilter::Info,
         Some("proxmox-mail-forward"),
     ) {
-        eprintln!("unable to inititialize syslog - {}", err);
+        eprintln!("unable to initialize syslog: {err}");
     }
 
-    let pbs_user_cfg_content = attempt_file_read(PBS_USER_CFG_FILENAME);
-    let pve_user_cfg_content = attempt_file_read(PVE_USER_CFG_FILENAME);
-    let pve_datacenter_cfg_content = attempt_file_read(PVE_DATACENTER_CFG_FILENAME);
-
-    let real_uid = nix::unistd::getuid();
-    if let Err(err) = nix::unistd::setresuid(real_uid, real_uid, real_uid) {
-        log::error!(
-            "mail forward failed: unable to set effective uid to {}: {}",
-            real_uid,
-            err
-        );
-        return;
-    }
+    // Read the mail that is to be forwarded from stdin
+    match read_stdin() {
+        Ok(mail) => {
+            let mut has_pve = false;
 
-    let pbs_mail_to = pbs_user_cfg_content.and_then(|content| get_pbs_mail_to(&content));
-    let pve_mail_to = pve_user_cfg_content.and_then(|content| get_pve_mail_to(&content));
-    let pve_mail_from = pve_datacenter_cfg_content.and_then(|content| get_pve_mail_from(&content));
-
-    let mail_from = pve_mail_from.unwrap_or_else(|| "root".to_string());
+            // Assume a PVE installation if /etc/pve exists
+            if Path::new(PVE_CFG_PATH).exists() {
+                has_pve = true;
+                if let Err(err) = forward_for_pve(&mail) {
+                    log::error!("could not forward mail for Proxmox VE: {err}");
+                }
+            }
 
-    let mut mail_to = vec![];
-    if let Some(pve_mail_to) = pve_mail_to {
-        mail_to.push(pve_mail_to);
-    }
-    if let Some(pbs_mail_to) = pbs_mail_to {
-        if !mail_to.contains(&pbs_mail_to) {
-            mail_to.push(pbs_mail_to);
+            // Assume a PBS installation if /etc/proxmox-backup exists
+            if Path::new(PBS_CFG_PATH).exists() {
+                if let Err(err) = forward_for_pbs(&mail, has_pve) {
+                    log::error!("could not forward mail for Proxmox Backup Server: {err}");
+                }
+            }
+        }
+        Err(err) => {
+            log::error!("could not read mail from STDIN: {err}")
         }
-    }
-
-    if let Err(err) = forward_mail(mail_from, mail_to) {
-        log::error!("mail forward failed: {}", err);
     }
 }
-- 
2.39.2

[pve-devel] [PATCH v3 proxmox-mail-forward 4/5] update d/control

[Lukas Wagner]

proxmox-schema and proxmox-section config is not required anymore.
add new dependency to proxmox-notify.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes v2 -> v3:
      - new in v3

 debian/control | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/debian/control b/debian/control
index 43af70e..eb83a32 100644
--- a/debian/control
+++ b/debian/control
@@ -6,8 +6,10 @@ Build-Depends: cargo:native,
                librust-anyhow-1+default-dev,
                librust-log-0.4+default-dev (>= 0.4.17~~),
                librust-nix-0.26+default-dev,
-               librust-proxmox-schema-1+default-dev (>= 1.3~~),
-               librust-proxmox-section-config-1+default-dev (>= 1.0.2-~~),
+               librust-proxmox-notify-0.2+default-dev,
+               librust-proxmox-notify-0.2+mail-forwarder-dev,
+               librust-proxmox-notify-0.2+pbs-context-dev,
+               librust-proxmox-notify-0.2+pve-context-dev,
                librust-proxmox-sys-0.5+default-dev,
                librust-serde-1+default-dev,
                librust-serde-1+derive-dev,
-- 
2.39.2

[pve-devel] [PATCH v3 pve-docs 5/5] notifications: add documentation for system mail forwarding

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes v2 -> v3:
      - Dropped paragraph about target/policy, since we now do routing in
        matchers

 notifications.adoc | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/notifications.adoc b/notifications.adoc
index 764ec72..f0eee9d 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -231,6 +231,7 @@ Notification Events
 | Cluster node fenced          |`fencing`          | `error`  | `hostname`
 | Storage replication failed   |`replication`      | `error`  | -
 | Backup finished              |`vzdump`           | `info` (`error` on failure) | `hostname`
+| Mail for root                |`system-mail`      | `unknown`| -
 |===========================================================================
 
 [width="100%",options="header"]
@@ -240,6 +241,21 @@ Notification Events
 | `hostname` | Hostname, including domain (e.g. `pve1.example.com`)
 |=======================================================================
 
+System Mail Forwarding
+---------------------
+
+Certain local system daemons, such as `smartd`, generate notification emails
+that are initially directed to the local `root` user. {pve} will
+feed these mails into the notification system as a notification of
+type `system-mail` and with severity `unknown`.
+
+When the forwarding process involves an email-based target
+(like `sendmail` or `smtp`), the email is forwarded exactly as received, with all
+original mail headers remaining intact. For all other targets,
+the system tries to extract both a subject line and the main text body
+from the email content. In instances where emails solely consist of HTML
+content, they will be transformed into plain text format during this process.
+
 Permissions
 -----------
 
-- 
2.39.2