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 88FC41FF0E4 for ; Tue, 28 Jul 2026 15:32:52 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 53EB021321; Tue, 28 Jul 2026 15:32:52 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 28 Jul 2026 15:32:48 +0200 Message-Id: From: "Shan Shaji" To: "Manuel Federanko" , , Subject: Re: [PATCH proxmox-backup 6/7] acme: fix #6372 implement ARI renewal information fetching. X-Mailer: aerc 0.20.0 References: <20260625141337.181684-1-m.federanko@proxmox.com> <20260625141337.181684-7-m.federanko@proxmox.com> In-Reply-To: <20260625141337.181684-7-m.federanko@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1785245531820 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.128 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: UQGNCXQORIY725ZSZTEETG2NP2JUL73F X-Message-ID-Hash: UQGNCXQORIY725ZSZTEETG2NP2JUL73F X-MailFrom: s.shaji@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 Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: [snip] > +/// ARI renewal time if available > +/// > +/// Query the ARI endpoint for a suggested renewal window, draw a unifor= m random time in this window > +/// Return None if ARI does not apply. > +async fn cert_renew_lead_time_ari( > + acme_config: &AcmeConfig, > + cert_info: &cert::CertInfo, > +) -> Result, Error> { > + let now =3D proxmox_time::epoch_i64(); > + if cert_info.is_expired_after_epoch(now)? { > + return Ok(Some(0)); > + } > + let ari_id =3D &match cert_info.ari_id() { =20 tiny nit: I think this `&` is not necessary here as deref coercion happens = later. > + Some(x) =3D> x, > + None =3D> return Ok(None), > + }; > + let window =3D match proxmox_acme_api::get_renewal_info(acme_config,= &ari_id).await? { > + Some(x) =3D> x, > + None =3D> return Ok(None), > + }; > + if let Some(reason) =3D window.data.explanation_url { > + info!( > + "Obtained renewal window, for information on this chosen win= dow please visit {reason}" > + ); > + } > + let window_start =3D proxmox_time::parse_rfc3339(&window.data.sugges= ted_window.start)?; > + let window_end =3D proxmox_time::parse_rfc3339(&window.data.suggeste= d_window.end)?; > + let rand =3D proxmox_sys::linux::random_data(8)? > + .into_iter() > + .enumerate() > + .fold(0, |acc, (index, x)| acc + ((x as u64) << (index * 8))) as= f64 optional: Instead of manually creating u64 from the bytes, would it be poss= ible to use `u64::from_le_bytes` here? May be like: **untested**=20 let mut buffer =3D [0u8; 8]; proxmox_sys::linux::fill_with_random_data(&mut buffer)?; let rand =3D u64::from_le_bytes(buffer) as f64 / u64::MAX as f64; > + / (u64::MAX as f64); > + let renew =3D window_start + (((window_end - window_start) as f64) *= rand) as i64; > + // need max since the randomness could result in negative values > + Ok(Some(std::cmp::max(0, renew - now))) > +} > + > /// Check whether the current certificate expires within its renewal lea= d time. > /// > /// Returns `(expires_soon, lead_time_in_days)`; the lead time is return= ed so callers can produce > /// consistent user-facing messages without re-reading and re-parsing th= e certificate. nit: Since the return type is now updated, I think we should update this do= c comment as well.=20 > -pub fn check_renewal_needed() -> Result<(bool, i64), Error> { > - let cert =3D pem_to_cert_info(get_certificate_pem()?.as_bytes())?; > - let lead =3D cert_renew_lead_time(&cert); > - let expires_soon =3D cert > - .is_expired_after_epoch(proxmox_time::epoch_i64() + lead) > - .map_err(|err| format_err!("Failed to check certificate expirati= on date: {}", err))?; > - Ok((expires_soon, lead / SECONDS_PER_DAY)) > +async fn check_renewal_needed( > + acme_config: &AcmeConfig, > + cert_info: &cert::CertInfo, > +) -> Result { > + let lead_ari =3D cert_renew_lead_time_ari(acme_config, cert_info).aw= ait?; > + if let Some(lead_ari) =3D lead_ari { > + // rfc9773 section 4.2 tells us to renew if the chosen renewal t= ime would be before the next check > + let expires_soon =3D lead_ari < SECONDS_PER_DAY; > + let ts =3D proxmox_time::TimeSpan::from(std::time::Duration::new= (lead_ari as u64, 0)); > + info!("Certificate is scheduled for renewal in {ts:.0} by ARI"); > + Ok(expires_soon) > + } else { > + let lead =3D cert_renew_lead_time(&cert_info); > + let expires_soon =3D cert_info > + .is_expired_after_epoch(proxmox_time::epoch_i64() + lead) > + .map_err(|err| format_err!("Failed to check certificate expi= ration date: {}", err))?; > + let ts =3D proxmox_time::TimeSpan::from(std::time::Duration::new= (lead as u64, 0)); > + info!("Certificate renewal lead time is {ts:.0}"); > + Ok(expires_soon) > + } > } > [snip] > -- > 2.47.3