Re: [PATCH proxmox-backup 04/11] config/server/api: add certificate renewal logic including notifications

["Lukas Wagner" <l.wagner@proxmox.com>]

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 = crate::api2::node::dns::read_etc_resolv_conf()?;
>  
>      let (priv_key, cert) = 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/mod.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(())
>  }
>  
> +/// Send email for upcoming self signed renewal.
> +pub fn send_upcoming_self_signed_renewal_notification() -> Result<(), Error> {
> +    let metadata = HashMap::from([
> +        ("hostname".into(), proxmox_sys::nodename().into()),
> +        ("type".into(), "cert".into()),
> +    ]);
> +
> +    let notification = 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 = 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 = match result {
> +        Err(e) => {
> +            let template_data = 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,
> +            )
> +        }
> +        _ => 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 threshold limit.
>  pub fn send_datastore_threshold_exceeded(
>      datastore: &str,
[...]
> diff --git a/templates/default/cert-refresh-body.txt.hbs b/templates/default/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/default/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/templates/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 30 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 between 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/templates/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