Skip to content

Commit

Permalink
Exports IsRuleEngineOff (#504)
Browse files Browse the repository at this point in the history
* Export engine status

* adding notes about current status returned

* adds example with RuleEngineStatus and RequestBodyAccessible in middleware, adds docs

* reverts status export, exports only engine off

* adds IsRuleEngineOff check into middleware, fixes name convention RuleEngine

* fix IsRuleEngineOff usage inside middleware, adds tests
  • Loading branch information
M4tteoP authored Nov 21, 2022
1 parent 69646c0 commit bffb435
Show file tree
Hide file tree
Showing 5 changed files with 164 additions and 67 deletions.
1 change: 1 addition & 0 deletions http/interceptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func wrap(w http.ResponseWriter, r *http.Request, tx types.Transaction) (
http.ResponseWriter,
func(types.Transaction, *http.Request) error,
) { // nolint:gocyclo

i := &rwInterceptor{w: w, tx: tx, proto: r.Proto}

responseProcessor := func(tx types.Transaction, r *http.Request) error {
Expand Down
9 changes: 9 additions & 0 deletions http/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ func processRequest(tx types.Transaction, req *http.Request) (*types.Interruptio
}

func WrapHandler(waf coraza.WAF, l Logger, h http.Handler) http.Handler {

fn := func(w http.ResponseWriter, r *http.Request) {
tx := waf.NewTransaction()
defer func() {
Expand All @@ -112,6 +113,14 @@ func WrapHandler(waf coraza.WAF, l Logger, h http.Handler) http.Handler {
}
}()

// Early return, Coraza is not going to process any rule
if tx.IsRuleEngineOff() {
// response writer is not going to be wrapped, but used as-is
// to generate the response
h.ServeHTTP(w, r)
return
}

// ProcessRequest is just a wrapper around ProcessConnection, ProcessURI,
// ProcessRequestHeaders and ProcessRequestBody.
// It fails if any of these functions returns an error and it stops on interruption.
Expand Down
201 changes: 134 additions & 67 deletions http/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,22 @@ func TestProcessRequest(t *testing.T) {
}
}

func TestProcessRequestEngineOff(t *testing.T) {
req, _ := http.NewRequest("POST", "https://www.coraza.io/test", strings.NewReader("test=456"))
waf := corazawaf.NewWAF()
waf.RuleEngine = types.RuleEngineOff
tx := waf.NewTransaction()
if _, err := processRequest(tx, req); err != nil {
t.Fatal(err)
}
if tx.Variables().RequestMethod().String() != "POST" {
t.Fatal("failed to set request from request object")
}
if err := tx.Close(); err != nil {
t.Fatal(err)
}
}

func TestProcessRequestMultipart(t *testing.T) {
req, _ := http.NewRequest("POST", "/some", nil)
if err := multipartRequest(t, req); err != nil {
Expand Down Expand Up @@ -228,17 +244,19 @@ func createWAF(t *testing.T) coraza.WAF {
return waf
}

type httpTest struct {
http2 bool
reqURI string
reqBody string
respHeaders map[string]string
respBody string
expectedProto string
expectedStatus int
expectedRespBody string
}

func TestHttpServer(t *testing.T) {
tests := map[string]struct {
http2 bool
reqURI string
reqBody string
respHeaders map[string]string
respBody string
expectedProto string
expectedStatus int
expectedRespBody string
}{
tests := map[string]httpTest{
"no blocking": {
reqURI: "/hello",
expectedProto: "HTTP/1.1",
Expand Down Expand Up @@ -286,71 +304,120 @@ func TestHttpServer(t *testing.T) {
// Perform tests
for name, tCase := range tests {
t.Run(name, func(t *testing.T) {
serverErrC := make(chan error, 1)
defer close(serverErrC)

// Spin up the test server
ts := httptest.NewUnstartedServer(WrapHandler(createWAF(t), t.Logf, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
if want, have := tCase.expectedProto, req.Proto; want != have {
t.Errorf("unexpected proto, want: %s, have: %s", want, have)
}

w.Header().Set("Content-Type", "text/plain")
w.Header().Add("coraza-middleware", "true")
for k, v := range tCase.respHeaders {
w.Header().Set(k, v)
}
w.WriteHeader(201)
_, err := w.Write([]byte(tCase.respBody))
if err != nil {
serverErrC <- err
}
})))
if tCase.http2 {
ts.EnableHTTP2 = true
ts.StartTLS()
} else {
ts.Start()
}
defer ts.Close()
runAgainstWaf(t, tCase, createWAF(t))

var reqBody io.Reader
if tCase.reqBody != "" {
reqBody = strings.NewReader(tCase.reqBody)
}
req, _ := http.NewRequest("POST", ts.URL+tCase.reqURI, reqBody)
// TODO(jcchavezs): Fix it once the discussion in https://github.com/corazawaf/coraza/issues/438 is settled
req.Header.Add("content-type", "application/x-www-form-urlencoded")
res, err := ts.Client().Do(req)
})
}
}

func TestHttpServerWithRuleEngineOff(t *testing.T) {
tests := map[string]httpTest{
"no blocking true negative": {
reqURI: "/hello",
expectedProto: "HTTP/1.1",
expectedStatus: 201,
respBody: "Hello!",
expectedRespBody: "Hello!",
},
"no blocking true positive header phase": {
reqURI: "/hello?id=0",
expectedProto: "HTTP/1.1",
expectedStatus: 201,
respBody: "Downstream works!",
expectedRespBody: "Downstream works!",
},
"no blocking true positive body phase": {
reqURI: "/hello",
reqBody: "eval('cat /etc/passwd')",
expectedProto: "HTTP/1.1",
expectedStatus: 201,
respBody: "Waf is Off!",
expectedRespBody: "Waf is Off!",
},
}
// Perform tests
for name, tCase := range tests {
t.Run(name, func(t *testing.T) {
waf, err := coraza.NewWAF(coraza.NewWAFConfig().
WithDirectives(`
SecRuleEngine Off
SecRequestBodyAccess On
SecRule ARGS:id "@eq 0" "id:1, phase:1,deny, status:403,msg:'Invalid id',log,auditlog"
SecRule REQUEST_BODY "@contains eval" "id:100, phase:2,deny, status:403,msg:'Invalid request body',log,auditlog"
`).WithErrorLogger(errLogger(t)).WithDebugLogger(&debugLogger{t: t}))
if err != nil {
t.Fatalf("unexpected error when performing the request: %v", err)
t.Fatal(err)
}
runAgainstWaf(t, tCase, waf)
})
}
}

if want, have := tCase.expectedStatus, res.StatusCode; want != have {
t.Errorf("unexpected status code, want: %d, have: %d", want, have)
}
func runAgainstWaf(t *testing.T, tCase httpTest, waf coraza.WAF) {
t.Helper()
serverErrC := make(chan error, 1)
defer close(serverErrC)

resBody, err := io.ReadAll(res.Body)
if err != nil {
t.Fatalf("unexpected error when reading the response body: %v", err)
}
// Spin up the test server
ts := httptest.NewUnstartedServer(WrapHandler(waf, t.Logf, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
if want, have := tCase.expectedProto, req.Proto; want != have {
t.Errorf("unexpected proto, want: %s, have: %s", want, have)
}

if want, have := tCase.expectedRespBody, string(resBody); want != have {
t.Errorf("unexpected response body, want: %q, have %q", want, have)
}
w.Header().Set("Content-Type", "text/plain")
w.Header().Add("coraza-middleware", "true")
for k, v := range tCase.respHeaders {
w.Header().Set(k, v)
}
w.WriteHeader(201)
_, err := w.Write([]byte(tCase.respBody))
if err != nil {
serverErrC <- err
}
})))
if tCase.http2 {
ts.EnableHTTP2 = true
ts.StartTLS()
} else {
ts.Start()
}
defer ts.Close()

err = res.Body.Close()
if err != nil {
t.Errorf("failed to close the body: %v", err)
}
var reqBody io.Reader
if tCase.reqBody != "" {
reqBody = strings.NewReader(tCase.reqBody)
}
req, _ := http.NewRequest("POST", ts.URL+tCase.reqURI, reqBody)
// TODO(jcchavezs): Fix it once the discussion in https://github.com/corazawaf/coraza/issues/438 is settled
req.Header.Add("content-type", "application/x-www-form-urlencoded")
res, err := ts.Client().Do(req)
if err != nil {
t.Fatalf("unexpected error when performing the request: %v", err)
}

select {
case err = <-serverErrC:
t.Errorf("unexpected error from server when writing response body: %v", err)
default:
return
}
})
if want, have := tCase.expectedStatus, res.StatusCode; want != have {
t.Errorf("unexpected status code, want: %d, have: %d", want, have)
}

resBody, err := io.ReadAll(res.Body)
if err != nil {
t.Fatalf("unexpected error when reading the response body: %v", err)
}

if want, have := tCase.expectedRespBody, string(resBody); want != have {
t.Errorf("unexpected response body, want: %q, have %q", want, have)
}

err = res.Body.Close()
if err != nil {
t.Errorf("failed to close the body: %v", err)
}

select {
case err = <-serverErrC:
t.Errorf("unexpected error from server when writing response body: %v", err)
default:
return
}
}

Expand Down
5 changes: 5 additions & 0 deletions internal/corazawaf/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,11 @@ func (tx *Transaction) ProcessLogging() {
}
}

// IsRuleEngineOff will return true if RuleEngine is set to Off
func (tx *Transaction) IsRuleEngineOff() bool {
return tx.RuleEngine == types.RuleEngineOff
}

// RequestBodyAccessible will return true if RequestBody access has been enabled by RequestBodyAccess
func (tx *Transaction) RequestBodyAccessible() bool {
return tx.RequestBodyAccess
Expand Down
15 changes: 15 additions & 0 deletions types/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,25 @@ type Transaction interface {
// delivered prior to the execution of this method.
ProcessLogging()

// IsRuleEngineOff will return true if RuleEngine is set to Off
IsRuleEngineOff() bool

// RequestBodyAccessible will return true if RequestBody access has been enabled by RequestBodyAccess
//
// This can be used to perform checks just before calling request body related functions.
// In order to avoid any risk of performing wrong early assumptions, perform early checks on this value
// only if the API consumer requires them for specific server/proxy actions
// (such as avoiding proxy side buffering).
// Note: it returns the current status, later rules may still change it via ctl actions.
RequestBodyAccessible() bool

// ResponseBodyAccessible will return true if ResponseBody access has been enabled by ResponseBodyAccess
//
// This can be used to perform checks just before calling response body related functions.
// In order to avoid any risk of performing wrong early assumptions, perform early checks on this value
// only if the API consumer requires them for specific server/proxy actions
// (such as avoiding proxy side buffering).
// Note: it returns the current status, later rules may still change it via ctl actions.
ResponseBodyAccessible() bool

// Interrupted will return true if the transaction was interrupted
Expand Down

0 comments on commit bffb435

Please sign in to comment.