diff --git a/README.md b/README.md index ddd8929..21d6c97 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/bouncer.go b/bouncer.go index 5b91106..3889a01 100644 --- a/bouncer.go +++ b/bouncer.go @@ -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) } } diff --git a/bouncer_test.go b/bouncer_test.go index fac1392..a120ba3 100644 --- a/bouncer_test.go +++ b/bouncer_test.go @@ -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) + } +}