From c851293904f8a7f941e09448b1f0fe5b4a935994 Mon Sep 17 00:00:00 2001 From: Dimitri Herzog Date: Mon, 8 Jun 2026 09:19:10 +0200 Subject: [PATCH] feat(blocking): log the matched rule in the block reason (#2091) --- cache/stringcache/chained_grouped_cache.go | 29 ++-- .../stringcache/chained_grouped_cache_test.go | 26 ++-- cache/stringcache/grouped_cache_interface.go | 5 +- cache/stringcache/in_memory_grouped_cache.go | 15 +- .../in_memory_grouped_cache_test.go | 42 ++++-- cache/stringcache/string_caches.go | 38 +++-- .../string_caches_benchmark_test.go | 2 +- cache/stringcache/string_caches_test.go | 137 +++++++++++++----- docs/configuration.md | 8 +- lists/list_cache.go | 12 +- lists/list_cache_test.go | 60 ++++---- resolver/blocking_resolver.go | 40 +++-- resolver/blocking_resolver_test.go | 90 +++++++----- resolver/ede_resolver.go | 26 +++- resolver/ede_resolver_test.go | 49 +++++++ trie/split.go | 7 + trie/split_test.go | 19 +++ trie/trie.go | 102 +++++++------ trie/trie_test.go | 108 +++++++++++--- 19 files changed, 572 insertions(+), 243 deletions(-) diff --git a/cache/stringcache/chained_grouped_cache.go b/cache/stringcache/chained_grouped_cache.go index 80442754..70a5f63b 100644 --- a/cache/stringcache/chained_grouped_cache.go +++ b/cache/stringcache/chained_grouped_cache.go @@ -1,10 +1,5 @@ package stringcache -import ( - "maps" - "slices" -) - type ChainedGroupedCache struct { caches []GroupedStringCache } @@ -24,18 +19,28 @@ func (c *ChainedGroupedCache) ElementCount(group string) int { return sum } -func (c *ChainedGroupedCache) Contains(searchString string, groups []string) []string { - groupMatchedMap := make(map[string]struct{}, len(groups)) +func (c *ChainedGroupedCache) Contains(searchString string, groups []string) map[string]string { + // result is allocated lazily so the common no-match case stays allocation-free. + // Ordering of matched groups is not defined here; callers that render the + // result sort it (see resolver.formatBlockReason). + // + // If a group matches in more than one chained cache (e.g. an exact entry and + // a wildcard), we keep a single rule per group: the last chained cache wins. + // One representative rule per group is enough for the block reason, and the + // chain order is fixed, so the choice is stable across requests. + var result map[string]string for _, cache := range c.caches { - for _, group := range cache.Contains(searchString, groups) { - groupMatchedMap[group] = struct{}{} + for group, rule := range cache.Contains(searchString, groups) { + if result == nil { + result = make(map[string]string, len(groups)) + } + + result[group] = rule } } - matchedGroups := slices.Sorted(maps.Keys(groupMatchedMap)) - - return matchedGroups + return result } func (c *ChainedGroupedCache) Refresh(group string) GroupFactory { diff --git a/cache/stringcache/chained_grouped_cache_test.go b/cache/stringcache/chained_grouped_cache_test.go index 8fe2432a..79f78b93 100644 --- a/cache/stringcache/chained_grouped_cache_test.go +++ b/cache/stringcache/chained_grouped_cache_test.go @@ -56,10 +56,12 @@ var _ = Describe("Chained grouped cache", func() { Expect(cache.ElementCount("group1")).Should(BeNumerically("==", 2)) }) - It("should find strings", func() { + It("should find strings and return the matched rule per group", func() { factory.Finish() - Expect(cache.Contains("string1", []string{"group1"})).Should(ConsistOf("group1")) - Expect(cache.Contains("string2", []string{"group1", "someOtherGroup"})).Should(ConsistOf("group1")) + Expect(cache.Contains("string1", []string{"group1"})). + Should(Equal(map[string]string{"group1": "string1"})) + Expect(cache.Contains("string2", []string{"group1", "someOtherGroup"})). + Should(Equal(map[string]string{"group1": "string2"})) }) }) }) @@ -86,9 +88,12 @@ var _ = Describe("Chained grouped cache", func() { It("should contain 4 elements in 2 groups", func() { Expect(cache.ElementCount("group1")).Should(BeNumerically("==", 2)) Expect(cache.ElementCount("group2")).Should(BeNumerically("==", 2)) - Expect(cache.Contains("g1", []string{"group1", "group2"})).Should(ConsistOf("group1")) - Expect(cache.Contains("g2", []string{"group1", "group2"})).Should(ConsistOf("group2")) - Expect(cache.Contains("both", []string{"group1", "group2"})).Should(ConsistOf("group1", "group2")) + Expect(cache.Contains("g1", []string{"group1", "group2"})). + Should(Equal(map[string]string{"group1": "g1"})) + Expect(cache.Contains("g2", []string{"group1", "group2"})). + Should(Equal(map[string]string{"group2": "g2"})) + Expect(cache.Contains("both", []string{"group1", "group2"})). + Should(Equal(map[string]string{"group1": "both", "group2": "both"})) }) It("should replace group content on refresh", func() { @@ -99,9 +104,12 @@ var _ = Describe("Chained grouped cache", func() { Expect(cache.ElementCount("group1")).Should(BeNumerically("==", 1)) Expect(cache.ElementCount("group2")).Should(BeNumerically("==", 2)) Expect(cache.Contains("g1", []string{"group1", "group2"})).Should(BeEmpty()) - Expect(cache.Contains("newString", []string{"group1", "group2"})).Should(ConsistOf("group1")) - Expect(cache.Contains("g2", []string{"group1", "group2"})).Should(ConsistOf("group2")) - Expect(cache.Contains("both", []string{"group1", "group2"})).Should(ConsistOf("group2")) + Expect(cache.Contains("newString", []string{"group1", "group2"})). + Should(Equal(map[string]string{"group1": "newstring"})) // rule is normalized to lower-case + Expect(cache.Contains("g2", []string{"group1", "group2"})). + Should(Equal(map[string]string{"group2": "g2"})) + Expect(cache.Contains("both", []string{"group1", "group2"})). + Should(Equal(map[string]string{"group2": "both"})) }) It("should replace empty groups on refresh", func() { diff --git a/cache/stringcache/grouped_cache_interface.go b/cache/stringcache/grouped_cache_interface.go index 53dec3c0..70d901c6 100644 --- a/cache/stringcache/grouped_cache_interface.go +++ b/cache/stringcache/grouped_cache_interface.go @@ -2,8 +2,9 @@ package stringcache type GroupedStringCache interface { // Contains checks if one or more groups in the cache contains the search string. - // Returns group(s) containing the string or empty slice if string was not found - Contains(searchString string, groups []string) []string + // Returns a map of matching group -> the rule that matched, or nil/empty when + // the string was not found. + Contains(searchString string, groups []string) map[string]string // Refresh creates new factory for the group to be refreshed. // Calling Finish on the factory will perform the group refresh. diff --git a/cache/stringcache/in_memory_grouped_cache.go b/cache/stringcache/in_memory_grouped_cache.go index db0e0127..daca41c9 100644 --- a/cache/stringcache/in_memory_grouped_cache.go +++ b/cache/stringcache/in_memory_grouped_cache.go @@ -43,16 +43,23 @@ func (c *InMemoryGroupedCache) ElementCount(group string) int { return cache.elementCount() } -func (c *InMemoryGroupedCache) Contains(searchString string, groups []string) []string { - var result []string +func (c *InMemoryGroupedCache) Contains(searchString string, groups []string) map[string]string { + // result is allocated lazily so the common no-match case stays allocation-free. + var result map[string]string for _, group := range groups { c.lock.RLock() cache, found := c.caches[group] c.lock.RUnlock() - if found && cache.contains(searchString) { - result = append(result, group) + if found { + if rule, ok := cache.findMatch(searchString); ok { + if result == nil { + result = make(map[string]string, len(groups)) + } + + result[group] = rule + } } } diff --git a/cache/stringcache/in_memory_grouped_cache_test.go b/cache/stringcache/in_memory_grouped_cache_test.go index 938d7b8a..ae75162f 100644 --- a/cache/stringcache/in_memory_grouped_cache_test.go +++ b/cache/stringcache/in_memory_grouped_cache_test.go @@ -63,10 +63,12 @@ var _ = Describe("In-Memory grouped cache", func() { Expect(cache.ElementCount("group1")).Should(BeNumerically("==", 2)) }) - It("should find strings", func() { + It("should find strings and return the matched rule per group", func() { factory.Finish() - Expect(cache.Contains("string1", []string{"group1"})).Should(ConsistOf("group1")) - Expect(cache.Contains("string2", []string{"group1", "someOtherGroup"})).Should(ConsistOf("group1")) + Expect(cache.Contains("string1", []string{"group1"})). + Should(Equal(map[string]string{"group1": "string1"})) + Expect(cache.Contains("string2", []string{"group1", "someOtherGroup"})). + Should(Equal(map[string]string{"group1": "string2"})) }) }) When("Regex grouped cache is used", func() { @@ -79,11 +81,13 @@ var _ = Describe("In-Memory grouped cache", func() { factory.Finish() }) - It("should ignore non-regex", func() { + It("should ignore non-regex and return the matching pattern", func() { Expect(cache.ElementCount("group1")).Should(BeNumerically("==", 1)) Expect(cache.Contains("string1", []string{"group1"})).Should(BeEmpty()) - Expect(cache.Contains("string2", []string{"group1"})).Should(ConsistOf("group1")) - Expect(cache.Contains("shouldalsomatchstring2", []string{"group1"})).Should(ConsistOf("group1")) + Expect(cache.Contains("string2", []string{"group1"})). + Should(Equal(map[string]string{"group1": "/string2/"})) + Expect(cache.Contains("shouldalsomatchstring2", []string{"group1"})). + Should(Equal(map[string]string{"group1": "/string2/"})) }) }) When("Wildcard grouped cache is used", func() { @@ -97,12 +101,14 @@ var _ = Describe("In-Memory grouped cache", func() { factory.Finish() }) - It("should ignore non-wildcard", func() { + It("should ignore non-wildcard and return the wildcard rule", func() { Expect(cache.ElementCount("group1")).Should(BeNumerically("==", 1)) Expect(cache.Contains("string1", []string{"group1"})).Should(BeEmpty()) Expect(cache.Contains("string2", []string{"group1"})).Should(BeEmpty()) - Expect(cache.Contains("string3", []string{"group1"})).Should(ConsistOf("group1")) - Expect(cache.Contains("shouldalsomatch.string3", []string{"group1"})).Should(ConsistOf("group1")) + Expect(cache.Contains("string3", []string{"group1"})). + Should(Equal(map[string]string{"group1": "*.string3"})) + Expect(cache.Contains("shouldalsomatch.string3", []string{"group1"})). + Should(Equal(map[string]string{"group1": "*.string3"})) }) }) }) @@ -126,9 +132,12 @@ var _ = Describe("In-Memory grouped cache", func() { It("should contain 4 elements in 2 groups", func() { Expect(cache.ElementCount("group1")).Should(BeNumerically("==", 2)) Expect(cache.ElementCount("group2")).Should(BeNumerically("==", 2)) - Expect(cache.Contains("g1", []string{"group1", "group2"})).Should(ConsistOf("group1")) - Expect(cache.Contains("g2", []string{"group1", "group2"})).Should(ConsistOf("group2")) - Expect(cache.Contains("both", []string{"group1", "group2"})).Should(ConsistOf("group1", "group2")) + Expect(cache.Contains("g1", []string{"group1", "group2"})). + Should(Equal(map[string]string{"group1": "g1"})) + Expect(cache.Contains("g2", []string{"group1", "group2"})). + Should(Equal(map[string]string{"group2": "g2"})) + Expect(cache.Contains("both", []string{"group1", "group2"})). + Should(Equal(map[string]string{"group1": "both", "group2": "both"})) }) It("Should replace group content on refresh", func() { @@ -139,9 +148,12 @@ var _ = Describe("In-Memory grouped cache", func() { Expect(cache.ElementCount("group1")).Should(BeNumerically("==", 1)) Expect(cache.ElementCount("group2")).Should(BeNumerically("==", 2)) Expect(cache.Contains("g1", []string{"group1", "group2"})).Should(BeEmpty()) - Expect(cache.Contains("newString", []string{"group1", "group2"})).Should(ConsistOf("group1")) - Expect(cache.Contains("g2", []string{"group1", "group2"})).Should(ConsistOf("group2")) - Expect(cache.Contains("both", []string{"group1", "group2"})).Should(ConsistOf("group2")) + Expect(cache.Contains("newString", []string{"group1", "group2"})). + Should(Equal(map[string]string{"group1": "newstring"})) // rule is normalized to lower-case + Expect(cache.Contains("g2", []string{"group1", "group2"})). + Should(Equal(map[string]string{"group2": "g2"})) + Expect(cache.Contains("both", []string{"group1", "group2"})). + Should(Equal(map[string]string{"group2": "both"})) }) }) }) diff --git a/cache/stringcache/string_caches.go b/cache/stringcache/string_caches.go index cc5738d6..1a170ee1 100644 --- a/cache/stringcache/string_caches.go +++ b/cache/stringcache/string_caches.go @@ -12,7 +12,9 @@ import ( type stringCache interface { elementCount() int - contains(searchString string) bool + // findMatch reports whether the cache matches searchString and, if so, + // returns the rule that matched. rule is empty when ok is false. + findMatch(searchString string) (rule string, ok bool) } type cacheFactory interface { @@ -37,12 +39,12 @@ func (cache stringMap) elementCount() int { return count } -func (cache stringMap) contains(searchString string) bool { +func (cache stringMap) findMatch(searchString string) (string, bool) { normalized := normalizeEntry(searchString) searchLen := len(normalized) if searchLen == 0 { - return false + return "", false } searchBucketLen := len(cache[searchLen]) / searchLen @@ -55,11 +57,11 @@ func (cache stringMap) contains(searchString string) bool { if blockRule == normalized { log.PrefixedLog("string_map").Debugf("block rule '%s' matched with '%s'", blockRule, searchString) - return true + return blockRule, true } } - return false + return "", false } type stringCacheFactory struct { @@ -126,16 +128,18 @@ func (cache regexCache) elementCount() int { return len(cache) } -func (cache regexCache) contains(searchString string) bool { +func (cache regexCache) findMatch(searchString string) (string, bool) { for _, regex := range cache { if regex.MatchString(searchString) { log.PrefixedLog("regex_cache").Debugf("regex '%s' matched with '%s'", regex, searchString) - return true + // re-wrap in the '/.../' delimiters that addEntry strips on insertion + // so the reported rule matches the entry as configured by the user. + return "/" + regex.String() + "/", true } } - return false + return "", false } type regexCacheFactory struct { @@ -189,8 +193,22 @@ func (cache wildcardCache) elementCount() int { return cache.cnt } -func (cache wildcardCache) contains(domain string) bool { - return cache.trie.HasParentOf(domain) +func (cache wildcardCache) findMatch(domain string) (string, bool) { + labels, ok := cache.trie.HasParentOf(domain) + if !ok { + return "", false + } + + // labels reconstruct the stored wildcard base (normalized, with the "*." + // prefix stripped on insertion); re-prepend "*." so the reported rule + // matches the entry as configured by the user. trie.JoinTLD pairs with the + // trie.SplitTLD this cache is built with, so the separator stays the trie's + // concern rather than being hard-coded here. + rule := "*." + trie.JoinTLD(labels) + + log.PrefixedLog("wildcard_cache").Debugf("wildcard block rule '%s' matched with '%s'", rule, domain) + + return rule, true } type wildcardCacheFactory struct { diff --git a/cache/stringcache/string_caches_benchmark_test.go b/cache/stringcache/string_caches_benchmark_test.go index c088806f..139811fe 100644 --- a/cache/stringcache/string_caches_benchmark_test.go +++ b/cache/stringcache/string_caches_benchmark_test.go @@ -224,7 +224,7 @@ func benchmarkCache(b *testing.B, data []string, newFactory func() cacheFactory) // - wildcards and regexes need a plain string query // - all benchmarks will do the same number of queries for _, s := range stringTestData { - if !cache.contains(s) { + if _, ok := cache.findMatch(s); !ok { b.Fatalf("cache is missing value from stringTestData: %s", s) } } diff --git a/cache/stringcache/string_caches_test.go b/cache/stringcache/string_caches_test.go index c9b1d41b..e3f3fed7 100644 --- a/cache/stringcache/string_caches_test.go +++ b/cache/stringcache/string_caches_test.go @@ -38,17 +38,33 @@ var _ = Describe("Caches", func() { }) It("should match if StringCache contains exact string", func() { - Expect(cache.contains("apple.com")).Should(BeTrue()) - Expect(cache.contains("google.com")).Should(BeTrue()) - Expect(cache.contains("www.google.com")).Should(BeFalse()) - Expect(cache.contains("")).Should(BeFalse()) + rule, ok := cache.findMatch("apple.com") + Expect(ok).Should(BeTrue()) + Expect(rule).Should(Equal("apple.com")) + + rule, ok = cache.findMatch("google.com") + Expect(ok).Should(BeTrue()) + Expect(rule).Should(Equal("google.com")) + + _, ok = cache.findMatch("www.google.com") + Expect(ok).Should(BeFalse()) + _, ok = cache.findMatch("") + Expect(ok).Should(BeFalse()) }) - It("should match case-insensitive", func() { - Expect(cache.contains("aPPle.com")).Should(BeTrue()) - Expect(cache.contains("google.COM")).Should(BeTrue()) - Expect(cache.contains("www.google.com")).Should(BeFalse()) - Expect(cache.contains("")).Should(BeFalse()) + It("should match case-insensitive and return the normalized rule", func() { + rule, ok := cache.findMatch("aPPle.com") + Expect(ok).Should(BeTrue()) + Expect(rule).Should(Equal("apple.com")) + + rule, ok = cache.findMatch("google.COM") + Expect(ok).Should(BeTrue()) + Expect(rule).Should(Equal("google.com")) + + _, ok = cache.findMatch("www.google.com") + Expect(ok).Should(BeFalse()) + _, ok = cache.findMatch("") + Expect(ok).Should(BeFalse()) }) It("should return correct element count", func() { @@ -77,7 +93,9 @@ var _ = Describe("Caches", func() { It("finds every entry regardless of insertion order", func() { for _, e := range entries { - Expect(cache.contains(e)).Should(BeTrue(), e) + rule, ok := cache.findMatch(e) + Expect(ok).Should(BeTrue(), e) + Expect(rule).Should(Equal(e), e) } }) @@ -117,19 +135,39 @@ var _ = Describe("Caches", func() { cache = factory.create() }) - It("should match if one regex in StringCache matches string", func() { - Expect(cache.contains("google.com")).Should(BeTrue()) - Expect(cache.contains("google.coma")).Should(BeTrue()) - Expect(cache.contains("agoogle.com")).Should(BeTrue()) - Expect(cache.contains("www.google.com")).Should(BeTrue()) - Expect(cache.contains("apple.com")).Should(BeTrue()) - Expect(cache.contains("apple.de")).Should(BeTrue()) - Expect(cache.contains("apple.it")).Should(BeFalse()) - Expect(cache.contains("www.apple.com")).Should(BeFalse()) - Expect(cache.contains("applecom")).Should(BeFalse()) - Expect(cache.contains("www.amazon.com")).Should(BeTrue()) - Expect(cache.contains("amazon.com")).Should(BeTrue()) - Expect(cache.contains("myamazon.com")).Should(BeTrue()) + It("should match if one regex in StringCache matches string and return the pattern", func() { + rule, ok := cache.findMatch("google.com") + Expect(ok).Should(BeTrue()) + Expect(rule).Should(Equal("/.*google.com/")) + + _, ok = cache.findMatch("google.coma") + Expect(ok).Should(BeTrue()) + _, ok = cache.findMatch("agoogle.com") + Expect(ok).Should(BeTrue()) + _, ok = cache.findMatch("www.google.com") + Expect(ok).Should(BeTrue()) + + rule, ok = cache.findMatch("apple.com") + Expect(ok).Should(BeTrue()) + Expect(rule).Should(Equal("/^apple\\.(de|com)$/")) + + _, ok = cache.findMatch("apple.de") + Expect(ok).Should(BeTrue()) + _, ok = cache.findMatch("apple.it") + Expect(ok).Should(BeFalse()) + _, ok = cache.findMatch("www.apple.com") + Expect(ok).Should(BeFalse()) + _, ok = cache.findMatch("applecom") + Expect(ok).Should(BeFalse()) + + rule, ok = cache.findMatch("www.amazon.com") + Expect(ok).Should(BeTrue()) + Expect(rule).Should(Equal("/amazon/")) + + _, ok = cache.findMatch("amazon.com") + Expect(ok).Should(BeTrue()) + _, ok = cache.findMatch("myamazon.com") + Expect(ok).Should(BeTrue()) }) It("should return correct element count", func() { @@ -168,30 +206,51 @@ var _ = Describe("Caches", func() { cache = factory.create() }) - It("should match if one regex in StringCache matches string", func() { + It("should match and return the wildcard rule including the '*.' prefix", func() { // first entry - Expect(cache.contains("example.com")).Should(BeTrue()) - Expect(cache.contains("www.example.com")).Should(BeTrue()) + rule, ok := cache.findMatch("example.com") + Expect(ok).Should(BeTrue()) + Expect(rule).Should(Equal("*.example.com")) + + rule, ok = cache.findMatch("www.example.com") + Expect(ok).Should(BeTrue()) + Expect(rule).Should(Equal("*.example.com")) // look alikes - Expect(cache.contains("com")).Should(BeFalse()) - Expect(cache.contains("example.coma")).Should(BeFalse()) - Expect(cache.contains("an-example.com")).Should(BeFalse()) - Expect(cache.contains("examplecom")).Should(BeFalse()) + _, ok = cache.findMatch("com") + Expect(ok).Should(BeFalse()) + _, ok = cache.findMatch("example.coma") + Expect(ok).Should(BeFalse()) + _, ok = cache.findMatch("an-example.com") + Expect(ok).Should(BeFalse()) + _, ok = cache.findMatch("examplecom") + Expect(ok).Should(BeFalse()) // other entry - Expect(cache.contains("example.org")).Should(BeTrue()) - Expect(cache.contains("www.example.org")).Should(BeTrue()) + rule, ok = cache.findMatch("example.org") + Expect(ok).Should(BeTrue()) + Expect(rule).Should(Equal("*.example.org")) + + _, ok = cache.findMatch("www.example.org") + Expect(ok).Should(BeTrue()) // unrelated - Expect(cache.contains("example.net")).Should(BeFalse()) - Expect(cache.contains("www.example.net")).Should(BeFalse()) + _, ok = cache.findMatch("example.net") + Expect(ok).Should(BeFalse()) + _, ok = cache.findMatch("www.example.net") + Expect(ok).Should(BeFalse()) - // third entry - Expect(cache.contains("blocked")).Should(BeTrue()) - Expect(cache.contains("sub.blocked")).Should(BeTrue()) - Expect(cache.contains("sub.sub.blocked")).Should(BeTrue()) - Expect(cache.contains("example.blocked")).Should(BeTrue()) + // third entry (single label) + rule, ok = cache.findMatch("blocked") + Expect(ok).Should(BeTrue()) + Expect(rule).Should(Equal("*.blocked")) + + _, ok = cache.findMatch("sub.blocked") + Expect(ok).Should(BeTrue()) + _, ok = cache.findMatch("sub.sub.blocked") + Expect(ok).Should(BeTrue()) + _, ok = cache.findMatch("example.blocked") + Expect(ok).Should(BeTrue()) }) It("should return correct element count", func() { diff --git a/docs/configuration.md b/docs/configuration.md index afe0bab0..5da00e11 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1015,7 +1015,9 @@ You can choose which information from processed DNS request and response should - `clientIP`: origin IP address from the request - `clientName`: resolved client name(s) from the origins request -- `responseReason`: reason for the response (e.g. from which upstream resolver), response type and code +- `responseReason`: reason for the response (e.g. from which upstream resolver), response type and code. For blocked + queries the reason also names the matched rule per group, e.g. `BLOCKED (ads: *.docler.com)`, so you can tell which + denylist entry caused the block - `responseAnswer`: returned DNS answer - `question`: DNS question from the request - `duration`: request processing time in milliseconds @@ -1128,6 +1130,10 @@ Configuration parameters: DNS responses can be extended with EDE codes according to [RFC8914](https://datatracker.ietf.org/doc/rfc8914/). +For blocked queries the EDE extra text carries the same reason as the query log, including the matched group and rule +(e.g. `BLOCKED CNAME (ads: *.docler.com)`), so a client that requests EDE information learns which denylist entry caused +the block. + Configuration parameters: | Parameter | Type | Mandatory | Default value | Description | diff --git a/lists/list_cache.go b/lists/list_cache.go index 27c1ce2f..97573f0d 100644 --- a/lists/list_cache.go +++ b/lists/list_cache.go @@ -36,8 +36,10 @@ type ListCacheType int // Matcher checks if a domain is in a list type Matcher interface { - // Match matches passed domain name against cached list entries - Match(domain string, groupsToCheck []string) (groups []string) + // Match matches passed domain name against cached list entries. + // Returns a map of matching group -> the rule that matched, or nil/empty + // when the domain is not in any of the groups. + Match(domain string, groupsToCheck []string) (matches map[string]string) } // ListCache generic cache of strings divided in groups @@ -112,8 +114,10 @@ func logger() *logrus.Entry { return log.PrefixedLog("list_cache") } -// Match matches passed domain name against cached list entries -func (b *ListCache) Match(domain string, groupsToCheck []string) (groups []string) { +// Match matches passed domain name against cached list entries. +// Returns a map of matching group -> the rule that matched, or nil/empty when +// the domain is not in any of the groups. +func (b *ListCache) Match(domain string, groupsToCheck []string) (matches map[string]string) { return b.groupedCache.Contains(domain, groupsToCheck) } diff --git a/lists/list_cache_test.go b/lists/list_cache_test.go index b5f756dc..27be98ad 100644 --- a/lists/list_cache_test.go +++ b/lists/list_cache_test.go @@ -125,7 +125,7 @@ var _ = Describe("ListCache", func() { It("should delete existing elements from group cache", func(ctx context.Context) { group := sut.Match("blocked1.com", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) err := sut.refresh(ctx) Expect(err).Should(Succeed()) @@ -149,10 +149,10 @@ var _ = Describe("ListCache", func() { It("should still other domains", func() { group := sut.Match("inlinedomain1.com", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) group = sut.Match("inlinedomain2.com", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) }) }) When("a temporary/transient err occurs on download", func() { @@ -178,7 +178,7 @@ var _ = Describe("ListCache", func() { By("Lists loaded without timeout", func() { Eventually(func(g Gomega) { group := sut.Match("blocked1.com", []string{"gr1"}) - g.Expect(group).Should(ContainElement("gr1")) + g.Expect(group).Should(HaveKey("gr1")) }, "1s").Should(Succeed()) }) @@ -186,14 +186,14 @@ var _ = Describe("ListCache", func() { By("List couldn't be loaded due to timeout", func() { group := sut.Match("blocked1.com", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) }) _ = sut.Refresh(ctx) By("List couldn't be loaded due to timeout", func() { group := sut.Match("blocked1.com", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) }) }) }) @@ -216,14 +216,14 @@ var _ = Describe("ListCache", func() { It("should keep existing elements from group cache", func(ctx context.Context) { By("Lists loaded without err", func() { group := sut.Match("blocked1.com", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) }) Expect(sut.refresh(ctx)).Should(HaveOccurred()) By("Lists from first load is kept", func() { group := sut.Match("blocked1.com", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) }) }) }) @@ -237,13 +237,13 @@ var _ = Describe("ListCache", func() { It("should download the list and match against", func() { group := sut.Match("blocked1.com", []string{"gr1", "gr2"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) group = sut.Match("blocked1a.com", []string{"gr1", "gr2"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) group = sut.Match("blocked1a.com", []string{"gr2"}) - Expect(group).Should(ContainElement("gr2")) + Expect(group).Should(HaveKey("gr2")) }) }) When("Configuration has some faulty urls", func() { @@ -256,13 +256,13 @@ var _ = Describe("ListCache", func() { It("should download the list and match against", func() { group := sut.Match("blocked1.com", []string{"gr1", "gr2"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) group = sut.Match("blocked1a.com", []string{"gr1", "gr2"}) - Expect(group).Should(ContainElements("gr1", "gr2")) + Expect(group).Should(SatisfyAll(HaveKey("gr1"), HaveKey("gr2"))) group = sut.Match("blocked1a.com", []string{"gr2"}) - Expect(group).Should(ContainElement("gr2")) + Expect(group).Should(HaveKey("gr2")) }) }) When("List will be updated", func() { @@ -297,13 +297,13 @@ var _ = Describe("ListCache", func() { Expect(sut.groupedCache.ElementCount("gr2")).Should(Equal(2)) group := sut.Match("blocked1.com", []string{"gr1", "gr2"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) group = sut.Match("blocked1a.com", []string{"gr1", "gr2"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) group = sut.Match("blocked1a.com", []string{"gr2"}) - Expect(group).Should(ContainElement("gr2")) + Expect(group).Should(HaveKey("gr2")) }) }) When("group with bigger files", func() { @@ -340,10 +340,10 @@ var _ = Describe("ListCache", func() { It("should match", func() { Expect(sut.groupedCache.ElementCount("gr1")).Should(Equal(2)) group := sut.Match("inlinedomain1.com", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) group = sut.Match("inlinedomain2.com", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) }) }) When("Text file can't be parsed", func() { @@ -360,7 +360,7 @@ var _ = Describe("ListCache", func() { It("should still match already imported strings", func() { group := sut.Match("inlinedomain1.com", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) }) }) When("Text file has too many errors", func() { @@ -387,7 +387,7 @@ var _ = Describe("ListCache", func() { It("should still parse the domain", func() { group := sut.Match("inlinedomain1.com", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) }) }) When("inline regex content is defined", func() { @@ -399,10 +399,10 @@ var _ = Describe("ListCache", func() { It("should match", func() { group := sut.Match("apple.com", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) group = sut.Match("apple.de", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) }) }) }) @@ -483,10 +483,10 @@ var _ = Describe("ListCache", func() { // Verify the successful source's entries are available group := sut.Match("blocked1.com", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1"), "entries from successful source should be available") + Expect(group).Should(HaveKey("gr1"), "entries from successful source should be available") group = sut.Match("blocked2.com", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1"), "entries from successful source should be available") + Expect(group).Should(HaveKey("gr1"), "entries from successful source should be available") }) }) @@ -515,7 +515,7 @@ var _ = Describe("ListCache", func() { Expect(sut).ShouldNot(BeNil()) group := sut.Match("blocked1.com", []string{"gr1"}) - Expect(group).Should(ContainElement("gr1")) + Expect(group).Should(HaveKey("gr1")) }) }) }) @@ -545,7 +545,7 @@ var _ = Describe("ListCache.seedFromDisk", func() { sut.seedFromDisk(ctx) - Expect(sut.Match("seeded.com", []string{"ads"})).Should(ContainElement("ads")) + Expect(sut.Match("seeded.com", []string{"ads"})).Should(HaveKey("ads")) }) It("also seeds inline and file sources that share a group with an HTTP source", func(ctx context.Context) { @@ -578,9 +578,9 @@ var _ = Describe("ListCache.seedFromDisk", func() { sut.seedFromDisk(ctx) - Expect(sut.Match("http-seeded.com", []string{"ads"})).Should(ContainElement("ads")) - Expect(sut.Match("file-seeded.com", []string{"ads"})).Should(ContainElement("ads")) - Expect(sut.Match("inline-seeded.com", []string{"ads"})).Should(ContainElement("ads")) + Expect(sut.Match("http-seeded.com", []string{"ads"})).Should(HaveKey("ads")) + Expect(sut.Match("file-seeded.com", []string{"ads"})).Should(HaveKey("ads")) + Expect(sut.Match("inline-seeded.com", []string{"ads"})).Should(HaveKey("ads")) }) }) diff --git a/resolver/blocking_resolver.go b/resolver/blocking_resolver.go index efd35e2c..a6ba089f 100644 --- a/resolver/blocking_resolver.go +++ b/resolver/blocking_resolver.go @@ -392,8 +392,8 @@ func (r *BlockingResolver) handleDenylist(ctx context.Context, groupsToCheck []s domain := util.ExtractDomain(question) logger := logger.WithField(logFieldDomain, domain) - if groups := r.matches(groupsToCheck, r.allowlistMatcher, domain); len(groups) > 0 { - logger.WithField("groups", groups).Debugf("domain is allowlisted") + if matches := r.matches(groupsToCheck, r.allowlistMatcher, domain); len(matches) > 0 { + logger.WithField("matches", matches).Debug("domain is allowlisted") resp, err := r.next.Resolve(ctx, request) if err != nil { @@ -412,8 +412,8 @@ func (r *BlockingResolver) handleDenylist(ctx context.Context, groupsToCheck []s return true, resp, err } - if groups := r.matches(groupsToCheck, r.denylistMatcher, domain); len(groups) > 0 { - resp, err := r.handleBlocked(logger, request, question, fmt.Sprintf("BLOCKED (%s)", strings.Join(groups, ","))) + if matches := r.matches(groupsToCheck, r.denylistMatcher, domain); len(matches) > 0 { + resp, err := r.handleBlocked(logger, request, question, formatBlockReason(matches, "")) return true, resp, err } @@ -442,11 +442,10 @@ func (r *BlockingResolver) Resolve(ctx context.Context, request *model.Request) if len(entryToCheck) > 0 { logger := logger.WithField("response_entry", entryToCheck) - if groups := r.matches(groupsToCheck, r.allowlistMatcher, entryToCheck); len(groups) > 0 { - logger.WithField("groups", groups).Debugf("%s is allowlisted", tName) - } else if groups := r.matches(groupsToCheck, r.denylistMatcher, entryToCheck); len(groups) > 0 { - return r.handleBlocked(logger, request, request.Req.Question[0], fmt.Sprintf("BLOCKED %s (%s)", tName, - strings.Join(groups, ","))) + if matches := r.matches(groupsToCheck, r.allowlistMatcher, entryToCheck); len(matches) > 0 { + logger.WithField("matches", matches).Debugf("%s is allowlisted", tName) + } else if matches := r.matches(groupsToCheck, r.denylistMatcher, entryToCheck); len(matches) > 0 { + return r.handleBlocked(logger, request, request.Req.Question[0], formatBlockReason(matches, tName)) } } } @@ -564,10 +563,31 @@ func (r *BlockingResolver) collectGroupsForClient(request *model.Request) []sche func (r *BlockingResolver) matches(groupsToCheck []string, m lists.Matcher, domain string, -) []string { +) map[string]string { return m.Match(domain, groupsToCheck) } +// formatBlockReason renders a block reason of the form +// "BLOCKED[ TYPE] (group: rule[, group: rule...])". Matched groups are sorted +// so the rendered reason is deterministic when more than one group matches. +func formatBlockReason(matches map[string]string, typeName string) string { + reason := "BLOCKED" + if typeName != "" { + reason += " " + typeName + } + + if len(matches) == 0 { + return reason + } + + entries := make([]string, 0, len(matches)) + for _, group := range slices.Sorted(maps.Keys(matches)) { + entries = append(entries, fmt.Sprintf("%s: %s", group, matches[group])) + } + + return fmt.Sprintf("%s (%s)", reason, strings.Join(entries, ", ")) +} + type blockHandler interface { handleBlock(question dns.Question, response *dns.Msg) } diff --git a/resolver/blocking_resolver_test.go b/resolver/blocking_resolver_test.go index 6b39fc44..fe64b04d 100644 --- a/resolver/blocking_resolver_test.go +++ b/resolver/blocking_resolver_test.go @@ -220,7 +220,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { BeDNSRecord("domain1.com.", A, "0.0.0.0"), HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (gr1)"), + HaveReason("BLOCKED (gr1: domain1.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -231,7 +231,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { BeDNSRecord("domain1.com.", A, "0.0.0.0"), HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (gr1)"), + HaveReason("BLOCKED (gr1: domain1.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -242,7 +242,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { BeDNSRecord("domain1.com.", A, "0.0.0.0"), HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (gr1)"), + HaveReason("BLOCKED (gr1: domain1.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -253,7 +253,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { BeDNSRecord("blocked2.com.", A, "0.0.0.0"), HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (gr2)"), + HaveReason("BLOCKED (gr2: blocked2.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -264,7 +264,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { BeDNSRecord("domain1.com.", AAAA, "::"), HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (gr1)"), + HaveReason("BLOCKED (gr1: domain1.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -286,7 +286,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { BeDNSRecord("domain1.com.", A, "0.0.0.0"), HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (gr1)"), + HaveReason("BLOCKED (gr1: domain1.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -326,7 +326,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { BeDNSRecord("domain1.com.", A, "0.0.0.0"), HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (gr1)"), + HaveReason("BLOCKED (gr1: domain1.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -337,7 +337,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { BeDNSRecord("domain1.com.", A, "0.0.0.0"), HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (gr1)"), + HaveReason("BLOCKED (gr1: domain1.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -351,7 +351,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { BeDNSRecord("domain1.com.", A, "0.0.0.0"), HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (gr1)"), + HaveReason("BLOCKED (gr1: domain1.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -362,7 +362,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { BeDNSRecord("blocked2.com.", A, "0.0.0.0"), HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (gr2)"), + HaveReason("BLOCKED (gr2: blocked2.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -375,7 +375,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { BeDNSRecord("domain1.com.", A, "0.0.0.0"), HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (gr1)"), + HaveReason("BLOCKED (gr1: domain1.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -389,7 +389,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { BeDNSRecord("blocked3.com.", A, "0.0.0.0"), HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (defaultGroup)"), + HaveReason("BLOCKED (defaultGroup: blocked3.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -416,7 +416,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { HaveNoAnswer(), HaveResponseType(ResponseTypeBLOCKED), HaveReturnCode(dns.RcodeNameError), - HaveReason("BLOCKED (defaultGroup)"), + HaveReason("BLOCKED (defaultGroup: blocked3.com)"), HaveAuthority(), HaveSOARecord(60, 60), // 1 minute = 60 seconds )) @@ -444,7 +444,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { HaveNoAnswer(), HaveResponseType(ResponseTypeBLOCKED), HaveReturnCode(dns.RcodeNameError), - HaveReason("BLOCKED (defaultGroup)"), + HaveReason("BLOCKED (defaultGroup: blocked3.com)"), HaveAuthority(), HaveSOARecord(7200, 7200), // 2 hours in seconds )) @@ -473,7 +473,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { HaveTTL(BeNumerically("==", 1234)), HaveResponseType(ResponseTypeBLOCKED), HaveReturnCode(dns.RcodeSuccess), - HaveReason("BLOCKED (defaultGroup)"), + HaveReason("BLOCKED (defaultGroup: blocked3.com)"), )) }) @@ -490,7 +490,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { HaveTTL(BeNumerically("==", 1234)), HaveResponseType(ResponseTypeBLOCKED), HaveReturnCode(dns.RcodeSuccess), - HaveReason("BLOCKED (defaultGroup)"), + HaveReason("BLOCKED (defaultGroup: blocked3.com)"), )) }) }) @@ -518,7 +518,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), HaveReturnCode(dns.RcodeSuccess), - HaveReason("BLOCKED (defaultGroup)"), + HaveReason("BLOCKED (defaultGroup: blocked3.com)"), )) }) @@ -530,7 +530,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), HaveReturnCode(dns.RcodeSuccess), - HaveReason("BLOCKED (defaultGroup)"), + HaveReason("BLOCKED (defaultGroup: blocked3.com)"), )) }) }) @@ -557,7 +557,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), HaveReturnCode(dns.RcodeSuccess), - HaveReason("BLOCKED (defaultGroup)"), + HaveReason("BLOCKED (defaultGroup: blocked3.com)"), )) }) }) @@ -576,7 +576,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), HaveReturnCode(dns.RcodeSuccess), - HaveReason("BLOCKED IP (defaultGroup)"), + HaveReason("BLOCKED IP (defaultGroup: 123.145.123.145)"), )) }) }) @@ -596,7 +596,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), HaveReturnCode(dns.RcodeSuccess), - HaveReason("BLOCKED IP (defaultGroup)"), + HaveReason("BLOCKED IP (defaultGroup: 2001:db8:85a3:8d3::370:7344)"), )) }) }) @@ -619,7 +619,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { HaveTTL(BeNumerically("==", 21600)), HaveResponseType(ResponseTypeBLOCKED), HaveReturnCode(dns.RcodeSuccess), - HaveReason("BLOCKED CNAME (defaultGroup)"), + HaveReason("BLOCKED CNAME (defaultGroup: badcnamedomain.com)"), )) }) }) @@ -1114,7 +1114,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { SatisfyAll( BeDNSRecord("blocked3.com.", A, "0.0.0.0"), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (defaultGroup)"), + HaveReason("BLOCKED (defaultGroup: blocked3.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -1125,7 +1125,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { SatisfyAll( BeDNSRecord("domain1.com.", A, "0.0.0.0"), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (group1)"), + HaveReason("BLOCKED (group1: domain1.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -1188,7 +1188,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { SatisfyAll( BeDNSRecord("domain1.com.", A, "0.0.0.0"), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (group1)"), + HaveReason("BLOCKED (group1: domain1.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -1203,7 +1203,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { SatisfyAll( BeDNSRecord("blocked3.com.", A, "0.0.0.0"), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (defaultGroup)"), + HaveReason("BLOCKED (defaultGroup: blocked3.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -1213,7 +1213,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { SatisfyAll( BeDNSRecord("domain1.com.", A, "0.0.0.0"), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (group1)"), + HaveReason("BLOCKED (group1: domain1.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -1269,7 +1269,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { SatisfyAll( BeDNSRecord("blocked3.com.", A, "0.0.0.0"), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (defaultGroup)"), + HaveReason("BLOCKED (defaultGroup: blocked3.com)"), HaveReturnCode(dns.RcodeSuccess), )) @@ -1278,7 +1278,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { SatisfyAll( BeDNSRecord("domain1.com.", A, "0.0.0.0"), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (group1)"), + HaveReason("BLOCKED (group1: domain1.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -1293,7 +1293,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { SatisfyAll( BeDNSRecord("blocked3.com.", A, "0.0.0.0"), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (defaultGroup)"), + HaveReason("BLOCKED (defaultGroup: blocked3.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -1303,7 +1303,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { SatisfyAll( BeDNSRecord("domain1.com.", A, "0.0.0.0"), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (group1)"), + HaveReason("BLOCKED (group1: domain1.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -1326,7 +1326,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { SatisfyAll( BeDNSRecord("blocked3.com.", A, "0.0.0.0"), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (defaultGroup)"), + HaveReason("BLOCKED (defaultGroup: blocked3.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -1357,7 +1357,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { SatisfyAll( BeDNSRecord("blocked3.com.", A, "0.0.0.0"), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (defaultGroup)"), + HaveReason("BLOCKED (defaultGroup: blocked3.com)"), HaveReturnCode(dns.RcodeSuccess), )) @@ -1366,7 +1366,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { SatisfyAll( BeDNSRecord("domain1.com.", A, "0.0.0.0"), HaveResponseType(ResponseTypeBLOCKED), - HaveReason("BLOCKED (group1)"), + HaveReason("BLOCKED (group1: domain1.com)"), HaveReturnCode(dns.RcodeSuccess), )) }) @@ -1459,3 +1459,25 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() { }) }) }) + +var _ = Describe("formatBlockReason", func() { + It("renders the matched group and rule", func() { + Expect(formatBlockReason(map[string]string{"gr1": "domain1.com"}, "")). + Should(Equal("BLOCKED (gr1: domain1.com)")) + }) + + It("includes the entry type when given (e.g. CNAME/IP)", func() { + Expect(formatBlockReason(map[string]string{"ads": "*.docler.com"}, "CNAME")). + Should(Equal("BLOCKED CNAME (ads: *.docler.com)")) + }) + + It("renders multiple matched groups sorted by group for deterministic output", func() { + matches := map[string]string{"zeta": "z.com", "alpha": "a.com", "mid": "m.com"} + + // repeat to guard against Go's randomized map iteration order + for range 20 { + Expect(formatBlockReason(matches, "")). + Should(Equal("BLOCKED (alpha: a.com, mid: m.com, zeta: z.com)")) + } + }) +}) diff --git a/resolver/ede_resolver.go b/resolver/ede_resolver.go index 61798f46..ac3d5057 100644 --- a/resolver/ede_resolver.go +++ b/resolver/ede_resolver.go @@ -2,6 +2,7 @@ package resolver import ( "context" + "unicode/utf8" "github.com/0xERR0R/blocky/config" "github.com/0xERR0R/blocky/model" @@ -9,6 +10,14 @@ import ( "github.com/miekg/dns" ) +// maxEDETextLength bounds the EDE extra text. The reason now embeds the matched +// denylist rule, which is unbounded for regex entries. dns.Msg.Truncate keeps +// the OPT record and subtracts its full length from the size budget, so an +// oversized extra text would shrink (or eliminate) the room left for the actual +// answer on a size-limited (typically UDP) response. Bounding it here keeps the +// OPT small; the full reason is still recorded in the query log. +const maxEDETextLength = 200 + // A EDEResolver is responsible for adding the reason for the response as EDNS0 option type EDEResolver struct { configurable[*config.EDE] @@ -53,7 +62,22 @@ func (r *EDEResolver) addExtraReasoning(res *model.Response) { edeOption := new(dns.EDNS0_EDE) edeOption.InfoCode = infocode - edeOption.ExtraText = res.Reason + edeOption.ExtraText = truncateText(res.Reason, maxEDETextLength) util.SetEdns0Option(res.Res, edeOption) } + +// truncateText limits s to at most maxLen bytes, cutting on a UTF-8 rune +// boundary so it never emits an invalid (partial) rune. +func truncateText(s string, maxLen int) string { + if len(s) <= maxLen { + return s + } + + cut := maxLen + for cut > 0 && !utf8.RuneStart(s[cut]) { + cut-- + } + + return s[:cut] +} diff --git a/resolver/ede_resolver_test.go b/resolver/ede_resolver_test.go index d49a12b8..61f9a121 100644 --- a/resolver/ede_resolver_test.go +++ b/resolver/ede_resolver_test.go @@ -5,6 +5,7 @@ import ( "context" "errors" "math" + "strings" "github.com/0xERR0R/blocky/config" . "github.com/0xERR0R/blocky/helpertest" @@ -111,6 +112,54 @@ var _ = Describe("EdeResolver", func() { )) }) + When("resolver returns a blocked response carrying the matched rule", func() { + BeforeEach(func() { + m = &mockResolver{} + m.On("Resolve", mock.Anything).Return(&Response{ + Res: mockAnswer, + RType: ResponseTypeBLOCKED, + Reason: "BLOCKED CNAME (ads: *.docler.com)", + }, nil) + }) + + It("returns the matched rule to the client in the EDE extra text", func() { + Expect(sut.Resolve(ctx, newRequest("example.com.", A))). + Should( + SatisfyAll( + HaveEdnsOption(dns.EDNS0EDE), + WithTransform(extractEdeOption, + SatisfyAll( + HaveField("InfoCode", Equal(dns.ExtendedErrorCodeBlocked)), + HaveField("ExtraText", Equal("BLOCKED CNAME (ads: *.docler.com)")), + )), + )) + }) + }) + + When("resolver returns a blocked response with an oversized reason", func() { + longReason := "BLOCKED (ads: /" + strings.Repeat("a", 500) + "/)" + + BeforeEach(func() { + m = &mockResolver{} + m.On("Resolve", mock.Anything).Return(&Response{ + Res: mockAnswer, + RType: ResponseTypeBLOCKED, + Reason: longReason, + }, nil) + }) + + It("caps the EDE extra text so it can't bloat the OPT record", func() { + Expect(sut.Resolve(ctx, newRequest("example.com.", A))). + Should( + SatisfyAll( + HaveEdnsOption(dns.EDNS0EDE), + WithTransform(extractEdeOption, + WithTransform(func(o dns.EDNS0_EDE) int { return len(o.ExtraText) }, + BeNumerically("<=", maxEDETextLength))), + )) + }) + }) + When("resolver returns other", func() { BeforeEach(func() { m = &mockResolver{} diff --git a/trie/split.go b/trie/split.go index c33d083c..29559557 100644 --- a/trie/split.go +++ b/trie/split.go @@ -18,3 +18,10 @@ func SplitTLD(domain string) (label, rest string) { return label, rest } + +// JoinTLD is the inverse of SplitTLD: it reconstructs an entry from the labels +// returned by HasParentOf on a trie built with SplitTLD. +// ["example", "com"] -> "example.com" +func JoinTLD(labels []string) string { + return strings.Join(labels, ".") +} diff --git a/trie/split_test.go b/trie/split_test.go index 707a3e6a..eee54480 100644 --- a/trie/split_test.go +++ b/trie/split_test.go @@ -44,3 +44,22 @@ var _ = Describe("SpltTLD", func() { Expect(rest).Should(Equal("www")) }) }) + +var _ = Describe("JoinTLD", func() { + It("reconstructs an entry from labels in entry order", func() { + Expect(JoinTLD([]string{"example", "com"})).Should(Equal("example.com")) + }) + + It("handles a single label", func() { + Expect(JoinTLD([]string{"blocked"})).Should(Equal("blocked")) + }) + + It("reproduces the labels returned by HasParentOf", func() { + sut := NewTrie(SplitTLD) + sut.Insert("sub.example.com") + + labels, ok := sut.HasParentOf("a.sub.example.com") + Expect(ok).Should(BeTrue()) + Expect(JoinTLD(labels)).Should(Equal("sub.example.com")) + }) +}) diff --git a/trie/trie.go b/trie/trie.go index 42036433..7ca098b1 100644 --- a/trie/trie.go +++ b/trie/trie.go @@ -1,11 +1,5 @@ package trie -import ( - "strings" - - "github.com/0xERR0R/blocky/log" -) - // Trie stores a set of strings and can quickly check // if it contains an element, or one of its parents. // @@ -37,12 +31,19 @@ func (t *Trie) Insert(key string) { t.root.insert(key, t.split) } -func (t *Trie) HasParentOf(key string) bool { +// HasParentOf reports whether the trie contains key or one of its parents. +// On a match it also returns the labels of the matched entry; joining them +// with the separator used by the trie's SplitFunc reproduces the entry. +// The labels are nil when there is no match. +func (t *Trie) HasParentOf(key string) ([]string, bool) { + // labels are built only on the matching path (during recursion unwind), + // so a miss does not allocate. They come back in entry order, ready to be + // joined by the caller with the separator matching its SplitFunc. return t.root.hasParentOf(key, t.split) } type node interface { - hasParentOf(key string, split SplitFunc) bool + hasParentOf(key string, split SplitFunc) (labels []string, ok bool) } // We save memory by not keeping track of children of @@ -97,7 +98,7 @@ func (n *parent) insert(key string, split SplitFunc) { continue case terminal: - if child.hasParentOf(rest, split) { + if _, ok := child.hasParentOf(rest, split); ok { // Found a parent/"prefix" in the set return } @@ -113,45 +114,30 @@ func (n *parent) insert(key string, split SplitFunc) { } } -func (n *parent) hasParentOf(key string, split SplitFunc) bool { - searchString := key - rule := "" +func (n *parent) hasParentOf(key string, split SplitFunc) ([]string, bool) { + label, rest := split(key) - for { - label, rest := split(key) - rule = strings.Join([]string{label, rule}, ".") - - child, ok := n.children[label] - if !ok { - return false - } - - switch child := child.(type) { - case *parent: - if len(rest) == 0 { - // The trie only contains children/"suffixes" of the - // key we're searching for - return false - } - - // Continue down the trie - key = rest - n = child - - continue - - case terminal: - // Continue down the trie - matched := child.hasParentOf(rest, split) - if matched { - rule = strings.Join([]string{child.String(), rule}, ".") - rule = strings.Trim(rule, ".") - log.PrefixedLog("trie").Debugf("wildcard block rule '%s' matched with '%s'", rule, searchString) - } - - return matched - } + child, ok := n.children[label] + if !ok { + return nil, false } + + // A *parent child means the trie only stores longer entries/"suffixes" below + // this node; if the search key has no more labels, none of them can be a + // parent of it. + if _, isParent := child.(*parent); isParent && len(rest) == 0 { + return nil, false + } + + labels, ok := child.hasParentOf(rest, split) + if !ok { + return nil, false + } + + // On the matching path only: prepend-by-append this node's label. The deeper + // labels were collected first, so appending the current (more significant) + // label keeps the entry's natural order. + return append(labels, label), true } type terminal string @@ -160,29 +146,41 @@ func (t terminal) String() string { return string(t) } -func (t terminal) hasParentOf(searchKey string, split SplitFunc) bool { +func (t terminal) hasParentOf(searchKey string, split SplitFunc) ([]string, bool) { tKey := t.String() if tKey == "" { - return true + return nil, true } + // labels of this terminal, collected only while it keeps matching so a + // mismatch stays allocation-free. + var labels []string + for { tLabel, tRest := split(tKey) searchLabel, searchRest := split(searchKey) if searchLabel != tLabel { - return false + return nil, false } + labels = append(labels, tLabel) + if len(tRest) == 0 { - // Found a parent/"prefix" in the set - return true + // Found a parent/"prefix" in the set. labels are in peel order + // (most significant last); reverse them into entry order so the + // caller's parent nodes can append their labels after these. + for i, j := 0, len(labels)-1; i < j; i, j = i+1, j-1 { + labels[i], labels[j] = labels[j], labels[i] + } + + return labels, true } if len(searchRest) == 0 { // The trie only contains children/"suffixes" of the // key we're searching for - return false + return nil, false } // Continue down the trie diff --git a/trie/trie_test.go b/trie/trie_test.go index 3a2ce085..bcf2a992 100644 --- a/trie/trie_test.go +++ b/trie/trie_test.go @@ -1,6 +1,8 @@ package trie import ( + "strings" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -19,12 +21,14 @@ var _ = Describe("Trie", func() { }) It("should not find domains", func() { - Expect(sut.HasParentOf("example.com")).Should(BeFalse()) + _, ok := sut.HasParentOf("example.com") + Expect(ok).Should(BeFalse()) }) It("should not insert the empty string", func() { sut.Insert("") - Expect(sut.HasParentOf("")).Should(BeFalse()) + _, ok := sut.HasParentOf("") + Expect(ok).Should(BeFalse()) }) }) @@ -37,13 +41,16 @@ var _ = Describe("Trie", func() { ) BeforeEach(func() { - Expect(sut.HasParentOf(domainOk)).Should(BeFalse()) + _, ok := sut.HasParentOf(domainOk) + Expect(ok).Should(BeFalse()) sut.Insert(domainOk) - Expect(sut.HasParentOf(domainOk)).Should(BeTrue()) + _, ok = sut.HasParentOf(domainOk) + Expect(ok).Should(BeTrue()) }) AfterEach(func() { - Expect(sut.HasParentOf(domainOk)).Should(BeTrue()) + _, ok := sut.HasParentOf(domainOk) + Expect(ok).Should(BeTrue()) }) It("should be found", func() {}) @@ -51,41 +58,49 @@ var _ = Describe("Trie", func() { It("should contain subdomains", func() { subdomain := "www." + domainOk - Expect(sut.HasParentOf(subdomain)).Should(BeTrue()) + _, ok := sut.HasParentOf(subdomain) + Expect(ok).Should(BeTrue()) }) It("should support inserting subdomains", func() { subdomain := "www." + domainOk - Expect(sut.HasParentOf(subdomain)).Should(BeTrue()) + _, ok := sut.HasParentOf(subdomain) + Expect(ok).Should(BeTrue()) sut.Insert(subdomain) - Expect(sut.HasParentOf(subdomain)).Should(BeTrue()) + _, ok = sut.HasParentOf(subdomain) + Expect(ok).Should(BeTrue()) }) It("should not find unrelated", func() { - Expect(sut.HasParentOf(domainKo)).Should(BeFalse()) + _, ok := sut.HasParentOf(domainKo) + Expect(ok).Should(BeFalse()) }) It("should not find uninserted parent", func() { - Expect(sut.HasParentOf(domainOkTLD)).Should(BeFalse()) + _, ok := sut.HasParentOf(domainOkTLD) + Expect(ok).Should(BeFalse()) }) It("should not find deep uninserted parent", func() { sut.Insert("sub.sub.sub.test") - Expect(sut.HasParentOf("sub.sub.test")).Should(BeFalse()) + _, ok := sut.HasParentOf("sub.sub.test") + Expect(ok).Should(BeFalse()) }) It("should find inserted parent", func() { sut.Insert(domainOkTLD) - Expect(sut.HasParentOf(domainOkTLD)).Should(BeTrue()) + _, ok := sut.HasParentOf(domainOkTLD) + Expect(ok).Should(BeTrue()) }) It("should insert sibling", func() { sibling := "other." + domainOkTLD sut.Insert(sibling) - Expect(sut.HasParentOf(sibling)).Should(BeTrue()) + _, ok := sut.HasParentOf(sibling) + Expect(ok).Should(BeTrue()) }) It("should insert grand-children siblings", func() { @@ -94,15 +109,70 @@ var _ = Describe("Trie", func() { xyzSub := "xyz." + base sut.Insert(abcSub) - Expect(sut.HasParentOf(abcSub)).Should(BeTrue()) - Expect(sut.HasParentOf(xyzSub)).Should(BeFalse()) - Expect(sut.HasParentOf(base)).Should(BeFalse()) + _, ok := sut.HasParentOf(abcSub) + Expect(ok).Should(BeTrue()) + _, ok = sut.HasParentOf(xyzSub) + Expect(ok).Should(BeFalse()) + _, ok = sut.HasParentOf(base) + Expect(ok).Should(BeFalse()) sut.Insert(xyzSub) - Expect(sut.HasParentOf(xyzSub)).Should(BeTrue()) - Expect(sut.HasParentOf(abcSub)).Should(BeTrue()) - Expect(sut.HasParentOf(base)).Should(BeFalse()) + _, ok = sut.HasParentOf(xyzSub) + Expect(ok).Should(BeTrue()) + _, ok = sut.HasParentOf(abcSub) + Expect(ok).Should(BeTrue()) + _, ok = sut.HasParentOf(base) + Expect(ok).Should(BeFalse()) }) }) }) + + Describe("Matched entry reconstruction", func() { + // HasParentOf returns the labels of the matched stored entry; joining + // them with the domain separator must reproduce the original entry. + reconstruct := func(labels []string) string { + return strings.Join(labels, ".") + } + + It("reconstructs a single-label-prefix entry from a subdomain match", func() { + sut.Insert("example.com") + + labels, ok := sut.HasParentOf("www.example.com") + Expect(ok).Should(BeTrue()) + Expect(reconstruct(labels)).Should(Equal("example.com")) + }) + + It("reconstructs the entry on an exact match", func() { + sut.Insert("example.com") + + labels, ok := sut.HasParentOf("example.com") + Expect(ok).Should(BeTrue()) + Expect(reconstruct(labels)).Should(Equal("example.com")) + }) + + It("reconstructs a multi-label entry", func() { + sut.Insert("sub.example.com") + + labels, ok := sut.HasParentOf("a.sub.example.com") + Expect(ok).Should(BeTrue()) + Expect(reconstruct(labels)).Should(Equal("sub.example.com")) + }) + + It("reconstructs the matched sibling under a shared parent node", func() { + sut.Insert("a.com") + sut.Insert("b.com") + + labels, ok := sut.HasParentOf("x.a.com") + Expect(ok).Should(BeTrue()) + Expect(reconstruct(labels)).Should(Equal("a.com")) + }) + + It("returns nil labels when there is no match", func() { + sut.Insert("example.com") + + labels, ok := sut.HasParentOf("example.org") + Expect(ok).Should(BeFalse()) + Expect(labels).Should(BeNil()) + }) + }) })