From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 2AC3A932A0 for ; Fri, 16 Sep 2022 08:58:03 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1DD1B209A4 for ; Fri, 16 Sep 2022 08:58:03 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 16 Sep 2022 08:58:01 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 7978F4446B for ; Fri, 16 Sep 2022 08:58:01 +0200 (CEST) Message-ID: <6d281db9-90e8-9164-6979-4b73b04cc627@proxmox.com> Date: Fri, 16 Sep 2022 08:58:00 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:105.0) Gecko/20100101 Thunderbird/105.0 Content-Language: en-GB To: Proxmox Backup Server development discussion , Stefan Hanreich References: <20220915140857.1041222-1-s.hanreich@proxmox.com> From: Thomas Lamprecht In-Reply-To: <20220915140857.1041222-1-s.hanreich@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.894 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -1.816 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH proxmox-backup] fix #4095: make http client read proxy config from envvars X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 16 Sep 2022 06:58:03 -0000 Am 15/09/2022 um 16:08 schrieb Stefan Hanreich: > In order to be able to use a proxy with the proxmox-backup-client, use > ProxyConfig for parsing proxy server config from the environment. Also > added a section in the documentation that describes how to configure the > environment if a proxy server should be used. Proxy config was more intended for the server, not the client side(s), and IMO proxy's never have any use outside of central surveillance of (tls) traffic, but well, we already got it and some user may want it, so can be fine, IMO close to a breaking change though, would at least require entries in all product's "noteable changes" section of their next point release. > > Signed-off-by: Stefan Hanreich > --- > docs/backup-client.rst | 7 +++++ > pbs-client/src/http_client.rs | 57 +++++++++++++++++++++++++++++++++++ > 2 files changed, 64 insertions(+) > > diff --git a/docs/backup-client.rst b/docs/backup-client.rst > index cc56d17e..45efc136 100644 > --- a/docs/backup-client.rst > +++ b/docs/backup-client.rst > @@ -69,6 +69,13 @@ Environment Variables > When set, this value is used to verify the server certificate (only used if > the system CA certificates cannot validate the certificate). > > +``ALL_PROXY`` > + When set, the client uses the specified HTTP proxy for all connections to the > + backup server. Currently only HTTP proxies are supported. Valid proxy > + configurations have the following format: > + `[http://][user:password@][:port]`. Default `port` is 1080, if not > + otherwise specified. Can you add a note here that we recommend using tunnels (e.g., wireguard) over HTTP proxies for shielding of hosts instead, maybe it helps to save some users from insecure proxy settings. > + > > .. Note:: Passwords must be valid UTF-8 and may not contain newlines. For your > convenience, Proxmox Backup Server only uses the first line as password, so > diff --git a/pbs-client/src/http_client.rs b/pbs-client/src/http_client.rs > index 4ef1350b..e41d94c0 100644 > --- a/pbs-client/src/http_client.rs > +++ b/pbs-client/src/http_client.rs > @@ -23,6 +23,7 @@ use proxmox_sys::linux::tty; > > use proxmox_async::broadcast_future::BroadcastFuture; > use proxmox_http::client::{HttpsConnector, RateLimiter}; > +use proxmox_http::ProxyConfig; > use proxmox_http::uri::{build_authority, json_object_to_query}; > > use pbs_api_types::percent_encoding::DEFAULT_ENCODE_SET; > @@ -389,6 +390,11 @@ impl HttpClient { > ))))); > } > > + let proxy_config = ProxyConfig::from_proxy_env()?; > + if let Some(config) = proxy_config { can we log that we're using a proxy to ease debugging of connection issues? > + https.set_proxy(config); > + } > + > let client = Client::builder() > //.http2_initial_stream_window_size( (1 << 31) - 2) > //.http2_initial_connection_window_size( (1 << 31) - 2) > @@ -1083,3 +1089,54 @@ impl H2Client { > Ok(request) > } > } > + > +#[cfg(test)] > +mod tests { > + use std::env; > + use super::*; > + > + #[test] > + fn test_proxy_config() { so, if I get this right this test requires the following in the build env: * needs a proxy to run on 8080 * needs PBS running * needs to build/tetst as root All three are a complete no-go, makes bootstrapping harder and is just a flaky test all together, I'd just drop it completely... Anyway, NAK until fixed. > + env::set_var("ALL_PROXY", "https://localhost:8080"); > + > + let mut http_client = HttpClient::new( > + "localhost", > + 8888, > + Authid::root_auth_id(), > + HttpClientOptions::new_non_interactive( > + String::from("test"), > + None > + ), > + ); > + > + assert!(http_client.is_err()); > + > + env::set_var("ALL_PROXY", "http://localhost:8080"); > + > + http_client = HttpClient::new( > + "localhost", > + 8888, > + Authid::root_auth_id(), > + HttpClientOptions::new_non_interactive( > + String::from("test"), > + None > + ), > + ); > + > + assert!(http_client.is_ok()); > + > + env::remove_var("ALL_PROXY"); > + > + http_client = HttpClient::new( > + "localhost", > + 8888, > + Authid::root_auth_id(), > + HttpClientOptions::new_non_interactive( > + String::from("test"), > + None > + ), > + ); > + > + assert!(http_client.is_ok()); > + } > +} > \ No newline at end of file ^- please check your editor to write sensible text out.