all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Manuel Federanko <m.federanko@proxmox.com>
To: Shan Shaji <s.shaji@proxmox.com>,
	pbs-devel@lists.proxmox.com, pdm-devel@lists.proxmox.com
Subject: Re: [PATCH proxmox-backup 6/7] acme: fix #6372 implement ARI renewal information fetching.
Date: Wed, 29 Jul 2026 11:08:26 +0200	[thread overview]
Message-ID: <3f6adb7e-3432-436d-a0d2-d2bac8959196@proxmox.com> (raw)
In-Reply-To: <DKA8QK61HAXK.33U6CVIMUD23T@proxmox.com>

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<Option<i64>, 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<bool, Error> {
>> +    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
> 




  reply	other threads:[~2026-07-29  9:09 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-25 14:13 [PATCH proxmox{,-backup,-datacenter-manager} 0/7] acme: fix #6372 implement basic ARI support Manuel Federanko
2026-06-25 14:13 ` [PATCH proxmox 1/7] acme: client: add methods to fetch renewal information Manuel Federanko
2026-07-24 11:16   ` Shan Shaji
2026-07-29  9:04     ` Manuel Federanko
2026-06-25 14:13 ` [PATCH proxmox 2/7] acme: add retry-after header to " Manuel Federanko
2026-06-25 14:13 ` [PATCH proxmox 3/7] acme: allow specifying the certificate that is replaced by an order Manuel Federanko
2026-06-25 14:13 ` [PATCH proxmox 4/7] acme: cert: add dedicated ari_id field to the certificate info Manuel Federanko
2026-06-25 14:13 ` [PATCH proxmox-backup 5/7] acme: add ari_id to cert info Manuel Federanko
2026-06-25 14:13 ` [PATCH proxmox-backup 6/7] acme: fix #6372 implement ARI renewal information fetching Manuel Federanko
2026-07-27 13:31   ` Shan Shaji
2026-07-29  9:07     ` Manuel Federanko
2026-07-30 15:09       ` Shan Shaji
2026-07-28 13:32   ` Shan Shaji
2026-07-29  9:08     ` Manuel Federanko [this message]
2026-06-25 14:13 ` [PATCH proxmox-datacenter-manager 7/7] acme: fix #6372 use ARI for renewal if available Manuel Federanko
2026-07-27 16:15 ` [PATCH proxmox{,-backup,-datacenter-manager} 0/7] acme: fix #6372 implement basic ARI support Shan Shaji
2026-07-29  9:01   ` Manuel Federanko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3f6adb7e-3432-436d-a0d2-d2bac8959196@proxmox.com \
    --to=m.federanko@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    --cc=s.shaji@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal