diff --git a/hscontrol/policy/v2/utils.go b/hscontrol/policy/v2/utils.go index ddf41f8e..2f22a9cf 100644 --- a/hscontrol/policy/v2/utils.go +++ b/hscontrol/policy/v2/utils.go @@ -2,6 +2,8 @@ package v2 import ( "errors" + "fmt" + "net/netip" "slices" "strconv" "strings" @@ -19,10 +21,44 @@ var ( ErrPortMustBePositive = errors.New("first port must be >0, or use '*' for wildcard") ErrInvalidPortNumber = errors.New("invalid port number") ErrPortNumberOutOfRange = errors.New("port number out of range") + ErrBracketsNotIPv6 = errors.New("square brackets are only valid around IPv6 addresses") ) // splitDestinationAndPort takes an input string and returns the destination and port as a tuple, or an error if the input is invalid. +// It supports two bracketed IPv6 forms: +// - "[addr]:port" (RFC 3986, e.g. "[::1]:80") +// - "[addr]/prefix:port" (e.g. "[fd7a::1]/128:80,443") +// +// Brackets are only accepted around IPv6 addresses, not IPv4, hostnames, or other alias types. +// Bracket stripping reduces both forms to bare "addr:port" or "addr/prefix:port", +// which the normal LastIndex(":") split handles correctly because port strings +// never contain colons. func splitDestinationAndPort(input string) (string, string, error) { + // Handle RFC 3986 bracketed IPv6 (e.g. "[::1]:80" or "[fd7a::1]/128:80,443"). + // Strip brackets after validation and fall through to normal parsing. + if strings.HasPrefix(input, "[") { + closeBracket := strings.Index(input, "]") + if closeBracket == -1 { + return "", "", ErrBracketsNotIPv6 + } + + host := input[1:closeBracket] + + addr, err := netip.ParseAddr(host) + if err != nil || !addr.Is6() { + return "", "", fmt.Errorf("%w: %q", ErrBracketsNotIPv6, host) + } + + rest := input[closeBracket+1:] + if len(rest) == 0 || (rest[0] != ':' && rest[0] != '/') { + return "", "", fmt.Errorf("%w: %q", ErrBracketsNotIPv6, input) + } + + // Strip brackets: "[addr]:port" → "addr:port", + // "[addr]/prefix:port" → "addr/prefix:port". + input = host + rest + } + // Find the last occurrence of the colon character lastColonIndex := strings.LastIndex(input, ":") diff --git a/hscontrol/policy/v2/utils_test.go b/hscontrol/policy/v2/utils_test.go index 496f4618..aa65bf10 100644 --- a/hscontrol/policy/v2/utils_test.go +++ b/hscontrol/policy/v2/utils_test.go @@ -1,39 +1,153 @@ package v2 import ( + "errors" "testing" "github.com/google/go-cmp/cmp" "tailscale.com/tailcfg" ) -// TestParseDestinationAndPort tests the parseDestinationAndPort function using table-driven tests. +// TestParseDestinationAndPort tests the splitDestinationAndPort function using table-driven tests. func TestParseDestinationAndPort(t *testing.T) { testCases := []struct { - input string - expectedDst string - expectedPort string - expectedErr error + input string + wantDst string + wantPort string + wantErrIs error + wantNoError bool }{ - {"git-server:*", "git-server", "*", nil}, - {"192.168.1.0/24:22", "192.168.1.0/24", "22", nil}, - {"fd7a:115c:a1e0::2:22", "fd7a:115c:a1e0::2", "22", nil}, - {"fd7a:115c:a1e0::2/128:22", "fd7a:115c:a1e0::2/128", "22", nil}, - {"tag:montreal-webserver:80,443", "tag:montreal-webserver", "80,443", nil}, - {"tag:api-server:443", "tag:api-server", "443", nil}, - {"example-host-1:*", "example-host-1", "*", nil}, - {"hostname:80-90", "hostname", "80-90", nil}, - {"invalidinput", "", "", ErrInputMissingColon}, - {":invalid", "", "", ErrInputStartsWithColon}, - {"invalid:", "", "", ErrInputEndsWithColon}, + // --- Non-bracketed inputs (existing behavior, unchanged) --- + + // Hostnames and tags + {"git-server:*", "git-server", "*", nil, true}, + {"example-host-1:*", "example-host-1", "*", nil, true}, + {"hostname:80-90", "hostname", "80-90", nil, true}, + {"tag:montreal-webserver:80,443", "tag:montreal-webserver", "80,443", nil, true}, + {"tag:api-server:443", "tag:api-server", "443", nil, true}, + + // IPv4 and IPv4 CIDR + {"192.168.1.0/24:22", "192.168.1.0/24", "22", nil, true}, + {"10.0.0.1:443", "10.0.0.1", "443", nil, true}, + + // Bare IPv6 (no brackets) — last colon splits correctly + {"fd7a:115c:a1e0::2:22", "fd7a:115c:a1e0::2", "22", nil, true}, + {"fd7a:115c:a1e0::2/128:22", "fd7a:115c:a1e0::2/128", "22", nil, true}, + + // --- Bracketed IPv6: [addr]:port --- + + // Single port + {"[fd7a:115c:a1e0::87e1]:22", "fd7a:115c:a1e0::87e1", "22", nil, true}, + {"[::1]:80", "::1", "80", nil, true}, + {"[2001:db8::1]:443", "2001:db8::1", "443", nil, true}, + {"[fe80::1]:22", "fe80::1", "22", nil, true}, + + // Multiple ports + {"[fd7a:115c:a1e0::87e1]:80,443", "fd7a:115c:a1e0::87e1", "80,443", nil, true}, + {"[::1]:22,80,443", "::1", "22,80,443", nil, true}, + + // Port range + {"[fd7a:115c:a1e0::2]:80-90", "fd7a:115c:a1e0::2", "80-90", nil, true}, + + // Wildcard port + {"[fd7a:115c:a1e0::87e1]:*", "fd7a:115c:a1e0::87e1", "*", nil, true}, + + // Unspecified address [::] + {"[::]:80", "::", "80", nil, true}, + {"[::]:*", "::", "*", nil, true}, + + // Full-length IPv6 + {"[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:443", "2001:0db8:85a3:0000:0000:8a2e:0370:7334", "443", nil, true}, + + // --- Bracketed IPv6 CIDR: [addr]/prefix:port --- + + {"[fd7a:115c:a1e0::2905]/128:80,443", "fd7a:115c:a1e0::2905/128", "80,443", nil, true}, + {"[fd7a:115c:a1e0::1]/128:22", "fd7a:115c:a1e0::1/128", "22", nil, true}, + {"[2001:db8::1]/32:443", "2001:db8::1/32", "443", nil, true}, + {"[::1]/128:*", "::1/128", "*", nil, true}, + {"[fd7a:115c:a1e0::2]/64:80-90", "fd7a:115c:a1e0::2/64", "80-90", nil, true}, + {"[::]/0:*", "::/0", "*", nil, true}, + + // --- Errors: brackets around non-IPv6 --- + + // IPv4 in brackets + {"[192.168.1.1]:80", "", "", ErrBracketsNotIPv6, false}, + {"[10.0.0.1]:443", "", "", ErrBracketsNotIPv6, false}, + {"[192.168.1.1]/32:80", "", "", ErrBracketsNotIPv6, false}, + + // IPv4 CIDR inside brackets + {"[10.0.0.0/8]:80", "", "", ErrBracketsNotIPv6, false}, + + // Hostnames in brackets + {"[my-hostname]:80", "", "", ErrBracketsNotIPv6, false}, + {"[git-server]:*", "", "", ErrBracketsNotIPv6, false}, + + // Tags in brackets + {"[tag:server]:80", "", "", ErrBracketsNotIPv6, false}, + + // --- Errors: CIDR inside brackets (must use [addr]/prefix:port) --- + + {"[fd7a:115c:a1e0::2/128]:22", "", "", ErrBracketsNotIPv6, false}, + {"[2001:db8::/32]:443", "", "", ErrBracketsNotIPv6, false}, + {"[::1/128]:80", "", "", ErrBracketsNotIPv6, false}, + + // --- Errors: malformed bracket syntax --- + + // No port after brackets + {"[::1]", "", "", ErrBracketsNotIPv6, false}, + {"[2001:db8::1]", "", "", ErrBracketsNotIPv6, false}, + + // Empty brackets + {"[]:80", "", "", ErrBracketsNotIPv6, false}, + + // Missing close bracket + {"[::1", "", "", ErrBracketsNotIPv6, false}, + {"[2001:db8::1:80", "", "", ErrBracketsNotIPv6, false}, + + // Empty port after colon + {"[fd7a:115c:a1e0::1]:", "", "", ErrInputEndsWithColon, false}, + {"[::1]:", "", "", ErrInputEndsWithColon, false}, + {"[fd7a::1]/128:", "", "", ErrInputEndsWithColon, false}, + + // Junk after close bracket (not : or /) + {"[::1]blah", "", "", ErrBracketsNotIPv6, false}, + {"[::1] :80", "", "", ErrBracketsNotIPv6, false}, + + // --- Errors: non-bracketed malformed input (unchanged) --- + + {"invalidinput", "", "", ErrInputMissingColon, false}, + {":invalid", "", "", ErrInputStartsWithColon, false}, + {"invalid:", "", "", ErrInputEndsWithColon, false}, } - for _, testCase := range testCases { - dst, port, err := splitDestinationAndPort(testCase.input) - if dst != testCase.expectedDst || port != testCase.expectedPort || (err != nil && err.Error() != testCase.expectedErr.Error()) { - t.Errorf("parseDestinationAndPort(%q) = (%q, %q, %v), want (%q, %q, %v)", - testCase.input, dst, port, err, testCase.expectedDst, testCase.expectedPort, testCase.expectedErr) - } + for _, tc := range testCases { + t.Run(tc.input, func(t *testing.T) { + dst, port, err := splitDestinationAndPort(tc.input) + + if tc.wantNoError { + if err != nil { + t.Fatalf("splitDestinationAndPort(%q) unexpected error: %v", tc.input, err) + } + + if dst != tc.wantDst { + t.Errorf("splitDestinationAndPort(%q) dst = %q, want %q", tc.input, dst, tc.wantDst) + } + + if port != tc.wantPort { + t.Errorf("splitDestinationAndPort(%q) port = %q, want %q", tc.input, port, tc.wantPort) + } + + return + } + + if err == nil { + t.Fatalf("splitDestinationAndPort(%q) = (%q, %q, nil), want error wrapping %v", tc.input, dst, port, tc.wantErrIs) + } + + if !errors.Is(err, tc.wantErrIs) { + t.Errorf("splitDestinationAndPort(%q) error = %v, want error wrapping %v", tc.input, err, tc.wantErrIs) + } + }) } }