coraza icon indicating copy to clipboard operation
coraza copied to clipboard

Streamed Responses aren't Coming Through Until Finalized

Open svenakela opened this issue 1 year ago • 23 comments

Description

NextJs uses streams to send data asynchronously to web clients.

When a NextJs application is deployed behind a Coraza WAF extended Caddy server, streamed responses are hindered by the SecRuleEngine. If there for example is a spinner that NextJs wants the browser to render until data is fetched in the background, it will not show up and the browser will be dead silent until the data is coming through.

I've tried to disable everything body access, I've tried to write a rule that allows any response. Nothing helps except disabling SecRuleEngine completely.

Steps to reproduce

We've setup an example repo that reproduces the problem. The project consists of a small NextJs application behind a Caddy server. The issue is appearing by default in this repo.

https://github.com/svenakela/coraza-streaming-issue

Disable SecRuleEngine and the app works as expected.

Expected result

In this example project the bottom text should change to Loading... until data is sent by the app server.

Actual result

No textual change at all. The bottom text only change when the response is ready in the app server.

svenakela avatar Aug 02 '24 13:08 svenakela

Wow! This is perhaps the most fully documented, step-by-step issue I've ever seen on GitHub. Thank you @svenakela!

I was easily able to reproduce your described issue, thanks to your instructions and repo with the pre-prepared docker-compose file (I doubt I would have had the time to look at this issue, otherwise.)

I made the following observations:

  1. Using ModSecurity v3 + Nginx seems to work as intended. The behaviour from going via the ModSec+Nginx proxy seems to be the same as going directly to the nextjs-spinner container: "Loading..." is displayed.
  2. Using ModSecurity v2 + Apache results in the same behaviour as Coraza + Caddy: the ModSec+Apache proxy seems to wait until it has all chunks of the response before sending a single byte back to the client.
  3. With ModSecurity v2 + Apache, setting SecResponseBodyAccess to Off makes things work as expected: now that the proxy isn't interested in the response data it sends the chunks to the client as soon as they're available and "Loading..." is displayed.
  4. With Coraza + Caddy, setting SecResponseBodyAccess to Off makes no difference. Either this setting isn't being respected or Coraza and/or Caddy will always wait for a full response before sending anything back to the client. I'm not sure which is the case, but I would guess maybe the latter.

I hope this helps drive your issue forwards!

RedXanadu avatar Aug 05 '24 23:08 RedXanadu

According to https://github.com/corazawaf/coraza-caddy/blob/c2e0fbdc9c648550df8b2466ee5bf86bebbf2494/interceptor.go#L89-L104 if the response body isn't accessible nor processable we skip buffering. Would you mind trying with https://github.com/jcchavezs/coraza-httpbin @RedXanadu ?

jcchavezs avatar Aug 06 '24 01:08 jcchavezs

Wow! This is perhaps the most fully documented, step-by-step issue I've ever seen on GitHub. Thank you @svenakela! ... I made the following observations: .... 4. With Coraza + Caddy, setting SecResponseBodyAccess to Off makes no difference. Either this setting isn't being respected or Coraza and/or Caddy will always wait for a full response before sending anything back to the client. I'm not sure which is the case, but I would guess maybe the latter.

Thanks for the kind words. 4. Yes, that was my observation too, the SecResponseBodyAccess made no difference at all. Disabling the SecRuleEngine makes the issue go away, and a clean Caddy installation is behaving as expected. I would think that Caddy is innocent.

svenakela avatar Aug 07 '24 07:08 svenakela

According to https://github.com/corazawaf/coraza-caddy/blob/c2e0fbdc9c648550df8b2466ee5bf86bebbf2494/interceptor.go#L89-L104 if the response body isn't accessible nor processable we skip buffering. Would you mind trying with https://github.com/jcchavezs/coraza-httpbin @RedXanadu ?

I haven't checked the code, but is there a guarantee that the tx settings are set from the config?

svenakela avatar Aug 07 '24 07:08 svenakela

How can I help to proceed with this lil' bug of ours?

svenakela avatar Aug 14 '24 06:08 svenakela

I can confirm that this does not seem to be related to Caddy.

Here's a minimal example to reproduce the issue:

package main

import (
	"github.com/corazawaf/coraza/v3"
	txhttp "github.com/corazawaf/coraza/v3/http"
	"net/http"
	"time"
)

func main() {
	cfg := coraza.NewWAFConfig().WithDirectivesFromFile("coraza.conf") // SecRuleEngine Off = unbuffered, SecRuleEngine On = buffered
	waf, _ := coraza.NewWAF(cfg)
	http.ListenAndServe(":8000", txhttp.WrapHandler(waf, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		flusher, _ := w.(http.Flusher)
		w.WriteHeader(http.StatusOK)
		w.Write([]byte("Hello "))
		flusher.Flush()
		time.Sleep(5 * time.Second)
		w.Write([]byte("world!"))
	})))
}

TAR5 avatar Aug 17 '24 22:08 TAR5

It seems that the ResponseWriter is always wrapped, and the "response processor" outputs the buffered response? https://github.com/corazawaf/coraza/blob/main/http/interceptor.go#L140-L147 https://github.com/corazawaf/coraza/blob/main/http/middleware.go#L163-L171 https://github.com/corazawaf/coraza-caddy/blob/main/coraza.go#L133-L140

Is it really necessary to wrap the ResponseWriter, If tx.IsResponseBodyAccessible() && tx.IsResponseBodyProcessable() is false?

TAR5 avatar Aug 17 '24 23:08 TAR5

We're interested in disabling response buffering when SecResponseBodyAccess Off is set, too, and there is a related issue in the coraza-caddy project.

agclark27 avatar Oct 02 '24 14:10 agclark27

What do we think about this one, are we getting into any conclusions?

svenakela avatar Oct 31 '24 10:10 svenakela

We found the problem, we are generating a patch. Technically we are forcing the interceptor even if its not required

jptosso avatar Nov 07 '24 10:11 jptosso

Any status?

svenakela avatar Jan 03 '25 12:01 svenakela

We tried to fix it but it is taking a lot of effort. The issue is here https://github.com/corazawaf/coraza/blob/44c991b123634020e2ccf986ef1d442d1602806d/http/interceptor.go#L160 , basically the inteceptor is always created even if there is no need for it. It requires a full refactor of this function

jptosso avatar Jan 03 '25 12:01 jptosso

Thanks for your effort though, I'll wait patiently.

svenakela avatar Jan 03 '25 16:01 svenakela

This is preventing use of SSE, so a resolution to this issue would be nice. The handler Coraza uses does not implements http.Flusher so even just adding that would help a lot.

MagicalTux avatar Sep 18 '25 03:09 MagicalTux

I might be missing something but according to this line we are not buffering unless we are supposed to inspect the response. See https://github.com/corazawaf/coraza/blob/44c991b123634020e2ccf986ef1d442d1602806d/http/interceptor.go#L99-L100

On Thu, Sep 18, 2025 at 5:41 AM Mark Karpelès @.***> wrote:

MagicalTux left a comment (corazawaf/coraza#1120) https://github.com/corazawaf/coraza/issues/1120#issuecomment-3305308297

This is preventing use of SSE https://developer.mozilla.org/en-US/docs/Web/API/Server-sent_events, so a resolution to this issue would be nice. The handler Coraza uses does not implements http.Flusher so even just adding that would help a lot.

— Reply to this email directly, view it on GitHub https://github.com/corazawaf/coraza/issues/1120#issuecomment-3305308297, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYARCPIWCXFU4WEQBYVD3TISWHAVCNFSM6AAAAACG2IJVFCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGMBVGMYDQMRZG4 . You are receiving this because you commented.Message ID: @.***>

jcchavezs avatar Sep 23 '25 19:09 jcchavezs

Sorry to bother, but any status update on this? :)

olgam4 avatar Oct 12 '25 19:10 olgam4

Nothing new I guess, but it is still a problem with streaming frameworks.

svenakela avatar Oct 13 '25 10:10 svenakela

@svenakela any chance you can write a failing test for this?

jcchavezs avatar Oct 13 '25 14:10 jcchavezs

@svenakela any chance you can write a failing test for this?

See the first post of this thread and its example.

svenakela avatar Oct 14 '25 13:10 svenakela

How can we solve this? When we stream, the first chunk and headers are sent before the body is inspected. If a rule matches later in phase 4, we can’t change headers/status that are already sent, we can only stop further writes or close the connection.

Same applies for the body... Even if we implement streaming inspection (i.e flushing the chunks immediately but process it first) we can't detect rules based on the entire body data because we have already sent it downstream...

So how would the decision be made whether to stream or buffer?

Additionally I am confused about the state of this issue. When I use our implementation as golang filter for envoy it looks like setting SecResponseBodyAccess off is enough to make streams work, while now as I am trying to write a testcase for this with "plain" coraza it looks like the engine has to be switched off completetly.

with this go file and curl we can see the difference... only when the rule engine is completely disabled the client gets a message every second... for both paths /body_access_off and /body_access_on the data only arrives after 10 seconds in one bunch:

package main

import (
	"fmt"
	"log"
	"net/http"
	"strings"
	"time"

	coraza "github.com/corazawaf/coraza/v3"
	txhttp "github.com/corazawaf/coraza/v3/http"
)

func main() {

	waf1 := createWAF(strings.TrimSpace(`SecRuleEngine Off`))

	waf2 := createWAF(strings.TrimSpace(`
SecRuleEngine On
SecResponseBodyAccess Off`))

	waf3 := createWAF(strings.TrimSpace(`
SecRuleEngine On
SecResponseBodyMimeType text/plain
SecResponseBodyAccess On`))

	mux := http.NewServeMux()
	myHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Content-Type", "text/plain")
		w.WriteHeader(http.StatusOK)
		// Loop 10 times, sleeping 1s between writes
		for i := 0; i < 10; i++ {
			fmt.Fprintf(w, "Hello world! (%d)\n", i)
			if f, ok := w.(http.Flusher); ok {
				f.Flush()
			}
			time.Sleep(1 * time.Second)
		}
	})

	// Route 1: Protected by waf1
	mux.Handle("/engine_off", txhttp.WrapHandler(waf1, myHandler))

	// Route 2: Protected by waf2
	mux.Handle("/body_access_off", txhttp.WrapHandler(waf2, myHandler))

	// Route 3: Protected by waf3
	mux.Handle("/body_access_on", txhttp.WrapHandler(waf3, myHandler))

	log.Printf("Starting WAF server on port 8080")
	log.Printf("Rule engine off: curl -i 'http://localhost:8080/engine_off'")
	log.Printf("Rule engine on, response body access off: curl -i 'http://localhost:8080/body_access_off'")
	log.Printf("Rule engine on, response body access on: curl -i 'http://localhost:8080/body_access_on'")
	if err := http.ListenAndServe(":8080", mux); err != nil {
		log.Fatal(err)
	}

}

func createWAF(directives string) coraza.WAF {
	var cfg coraza.WAFConfig
	cfg = coraza.NewWAFConfig()

	waf, err := coraza.NewWAF(cfg.WithDirectives(directives))
	if err != nil {
		log.Fatalf("failed to create WAF: %v", err)
	}
	return waf
}

daum3ns avatar Nov 19 '25 10:11 daum3ns

i have added streaming_test.go in my fork: https://github.com/daum3ns/coraza/blob/add-testcase-for-streamed-responses/http/streaming_test.go

but it confuses me even more because only the last test fails... ()

I would have expected all testcases except the first one to fail based on my manual analysis with the go file above...

EDIT: Tests now are correct and show the same behavior then the go script above!

daum3ns avatar Nov 19 '25 16:11 daum3ns

@jcchavezs i think the testcases prove this is a bug. the underlying problem is that, if SecResponseBodyAccess is off we should not buffer the response body data...

I think implementing a "streaming mode" is not easy if even possible, but at least we should address the buffering topic...

daum3ns avatar Nov 20 '25 12:11 daum3ns

@jcchavezs i think the testcases prove this is a bug. the underlying problem is that, if SecResponseBodyAccess is off we should not buffer the response body data...

I think implementing a "streaming mode" is not easy if even possible, but at least we should address the buffering topic...

Yes, I agree with this. The SecResponseBodyAccess setting should let the stream flow by.

svenakela avatar Nov 22 '25 15:11 svenakela