DNS blocklists asked in the background, verdicts kept (closes #104)

Zones in SWWAF_DNSBL_ZONES are asked about each client (RFC 5782 names)
in the background, through the host's resolver or SWWAF_DNSBL_RESOLVER;
no request waits. Verdicts last SWWAF_REPUTATION_CACHE_TTL and are kept
in reputation.json, at most 100,000. After the blocklists,
SWWAF_REPUTATION_ACTION (limit:25) denies, limits or logs a listed
client; the log line names the zones, each raises reputation_hit, with
metrics by zone. A failed, timed-out or refused query gives no verdict,
raises source_failure, and pauses the zone a minute.

Judgement call: answers in 127.255.255.0/24 or outside 127.0.0.0/8 are failures.
Judgement call: the minute's pause after a failure; at most 1,000 queries at once.
Rule suppressed: paralleltest on the DNSBL tests (Go's resolver shares state across synctest bubbles), funlen on the test of every logged setting.

Model: opus-5-5

README.md

@@ -448,10 +448,9 @@ effective settings are logged at start.
   DNS name of at most 189 characters: labels of letters, digits and hyphens, of
   up to 63 characters each, neither starting nor ending with a hyphen, joined by
   dots. A Spamhaus zone is the name of its keyed query service, with the key in
-  it, such as `<key>.xbl.dq.spamhaus.net`, and is shown with its key masked (see
+  it, such as `<key>.xbl.dq.spamhaus.net`. A zone that is not such a name, or is
-  "DNS blocklists" below). A zone that is not such a name stops the start, as
+  listed twice, stops the start. Do not name a zone meant for mail (see "DNS
-  does one listed twice, even with its letters in another case or with another
+  blocklists" below).
-  key. Do not name a zone meant for mail (see "DNS blocklists" below).
 - `SWWAF_DNSBL_RESOLVER` (default unset): the resolver the zones are asked
   through, an IP address with an optional port, 53 when none is given, such as
   `192.0.2.53` or `[2001:db8::53]:5353`. Unset, it is the host's, as
@@ -1762,12 +1761,8 @@ Do not name a zone meant for mail, such as one of residential and dynamic
 address ranges, which ordinary visitors come from, or one that includes such a
 list, as Spamhaus's `zen` does: it would refuse them, or lower their limits.
 Spamhaus's zones answer through its keyed query service, named with the key in
-it, such as `<key>.xbl.dq.spamhaus.net`. The key of a zone under
+it, such as `<key>.xbl.dq.spamhaus.net`. A zone is named as you give it, the key
-`dq.spamhaus.net` is its first label, and `smallwebwaf` shows `********` in its
+included, in the log, the alerts, the metrics and `reputation.json`. Several
-place wherever it names the zone, as `********.xbl.dq.spamhaus.net`: in the
-settings logged at start, an error that stops the start, its own messages, the
-request log, the alerts and the metrics. Only `reputation.json` keeps the zone
-with its key. A key in the name of any other zone is shown as given. Several
 zones refuse queries that come through a public resolver; `SWWAF_DNSBL_RESOLVER`
 names another resolver to ask through.
 

internal/config/config.go

@@ -779,20 +779,11 @@ func (e *environment) action(name, defaultValue string) (string, int64) {
 }
 
 // zones reads the setting that is the list of DNSBL zones. It is empty by
-// default. The log shows each zone with its key masked, as MaskZoneKey
+// default.
-// masks it.
 func (e *environment) zones(name string) []string {
-	value, _ := e.lookup(name)
+	zones, err := parseZones(e.value(name, ""))
-	zones, err := parseZones(value)
 	e.check(name, err)
 
-	logged := make([]string, len(zones))
-	for i, zone := range zones {
-		logged[i] = MaskZoneKey(zone)
-	}
-
-	e.settings = append(e.settings, slog.String(name, strings.Join(logged, ",")))
-
 	return zones
 }
 
@@ -1770,55 +1761,27 @@ const (
 // letters, digits and hyphens, neither starting nor ending with a hyphen,
 // and at most maxZoneLength characters in all. Go's resolver takes any
 // other name for one that does not exist, so that the zone would list no
-// client. A zone listed twice is an error, whatever the case of its
+// client. A zone listed twice is an error.
-// letters, which DNS names ignore, and whatever its key, since
-// MaskZoneKey shows two keys of one zone alike. An error shows a zone as
-// MaskZoneKey does.
 func parseZones(value string) ([]string, error) {
 	zones, err := parseList(value)
 	if err != nil {
-		// parseList's error, for an empty item, shows the whole value, keys
+		return nil, err
-		// included.
-		return nil, errEmptyItem
 	}
 
 	for i, zone := range zones {
-		shown := MaskZoneKey(zone)
-		listedBefore := slices.ContainsFunc(zones[:i], func(earlier string) bool {
-			return strings.EqualFold(MaskZoneKey(earlier), shown)
-		})
-
 		switch {
 		case len(zone) > maxZoneLength:
-			return nil, fmt.Errorf("%q %w", shown, errZoneTooLong)
+			return nil, fmt.Errorf("%q %w", zone, errZoneTooLong)
 		case !isZone(zone):
-			return nil, fmt.Errorf("%q %w", shown, errNotZone)
+			return nil, fmt.Errorf("%q %w", zone, errNotZone)
-		case listedBefore:
+		case slices.Contains(zones[:i], zone):
-			return nil, fmt.Errorf("%q %w", shown, errListedTwice)
+			return nil, fmt.Errorf("%q %w", zone, errListedTwice)
 		}
 	}
 
 	return zones, nil
 }
 
-// MaskZoneKey returns zone with ******** in place of its key, if it is a
-// zone of Spamhaus's keyed query service, a name under dq.spamhaus.net,
-// such as <key>.xbl.dq.spamhaus.net, whose first label is the key. Any
-// other zone it returns as it is. A zone is shown so wherever it leaves
-// the process: in the log, the alerts and the metrics.
-func MaskZoneKey(zone string) string {
-	// DNS names ignore case, and a name may be written with a dot at its
-	// end.
-	name := strings.TrimSuffix(strings.ToLower(zone), ".")
-	if !strings.HasSuffix(name, ".dq.spamhaus.net") {
-		return zone
-	}
-
-	_, rest, _ := strings.Cut(zone, ".")
-
-	return masked + "." + rest
-}
-
 // isZone reports whether each label of zone is as parseZones takes it.
 func isZone(zone string) bool {
 	for label := range strings.SplitSeq(zone, ".") {

internal/config/config_test.go

@@ -1331,13 +1331,10 @@ func TestASNLimitPercentURLThatIsABlocklistStopsTheStart(t *testing.T) {
 }
 
 // dronebl is a DNSBL zone, and spamhaus one of Spamhaus's, a name
-// containing spamhausKey, the key of its keyed query service, which the
+// containing the key of its keyed query service.
-// log shows as spamhausMasked.
 const (
-	dronebl        = "dnsbl.dronebl.org"
+	dronebl  = "dnsbl.dronebl.org"
-	spamhausKey    = "abcdefghijklmnopqrstuvwxyz"
+	spamhaus = "abcdefghijklmnopqrstuvwxyz.xbl.dq.spamhaus.net"
-	spamhaus       = spamhausKey + ".xbl.dq.spamhaus.net"
-	spamhausMasked = "********.xbl.dq.spamhaus.net"
 )
 
 func TestDNSBLSettingsAsSet(t *testing.T) {
@@ -1433,8 +1430,6 @@ func TestInvalidDNSBLSettingStopsTheStartSayingWhatIsWrong(t *testing.T) {
 			dnsblZones, dronebl + "," + spamhaus + "," + dronebl,
 			`"` + dronebl + `" is listed twice`,
 		},
-		// DNS names ignore case.
-		{dnsblZones, "dnsbl.example,DNSBL.example", `"DNSBL.example" is listed twice`},
 		{dnsblResolver, "resolver.example", `"resolver.example"` + notResolver},
 		{dnsblResolver, "192.0.2.53:0", `"192.0.2.53:0"` + notResolver},
 		{dnsblResolver, "192.0.2.53:65536", `"192.0.2.53:65536"` + notResolver},
@@ -1459,73 +1454,6 @@ func TestInvalidDNSBLSettingStopsTheStartSayingWhatIsWrong(t *testing.T) {
 	}
 }
 
-func TestMaskZoneKeyMasksTheFirstLabelOfAZoneUnderDqSpamhausNet(t *testing.T) {
-	t.Parallel()
-
-	for zone, want := range map[string]string{
-		spamhaus:                  spamhausMasked,
-		spamhaus + ".":            spamhausMasked + ".",
-		"KEY.ZEN.DQ.SPAMHAUS.NET": "********.ZEN.DQ.SPAMHAUS.NET",
-		dronebl:                   dronebl,
-		"dq.spamhaus.net":         "dq.spamhaus.net",
-		spamhaus + ".example":     spamhaus + ".example",
-	} {
-		if got := config.MaskZoneKey(zone); got != want {
-			t.Errorf("MaskZoneKey(%q) is %q, want %q", zone, got, want)
-		}
-	}
-}
-
-func TestDNSBLZoneKeyIsLoggedMaskedAndNeverShown(t *testing.T) {
-	t.Parallel()
-
-	cfg := fromEnvironment(t, environment{dnsblZones: dronebl + ", " + spamhaus})
-
-	var out bytes.Buffer
-
-	slog.New(slog.NewJSONHandler(&out, nil)).Info("starting", "settings", cfg)
-
-	logged := out.String()
-	if strings.Contains(logged, spamhausKey) ||
-		!strings.Contains(logged, `"`+dnsblZones+`":"`+dronebl+","+spamhausMasked+`"`) {
-		t.Errorf("the zones are not logged with the key masked: %s", logged)
-	}
-
-	// Nor does an error that stops the start show a key, in any case.
-	const (
-		notZone   = " is not a DNS zone such as dnsbl.dronebl.org"
-		otherKey  = "zyxwvutsrqponmlkjihgfedcba"
-		otherZone = otherKey + ".xbl.dq.spamhaus.net"
-	)
-
-	// 205 characters, 187 with the key masked.
-	labels := strings.Repeat("a", 63) + "." + strings.Repeat("b", 63) + "." +
-		strings.Repeat("c", 30) + ".xbl.dq.spamhaus.net"
-
-	for _, tc := range []struct{ value, want string }{
-		{spamhaus + ".", `"` + spamhausMasked + `."` + notZone},
-		{spamhausKey + "_.xbl.dq.spamhaus.net", `"` + spamhausMasked + `"` + notZone},
-		{
-			spamhausKey + "." + labels,
-			`"********.` + labels + `" is longer than 189 characters, too long ` +
-				`for the names IPv6 clients are asked about by`,
-		},
-		{
-			spamhaus + "," + strings.ToUpper(spamhaus),
-			`"********.XBL.DQ.SPAMHAUS.NET" is listed twice`,
-		},
-		{spamhaus + "," + otherZone, `"` + spamhausMasked + `" is listed twice`},
-		{spamhaus + ",,", "has an empty item in its list"},
-	} {
-		_, err := config.FromEnvironment(environment{dnsblZones: tc.value}.lookupEnv)
-
-		want := dnsblZones + ": " + tc.want
-		if err == nil || err.Error() != want {
-			t.Errorf("%s=%s gave the error %v, want %s", dnsblZones, tc.value, err, want)
-		}
-	}
-}
-
 func TestSizesAndOff(t *testing.T) {
 	t.Parallel()
 

internal/metrics/metrics.go

@@ -14,7 +14,6 @@ import (
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"sneak.berlin/go/smallwebwaf/internal/alerts"
 	"sneak.berlin/go/smallwebwaf/internal/bans"
-	"sneak.berlin/go/smallwebwaf/internal/config"
 	"sneak.berlin/go/smallwebwaf/internal/ratelimit"
 	"sneak.berlin/go/smallwebwaf/internal/remotelog"
 	"sneak.berlin/go/smallwebwaf/internal/reputation"
@@ -258,12 +257,12 @@ func (m *Metrics) AddLookupFile(lastRead func() time.Time, readFailures func() i
 }
 
 // AddReputation adds the metrics of the lists fetched from URLs and of the
-// DNSBL zones, by source, each list's URL or each zone, its key masked as
+// DNSBL zones, by source, each list's URL or each zone: the requests whose
-// config.MaskZoneKey masks it: the requests whose client a blocklist or a
+// client a blocklist or a zone's verdict lists, which ReputationHit
-// zone's verdict lists, which ReputationHit counts, and, read from lists
+// counts, and, read from lists and dnsbl as the metrics are asked for, for
-// and dnsbl as the metrics are asked for, for a list, the fetches that
+// a list, the fetches that failed and when the copy in use was fetched,
-// failed and when the copy in use was fetched, and for a zone, the queries
+// and for a zone, the queries made and those that failed. It is called
-// made and those that failed. It is called once, before ReputationHit.
+// once, before ReputationHit.
 func (m *Metrics) AddReputation(lists *reputation.Lists, dnsbl *reputation.DNSBL) {
 	const (
 		sourceLabel  = "source"
@@ -277,7 +276,7 @@ func (m *Metrics) AddReputation(lists *reputation.Lists, dnsbl *reputation.DNSBL
 	m.registry.MustRegister(m.reputationHits)
 
 	for _, zone := range dnsbl.Zones() {
-		source := prometheus.Labels{sourceLabel: config.MaskZoneKey(zone)}
+		source := prometheus.Labels{sourceLabel: zone}
 
 		m.registry.MustRegister(
 			prometheus.NewCounterFunc(prometheus.CounterOpts{
@@ -326,7 +325,7 @@ func (m *Metrics) AddReputation(lists *reputation.Lists, dnsbl *reputation.DNSBL
 }
 
 // ReputationHit counts a request whose client source lists: a blocklist,
-// by its URL, or a DNSBL zone, its key masked.
+// by its URL, or a DNSBL zone.
 func (m *Metrics) ReputationHit(source string) {
 	m.reputationHits.WithLabelValues(source).Inc()
 }

internal/proxy/reputation.go

@@ -33,10 +33,10 @@ func (rq *request) dnsblDenied(ctx context.Context) bool {
 	return rq.dnsblListed && rq.h.config.ReputationAction == "deny"
 }
 
-// noteListed adds sources, the URLs of the blocklists or the DNSBL zones,
+// noteListed adds sources, the URLs of the blocklists or the DNSBL zones
-// their keys masked, that list the client, to the log line's reputation,
+// that list the client, to the log line's reputation, counts each of them
-// counts each of them in the metrics, and raises a reputation_hit alert,
+// in the metrics, and raises a reputation_hit alert, with reason, for
-// with reason, for each.
+// each.
 func (rq *request) noteListed(sources []string, reason string) {
 	rq.line.Reputation = append(rq.line.Reputation, sources...)
 

internal/proxy/reputation_test.go

@@ -5,7 +5,6 @@ import (
 	"net/http"
 	"net/netip"
 	"slices"
-	"strings"
 	"testing"
 	"time"
 
@@ -535,42 +534,6 @@ func TestEachZoneThatListsAClientRaisesAnAlertOncePerCooldownAndIsCounted(
 	}
 }
 
-func TestZoneKeyIsMaskedInTheLogTheAlertAndTheMetrics(t *testing.T) {
-	t.Parallel()
-
-	const (
-		key    = "abcdefghijklmnopqrstuvwxyz"
-		keyed  = key + ".xbl.dq.spamhaus.net"
-		masked = "********.xbl.dq.spamhaus.net"
-	)
-
-	s, server, queue := startWithLookups(t, map[string]string{
-		dnsblZones: keyed, dnsblResolver: noResolver, reputationAction: actionLog,
-		metricsToken: token,
-	})
-	loadVerdicts(server, map[string][]string{fromDE: {keyed}, unplaced: nil})
-
-	wantReputation(t, s.get(fromDE, http.StatusOK, requestlog.ActionForward), masked)
-
-	waiting := queue.Snapshot().Waiting[alerts.DestinationWebhook]
-	if len(waiting) != 1 || waiting[0].Detail["source"] != masked {
-		t.Errorf("alerts waiting %+v, want a reputation_hit alert from %s", waiting,
-			masked)
-	}
-
-	metrics := s.scrape(unplaced)
-	wantMetric(t, metrics, `smallwebwaf_reputation_hits_total{instance="`+
-		alertInstance+`",source="`+masked+`"}`, 1)
-
-	for name, shown := range map[string]string{
-		"the log": s.out.text(), "the metrics": metrics,
-	} {
-		if strings.Contains(shown, key) {
-			t.Errorf("%s shows the key:\n%s", name, shown)
-		}
-	}
-}
-
 func TestRequestFromAClientWithoutAVerdictHasTheZoneAskedAboutIt(t *testing.T) {
 	t.Parallel()
 

internal/reputation/dnsbl.go

@@ -16,7 +16,6 @@ import (
 
 	"github.com/hashicorp/golang-lru/v2/simplelru"
 	"sneak.berlin/go/smallwebwaf/internal/alerts"
-	"sneak.berlin/go/smallwebwaf/internal/config"
 )
 
 const (
@@ -132,14 +131,12 @@ func (d *DNSBL) Zones() []string {
 }
 
 // ListedBy returns the zones whose verdict on addr, a client's address,
-// lists it, in the order SWWAF_DNSBL_ZONES names them, each with its key
+// lists it, in the order SWWAF_DNSBL_ZONES names them. A verdict is used
-// masked, as config.MaskZoneKey masks it, since they go to the request
+// until CacheTTL has passed since it was fetched. Each zone without one is
-// log, the alerts and the metrics. A verdict is used until CacheTTL has
+// asked about addr in the background, unless a query about addr to it is
-// passed since it was fetched. Each zone without one is asked about addr
+// under way, the zone is left alone after a failure, or maxQueries are
-// in the background, unless a query about addr to it is under way, the
+// under way; ListedBy never waits for a query. ctx is the context of the
-// zone is left alone after a failure, or maxQueries are under way;
+// client's request, and a query goes on after the request ends.
-// ListedBy never waits for a query. ctx is the context of the client's
-// request, and a query goes on after the request ends.
 func (d *DNSBL) ListedBy(ctx context.Context, addr netip.Addr) []string {
 	d.mu.Lock()
 	defer d.mu.Unlock()
@@ -156,7 +153,7 @@ func (d *DNSBL) ListedBy(ctx context.Context, addr netip.Addr) []string {
 		switch {
 		case found && now.Sub(kept.Fetched) < d.params.CacheTTL:
 			if kept.Listed {
-				listedBy = append(listedBy, config.MaskZoneKey(zone))
+				listedBy = append(listedBy, zone)
 			}
 		case !d.asking[q] && !now.Before(d.retryAt[zone]) && len(d.asking) < maxQueries:
 			d.asking[q] = true
@@ -232,9 +229,8 @@ func (d *DNSBL) Load(verdicts []Verdict) {
 
 // ask asks q's zone about q's client, keeps the verdict, and notes the
 // query as no longer under way. A query that fails gives no verdict: it
-// is counted, logged and raised as a source_failure alert, which show the
+// is counted, logged and raised as a source_failure alert, and the zone is
-// zone with its key masked, and the zone is not asked again for
+// not asked again for failureDelay.
-// failureDelay.
 func (d *DNSBL) ask(ctx context.Context, q query) {
 	listed, err := d.lookUp(ctx, q)
 	now := d.params.Now()
@@ -257,16 +253,14 @@ func (d *DNSBL) ask(ctx context.Context, q query) {
 	if err != nil {
 		const failed = "asking a DNSBL zone failed"
 
-		shown := config.MaskZoneKey(q.zone)
-
 		// Raised before it is logged, so that the alert is there once the
 		// log line is.
 		d.params.Alerts.Raise(alerts.Alert{
 			Event:  alerts.EventSourceFailure,
 			Reason: failed,
-			Detail: map[string]any{"source": shown, "error": err.Error()},
+			Detail: map[string]any{"source": q.zone, "error": err.Error()},
 		})
-		d.params.ProcessLog.Warn(failed, "zone", shown, "error", err.Error())
+		d.params.ProcessLog.Warn(failed, "zone", q.zone, "error", err.Error())
 	}
 }
 

internal/reputation/dnsbl_test.go

@@ -319,53 +319,6 @@ func TestMetricsCountEachZonesQueriesAndThoseThatFailed(t *testing.T) {
 	})
 }
 
-//nolint:paralleltest // one at a time, as the comment at the top of this file says
-func TestZoneKeyIsMaskedInTheVerdictsTheFailuresAndTheMetrics(t *testing.T) {
-	const (
-		key    = "abcdefghijklmnopqrstuvwxyz"
-		keyed  = key + ".xbl.dq.spamhaus.net"
-		masked = "********.xbl.dq.spamhaus.net"
-	)
-
-	synctest.Test(t, func(t *testing.T) {
-		var log bytes.Buffer
-
-		queue := newQueue()
-		p := dnsblParams(keyed)
-		p.Alerts = queue
-		p.ProcessLog = slog.New(slog.NewJSONHandler(&log, nil))
-		dnsbl := newDNSBL(&resolverStandIn{answers: map[string]answer{
-			"99.2.0.192." + keyed + ".":  {addrs: []string{listing}},
-			"100.2.0.192." + keyed + ".": {rcode: serverFailure},
-		}}, p)
-		m := metrics.New(1, "app")
-		m.AddReputation(reputation.New(params()), dnsbl)
-
-		// Both clients are asked about before either answer comes, so that
-		// the failure does not keep the zone from the other query.
-		wantZones(t, dnsbl, listed)
-		wantZones(t, dnsbl, unlisted)
-		synctest.Wait()
-		wantZones(t, dnsbl, listed, masked)
-
-		if got := waiting(queue); len(got) != 1 || got[0].Detail["source"] != masked {
-			t.Errorf("alerts waiting %+v, want the failure's, from %s", got, masked)
-		}
-
-		scraped := httptest.NewRecorder()
-		m.ServeHTTP(scraped, httptest.NewRequestWithContext(t.Context(), http.MethodGet,
-			"/", http.NoBody))
-
-		for name, shown := range map[string]string{
-			"the log": log.String(), "the metrics": scraped.Body.String(),
-		} {
-			if strings.Contains(shown, key) || !strings.Contains(shown, masked) {
-				t.Errorf("%s shows the key, or does not name the zone:\n%s", name, shown)
-			}
-		}
-	})
-}
-
 //nolint:paralleltest // one at a time, as the comment at the top of this file says
 func TestVerdictsKeptAcrossARestart(t *testing.T) {
 	synctest.Test(t, func(t *testing.T) {

internal/requestlog/requestlog.go

@@ -139,7 +139,7 @@ type Line struct {
 	// minute_bytes, hour_bytes or day_bytes for a byte limit.
 	LimitHit string `json:"limit_hit,omitempty"`
 	// Reputation are the URLs of the blocklists that list the client, then
-	// the DNSBL zones whose verdict lists it, their keys masked.
+	// the DNSBL zones whose verdict lists it.
 	Reputation []string `json:"reputation,omitempty"`
 	// Offence is the offence the request was held as, OffenceLimit.
 	Offence string `json:"offence,omitempty"`