From e0b15d0dd9c7a16fac7126e0cd1d95d5c6f9994f Mon Sep 17 00:00:00 2001 From: Matthew DeVenny Date: Wed, 16 Sep 2026 10:18:51 -0700 Subject: [PATCH 1/2] #125 refuse the empty hostname in the pattern matcher strings.Split("", ".") yields one empty label, which both `*` and `**` consumed, so an IP with no reverse name matched any wildcard rule. gateExistingConnections then opened it on the rule's ports instead of its observed ports (dropping the in-flight flow mid-stream a minute later) and pinned L7 scope to an empty name. matchSegments now leaves an empty label unmatched: "" is never a DNS label, so no segment may consume it. That makes "" match nothing at every entry point (MatchHostnameRule, FindTrackedHostname, GetAutoAllowedTypeForHostname) and also stops `**` from bridging a stray empty label ("a..b", "github.com."). Signed-off-by: Matthew DeVenny --- cmd/start_test.go | 22 ++++++++++++++++ pkg/config/config_test.go | 27 ++++++++++++++++++++ pkg/config/pattern.go | 21 +++++++++++++--- pkg/config/pattern_test.go | 51 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 4 deletions(-) diff --git a/cmd/start_test.go b/cmd/start_test.go index 1fad96f..809eed7 100644 --- a/cmd/start_test.go +++ b/cmd/start_test.go @@ -426,6 +426,28 @@ func TestGateExistingConnections_UnresolvableAllowsObservedPortsOnly(t *testing. gateExistingConnections(existingConns{"203.0.113.5": observed}, cm, fw, nil, nil, quietLogger()) } +// The unresolvable carve-out must survive a `*` / `**` hostname rule in the +// policy. "" is not a hostname, so the wildcard must not fire on an IP with +// no reverse name: the IP is allowed on its observed ports (443 here), not +// the rule's (123), and nothing is scoped at L7 under an empty name (#125). +func TestGateExistingConnections_UnresolvableIgnoresWildcardRule(t *testing.T) { + cm := config.NewConfigManager() + require.NoError(t, cm.LoadConfigFromRules([]config.Rule{ + {Type: config.RuleTypeHostname, Value: "**", Ports: []config.Port{{Port: 123, Protocol: config.ProtocolUDP}}, Action: config.ActionAllow}, + }, config.ActionDeny)) + + observed := []config.Port{{Port: 443, Protocol: config.ProtocolTCP}} + fw := firewall.NewMockFirewall(t) + fw.EXPECT().AddIP(net.ParseIP("20.96.133.71"), config.ActionAllow, observed).Return(true, nil).Once() + + rec := &cmdRecordingRegistrar{} + reg := l7LateRegistrar{l7: rec, cm: cm, logger: quietLogger()} + + gateExistingConnections(existingConns{"20.96.133.71": observed}, cm, fw, reg, nil, quietLogger()) + + assert.Empty(t, rec.scopes, "scoped an IP under an empty hostname") +} + // Denied pre-existing connections must not be added to the allowlist — // NewMockFirewall(t) fails the test on any unexpected AddIP call. func TestGateExistingConnections_DeniedHostnameNotAdded(t *testing.T) { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index fb639bb..6437cee 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -4400,3 +4400,30 @@ func TestLoadConfigFromCargoWall_RejectedPolicyKeepsPosture(t *testing.T) { t.Fatal("rejected policy must not change posture") } } + +// An IP with no reverse name reaches the matcher as "". A `*` or `**` rule +// must not fire on it: gateExistingConnections would open the IP on the +// wildcard's ports instead of its observed ports and pin L7 scope to an +// empty name (#125). Every pattern shape, and the attribution lookup, must +// agree that "" matches nothing. +func TestMatchHostnameRule_EmptyHostnameMatchesNothing(t *testing.T) { + for _, pattern := range []string{"*", "**", "*.example.com", "**.example.com"} { + t.Run(pattern, func(t *testing.T) { + cm := NewConfigManager() + if err := cm.LoadConfigFromRules([]Rule{ + {Type: RuleTypeHostname, Value: pattern, Ports: []Port{{Port: 123, Protocol: ProtocolUDP}}, Action: ActionAllow}, + }, ActionDeny); err != nil { + t.Fatal(err) + } + if v := cm.MatchHostnameRule(""); v.Matched() { + t.Errorf("MatchHostnameRule(\"\") = %+v, want no match", v) + } + if got := cm.FindTrackedHostname(""); got != "" { + t.Errorf("FindTrackedHostname(\"\") = %q, want \"\"", got) + } + if got := cm.GetAutoAllowedTypeForHostname(""); got != AutoAddedTypeNone { + t.Errorf("GetAutoAllowedTypeForHostname(\"\") = %q, want none", got) + } + }) + } +} diff --git a/pkg/config/pattern.go b/pkg/config/pattern.go index fe8e779..d70b4f1 100644 --- a/pkg/config/pattern.go +++ b/pkg/config/pattern.go @@ -46,6 +46,11 @@ func compileHostnamePattern(raw string) (hostnamePattern, error) { } // Matches returns true if hostname matches the glob pattern. +// +// The empty hostname never matches. strings.Split("", ".") yields one empty +// label, which "*" and "**" would otherwise consume — and "" is what an IP +// with no reverse name looks like to callers such as gateExistingConnections, +// which must not hand such an IP a wildcard rule's ports (#125). func (p *hostnamePattern) Matches(hostname string) bool { labels := strings.Split(hostname, ".") return matchSegments(p.Segments, labels) @@ -68,21 +73,29 @@ func matchSegments(segments []string, labels []string) bool { for si := segCount - 1; si >= 0; si-- { seg := segments[si] - for li := labelCount; li >= 0; li-- { + // dp[si][labelCount] stays false: every segment consumes at least + // one label. An empty label is left false for the same reason — "" + // is never a DNS label (it is what "" and "a..b" split to), so no + // segment, wildcard or literal, may consume it. Literal segments are + // non-empty by construction (compileHostnamePattern). + for li := labelCount - 1; li >= 0; li-- { + if labels[li] == "" { + continue + } switch seg { case "**": // ** matches one or more labels - if li < labelCount && (dp[si][li+1] || dp[si+1][li+1]) { + if dp[si][li+1] || dp[si+1][li+1] { dp[si][li] = true } case "*": // * matches exactly one label - if li < labelCount && dp[si+1][li+1] { + if dp[si+1][li+1] { dp[si][li] = true } default: // Literal match - if li < labelCount && labels[li] == seg && dp[si+1][li+1] { + if labels[li] == seg && dp[si+1][li+1] { dp[si][li] = true } } diff --git a/pkg/config/pattern_test.go b/pkg/config/pattern_test.go index ad863cf..e63bc62 100644 --- a/pkg/config/pattern_test.go +++ b/pkg/config/pattern_test.go @@ -183,6 +183,57 @@ func TestHostnamePatternMatches(t *testing.T) { "other.foo.internal.cloudapp.net", false, }, + // Empty hostname / empty labels. "" splits to one empty label, which + // no segment may consume: an IP with no reverse name must not take a + // wildcard rule's verdict (#125). + { + "doublestar rejects empty hostname", + "**", + "", + false, + }, + { + "star rejects empty hostname", + "*", + "", + false, + }, + { + "doublestar suffix rejects empty hostname", + "**.github.com", + "", + false, + }, + { + "star suffix rejects empty hostname", + "*.github.com", + "", + false, + }, + { + "literal rejects empty hostname", + "github.com", + "", + false, + }, + { + "doublestar rejects empty label", + "**", + "a..b", + false, + }, + { + "doublestar rejects trailing empty label", + "**", + "github.com.", + false, + }, + { + "star rejects empty label", + "*.github.com", + ".github.com", + false, + }, } for _, tt := range tests { From dd00a9f0f1c0930a3215bb1aa12953497f3a69ff Mon Sep 17 00:00:00 2001 From: Matthew DeVenny Date: Thu, 17 Sep 2026 08:04:48 -0700 Subject: [PATCH 2/2] #125 reject empty labels at the Matches boundary, not inside the DP Review: the empty-label refusal belongs at the edge, the same way compileHostnamePattern rejects empty segments, so matchSegments stays a plain glob over well-formed labels. Matches now returns false for any hostname with an empty label; the DP loses its special case and keeps only the loop-bound cleanup. The appended MatchHostnameRule test asserted two APIs that cannot observe the bug (FindTrackedHostname attributes patterns to the query name, so "" in is "" out either way; GetAutoAllowedTypeForHostname skips every rule LoadConfigFromRules produces). Replaced by one query-"" row in TestMatchHostnameRule_Table. Signed-off-by: Matthew DeVenny --- pkg/config/config_test.go | 37 ++++++++++--------------------------- pkg/config/pattern.go | 22 +++++++++++----------- 2 files changed, 21 insertions(+), 38 deletions(-) diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 6437cee..f1bef8b 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -3322,6 +3322,16 @@ func TestMatchHostnameRule_Table(t *testing.T) { query: "evil.example.com", skipPortCheck: true, }, + { + // "" is what an IP with no reverse name looks up as; a wildcard + // must not fire on it (#125). + name: "empty hostname matches nothing, even a bare wildcard", + rules: []Rule{ + {Type: RuleTypeHostname, Value: "**", Ports: []Port{{Port: 123, Protocol: ProtocolUDP}}, Action: ActionAllow}, + }, + query: "", + skipPortCheck: true, + }, // ----- Deny precedence ----- { @@ -4400,30 +4410,3 @@ func TestLoadConfigFromCargoWall_RejectedPolicyKeepsPosture(t *testing.T) { t.Fatal("rejected policy must not change posture") } } - -// An IP with no reverse name reaches the matcher as "". A `*` or `**` rule -// must not fire on it: gateExistingConnections would open the IP on the -// wildcard's ports instead of its observed ports and pin L7 scope to an -// empty name (#125). Every pattern shape, and the attribution lookup, must -// agree that "" matches nothing. -func TestMatchHostnameRule_EmptyHostnameMatchesNothing(t *testing.T) { - for _, pattern := range []string{"*", "**", "*.example.com", "**.example.com"} { - t.Run(pattern, func(t *testing.T) { - cm := NewConfigManager() - if err := cm.LoadConfigFromRules([]Rule{ - {Type: RuleTypeHostname, Value: pattern, Ports: []Port{{Port: 123, Protocol: ProtocolUDP}}, Action: ActionAllow}, - }, ActionDeny); err != nil { - t.Fatal(err) - } - if v := cm.MatchHostnameRule(""); v.Matched() { - t.Errorf("MatchHostnameRule(\"\") = %+v, want no match", v) - } - if got := cm.FindTrackedHostname(""); got != "" { - t.Errorf("FindTrackedHostname(\"\") = %q, want \"\"", got) - } - if got := cm.GetAutoAllowedTypeForHostname(""); got != AutoAddedTypeNone { - t.Errorf("GetAutoAllowedTypeForHostname(\"\") = %q, want none", got) - } - }) - } -} diff --git a/pkg/config/pattern.go b/pkg/config/pattern.go index d70b4f1..573c24e 100644 --- a/pkg/config/pattern.go +++ b/pkg/config/pattern.go @@ -47,12 +47,18 @@ func compileHostnamePattern(raw string) (hostnamePattern, error) { // Matches returns true if hostname matches the glob pattern. // -// The empty hostname never matches. strings.Split("", ".") yields one empty -// label, which "*" and "**" would otherwise consume — and "" is what an IP -// with no reverse name looks like to callers such as gateExistingConnections, -// which must not hand such an IP a wildcard rule's ports (#125). +// A hostname with an empty label never matches: "" is not a DNS label, so +// no segment, literal or wildcard, may consume one. That covers the empty +// hostname itself, which splits to a single empty label. Rejected here, at +// the boundary, the same way compileHostnamePattern rejects empty segments, +// so matchSegments only ever sees well-formed labels. func (p *hostnamePattern) Matches(hostname string) bool { labels := strings.Split(hostname, ".") + for _, label := range labels { + if label == "" { + return false + } + } return matchSegments(p.Segments, labels) } @@ -74,14 +80,8 @@ func matchSegments(segments []string, labels []string) bool { for si := segCount - 1; si >= 0; si-- { seg := segments[si] // dp[si][labelCount] stays false: every segment consumes at least - // one label. An empty label is left false for the same reason — "" - // is never a DNS label (it is what "" and "a..b" split to), so no - // segment, wildcard or literal, may consume it. Literal segments are - // non-empty by construction (compileHostnamePattern). + // one label. for li := labelCount - 1; li >= 0; li-- { - if labels[li] == "" { - continue - } switch seg { case "**": // ** matches one or more labels