CrowdSec decision list fetched, kept, and its clients banned until the decision ends (closes #106)

SWWAF_CROWDSEC_LAPI_URL and SWWAF_CROWDSEC_LAPI_KEY name an engine whose
decision list, <url>/v1/decisions, is fetched every minute with the key in
X-Api-Key, following no redirect, and kept as a blocklist is: used while a
fetch fails, and across restarts through reputation.json. Ban decisions on an
Ip or a Range end at the fetch time plus their duration. A listed client's
request is refused and bans its netblock with the cause crowdsec until the
decision ends; bans.json, ban notes and metrics take the cause.

Judgement call: fetched every minute, not a setting.
Judgement call: a crowdsec ban never lengthens a limit ban.
Judgement call: a lifted crowdsec ban is remade while its decision lasts.

Model: opus-5-5

README.md

@@ -2000,12 +2000,13 @@ The list is fetched again a minute after it was last fetched or tried, the fetch
 failed or not, and is kept as a blocklist is (see "Blocklists" above): its last
 good copy, the engine's answer as it came, stays in use while a fetch fails, and
 `reputation.json` keeps it with the time it was fetched, so that a restart keeps
-it in use too. A fetch fails when the engine answers other than `200`, such as
-`403` for a key it does not know, when it does not finish within a minute, when
-the answer is longer than 16 MiB or is not a JSON list of decisions, or when a
-decision to ban gives a value that is not an address or a netblock, or a
-`duration` that does not read. A failure is counted, logged and raised as a
-`source_failure` alert, held back as a repeat within `SWWAF_ALERT_COOLDOWN`.
+it in use too. A fetch fails when the engine answers other than `200`, a
+redirect included, such as `403` for a key it does not know, when it does not
+finish within a minute, when the answer is longer than 16 MiB or is not a JSON
+list of decisions, or when a decision to ban gives a value that is not an
+address or a netblock, or a `duration` that does not read. A failure is counted,
+logged and raised as a `source_failure` alert, held back as a repeat within
+`SWWAF_ALERT_COOLDOWN`.
 
 The decisions of the type `ban` on an address or a netblock, the scopes `Ip` and
 `Range`, are used; any other, such as one to show a captcha or one on a country,

internal/reputation/crowdsec_test.go

@@ -31,8 +31,10 @@ const (
 	// sshBF and probing are scenarios of the engine's decisions.
 	sshBF   = "crowdsecurity/ssh-bf"
 	probing = "crowdsecurity/http-probing"
-	// ban is the type of a decision to ban, as CrowdSec names it.
+	// ban is the type of a decision to ban, and rangeScope the scope of a
+	// decision on a netblock, as CrowdSec names them.
 	ban        = "ban"
+	rangeScope = "Range"
 )
 
 func TestCrowdSecDecisionBansItsNetblockUntilItEndsEvenWithTheEngineDown(t *testing.T) {
@@ -46,7 +48,7 @@ func TestCrowdSecDecisionBansItsNetblockUntilItEndsEvenWithTheEngineDown(t *test
 			// A shorter decision on the same address, which is not the one
 			// used.
 			{"Ip", suspect, ban, sshBF, began.Add(4 * time.Hour)},
-			{"Range", "198.51.100.0/24", ban, probing, began.Add(time.Hour)},
+			{rangeScope, "198.51.100.0/24", ban, probing, began.Add(time.Hour)},
 			{"Ip", "2001:db8::1", ban, sshBF, began.Add(2 * time.Hour)},
 			// Left out: a decision to show a captcha, and one on a country.
 			{"Ip", "192.0.2.50", "captcha", probing, began.Add(time.Hour)},
@@ -111,6 +113,64 @@ func TestCrowdSecDecisionListFetchedAgainEveryMinute(t *testing.T) {
 	})
 }
 
+func TestCrowdSecDecisionOnAClientIsTheOneThatEndsLast(t *testing.T) {
+	t.Parallel()
+
+	synctest.Test(t, func(t *testing.T) {
+		began := time.Now()
+		e := &engine{key: engineKey, decisions: []decision{
+			// Two decisions on one address, the shorter listed first.
+			{"Ip", suspect, ban, sshBF, began.Add(2 * time.Hour)},
+			{"Ip", suspect, ban, probing, began.Add(4 * time.Hour)},
+			// 198.51.100.130 is held by a decision on its address that ends
+			// after the one on its netblock, and 192.0.2.20 by one that ends
+			// before.
+			{rangeScope, "198.51.100.128/25", ban, sshBF, began.Add(time.Hour)},
+			{"Ip", "198.51.100.130", ban, probing, began.Add(3 * time.Hour)},
+			{rangeScope, "192.0.2.0/24", ban, probing, began.Add(5 * time.Hour)},
+			{"Ip", "192.0.2.20", ban, sshBF, began.Add(2 * time.Hour)},
+		}}
+		lists := start(t, e, crowdSecParams())
+
+		wantDecision(t, lists, suspect,
+			reputation.Decision{Expires: began.Add(4 * time.Hour), Scenario: probing})
+		wantDecision(t, lists, "198.51.100.130",
+			reputation.Decision{Expires: began.Add(3 * time.Hour), Scenario: probing})
+		wantDecision(t, lists, "192.0.2.20",
+			reputation.Decision{Expires: began.Add(5 * time.Hour), Scenario: probing})
+	})
+}
+
+func TestCrowdSecAnswerOfNoDecisionIsAGoodCopyThatListsNoClient(t *testing.T) {
+	t.Parallel()
+
+	synctest.Test(t, func(t *testing.T) {
+		began := time.Now()
+		e := &engine{key: engineKey, decisions: []decision{
+			{"Ip", suspect, ban, sshBF, began.Add(4 * time.Hour)},
+		}}
+		lists := start(t, e, crowdSecParams())
+
+		// With its decision deleted, the engine answers null.
+		e.set(func(e *engine) { e.decisions = nil })
+		time.Sleep(time.Minute)
+		wantEngineFetches(t, e, 2)
+
+		want := []reputation.List{{
+			URL: decisionsURL, Tried: time.Now(), Fetched: time.Now(), Lines: []string{"null"},
+		}}
+		if got := lists.Snapshot(); !reflect.DeepEqual(got, want) {
+			t.Errorf("lists %+v, want %+v", got, want)
+		}
+
+		if lists.Failures(decisionsURL) != 0 {
+			t.Errorf("%d failures, want 0", lists.Failures(decisionsURL))
+		}
+
+		wantDecision(t, lists, suspect, reputation.Decision{})
+	})
+}
+
 func TestCrowdSecFailureKeepsTheLastGoodCopyAlertsOncePerCooldownAndHidesTheKey(
 	t *testing.T,
 ) {
@@ -167,7 +227,7 @@ func TestCrowdSecFailureKeepsTheLastGoodCopyAlertsOncePerCooldownAndHidesTheKey(
 					t.Errorf("logged\n%s\nwant the failures", log.String())
 				}
 
-				wantKeyNotShown(t, log.String(), lists, queue)
+				wantKeyNotShown(t, e, log.String(), lists, queue)
 			})
 		})
 	}
@@ -197,6 +257,11 @@ func crowdSecFailures() []crowdSecFailure {
 			func(e *engine) { e.key = "another-key-0123456789abcdef" },
 			"the server answered 403 Forbidden",
 		},
+		{
+			"a redirect",
+			func(e *engine) { e.redirect = "http://elsewhere.example/v1/decisions" },
+			"the server answered 302 Found",
+		},
 		{
 			"an answer that does not read",
 			func(e *engine) { e.answer = "<html>" },
@@ -222,14 +287,24 @@ func crowdSecFailures() []crowdSecFailure {
 	}
 }
 
-// wantKeyNotShown checks that the engine's key is in none of what the
-// fetches leave behind: log, the process log, the alerts waiting in queue,
-// and the copies of lists, which reputation.json keeps.
+// wantKeyNotShown checks that no fetch carried the engine's key to a URL
+// other than its decision list, such as the one a redirect names, and that
+// the key is in none of what the fetches leave behind: log, the process
+// log, the alerts waiting in queue, and the copies of lists, which
+// reputation.json keeps.
 func wantKeyNotShown(
-	t *testing.T, log string, lists *reputation.Lists, queue *alerts.Queue,
+	t *testing.T, e *engine, log string, lists *reputation.Lists, queue *alerts.Queue,
 ) {
 	t.Helper()
 
+	e.mu.Lock()
+	keySentTo := e.keySentTo
+	e.mu.Unlock()
+
+	if len(keySentTo) != 0 {
+		t.Errorf("the key was sent to %v", keySentTo)
+	}
+
 	shown, err := json.Marshal([]any{lists.Snapshot(), waiting(queue)})
 	if err != nil {
 		t.Fatalf("encode: %v", err)
@@ -246,7 +321,7 @@ func TestCrowdSecDecisionListKeptAcrossARestartEndsWhenItsDecisionsDo(t *testing
 	synctest.Test(t, func(t *testing.T) {
 		began := time.Now()
 		e := &engine{key: engineKey, decisions: []decision{
-			{"Range", "198.51.100.0/24", ban, probing, began.Add(time.Hour)},
+			{rangeScope, "198.51.100.0/24", ban, probing, began.Add(time.Hour)},
 		}}
 		lists := start(t, e, crowdSecParams())
 		kept := lists.Snapshot()
@@ -313,14 +388,18 @@ func TestLoadTakesACrowdSecListNeverFetchedAndRefusesACopyThatDoesNotRead(
 // decisions still in force, each with the time it has left as it answers,
 // by the bubble's clock, as an engine does, or with answer while that is
 // not "". It answers 403 to a fetch without its key, as an engine does,
-// and 503 while failing. It counts the fetches.
+// with a redirect to redirect while that is not "", and 503 while failing.
+// It counts the fetches, and notes in keySentTo the URL of each fetch of
+// another URL that carries a key, as one following a redirect would.
 type engine struct {
 	mu        sync.Mutex
 	key       string
 	decisions []decision
 	answer    string
+	redirect  string
 	failing   bool
 	fetches   int
+	keySentTo []string
 }
 
 // decision is a decision of the engine, which ends at expires.
@@ -336,11 +415,17 @@ func (e *engine) RoundTrip(req *http.Request) (*http.Response, error) {
 
 	e.fetches++
 
-	status, body := http.StatusOK, e.answer
+	if req.URL.String() != decisionsURL && req.Header.Get("X-Api-Key") != "" {
+		e.keySentTo = append(e.keySentTo, req.URL.String())
+	}
+
+	status, header, body := http.StatusOK, http.Header{}, e.answer
 
 	switch {
 	case req.URL.String() != decisionsURL || req.Header.Get("X-Api-Key") != e.key:
 		status, body = http.StatusForbidden, `{"message":"access forbidden"}`
+	case e.redirect != "":
+		status, header = http.StatusFound, http.Header{"Location": {e.redirect}}
 	case e.failing:
 		status, body = http.StatusServiceUnavailable, ""
 	case body == "":
@@ -350,7 +435,7 @@ func (e *engine) RoundTrip(req *http.Request) (*http.Response, error) {
 	return &http.Response{
 		StatusCode: status,
 		Status:     fmt.Sprintf("%d %s", status, http.StatusText(status)),
-		Header:     http.Header{},
+		Header:     header,
 		Body:       io.NopCloser(strings.NewReader(body)),
 		Request:    req,
 	}, nil

internal/reputation/export_test.go

@@ -11,6 +11,7 @@ import (
 // network.
 func (l *Lists) SetTransport(transport http.RoundTripper) {
 	l.httpClient.Transport = transport
+	l.crowdSecClient.Transport = transport
 }
 
 // SetTransport has a's checks go through transport instead of the

internal/reputation/reputation.go

@@ -93,6 +93,10 @@ type Params struct {
 type Lists struct {
 	params     Params
 	httpClient *http.Client
+	// crowdSecClient fetches the CrowdSec decision list. It follows no
+	// redirect, so that the key goes to the engine alone: a redirect is a
+	// failure.
+	crowdSecClient *http.Client
 
 	mu sync.Mutex
 	// lists are by URL, one for each URL Params names.
@@ -129,7 +133,16 @@ type Decision struct {
 
 // New returns the lists, without a copy of any yet.
 func New(params Params) *Lists {
-	l := &Lists{params: params, httpClient: &http.Client{}, lists: map[string]*list{}}
+	l := &Lists{
+		params:     params,
+		httpClient: &http.Client{},
+		crowdSecClient: &http.Client{
+			CheckRedirect: func(*http.Request, []*http.Request) error {
+				return http.ErrUseLastResponse
+			},
+		},
+		lists: map[string]*list{},
+	}
 
 	for _, listURL := range l.URLs() {
 		l.lists[listURL] = &list{kept: List{URL: listURL}}
@@ -416,8 +429,9 @@ func raiseFailure(queue *alerts.Queue, reason, source string, err error) {
 
 // get fetches the list at listURL, and returns its lines. The CrowdSec
 // decision list is fetched with CrowdSecKey in the header X-Api-Key, where
-// the engine looks for it. An answer other than 200, or a list longer
-// than maxListBytes, is a failure.
+// the engine looks for it, by crowdSecClient, which follows no redirect.
+// An answer other than 200, or a list longer than maxListBytes, is a
+// failure.
 func (l *Lists) get(ctx context.Context, listURL string) ([]string, error) {
 	ctx, cancel := context.WithTimeout(ctx, fetchTimeout)
 	defer cancel()
@@ -427,11 +441,14 @@ func (l *Lists) get(ctx context.Context, listURL string) ([]string, error) {
 		return nil, fmt.Errorf("make the request: %w", err)
 	}
 
+	client := l.httpClient
+
 	if listURL == l.params.CrowdSecDecisionsURL {
 		req.Header.Set("X-Api-Key", l.params.CrowdSecKey)
+		client = l.crowdSecClient
 	}
 
-	res, err := l.httpClient.Do(req)
+	res, err := client.Do(req)
 	if err != nil {
 		// Do's error names the URL, which the log line and the alert name
 		// already: only what went wrong is kept.