[pve-devel] [PATCH manager/storage 0/2] fix #3716: allow downloading iso/vztmpl/ova via https in proxied environments

* [pve-devel] [PATCH manager/storage 0/2] fix #3716: allow downloading iso/vztmpl/ova via https in proxied environments
@ 2025-03-26 10:51 Friedrich Weber
  2025-03-26 10:51 ` [pve-devel] [PATCH storage 1/2] fix #3716: api: download from url: use proxy option for https Friedrich Weber
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Friedrich Weber @ 2025-03-26 10:51 UTC (permalink / raw)
  To: pve-devel

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>

storage:

Friedrich Weber (1):
  fix #3716: api: download from url: use proxy option for https

 src/PVE/API2/Storage/Status.pm | 1 +
 1 file changed, 1 insertion(+)

manager:

Friedrich Weber (1):
  fix #3716: api: nodes: query metadata: use proxy option for https

 PVE/API2/Nodes.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Summary over all repositories:
  2 files changed, 2 insertions(+), 1 deletions(-)

-- 
Generated by git-murpp 0.7.3

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread