policy/v2: accept RFC 3986 bracketed IPv6 in ACL destinations

Headscale rejects IPv6 addresses with square brackets in ACL policy
destinations (e.g. "[fd7a:115c:a1e0::87e1]:80,443"), while Tailscale
SaaS accepts them. The root cause is that splitDestinationAndPort uses
strings.LastIndex(":") which leaves brackets on the destination string,
and netip.ParseAddr does not accept brackets.

Add a bracket-handling branch at the top of splitDestinationAndPort that
uses net.SplitHostPort for RFC 3986 parsing when input starts with "[".
The extracted host is validated with netip.ParseAddr/ParsePrefix to
ensure brackets are only accepted around IP addresses and CIDR prefixes,
not hostnames or other alias types like tags and groups.

Fixes #2754
This commit is contained in:
Kristoffer Dalby
2026-02-18 08:27:42 +00:00
parent a0d6802d5b
commit 49744cd467
2 changed files with 172 additions and 22 deletions

View File

@@ -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, ":")

View File

@@ -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)
}
})
}
}