🐛 appsec: do not buffer unreadable (gRPC/HTTP2) request bodies#332
Open
mathieuHa wants to merge 3 commits into
Open
🐛 appsec: do not buffer unreadable (gRPC/HTTP2) request bodies#332mathieuHa wants to merge 3 commits into
mathieuHa wants to merge 3 commits into
Conversation
maxlerebourg
reviewed
Jun 5, 2026
| switch { | ||
| case isBodyUnreadable(httpReq): | ||
| if bouncer.appsecDropUnreadableBody { | ||
| bouncer.log.Debug(fmt.Sprintf("appsecQuery:unreadableBody ip:%s dropped:true", ip)) |
Owner
There was a problem hiding this comment.
not needed, it's logged at the caller lever
Collaborator
Author
There was a problem hiding this comment.
Fixed in 3ca63bc — removed the redundant log.Debug; the caller (handleNextServeHTTP) already logs the returned error with the IP.
A bidirectional gRPC stream is an HTTP/2 request with no Content-Length whose body never reaches EOF. Since #321 removed the ContentLength guard, appsecQuery buffered it with io.ReadAll, which blocked until the request timed out and was turned into a 403 (issue #323). The backend was never reached (OriginStatus:0). Mirror the reference lua-cs-bouncer behaviour: detect an unreadable body (ProtoMajor >= 2 && ContentLength < 0) and, instead of buffering it, forward the request to Appsec with headers only. Add a new CrowdsecAppsecDropUnreadableBody option (default false) that mirrors the reference APPSEC_DROP_UNREADABLE_BODY: when true, such requests are blocked outright instead of forwarded without their body. Readable HTTP/1.1 bodies are still buffered and inspected, so the bypass closed by #321 stays closed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rewrite the body-handling if/else chain in appsecQuery as a switch (gocritic) and use US spelling "behavior" (misspell). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mathieuHa
force-pushed
the
fix/323-grpc-appsec-unreadable-body
branch
from
1de51c4 to
4305080
Compare
Address review on #332: the caller (handleNextServeHTTP) already logs the returned error with the request IP, so the inner Debug line duplicated it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Fixes #323.
After #321, AppSec silently returns
403on long-lived gRPC streams (e.g. NetBirdConnectStream) when AppSec is enabled.A bidirectional gRPC stream is an HTTP/2 request with no
Content-Lengthwhose body never reaches EOF. #321 removed thehttpReq.ContentLength > 0guard inappsecQuery, so the AppSec path now buffers every body withio.ReadAll. For an open stream that read blocks until the request times out, producing a403before the backend is reached (OriginStatus:0). Short unary gRPC calls succeed; only streams hang.Fix
Mirror the reference
lua-cs-bouncer, which refuses to read the body of an HTTP/2+ request with noContent-Lengthand then either forwards headers-only or drops it (APPSEC_DROP_UNREADABLE_BODY).isBodyUnreadable(req)→req.Body != nil && req.ProtoMajor >= 2 && req.ContentLength < 0(the Go equivalent of the luahttp_version() >= 2 and http_content_length == nilcheck).appsecQueryno longer buffers it:CrowdsecAppsecDropUnreadableBodyoption (defaultfalse, mirrorsAPPSEC_DROP_UNREADABLE_BODY) → block the request outright.Tests
Test_isBodyUnreadable— HTTP/2 & HTTP/3 without Content-Length (true), HTTP/2 with Content-Length (false), HTTP/1.1 chunked (false), no body (false).Test_appsecQuery_streamingDoesNotBlock— regression for [BUG] Silently returns 403 on gRPC streaming connections #323: a never-EOF gRPC body returns within 2s (hangs ~forever / 2s timeout without the fix).Test_appsecQuery_dropUnreadableBody— verifies drop mode blocks.go test ./...,go vet ./...andyaegi test -v .(Traefik plugin runtime) all pass.Docs
CrowdsecAppsecDropUnreadableBodydocumented in the variables list and the full dynamic-configuration example.