Skip to content

Fail closed on AppSec connection errors (by default), and add configurable timeout#110

Merged
hslatman merged 6 commits into
hslatman:mainfrom
com6056:fail-open-appsec-connection-error
Apr 4, 2026
Merged

Fail closed on AppSec connection errors (by default), and add configurable timeout#110
hslatman merged 6 commits into
hslatman:mainfrom
com6056:fail-open-appsec-connection-error

Conversation

@com6056

@com6056 com6056 commented Mar 13, 2026 •
edited by hslatman
Loading

Copy link
Copy Markdown
Contributor

Summary

When the AppSec component is unreachable (e.g. CrowdSec is stopped or restarting), the current behavior blocks or errors requests. This PR changes it to be configurable to fail open: if enabled, and the AppSec endpoint cannot be contacted, the request is logged and allowed through rather than being blocked.

This also adds a configurable appsec_timeout Caddyfile option (default: 2s) that controls how long the AppSec client waits before giving up. This is important for tuning fail-open latency — on a local/Docker network, operators may want to set a lower value (we tested with 100ms) so that the overhead when AppSec is unavailable stays negligible.

Changes

  • Fail open on connection errors (if enabled): checkRequest now distinguishes connection errors from AppSec deny decisions. On connection error, logs a warning and returns nil (allow) rather than propagating the error.
  • Configurable appsec_timeout: New Caddyfile option on the crowdsec global block. Defaults to 2s. Sets the HTTP client timeout, dial timeout, and TLS handshake timeout for AppSec requests uniformly.
  • Reduced default timeouts: The AppSec HTTP client previously used Go's transport defaults (30s dial timeout). The new default of 2s is more appropriate for an inline security check that should fail fast.

Tested

Verified on a live setup with Caddy + CrowdSec in Docker, using appsec_timeout 100ms:

  • CrowdSec running: ~16ms per request (no change from baseline)
  • CrowdSec stopped: ~100ms per request (timeout fires, fail open, request succeeds)

🤖 Generated with Claude Code

com6056 and others added 3 commits March 5, 2026 22:25
@com6056 @claude
fail open when AppSec component is unreachable
6682b22 
Connection errors (e.g. connection refused) were returned as errors,
causing a 500 response to clients when the AppSec component is down.
This is inconsistent with how HTTP 401/404/500 responses from AppSec
are handled (all of which log and return nil to fail open).

Apply the same fail-open behavior for connection-level errors, and add
a test case covering this scenario.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@com6056 @claude
reduce AppSec client timeouts to minimize latency on unavailability
874bda1 
The previous 10s client / 30s dial timeouts meant every request could
block for up to 10 seconds waiting for the AppSec connection to fail
when CrowdSec is down. Reduce the dial and TLS timeouts to 500ms (fast
enough for any local Docker network) and cap the overall request at 2s.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@com6056 @claude
add configurable appsec_timeout option
92bce23 
The AppSec client timeout now defaults to 2s and is configurable via
appsec_timeout in the Caddyfile. Dial and TLS handshake timeouts are
set equal to the overall timeout, so all timeouts scale together.

This is particularly relevant with fail-open behavior: when the AppSec
component is unavailable, requests are delayed by up to appsec_timeout
before failing open, so users should tune this to their network latency.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@com6056

com6056 commented Mar 20, 2026

Copy link
Copy Markdown
Contributor Author

👋 Hey @hslatman, no rush but would love a review whenever you get a chance, thanks!

hslatman
hslatman reviewed Mar 26, 2026
View reviewed changes
Comment thread internal/bouncer/bouncer_test.go Outdated
Comment thread internal/bouncer/appsec.go Outdated
Comment thread crowdsec/crowdsec.go
Comment on lines +301 to +306
func (c *CrowdSec) appSecTimeout() time.Duration {
if c.AppSecTimeout == 0 {
return 2 * time.Second
}
return time.Duration(c.AppSecTimeout)
}

@hslatman hslatman Mar 26, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@hslatman

hslatman commented Mar 27, 2026

Copy link
Copy Markdown
Owner

Btw, the change to have a shorter timeout by default certainly makes sense. I don't use AppSec myself, so I'm only testing it locally during development, and not in an actual production environment, so never hit that issue myself.

@com6056 @claude
add configurable appsec fail mode with hard-fail default
dcefbb6 
Address review feedback:
- Add enable_appsec_fail_open config knob (Caddyfile + JSON) to control
  whether AppSec errors allow requests through or block them
- Default to fail-hard (blocking) for safety; users can opt into
  fail-open with enable_appsec_fail_open
- Extract appsec timeout to named var in tests for clarity
- Add test case for fail-hard on connection error

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@hslatman hslatman changed the title Fail open on AppSec connection errors, add configurable timeout Fail closed on AppSec connection errors (by default), and add configurable timeout Mar 31, 2026
hslatman
hslatman reviewed Mar 31, 2026
View reviewed changes

@hslatman hslatman left a comment •
edited
Loading

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor rename of the setting left. appsec_enable_fail_open or appsec_enable_hard_fails could also work. The last one is most in line with the more general enable_hard_fails, but looking back at that naming choice doesn't make me happy 😅

Comment thread crowdsec/crowdsec.go Outdated
Comment thread crowdsec/caddyfile.go Outdated
@com6056 @hslatman
Apply suggestions from code review
7f10d43 
Co-authored-by: Herman Slatman <hslatman@users.noreply.github.com>
@hslatman hslatman merged commit 56f761d into hslatman:main Apr 4, 2026
2 checks passed
@hslatman

hslatman commented Apr 4, 2026

Copy link
Copy Markdown
Owner

I've released https://github.com/hslatman/caddy-crowdsec-bouncer/releases/tag/v0.11.0 with the changes from this PR.

Thank you, @com6056! 😄

@ValentinVie

ValentinVie commented Apr 7, 2026

Copy link
Copy Markdown

Hey, thanks for the new feature. I would like to use it but I could not find the documentation on how to use it ? Does it apply to all hosts in caddy or we can select the configuration per host ? Thanks for your help !

@hslatman

hslatman commented Apr 7, 2026

Copy link
Copy Markdown
Owner

Hey, thanks for the new feature. I would like to use it but I could not find the documentation on how to use it ? Does it apply to all hosts in caddy or we can select the configuration per host ? Thanks for your help !

The new options in the Caddyfile are here: https://github.com/hslatman/caddy-crowdsec-bouncer/pull/110/changes#diff-71746e5ee17fe78a2ad76d2fcb3be5ab596738f40c9b0c907efce1b68481152aR89-R102.

Currently these configurations are only available at the global Caddy level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hslatman hslatman hslatman approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@com6056 @hslatman @ValentinVie