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 04DA61FF0E5 for ; Wed, 29 Jul 2026 12:09:18 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id B00C9213FA; Wed, 29 Jul 2026 12:09:17 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Wed, 29 Jul 2026 12:08:52 +0200 Message-Id: To: "Shannon Sterz" , "Lukas Wagner" , Subject: Re: [PATCH proxmox-backup 04/11] config/server/api: add certificate renewal logic including notifications From: "Lukas Wagner" 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: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1785319693890 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.251 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) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust 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: KHTFKWE5M6QH55IZ5HKE4IXRPOQR2D5T X-Message-ID-Hash: KHTFKWE5M6QH55IZ5HKE4IXRPOQR2D5T 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: On Wed Jul 29, 2026 at 11:53 AM CEST, Shannon Sterz wrote: >>> +/// Send email for upcoming self signed renewal. >>> +pub fn send_upcoming_self_signed_renewal_notification() -> Result<(), = Error> { >>> + 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? > > imo no, at least not yet. this series does not stage certificates yet > (like my series for pve would [1]), so there is nothing a user can do at > this stage. however, the notification further down that a renewal has > happened needs more attention as fingerprints would need to be > exchanged. so i'll adjust the severity there. > > once we have a staging mechanism in place, we can include the new > fingerprint in the notification and increase the severity here to make > users aware this will happen. we can then tell the user to make sure to > check that any clients are either updated automatically (through a > mechanism similar to the one in that series) or that they should add the > fingerprint manually. > > [1]: https://lore.proxmox.com/all/20260611120327.257523-1-s.sterz@proxmox= .com/ > Ack! -->8 snip 8<-- >> >> 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. > > we could, but the point of using acme usually is that you use > certificates that are trusted by other hosts in you environment. > meaning, you either use a public acme provider that is generally trusted > or a private one and add your root cert to the trust stores of the > systems in your environment. > > this means, most users using acme (usually) don't need to do anything > when successfully refreshing. whereas, a refreshed self-signed > certificate requires exchanging fingerprints again. otherwise, backups > will start failing. > > so imo, if we want to add a notification for that, it should be lower > severity than the self-signed certificate one. > ack! >>> + }; >>> + >>> + send_notification(notification) >>> +} > > -->8 snip 8<-- > >>> diff --git a/templates/default/cert-upcoming-refresh-body.txt.hbs b/tem= plates/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. > > i can add a line that the earliest a certificate will updated is in X > days. > > adding a date is imo a bit trickier due to localization. as far as i can > tell, handlebars [1] currently has no date formatting helpers. so either > we implement our own, which seems orthogonal to this series and probably > not worth it if we do decide to switch templating engines in the near > future. or we don't care about localization and simply decide on one > format (e.g. YYYY-MM-DD). > > what do you think? > > [1]: https://github.com/sunng87/handlebars-rust/issues/614 Notifications are not localized at all right now, so I think if we just use ISO 8601 (so YYYY-MM-DD), it should be clear enough for most users. > > -->8 snip 8<--