From c455a4ac31041e40bd277cc9a6956d206e7cb890 Mon Sep 17 00:00:00 2001 From: dmitri-netbird Date: Fri, 4 Sep 2026 10:00:35 +0200 Subject: [PATCH] [management] disallow weird ip addresses for direct upstream hosts (#7400) * disallow weird ip addresses for direct upstream hosts Signed-off-by: Dmitri Dolguikh * handle bracketed ipv6 addresses Signed-off-by: Dmitri Dolguikh * extend the check to subnet service targets Signed-off-by: Dmitri Dolguikh * fix spelling Signed-off-by: Dmitri Dolguikh * reject ipv6 addresses with zones Signed-off-by: Dmitri Dolguikh * catch host:port hostnames in services with subnet targets Signed-off-by: Dmitri Dolguikh * make linter happy Signed-off-by: Dmitri Dolguikh --------- Signed-off-by: Dmitri Dolguikh --- .../modules/reverseproxy/service/service.go | 43 +++++++++++++- .../reverseproxy/service/service_test.go | 58 +++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) diff --git a/management/internals/modules/reverseproxy/service/service.go b/management/internals/modules/reverseproxy/service/service.go index 1488003cb..ef92fbbf4 100644 --- a/management/internals/modules/reverseproxy/service/service.go +++ b/management/internals/modules/reverseproxy/service/service.go @@ -55,6 +55,8 @@ const ( SourceEphemeral = "ephemeral" ) +var ErrUnsupportedIPAddressUpstreamHost = errors.New("unsupported ip address for a direct upstream host") + type TargetOptions struct { SkipTLSVerify bool `json:"skip_tls_verify"` RequestTimeout time.Duration `json:"request_timeout,omitempty"` @@ -962,8 +964,8 @@ func (s *Service) validateHTTPTargets() error { return err } case TargetTypeSubnet: - if target.Host == "" { - return fmt.Errorf("target %d has empty host but target_type is %q", i, target.TargetType) + if err := validateSubnetTarget(i, target); err != nil { + return err } case TargetTypeCluster: if err := validateClusterTarget(i, target); err != nil { @@ -986,6 +988,34 @@ func (s *Service) validateHTTPTargets() error { return nil } +func validateSubnetTarget(idx int, target *Target) error { + host := strings.TrimSpace(target.Host) + if host == "" { + return fmt.Errorf("target %d has empty host but target_type is %q", idx, target.TargetType) + } + if strings.ContainsAny(host, " \t/") { + return fmt.Errorf("target %d: host %q contains invalid characters", idx, host) + } + if _, _, err := net.SplitHostPort(host); err == nil { + return fmt.Errorf("target %d: host %q must not include a port (set target.port instead)", idx, host) + } + noBrackets := strings.TrimSuffix(strings.TrimPrefix(host, "["), "]") + maybeip, err := netip.ParseAddr(noBrackets) + if err != nil { // not an ip + return nil //nolint:nilerr + } + if maybeip.Zone() != "" { + return fmt.Errorf("invalid direct upstream host ip %s %w", maybeip.String(), ErrUnsupportedIPAddressUpstreamHost) + } + if !target.Options.DirectUpstream { + return nil + } + if maybeip.IsLoopback() || maybeip.IsMulticast() || maybeip.IsLinkLocalUnicast() { + return fmt.Errorf("invalid direct upstream host ip %s %w", maybeip.String(), ErrUnsupportedIPAddressUpstreamHost) + } + return nil +} + // validateClusterTarget cluster targets should not have empty hosts and should have direct upstream enabled. func validateClusterTarget(idx int, target *Target) error { host := strings.TrimSpace(target.Host) @@ -1020,6 +1050,15 @@ func validateDirectUpstreamHost(idx int, target *Target) error { if _, _, err := net.SplitHostPort(host); err == nil { return fmt.Errorf("target %d: host %q must not include a port (set target.port instead)", idx, host) } + noBrackets := strings.TrimSuffix(strings.TrimPrefix(host, "["), "]") + maybeip, err := netip.ParseAddr(noBrackets) + if err != nil { // not an ip + return nil //nolint:nilerr + } + if maybeip.Zone() != "" || maybeip.IsLoopback() || maybeip.IsMulticast() || maybeip.IsLinkLocalUnicast() { + return fmt.Errorf("invalid direct upstream host ip %s %w", maybeip.String(), ErrUnsupportedIPAddressUpstreamHost) + } + return nil } diff --git a/management/internals/modules/reverseproxy/service/service_test.go b/management/internals/modules/reverseproxy/service/service_test.go index fc59968a2..84343bcdd 100644 --- a/management/internals/modules/reverseproxy/service/service_test.go +++ b/management/internals/modules/reverseproxy/service/service_test.go @@ -216,6 +216,64 @@ func TestValidateTargetOptions_CustomHeaders(t *testing.T) { }) } +func TestValidate_DirectUpstreamHost(t *testing.T) { + target := Target{TargetId: "id-1", TargetType: TargetTypePeer, Host: "10.0.0.1", Port: 80, Protocol: "http", Enabled: true, Options: TargetOptions{DirectUpstream: true}} + assert.ErrorIs(t, validateDirectUpstreamHost(0, targetWithHost(&target, "127.0.0.2")), ErrUnsupportedIPAddressUpstreamHost) + assert.NotNil(t, validateDirectUpstreamHost(0, targetWithHost(&target, "127.0.0.2:80")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateDirectUpstreamHost(0, targetWithHost(&target, "::1")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateDirectUpstreamHost(0, targetWithHost(&target, "::1%lo0")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateDirectUpstreamHost(0, targetWithHost(&target, "[::1]")), ErrUnsupportedIPAddressUpstreamHost) + assert.NotNil(t, validateDirectUpstreamHost(0, targetWithHost(&target, "[::1]:80")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateDirectUpstreamHost(0, targetWithHost(&target, "[::1%lo0]")), ErrUnsupportedIPAddressUpstreamHost) + assert.NotNil(t, validateDirectUpstreamHost(0, targetWithHost(&target, "[::1%lo0]:80")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateDirectUpstreamHost(0, targetWithHost(&target, "169.254.100.100")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateDirectUpstreamHost(0, targetWithHost(&target, "fe80::1")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateDirectUpstreamHost(0, targetWithHost(&target, "[fe80::1]")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateDirectUpstreamHost(0, targetWithHost(&target, "224.100.100.100")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateDirectUpstreamHost(0, targetWithHost(&target, "ff00::ffff")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateDirectUpstreamHost(0, targetWithHost(&target, "[ff00::ffff]")), ErrUnsupportedIPAddressUpstreamHost) + + // empty host + assert.Nil(t, validateDirectUpstreamHost(0, &Target{Options: TargetOptions{DirectUpstream: true}, Host: " "})) + // host with a space + assert.NotNil(t, validateDirectUpstreamHost(0, &Target{Options: TargetOptions{DirectUpstream: true}, Host: "with space"})) + // host with a tab + assert.NotNil(t, validateDirectUpstreamHost(0, &Target{Options: TargetOptions{DirectUpstream: true}, Host: "with\ttab"})) + // host with a slash + assert.NotNil(t, validateDirectUpstreamHost(0, &Target{Options: TargetOptions{DirectUpstream: true}, Host: "with/slash"})) +} + +func TestValidate_ValidateSubnetTarget(t *testing.T) { + target := Target{TargetId: "id-1", TargetType: TargetTypeSubnet, Host: "10.0.0.1", Port: 80, Protocol: "http", Enabled: true, Options: TargetOptions{DirectUpstream: true}} + assert.ErrorIs(t, validateSubnetTarget(0, targetWithHost(&target, "127.0.0.2")), ErrUnsupportedIPAddressUpstreamHost) + assert.NotNil(t, validateSubnetTarget(0, targetWithHost(&target, "127.0.0.2:80")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateSubnetTarget(0, targetWithHost(&target, "::1")), ErrUnsupportedIPAddressUpstreamHost) + assert.NotNil(t, validateSubnetTarget(0, targetWithHost(&target, "[::1]:80")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateSubnetTarget(0, targetWithHost(&target, "::1%lo0")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateSubnetTarget(0, targetWithHost(&target, "[::1%lo0]")), ErrUnsupportedIPAddressUpstreamHost) + assert.NotNil(t, validateSubnetTarget(0, targetWithHost(&target, "[::1%lo0]:80")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateSubnetTarget(0, targetWithHost(&target, "169.254.100.100")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateSubnetTarget(0, targetWithHost(&target, "fe80::1")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateSubnetTarget(0, targetWithHost(&target, "[fe80::1]")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateSubnetTarget(0, targetWithHost(&target, "224.100.100.100")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateSubnetTarget(0, targetWithHost(&target, "ff00::ffff")), ErrUnsupportedIPAddressUpstreamHost) + assert.ErrorIs(t, validateSubnetTarget(0, targetWithHost(&target, "[ff00::ffff]")), ErrUnsupportedIPAddressUpstreamHost) + + // empty host + assert.NotNil(t, validateSubnetTarget(0, &Target{Options: TargetOptions{DirectUpstream: true}, Host: " "})) + // host with a space + assert.NotNil(t, validateSubnetTarget(0, &Target{Options: TargetOptions{DirectUpstream: true}, Host: "with space"})) + // host with a tab + assert.NotNil(t, validateSubnetTarget(0, &Target{Options: TargetOptions{DirectUpstream: true}, Host: "with\ttab"})) + // host with a slash + assert.NotNil(t, validateSubnetTarget(0, &Target{Options: TargetOptions{DirectUpstream: true}, Host: "with/slash"})) +} + +func targetWithHost(t *Target, host string) *Target { + t.Host = host + return t +} + func TestToProtoMapping_TargetOptions(t *testing.T) { rp := &Service{ ID: "svc-1",