Skip to content
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

Fix for out of memory error with request body #806

Merged
merged 19 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from 12 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
2 changes: 2 additions & 0 deletions v3/integrations/nrsecurityagent/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ validator_service_url: wss://csec.nr-data.net
detection:
rxss:
enabled: true
request:
body_limit:1
```

* Based on additional packages imported by the user application, add suitable instrumentation package imports.
Expand Down
2 changes: 1 addition & 1 deletion v3/integrations/nrsecurityagent/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/newrelic/go-agent/v3/integrations/nrsecurityagent
go 1.19

require (
github.com/newrelic/csec-go-agent v0.4.0
github.com/newrelic/csec-go-agent v0.5.0
github.com/newrelic/go-agent/v3 v3.26.0
github.com/newrelic/go-agent/v3/integrations/nrsqlite3 v1.2.0
gopkg.in/yaml.v2 v2.4.0
Expand Down
21 changes: 21 additions & 0 deletions v3/integrations/nrsecurityagent/nrsecurityagent.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func defaultSecurityConfig() SecurityConfig {
cfg.Security.Mode = "IAST"
cfg.Security.Agent.Enabled = true
cfg.Security.Detection.Rxss.Enabled = true
cfg.Security.Request.BodyLimit = 300
return cfg
}

Expand Down Expand Up @@ -108,6 +109,8 @@ func ConfigSecurityFromYaml() ConfigOption {
// NEW_RELIC_SECURITY_MODE scanning mode: "IAST" for now
// NEW_RELIC_SECURITY_AGENT_ENABLED (boolean)
// NEW_RELIC_SECURITY_DETECTION_RXSS_ENABLED (boolean)
// NEW_RELIC_SECURITY_REQUEST_BODY_LIMIT (integer) set limit on read request body in kb. By default, this is "300"

func ConfigSecurityFromEnvironment() ConfigOption {
return func(cfg *SecurityConfig) {
assignBool := func(field *bool, name string) {
Expand All @@ -125,11 +128,22 @@ func ConfigSecurityFromEnvironment() ConfigOption {
}
}

assignInt := func(field *int, name string) {
if env := os.Getenv(name); env != "" {
if i, err := strconv.Atoi(env); nil != err {
cfg.Error = fmt.Errorf("invalid %s value: %s", name, env)
} else {
*field = i
}
}
}

assignBool(&cfg.Security.Enabled, "NEW_RELIC_SECURITY_ENABLED")
assignString(&cfg.Security.Validator_service_url, "NEW_RELIC_SECURITY_VALIDATOR_SERVICE_URL")
assignString(&cfg.Security.Mode, "NEW_RELIC_SECURITY_MODE")
assignBool(&cfg.Security.Agent.Enabled, "NEW_RELIC_SECURITY_AGENT_ENABLED")
assignBool(&cfg.Security.Detection.Rxss.Enabled, "NEW_RELIC_SECURITY_DETECTION_RXSS_ENABLED")
assignInt(&cfg.Security.Request.BodyLimit, "NEW_RELIC_SECURITY_REQUEST_BODY_LIMIT")
}
}

Expand Down Expand Up @@ -160,3 +174,10 @@ func ConfigSecurityEnable(isEnabled bool) ConfigOption {
cfg.Security.Enabled = isEnabled
}
}

// ConfigSecurityRequestBodyLimit set limit on read request body in kb. By default, this is "300"
func ConfigSecurityRequestBodyLimit(bodyLimit int) ConfigOption {
return func(cfg *SecurityConfig) {
cfg.Security.Request.BodyLimit = bodyLimit
}
}
40 changes: 34 additions & 6 deletions v3/newrelic/secure_agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,15 @@ import (
"net/http"
)

//
// secureAgent is a global interface point for the nrsecureagent's hooks into the go agent.
// The default value for this is a noOpSecurityAgent value, which has null definitions for
// the methods. The Go compiler is expected to optimize away all the securityAgent method
// calls in this case, effectively removing the hooks from the running agent.
//
// If the nrsecureagent integration was initialized, it will register a real securityAgent
// value in the securityAgent varialble instead, thus "activating" the hooks.
//
var secureAgent securityAgent = noOpSecurityAgent{}

//
// GetSecurityAgentInterface returns the securityAgent value
// which provides the working interface to the installed
// security agent (or to a no-op interface if none were
Expand All @@ -26,7 +23,6 @@ var secureAgent securityAgent = noOpSecurityAgent{}
// This avoids exposing the variable itself so it's not
// writable externally and also sets up for the future if this
// ends up not being a global variable later.
//
func GetSecurityAgentInterface() securityAgent {
return secureAgent
}
Expand All @@ -38,6 +34,7 @@ type securityAgent interface {
IsSecurityActive() bool
DistributedTraceHeaders(hdrs *http.Request, secureAgentevent any)
SendExitEvent(any, error)
RequestBodyReadLimit() int
}

func (app *Application) RegisterSecurityAgent(s securityAgent) {
Expand Down Expand Up @@ -88,13 +85,44 @@ func (t noOpSecurityAgent) DistributedTraceHeaders(hdrs *http.Request, secureAge

func (t noOpSecurityAgent) SendExitEvent(secureAgentevent any, err error) {
}
func (t noOpSecurityAgent) RequestBodyReadLimit() int {
return 300 * 1000
}

//
// IsSecurityAgentPresent returns true if there's an actual security agent hooked in to the
// Go APM agent, whether or not it's enabled or operating in any particular mode. It returns
// false only if the hook-in interface for those functions is a No-Op will null functionality.
//
func IsSecurityAgentPresent() bool {
_, isNoOp := secureAgent.(noOpSecurityAgent)
return !isNoOp
}

type BodyBuffer struct {
buf []byte
isDataTruncated bool
}

func (b *BodyBuffer) Write(p []byte) (int, error) {
if l := len(b.buf); len(p) <= cap(b.buf)-l {
b.buf = append(b.buf, p...)
return len(p), nil
} else {
b.isDataTruncated = true
return 0, nil
}
}

func (b *BodyBuffer) Len() int {
if b == nil {
return 0
}
return len(b.buf)

}
func (b *BodyBuffer) String() (string, bool) {
if b == nil {
return "", false
}
return string(b.buf), b.isDataTruncated

}
25 changes: 9 additions & 16 deletions v3/newrelic/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package newrelic

import (
"bytes"
"encoding/json"
"fmt"
"io"
Expand Down Expand Up @@ -246,20 +245,14 @@ func serverName(r *http.Request) string {
return ""
}

func reqBody(req *http.Request) []byte {
var bodyBuffer bytes.Buffer
requestBuffer := make([]byte, 0)
bodyReader := io.TeeReader(req.Body, &bodyBuffer)

if bodyReader != nil && req.Body != nil {
reqBuffer, err := io.ReadAll(bodyReader)
if err == nil {
requestBuffer = reqBuffer
}
r := io.NopCloser(bytes.NewBuffer(requestBuffer))
req.Body = r
func reqBody(req *http.Request) io.Writer {
if IsSecurityAgentPresent() {
buf := &BodyBuffer{buf: make([]byte, 0, secureAgent.RequestBodyReadLimit())}
tee := io.TeeReader(req.Body, buf)
req.Body = io.NopCloser(tee)
return buf
}
return bytes.TrimRight(requestBuffer, "\x00")
return nil
}

// SetWebRequest marks the transaction as a web transaction. SetWebRequest
Expand Down Expand Up @@ -607,7 +600,7 @@ type WebRequest struct {

// The following fields are needed for the secure agent's vulnerability
// detection features.
Body []byte
Body io.Writer
ServerName string
Type string
RemoteAddress string
Expand All @@ -633,7 +626,7 @@ func (webrequest WebRequest) GetHost() string {
return webrequest.Host
}

func (webrequest WebRequest) GetBody() []byte {
func (webrequest WebRequest) GetBody() interface{} {
aayush-ap marked this conversation as resolved.
Show resolved Hide resolved
return webrequest.Body
}

Expand Down
Loading