From 3c15624acc2f28e8ab879a71aae01c9af9476146 Mon Sep 17 00:00:00 2001 From: Matteo Pace Date: Tue, 6 Feb 2024 17:50:30 +0100 Subject: [PATCH] fix: BodyLimit related documented default values, default RequestBodyLimitAction, adds some tests (#895) * fixes documented default values and default requestbodyLimit * mod tidy * comments out SecRequestBodyNoFilesLimit * Add comment about not implemented SecRequestBodyNoFilesLimit in coraza.conf-recommended * Aligns RequestBodyLimit and ResponseBodyLimit defaults values to doc * adds link to issue in TODO comment * fix comment for autogeneration --- coraza.conf-recommended | 3 ++- internal/corazawaf/transaction_test.go | 28 ++++++++++++++++++++------ internal/corazawaf/waf.go | 8 +++++--- internal/seclang/directives.go | 7 ++++--- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/coraza.conf-recommended b/coraza.conf-recommended index 3b9d8a741..532817ca9 100644 --- a/coraza.conf-recommended +++ b/coraza.conf-recommended @@ -45,7 +45,8 @@ SecRequestBodyLimit 13107200 SecRequestBodyInMemoryLimit 131072 -SecRequestBodyNoFilesLimit 131072 +# SecRequestBodyNoFilesLimit is currently not supported by Coraza +# SecRequestBodyNoFilesLimit 131072 # What to do if the request body size is above our configured limit. # Keep in mind that this setting will automatically be set to ProcessPartial diff --git a/internal/corazawaf/transaction_test.go b/internal/corazawaf/transaction_test.go index 34ccb9a3e..d229d3dd8 100644 --- a/internal/corazawaf/transaction_test.go +++ b/internal/corazawaf/transaction_test.go @@ -162,10 +162,11 @@ func TestWriteRequestBody(t *testing.T) { ) testCases := []struct { - name string - requestBodyLimit int - requestBodyLimitAction types.BodyLimitAction - shouldInterrupt bool + name string + requestBodyLimit int + requestBodyLimitAction types.BodyLimitAction + avoidRequestBodyLimitActionInit bool + shouldInterrupt bool }{ { name: "LimitNotReached", @@ -178,6 +179,14 @@ func TestWriteRequestBody(t *testing.T) { requestBodyLimitAction: types.BodyLimitActionReject, shouldInterrupt: true, }, + { + name: "LimitReachedAndRejectsDefaultValue", + requestBodyLimit: urlencodedBodyLen - 3, + // Omitting requestBodyLimitAction defaults to Reject + // requestBodyLimitAction: types.BodyLimitActionReject, + avoidRequestBodyLimitActionInit: true, + shouldInterrupt: true, + }, { name: "LimitReachedAndPartialProcessing", requestBodyLimit: urlencodedBodyLen - 3, @@ -201,8 +210,9 @@ func TestWriteRequestBody(t *testing.T) { waf.RuleEngine = types.RuleEngineOn waf.RequestBodyAccess = true waf.RequestBodyLimit = int64(testCase.requestBodyLimit) - waf.RequestBodyLimitAction = testCase.requestBodyLimitAction - + if !testCase.avoidRequestBodyLimitActionInit { + waf.RequestBodyLimitAction = testCase.requestBodyLimitAction + } tx := waf.NewTransaction() tx.AddRequestHeader("content-type", "application/x-www-form-urlencoded") @@ -472,6 +482,12 @@ func TestWriteResponseBody(t *testing.T) { responseBodyLimit: urlencodedBodyLen - 3, responseBodyLimitAction: types.BodyLimitActionProcessPartial, }, + { + name: "LimitReachedAndPartialProcessingDefaultValue", + responseBodyLimit: urlencodedBodyLen - 3, + // Omitting requestBodyLimitAction defaults to ProcessPartial + // responseBodyLimitAction: types.BodyLimitActionProcessPartial, + }, } urlencodedBodyLenThird := urlencodedBodyLen / 3 diff --git a/internal/corazawaf/waf.go b/internal/corazawaf/waf.go index 40441b6ca..e6946a3da 100644 --- a/internal/corazawaf/waf.go +++ b/internal/corazawaf/waf.go @@ -91,6 +91,7 @@ type WAF struct { UploadDir string // Request body in memory limit excluding the size of any files being transported in the request. + // TODO: SecRequestBodyNoFilesLimit directive is retrieving the value, but no logic based on it is implemented. See https://github.com/corazawaf/coraza/issues/896 RequestBodyNoFilesLimit int64 RequestBodyLimitAction types.BodyLimitAction @@ -196,7 +197,7 @@ func (w *WAF) newTransaction(opts Options) *Transaction { // Always non-nil if buffers / collections were already initialized so we don't do any of them // based on the presence of RequestBodyBuffer. if tx.requestBodyBuffer == nil { - // if no requestBodyInMemoryLimit has been set we default to the + // if no requestBodyInMemoryLimit has been set we default to the requestBodyLimit var requestBodyInMemoryLimit int64 = w.RequestBodyLimit if w.requestBodyInMemoryLimit != nil { requestBodyInMemoryLimit = int64(*w.requestBodyInMemoryLimit) @@ -291,9 +292,10 @@ func NewWAF() *WAF { // These defaults are unavoidable as they are zero values for the variables RuleEngine: types.RuleEngineOn, RequestBodyAccess: false, - RequestBodyLimit: _1gb, + RequestBodyLimit: 134217728, // Hard limit equal to _1gb + RequestBodyLimitAction: types.BodyLimitActionReject, ResponseBodyAccess: false, - ResponseBodyLimit: _1gb, + ResponseBodyLimit: 524288, // Hard limit equal to _1gb auditLogWriter: logWriter, auditLogWriterInitialized: false, AuditLogWriterConfig: auditlog.NewConfig(), diff --git a/internal/seclang/directives.go b/internal/seclang/directives.go index 4dca4bd63..f5536ce93 100644 --- a/internal/seclang/directives.go +++ b/internal/seclang/directives.go @@ -229,7 +229,7 @@ func directiveSecResponseBodyAccess(options *DirectiveOptions) error { } // Description: Configures the maximum request body size Coraza will accept for buffering. -// Default: 134217728 (131072 KB) +// Default: 134217728 (128 Mib) // Syntax: SecRequestBodyLimit [LIMIT_IN_BYTES] // --- // Anything over the limit will be rejected with status code 413 (Request Entity Too Large). @@ -423,7 +423,7 @@ func directiveSecResponseBodyLimitAction(options *DirectiveOptions) error { // Description: Configures the maximum response body size that will be accepted for buffering. // Syntax: SecResponseBodyLimit [LIMIT_IN_BYTES] -// Default: 524288 (512 KB) +// Default: 524288 (512 Kib) // --- // Anything over this limit will be rejected with status code 500 (Internal Server Error). // This setting will not affect the responses with MIME types that are not selected for @@ -461,7 +461,7 @@ func directiveSecRequestBodyLimitAction(options *DirectiveOptions) error { } // Description: Configures the maximum request body size that Coraza will store in memory. -// Default: 131072 (128 KB) +// Default: defaults to RequestBodyLimit // Syntax: SecRequestBodyInMemoryLimit [LIMIT_IN_BYTES] // --- // When a `multipart/form-data` request is being processed, once the in-memory limit is reached, @@ -903,6 +903,7 @@ func directiveSecUploadDir(options *DirectiveOptions) error { // Generally speaking, the default value is not small enough. For most applications, you // should be able to reduce it down to 128 KB or lower. Anything over the limit will be // rejected with status code 413 (Request Entity Too Large). There is a hard limit of 1 GB. +// Note: not implemented yet func directiveSecRequestBodyNoFilesLimit(options *DirectiveOptions) error { if len(options.Opts) == 0 { return errEmptyOptions