diff --git a/hscontrol/policy/matcher/matcher.go b/hscontrol/policy/matcher/matcher.go index b52cb4dc..275f2463 100644 --- a/hscontrol/policy/matcher/matcher.go +++ b/hscontrol/policy/matcher/matcher.go @@ -53,6 +53,11 @@ func MatchFromFilterRule(rule tailcfg.FilterRule) Match { return MatchFromStrings(rule.SrcIPs, dests) } +// MatchFromStrings builds a Match from raw source and destination +// strings. Unparseable entries are silently dropped (fail-open): the +// resulting Match is narrower than the input described, but never +// wider. Callers that need strict validation should pre-validate +// their inputs via util.ParseIPSet. func MatchFromStrings(sources, destinations []string) Match { srcs := new(netipx.IPSetBuilder) dests := new(netipx.IPSetBuilder) @@ -96,18 +101,20 @@ func (m *Match) DestsOverlapsPrefixes(prefixes ...netip.Prefix) bool { return slices.ContainsFunc(prefixes, m.dests.OverlapsPrefix) } -// DestsIsTheInternet reports if the destination contains "the internet" -// which is a IPSet that represents "autogroup:internet" and is special -// cased for exit nodes. -// This checks if dests is a superset of TheInternet(), which handles -// merged filter rules where TheInternet is combined with other destinations. +// DestsIsTheInternet reports whether the destination covers "the +// internet" — the set represented by autogroup:internet, special-cased +// for exit nodes. Returns true if either family's /0 is contained +// (0.0.0.0/0 or ::/0), or if dests is a superset of TheInternet(). A +// single-family /0 counts because operators may write it directly and +// it still denotes the whole internet for that family. func (m *Match) DestsIsTheInternet() bool { if m.dests.ContainsPrefix(tsaddr.AllIPv4()) || m.dests.ContainsPrefix(tsaddr.AllIPv6()) { return true } - // Check if dests contains all prefixes of TheInternet (superset check) + // Superset-of-TheInternet check handles merged filter rules + // where the internet prefixes are combined with other dests. theInternet := util.TheInternet() for _, prefix := range theInternet.Prefixes() { if !m.dests.ContainsPrefix(prefix) { diff --git a/hscontrol/policy/matcher/matcher_test.go b/hscontrol/policy/matcher/matcher_test.go index 588531bf..59c712c4 100644 --- a/hscontrol/policy/matcher/matcher_test.go +++ b/hscontrol/policy/matcher/matcher_test.go @@ -2,6 +2,7 @@ package matcher import ( "net/netip" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -428,4 +429,44 @@ func TestDebugString(t *testing.T) { assert.Contains(t, s, "Destinations:") assert.Contains(t, s, "10.0.0.0/8") assert.Contains(t, s, "192.168.1.0/24") + + // Sources appear before Destinations in the output. + assert.Less( + t, + strings.Index(s, "Sources:"), + strings.Index(s, "Destinations:"), + "Sources section must precede Destinations", + ) +} + +func TestDebugString_Empty(t *testing.T) { + t.Parallel() + + m := MatchFromStrings(nil, nil) + + s := m.DebugString() + assert.Contains(t, s, "Match:") + assert.Contains(t, s, "Sources:") + assert.Contains(t, s, "Destinations:") + assert.NotContains(t, s, "/") +} + +// TestMatchFromStrings_MalformedFailsOpen asserts that unparseable +// entries are silently dropped and do not crash or widen the Match. +func TestMatchFromStrings_MalformedFailsOpen(t *testing.T) { + t.Parallel() + + m := MatchFromStrings( + []string{"not-a-cidr", "10.0.0.0/8"}, + []string{"also-bogus", "192.168.1.0/24"}, + ) + + assert.True(t, m.SrcsContainsIPs(netip.MustParseAddr("10.1.2.3")), + "valid src entry must still match") + assert.False(t, m.SrcsContainsIPs(netip.MustParseAddr("1.1.1.1")), + "malformed src entry must not widen the set") + assert.True(t, m.DestsContainsIP(netip.MustParseAddr("192.168.1.10")), + "valid dst entry must still match") + assert.False(t, m.DestsContainsIP(netip.MustParseAddr("8.8.8.8")), + "malformed dst entry must not widen the set") }