🐛 close the remaining gaps with CrowdSec's bot-detection protocol (#403)

* 🐛 ban LAPI decisions of an unknown type, such as challenge In stream mode a decision whose type is neither ban nor captcha was cached as an empty value, which reads back as a cache miss, so the request was allowed. The challenge protocol says a challenge decision from the LAPI or a blocklist carries nothing to serve and must fail closed to ban, and CrowdSec's own bouncer falls back to ban for any type it does not support. Unknown types are now cached as a ban in both modes. Live mode already banned them, but it never cached them, so it queried the LAPI again on every request. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * 🐛 log unexpected AppSec statuses at INFO A 401 or any other unexpected AppSec status was turned into a ban and only logged at DEBUG, in handleNextServeHTTP. AppSec answers 401 when it rejects the bouncer key, and also when it cannot reach the LAPI to validate a key it has not cached yet. With the default INFO logs, every visitor was then banned with no trace in the plugin logs. The status is now logged at INFO, as a 500 already is. The behaviour does not change, and the README now states it: such a status always blocks, whatever crowdsecAppsecFailureBlock says, and a 502, 503 or 504 counts as unreachable. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * 🐛 serve the captcha when AppSec answers with a captcha action Any AppSec action other than allow or challenge was turned into a ban, so a captcha remediation from AppSec (set by a rule or a hook) banned the visitor. The challenge protocol says to apply the action AppSec returns, and CrowdSec's own bouncer serves the captcha. A captcha action now uses the configured captcha the same way a LAPI captcha decision does: an IP that already solved it goes through, and other requests get the captcha page. The form posts back to the same URL, so it is validated when AppSec answers that post with a captcha again. Without a provider, or for a HEAD request, it falls back to ban as before. The decision handling moves to its own method so that it does not loop back into the AppSec query. In crowdsecMode appsec, New returned before setting up the cache and the captcha client, so the captcha could never be served there. It now returns after them. Without captchaProvider nothing changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * 🐛 never forward /crowdsec-internal/challenge/* to the backend The plugin relied on AppSec to answer the internal challenge paths. When AppSec did not, the request went on to the backend: AppSec allows the custom script path when no custom script is configured, and every path passes when the plugin fails open because AppSec is unreachable or failing. The challenge protocol requires the remediation component to terminate these paths and never pass them to the origin. With AppSec enabled, a request under that prefix that AppSec did not answer now gets a 404. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * 🐛 count a challenge once in the blocked requests, not its assets Every challenge response counted as a dropped request in the metrics pushed to the LAPI: the page, the fingerprint and proof-of-work scripts, and the proof submission, even a successful one. A visitor who passed the challenge showed up as four blocked requests. Responses on /crowdsec-internal/challenge/* are part of a challenge that was already counted, so only the challenge page counts now. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * 📝 document that bot detection needs the challenge submission body The challenge proof is posted to /crowdsec-internal/challenge/submit, and AppSec only accepts it if it receives the whole form. With crowdsecAppsecBodyLimit set to 0 the plugin sends no body, and with a smaller limit the form is cut short. In both cases the proof is rejected and the visitor is challenged forever, with nothing in the logs to explain it. The default of 10MB is enough. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-10-17 23:19:37 +02:00 · 2026-10-17 19:40:36 +02:00
1 parent e740cdaee4
commit 3926309ec6
@@ -376,10 +376,12 @@ make run
  - bool
  - default: true
  - Block request when Crowdsec Appsec Server have a [status 500](https://docs.crowdsec.net/docs/next/appsec/protocol#response-code).
+  - A status 401 (API key rejected) or any other unexpected status always blocks the request, whatever this option says, and is logged at INFO.
 - CrowdsecAppsecUnreachableBlock
  - bool
  - default: true
  - Block request when Crowdsec Appsec Server is unreachable.
+  - A status 502, 503 or 504 counts as unreachable.
 - CrowdsecAppsecBodyLimit
  - int64
  - default: 10485760 (= 10MB)
@@ -503,6 +505,7 @@ make run
 - CaptchaProvider
  - string
  - Provider to validate the captcha, expected values are: `hcaptcha`, `recaptcha`, `turnstile` or `custom`
+  - Also used when AppSec answers with a `captcha` action. Without a provider, those requests are banned.
 - CaptchaCustomJsURL
  - string
  - If CaptchaProvider is `custom`, URL used to load the challenge in the HTML (in case of hcaptcha: `https://hcaptcha.com/1/api.js`)
@@ -771,8 +774,16 @@ with no error anywhere:
 ```

 The backend service never sees these requests: the plugin forwards them to the AppSec
-component and returns its response directly, so the prefix only needs to reach a router
-carrying the middleware.
+component and returns its response directly, and answers `404` when AppSec does not serve
+them (for example when it is unreachable), so the prefix only needs to reach a router
+carrying the middleware. Requests from `clientTrustedIps` bypass the plugin, so they are
+the only exception.
+
+The proof-of-work result is posted to `/crowdsec-internal/challenge/submit`, and AppSec
+has to receive that form in full. Keep `crowdsecAppsecBodyLimit` above `0` and large enough
+for it, which the default of 10MB is. With `0` the plugin sends no body to AppSec, and with
+a smaller limit the proof is cut short: either way it is rejected and the visitor is
+challenged again forever.

 CrowdSec documents the same requirement, that the bouncer must forward
 `/crowdsec-internal/challenge/*` unchanged: see
@@ -45,6 +45,8 @@ const (
 	cacheTimeoutKey                 = "updated"
 	appsecAllowAction               = "allow"
 	appsecChallengeAction           = "challenge"
+	appsecCaptchaAction             = "captcha"
+	appsecChallengePathPrefix       = "/crowdsec-internal/challenge/"
 	appsecResponseBodyLimit         = 1 << 20 // 1 MiB
 )

@@ -275,9 +277,6 @@ func New(_ context.Context, next http.Handler, config *configuration.Config, nam
 		cacheClient:   &cache.Client{},
 		captchaClient: &captcha.Client{},
 	}
-	if config.CrowdsecMode == configuration.AppsecMode {
-		return bouncer, nil
-	}
 	config.RedisCachePassword, _ = configuration.GetVariable(config, "RedisCachePassword")
 	bouncer.cacheClient.New(
 		log,
@@ -311,6 +310,9 @@ func New(_ context.Context, next http.Handler, config *configuration.Config, nam
 		log.Error("CaptchaClient not valid " + err.Error())
 		return nil, err
 	}
+	if config.CrowdsecMode == configuration.AppsecMode {
+		return bouncer, nil
+	}

 	if (config.CrowdsecMode == configuration.StreamMode || config.CrowdsecMode == configuration.AloneMode) && streamTicker == nil {
 		if config.CrowdsecMode == configuration.AloneMode {
@@ -508,20 +510,39 @@ func (bouncer *Bouncer) handleNextServeHTTP(rw http.ResponseWriter, req *http.Re
 			bouncer.handleBanServeHTTP(rw, req, remoteIP, configuration.ReasonAPPSEC)
 			return
 		}
-		if decision != nil && decision.Action != appsecAllowAction {
-			if decision.Action == appsecChallengeAction {
-				bouncer.handleAppsecResponseServeHTTP(rw, req, decision)
-				return
-			}
-			bouncer.handleBanServeHTTP(rw, req, remoteIP, configuration.ReasonAPPSEC)
+		if bouncer.handleAppsecDecisionServeHTTP(rw, req, remoteIP, decision) {
+			return
+		}
+		if strings.HasPrefix(req.URL.Path, appsecChallengePathPrefix) {
+			http.NotFound(rw, req)
 			return
 		}
 	}
 	bouncer.next.ServeHTTP(rw, req)
 }

+func (bouncer *Bouncer) handleAppsecDecisionServeHTTP(rw http.ResponseWriter, req *http.Request, remoteIP string, decision *AppSecResponse) bool {
+	switch {
+	case decision == nil || decision.Action == appsecAllowAction:
+		return false
+	case decision.Action == appsecChallengeAction:
+		bouncer.handleAppsecResponseServeHTTP(rw, req, decision)
+	case decision.Action == appsecCaptchaAction && bouncer.captchaClient.Valid && req.Method != http.MethodHead:
+		if bouncer.captchaClient.Check(remoteIP) {
+			return false
+		}
+		atomic.AddInt64(&blockedRequests, 1)
+		bouncer.captchaClient.ServeHTTP(rw, req, remoteIP)
+	default:
+		bouncer.handleBanServeHTTP(rw, req, remoteIP, configuration.ReasonAPPSEC)
+	}
+	return true
+}
+
 func (bouncer *Bouncer) handleAppsecResponseServeHTTP(rw http.ResponseWriter, req *http.Request, decision *AppSecResponse) {
-	atomic.AddInt64(&blockedRequests, 1)
+	if !strings.HasPrefix(req.URL.Path, appsecChallengePathPrefix) {
+		atomic.AddInt64(&blockedRequests, 1)
+	}

 	for name, values := range decision.UserHeaders {
 		rw.Header().Del(name)
@@ -631,6 +652,7 @@ func handleNoStreamCache(bouncer *Bouncer, remoteIP string) (string, error) {
 		value = cache.CaptchaValue
 	default:
 		bouncer.log.Info("handleStreamCache:unknownType " + decision.Type)
+		value = cache.BannedValue
 	}
 	if isLiveMode && bouncer.defaultDecisionTimeout > 0 {
 		durationSecond := int64(duration.Seconds())
@@ -720,6 +742,7 @@ func handleStreamCache(bouncer *Bouncer) error {
 				value = cache.CaptchaValue
 			default:
 				bouncer.log.Info("handleStreamCache:unknownType " + decision.Type)
+				value = cache.BannedValue
 			}
 			bouncer.cacheClient.Set(decision.Value, value, int64(duration.Seconds()))
 		}
@@ -896,6 +919,7 @@ func appsecQuery(bouncer *Bouncer, ip string, httpReq *http.Request) (*AppSecRes
 		}
 		return &decision, nil
 	default:
+		bouncer.log.Info(fmt.Sprintf("appsecQuery:unexpectedStatus statusCode:%d", res.StatusCode))
 		return nil, fmt.Errorf("appsecQuery: statusCode:%d", res.StatusCode)
 	}
 }
@@ -1037,3 +1037,86 @@ func Test_appsecQuery_forwardsHTTPVersion(t *testing.T) {
 		})
 	}
 }
+
+func TestServeHTTPServesAppsecCaptchaInAppsecMode(t *testing.T) {
+	appsecServer := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) {
+		rw.WriteHeader(http.StatusForbidden)
+		_, _ = rw.Write([]byte(`{"action":"captcha","http_status":403}`))
+	}))
+	defer appsecServer.Close()
+
+	appsecURL, err := url.Parse(appsecServer.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	cfg := CreateConfig()
+	cfg.Enabled = true
+	cfg.CrowdsecMode = configuration.AppsecMode
+	cfg.CrowdsecLapiKey = "test"
+	cfg.CrowdsecAppsecEnabled = true
+	cfg.CrowdsecAppsecHost = appsecURL.Host
+	cfg.CaptchaProvider = configuration.HcaptchaProvider
+	cfg.CaptchaSiteKey = "test-site-key"
+	cfg.CaptchaSecretKey = "secret"
+	cfg.CaptchaFilePath = "captcha.html"
+	nextCalled := false
+	handler, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {
+		nextCalled = true
+	}), cfg, "test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	cacheClient := &cache.Client{}
+	cacheClient.New(logger.New("INFO", ""), false, "", nil, "", "")
+	t.Cleanup(func() { cacheClient.Delete("192.0.2.30_captcha") })
+
+	for _, solved := range []bool{false, true} {
+		nextCalled = false
+		if solved {
+			cacheClient.Set("192.0.2.30_captcha", cache.CaptchaDoneValue, 60)
+		}
+		req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
+		req.RemoteAddr = "192.0.2.30:1234"
+		recorder := httptest.NewRecorder()
+		handler.ServeHTTP(recorder, req)
+
+		if nextCalled != solved {
+			t.Errorf("solved=%t: expected next handler called=%t", solved, solved)
+		}
+		if !solved && (recorder.Code != http.StatusOK || !strings.Contains(recorder.Body.String(), "test-site-key")) {
+			t.Errorf("expected the captcha page, got status %d", recorder.Code)
+		}
+	}
+}
+
+func TestHandleNextServeHTTPKeepsChallengePathsFromTheBackend(t *testing.T) {
+	appsecServer := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) {
+		rw.WriteHeader(http.StatusOK)
+	}))
+	defer appsecServer.Close()
+
+	appsecURL, err := url.Parse(appsecServer.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	nextCalled := false
+	bouncer := &Bouncer{
+		next: http.HandlerFunc(func(http.ResponseWriter, *http.Request) {
+			nextCalled = true
+		}),
+		appsecEnabled:    true,
+		appsecScheme:     appsecURL.Scheme,
+		appsecHost:       appsecURL.Host,
+		appsecPath:       "/",
+		httpAppsecClient: appsecServer.Client(),
+		log:              logger.New("INFO", ""),
+	}
+
+	recorder := httptest.NewRecorder()
+	req := httptest.NewRequest(http.MethodGet, "http://example.com/crowdsec-internal/challenge/custom.js", nil)
+	bouncer.handleNextServeHTTP(recorder, req, "192.0.2.10")
+
+	if nextCalled || recorder.Code != http.StatusNotFound {
+		t.Fatalf("expected a 404 without reaching the backend, got status %d (next called: %t)", recorder.Code, nextCalled)
+	}
+}