From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id BBE981FF13A for ; Wed, 15 Apr 2026 09:02:46 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id BEE40317E; Wed, 15 Apr 2026 09:02:33 +0200 (CEST) From: Arthur Bied-Charreton To: pve-devel@lists.proxmox.com Subject: [PATCH proxmox v3 03/23] notify: smtp: Introduce state management Date: Wed, 15 Apr 2026 09:02:00 +0200 Message-ID: <20260415070220.100306-4-a.bied-charreton@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260415070220.100306-1-a.bied-charreton@proxmox.com> References: <20260415070220.100306-1-a.bied-charreton@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.125 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [smtp.rs,pve.rs,xoauth2.rs,mod.rs,test.rs,lib.rs,pbs.rs] Message-ID-Hash: FPVUWSKXBEEWYL37BKYXT2MTHE4NDNDQ X-Message-ID-Hash: FPVUWSKXBEEWYL37BKYXT2MTHE4NDNDQ X-MailFrom: abied-charreton@jett.proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Export a new State struct in the xoauth2 module with associated functionality for loading, updating, and persisting the OAuth2 state for SMTP endpoints. The API for loading and saving the state is exposed through the Context trait, in order to make migration as easy as possible in a future where we might want to move towards KV storage instead of files for secret management. It is made specific to oauth state, because this implementation assumes invariants that hold for oauth2 refresh tokens (documented in the smtp::xoauth2 module's doc comments), but are likely to be incorrect for other kinds of state that may be added in the future. The State struct is public in order to support the long-term goal for the Context trait to be implemented by the products themselves. The nix crate is added for the sys::stat::Mode struct, and proxmox-sys is now pulled in unconditionally since it is used in the Context implementations. Signed-off-by: Arthur Bied-Charreton Reviewed-by: Lukas Wagner --- proxmox-notify/Cargo.toml | 12 ++- proxmox-notify/debian/control | 41 +++------ proxmox-notify/src/context/mod.rs | 17 ++++ proxmox-notify/src/context/pbs.rs | 23 +++++ proxmox-notify/src/context/pve.rs | 25 ++++- proxmox-notify/src/context/test.rs | 22 +++++ proxmox-notify/src/endpoints/smtp.rs | 2 + proxmox-notify/src/endpoints/smtp/xoauth2.rs | 97 ++++++++++++++++++++ proxmox-notify/src/lib.rs | 12 +++ 9 files changed, 218 insertions(+), 33 deletions(-) diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml index c0d9921b..873a7d6f 100644 --- a/proxmox-notify/Cargo.toml +++ b/proxmox-notify/Cargo.toml @@ -36,16 +36,18 @@ proxmox-schema = { workspace = true, features = ["api-macro", "api-types"] } proxmox-section-config = { workspace = true } proxmox-serde.workspace = true proxmox-sendmail = { workspace = true, optional = true } -proxmox-sys = { workspace = true, optional = true } +proxmox-sys = { workspace = true } proxmox-time.workspace = true proxmox-uuid = { workspace = true, features = ["serde"] } +nix = { workspace = true } + [features] default = ["sendmail", "gotify", "smtp", "webhook"] -mail-forwarder = ["dep:mail-parser", "dep:proxmox-sys", "proxmox-sendmail/mail-forwarder"] -sendmail = ["dep:proxmox-sys", "dep:proxmox-sendmail"] +mail-forwarder = ["dep:mail-parser", "proxmox-sendmail/mail-forwarder"] +sendmail = ["dep:proxmox-sendmail"] gotify = ["dep:proxmox-http", "dep:http"] -pve-context = ["dep:proxmox-sys"] -pbs-context = ["dep:proxmox-sys"] +pve-context = [] +pbs-context = [] smtp = ["dep:lettre", "dep:oauth2", "dep:ureq", "dep:http"] webhook = ["dep:http", "dep:percent-encoding", "dep:proxmox-base64", "dep:proxmox-http"] diff --git a/proxmox-notify/debian/control b/proxmox-notify/debian/control index 1b5c4068..b8db398c 100644 --- a/proxmox-notify/debian/control +++ b/proxmox-notify/debian/control @@ -11,6 +11,7 @@ Build-Depends-Arch: cargo:native , librust-handlebars-5+default-dev , librust-http-1+default-dev , librust-lettre-0.11+default-dev (>= 0.11.1-~~) , + librust-nix-0.29+default-dev , librust-oauth2-5-dev , librust-openssl-0.10+default-dev , librust-percent-encoding-2+default-dev (>= 2.1-~~) , @@ -51,6 +52,7 @@ Depends: librust-anyhow-1+default-dev, librust-const-format-0.2+default-dev, librust-handlebars-5+default-dev, + librust-nix-0.29+default-dev, librust-openssl-0.10+default-dev, librust-proxmox-http-error-1+default-dev, librust-proxmox-human-byte-1+default-dev, @@ -60,6 +62,7 @@ Depends: librust-proxmox-section-config-3+default-dev (>= 3.1.0-~~), librust-proxmox-serde-1+default-dev, librust-proxmox-serde-1+serde-json-dev, + librust-proxmox-sys-1+default-dev (>= 1.0.1-~~), librust-proxmox-time-2+default-dev (>= 2.1.0-~~), librust-proxmox-uuid-1+default-dev (>= 1.1.0-~~), librust-proxmox-uuid-1+serde-dev (>= 1.1.0-~~), @@ -73,14 +76,21 @@ Recommends: Suggests: librust-proxmox-notify+gotify-dev (= ${binary:Version}), librust-proxmox-notify+mail-forwarder-dev (= ${binary:Version}), - librust-proxmox-notify+pbs-context-dev (= ${binary:Version}), librust-proxmox-notify+sendmail-dev (= ${binary:Version}), librust-proxmox-notify+smtp-dev (= ${binary:Version}), librust-proxmox-notify+webhook-dev (= ${binary:Version}) Provides: + librust-proxmox-notify+pbs-context-dev (= ${binary:Version}), + librust-proxmox-notify+pve-context-dev (= ${binary:Version}), librust-proxmox-notify-1-dev (= ${binary:Version}), + librust-proxmox-notify-1+pbs-context-dev (= ${binary:Version}), + librust-proxmox-notify-1+pve-context-dev (= ${binary:Version}), librust-proxmox-notify-1.0-dev (= ${binary:Version}), - librust-proxmox-notify-1.0.3-dev (= ${binary:Version}) + librust-proxmox-notify-1.0+pbs-context-dev (= ${binary:Version}), + librust-proxmox-notify-1.0+pve-context-dev (= ${binary:Version}), + librust-proxmox-notify-1.0.3-dev (= ${binary:Version}), + librust-proxmox-notify-1.0.3+pbs-context-dev (= ${binary:Version}), + librust-proxmox-notify-1.0.3+pve-context-dev (= ${binary:Version}) Description: Notification base and plugins - Rust source code Source code for Debianized Rust crate "proxmox-notify" @@ -126,8 +136,7 @@ Depends: ${misc:Depends}, librust-proxmox-notify-dev (= ${binary:Version}), librust-mail-parser-0.11+default-dev, - librust-proxmox-sendmail-1+mail-forwarder-dev (>= 1.0.2-~~), - librust-proxmox-sys-1+default-dev (>= 1.0.1-~~) + librust-proxmox-sendmail-1+mail-forwarder-dev (>= 1.0.2-~~) Provides: librust-proxmox-notify-1+mail-forwarder-dev (= ${binary:Version}), librust-proxmox-notify-1.0+mail-forwarder-dev (= ${binary:Version}), @@ -136,35 +145,13 @@ Description: Notification base and plugins - feature "mail-forwarder" This metapackage enables feature "mail-forwarder" for the Rust proxmox-notify crate, by pulling in any additional dependencies needed by that feature. -Package: librust-proxmox-notify+pbs-context-dev -Architecture: any -Multi-Arch: same -Depends: - ${misc:Depends}, - librust-proxmox-notify-dev (= ${binary:Version}), - librust-proxmox-sys-1+default-dev (>= 1.0.1-~~) -Provides: - librust-proxmox-notify+pve-context-dev (= ${binary:Version}), - librust-proxmox-notify-1+pbs-context-dev (= ${binary:Version}), - librust-proxmox-notify-1+pve-context-dev (= ${binary:Version}), - librust-proxmox-notify-1.0+pbs-context-dev (= ${binary:Version}), - librust-proxmox-notify-1.0+pve-context-dev (= ${binary:Version}), - librust-proxmox-notify-1.0.3+pbs-context-dev (= ${binary:Version}), - librust-proxmox-notify-1.0.3+pve-context-dev (= ${binary:Version}) -Description: Notification base and plugins - feature "pbs-context" and 1 more - This metapackage enables feature "pbs-context" for the Rust proxmox-notify - crate, by pulling in any additional dependencies needed by that feature. - . - Additionally, this package also provides the "pve-context" feature. - Package: librust-proxmox-notify+sendmail-dev Architecture: any Multi-Arch: same Depends: ${misc:Depends}, librust-proxmox-notify-dev (= ${binary:Version}), - librust-proxmox-sendmail-1+default-dev (>= 1.0.2-~~), - librust-proxmox-sys-1+default-dev (>= 1.0.1-~~) + librust-proxmox-sendmail-1+default-dev (>= 1.0.2-~~) Provides: librust-proxmox-notify-1+sendmail-dev (= ${binary:Version}), librust-proxmox-notify-1.0+sendmail-dev (= ${binary:Version}), diff --git a/proxmox-notify/src/context/mod.rs b/proxmox-notify/src/context/mod.rs index 8b6e2c43..a3942e5f 100644 --- a/proxmox-notify/src/context/mod.rs +++ b/proxmox-notify/src/context/mod.rs @@ -1,6 +1,8 @@ use std::fmt::Debug; use std::sync::Mutex; +#[cfg(feature = "smtp")] +use crate::endpoints::smtp::State; use crate::renderer::TemplateSource; use crate::Error; @@ -32,6 +34,21 @@ pub trait Context: Send + Sync + Debug { namespace: Option<&str>, source: TemplateSource, ) -> Result, Error>; + /// Load OAuth state for `endpoint_name`. + /// + /// The state file does not need to be locked, it is okay to just let the faster node "win" + /// as long as the invariants documented by [`smtp::xoauth2::get_microsoft_token`] and + /// [`smtp::xoauth2::get_google_token`] hold, see those functions' doc comments for details. + #[cfg(feature = "smtp")] + fn load_oauth_state(&self, endpoint_name: &str) -> Result; + /// Save OAuth state `state` for `endpoint_name`. Passing `None` deletes + /// the state file for `endpoint_name`. + /// + /// The state file does not need to be locked, it is okay to just let the faster node "win" + /// as long as the invariants documented by [`smtp::xoauth2::get_microsoft_token`] and + /// [`smtp::xoauth2::get_google_token`] hold, see those functions' doc comments for details. + #[cfg(feature = "smtp")] + fn save_oauth_state(&self, endpoint_name: &str, state: Option) -> Result<(), Error>; } #[cfg(not(test))] diff --git a/proxmox-notify/src/context/pbs.rs b/proxmox-notify/src/context/pbs.rs index 3e5da59c..8c5fce6b 100644 --- a/proxmox-notify/src/context/pbs.rs +++ b/proxmox-notify/src/context/pbs.rs @@ -1,5 +1,6 @@ use std::path::Path; +use proxmox_sys::fs::CreateOptions; use serde::Deserialize; use tracing::error; @@ -7,6 +8,8 @@ use proxmox_schema::{ObjectSchema, Schema, StringSchema}; use proxmox_section_config::{SectionConfig, SectionConfigPlugin}; use crate::context::{common, Context}; +#[cfg(feature = "smtp")] +use crate::endpoints::smtp::State; use crate::renderer::TemplateSource; use crate::Error; @@ -125,6 +128,26 @@ impl Context for PBSContext { .map_err(|err| Error::Generic(format!("could not load template: {err}")))?; Ok(template_string) } + + #[cfg(feature = "smtp")] + fn load_oauth_state(&self, endpoint_name: &str) -> Result { + let path = + format!("/var/lib/proxmox-backup/notifications/oauth-state/{endpoint_name}.json"); + State::load(path) + } + + #[cfg(feature = "smtp")] + fn save_oauth_state(&self, endpoint_name: &str, state: Option) -> Result<(), Error> { + let path = + format!("/var/lib/proxmox-backup/notifications/oauth-state/{endpoint_name}.json"); + match state { + Some(s) => s.save( + path, + CreateOptions::new().perm(nix::sys::stat::Mode::from_bits_truncate(0o600)), + ), + None => Ok(State::delete(path)), + } + } } #[cfg(test)] diff --git a/proxmox-notify/src/context/pve.rs b/proxmox-notify/src/context/pve.rs index a97cce26..2befd53b 100644 --- a/proxmox-notify/src/context/pve.rs +++ b/proxmox-notify/src/context/pve.rs @@ -1,7 +1,12 @@ +use std::path::Path; + +use proxmox_sys::fs::CreateOptions; + use crate::context::{common, Context}; +#[cfg(feature = "smtp")] +use crate::endpoints::smtp::State; use crate::renderer::TemplateSource; use crate::Error; -use std::path::Path; fn lookup_mail_address(content: &str, user: &str) -> Option { common::normalize_for_return(content.lines().find_map(|line| { @@ -74,6 +79,24 @@ impl Context for PVEContext { .map_err(|err| Error::Generic(format!("could not load template: {err}")))?; Ok(template_string) } + + #[cfg(feature = "smtp")] + fn load_oauth_state(&self, endpoint_name: &str) -> Result { + let path = format!("/etc/pve/priv/notifications/oauth-state/{endpoint_name}.json"); + State::load(path) + } + + #[cfg(feature = "smtp")] + fn save_oauth_state(&self, endpoint_name: &str, state: Option) -> Result<(), Error> { + let path = format!("/etc/pve/priv/notifications/oauth-state/{endpoint_name}.json"); + match state { + Some(s) => s.save( + path, + CreateOptions::new().perm(nix::sys::stat::Mode::from_bits_truncate(0o600)), + ), + None => Ok(State::delete(path)), + } + } } pub static PVE_CONTEXT: PVEContext = PVEContext; diff --git a/proxmox-notify/src/context/test.rs b/proxmox-notify/src/context/test.rs index 2c236b4c..9a653343 100644 --- a/proxmox-notify/src/context/test.rs +++ b/proxmox-notify/src/context/test.rs @@ -1,4 +1,8 @@ +use proxmox_sys::fs::CreateOptions; + use crate::context::Context; +#[cfg(feature = "smtp")] +use crate::endpoints::smtp::State; use crate::renderer::TemplateSource; use crate::Error; @@ -40,4 +44,22 @@ impl Context for TestContext { ) -> Result, Error> { Ok(Some(String::new())) } + + #[cfg(feature = "smtp")] + fn load_oauth_state(&self, endpoint_name: &str) -> Result { + let path = format!("/tmp/notifications/oauth-state/{endpoint_name}.json"); + State::load(path) + } + + #[cfg(feature = "smtp")] + fn save_oauth_state(&self, endpoint_name: &str, state: Option) -> Result<(), Error> { + let path = format!("/tmp/notifications/oauth-state/{endpoint_name}.json"); + match state { + Some(s) => s.save( + path, + CreateOptions::new().perm(nix::sys::stat::Mode::from_bits_truncate(0o750)), + ), + None => Ok(State::delete(path)), + } + } } diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs index d1cdb540..172bcdba 100644 --- a/proxmox-notify/src/endpoints/smtp.rs +++ b/proxmox-notify/src/endpoints/smtp.rs @@ -25,6 +25,8 @@ const SMTP_TIMEOUT: u16 = 5; mod xoauth2; +pub use xoauth2::State; + #[api] #[derive(Debug, Serialize, Deserialize, Default, Clone, Copy)] #[serde(rename_all = "kebab-case")] diff --git a/proxmox-notify/src/endpoints/smtp/xoauth2.rs b/proxmox-notify/src/endpoints/smtp/xoauth2.rs index 06da0e79..1df5b447 100644 --- a/proxmox-notify/src/endpoints/smtp/xoauth2.rs +++ b/proxmox-notify/src/endpoints/smtp/xoauth2.rs @@ -1,10 +1,107 @@ +use std::path::Path; + use oauth2::{ basic::BasicClient, AccessToken, AuthUrl, ClientId, ClientSecret, RefreshToken, TokenResponse, TokenUrl, }; +use serde::{Deserialize, Serialize}; +use tracing::{debug, error}; use crate::Error; +#[derive(Serialize, Deserialize, Clone, Debug, Default)] +#[serde(rename_all = "kebab-case")] +/// Persistent state for XOAUTH2 SMTP endpoints. +pub struct State { + /// OAuth2 refresh token for this endpoint. + #[serde(skip_serializing_if = "Option::is_none")] + pub oauth2_refresh_token: Option, + /// Unix timestamp (seconds) of the last successful token refresh. + pub last_refreshed: i64, +} + +impl State { + /// Instantiate a new [`State`]. `last_refreshed` is expected to be the UNIX + /// timestamp (seconds) of the instantiation time. + pub fn new(refresh_token: String, last_refreshed: i64) -> Self { + Self { + oauth2_refresh_token: Some(refresh_token), + last_refreshed, + } + } + + /// Load state from `path` instantiating a default object if no state exists. + /// + /// # Errors + /// An [`Error`] is returned if deserialization of the state object fails. + pub fn load>(path: P) -> Result { + let path_str = path.as_ref().to_string_lossy(); + match proxmox_sys::fs::file_get_optional_contents(&path) + .map_err(|e| Error::StateRetrieval(path_str.to_string(), e.into()))? + { + Some(bytes) => { + debug!("loaded state file from {path_str}"); + serde_json::from_slice(&bytes) + .map_err(|e| Error::StateRetrieval(path_str.to_string(), e.into())) + } + None => { + debug!( + "no existing state file found for endpoint at {path_str}, creating empty state" + ); + Ok(State::default()) + } + } + } + + /// Persist the state at `path` with `options`. + /// + /// # Errors + /// An [`Error`] is returned if serialization of the state object, or the final write, fail. + pub fn save>( + self, + path: P, + options: proxmox_sys::fs::CreateOptions, + ) -> Result<(), Error> { + let path_str = path.as_ref().to_string_lossy(); + let parent = path.as_ref().parent().unwrap(); + + debug!("attempting to persist state at {path_str}"); + + proxmox_sys::fs::create_path(parent, Some(options), Some(options)) + .map_err(|e| Error::StatePersistence(path_str.to_string(), e.into()))?; + + let s = serde_json::to_string_pretty(&self) + .map_err(|e| Error::StatePersistence(path_str.to_string(), e.into()))?; + + proxmox_sys::fs::replace_file(&path, s.as_bytes(), options, false) + .map_err(|e| Error::StatePersistence(path_str.to_string(), e.into())) + } + + /// Delete the state file at `path`. + /// + /// Errors are logged but not propagated. + pub fn delete>(path: P) { + if let Err(e) = std::fs::remove_file(&path) + && e.kind() != std::io::ErrorKind::NotFound + { + let path_str = path.as_ref().to_string_lossy(); + error!("could not delete state file at {path_str}: {e}"); + } + } + + /// Set `last_refreshed`. + pub fn set_last_refreshed(mut self, last_refreshed: i64) -> Self { + self.last_refreshed = last_refreshed; + self + } + + /// Set `oauth2_refresh_token`. + pub fn set_oauth2_refresh_token(mut self, oauth2_refresh_token: Option) -> Self { + self.oauth2_refresh_token = oauth2_refresh_token; + self + } +} + /// This newtype implements the `SyncHttpClient` trait for [`ureq::Agent`]. This allows /// us to avoid pulling in a different backend like `reqwest`. /// diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs index 879f8326..619dd7db 100644 --- a/proxmox-notify/src/lib.rs +++ b/proxmox-notify/src/lib.rs @@ -41,6 +41,10 @@ pub enum Error { FilterFailed(String), /// The notification's template string could not be rendered RenderError(Box), + /// The state for an endpoint could not be persisted + StatePersistence(String, Box), + /// The state for an endpoint could not be retrieved + StateRetrieval(String, Box), /// Generic error for anything else Generic(String), } @@ -70,6 +74,12 @@ impl Display for Error { Error::FilterFailed(message) => { write!(f, "could not apply filter: {message}") } + Error::StatePersistence(path, err) => { + write!(f, "could not persist state at {path}: {err}") + } + Error::StateRetrieval(path, err) => { + write!(f, "could not retrieve state from {path}: {err}") + } Error::RenderError(err) => write!(f, "could not render notification template: {err}"), Error::Generic(message) => f.write_str(message), } @@ -86,6 +96,8 @@ impl StdError for Error { Error::TargetTestFailed(errs) => Some(&*errs[0]), Error::FilterFailed(_) => None, Error::RenderError(err) => Some(&**err), + Error::StatePersistence(_, err) => Some(&**err), + Error::StateRetrieval(_, err) => Some(&**err), Error::Generic(_) => None, } } -- 2.47.3