public inbox for pdm-devel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal