Fix redirect ports and linked-server WebSocket routing

ⓘ Follow up on #2794 by making `redirect_port` work even when `redirect_host` is not set. Keep existing host precedence and correctly handle proxy headers, IPv6 addresses, default ports, and invalid port values. ⓘ Fix linked-server by keeping the child URL recognizable to the parent, then generating the final WebSocket URL on the same host and port the browser is using. This keeps the connection working with the current Webmin session.
2026-07-26 19:30:33 +01:00 · 2026-07-15 20:38:41 +02:00
parent f220c99a10
commit 0eaf0ef80d
3 changed files with 269 additions and 16 deletions
--- a/servers/link.cgi
+++ b/servers/link.cgi
@@ -325,8 +325,16 @@ $miniserv{"websockets_$wspath"} =

 # Pass the fresh token directly. Re-reading miniserv.conf here can race the
 # config cache and return a stale token for the same ws-link path.
+# The route belongs to this parent request, so keep it on the browser-facing
+# parent authority instead of replacing it with redirect_host. This also
+# ensures the browser sends its session cookie to the websocket endpoint.
+my $browser_host = $ENV{'HTTP_HOST'};
+if ($miniserv{'trust_real_ip'} && $ENV{'HTTP_X_FORWARDED_HOST'}) {
+	$browser_host = (split(/\s*,\s*/,
+			      $ENV{'HTTP_X_FORWARDED_HOST'}))[0];
+	}
 my $rv = &get_miniserv_websocket_url(
-	undef, undef, $module_name, $wspath, $token);
+	undef, $browser_host, $module_name, $wspath, $token);
 $cache->{$cache_key} = $rv if ($cache);
 $rv =~ s#/#\\/#g if ($escaped_slashes);
 return $rv;
--- a/t/web-lib-funcs-websocket-url.t
+++ b/t/web-lib-funcs-websocket-url.t
@@ -0,0 +1,198 @@
+#!/usr/bin/perl
+# Tests for browser-visible WebSocket URL generation in web-lib-funcs.pl.
+
+use strict;
+use warnings;
+use Test::More;
+use File::Basename qw(dirname);
+use File::Spec;
+
+my $script = File::Spec->rel2abs(
+	File::Spec->catfile(dirname(__FILE__), '..', 'web-lib-funcs.pl'));
+require $script;
+
+my %test_miniserv;
+my $test_webprefix;
+{
+	no warnings qw(redefine once);
+	*main::get_miniserv_config = sub { %{$_[0]} = %test_miniserv; };
+	*main::get_webprefix = sub { return $test_webprefix; };
+}
+
+sub websocket_url {
+	my (%args) = @_;
+	%test_miniserv = %{$args{'config'} || { }};
+	$test_webprefix = $args{'webprefix'} || '';
+	my %saved_env = %ENV;
+	local %ENV = %saved_env;
+	delete @ENV{qw(HTTPS HTTP_HOST HTTP_X_FORWARDED_HOST
+			HTTP_X_FORWARDED_PROTO HTTP_FORWARDED
+			HTTP_WEBMIN_PATH)};
+	$ENV{'HTTPS'} = defined($args{'https'}) ? $args{'https'} : 'off';
+	$ENV{'HTTP_HOST'} = $args{'http_host'} || 'internal.example:10000';
+	if ($args{'env'}) {
+		foreach my $key (keys(%{$args{'env'}})) {
+			$ENV{$key} = $args{'env'}->{$key};
+			}
+		}
+	return main::get_miniserv_websocket_url(
+		$args{'port'} || 555,
+		$args{'host'},
+		$args{'module'} || 'xterm',
+		$args{'path'},
+		$args{'token'});
+}
+
+is(websocket_url(),
+	'ws://internal.example:10000/xterm/ws-555',
+	'direct HTTP URL is unchanged');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, redirect_host => 'public.example' }),
+	'wss://public.example/xterm/ws-555',
+	'redirect scheme and host are used');
+
+is(websocket_url(
+	config => { redirect_ssl => 1,
+		    redirect_host => 'rocky9-pro.webmin.dev' },
+	host => 'host.rocky9-pro.virtualmin.dev:10000',
+	path => '/servers/ws-link-1-563-linktoken',
+	token => 'linktoken'),
+	'wss://host.rocky9-pro.virtualmin.dev:10000/servers/'.
+	'ws-link-1-563-linktoken?token=linktoken',
+	'linked-server proxy URL preserves the browser-facing parent authority');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, redirect_host => 'public.example',
+		    redirect_port => 8443 }),
+	'wss://public.example:8443/xterm/ws-555',
+	'non-default redirect port is used with redirect host');
+
+is(websocket_url(
+	config => { redirect_ssl => 1,
+		    redirect_host => 'public.example:9443' }),
+	'wss://public.example:9443/xterm/ws-555',
+	'embedded redirect host port is preserved without port override');
+
+is(websocket_url(
+	config => { redirect_port => 8443 },
+	http_host => 'public.example'),
+	'ws://public.example:8443/xterm/ws-555',
+	'redirect port is used without redirect scheme or host');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, redirect_port => 8443 },
+	http_host => 'public.example:10000'),
+	'wss://public.example:8443/xterm/ws-555',
+	'redirect port replaces request host port');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, redirect_port => 8443,
+		    trust_real_ip => 1 },
+	env => { HTTP_X_FORWARDED_HOST => 'proxy.example:9443' }),
+	'wss://proxy.example:8443/xterm/ws-555',
+	'redirect port replaces trusted proxy host port');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, redirect_port => 443 },
+	http_host => 'public.example:10000'),
+	'wss://public.example/xterm/ws-555',
+	'default secure redirect port is omitted');
+
+is(websocket_url(
+	config => { redirect_ssl => 0, redirect_port => 80 },
+	https => 'on', http_host => 'public.example:10000'),
+	'ws://public.example/xterm/ws-555',
+	'default insecure redirect port is omitted');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, redirect_port => 8443,
+		    websocket_host => 'wss://socket.example:9443' }),
+	'wss://socket.example:9443/xterm/ws-555',
+	'explicit WebSocket host and port take precedence');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, redirect_port => 8443 },
+	host => 'caller.example:9555'),
+	'wss://caller.example:9555/xterm/ws-555',
+	'caller-provided host and port take precedence');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, redirect_port => 8443 },
+	http_host => '[2001:db8::1]:10000'),
+	'wss://[2001:db8::1]:8443/xterm/ws-555',
+	'redirect port replaces bracketed IPv6 request port');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, redirect_port => 8443 },
+	http_host => 'cafe:8080'),
+	'wss://cafe:8443/xterm/ws-555',
+	'hexadecimal-looking hostname is not mistaken for IPv6');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, redirect_port => 8443 },
+	http_host => 'beef'),
+	'wss://beef:8443/xterm/ws-555',
+	'single-label hexadecimal hostname is not bracketed');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, redirect_host => '2001:db8::2',
+		    redirect_port => 443 }),
+	'wss://[2001:db8::2]/xterm/ws-555',
+	'IPv6 redirect host is bracketed and default port omitted');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, redirect_host => 'public.example',
+		    redirect_prefix => '/webmin' },
+	webprefix => '/webmin'),
+	'wss://public.example/webmin/xterm/ws-555',
+	'documented subdirectory proxy prefix is appended once');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, trust_real_ip => 1 },
+	env => { HTTP_X_FORWARDED_PROTO => 'http',
+		 HTTP_X_FORWARDED_HOST => 'proxy.example' }),
+	'wss://proxy.example/xterm/ws-555',
+	'redirect scheme takes precedence over trusted proxy scheme');
+
+is(websocket_url(
+	config => { trust_real_ip => 1 },
+	env => { HTTP_X_FORWARDED_PROTO => 'https',
+		 HTTP_X_FORWARDED_HOST => 'proxy.example:9443' }),
+	'wss://proxy.example:9443/xterm/ws-555',
+	'trusted proxy host and port pass through without redirect overrides');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, redirect_port => 8443,
+		    trust_real_ip => 1 },
+	env => { HTTP_X_FORWARDED_HOST => 'public.example, edge.internal:10000' }),
+	'wss://public.example:8443/xterm/ws-555',
+	'comma-separated forwarded host uses the first entry');
+
+is(websocket_url(
+	config => { redirect_port => 'bogus' },
+	http_host => 'public.example:10000'),
+	'ws://public.example:10000/xterm/ws-555',
+	'non-numeric redirect port is ignored');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, redirect_host => 'public.example',
+		    redirect_port => 8443 },
+	http_host => 'child.example',
+	env => { HTTP_WEBMIN_PATH =>
+		 'https://parent.example/servers/link.cgi/1/xterm/index.cgi' }),
+	'ws://child.example/xterm/ws-555',
+	'linked-server response retains the child connection authority');
+
+is(websocket_url(
+	config => { redirect_ssl => 1, redirect_host => 'public.example',
+		    redirect_port => 8443,
+		    websocket_host => 'wss://socket.example:9443' },
+	host => 'caller.example:9555',
+	http_host => 'child.example',
+	env => { HTTP_WEBMIN_PATH =>
+		 'https://parent.example/servers/link.cgi/1/xterm/index.cgi' }),
+	'ws://child.example/xterm/ws-555',
+	'linked-server response ignores explicit public WebSocket hosts');
+
+done_testing();
--- a/web-lib-funcs.pl
+++ b/web-lib-funcs.pl
@@ -14763,10 +14763,15 @@ my %miniserv;
 my $webprefix = &get_webprefix();
 &get_miniserv_config(\%miniserv);
 my $trust_proxy = $miniserv{'trust_real_ip'};
+my $linked_server_request = $ENV{'HTTP_WEBMIN_PATH'};
+my $redirect_port_conf = $miniserv{'redirect_port'};
+$redirect_port_conf = undef if ($linked_server_request);
 my $default_ws_proto = lc($ENV{'HTTPS'}) eq 'on' ? 'wss' : 'ws';
 my $ws_proto;
-# Match the canonical redirect scheme when one has been configured.
-if ($miniserv{'redirect_ssl'} ne '') {
+# Match the canonical redirect scheme when one has been configured. Linked
+# server responses are rewritten by the parent, so they must retain the child
+# connection scheme and authority that the parent recognizes.
+if (!$linked_server_request && $miniserv{'redirect_ssl'} ne '') {
 	$ws_proto = $miniserv{'redirect_ssl'} ? 'wss' : 'ws';
 	}
 # Match the public browser scheme when Webmin is behind a trusted reverse
@@ -14785,6 +14790,38 @@ $ws_proto = lc($ws_proto);
 $ws_proto = 'wss' if ($ws_proto eq 'https');
 $ws_proto = 'ws' if ($ws_proto eq 'http');
 $ws_proto = $default_ws_proto if ($ws_proto ne 'wss' && $ws_proto ne 'ws');
+# Formats a host with the configured public port, replacing any port already
+# present on a request or proxy host. Explicit WebSocket and caller hosts
+# bypass this.
+my $format_hostport = sub {
+	my ($authority, $redirect_port) = @_;
+	return $authority if (!$authority);
+	# A non-numeric configured port is treated as unset, so a malformed
+	# value can never reach the returned URL.
+	$redirect_port = undef if (defined($redirect_port) &&
+				   $redirect_port !~ /^\d+$/);
+	# check_ip6address also accepts short hexadecimal strings. An unbracketed
+	# authority needs at least two colons to have the shape of an IPv6 address.
+	my $is_ip6 = $authority =~ /:.*:/ && &check_ip6address($authority);
+	if (!$redirect_port) {
+		$authority = "[".$authority."]"
+			if ($is_ip6);
+		return $authority;
+		}
+	if ($authority =~ /^\[([^\]]+)\](?::\d+)?$/) {
+		$authority = $1;
+		$is_ip6 = 1;
+		}
+	elsif (!$is_ip6) {
+		$authority =~ s/:\d+$//;
+		}
+	$authority = "[".$authority."]"
+		if ($is_ip6);
+	my $portstr = !($redirect_port == 80 && $ws_proto eq 'ws') &&
+		!($redirect_port == 443 && $ws_proto eq 'wss') ?
+		":".$redirect_port : "";
+	return $authority.$portstr;
+	};
 my $wspath = $path || "/$module/ws-".$port;
 # If the caller already generated the token, use it directly; otherwise fall
 # back to reading the token stored for the normal allocated websocket route.
@@ -14792,8 +14829,9 @@ if (!defined($wstoken) && $miniserv{'websockets_'.$wspath} &&
    $miniserv{'websockets_'.$wspath} =~ /\btoken=(\S+)/) {
 	$wstoken = $1;
 	}
-my $http_host_conf = &trim($miniserv{'websocket_host'} || $host);
-# Prefer the explicit websocket host when configured
+my $http_host_conf = $linked_server_request ? undef :
+			&trim($miniserv{'websocket_host'} || $host);
+# Prefer the explicit websocket host for direct browser requests
 if ($http_host_conf) {
 	if ($http_host_conf !~ /^wss?:\/\//) {
 		$http_host_conf = "$ws_proto://$http_host_conf";
@@ -14801,25 +14839,34 @@ if ($http_host_conf) {
 	$http_host_conf =~ s/[\/]+$//g;
 	}
 # Otherwise use the canonical redirect host and port when configured
-if (!$http_host_conf && $miniserv{'redirect_host'}) {
-	my $redirect_host = $miniserv{'redirect_host'};
-	my $redirect_port = $miniserv{'redirect_port'};
-	my $port = $redirect_port &&
-		!($redirect_port == 80 && $ws_proto eq 'ws') &&
-		!($redirect_port == 443 && $ws_proto eq 'wss') ?
-		":".$redirect_port : "";
-	$redirect_host = "[".$redirect_host."]" if (&check_ip6address($redirect_host));
-	$http_host_conf = "$ws_proto://$redirect_host$port";
+if (!$http_host_conf && !$linked_server_request &&
+    $miniserv{'redirect_host'}) {
+	my $redirect_host = &$format_hostport($miniserv{'redirect_host'},
+					       $redirect_port_conf);
+	$http_host_conf = "$ws_proto://$redirect_host";
 	}
 # Otherwise use trusted proxy headers when available
 if ($trust_proxy && !defined($http_host_conf)) {
-	my $forwarded_host = $ENV{'HTTP_X_FORWARDED_HOST'};
+	# A multi-proxy chain can send a comma-separated list; use the first
+	# entry, matching the X-Forwarded-Proto handling above.
+	my $forwarded_host = (split(/\s*,\s*/,
+				    $ENV{'HTTP_X_FORWARDED_HOST'}))[0];
 	if ($forwarded_host) {
+		$forwarded_host = &$format_hostport(
+			$forwarded_host, $redirect_port_conf)
+			if ($redirect_port_conf);
 		$http_host_conf = "$ws_proto://$forwarded_host";
 		$http_host_conf =~ s/[\/]+$//g;
 		}
 	}
-my $http_host = $http_host_conf || "$ws_proto://$ENV{'HTTP_HOST'}";
+my $http_host = $http_host_conf;
+if (!$http_host) {
+	my $request_host = $ENV{'HTTP_HOST'};
+	$request_host = &$format_hostport(
+		$request_host, $redirect_port_conf)
+		if ($redirect_port_conf);
+	$http_host = "$ws_proto://$request_host";
+	}
 $http_host .= $webprefix if ($webprefix);
 my $url = "$http_host$wspath";
 $url .= "?token=".&urlize($wstoken) if ($wstoken);