From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <pve-devel-bounces@lists.proxmox.com> Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id ED4E71FF15C for <inbox@lore.proxmox.com>; Wed, 26 Mar 2025 16:08:20 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3FDDE3AE12; Wed, 26 Mar 2025 16:08:16 +0100 (CET) References: <20250326105108.34911-1-f.weber@proxmox.com> User-agent: mu4e 1.10.8; emacs 30.1 From: Maximiliano Sandoval <m.sandoval@proxmox.com> To: Friedrich Weber <f.weber@proxmox.com> Date: Wed, 26 Mar 2025 16:07:08 +0100 In-reply-to: <20250326105108.34911-1-f.weber@proxmox.com> Message-ID: <s8oh63f4yoz.fsf@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.098 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH manager/storage 0/2] fix #3716: allow downloading iso/vztmpl/ova via https in proxied environments X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com> List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe> List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/> List-Post: <mailto:pve-devel@lists.proxmox.com> List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help> List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe> Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com> Cc: pve-devel@lists.proxmox.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com> Friedrich Weber <f.weber@proxmox.com> writes: > A user in enterprise support reported (and users also reported elsewhere [1] > [2]) that ISO downloads via https currently do not work in environments using a > proxy configured via the datacenter option `http_proxy`, if the connection to > the ISO repository needs to go via the proxy. Proxied ISO downloads via http > work. OVA and VZ template downloads are also affected. > > The reason is that > > - when querying the metadata via LWP, the proxy is only set for the `http` > scheme, not `https` > > - when spawning wget to download the ISO, only the `http_proxy` environment > variable is set, not `https_proxy`, so wget will not use a proxy for the https > connection > > Hence, neither operation uses the proxy for https. If the node cannot reach the > destination without the proxy, both operations time out. > > Fix this by > > - patch 1: setting the `http_proxy` from the datacenter config also for the > https scheme when querying the metadata via LWP > > - patch 2: passing the `https_proxy` environment variable to the wget command, > setting it to the `http_proxy` from the datacenter config > > Tested by running squid in a container, and setting up the firewall to drop > outgoing traffic from the PVE to everything but the proxy. Running > tcpconnect-bpfcc from bpfcc-tools helps for tracing the destination of the > http/https connections. > > Maximiliano and I discussed this and looked into earlier iterations on this. > > When a similar series was initially sent in 2021 [3], Thomas raised the concern > [4] that our proxy settings should be more fine-grained and allow the user to > differentiate between resources that should be proxied and that should not be > proxied. For example, ISO repositories may be external (and thus may only be > reachable via proxying) or internal (and thus may not be reachable via the > proxy). Same for ACME endpoints. One idea was to group http requests issued by > our stack into categories (such as `base`, `acme`, `template`) and allow the > proxy setting to only apply to certain categories. I agree that something like > this sounds like a useful feature, and one user also requested [5] something > along these lines. > > However, Maximiliano and I would argue this concern is orthogonal to the issue > fixed by this patch. If a user has configured `http_proxy`, having the ISO > download work via http and fail via https is inconsistent and thus confusing > (see also Dominik's post [6] from back then). It might even nudge users into > using http instead (which can still give the same integrity guarantees if they > retrieve the checksum via https and compare them, but this is easy to get > wrong). We'd propose we use the `http_proxy` for both https and https for now, > and can still look into the categorization feature #5420 later. > > Other places in our stack also use the `http_proxy` datacenter option for https > connections, e.g. the ones that use proxmox_http::HttpClient with ProxyConfig > such as with the notification system's webhook endpoint. > > One argument against this patch is that it breaks backwards compatibility: > Existing setups with `http_proxy` and an *internal* ISO repository from which > they download via https will break. If this is a concern, I'd suggest we wait > for PVE 9 to apply this. > > What do you think? > > FTR, there is some overlap with a patch series by Hannes Laimer [7] but that > one only concerned the `query-url-metadata` and the `apl_download` endpoints > (for downloading appliance templates), not the ISO download. > > [1] https://bugzilla.proxmox.com/show_bug.cgi?id=3716 > [2] https://bugzilla.proxmox.com/show_bug.cgi?id=5420#c2 > [3] https://lore.proxmox.com/pve-devel/20211109141359.990235-1-o.bektas@proxmox.com/ > [4] https://lore.proxmox.com/pve-devel/42391428-bd80-2d55-5cb6-7c8ecd97a3a8@proxmox.com/ > [5] https://bugzilla.proxmox.com/show_bug.cgi?id=5420#c0 > [6] https://lore.proxmox.com/pve-devel/a03631a3-fe78-7f6f-137d-7ee6fdf8f9ed@proxmox.com/ > [7] https://git.proxmox.com/?p=proxmox.git;a=blob;f=proxmox-notify/src/endpoints/webhook.rs;h=34dbac5488;hb=7abd2da759d#l266 > [8] https://lore.proxmox.com/pve-devel/20240308123535.1500-1-h.laimer@proxmox.com/ > > Co-authored-by: Maximiliano Sandoval <m.sandoval@proxmox.com> I am not sure whether the following trailers are redundant given the Co-authored-by trailer, nevertheless: Reviewed-by: Maximiliano Sandoval <m.sandoval@proxmox.com> Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com> _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel