From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id C30B51FF0E7 for ; Fri, 10 Jul 2026 13:17:45 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 43ED92143D; Fri, 10 Jul 2026 13:17:45 +0200 (CEST) Content-Type: text/plain; charset=UTF-8 Date: Fri, 10 Jul 2026 13:17:10 +0200 Message-Id: Subject: Re: [PATCH proxmox-backup 04/11] config/server/api: add certificate renewal logic including notifications From: "Lukas Wagner" To: "Shannon Sterz" , Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20260618115443.48618-1-s.sterz@proxmox.com> <20260618115443.48618-5-s.sterz@proxmox.com> In-Reply-To: <20260618115443.48618-5-s.sterz@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1783682219705 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.000 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: FQ4ULXK5M3326T7K23MJM4MZTBIATTFN X-Message-ID-Hash: FQ4ULXK5M3326T7K23MJM4MZTBIATTFN X-MailFrom: l.wagner@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 Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Hi Shannon, some notes inline. On Thu Jun 18, 2026 at 1:54 PM CEST, Shannon Sterz wrote: > /// Check configuration directory permissions > /// > /// For security reasons, we want to make sure they are set correctly: > @@ -89,7 +91,7 @@ pub fn update_self_signed_cert(force: bool) -> Result<(= ), Error> { > let resolv_conf =3D crate::api2::node::dns::read_etc_resolv_conf()?; > =20 > let (priv_key, cert) =3D proxmox_acme_api::create_self_signed_cert( > - "Proxmox Backup Server", > + PRODUCT_NAME, > proxmox_sys::nodename(), > resolv_conf["search"].as_str(), > None, > diff --git a/src/server/notifications/mod.rs b/src/server/notifications/m= od.rs > index 0f772ddd9..2b55e980e 100644 > --- a/src/server/notifications/mod.rs > +++ b/src/server/notifications/mod.rs > @@ -575,6 +575,56 @@ pub fn send_certificate_renewal_mail(result: &Result= <(), Error>) -> Result<(), E > Ok(()) > } > =20 > +/// Send email for upcoming self signed renewal. > +pub fn send_upcoming_self_signed_renewal_notification() -> Result<(), Er= ror> { > + let metadata =3D HashMap::from([ > + ("hostname".into(), proxmox_sys::nodename().into()), > + ("type".into(), "cert".into()), > + ]); > + > + let notification =3D Notification::from_template( > + Severity::Info, Maybe this should use Severity::Notice? Since it might require some attention from the user? > + "cert-upcoming-refresh", > + serde_json::to_value(CommonData::new())?, > + metadata, > + ); > + > + send_notification(notification)?; > + Ok(()) > +} > + > +/// Send email renewed self-signed certificate > +pub fn send_self_signed_renewal_notification(result: &Result<(), Error>)= -> Result<(), Error> { > + let metadata =3D HashMap::from([ > + ("hostname".into(), proxmox_sys::nodename().into()), > + ("type".into(), "acme".into()), I guess the type should be "cert"? Also, maybe a longer 'cert-renewal' might be clearer. Maybe we could consolidate the ACME and self-signed notifications at some point (probably during a major upgrade, since notification matchers might rely on the type being 'acme') > + ]); > + > + let notification =3D match result { > + Err(e) =3D> { > + let template_data =3D AcmeErrTemplateData { > + common: CommonData::new(), > + error: format!("{e:#}"), > + }; > + > + Notification::from_template( > + Severity::Info, This should be Severity::Error. Seems like in the pre-existing ACME certificate notification it also only uses Info, which seems like a bug to me. Feel free to fix it there as well. > + "acme-err", Considering that templates can be overridden by users, we might want use a dedicated template for the self-signed ones. Or, as mentioned above, consolidate the notification types, which *could* (but does not have to) mean that we also use the same templates for both, ACME and self-signed. > + serde_json::to_value(template_data)?, > + metadata, > + ) > + } > + _ =3D> Notification::from_template( > + Severity::Info, > + "cert-refresh", > + serde_json::to_value(CommonData::new())?, > + metadata, > + ), I've just realized that we don't really send notifications for successful ACME cert refreshes, so maybe we should do this in the future as well. This would of course be a separate patch series, just thinking out aloud here. > + }; > + > + send_notification(notification) > +} > + > /// Send notification if datastore values are exceeding the set threshol= d limit. > pub fn send_datastore_threshold_exceeded( > datastore: &str, [...] > diff --git a/templates/default/cert-refresh-body.txt.hbs b/templates/defa= ult/cert-refresh-body.txt.hbs > new file mode 100644 > index 000000000..608ba30c0 > --- /dev/null > +++ b/templates/default/cert-refresh-body.txt.hbs > @@ -0,0 +1,8 @@ > +Proxmox Backup Server has refreshed its self-signed TLS certificate. > + > +The new certificate is now active. Please update any clients relying on = the > +certificate fingerprint to verify their connection to the server. > + > +Please visit the web interface for further details: > + > +<{{base-url}}/#pbsCertificateConfiguration> > diff --git a/templates/default/cert-refresh-subject.txt.hbs b/templates/d= efault/cert-refresh-subject.txt.hbs > new file mode 100644 > index 000000000..e57f5cd4c > --- /dev/null > +++ b/templates/default/cert-refresh-subject.txt.hbs > @@ -0,0 +1 @@ > +Self-Signed Certificate Has Been Refreshed > diff --git a/templates/default/cert-upcoming-refresh-body.txt.hbs b/templ= ates/default/cert-upcoming-refresh-body.txt.hbs > new file mode 100644 > index 000000000..8d199c9fd > --- /dev/null > +++ b/templates/default/cert-upcoming-refresh-body.txt.hbs > @@ -0,0 +1,9 @@ > +Proxmox Backup Server will refresh its TLS certificate within the next 3= 0 days. I wonder, would it be possible to include a concrete date here? If I understood it correctly, the first time the user receives this notification, the renewal would be first attempted in 15 days at the earliest, so maybe this should be mentioned. > + > +If you rely on the certificate's fingerprint to verify TLS sessions betw= een the > +server and a client, please update the fingerprint once the certificate = was > +updated. Otherwise, no action is required. > + > +Please visit the web interface for further details: > + > +<{{base-url}}/#pbsCertificateConfiguration> > diff --git a/templates/default/cert-upcoming-refresh-subject.txt.hbs b/te= mplates/default/cert-upcoming-refresh-subject.txt.hbs > new file mode 100644 > index 000000000..f188e391c > --- /dev/null > +++ b/templates/default/cert-upcoming-refresh-subject.txt.hbs > @@ -0,0 +1 @@ > +Self-signed Certificate is About to be Refreshed