[PATCH proxmox{,-backup,-datacenter-manager} 0/4] fix #6173: acme: use the configured HTTP proxy

[PATCH proxmox{,-backup,-datacenter-manager} 0/4] fix #6173: acme: use the configured HTTP proxy

[Samuel Rufinatscha]

ACME requests in PBS and PDM ignore the node's HTTP proxy setting and
fail when the CA is reachable only through a proxy [0].

Add optional proxy support to the shared ACME client and pass the node
proxy configuration to each operation that contacts the CA.

Testing:

Reproduced the issue on PBS 4.2 using Pebble [1] as the ACME server
and Tinyproxy [2] as the HTTP proxy configured in PBS. Added a firewall
rule to block direct access to Pebble while allowing connections through
Tinyproxy. Operations succeeded through the configured proxy.

Tested for:

(1) HTTP-01 and DNS-01, including proxies requiring
authentication.
(2) Automatic renewal using a short-lived Pebble certificate.
(3) Changed the HTTP proxy setting to a second Tinyproxy instance
without restarting the PBS services. The next ACME operations used
the new proxy.
(4) PDM was built against the shared libraries (only).

Maintainer notes:

- proxmox-acme-api has breaking changes because of the extra
Option<ProxyConfig> argument.
- proxmox-acme-api adds the proxmox-http dependency to the impl feature

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=6173
[1] https://github.com/letsencrypt/pebble
[2] https://tinyproxy.github.io/

proxmox:

Samuel Rufinatscha (2):
  acme: async_client: support HTTP proxies
  acme-api: support HTTP proxies

 proxmox-acme-api/Cargo.toml                 |  2 ++
 proxmox-acme-api/src/account_api_impl.rs    | 27 +++++++++++++++------
 proxmox-acme-api/src/account_config.rs      |  5 ++--
 proxmox-acme-api/src/certificate_helpers.rs | 12 ++++++---
 proxmox-acme/src/async_client.rs            |  9 +++++--
 5 files changed, 41 insertions(+), 14 deletions(-)


proxmox-backup:

Samuel Rufinatscha (1):
  fix #6173: acme: use the configured HTTP proxy

 src/api2/config/acme.rs                | 10 +++++++---
 src/api2/node/certificates.rs          |  8 ++++++--
 src/bin/proxmox_backup_manager/acme.rs |  4 +++-
 3 files changed, 16 insertions(+), 6 deletions(-)


proxmox-datacenter-manager:

Samuel Rufinatscha (1):
  fix #6173: acme: use the configured HTTP proxy

 cli/admin/src/acme.rs                |  4 +++-
 server/src/api/config/acme.rs        | 26 ++++++++++++++++++++------
 server/src/api/nodes/certificates.rs |  9 +++++++--
 3 files changed, 30 insertions(+), 9 deletions(-)


Summary over all repositories:
  11 files changed, 87 insertions(+), 29 deletions(-)

-- 
Generated by git-murpp 0.8.1

[PATCH proxmox 1/2] acme: async_client: support HTTP proxies

[Samuel Rufinatscha]

Add a constructor that passes an optional proxy configuration to the
HTTP transport.

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
 proxmox-acme/src/async_client.rs | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/proxmox-acme/src/async_client.rs b/proxmox-acme/src/async_client.rs
index bba92023..6df2737d 100644
--- a/proxmox-acme/src/async_client.rs
+++ b/proxmox-acme/src/async_client.rs
@@ -6,7 +6,7 @@ use http_body_util::BodyExt;
 use hyper::Request;
 use serde::{Deserialize, Serialize};
 
-use proxmox_http::{Body, client::Client};
+use proxmox_http::{Body, ProxyConfig, client::Client};
 
 use crate::Request as AcmeRequest;
 use crate::account::AccountCreator;
@@ -25,11 +25,16 @@ pub struct AcmeClient {
 impl AcmeClient {
     /// Create a new ACME client for a given ACME directory URL.
     pub fn new(directory_url: String) -> Self {
+        Self::with_proxy(directory_url, None)
+    }
+
+    /// Create a new ACME client with an optional HTTP proxy.
+    pub fn with_proxy(directory_url: String, proxy_config: Option<ProxyConfig>) -> Self {
         const USER_AGENT_STRING: &str = "proxmox-acme-client/1.0";
         const TCP_KEEPALIVE_TIME: u32 = 120;
 
         let options = proxmox_http::HttpOptions {
-            proxy_config: None, // fixme???
+            proxy_config,
             user_agent: Some(USER_AGENT_STRING.to_string()),
             tcp_keepalive: Some(TCP_KEEPALIVE_TIME),
         };
-- 
2.47.3

[PATCH proxmox 2/2] acme-api: support HTTP proxies

[Samuel Rufinatscha]

Accept an optional proxy configuration for operations that contact
the ACME server and pass it to each new client.

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
 proxmox-acme-api/Cargo.toml                 |  2 ++
 proxmox-acme-api/src/account_api_impl.rs    | 27 +++++++++++++++------
 proxmox-acme-api/src/account_config.rs      |  5 ++--
 proxmox-acme-api/src/certificate_helpers.rs | 12 ++++++---
 4 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/proxmox-acme-api/Cargo.toml b/proxmox-acme-api/Cargo.toml
index 4bb1720b..9229d4ea 100644
--- a/proxmox-acme-api/Cargo.toml
+++ b/proxmox-acme-api/Cargo.toml
@@ -30,6 +30,7 @@ openssl = { workspace = true, optional = true }
 proxmox-acme = { workspace = true, features = ["api-types"] }
 proxmox-base64 = { workspace = true, optional = true }
 proxmox-config-digest = { workspace = true, optional = true }
+proxmox-http = { workspace = true, optional = true }
 proxmox-log = { workspace = true, optional = true }
 proxmox-product-config = { workspace = true, optional = true }
 proxmox-rest-server = { workspace = true, optional = true }
@@ -57,6 +58,7 @@ impl = [
 
     "dep:proxmox-base64",
     "dep:proxmox-config-digest",
+    "dep:proxmox-http",
     "dep:proxmox-log",
     "dep:proxmox-product-config",
     "dep:proxmox-rest-server",
diff --git a/proxmox-acme-api/src/account_api_impl.rs b/proxmox-acme-api/src/account_api_impl.rs
index ef195908..d42a475f 100644
--- a/proxmox-acme-api/src/account_api_impl.rs
+++ b/proxmox-acme-api/src/account_api_impl.rs
@@ -7,6 +7,7 @@ use serde_json::json;
 
 use proxmox_acme::async_client::AcmeClient;
 use proxmox_acme::types::AccountData as AcmeAccountData;
+use proxmox_http::ProxyConfig;
 use proxmox_log::warn;
 
 use crate::account_config::AccountData;
@@ -41,9 +42,12 @@ pub async fn get_account(account_name: AcmeAccountName) -> Result<AccountInfo, E
     })
 }
 
-pub async fn get_tos(directory: Option<String>) -> Result<Option<String>, Error> {
+pub async fn get_tos(
+    directory: Option<String>,
+    proxy_config: Option<ProxyConfig>,
+) -> Result<Option<String>, Error> {
     let directory = directory.unwrap_or_else(|| DEFAULT_ACME_DIRECTORY_ENTRY.url.to_string());
-    Ok(AcmeClient::new(directory)
+    Ok(AcmeClient::with_proxy(directory, proxy_config)
         .terms_of_service_url()
         .await?
         .map(str::to_owned))
@@ -55,11 +59,12 @@ pub async fn register_account(
     tos_url: Option<String>,
     directory_url: Option<String>,
     eab_creds: Option<(String, String)>,
+    proxy_config: Option<ProxyConfig>,
 ) -> Result<String, Error> {
     let directory_url =
         directory_url.unwrap_or_else(|| DEFAULT_ACME_DIRECTORY_ENTRY.url.to_string());
 
-    let mut client = AcmeClient::new(directory_url.clone());
+    let mut client = AcmeClient::with_proxy(directory_url.clone(), proxy_config);
 
     let contact = account_contact_from_string(&contact);
     let account = client
@@ -73,9 +78,13 @@ pub async fn register_account(
     Ok(account.location)
 }
 
-pub async fn deactivate_account(name: &AcmeAccountName, force: bool) -> Result<(), Error> {
+pub async fn deactivate_account(
+    name: &AcmeAccountName,
+    force: bool,
+    proxy_config: Option<ProxyConfig>,
+) -> Result<(), Error> {
     let mut account_data = super::account_config::load_account_config(name).await?;
-    let mut client = account_data.client();
+    let mut client = account_data.client(proxy_config);
 
     match client
         .update_account(&json!({"status": "deactivated"}))
@@ -99,9 +108,13 @@ pub async fn deactivate_account(name: &AcmeAccountName, force: bool) -> Result<(
     Ok(())
 }
 
-pub async fn update_account(name: &AcmeAccountName, contact: Option<String>) -> Result<(), Error> {
+pub async fn update_account(
+    name: &AcmeAccountName,
+    contact: Option<String>,
+    proxy_config: Option<ProxyConfig>,
+) -> Result<(), Error> {
     let mut account_data = super::account_config::load_account_config(name).await?;
-    let mut client = account_data.client();
+    let mut client = account_data.client(proxy_config);
 
     let data = match contact {
         Some(contact) => json!({
diff --git a/proxmox-acme-api/src/account_config.rs b/proxmox-acme-api/src/account_config.rs
index ce128c45..387f002c 100644
--- a/proxmox-acme-api/src/account_config.rs
+++ b/proxmox-acme-api/src/account_config.rs
@@ -8,6 +8,7 @@ use std::path::{Path, PathBuf};
 use anyhow::{Error, bail, format_err};
 use serde::{Deserialize, Serialize};
 
+use proxmox_http::ProxyConfig;
 use proxmox_product_config::replace_secret_config;
 use proxmox_sys::error::SysError;
 
@@ -68,8 +69,8 @@ impl AccountData {
         }
     }
 
-    pub fn client(&self) -> AcmeClient {
-        let mut client = AcmeClient::new(self.directory_url.clone());
+    pub fn client(&self, proxy_config: Option<ProxyConfig>) -> AcmeClient {
+        let mut client = AcmeClient::with_proxy(self.directory_url.clone(), proxy_config);
         client.set_account(Account {
             location: self.location.clone(),
             private_key: self.key.clone(),
diff --git a/proxmox-acme-api/src/certificate_helpers.rs b/proxmox-acme-api/src/certificate_helpers.rs
index 323f4b4a..d43e8f62 100644
--- a/proxmox-acme-api/src/certificate_helpers.rs
+++ b/proxmox-acme-api/src/certificate_helpers.rs
@@ -10,6 +10,7 @@ use openssl::rsa::Rsa;
 use openssl::x509::{X509, X509Builder};
 
 use proxmox_acme::async_client::AcmeClient;
+use proxmox_http::ProxyConfig;
 use proxmox_log::{info, warn};
 use proxmox_rest_server::WorkerTask;
 
@@ -18,10 +19,14 @@ use crate::types::{AcmeConfig, AcmeDomain};
 
 const ACME_POLL_TIMEOUT: Duration = Duration::from_secs(5 * 60);
 
-pub async fn revoke_certificate(acme_config: &AcmeConfig, certificate: &[u8]) -> Result<(), Error> {
+pub async fn revoke_certificate(
+    acme_config: &AcmeConfig,
+    certificate: &[u8],
+    proxy_config: Option<ProxyConfig>,
+) -> Result<(), Error> {
     let mut acme = super::account_config::load_account_config(&acme_config.account)
         .await?
-        .client();
+        .client(proxy_config);
 
     acme.revoke_certificate(certificate, None).await?;
 
@@ -37,6 +42,7 @@ pub async fn order_certificate(
     worker: Arc<WorkerTask>,
     acme_config: &AcmeConfig,
     domains: &[AcmeDomain],
+    proxy_config: Option<ProxyConfig>,
 ) -> Result<Option<OrderedCertificate>, Error> {
     use proxmox_acme::authorization::Status;
     use proxmox_acme::order::Identifier;
@@ -55,7 +61,7 @@ pub async fn order_certificate(
 
     let mut acme = super::account_config::load_account_config(&acme_config.account)
         .await?
-        .client();
+        .client(proxy_config);
 
     let (plugins, _) = super::plugin_config::plugin_config()?;
 
-- 
2.47.3

[PATCH proxmox-backup 1/1] fix #6173: acme: use the configured HTTP proxy

[Samuel Rufinatscha]

ACME requests ignore the node's HTTP proxy setting and fail when the
CA is reachable only through a proxy.

Pass the node proxy configuration to each ACME operation that contacts
the CA.

Link: https://bugzilla.proxmox.com/show_bug.cgi?id=6173
Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
 src/api2/config/acme.rs                | 10 +++++++---
 src/api2/node/certificates.rs          |  8 ++++++--
 src/bin/proxmox_backup_manager/acme.rs |  4 +++-
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/api2/config/acme.rs b/src/api2/config/acme.rs
index 16f24ea55..20f73e70a 100644
--- a/src/api2/config/acme.rs
+++ b/src/api2/config/acme.rs
@@ -170,6 +170,7 @@ fn register_account(
                 tos_url,
                 Some(directory),
                 eab_kid.zip(eab_hmac_key),
+                pbs_config::node::node_http_proxy_config()?,
             )
             .await?;
 
@@ -210,7 +211,8 @@ pub fn update_account(
         auth_id.to_string(),
         true,
         move |_worker| async move {
-            proxmox_acme_api::update_account(&name, contact).await?;
+            let proxy_config = pbs_config::node::node_http_proxy_config()?;
+            proxmox_acme_api::update_account(&name, contact, proxy_config).await?;
 
             Ok(())
         },
@@ -248,7 +250,8 @@ pub fn deactivate_account(
         auth_id.to_string(),
         true,
         move |_worker| async move {
-            proxmox_acme_api::deactivate_account(&name, force).await?;
+            let proxy_config = pbs_config::node::node_http_proxy_config()?;
+            proxmox_acme_api::deactivate_account(&name, force, proxy_config).await?;
 
             Ok(())
         },
@@ -276,7 +279,8 @@ pub fn deactivate_account(
 )]
 /// Get the Terms of Service URL for an ACME directory.
 async fn get_tos(directory: Option<String>) -> Result<Option<String>, Error> {
-    proxmox_acme_api::get_tos(directory).await
+    let proxy_config = pbs_config::node::node_http_proxy_config()?;
+    proxmox_acme_api::get_tos(directory, proxy_config).await
 }
 
 #[api(
diff --git a/src/api2/node/certificates.rs b/src/api2/node/certificates.rs
index 3df05b020..e93e84f93 100644
--- a/src/api2/node/certificates.rs
+++ b/src/api2/node/certificates.rs
@@ -368,6 +368,7 @@ fn spawn_certificate_worker(
     let auth_id = rpcenv.get_auth_id().unwrap();
 
     let acme_config = node_config.acme_config()?;
+    let proxy_config = node_config.http_proxy();
 
     let domains = node_config.acme_domains().try_fold(
         Vec::<AcmeDomain>::new(),
@@ -385,7 +386,8 @@ fn spawn_certificate_worker(
     WorkerTask::spawn(name, None, auth_id, true, move |worker| async move {
         let work = || async {
             if let Some(cert) =
-                proxmox_acme_api::order_certificate(worker, &acme_config, &domains).await?
+                proxmox_acme_api::order_certificate(worker, &acme_config, &domains, proxy_config)
+                    .await?
             {
                 crate::config::set_proxy_certificate(&cert.certificate, &cert.private_key_pem)?;
                 crate::server::reload_proxy_certificate().await?;
@@ -423,6 +425,7 @@ pub fn revoke_acme_cert(rpcenv: &mut dyn RpcEnvironment) -> Result<String, Error
     let auth_id = rpcenv.get_auth_id().unwrap();
 
     let acme_config = node_config.acme_config()?;
+    let proxy_config = node_config.http_proxy();
 
     WorkerTask::spawn(
         "acme-revoke-cert",
@@ -431,7 +434,8 @@ pub fn revoke_acme_cert(rpcenv: &mut dyn RpcEnvironment) -> Result<String, Error
         true,
         move |_worker| async move {
             info!("Revoking old certificate");
-            proxmox_acme_api::revoke_certificate(&acme_config, cert_pem.as_bytes()).await?;
+            proxmox_acme_api::revoke_certificate(&acme_config, cert_pem.as_bytes(), proxy_config)
+                .await?;
             info!("Deleting certificate and regenerating a self-signed one");
             delete_custom_certificate().await?;
             Ok(())
diff --git a/src/bin/proxmox_backup_manager/acme.rs b/src/bin/proxmox_backup_manager/acme.rs
index ed9e5868c..9ef4479b9 100644
--- a/src/bin/proxmox_backup_manager/acme.rs
+++ b/src/bin/proxmox_backup_manager/acme.rs
@@ -141,7 +141,8 @@ async fn register_account(
     };
 
     println!("Attempting to fetch Terms of Service from {directory_url:?}");
-    let mut client = AcmeClient::new(directory_url.clone());
+    let proxy_config = pbs_config::node::node_http_proxy_config()?;
+    let mut client = AcmeClient::with_proxy(directory_url.clone(), proxy_config.clone());
     let directory = client.directory().await?;
     let tos_agreed = if let Some(tos_url) = directory.terms_of_service_url() {
         println!("Terms of Service: {tos_url}");
@@ -196,6 +197,7 @@ async fn register_account(
         tos_agreed,
         Some(directory_url),
         eab_creds,
+        proxy_config,
     )
     .await?;
 
-- 
2.47.3

[PATCH proxmox-datacenter-manager 1/1] fix #6173: acme: use the configured HTTP proxy

[Samuel Rufinatscha]

ACME requests ignore the node's HTTP proxy setting and fail when the
CA is reachable only through a proxy.

Pass the node proxy configuration to each ACME operation that contacts
the CA.

Link: https://bugzilla.proxmox.com/show_bug.cgi?id=6173
Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
 cli/admin/src/acme.rs                |  4 +++-
 server/src/api/config/acme.rs        | 26 ++++++++++++++++++++------
 server/src/api/nodes/certificates.rs |  9 +++++++--
 3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/cli/admin/src/acme.rs b/cli/admin/src/acme.rs
index e61bb1ef..28a29769 100644
--- a/cli/admin/src/acme.rs
+++ b/cli/admin/src/acme.rs
@@ -146,7 +146,9 @@ async fn register_account(
     };
 
     println!("Attempting to fetch Terms of Service from {directory_url:?}");
-    let mut client = AcmeClient::new(directory_url.clone());
+    let (node_config, _) = pdm_config::node::config()?;
+    let proxy_config = pdm_config::node::get_http_proxy_config(&node_config);
+    let mut client = AcmeClient::with_proxy(directory_url.clone(), proxy_config);
     let directory = client.directory().await?;
     let tos_agreed = if let Some(tos_url) = directory.terms_of_service_url() {
         println!("Terms of Service: {tos_url}");
diff --git a/server/src/api/config/acme.rs b/server/src/api/config/acme.rs
index 838ad173..f852fa45 100644
--- a/server/src/api/config/acme.rs
+++ b/server/src/api/config/acme.rs
@@ -136,9 +136,17 @@ pub fn register_account(
         move |_worker| async move {
             proxmox_log::info!("Registering ACME account '{}'...", &name,);
 
-            let location =
-                proxmox_acme_api::register_account(&name, contact, tos_url, directory, eab_cread)
-                    .await?;
+            let (node_config, _) = pdm_config::node::config()?;
+            let proxy_config = pdm_config::node::get_http_proxy_config(&node_config);
+            let location = proxmox_acme_api::register_account(
+                &name,
+                contact,
+                tos_url,
+                directory,
+                eab_cread,
+                proxy_config,
+            )
+            .await?;
 
             proxmox_log::info!("Registration successful, account URL: {}", location);
 
@@ -198,7 +206,9 @@ pub fn update_account(
         move |_worker| async move {
             proxmox_log::info!("Update ACME account '{}'...", &name,);
 
-            proxmox_acme_api::update_account(&name, contact).await?;
+            let (node_config, _) = pdm_config::node::config()?;
+            let proxy_config = pdm_config::node::get_http_proxy_config(&node_config);
+            proxmox_acme_api::update_account(&name, contact, proxy_config).await?;
 
             proxmox_log::info!("Update ACME account '{}' successful", &name,);
 
@@ -243,7 +253,9 @@ pub fn deactivate_account(
         move |_worker| async move {
             proxmox_log::info!("Deactivate ACME account '{}'...", &name,);
 
-            proxmox_acme_api::deactivate_account(&name, force).await?;
+            let (node_config, _) = pdm_config::node::config()?;
+            let proxy_config = pdm_config::node::get_http_proxy_config(&node_config);
+            proxmox_acme_api::deactivate_account(&name, force, proxy_config).await?;
 
             proxmox_log::info!("Deactivate ACME account '{}' successful", &name,);
 
@@ -433,5 +445,7 @@ fn get_directories() -> Result<&'static [KnownAcmeDirectory], Error> {
 )]
 /// Get the Terms of Service URL for an ACME directory.
 async fn get_tos(directory: Option<String>) -> Result<Option<String>, Error> {
-    proxmox_acme_api::get_tos(directory).await
+    let (node_config, _) = pdm_config::node::config()?;
+    let proxy_config = pdm_config::node::get_http_proxy_config(&node_config);
+    proxmox_acme_api::get_tos(directory, proxy_config).await
 }
diff --git a/server/src/api/nodes/certificates.rs b/server/src/api/nodes/certificates.rs
index c3765415..753a8c0a 100644
--- a/server/src/api/nodes/certificates.rs
+++ b/server/src/api/nodes/certificates.rs
@@ -281,8 +281,11 @@ fn spawn_certificate_worker(
 
     WorkerTask::spawn(name, None, auth_id, true, move |worker| async move {
         let work = || async {
+            let (node_config, _) = pdm_config::node::config()?;
+            let proxy_config = pdm_config::node::get_http_proxy_config(&node_config);
             if let Some(cert) =
-                proxmox_acme_api::order_certificate(worker, &acme_config, &domains).await?
+                proxmox_acme_api::order_certificate(worker, &acme_config, &domains, proxy_config)
+                    .await?
             {
                 crate::auth::certs::set_api_certificate(&cert.certificate, &cert.private_key_pem)?;
                 crate::reload_api_certificate().await?;
@@ -336,7 +339,9 @@ pub fn revoke_acme_cert(rpcenv: &mut dyn RpcEnvironment) -> Result<String, Error
         true,
         move |_worker| async move {
             info!("Revoking old certificate");
-            proxmox_acme_api::revoke_certificate(&acme_config, &cert_pem).await?;
+            let (node_config, _) = pdm_config::node::config()?;
+            let proxy_config = pdm_config::node::get_http_proxy_config(&node_config);
+            proxmox_acme_api::revoke_certificate(&acme_config, &cert_pem, proxy_config).await?;
             info!("Deleting certificate and regenerating a self-signed one");
             delete_custom_certificate().await?;
             Ok(())
-- 
2.47.3