Re: [PATCH proxmox-backup 6/7] acme: fix #6372 implement ARI renewal information fetching.

["Shan Shaji" <s.shaji@proxmox.com>]

[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.

> +        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;

> +        / (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. 

> -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