* [pbs-devel] [PATCH v2 proxmox 1/1] http: client: make https connector generic over resolver
2025-01-13 14:42 [pbs-devel] [PATCH v2 proxmox-backup 0/5] fix 4788: statically linked proxmox-backup-client Christian Ebner
@ 2025-01-13 14:42 ` Christian Ebner
2025-01-13 14:42 ` [pbs-devel] [PATCH v2 proxmox-backup 2/5] fix: 4788: Makefile: target for statically linked client binary Christian Ebner
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Christian Ebner @ 2025-01-13 14:42 UTC (permalink / raw)
To: pbs-devel
Allow to instantiate a `HttpsConnector` not using the default
`getaddrinfo` based `GaiResolver` for domain name resolution, but
rather a custom resolver implementing the required traits.
The usecase for this is to swap out the DNS resolver for the
statically linked proxmox-backup-client binary, where the glibc
dependency is problematic because of possible ABI incompatibility.
Adds tower-service as cargo workspace dependency and build dependency
to debian/control.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- no changes
proxmox-http/Cargo.toml | 1 +
proxmox-http/debian/control | 6 ++++--
proxmox-http/src/client/connector.rs | 17 ++++++++++++-----
proxmox-http/src/client/simple.rs | 3 ++-
4 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/proxmox-http/Cargo.toml b/proxmox-http/Cargo.toml
index c8c963f7..6bfb1413 100644
--- a/proxmox-http/Cargo.toml
+++ b/proxmox-http/Cargo.toml
@@ -22,6 +22,7 @@ openssl = { version = "0.10", optional = true }
serde_json = { workspace = true, optional = true }
tokio = { workspace = true, features = [], optional = true }
tokio-openssl = { workspace = true, optional = true }
+tower-service.workspace = true
ureq = { version = "2.4", features = ["native-certs", "native-tls"], optional = true, default-features = false }
url = { workspace = true, optional = true }
diff --git a/proxmox-http/debian/control b/proxmox-http/debian/control
index bd5ad8df..ead7a223 100644
--- a/proxmox-http/debian/control
+++ b/proxmox-http/debian/control
@@ -6,7 +6,8 @@ Build-Depends: debhelper-compat (= 13),
cargo:native <!nocheck>,
rustc:native (>= 1.80) <!nocheck>,
libstd-rust-dev <!nocheck>,
- librust-anyhow-1+default-dev <!nocheck>
+ librust-anyhow-1+default-dev <!nocheck>,
+ librust-tower-service-0.3+default-dev <!nocheck>
Maintainer: Proxmox Support Team <support@proxmox.com>
Standards-Version: 4.7.0
Vcs-Git: git://git.proxmox.com/git/proxmox.git
@@ -20,7 +21,8 @@ Architecture: any
Multi-Arch: same
Depends:
${misc:Depends},
- librust-anyhow-1+default-dev
+ librust-anyhow-1+default-dev,
+ librust-tower-service-0.3+default-dev
Suggests:
librust-proxmox-http+client-dev (= ${binary:Version}),
librust-proxmox-http+client-sync-dev (= ${binary:Version}),
diff --git a/proxmox-http/src/client/connector.rs b/proxmox-http/src/client/connector.rs
index 63b9d10c..c0435c60 100644
--- a/proxmox-http/src/client/connector.rs
+++ b/proxmox-http/src/client/connector.rs
@@ -6,6 +6,7 @@ use std::task::{Context, Poll};
use futures::*;
use http::Uri;
+use hyper::client::connect::dns::Name;
use hyper::client::HttpConnector;
use openssl::ssl::SslConnector;
use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
@@ -23,8 +24,8 @@ use crate::{RateLimitedStream, ShareableRateLimit};
type SharedRateLimit = Arc<dyn ShareableRateLimit>;
#[derive(Clone)]
-pub struct HttpsConnector {
- connector: HttpConnector,
+pub struct HttpsConnector<T> {
+ connector: HttpConnector<T>,
ssl_connector: Arc<SslConnector>,
proxy: Option<ProxyConfig>,
tcp_keepalive: u32,
@@ -32,9 +33,9 @@ pub struct HttpsConnector {
write_limiter: Option<SharedRateLimit>,
}
-impl HttpsConnector {
+impl<T> HttpsConnector<T> {
pub fn with_connector(
- mut connector: HttpConnector,
+ mut connector: HttpConnector<T>,
ssl_connector: SslConnector,
tcp_keepalive: u32,
) -> Self {
@@ -122,7 +123,13 @@ impl HttpsConnector {
}
}
-impl hyper::service::Service<Uri> for HttpsConnector {
+impl<T> hyper::service::Service<Uri> for HttpsConnector<T>
+where
+ T: tower_service::Service<Name> + Clone + Send + Sync + 'static,
+ T::Future: Send,
+ T::Error: Into<Box<(dyn std::error::Error + Send + Sync + 'static)>>,
+ T::Response: std::iter::Iterator<Item = std::net::SocketAddr>,
+{
type Response = MaybeTlsStream<RateLimitedStream<TcpStream>>;
type Error = Error;
#[allow(clippy::type_complexity)]
diff --git a/proxmox-http/src/client/simple.rs b/proxmox-http/src/client/simple.rs
index 062889ac..cb8bb777 100644
--- a/proxmox-http/src/client/simple.rs
+++ b/proxmox-http/src/client/simple.rs
@@ -8,6 +8,7 @@ use futures::*;
#[cfg(all(feature = "client-trait", feature = "proxmox-async"))]
use http::header::HeaderName;
use http::{HeaderValue, Request, Response};
+use hyper::client::connect::dns::GaiResolver;
use hyper::client::Client as HyperClient;
use hyper::client::HttpConnector;
use hyper::Body;
@@ -18,7 +19,7 @@ use crate::HttpOptions;
/// Asynchronous HTTP client implementation
pub struct Client {
- client: HyperClient<HttpsConnector, Body>,
+ client: HyperClient<HttpsConnector<GaiResolver>, Body>,
options: HttpOptions,
}
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [pbs-devel] [PATCH v2 proxmox-backup 2/5] fix: 4788: Makefile: target for statically linked client binary
2025-01-13 14:42 [pbs-devel] [PATCH v2 proxmox-backup 0/5] fix 4788: statically linked proxmox-backup-client Christian Ebner
2025-01-13 14:42 ` [pbs-devel] [PATCH v2 proxmox 1/1] http: client: make https connector generic over resolver Christian Ebner
@ 2025-01-13 14:42 ` Christian Ebner
2025-01-13 14:42 ` [pbs-devel] [PATCH v2 proxmox-backup 3/5] Makefile: switch path based on build mode and target Christian Ebner
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Christian Ebner @ 2025-01-13 14:42 UTC (permalink / raw)
To: pbs-devel
Adds the build target including workarounds to generate a statically
linked version of the proxmox-backup-client binary.
Suggested-by: Christoph Heiss <c.heiss@proxmox.com>
Originally-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- no changes
Makefile | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Makefile b/Makefile
index dfbaacab4..00e7f7401 100644
--- a/Makefile
+++ b/Makefile
@@ -227,3 +227,12 @@ upload: $(SERVER_DEB) $(CLIENT_DEB) $(RESTORE_DEB) $(DOC_DEB)
| ssh -X repoman@repo.proxmox.com upload --product pbs --dist $(UPLOAD_DIST)
tar cf - $(CLIENT_DEB) $(CLIENT_DBG_DEB) | ssh -X repoman@repo.proxmox.com upload --product "pve,pmg,pbs-client" --dist $(UPLOAD_DIST)
tar cf - $(RESTORE_DEB) $(RESTORE_DBG_DEB) | ssh -X repoman@repo.proxmox.com upload --product "pve" --dist $(UPLOAD_DIST)
+
+.PHONY: proxmox-backup-client-static
+proxmox-backup-client-static:
+ mkdir -p target/release/deps/ && \
+ echo '!<arch>' > target/release/deps/libsystemd.a # workaround for to greedy linkage and proxmox-systemd
+ RUSTFLAGS='-C target-feature=+crt-static -C strip=debuginfo' \
+ $(CARGO) build $(CARGO_BUILD_ARGS) \
+ --package proxmox-backup-client --bin proxmox-backup-client \
+ --target x86_64-unknown-linux-gnu
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [pbs-devel] [PATCH v2 proxmox-backup 3/5] Makefile: switch path based on build mode and target
2025-01-13 14:42 [pbs-devel] [PATCH v2 proxmox-backup 0/5] fix 4788: statically linked proxmox-backup-client Christian Ebner
2025-01-13 14:42 ` [pbs-devel] [PATCH v2 proxmox 1/1] http: client: make https connector generic over resolver Christian Ebner
2025-01-13 14:42 ` [pbs-devel] [PATCH v2 proxmox-backup 2/5] fix: 4788: Makefile: target for statically linked client binary Christian Ebner
@ 2025-01-13 14:42 ` Christian Ebner
2025-01-13 14:42 ` [pbs-devel] [PATCH v2 proxmox-backup 4/5] cargo: add stubs for `ipconfig` windows build dependency Christian Ebner
2025-01-13 14:42 ` [pbs-devel] [PATCH v2 proxmox-backup 5/5] client: http: Use custom resolver for statically linked binary Christian Ebner
4 siblings, 0 replies; 6+ messages in thread
From: Christian Ebner @ 2025-01-13 14:42 UTC (permalink / raw)
To: pbs-devel
Define variables for the compile path and target and use these
instead of hard-coding the path. Allows to easily switch between
debug and release mode for compilation.
Also, place the libsystemd stub into its own subdirectory for cleaner
separation from the compiled artifacts.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- use dedicated variables for compile dir and build target
- place libsystemd stub into dedicated directory for clear separation
Makefile | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index 00e7f7401..428ef40b9 100644
--- a/Makefile
+++ b/Makefile
@@ -36,11 +36,15 @@ SUBCRATES != cargo metadata --no-deps --format-version=1 \
| grep "$$PWD/" \
| sed -e "s!.*$$PWD/!!g" -e 's/\#.*$$//g' -e 's/)$$//g'
+STATIC_TARGET ?= x86_64-unknown-linux-gnu
+
ifeq ($(BUILD_MODE), release)
CARGO_BUILD_ARGS += --release
COMPILEDIR := target/$(DEB_HOST_RUST_TYPE)/release
+STATIC_COMPILEDIR := target/$(STATIC_TARGET)/release
else
COMPILEDIR := target/$(DEB_HOST_RUST_TYPE)/debug
+STATIC_COMPILEDIR := target/$(STATIC_TARGET)/debug
endif
ifeq ($(valgrind), yes)
@@ -230,9 +234,9 @@ upload: $(SERVER_DEB) $(CLIENT_DEB) $(RESTORE_DEB) $(DOC_DEB)
.PHONY: proxmox-backup-client-static
proxmox-backup-client-static:
- mkdir -p target/release/deps/ && \
- echo '!<arch>' > target/release/deps/libsystemd.a # workaround for to greedy linkage and proxmox-systemd
- RUSTFLAGS='-C target-feature=+crt-static -C strip=debuginfo' \
+ mkdir -p $(STATIC_COMPILEDIR)/deps-stubs/ && \
+ echo '!<arch>' > $(STATIC_COMPILEDIR)/deps-stubs/libsystemd.a # workaround for to greedy linkage and proxmox-systemd
+ RUSTFLAGS='-C target-feature=+crt-static -C strip=debuginfo -L $(STATIC_COMPILEDIR)/deps-stubs/' \
$(CARGO) build $(CARGO_BUILD_ARGS) \
--package proxmox-backup-client --bin proxmox-backup-client \
- --target x86_64-unknown-linux-gnu
+ --target $(STATIC_TARGET)
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [pbs-devel] [PATCH v2 proxmox-backup 4/5] cargo: add stubs for `ipconfig` windows build dependency
2025-01-13 14:42 [pbs-devel] [PATCH v2 proxmox-backup 0/5] fix 4788: statically linked proxmox-backup-client Christian Ebner
` (2 preceding siblings ...)
2025-01-13 14:42 ` [pbs-devel] [PATCH v2 proxmox-backup 3/5] Makefile: switch path based on build mode and target Christian Ebner
@ 2025-01-13 14:42 ` Christian Ebner
2025-01-13 14:42 ` [pbs-devel] [PATCH v2 proxmox-backup 5/5] client: http: Use custom resolver for statically linked binary Christian Ebner
4 siblings, 0 replies; 6+ messages in thread
From: Christian Ebner @ 2025-01-13 14:42 UTC (permalink / raw)
To: pbs-devel
`hickeroy-resolver` depends on ipconfig for name resolution on
Windows. Eventhough the build is not configured for windows and the
dependency should be disabled, cargo nevertheless expects the
dependency to be found in the registry.
As a workaround, add a dummy crate and point the registry to it,
in order to build without having to pull in the actually unused
dependency.
In preparation for adding `hickeroy-resolver` as alternative to
`getaddrinfo` based name resolution to be used for the statically
linked proxmox-backup-client.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- not present in previous version
Makefile | 2 +-
cargo-stubs/ipconfig/Cargo.toml | 6 ++++++
cargo-stubs/ipconfig/src/main.rs | 2 ++
3 files changed, 9 insertions(+), 1 deletion(-)
create mode 100644 cargo-stubs/ipconfig/Cargo.toml
create mode 100644 cargo-stubs/ipconfig/src/main.rs
diff --git a/Makefile b/Makefile
index 428ef40b9..4fe1201ca 100644
--- a/Makefile
+++ b/Makefile
@@ -98,7 +98,7 @@ build:
cp -a debian \
Cargo.toml src \
$(SUBCRATES) \
- docs etc examples tests www zsh-completions templates \
+ docs etc examples tests www zsh-completions templates cargo-stubs \
defines.mk Makefile \
./build/
rm -f build/Cargo.lock
diff --git a/cargo-stubs/ipconfig/Cargo.toml b/cargo-stubs/ipconfig/Cargo.toml
new file mode 100644
index 000000000..3bd3b0a88
--- /dev/null
+++ b/cargo-stubs/ipconfig/Cargo.toml
@@ -0,0 +1,6 @@
+[package]
+name = "ipconfig"
+version = "0.3.2"
+edition = "2021"
+
+[dependencies]
diff --git a/cargo-stubs/ipconfig/src/main.rs b/cargo-stubs/ipconfig/src/main.rs
new file mode 100644
index 000000000..f79c691f0
--- /dev/null
+++ b/cargo-stubs/ipconfig/src/main.rs
@@ -0,0 +1,2 @@
+fn main() {
+}
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [pbs-devel] [PATCH v2 proxmox-backup 5/5] client: http: Use custom resolver for statically linked binary
2025-01-13 14:42 [pbs-devel] [PATCH v2 proxmox-backup 0/5] fix 4788: statically linked proxmox-backup-client Christian Ebner
` (3 preceding siblings ...)
2025-01-13 14:42 ` [pbs-devel] [PATCH v2 proxmox-backup 4/5] cargo: add stubs for `ipconfig` windows build dependency Christian Ebner
@ 2025-01-13 14:42 ` Christian Ebner
4 siblings, 0 replies; 6+ messages in thread
From: Christian Ebner @ 2025-01-13 14:42 UTC (permalink / raw)
To: pbs-devel
The dependency on the `getaddrinfo` based `GaiResolver` used by
default for the `HttpClient` is not suitable for the statically
linked binary of the `proxmox-backup-client`, because of the
dependency on glibc NSS libraries, as described in glibc's FAQs [0].
As a workaround, conditionally compile the binary using the `hickory-dns`
resolver.
[0] https://sourceware.org/glibc/wiki/FAQ#Even_statically_linked_programs_need_some_shared_libraries_which_is_not_acceptable_for_me.__What_can_I_do.3F
Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- do not depend on Windows subdependencies by registering a dummy crate
Cargo.toml | 3 ++
pbs-client/Cargo.toml | 1 +
pbs-client/src/http_client.rs | 81 +++++++++++++++++++++++++++++++++--
3 files changed, 81 insertions(+), 4 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index 9354fb175..98b5b8193 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -161,6 +161,7 @@ walkdir = "2"
xdg = "2.2"
zstd = { version = "0.12", features = [ "bindgen" ] }
zstd-safe = "6.0"
+hickory-resolver = { git = "https://github.com/hickory-dns/hickory-dns", tag = "v0.24.1", default-features = false }
[dependencies]
anyhow.workspace = true
@@ -256,6 +257,8 @@ proxmox-rrd-api-types.workspace = true
# Local path overrides
# NOTE: You must run `cargo update` after changing this for it to take effect!
[patch.crates-io]
+hickory-proto = { git = "https://github.com/hickory-dns/hickory-dns", tag = "v0.24.1" }
+ipconfig = { path = "cargo-stubs/ipconfig", version = "0.3.2" }
#proxmox-apt = { path = "../proxmox/proxmox-apt" }
#proxmox-apt-api-types = { path = "../proxmox/proxmox-apt-api-types" }
diff --git a/pbs-client/Cargo.toml b/pbs-client/Cargo.toml
index 212f62f2a..faa0cffe4 100644
--- a/pbs-client/Cargo.toml
+++ b/pbs-client/Cargo.toml
@@ -28,6 +28,7 @@ tokio = { workspace = true, features = [ "fs", "signal" ] }
tokio-stream.workspace = true
tower-service.workspace = true
xdg.workspace = true
+hickory-resolver = { workspace = true, features = [ "system-config", "tokio-runtime" ] }
pathpatterns.workspace = true
diff --git a/pbs-client/src/http_client.rs b/pbs-client/src/http_client.rs
index e97b4e549..9f3419eb6 100644
--- a/pbs-client/src/http_client.rs
+++ b/pbs-client/src/http_client.rs
@@ -7,6 +7,8 @@ use futures::*;
use http::header::HeaderValue;
use http::Uri;
use http::{Request, Response};
+#[cfg(not(target_feature = "crt-static"))]
+use hyper::client::connect::dns::GaiResolver;
use hyper::client::{Client, HttpConnector};
use hyper::Body;
use openssl::{
@@ -33,6 +35,74 @@ use pbs_api_types::{Authid, RateLimitConfig, Userid};
use super::pipe_to_stream::PipeToSendStream;
use super::PROXMOX_BACKUP_TCP_KEEPALIVE_TIME;
+#[cfg(not(target_feature = "crt-static"))]
+type DnsResolver = GaiResolver;
+
+#[cfg(target_feature = "crt-static")]
+type DnsResolver = resolver::HickoryDnsResolver;
+
+#[cfg(target_feature = "crt-static")]
+mod resolver {
+ use std::net::SocketAddr;
+ use std::pin::Pin;
+ use std::sync::Arc;
+ use std::task::{Context, Poll};
+
+ use futures::Future;
+ use hickory_resolver::error::ResolveError;
+ use hickory_resolver::lookup_ip::LookupIpIntoIter;
+ use hickory_resolver::TokioAsyncResolver;
+ use hyper::client::connect::dns::Name;
+ use tower_service::Service;
+
+ pub(crate) struct SocketAddrIter {
+ inner: LookupIpIntoIter,
+ }
+
+ impl Iterator for SocketAddrIter {
+ type Item = SocketAddr;
+
+ fn next(&mut self) -> Option<Self::Item> {
+ self.inner.next().map(|ip_addr| SocketAddr::new(ip_addr, 0))
+ }
+ }
+
+ #[derive(Clone)]
+ pub(crate) struct HickoryDnsResolver {
+ inner: Arc<TokioAsyncResolver>,
+ }
+
+ impl HickoryDnsResolver {
+ pub(crate) fn new() -> Self {
+ Self {
+ inner: Arc::new(TokioAsyncResolver::tokio_from_system_conf().unwrap()),
+ }
+ }
+ }
+
+ impl Service<Name> for HickoryDnsResolver {
+ type Response = SocketAddrIter;
+ type Error = ResolveError;
+ type Future = Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>> + Send>>;
+
+ fn poll_ready(&mut self, _ctx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
+ Poll::Ready(Ok(()))
+ }
+
+ fn call(&mut self, name: Name) -> Self::Future {
+ let inner = self.inner.clone();
+ Box::pin(async move {
+ inner
+ .lookup_ip(name.as_str())
+ .await
+ .map(|r| SocketAddrIter {
+ inner: r.into_iter(),
+ })
+ })
+ }
+ }
+}
+
/// Timeout used for several HTTP operations that are expected to finish quickly but may block in
/// certain error conditions. Keep it generous, to avoid false-positive under high load.
const HTTP_TIMEOUT: Duration = Duration::from_secs(2 * 60);
@@ -134,7 +204,7 @@ impl Default for HttpClientOptions {
/// HTTP(S) API client
pub struct HttpClient {
- client: Client<HttpsConnector>,
+ client: Client<HttpsConnector<DnsResolver>>,
server: String,
port: u16,
fingerprint: Arc<Mutex<Option<String>>>,
@@ -365,7 +435,8 @@ impl HttpClient {
ssl_connector_builder.set_verify(openssl::ssl::SslVerifyMode::NONE);
}
- let mut httpc = HttpConnector::new();
+ let resolver = DnsResolver::new();
+ let mut httpc = HttpConnector::new_with_resolver(resolver);
httpc.set_nodelay(true); // important for h2 download performance!
httpc.enforce_http(false); // we want https...
@@ -526,7 +597,9 @@ impl HttpClient {
_options: options,
})
}
+}
+impl HttpClient {
/// Login
///
/// Login is done on demand, so this is only required if you need
@@ -815,7 +888,7 @@ impl HttpClient {
}
async fn credentials(
- client: Client<HttpsConnector>,
+ client: Client<HttpsConnector<DnsResolver>>,
server: String,
port: u16,
username: Userid,
@@ -860,7 +933,7 @@ impl HttpClient {
}
async fn api_request(
- client: Client<HttpsConnector>,
+ client: Client<HttpsConnector<DnsResolver>>,
req: Request<Body>,
) -> Result<Value, Error> {
Self::api_response(
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread