DetectionOnly mode seems to change 201 response to 200

[MrWako]

Description

I'm running the WAF v3.0.4 via the http middleware in DetectionOnly mode, using the coraza packaged core ruleset (with a few additional custom rules), and with request body access on, but without response body access off. For some POST requests with a JSON body we are seeing the expected status change from 201, to 200. Its not on every request and I haven't identified what is triggering it.

I'm wondering if for some reason the requests are falling into line 146 somehow:

w.WriteHeader(obtainStatusCodeFromInterruptionOrDefault(it, http.StatusOK))

Steps to reproduce

I'm working on a reproducer and appreciate that we can't start analyzing the issue until we have one. However thought it was worth logging in case anyone else is seeing the same issue or has made progress analysing it.

Expected result

In DetectionOnly mode I don't expect any effect on the traffic flowing through the WAF, only logging if any rules are broken

Actual result

• I'm running with an WithErrorCallback logger specified but without a WithDebugLogger
• I don't see any logs from the WAF to say that a rule has been triggered
• however on some requests the response code is modified from 201 to 200 (which causes downstream problems)

[jcchavezs]

Detection mode isn't supposed to perform any change in the request/response.

If it was the line you mentioned it must be a rule with action DENY and that sets code to 201. My suggestion is you enable debugLogger to see what are the logs around the blocking and possibly work out a feature that @airween mentioned a couple of times where we enable a feature in http middleware to return the matched rule in the response.

Related to https://github.com/corazawaf/coraza/issues/711

[jcchavezs]

You can do something like

package main

import (
    "context"
    "fmt"
    "log"
    "net/http"
    "os"
    "strings"

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

type reqIDKey struct{}

func requestIDWrap(next http.Handler) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
        requestID := req.Header.Get("X-Request-ID")
        if requestID == "" {
            next.ServeHTTP(w, req)
        }

        ctx := context.WithValue(req.Context(), reqIDKey{}, requestID)
        next.ServeHTTP(w, req.WithContext(ctx))
    })
}

func exampleHandler(w http.ResponseWriter, req *http.Request) {
    w.Header().Set("Content-Type", "text/plain")
    resBody := "Hello world, transaction not disrupted."

    if body := os.Getenv("RESPONSE_BODY"); body != "" {
        resBody = body
    }

    if h := os.Getenv("RESPONSE_HEADERS"); h != "" {
        key, val, _ := strings.Cut(h, ":")
        w.Header().Set(key, val)
    }

    // The server generates the response
    w.Write([]byte(resBody))
}

func main() {
    waf := createWAF()

    http.Handle("/", requestIDWrap(txhttp.WrapHandler(waf, http.HandlerFunc(exampleHandler))))

    fmt.Println("Server is running. Listening port: 8090")

    log.Fatal(http.ListenAndServe(":8090", nil))
}

func createWAF() coraza.WAF {
    directivesFile := "./default.conf"
    if s := os.Getenv("DIRECTIVES_FILE"); s != "" {
        directivesFile = s
    }

    waf, err := coraza.NewWAF(
        coraza.NewWAFConfig().
            WithErrorCallback(logError).
            WithDirectivesFromFile(directivesFile),
    )
    if err != nil {
        log.Fatal(err)
    }
    return waf
}

type Contextual interface {
    Context() context.Context
}

func logError(r types.MatchedRule) {
    msg := r.ErrorLog()

    if cr, ok := r.(Contextual); ok {
        fmt.Printf("[logError][%s][%s] %s\n", r.Rule().Severity(), cr.Context().Value(reqIDKey{}), msg)
        return
    }

    fmt.Printf("[logError][%s] %s\n", r.Rule().Severity(), msg)
}

and use the request

curl -i -H "X-Request-ID: abc123" 'localhost:8090/hello?id=0'
HTTP/1.1 403 Forbidden
Date: Mon, 29 Jan 2024 12:11:16 GMT
Content-Length: 0

2024/01/29 13:11:16 [DEBUG] Rule matched tx_id="gbxGyHdssGPMEumIrWc" rule_id=1
[logError][emergency][abc123] [client "127.0.0.1"] Coraza: Access denied (phase 1). Invalid id [file "default.conf"] [line "4"] [id "1"] [rev ""] [msg "Invalid id"] [data ""] [severity "emergency"] [ver ""] [maturity "0"] [accuracy "0"] [hostname ""] [uri "/hello?id=0"] [unique_id "gbxGyHdssGPMEumIrWc"]

[MrWako]

Ok - think I've made a bit of progress identifying the problem I'm seeing. Seems like its related to using the http middleware along with the httputil.NewSingleHostReverseProxy, I've added a reproducer below. In my situation we are adding the WAF via the http middleware to a Go reverse proxy that sits in front of multiple services. The proxy stack uses negroni and we end up wrapping the response writer a number of times including with the negroni response writer, httpsnoop (an interesting one to look at but I've left it out of the reproducer) and the coraza interceptor. The issue seems to be that httputil.NewSingleHostReverseProxy flushes the response before the status has been written and without a status provided sets it to the default 200. In the reproducer below I force this with proxy.FlushInterval = -1. I don't have this in my actual proxy but it seems to accurately reproduce the issue that I'm seeing. Without the WAF middleware the test passes.

I haven't tested this but I think that if the response body triggered a rule and generated a 403 response, then this would also be returned as a 200.

running go1.20.1 linux/amd64

import (
    "bytes"
    "fmt"
    "net/http"
    "net/http/httptest"
    "net/http/httputil"
    "net/url"
    "testing"
    "time"

    coreruleset "github.com/corazawaf/coraza-coreruleset"
    "github.com/corazawaf/coraza/v3"
    coraza_http "github.com/corazawaf/coraza/v3/http"
    "github.com/corazawaf/coraza/v3/types"

    "github.com/stretchr/testify/assert"
    "github.com/urfave/negroni/v3"
)

func Test_WAF_ReverseProxy(t *testing.T) {
    // create the backend server for the proxy to hit. The backend server does some work then responds
    backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        time.Sleep(time.Second)
        w.WriteHeader(http.StatusCreated)
        w.Write([]byte("created"))
    }))
    backendURL, err := url.Parse(backend.URL)
    assert.NoError(t, err)

    // create the proxy middleware stack based on negroni with logger first
    m := negroni.New()
    m.Use(negroni.HandlerFunc(func(rw http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
        nrw := rw.(negroni.ResponseWriter)
        next(nrw, r)
        fmt.Printf("requst: %s, status: %d\n", r.RequestURI, nrw.Status())
    }))

    // add the WAF. Comment out this block and we get the expected resonse codes back
    waf := testWAF(t)
    m.Use(negroni.HandlerFunc(func(rw http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
        h := coraza_http.WrapHandler(waf, next)
        h.ServeHTTP(rw, r)
    }))

    // point the proxy at the backend. We don't have FlushInterval set to -1 in our proxy but some
    // requests seem to trigger the same behaviour
    proxy := httputil.NewSingleHostReverseProxy(backendURL)
    proxy.FlushInterval = -1 // comment out this line and we get the expected status back
    m.UseHandler(proxy)

    // send the request
    rw := httptest.NewRecorder()
    r := testRequest()
    m.ServeHTTP(rw, r)
    assert.Equal(t, http.StatusCreated, rw.Code)
}

func testWAF(t *testing.T) coraza.WAF {
    errorCallBack := func(err types.MatchedRule) {
        msg := err.Message()
        id := err.Rule().ID()
        file := err.Rule().File()
        serverity := err.Rule().Severity().String()
        uri := err.URI()
        data := err.Data()
        fmt.Printf("WAF [%s] %s [%s] ID: %d [%s] URI: %s\n", serverity, msg, data, id, file, uri)
    }

    config := coraza.NewWAFConfig().
        WithErrorCallback(errorCallBack).
        WithRootFS(coreruleset.FS).
        WithDirectives("Include @coraza.conf-recommended").
        WithDirectives("Include @crs-setup.conf.example").
        WithDirectives("SecResponseBodyAccess Off").
        WithDirectives("SecRuleEngine On").
        WithDirectives("Include @owasp_crs/*.conf")

    waf, err := coraza.NewWAF(config)
    assert.NoError(t, err)
    return waf
}

func testRequest() *http.Request {
    target := `/foo`
    body := `{"names":["ann", "bob"]}`
    return httptest.NewRequest(http.MethodPost, target, bytes.NewBuffer([]byte(body)))
}

[jcchavezs]

I am running the test in https://github.com/corazawaf/coraza/pull/984 and in local it says green.

[MrWako]

Thanks @jcchavezs for taking a look at this. I can confirm that if I pull your branch the test passes. I can see that in the PR we are trying to run the test against v3.0.4. However, if I checkout the repo at v3.0.4 and add in the new /http/integration directory (and update the go.work file) then the test fails. My suspicion now is that the issue is fixed in master, possibly https://github.com/corazawaf/coraza/pull/923.

Unfortunately, by default Go will pull the latest tagged version so we're missing this fix. I'll pull the head of master into our service and see if we still observe the issue. Any plan to tag a v3.0.5 in the near future?

[jcchavezs]

I see. Yes, it is very likely we push s new release 3.1 between today and tomorrow.

On Tue, 6 Feb 2024, 17:28 Mark Wakefield, @.***> wrote:

Thanks @jcchavezs https://github.com/jcchavezs for taking a look at this. I can confirm that if I pull your branch the test passes. I can see that in the PR we are trying to run the test against v3.0.4. However, if I checkout the repo at v3.0.4 and add in the new /http/integration directory (and update the go.work file) then the test fails. My suspicion now is that the issue is fixed in master, possibly #923 https://github.com/corazawaf/coraza/pull/923.

Unfortunately, by default Go will pull the latest tagged version so we're missing this fix. I'll pull the head of master into our service and see if we still observe the issue. Any plan to tag a v3.0.5 in the near future?

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

[MrWako]

I can confirm that the issue is fixed in master so I think this can be resolved/closed if you want (might want to wait until we have a new version tag). Thanks.

[jcchavezs]

3.1 is in the oven. We are just waiting for a release on go-ftw.

[MrWako]

awesome - thanks!

[jcchavezs]

New release is out https://github.com/corazawaf/coraza/releases/tag/v3.1.0 :D