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 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,13 +2000,12 @@ 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`, a
+it in use too. A fetch fails when the engine answers other than `200`, such as
-redirect included, such as `403` for a key it does not know, when it does not
+`403` for a key it does not know, when it does not finish within a minute, when
-finish within a minute, when the answer is longer than 16 MiB or is not a JSON
+the answer is longer than 16 MiB or is not a JSON list of decisions, or when a
-list of decisions, or when a decision to ban gives a value that is not an
+decision to ban gives a value that is not an address or a netblock, or a
-address or a netblock, or a `duration` that does not read. A failure is counted,
+`duration` that does not read. A failure is counted, logged and raised as a
-logged and raised as a `source_failure` alert, held back as a repeat within
+`source_failure` alert, held back as a repeat within `SWWAF_ALERT_COOLDOWN`.
-`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,10 +31,8 @@ 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, and rangeScope the scope of a
+	// ban is the type of a decision to ban, as CrowdSec names it.
-	// decision on a netblock, as CrowdSec names them.
+	ban = "ban"
-	ban        = "ban"
-	rangeScope = "Range"
 )
 
 func TestCrowdSecDecisionBansItsNetblockUntilItEndsEvenWithTheEngineDown(t *testing.T) {
@@ -48,7 +46,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)},
-			{rangeScope, "198.51.100.0/24", ban, probing, began.Add(time.Hour)},
+			{"Range", "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)},
@@ -113,64 +111,6 @@ 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,
 ) {
@@ -227,7 +167,7 @@ func TestCrowdSecFailureKeepsTheLastGoodCopyAlertsOncePerCooldownAndHidesTheKey(
 					t.Errorf("logged\n%s\nwant the failures", log.String())
 				}
 
-				wantKeyNotShown(t, e, log.String(), lists, queue)
+				wantKeyNotShown(t, log.String(), lists, queue)
 			})
 		})
 	}
@@ -257,11 +197,6 @@ 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>" },
@@ -287,24 +222,14 @@ func crowdSecFailures() []crowdSecFailure {
 	}
 }
 
-// wantKeyNotShown checks that no fetch carried the engine's key to a URL
+// wantKeyNotShown checks that the engine's key is in none of what the
-// other than its decision list, such as the one a redirect names, and that
+// fetches leave behind: log, the process log, the alerts waiting in queue,
-// the key is in none of what the fetches leave behind: log, the process
+// and the copies of lists, which reputation.json keeps.
-// log, the alerts waiting in queue, and the copies of lists, which
-// reputation.json keeps.
 func wantKeyNotShown(
-	t *testing.T, e *engine, log string, lists *reputation.Lists, queue *alerts.Queue,
+	t *testing.T, 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)
@@ -321,7 +246,7 @@ func TestCrowdSecDecisionListKeptAcrossARestartEndsWhenItsDecisionsDo(t *testing
 	synctest.Test(t, func(t *testing.T) {
 		began := time.Now()
 		e := &engine{key: engineKey, decisions: []decision{
-			{rangeScope, "198.51.100.0/24", ban, probing, began.Add(time.Hour)},
+			{"Range", "198.51.100.0/24", ban, probing, began.Add(time.Hour)},
 		}}
 		lists := start(t, e, crowdSecParams())
 		kept := lists.Snapshot()
@@ -388,18 +313,14 @@ 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,
-// with a redirect to redirect while that is not "", and 503 while failing.
+// and 503 while failing. It counts the fetches.
-// 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.
@@ -415,17 +336,11 @@ func (e *engine) RoundTrip(req *http.Request) (*http.Response, error) {
 
 	e.fetches++
 
-	if req.URL.String() != decisionsURL && req.Header.Get("X-Api-Key") != "" {
+	status, body := http.StatusOK, e.answer
-		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 == "":
@@ -435,7 +350,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:     header,
+		Header:     http.Header{},
 		Body:       io.NopCloser(strings.NewReader(body)),
 		Request:    req,
 	}, nil

internal/reputation/export_test.go

@@ -11,7 +11,6 @@ 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,10 +93,6 @@ 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.
@@ -133,16 +129,7 @@ type Decision struct {
 
 // New returns the lists, without a copy of any yet.
 func New(params Params) *Lists {
-	l := &Lists{
+	l := &Lists{params: params, httpClient: &http.Client{}, lists: map[string]*list{}}
-		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}}
@@ -429,9 +416,8 @@ 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, by crowdSecClient, which follows no redirect.
+// the engine looks for it. An answer other than 200, or a list longer
-// An answer other than 200, or a list longer than maxListBytes, is a
+// than maxListBytes, is a failure.
-// failure.
 func (l *Lists) get(ctx context.Context, listURL string) ([]string, error) {
 	ctx, cancel := context.WithTimeout(ctx, fetchTimeout)
 	defer cancel()
@@ -441,14 +427,11 @@ 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 := client.Do(req)
+	res, err := l.httpClient.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.