Re: [PATCH proxmox 1/7] acme: client: add methods to fetch renewal information.

[Manuel Federanko <m.federanko@proxmox.com>]

On 2026-07-24 1:16 PM, Shan Shaji wrote:
> On Thu Jun 25, 2026 at 4:13 PM CEST, Manuel Federanko wrote:
>> Add new structs representing the renewal information returned by the
>> server.
>>
>> Introduce a method on the client that fetches the renewal information
>> from the server. A helper method computes the certificate ID passed to
>> this method.
>>
>> Signed-off-by: Manuel Federanko <m.federanko@proxmox.com>
>> ---
>>  proxmox-acme-api/src/certificate_helpers.rs | 30 +++++++++++++++++++
>>  proxmox-acme-api/src/lib.rs                 |  5 +++-
>>  proxmox-acme/src/async_client.rs            | 31 +++++++++++++++++++
>>  proxmox-acme/src/directory.rs               |  8 +++++
>>  proxmox-acme/src/lib.rs                     |  3 ++
>>  proxmox-acme/src/renewal.rs                 | 33 +++++++++++++++++++++
>>  6 files changed, 109 insertions(+), 1 deletion(-)
>>  create mode 100644 proxmox-acme/src/renewal.rs
>>
>> diff --git a/proxmox-acme-api/src/certificate_helpers.rs b/proxmox-acme-api/src/certificate_helpers.rs
>> index 3921b18e..7dc06c2d 100644
>> --- a/proxmox-acme-api/src/certificate_helpers.rs
>> +++ b/proxmox-acme-api/src/certificate_helpers.rs
>> @@ -31,6 +31,36 @@ pub struct OrderedCertificate {
>>      pub private_key_pem: Vec<u8>,
>>  }
>>  
>> +pub fn compute_ari_certificate_id(cert: &openssl::x509::X509) -> Option<String> {
>> +    let authority_key_identifier = match cert.authority_key_id() {
>> +        Some(v) => v.as_slice(),
>> +        None => return None,
>> +    };
>> +    let mut serial_number = (match cert.serial_number().to_bn() {
>> +        Ok(v) => v,
>> +        Err(_) => return None,
>> +    })
>> +    .to_vec();
>> +    if !serial_number.is_empty() && (serial_number[0] & 0x80) > 0 {
>> +        // check for negative numbers and prepend leading 0
>> +        serial_number.insert(0, 0);
>> +    }
>> +
>> +    let authority_key_identifier = proxmox_base64::url::encode_no_pad(authority_key_identifier);
>> +    let serial_number = proxmox_base64::url::encode_no_pad(serial_number);
>> +    Some(format!("{authority_key_identifier}.{serial_number}"))
>> +}
>> +
>> +pub async fn get_renewal_info(
>> +    acme_config: &AcmeConfig,
>> +    certificate_id: &str,
>> +) -> Result<Option<proxmox_acme::renewal::RenewalInformation>, Error> {
>> +    let mut acme = super::account_config::load_account_config(&acme_config.account)
>> +        .await?
>> +        .client();
>> +    acme.get_renewal_info(certificate_id).await
>> +}
>> +
>>  pub async fn order_certificate(
>>      worker: Arc<WorkerTask>,
>>      acme_config: &AcmeConfig,
>> diff --git a/proxmox-acme-api/src/lib.rs b/proxmox-acme-api/src/lib.rs
>> index c315d137..89a5e9a2 100644
>> --- a/proxmox-acme-api/src/lib.rs
>> +++ b/proxmox-acme-api/src/lib.rs
>> @@ -45,7 +45,10 @@ pub(crate) mod acme_plugin;
>>  #[cfg(feature = "impl")]
>>  mod certificate_helpers;
>>  #[cfg(feature = "impl")]
>> -pub use certificate_helpers::{create_self_signed_cert, order_certificate, revoke_certificate};
>> +pub use certificate_helpers::{
>> +    compute_ari_certificate_id, create_self_signed_cert, get_renewal_info, order_certificate,
>> +    revoke_certificate,
>> +};
>>  
>>  #[cfg(feature = "impl")]
>>  pub mod completion {
>> diff --git a/proxmox-acme/src/async_client.rs b/proxmox-acme/src/async_client.rs
>> index bba92023..6670bc24 100644
>> --- a/proxmox-acme/src/async_client.rs
>> +++ b/proxmox-acme/src/async_client.rs
>> @@ -335,6 +335,37 @@ impl AcmeClient {
>>          }
>>      }
>>  
>> +    /// Get the renewal information for a certificate
>> +    /// Returns None if the server does not support ARI
>> +    pub async fn get_renewal_info(
>> +        &mut self,
>> +        certificate_id: &str, // computed according to rfc9773 section 4.1
> 
> nit: Would it be better to add this inline comment over `compute_ari_certificate_id` helper?
>  
ack, though I would also leave a reference how this should be generated. The nicer
solution would be it's own type i guess.
>> +    ) -> Result<Option<crate::renewal::RenewalInformation>, anyhow::Error> {
>> +        let directory = self.directory().await?;
>> +        if directory.renewal_info_url().is_none() {
>> +            return Ok(None);
>> +        }
>> +        let url = format!(
>> +            "{}/{}",
>> +            directory.renewal_info_url().unwrap(),
>> +            certificate_id,
>> +        );
> 
> tiny nit: May be we could rewrite this to avoid calling is_none then calling unwrap?
> 
>           let Some(renewal_info_url) = directory.renewal_info_url() else {
>             return Ok(None)
>           };
> 
>           let url = format!("{renewal_info_url}/{certificate_id}")
ack, that would be cleaner.
>> +        let request = crate::request::Request {
>> +            url,
>> +            method: "GET",
>> +            content_type: crate::request::JSON_CONTENT_TYPE,
>> +            body: "".into(),
>> +            expected: &[crate::request::http_status::OK],
>> +        };
>> +        match Self::execute(&mut self.http_client, request, &mut self.nonce).await {
>> +            Ok(response) => {
>> +                let data: crate::renewal::RenewalInformationData = response.json()?;
>> +                Ok(Some(crate::renewal::RenewalInformation { data }))
>> +            }
>> +            Err(err) => Err(err.into()),
>> +        }
>> +    }
>> +
>>      fn need_account(account: &Option<Account>) -> Result<&Account, anyhow::Error> {
>>          account
>>              .as_ref()
>> diff --git a/proxmox-acme/src/directory.rs b/proxmox-acme/src/directory.rs
>> index b940901a..bbce4da5 100644
>> --- a/proxmox-acme/src/directory.rs
>> +++ b/proxmox-acme/src/directory.rs
>> @@ -38,6 +38,10 @@ pub struct DirectoryData {
>>      #[serde(skip_serializing_if = "Option::is_none")]
>>      pub key_change: Option<String>,
>>  
>> +    /// URL to get renewal information
>> +    #[serde(skip_serializing_if = "Option::is_none")]
>> +    pub renewal_info: Option<String>,
>> +
>>      /// Metadata object, for additional information which aren't directly part of the API
>>      /// itself, such as the terms of service.
>>      #[serde(skip_serializing_if = "Option::is_none")]
>> @@ -104,6 +108,10 @@ impl Directory {
>>          self.data.new_order.as_deref()
>>      }
>>  
>> +    pub(crate) fn renewal_info_url(&self) -> Option<&str> {
>> +        self.data.renewal_info.as_deref()
>> +    }
>> +
>>      /// Access to the in the Acme spec defined metadata structure.
>>      pub fn meta(&self) -> Option<&Meta> {
>>          self.data.meta.as_ref()
>> diff --git a/proxmox-acme/src/lib.rs b/proxmox-acme/src/lib.rs
>> index 6b774746..370d5ec0 100644
>> --- a/proxmox-acme/src/lib.rs
>> +++ b/proxmox-acme/src/lib.rs
>> @@ -43,6 +43,9 @@ pub mod error;
>>  #[cfg(feature = "impl")]
>>  pub mod order;
>>  
>> +#[cfg(feature = "impl")]
>> +pub mod renewal;
>> +
>>  #[cfg(feature = "impl")]
>>  pub mod util;
>>  
>> diff --git a/proxmox-acme/src/renewal.rs b/proxmox-acme/src/renewal.rs
>> new file mode 100644
>> index 00000000..eb4ff96a
>> --- /dev/null
>> +++ b/proxmox-acme/src/renewal.rs
>> @@ -0,0 +1,33 @@
>> +//! Acme renewal information
>> +use serde::{Deserialize, Serialize};
>> +
>> +/// The suggested renewal time window
>> +#[derive(Clone, Debug, Default, Deserialize, Serialize)]
>> +#[serde(rename_all = "camelCase")]
>> +pub struct SuggestedWindowData {
>> +    /// RFC3339 encoded time strings
>> +    pub start: String,
>> +    /// RFC3339 encoded time strings
>> +    pub end: String,
>> +}
>> +
>> +/// This contains the renewal information data returned by the ACME server
>> +#[derive(Clone, Debug, Default, Deserialize, Serialize)]
>> +#[serde(rename_all = "camelCase")]
>> +pub struct RenewalInformationData {
>> +    /// the suggested time window to renew the certificate
>> +    pub suggested_window: SuggestedWindowData,
>> +
>> +    /// explanatory URL why the windows is suggested
>> +    #[serde(skip_serializing_if = "Option::is_none")]
>> +    #[serde(rename = "explanatoryURL")]
>> +    pub explanation_url: Option<String>,
>> +}
>> +
>> +/// Renewal and retry information
>> +#[derive(Clone, Debug, Default, Deserialize, Serialize)]
>> +#[serde(rename_all = "kebab-case")]
>> +pub struct RenewalInformation {
>> +    /// the actual response of the acme server
>> +    pub data: RenewalInformationData,
>> +}
>