Gate hostselection=any to public destinations and a port allow-list (#188)

The `roundrobin`, `signed`, and `unsigned` host-selection modes route
requests against an operator-curated `Server.Hosts` list. The `any`
mode does not -- it forwards to whatever `?host=` value the request
carries, which makes the gateway usable as a generic TCP relay
against whatever the gateway can reach (loopback, RFC1918, link-local,
the cloud metadata service, arbitrary high-numbered ports on public
hosts).

Add a small destination policy applied only in `any` mode:

* Reject hosts that resolve to loopback, RFC1918, IPv6 ULA,
  link-local, unspecified, or multicast addresses. Operators can opt
  back in with `Server.AllowPrivateDestinations: true`.
* Restrict the destination port to `Server.AllowedDestinationPorts`
  (default {3389}).

The other host-selection modes are unaffected -- the operator already
curates their hosts list.

The DestinationPolicy zero value is the secure default, so direct
&Handler{} constructions in tests still get the expected behavior.
DNS names are resolved at validation time and every returned address
is checked.

CHANGELOG.md

@@ -0,0 +1,24 @@
+# Changelog
+
+All user-visible changes to rdpgw will be documented in this file.
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
+and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
+
+## [Unreleased]
+
+### Changed
+
+- `server.hostselection: any` now refuses destinations that resolve to
+  loopback, RFC1918, link-local, IPv6 ULA, unspecified, or multicast
+  addresses, and only forwards to ports in `Server.AllowedDestinationPorts`
+  (default `[3389]`). Operators that need the old behavior can opt back in
+  with `Server.AllowPrivateDestinations: true` and an extended port list.
+  See [UPGRADING.md](UPGRADING.md) for migration notes. The other
+  host-selection modes (`roundrobin`, `signed`, `unsigned`) already used
+  the operator-curated `Server.Hosts` list and are unaffected.
+
+### Added
+
+- `Server.AllowedDestinationPorts` (`[]int`, default `[3389]`).
+- `Server.AllowPrivateDestinations` (`bool`, default `false`).

README.md

@@ -166,8 +166,18 @@ Server:
  #  - roundrobin, which selects a random host from the list (default)
  #  - signed, a listed host specified in the signed query parameter
  #  - unsigned, a listed host specified in the query parameter
- #  - any, insecurely allow any host specified in the query parameter
+ #  - any, allow any host specified in the query parameter, gated by the
+ #    AllowedDestinationPorts / AllowPrivateDestinations options below.
  HostSelection: roundrobin 
+ # When HostSelection: any, only these TCP ports may be forwarded to.
+ # Empty defaults to [3389]. Ignored for the curated host modes
+ # (roundrobin, signed, unsigned).
+ # AllowedDestinationPorts:
+ #  - 3389
+ # When HostSelection: any, refuse to forward to loopback / RFC1918 /
+ # link-local / IPv6 ULA / unspecified / multicast destinations unless
+ # this is true. Default false. Has no effect on the curated host modes.
+ # AllowPrivateDestinations: false
  # a random strings of at least 32 characters to secure cookies on the client
  # make sure to share this across the different pods
  SessionKey: thisisasessionkeyreplacethisjetzt

UPGRADING.md

@@ -1,3 +1,35 @@
+# Upgrading
+
+## Unreleased
+
+### `hostselection: any` now refuses non-routable destinations and non-RDP ports by default
+
+Previously, when `server.hostselection: any` was set, rdpgw forwarded
+to whatever `?host=` value the request carried with no check on the
+target. The gateway would happily relay TCP traffic to loopback,
+RFC1918, link-local, or arbitrary high-numbered ports on public hosts.
+
+After upgrading, `any` mode rejects any destination that resolves to a
+loopback / RFC1918 / link-local / IPv6 ULA / unspecified / multicast
+address, and any port that is not in `AllowedDestinationPorts`. The
+default port allow-list is `[3389]`.
+
+If your deployment legitimately reaches private destinations or extra
+ports through `any` mode, opt back in:
+
+```yaml
+Server:
+  HostSelection: any
+  AllowedDestinationPorts:
+    - 3389
+    - 5985        # add what you actually need
+  AllowPrivateDestinations: true
+```
+
+The other host-selection modes (`roundrobin`, `signed`, `unsigned`)
+already use the operator-curated `Server.Hosts` allow-list and are
+unaffected by this change.
+
 # Upgrading from 1.X to 2.0
 
 In 2.0 the options for configuring client side RDP settings have been removed in favor of template file.

cmd/rdpgw/config/configuration.go

@@ -56,6 +56,14 @@ type ServerConfig struct {
 	Authentication       []string `koanf:"authentication"`
 	AuthSocket           string   `koanf:"authsocket"`
 	BasicAuthTimeout     int      `koanf:"basicauthtimeout"`
+	// AllowedDestinationPorts gates the TCP ports `hostselection: any` may
+	// forward to. Empty defaults to {3389}. Ignored for the curated host
+	// modes (roundrobin, signed, unsigned).
+	AllowedDestinationPorts []int `koanf:"alloweddestinationports"`
+	// AllowPrivateDestinations, when true, lets `hostselection: any`
+	// forward to loopback, RFC1918, link-local, and IPv6 ULA targets.
+	// Default false.
+	AllowPrivateDestinations bool `koanf:"allowprivatedestinations"`
 }
 
 type KerberosConfig struct {

cmd/rdpgw/main.go

@@ -114,10 +114,12 @@ func main() {
 			NoUsername:         conf.Client.NoUsername,
 			OverridableRdpKeys: conf.Client.RdpOverridableKeys,
 		},
-		GatewayAddress: url,
-		TemplateFile:   conf.Client.Defaults,
-		RdpSigningCert: conf.Client.SigningCert,
-		RdpSigningKey:  conf.Client.SigningKey,
+		GatewayAddress:           url,
+		TemplateFile:             conf.Client.Defaults,
+		RdpSigningCert:           conf.Client.SigningCert,
+		RdpSigningKey:            conf.Client.SigningKey,
+		AllowedDestinationPorts:  conf.Server.AllowedDestinationPorts,
+		AllowPrivateDestinations: conf.Server.AllowPrivateDestinations,
 	}
 
 	if conf.Caps.TokenAuth {

cmd/rdpgw/web/web.go

@@ -12,10 +12,12 @@ import (
 	"html/template"
 	"log"
 	rnd "math/rand"
+	"net"
 	"net/http"
 	"net/url"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"time"
 
@@ -42,6 +44,16 @@ type Config struct {
 	RdpSigningCert     string
 	RdpSigningKey      string
 	TemplatesPath      string
+	// AllowedDestinationPorts gates which TCP ports the `any` host-selection
+	// mode may forward to. Empty defaults to {3389}. Ignored for
+	// roundrobin / signed / unsigned where the operator already curates the
+	// hosts list.
+	AllowedDestinationPorts []int
+	// AllowPrivateDestinations, when true, lets `any` mode forward to
+	// loopback / RFC1918 / link-local / IPv6 ULA destinations. Default
+	// false: only globally-routable addresses are accepted. Operators that
+	// genuinely need to reach private destinations from `any` must opt in.
+	AllowPrivateDestinations bool
 }
 
 // WebConfig represents the web interface configuration
@@ -96,6 +108,88 @@ type Handler struct {
 	templatesPath      string
 	webConfig          *WebConfig
 	htmlTemplate       *template.Template
+	destPolicy         destinationPolicy
+}
+
+// destinationPolicy gates the host strings accepted in `any` host-selection
+// mode. With `signed` / `unsigned` / `roundrobin` the operator curates the
+// hosts list; with `any` the value comes from the request, so the gateway
+// must ensure it isn't being asked to act as a TCP relay against an
+// internal-only target.
+type destinationPolicy struct {
+	allowedPorts             map[int]struct{}
+	allowPrivateDestinations bool
+}
+
+func newDestinationPolicy(allowedPorts []int, allowPrivate bool) destinationPolicy {
+	if len(allowedPorts) == 0 {
+		allowedPorts = []int{3389}
+	}
+	set := make(map[int]struct{}, len(allowedPorts))
+	for _, p := range allowedPorts {
+		set[p] = struct{}{}
+	}
+	return destinationPolicy{
+		allowedPorts:             set,
+		allowPrivateDestinations: allowPrivate,
+	}
+}
+
+// allow validates a host:port (or bare host) string against the policy.
+// Returns nil if the destination is acceptable, an error otherwise. The
+// zero-value policy is treated as the secure default ({3389}, public-only).
+func (p destinationPolicy) allow(hostport string) error {
+	host, port, err := net.SplitHostPort(hostport)
+	if err != nil {
+		// no port present -- assume the protocol default
+		host = hostport
+		port = "3389"
+	}
+	portNum, err := strconv.Atoi(port)
+	if err != nil {
+		return fmt.Errorf("invalid port %q in %q", port, hostport)
+	}
+	allowedPorts := p.allowedPorts
+	if len(allowedPorts) == 0 {
+		allowedPorts = map[int]struct{}{3389: {}}
+	}
+	if _, ok := allowedPorts[portNum]; !ok {
+		return fmt.Errorf("port %d not in allow-list", portNum)
+	}
+
+	if p.allowPrivateDestinations {
+		return nil
+	}
+
+	if ip := net.ParseIP(host); ip != nil {
+		return checkPublicIP(host, ip)
+	}
+	addrs, err := net.LookupIP(host)
+	if err != nil {
+		return fmt.Errorf("cannot resolve %q: %s", host, err)
+	}
+	for _, ip := range addrs {
+		if err := checkPublicIP(host, ip); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func checkPublicIP(host string, ip net.IP) error {
+	switch {
+	case ip.IsLoopback():
+		return fmt.Errorf("destination %q (%s) is loopback", host, ip)
+	case ip.IsPrivate():
+		return fmt.Errorf("destination %q (%s) is in a private range", host, ip)
+	case ip.IsLinkLocalUnicast():
+		return fmt.Errorf("destination %q (%s) is link-local", host, ip)
+	case ip.IsUnspecified():
+		return fmt.Errorf("destination %q (%s) is unspecified", host, ip)
+	case ip.IsMulticast():
+		return fmt.Errorf("destination %q (%s) is multicast", host, ip)
+	}
+	return nil
 }
 
 func (c *Config) NewHandler() *Handler {
@@ -115,6 +209,7 @@ func (c *Config) NewHandler() *Handler {
 		rdpOpts:            c.RdpOpts,
 		rdpDefaults:        c.TemplateFile,
 		templatesPath:      c.TemplatesPath,
+		destPolicy:         newDestinationPolicy(c.AllowedDestinationPorts, c.AllowPrivateDestinations),
 	}
 
 	// set up RDP signer if config values are set
@@ -401,6 +496,10 @@ func (h *Handler) getHost(ctx context.Context, u *url.URL) (string, error) {
 		if !ok {
 			return "", errors.New("invalid query parameter")
 		}
+		if err := h.destPolicy.allow(hosts[0]); err != nil {
+			log.Printf("rejecting `any` destination %q: %s", hosts[0], err)
+			return "", fmt.Errorf("destination not allowed: %s", err)
+		}
 		return hosts[0], nil
 	default:
 		return h.selectRandomHost(), nil

cmd/rdpgw/web/web_interface_test.go

@@ -341,9 +341,12 @@ func TestHostSelectionIntegration(t *testing.T) {
 			name:          "any host allowed",
 			hostSelection: "any",
 			hosts:         []string{"host1.com"},
-			queryParams:   "?host=any-host.com",
-			expectHost:    "any-host.com",
-			expectError:   false,
+			// TEST-NET-3 literal stays in the policy's "publicly
+			// routable + RDP port" branch, which is the only thing
+			// `any` mode accepts by default.
+			queryParams: "?host=203.0.113.5:3389",
+			expectHost:  "203.0.113.5:3389",
+			expectError: false,
 		},
 	}
 

cmd/rdpgw/web/web_test.go

@@ -84,9 +84,10 @@ func TestGetHost(t *testing.T) {
 		t.Fatalf("host %s is not equal to input %s", host, hosts[0])
 	}
 
-	// check any
+	// check any -- TEST-NET-3 literal stays in the policy's "publicly
+	// routable" branch so this case still exercises the happy path.
 	c.HostSelection = "any"
-	test := "bla.bla.com"
+	test := "203.0.113.5:3389"
 	vals.Set("host", test)
 	u.RawQuery = vals.Encode()
 	h = c.NewHandler()
@@ -119,6 +120,77 @@ func TestGetHost(t *testing.T) {
 	}
 }
 
+// TestGetHostAnyRejectsSensitiveDestinations asserts that with
+// HostSelection="any" the gateway refuses hosts that resolve to addresses
+// it should not be reachable as: loopback, RFC1918, link-local, the cloud
+// metadata service, IPv6 loopback / ULA. Without this check an
+// authenticated user can use the gateway as a generic TCP relay against
+// any internal target the gateway can reach.
+func TestGetHostAnyRejectsSensitiveDestinations(t *testing.T) {
+	cases := []struct {
+		name string
+		host string
+	}{
+		{"loopback v4", "127.0.0.1:3389"},
+		{"loopback name", "localhost:3389"},
+		{"cloud metadata", "169.254.169.254:80"},
+		{"rfc1918 10/8", "10.0.0.5:3389"},
+		{"rfc1918 192.168/16", "192.168.1.10:3389"},
+		{"rfc1918 172.16/12", "172.16.5.10:3389"},
+		{"ipv6 loopback", "[::1]:3389"},
+		{"ipv6 ula", "[fc00::1]:3389"},
+		{"non-rdp port on public host", "203.0.113.5:6379"},
+	}
+
+	c := Config{
+		HostSelection: "any",
+		Hosts:         hosts,
+	}
+	h := c.NewHandler()
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			u := &url.URL{Host: "example.com"}
+			vals := u.Query()
+			vals.Set("host", tc.host)
+			u.RawQuery = vals.Encode()
+
+			got, err := h.getHost(context.Background(), u)
+			if err == nil {
+				t.Errorf("getHost(%q) returned %q with no error; sensitive destinations must be refused in 'any' mode", tc.host, got)
+			}
+		})
+	}
+}
+
+// TestGetHostAnyAllowsExplicitOptIn confirms that an operator can re-enable
+// access to private destinations and additional ports for `any` mode when
+// the deployment legitimately needs it.
+func TestGetHostAnyAllowsExplicitOptIn(t *testing.T) {
+	c := Config{
+		HostSelection:            "any",
+		Hosts:                    hosts,
+		AllowedDestinationPorts:  []int{3389, 5985},
+		AllowPrivateDestinations: true,
+	}
+	h := c.NewHandler()
+
+	for _, target := range []string{"10.0.0.1:3389", "127.0.0.1:5985"} {
+		u := &url.URL{Host: "example.com"}
+		vals := u.Query()
+		vals.Set("host", target)
+		u.RawQuery = vals.Encode()
+
+		got, err := h.getHost(context.Background(), u)
+		if err != nil {
+			t.Errorf("getHost(%q) rejected with %v; explicit opt-in must allow private and extra-port destinations", target, err)
+		}
+		if got != target {
+			t.Errorf("getHost(%q) = %q, want unchanged", target, got)
+		}
+	}
+}
+
 func TestHandler_HandleDownload(t *testing.T) {
 	req, err := http.NewRequest("GET", "/connect", nil)
 	if err != nil {