From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 840151FF0E5 for ; Wed, 29 Jul 2026 11:09:13 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 564CA214B3; Wed, 29 Jul 2026 11:09:13 +0200 (CEST) Message-ID: <3f6adb7e-3432-436d-a0d2-d2bac8959196@proxmox.com> Date: Wed, 29 Jul 2026 11:08:26 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox-backup 6/7] acme: fix #6372 implement ARI renewal information fetching. To: Shan Shaji , pbs-devel@lists.proxmox.com, pdm-devel@lists.proxmox.com References: <20260625141337.181684-1-m.federanko@proxmox.com> <20260625141337.181684-7-m.federanko@proxmox.com> Content-Language: en-US From: Manuel Federanko In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1785316068227 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.087 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: 6XQOECYK5L7KI4WW5DKYV3BJCIOYAWXP X-Message-ID-Hash: 6XQOECYK5L7KI4WW5DKYV3BJCIOYAWXP X-MailFrom: m.federanko@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 2026-07-28 3:32 PM, Shan Shaji wrote: > [snip] > >> +/// ARI renewal time if available >> +/// >> +/// Query the ARI endpoint for a suggested renewal window, draw a uniform 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 = proxmox_time::epoch_i64(); >> + if cert_info.is_expired_after_epoch(now)? { >> + return Ok(Some(0)); >> + } >> + let ari_id = &match cert_info.ari_id() { > > tiny nit: I think this `&` is not necessary here as deref coercion happens later. ack >> + Some(x) => x, >> + None => return Ok(None), >> + }; >> + let window = match proxmox_acme_api::get_renewal_info(acme_config, &ari_id).await? { >> + Some(x) => x, >> + None => return Ok(None), >> + }; >> + if let Some(reason) = window.data.explanation_url { >> + info!( >> + "Obtained renewal window, for information on this chosen window please visit {reason}" >> + ); >> + } >> + let window_start = proxmox_time::parse_rfc3339(&window.data.suggested_window.start)?; >> + let window_end = proxmox_time::parse_rfc3339(&window.data.suggested_window.end)?; >> + let rand = 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 possible to use > `u64::from_le_bytes` here? > > May be like: **untested** > > let mut buffer = [0u8; 8]; > proxmox_sys::linux::fill_with_random_data(&mut buffer)?; > let rand = u64::from_le_bytes(buffer) as f64 / u64::MAX as f64; yeah that makes sense >> + / (u64::MAX as f64); >> + let renew = 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 lead time. >> /// >> /// Returns `(expires_soon, lead_time_in_days)`; the lead time is returned so callers can produce >> /// consistent user-facing messages without re-reading and re-parsing the certificate. > > nit: Since the return type is now updated, I think we should update this doc comment as well. ack, thanks for pointing that out. >> -pub fn check_renewal_needed() -> Result<(bool, i64), Error> { >> - let cert = pem_to_cert_info(get_certificate_pem()?.as_bytes())?; >> - let lead = cert_renew_lead_time(&cert); >> - let expires_soon = cert >> - .is_expired_after_epoch(proxmox_time::epoch_i64() + lead) >> - .map_err(|err| format_err!("Failed to check certificate expiration 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 = cert_renew_lead_time_ari(acme_config, cert_info).await?; >> + if let Some(lead_ari) = lead_ari { >> + // rfc9773 section 4.2 tells us to renew if the chosen renewal time would be before the next check >> + let expires_soon = lead_ari < SECONDS_PER_DAY; >> + let ts = 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 = cert_renew_lead_time(&cert_info); >> + let expires_soon = cert_info >> + .is_expired_after_epoch(proxmox_time::epoch_i64() + lead) >> + .map_err(|err| format_err!("Failed to check certificate expiration date: {}", err))?; >> + let ts = 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 >