Skip to content

enhance: Attempt to reduce body reads on appsec #3805

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 41 additions & 8 deletions pkg/acquisition/modules/appsec/appsec_runner.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package appsecacquisition

import (
"bytes"
"fmt"
"io"
"os"
"slices"
"strings"
Expand Down Expand Up @@ -190,14 +192,26 @@
return nil
}

if len(request.Body) > 0 {
in, _, err = request.Tx.WriteRequestBody(request.Body)
if err != nil {
r.logger.Errorf("unable to write request body : %s", err)
return err
}
if in != nil {
return nil
// Body handling: prefer using an existing buffer, otherwise stream from the reader if accessible
if request.Tx.IsRequestBodyAccessible() {
if len(request.Body) > 0 {
in, _, err = request.Tx.WriteRequestBody(request.Body)
if err != nil {
r.logger.Errorf("unable to write request body : %s", err)
return err
}
if in != nil {
return nil
}
} else if request.HTTPRequest != nil && request.HTTPRequest.Body != nil {
in, _, err = request.Tx.ReadRequestBodyFrom(request.HTTPRequest.Body)
if err != nil && err != io.ErrUnexpectedEOF {

Check failure on line 208 in pkg/acquisition/modules/appsec/appsec_runner.go

View workflow job for this annotation

GitHub Actions / Build + tests

comparing with != will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)

Check failure on line 208 in pkg/acquisition/modules/appsec/appsec_runner.go

View workflow job for this annotation

GitHub Actions / Build + tests

comparing with != will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
r.logger.Errorf("unable to read request body from reader: %s", err)
return err
}
if in != nil {
return nil
}
}
}

Expand Down Expand Up @@ -352,6 +366,25 @@
startInBandParsing := time.Now()
startGlobalParsing := time.Now()

// If we will need the body later for out-of-band processing with body inspection,
// pre-buffer it once here to avoid re-reading later.
needOutOfBandBody := len(r.AppsecRuntime.OutOfBandRules) > 0 && !r.AppsecRuntime.Config.OutOfBandOptions.DisableBodyInspection
if needOutOfBandBody && len(request.Body) == 0 && request.HTTPRequest != nil && request.HTTPRequest.Body != nil {
// Optional: respect a soft cap using the configured in-memory limit when available
var reader io.Reader = request.HTTPRequest.Body
if r.AppsecRuntime.Config.OutOfBandOptions.RequestBodyInMemoryLimit != nil {
reader = io.LimitReader(reader, int64(*r.AppsecRuntime.Config.OutOfBandOptions.RequestBodyInMemoryLimit))
}
buf, err := io.ReadAll(reader)
if err != nil {
logger.Errorf("unable to pre-buffer request body: %s", err)
} else {
request.Body = buf
// Reset the body reader in case it is used elsewhere
request.HTTPRequest.Body = io.NopCloser(bytes.NewReader(request.Body))
}
}

//inband appsec rules
err := r.ProcessInBandRules(request)
if err != nil {
Expand Down
42 changes: 29 additions & 13 deletions pkg/appsec/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,30 @@
if r.BodyDrop {
return nil
}
out.Body = r.req.Body
// If already buffered, reuse it
if len(r.req.Body) > 0 {
out.Body = r.req.Body
return nil
}

// Lazily read from the original HTTP request body if available
if r.req.HTTPRequest != nil && r.req.HTTPRequest.Body != nil {
b, err := io.ReadAll(r.req.HTTPRequest.Body)
// Always close after attempt
_ = r.req.HTTPRequest.Body.Close()
if err != nil && err != io.EOF && err != io.ErrUnexpectedEOF {

Check failure on line 148 in pkg/appsec/request.go

View workflow job for this annotation

GitHub Actions / Build + tests

comparing with != will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)

Check failure on line 148 in pkg/appsec/request.go

View workflow job for this annotation

GitHub Actions / Build + tests

comparing with != will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
return fmt.Errorf("reading request body for dump: %w", err)
}

// Cache for future use and reset reader for subsequent consumers
r.req.Body = b
r.req.HTTPRequest.Body = io.NopCloser(bytes.NewReader(r.req.Body))
out.Body = r.req.Body
return nil
}

// No body available
out.Body = nil
return nil
}

Expand Down Expand Up @@ -286,19 +309,11 @@
}

// Generate a ParsedRequest from a http.Request. ParsedRequest can be consumed by the App security Engine
// If readBody is false, the body will not be read into memory and ParsedRequest.Body will be nil.
func NewParsedRequestFromRequest(r *http.Request, logger *log.Entry) (ParsedRequest, error) {
var err error
contentLength := max(r.ContentLength, 0)
body := make([]byte, contentLength)
if r.Body != nil {
_, err = io.ReadFull(r.Body, body)
if err != nil {
return ParsedRequest{}, fmt.Errorf("unable to read body: %s", err)
}
// reset the original body back as it's been read, i'm not sure its needed?
r.Body = io.NopCloser(bytes.NewBuffer(body))

}
var body []byte
// Body is not read here; it will be streamed by the runner if needed.
clientIP := r.Header.Get(IPHeaderName)
if clientIP == "" {
return ParsedRequest{}, fmt.Errorf("missing '%s' header", IPHeaderName)
Expand Down Expand Up @@ -353,7 +368,8 @@
delete(r.Header, HTTPVersionHeaderName)

originalHTTPRequest := r.Clone(r.Context())
originalHTTPRequest.Body = io.NopCloser(bytes.NewBuffer(body))
// Keep the original body reader so the runner can stream it when needed.
originalHTTPRequest.Body = r.Body
originalHTTPRequest.RemoteAddr = clientIP
originalHTTPRequest.RequestURI = clientURI
originalHTTPRequest.Method = clientMethod
Expand Down
12 changes: 12 additions & 0 deletions pkg/appsec/tx.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package appsec

import (
"io"

"github.com/corazawaf/coraza/v3"
"github.com/corazawaf/coraza/v3/experimental"
"github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes"
Expand Down Expand Up @@ -72,6 +74,11 @@ func (t *ExtendedTransaction) WriteRequestBody(body []byte) (*types.Interruption
return t.Tx.WriteRequestBody(body)
}

// ReadRequestBodyFrom streams the request body from the provided reader into the transaction.
func (t *ExtendedTransaction) ReadRequestBodyFrom(r io.Reader) (*types.Interruption, int, error) {
return t.Tx.ReadRequestBodyFrom(r)
}

func (t *ExtendedTransaction) Interruption() *types.Interruption {
return t.Tx.Interruption()
}
Expand All @@ -95,3 +102,8 @@ func (t *ExtendedTransaction) ID() string {
func (t *ExtendedTransaction) Close() error {
return t.Tx.Close()
}

// IsRequestBodyAccessible exposes whether the engine has request body access enabled.
func (t *ExtendedTransaction) IsRequestBodyAccessible() bool {
return t.Tx.IsRequestBodyAccessible()
}
Loading