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..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 ----- { diff --git a/pkg/config/pattern.go b/pkg/config/pattern.go index fe8e779..573c24e 100644 --- a/pkg/config/pattern.go +++ b/pkg/config/pattern.go @@ -46,8 +46,19 @@ func compileHostnamePattern(raw string) (hostnamePattern, error) { } // Matches returns true if hostname matches the glob pattern. +// +// 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) } @@ -68,21 +79,23 @@ 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. + for li := labelCount - 1; li >= 0; li-- { 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 {