From c39c921e61f8a3eeb36ee845035252a63b9d950c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 10 Jan 2026 12:48:27 +0000 Subject: [PATCH 1/7] Initial plan From b66f114f3c44dbb205799de8f33203e007690bcc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 10 Jan 2026 12:53:57 +0000 Subject: [PATCH 2/7] Implement per-response-code DNSBL scoring with custom messages Co-authored-by: thisisjaymehta <31812582+thisisjaymehta@users.noreply.github.com> --- docs/reference/checks/dnsbl.md | 76 ++++++++ internal/check/dnsbl/common.go | 58 +++++- internal/check/dnsbl/dnsbl.go | 87 ++++++++- internal/check/dnsbl/dnsbl_test.go | 280 +++++++++++++++++++++++++++++ 4 files changed, 496 insertions(+), 5 deletions(-) diff --git a/docs/reference/checks/dnsbl.md b/docs/reference/checks/dnsbl.md index a2d27362..be31a67f 100644 --- a/docs/reference/checks/dnsbl.md +++ b/docs/reference/checks/dnsbl.md @@ -29,6 +29,30 @@ check.dnsbl { mailfrom yes score 1 } + + # Example with per-response-code scoring (new in 0.8) + zen.spamhaus.org { + client_ipv4 yes + client_ipv6 yes + + # SBL - Spamhaus Block List (known spam sources) + response 127.0.0.2 127.0.0.3 { + score 10 + message "Listed in Spamhaus SBL. See https://check.spamhaus.org/" + } + + # XBL - Exploits Block List (compromised hosts) + response 127.0.0.4 127.0.0.5 127.0.0.6 127.0.0.7 { + score 10 + message "Listed in Spamhaus XBL. See https://check.spamhaus.org/" + } + + # PBL - Policy Block List (dynamic IPs) + response 127.0.0.10 127.0.0.11 { + score 5 + message "Listed in Spamhaus PBL. See https://check.spamhaus.org/" + } + } } ``` @@ -171,3 +195,55 @@ will be rejected. It is possible to specify a negative value to make list act like a whitelist and override results of other blocklists. + +**Note:** When using `response` blocks (see below), the score from matching response +rules is used instead of this flat score value. + +--- + +### response _ip..._ + +**New in 0.8** + +Defines per-response-code rules for scoring and custom messages. This is useful +for combined DNSBLs like Spamhaus ZEN that return different codes for different +listing types. + +Each `response` block takes one or more IP addresses or CIDR ranges as arguments +and contains the following directives: + +#### score _integer_ +**Required** + +Score to add when this response code is returned. If multiple response codes +are returned by the DNSBL, scores are summed together. + +#### message _string_ +**Optional** + +Custom rejection or quarantine message to include when this response code +matches. This message is shown to the client or logged when the threshold +is reached. + +**Example:** + +``` +zen.spamhaus.org { + client_ipv4 yes + + # High severity - known spam sources + response 127.0.0.2 127.0.0.3 { + score 10 + message "Listed in Spamhaus SBL" + } + + # Lower severity - dynamic IPs + response 127.0.0.10 127.0.0.11 { + score 5 + message "Listed in Spamhaus PBL" + } +} +``` + +**Backwards compatibility:** When `response` blocks are not used, the legacy +`responses` and `score` directives work as before. diff --git a/internal/check/dnsbl/common.go b/internal/check/dnsbl/common.go index 7b874cb9..2f317097 100644 --- a/internal/check/dnsbl/common.go +++ b/internal/check/dnsbl/common.go @@ -32,9 +32,15 @@ type ListedErr struct { Identity string List string Reason string + Score int + Message string } func (le ListedErr) Fields() map[string]interface{} { + msg := "Client identity listed in the used DNSBL" + if le.Message != "" { + msg = le.Message + } return map[string]interface{}{ "check": "dnsbl", "list": le.List, @@ -42,7 +48,7 @@ func (le ListedErr) Fields() map[string]interface{} { "reason": le.Reason, "smtp_code": 554, "smtp_enchcode": exterrors.EnhancedCode{5, 7, 0}, - "smtp_msg": "Client identity listed in the used DNSBL", + "smtp_msg": msg, } } @@ -113,6 +119,56 @@ func checkIP(ctx context.Context, resolver dns.Resolver, cfg List, ip net.IP) er return err } + // If ResponseRules is configured, use new behavior + if len(cfg.ResponseRules) > 0 { + totalScore := 0 + var matchedMessages []string + var matchedReasons []string + + for _, addr := range addrs { + for _, rule := range cfg.ResponseRules { + for _, respNet := range rule.Networks { + if respNet.Contains(addr.IP) { + totalScore += rule.Score + if rule.Message != "" { + matchedMessages = append(matchedMessages, rule.Message) + } + matchedReasons = append(matchedReasons, addr.IP.String()) + break // Only match once per rule + } + } + } + } + + if totalScore == 0 { + return nil + } + + // Attempt to extract explanation string from TXT records + txts, err := resolver.LookupTXT(ctx, query) + var reason string + if err == nil && len(txts) > 0 { + reason = strings.Join(txts, "; ") + } else { + reason = strings.Join(matchedReasons, "; ") + } + + // Use first matched message if available + message := "" + if len(matchedMessages) > 0 { + message = matchedMessages[0] + } + + return ListedErr{ + Identity: ip.String(), + List: cfg.Zone, + Reason: reason, + Score: totalScore, + Message: message, + } + } + + // Legacy behavior: use flat Responses filter filteredAddrs := make([]net.IPAddr, 0, len(addrs)) addrsLoop: for _, addr := range addrs { diff --git a/internal/check/dnsbl/dnsbl.go b/internal/check/dnsbl/dnsbl.go index 2c91c838..fff87c37 100644 --- a/internal/check/dnsbl/dnsbl.go +++ b/internal/check/dnsbl/dnsbl.go @@ -38,6 +38,12 @@ import ( "golang.org/x/sync/errgroup" ) +type ResponseRule struct { + Networks []net.IPNet + Score int + Message string +} + type List struct { Zone string @@ -49,6 +55,8 @@ type List struct { ScoreAdj int Responses []net.IPNet + + ResponseRules []ResponseRule } var defaultBL = List{ @@ -126,7 +134,9 @@ func (bl *DNSBL) readListCfg(node config.Node) error { cfg.Bool("mailfrom", false, defaultBL.EHLO, &listCfg.MAILFROM) cfg.Int("score", false, false, 1, &listCfg.ScoreAdj) cfg.StringList("responses", false, false, []string{"127.0.0.1/24"}, &responseNets) - if _, err := cfg.Process(); err != nil { + cfg.AllowUnknown() + unknown, err := cfg.Process() + if err != nil { return err } @@ -144,6 +154,19 @@ func (bl *DNSBL) readListCfg(node config.Node) error { listCfg.Responses = append(listCfg.Responses, *ipNet) } + // Parse response blocks + for _, child := range unknown { + if child.Name == "response" { + rule, err := parseResponseRule(child) + if err != nil { + return err + } + listCfg.ResponseRules = append(listCfg.ResponseRules, rule) + } else { + return config.NodeErr(child, "unknown directive: %s", child.Name) + } + } + for _, zone := range append([]string{node.Name}, node.Args...) { zoneCfg := listCfg zoneCfg.Zone = zone @@ -173,6 +196,44 @@ func (bl *DNSBL) readListCfg(node config.Node) error { return nil } +func parseResponseRule(node config.Node) (ResponseRule, error) { + var rule ResponseRule + + if len(node.Args) == 0 { + return rule, config.NodeErr(node, "response block requires at least one IP address or CIDR as argument") + } + + // Parse IP addresses/CIDRs from arguments + for _, arg := range node.Args { + // If there is no / - it is a plain IP address, append '/32' or '/128' + resp := arg + if !strings.Contains(resp, "/") { + // Check if it's IPv6 to determine the mask + if strings.Contains(resp, ":") { + resp += "/128" + } else { + resp += "/32" + } + } + + _, ipNet, err := net.ParseCIDR(resp) + if err != nil { + return rule, config.NodeErr(node, "invalid IP address or CIDR: %s: %v", arg, err) + } + rule.Networks = append(rule.Networks, *ipNet) + } + + // Parse directives within the response block + cfg := config.NewMap(nil, node) + cfg.Int("score", false, true, 0, &rule.Score) + cfg.String("message", false, false, "", &rule.Message) + if _, err := cfg.Process(); err != nil { + return rule, err + } + + return rule, nil +} + func (bl *DNSBL) testList(listCfg List) { // Check RFC 5782 Section 5 requirements. @@ -298,6 +359,7 @@ func (bl *DNSBL) checkLists(ctx context.Context, ip net.IP, ehlo, mailFrom strin score int listedOn []string reasons []string + messages []string ) for _, list := range bl.bls { @@ -313,7 +375,18 @@ func (bl *DNSBL) checkLists(ctx context.Context, ip net.IP, ehlo, mailFrom strin defer lck.Unlock() listedOn = append(listedOn, listErr.List) reasons = append(reasons, listErr.Reason) - score += list.ScoreAdj + + // Use score from ListedErr if set (new behavior), otherwise use legacy ScoreAdj + if listErr.Score != 0 { + score += listErr.Score + } else { + score += list.ScoreAdj + } + + // Collect custom messages if available + if listErr.Message != "" { + messages = append(messages, listErr.Message) + } } return nil }) @@ -334,13 +407,19 @@ func (bl *DNSBL) checkLists(ctx context.Context, ip net.IP, ehlo, mailFrom strin } } + // Use custom message if available, otherwise use default + message := "Client identity is listed in the used DNSBL" + if len(messages) > 0 { + message = strings.Join(messages, "; ") + } + if score >= bl.rejectThres { return module.CheckResult{ Reject: true, Reason: &exterrors.SMTPError{ Code: 554, EnhancedCode: exterrors.EnhancedCode{5, 7, 0}, - Message: "Client identity is listed in the used DNSBL", + Message: message, Err: err, CheckName: "dnsbl", }, @@ -352,7 +431,7 @@ func (bl *DNSBL) checkLists(ctx context.Context, ip net.IP, ehlo, mailFrom strin Reason: &exterrors.SMTPError{ Code: 554, EnhancedCode: exterrors.EnhancedCode{5, 7, 0}, - Message: "Client identity is listed in the used DNSBL", + Message: message, Err: err, CheckName: "dnsbl", }, diff --git a/internal/check/dnsbl/dnsbl_test.go b/internal/check/dnsbl/dnsbl_test.go index 1845aeb7..4dce6c9e 100644 --- a/internal/check/dnsbl/dnsbl_test.go +++ b/internal/check/dnsbl/dnsbl_test.go @@ -211,3 +211,283 @@ func TestCheckLists(t *testing.T) { true, false, ) } + +func TestCheckIPWithResponseRules(t *testing.T) { + test := func(zones map[string]mockdns.Zone, cfg List, ip net.IP, expectedErr error) { + t.Helper() + resolver := mockdns.Resolver{Zones: zones} + err := checkIP(context.Background(), &resolver, cfg, ip) + if expectedErr == nil { + if err != nil { + t.Errorf("expected no error, got '%#v'", err) + } + } else { + if err == nil { + t.Errorf("expected err to be '%#v', got nil", expectedErr) + } else { + expectedLE, okExpected := expectedErr.(ListedErr) + actualLE, okActual := err.(ListedErr) + if !okExpected || !okActual { + t.Errorf("expected err to be '%#v', got '%#v'", expectedErr, err) + } else { + if expectedLE.Identity != actualLE.Identity || + expectedLE.List != actualLE.List || + expectedLE.Score != actualLE.Score || + expectedLE.Message != actualLE.Message { + t.Errorf("expected err to be '%#v', got '%#v'", expectedErr, err) + } + } + } + } + } + + // Test single response code with score and message + test(map[string]mockdns.Zone{ + "4.3.2.1.example.org.": { + A: []string{"127.0.0.2"}, + }, + }, List{ + Zone: "example.org", + ClientIPv4: true, + ResponseRules: []ResponseRule{ + { + Networks: []net.IPNet{ + {IP: net.IPv4(127, 0, 0, 2), Mask: net.IPv4Mask(255, 255, 255, 255)}, + }, + Score: 10, + Message: "Listed in SBL", + }, + }, + }, net.IPv4(1, 2, 3, 4), ListedErr{ + Identity: "1.2.3.4", + List: "example.org", + Score: 10, + Message: "Listed in SBL", + }) + + // Test multiple response codes with different scores - scores should sum + test(map[string]mockdns.Zone{ + "4.3.2.1.example.org.": { + A: []string{"127.0.0.2", "127.0.0.11"}, + }, + }, List{ + Zone: "example.org", + ClientIPv4: true, + ResponseRules: []ResponseRule{ + { + Networks: []net.IPNet{ + {IP: net.IPv4(127, 0, 0, 2), Mask: net.IPv4Mask(255, 255, 255, 255)}, + {IP: net.IPv4(127, 0, 0, 3), Mask: net.IPv4Mask(255, 255, 255, 255)}, + }, + Score: 10, + Message: "Listed in SBL", + }, + { + Networks: []net.IPNet{ + {IP: net.IPv4(127, 0, 0, 10), Mask: net.IPv4Mask(255, 255, 255, 255)}, + {IP: net.IPv4(127, 0, 0, 11), Mask: net.IPv4Mask(255, 255, 255, 255)}, + }, + Score: 5, + Message: "Listed in PBL", + }, + }, + }, net.IPv4(1, 2, 3, 4), ListedErr{ + Identity: "1.2.3.4", + List: "example.org", + Score: 15, // 10 + 5 + Message: "Listed in SBL", + }) + + // Test response code that doesn't match any rule - should return nil + test(map[string]mockdns.Zone{ + "4.3.2.1.example.org.": { + A: []string{"127.0.0.99"}, + }, + }, List{ + Zone: "example.org", + ClientIPv4: true, + ResponseRules: []ResponseRule{ + { + Networks: []net.IPNet{ + {IP: net.IPv4(127, 0, 0, 2), Mask: net.IPv4Mask(255, 255, 255, 255)}, + }, + Score: 10, + Message: "Listed in SBL", + }, + }, + }, net.IPv4(1, 2, 3, 4), nil) + + // Test low severity only - should get score 5 + test(map[string]mockdns.Zone{ + "4.3.2.1.example.org.": { + A: []string{"127.0.0.10"}, + }, + }, List{ + Zone: "example.org", + ClientIPv4: true, + ResponseRules: []ResponseRule{ + { + Networks: []net.IPNet{ + {IP: net.IPv4(127, 0, 0, 2), Mask: net.IPv4Mask(255, 255, 255, 255)}, + }, + Score: 10, + Message: "Listed in SBL", + }, + { + Networks: []net.IPNet{ + {IP: net.IPv4(127, 0, 0, 10), Mask: net.IPv4Mask(255, 255, 255, 255)}, + }, + Score: 5, + Message: "Listed in PBL", + }, + }, + }, net.IPv4(1, 2, 3, 4), ListedErr{ + Identity: "1.2.3.4", + List: "example.org", + Score: 5, + Message: "Listed in PBL", + }) + + // Test high severity - should get score 10 + test(map[string]mockdns.Zone{ + "4.3.2.1.example.org.": { + A: []string{"127.0.0.2"}, + }, + }, List{ + Zone: "example.org", + ClientIPv4: true, + ResponseRules: []ResponseRule{ + { + Networks: []net.IPNet{ + {IP: net.IPv4(127, 0, 0, 2), Mask: net.IPv4Mask(255, 255, 255, 255)}, + }, + Score: 10, + Message: "Listed in SBL", + }, + }, + }, net.IPv4(1, 2, 3, 4), ListedErr{ + Identity: "1.2.3.4", + List: "example.org", + Score: 10, + Message: "Listed in SBL", + }) +} + +func TestCheckListsWithResponseRules(t *testing.T) { + test := func(zones map[string]mockdns.Zone, bls []List, ip net.IP, ehlo, mailFrom string, reject, quarantine bool) { + mod := &DNSBL{ + bls: bls, + resolver: &mockdns.Resolver{Zones: zones}, + log: testutils.Logger(t, "dnsbl"), + quarantineThres: 5, + rejectThres: 10, + } + result := mod.checkLists(context.Background(), ip, ehlo, mailFrom) + + if result.Reject && !reject { + t.Errorf("Expected message to not be rejected") + } + if !result.Reject && reject { + t.Errorf("Expected message to be rejected") + } + if result.Quarantine && !quarantine { + t.Errorf("Expected message to not be quarantined") + } + if !result.Quarantine && quarantine { + t.Errorf("Expected message to be quarantined") + } + } + + // Test: Only low-severity code returned -> quarantine but not reject + test(map[string]mockdns.Zone{ + "4.3.2.1.zen.example.org.": { + A: []string{"127.0.0.11"}, + }, + }, []List{ + { + Zone: "zen.example.org", + ClientIPv4: true, + ResponseRules: []ResponseRule{ + { + Networks: []net.IPNet{ + {IP: net.IPv4(127, 0, 0, 2), Mask: net.IPv4Mask(255, 255, 255, 255)}, + }, + Score: 10, + Message: "Listed in SBL", + }, + { + Networks: []net.IPNet{ + {IP: net.IPv4(127, 0, 0, 10), Mask: net.IPv4Mask(255, 255, 255, 255)}, + {IP: net.IPv4(127, 0, 0, 11), Mask: net.IPv4Mask(255, 255, 255, 255)}, + }, + Score: 5, + Message: "Listed in PBL", + }, + }, + }, + }, net.IPv4(1, 2, 3, 4), "mx.example.com", "foo@example.com", false, true) + + // Test: High-severity code returned -> reject + test(map[string]mockdns.Zone{ + "4.3.2.1.zen.example.org.": { + A: []string{"127.0.0.2"}, + }, + }, []List{ + { + Zone: "zen.example.org", + ClientIPv4: true, + ResponseRules: []ResponseRule{ + { + Networks: []net.IPNet{ + {IP: net.IPv4(127, 0, 0, 2), Mask: net.IPv4Mask(255, 255, 255, 255)}, + }, + Score: 10, + Message: "Listed in SBL", + }, + }, + }, + }, net.IPv4(1, 2, 3, 4), "mx.example.com", "foo@example.com", true, false) + + // Test: Legacy configuration without response blocks -> existing behavior preserved + test(map[string]mockdns.Zone{ + "4.3.2.1.example.org.": { + A: []string{"127.0.0.1"}, + }, + }, []List{ + { + Zone: "example.org", + ClientIPv4: true, + ScoreAdj: 10, + }, + }, net.IPv4(1, 2, 3, 4), "mx.example.com", "foo@example.com", true, false) + + // Test: Mixed configuration (some lists with response blocks, some without) -> both work correctly + test(map[string]mockdns.Zone{ + "4.3.2.1.zen.example.org.": { + A: []string{"127.0.0.11"}, + }, + "4.3.2.1.legacy.example.org.": { + A: []string{"127.0.0.1"}, + }, + }, []List{ + { + Zone: "zen.example.org", + ClientIPv4: true, + ResponseRules: []ResponseRule{ + { + Networks: []net.IPNet{ + {IP: net.IPv4(127, 0, 0, 11), Mask: net.IPv4Mask(255, 255, 255, 255)}, + }, + Score: 5, + Message: "Listed in PBL", + }, + }, + }, + { + Zone: "legacy.example.org", + ClientIPv4: true, + ScoreAdj: 3, + }, + }, net.IPv4(1, 2, 3, 4), "mx.example.com", "foo@example.com", false, true) // 5 + 3 = 8, quarantine but not reject +} + From 3404ef66e6522249e9bb12528fcedcff62419e83 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 11 Jan 2026 04:50:36 +0000 Subject: [PATCH 3/7] Fix config parsing to use node.Children instead of AllowUnknown Co-authored-by: thisisjaymehta <31812582+thisisjaymehta@users.noreply.github.com> --- internal/check/dnsbl/dnsbl.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/internal/check/dnsbl/dnsbl.go b/internal/check/dnsbl/dnsbl.go index fff87c37..37b5530a 100644 --- a/internal/check/dnsbl/dnsbl.go +++ b/internal/check/dnsbl/dnsbl.go @@ -134,9 +134,7 @@ func (bl *DNSBL) readListCfg(node config.Node) error { cfg.Bool("mailfrom", false, defaultBL.EHLO, &listCfg.MAILFROM) cfg.Int("score", false, false, 1, &listCfg.ScoreAdj) cfg.StringList("responses", false, false, []string{"127.0.0.1/24"}, &responseNets) - cfg.AllowUnknown() - unknown, err := cfg.Process() - if err != nil { + if _, err := cfg.Process(); err != nil { return err } @@ -154,8 +152,8 @@ func (bl *DNSBL) readListCfg(node config.Node) error { listCfg.Responses = append(listCfg.Responses, *ipNet) } - // Parse response blocks - for _, child := range unknown { + // Parse response blocks from node children + for _, child := range node.Children { if child.Name == "response" { rule, err := parseResponseRule(child) if err != nil { From fe91076f597d496ecceab067b88c6199acb35779 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 11 Jan 2026 04:51:43 +0000 Subject: [PATCH 4/7] Fix rule matching to count each rule only once Co-authored-by: thisisjaymehta <31812582+thisisjaymehta@users.noreply.github.com> --- internal/check/dnsbl/common.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/internal/check/dnsbl/common.go b/internal/check/dnsbl/common.go index 2f317097..d11626d4 100644 --- a/internal/check/dnsbl/common.go +++ b/internal/check/dnsbl/common.go @@ -125,8 +125,16 @@ func checkIP(ctx context.Context, resolver dns.Resolver, cfg List, ip net.IP) er var matchedMessages []string var matchedReasons []string + // Track which rules have been matched to avoid counting the same rule multiple times + matchedRules := make(map[int]bool) + for _, addr := range addrs { - for _, rule := range cfg.ResponseRules { + for ruleIdx, rule := range cfg.ResponseRules { + // Skip if this rule has already been matched + if matchedRules[ruleIdx] { + continue + } + for _, respNet := range rule.Networks { if respNet.Contains(addr.IP) { totalScore += rule.Score @@ -134,7 +142,8 @@ func checkIP(ctx context.Context, resolver dns.Resolver, cfg List, ip net.IP) er matchedMessages = append(matchedMessages, rule.Message) } matchedReasons = append(matchedReasons, addr.IP.String()) - break // Only match once per rule + matchedRules[ruleIdx] = true + break // Move to next rule } } } From 11be6679520b23ec1fba38bf0ef37a39cec9e904 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 11 Jan 2026 04:52:26 +0000 Subject: [PATCH 5/7] Improve documentation with clearer scoring behavior examples Co-authored-by: thisisjaymehta <31812582+thisisjaymehta@users.noreply.github.com> --- docs/reference/checks/dnsbl.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/reference/checks/dnsbl.md b/docs/reference/checks/dnsbl.md index be31a67f..5766de1a 100644 --- a/docs/reference/checks/dnsbl.md +++ b/docs/reference/checks/dnsbl.md @@ -216,7 +216,9 @@ and contains the following directives: **Required** Score to add when this response code is returned. If multiple response codes -are returned by the DNSBL, scores are summed together. +are returned by the DNSBL, and they match different rules, the scores from +all matched rules are summed together. Each rule is counted only once, even +if multiple returned IPs match networks within that rule. #### message _string_ **Optional** @@ -245,5 +247,11 @@ zen.spamhaus.org { } ``` +**Scoring behavior:** +- If DNSBL returns `127.0.0.2` only → Score: 10 (matches first rule) +- If DNSBL returns `127.0.0.11` only → Score: 5 (matches second rule) +- If DNSBL returns both `127.0.0.2` and `127.0.0.11` → Score: 15 (both rules match, scores sum) +- If DNSBL returns both `127.0.0.2` and `127.0.0.3` → Score: 10 (same rule matches, counted once) + **Backwards compatibility:** When `response` blocks are not used, the legacy `responses` and `score` directives work as before. From 66339ddefb8f13303af3377039f2d48c4cd84153 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 11 Jan 2026 05:02:18 +0000 Subject: [PATCH 6/7] Address upstream PR feedback: use cfg.Callback, extract function, remove version annotation Co-authored-by: thisisjaymehta <31812582+thisisjaymehta@users.noreply.github.com> --- docs/reference/checks/dnsbl.md | 2 - internal/check/dnsbl/common.go | 147 +++++++++++++++------------------ internal/check/dnsbl/dnsbl.go | 22 ++--- 3 files changed, 79 insertions(+), 92 deletions(-) diff --git a/docs/reference/checks/dnsbl.md b/docs/reference/checks/dnsbl.md index 5766de1a..0dd62e57 100644 --- a/docs/reference/checks/dnsbl.md +++ b/docs/reference/checks/dnsbl.md @@ -203,8 +203,6 @@ rules is used instead of this flat score value. ### response _ip..._ -**New in 0.8** - Defines per-response-code rules for scoring and custom messages. This is useful for combined DNSBLs like Spamhaus ZEN that return different codes for different listing types. diff --git a/internal/check/dnsbl/common.go b/internal/check/dnsbl/common.go index d11626d4..7541d24f 100644 --- a/internal/check/dnsbl/common.go +++ b/internal/check/dnsbl/common.go @@ -94,6 +94,34 @@ func checkDomain(ctx context.Context, resolver dns.Resolver, cfg List, domain st } } +func matchResponseRules(addrs []net.IPAddr, rules []ResponseRule) (score int, messages []string, reasons []string, matched bool) { + // Track which rules have been matched to avoid counting the same rule multiple times + matchedRules := make(map[int]bool) + + for _, addr := range addrs { + for ruleIdx, rule := range rules { + // Skip if this rule has already been matched + if matchedRules[ruleIdx] { + continue + } + + for _, respNet := range rule.Networks { + if respNet.Contains(addr.IP) { + score += rule.Score + if rule.Message != "" { + messages = append(messages, rule.Message) + } + reasons = append(reasons, addr.IP.String()) + matchedRules[ruleIdx] = true + matched = true + break // Move to next rule + } + } + } + } + return +} + func checkIP(ctx context.Context, resolver dns.Resolver, cfg List, ip net.IP) error { ipv6 := true if ipv4 := ip.To4(); ipv4 != nil { @@ -119,111 +147,72 @@ func checkIP(ctx context.Context, resolver dns.Resolver, cfg List, ip net.IP) er return err } + var filteredAddrs []net.IPAddr + var score int + var customMessage string + // If ResponseRules is configured, use new behavior if len(cfg.ResponseRules) > 0 { - totalScore := 0 - var matchedMessages []string - var matchedReasons []string - - // Track which rules have been matched to avoid counting the same rule multiple times - matchedRules := make(map[int]bool) - - for _, addr := range addrs { - for ruleIdx, rule := range cfg.ResponseRules { - // Skip if this rule has already been matched - if matchedRules[ruleIdx] { - continue - } - - for _, respNet := range rule.Networks { - if respNet.Contains(addr.IP) { - totalScore += rule.Score - if rule.Message != "" { - matchedMessages = append(matchedMessages, rule.Message) - } - matchedReasons = append(matchedReasons, addr.IP.String()) - matchedRules[ruleIdx] = true - break // Move to next rule - } - } - } - } - - if totalScore == 0 { + matchedScore, matchedMessages, matchedReasons, matched := matchResponseRules(addrs, cfg.ResponseRules) + if !matched { return nil } - - // Attempt to extract explanation string from TXT records - txts, err := resolver.LookupTXT(ctx, query) - var reason string - if err == nil && len(txts) > 0 { - reason = strings.Join(txts, "; ") - } else { - reason = strings.Join(matchedReasons, "; ") - } - + score = matchedScore + // Use first matched message if available - message := "" if len(matchedMessages) > 0 { - message = matchedMessages[0] + customMessage = matchedMessages[0] } - - return ListedErr{ - Identity: ip.String(), - List: cfg.Zone, - Reason: reason, - Score: totalScore, - Message: message, + + // Build filteredAddrs from matched reasons for TXT lookup fallback + for _, reason := range matchedReasons { + filteredAddrs = append(filteredAddrs, net.IPAddr{IP: net.ParseIP(reason)}) } - } - - // Legacy behavior: use flat Responses filter - filteredAddrs := make([]net.IPAddr, 0, len(addrs)) -addrsLoop: - for _, addr := range addrs { - // No responses whitelist configured - permit all. - if len(cfg.Responses) == 0 { - filteredAddrs = append(filteredAddrs, addr) - continue - } - - for _, respNet := range cfg.Responses { - if respNet.Contains(addr.IP) { + } else { + // Legacy behavior: use flat Responses filter + filteredAddrs = make([]net.IPAddr, 0, len(addrs)) + addrsLoop: + for _, addr := range addrs { + // No responses whitelist configured - permit all. + if len(cfg.Responses) == 0 { filteredAddrs = append(filteredAddrs, addr) - continue addrsLoop + continue + } + + for _, respNet := range cfg.Responses { + if respNet.Contains(addr.IP) { + filteredAddrs = append(filteredAddrs, addr) + continue addrsLoop + } } } - } - if len(filteredAddrs) == 0 { - return nil + if len(filteredAddrs) == 0 { + return nil + } } - // Attempt to extract explanation string. + // Attempt to extract explanation string from TXT records (shared by both paths) txts, err := resolver.LookupTXT(ctx, query) - if err != nil || len(txts) == 0 { + var reason string + if err == nil && len(txts) > 0 { + reason = strings.Join(txts, "; ") + } else { // Not significant, include addresses as reason. Usually they are // mapped to some predefined 'reasons' by BL. - reasonParts := make([]string, 0, len(filteredAddrs)) for _, addr := range filteredAddrs { reasonParts = append(reasonParts, addr.IP.String()) } - - return ListedErr{ - Identity: ip.String(), - List: cfg.Zone, - Reason: strings.Join(reasonParts, "; "), - } + reason = strings.Join(reasonParts, "; ") } - // Some BLs provide multiple reasons (meta-BLs such as Spamhaus Zen) so - // don't mangle them by joining with "", instead join with "; ". - return ListedErr{ Identity: ip.String(), List: cfg.Zone, - Reason: strings.Join(txts, "; "), + Reason: reason, + Score: score, + Message: customMessage, } } diff --git a/internal/check/dnsbl/dnsbl.go b/internal/check/dnsbl/dnsbl.go index 37b5530a..94cbfe80 100644 --- a/internal/check/dnsbl/dnsbl.go +++ b/internal/check/dnsbl/dnsbl.go @@ -134,6 +134,14 @@ func (bl *DNSBL) readListCfg(node config.Node) error { cfg.Bool("mailfrom", false, defaultBL.EHLO, &listCfg.MAILFROM) cfg.Int("score", false, false, 1, &listCfg.ScoreAdj) cfg.StringList("responses", false, false, []string{"127.0.0.1/24"}, &responseNets) + cfg.Callback("response", func(_ *config.Map, node config.Node) error { + rule, err := parseResponseRule(node) + if err != nil { + return err + } + listCfg.ResponseRules = append(listCfg.ResponseRules, rule) + return nil + }) if _, err := cfg.Process(); err != nil { return err } @@ -152,17 +160,9 @@ func (bl *DNSBL) readListCfg(node config.Node) error { listCfg.Responses = append(listCfg.Responses, *ipNet) } - // Parse response blocks from node children - for _, child := range node.Children { - if child.Name == "response" { - rule, err := parseResponseRule(child) - if err != nil { - return err - } - listCfg.ResponseRules = append(listCfg.ResponseRules, rule) - } else { - return config.NodeErr(child, "unknown directive: %s", child.Name) - } + // Warn if both response and responses are configured + if len(listCfg.ResponseRules) > 0 && len(responseNets) > 0 { + bl.log.Msg("both 'response' blocks and 'responses' directive are specified, 'response' blocks take precedence", "list", node.Name) } for _, zone := range append([]string{node.Name}, node.Args...) { From c29208c95d75e813d4c3bab57a7cf3273a9437c2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 11 Jan 2026 05:21:16 +0000 Subject: [PATCH 7/7] Add ResponseRules support for checkDomain (domain-based lookups) Co-authored-by: thisisjaymehta <31812582+thisisjaymehta@users.noreply.github.com> --- docs/reference/checks/dnsbl.md | 3 + internal/check/dnsbl/common.go | 53 +++++++++++---- internal/check/dnsbl/common_test.go | 101 ++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 12 deletions(-) diff --git a/docs/reference/checks/dnsbl.md b/docs/reference/checks/dnsbl.md index 0dd62e57..d7bb74cf 100644 --- a/docs/reference/checks/dnsbl.md +++ b/docs/reference/checks/dnsbl.md @@ -207,6 +207,9 @@ Defines per-response-code rules for scoring and custom messages. This is useful for combined DNSBLs like Spamhaus ZEN that return different codes for different listing types. +This works for both IP-based lookups (client_ipv4, client_ipv6) and domain-based +lookups (ehlo, mailfrom). + Each `response` block takes one or more IP addresses or CIDR ranges as arguments and contains the following directives: diff --git a/internal/check/dnsbl/common.go b/internal/check/dnsbl/common.go index 7541d24f..4e90e7e2 100644 --- a/internal/check/dnsbl/common.go +++ b/internal/check/dnsbl/common.go @@ -72,25 +72,54 @@ func checkDomain(ctx context.Context, resolver dns.Resolver, cfg List, domain st return nil } - // Attempt to extract explanation string. - txts, err := resolver.LookupTXT(context.Background(), query) - if err != nil || len(txts) == 0 { - // Not significant, include addresses as reason. Usually they are - // mapped to some predefined 'reasons' by BL. - return ListedErr{ - Identity: domain, - List: cfg.Zone, - Reason: strings.Join(addrs, "; "), + var score int + var customMessage string + var filteredAddrs []string + + // If ResponseRules is configured, use new behavior + if len(cfg.ResponseRules) > 0 { + // Convert string addresses to IPAddr for matching + ipAddrs := make([]net.IPAddr, 0, len(addrs)) + for _, addr := range addrs { + if ip := net.ParseIP(addr); ip != nil { + ipAddrs = append(ipAddrs, net.IPAddr{IP: ip}) + } } + + matchedScore, matchedMessages, matchedReasons, matched := matchResponseRules(ipAddrs, cfg.ResponseRules) + if !matched { + return nil + } + score = matchedScore + + // Use first matched message if available + if len(matchedMessages) > 0 { + customMessage = matchedMessages[0] + } + + filteredAddrs = matchedReasons + } else { + // Legacy behavior: accept all addresses + filteredAddrs = addrs } - // Some BLs provide multiple reasons (meta-BLs such as Spamhaus Zen) so - // don't mangle them by joining with "", instead join with "; ". + // Attempt to extract explanation string from TXT records (shared by both paths) + txts, err := resolver.LookupTXT(ctx, query) + var reason string + if err == nil && len(txts) > 0 { + reason = strings.Join(txts, "; ") + } else { + // Not significant, include addresses as reason. Usually they are + // mapped to some predefined 'reasons' by BL. + reason = strings.Join(filteredAddrs, "; ") + } return ListedErr{ Identity: domain, List: cfg.Zone, - Reason: strings.Join(txts, "; "), + Reason: reason, + Score: score, + Message: customMessage, } } diff --git a/internal/check/dnsbl/common_test.go b/internal/check/dnsbl/common_test.go index caa06575..8d2b24e3 100644 --- a/internal/check/dnsbl/common_test.go +++ b/internal/check/dnsbl/common_test.go @@ -236,3 +236,104 @@ func TestCheckIP(t *testing.T) { Reason: "127.0.0.1", }) } + +func TestCheckDomainWithResponseRules(t *testing.T) { + test := func(zones map[string]mockdns.Zone, cfg List, domain string, expectedErr error) { + t.Helper() + resolver := mockdns.Resolver{Zones: zones} + err := checkDomain(context.Background(), &resolver, cfg, domain) + if expectedErr == nil { + if err != nil { + t.Errorf("expected no error, got '%#v'", err) + } + } else { + if err == nil { + t.Errorf("expected err to be '%#v', got nil", expectedErr) + } else { + expectedLE, okExpected := expectedErr.(ListedErr) + actualLE, okActual := err.(ListedErr) + if !okExpected || !okActual { + t.Errorf("expected err to be '%#v', got '%#v'", expectedErr, err) + } else { + if expectedLE.Identity != actualLE.Identity || + expectedLE.List != actualLE.List || + expectedLE.Score != actualLE.Score || + expectedLE.Message != actualLE.Message { + t.Errorf("expected err to be '%#v', got '%#v'", expectedErr, err) + } + } + } + } + } + + // Test domain with single response code and custom message + test(map[string]mockdns.Zone{ + "spam.example.com.dnsbl.example.org.": { + A: []string{"127.0.0.2"}, + }, + }, List{ + Zone: "dnsbl.example.org", + ResponseRules: []ResponseRule{ + { + Networks: []net.IPNet{ + {IP: net.IPv4(127, 0, 0, 2), Mask: net.IPv4Mask(255, 255, 255, 255)}, + }, + Score: 10, + Message: "Domain listed as spam source", + }, + }, + }, "spam.example.com", ListedErr{ + Identity: "spam.example.com", + List: "dnsbl.example.org", + Score: 10, + Message: "Domain listed as spam source", + }) + + // Test domain with multiple response codes - scores should sum + test(map[string]mockdns.Zone{ + "multi.example.com.dnsbl.example.org.": { + A: []string{"127.0.0.2", "127.0.0.11"}, + }, + }, List{ + Zone: "dnsbl.example.org", + ResponseRules: []ResponseRule{ + { + Networks: []net.IPNet{ + {IP: net.IPv4(127, 0, 0, 2), Mask: net.IPv4Mask(255, 255, 255, 255)}, + }, + Score: 10, + Message: "High severity", + }, + { + Networks: []net.IPNet{ + {IP: net.IPv4(127, 0, 0, 11), Mask: net.IPv4Mask(255, 255, 255, 255)}, + }, + Score: 5, + Message: "Low severity", + }, + }, + }, "multi.example.com", ListedErr{ + Identity: "multi.example.com", + List: "dnsbl.example.org", + Score: 15, // 10 + 5 + Message: "High severity", + }) + + // Test domain with no matching response codes + test(map[string]mockdns.Zone{ + "unknown.example.com.dnsbl.example.org.": { + A: []string{"127.0.0.99"}, + }, + }, List{ + Zone: "dnsbl.example.org", + ResponseRules: []ResponseRule{ + { + Networks: []net.IPNet{ + {IP: net.IPv4(127, 0, 0, 2), Mask: net.IPv4Mask(255, 255, 255, 255)}, + }, + Score: 10, + Message: "Listed", + }, + }, + }, "unknown.example.com", nil) +}