diff --git a/appsec/appsec.go b/appsec/appsec.go index 8487d30d29..36833749a9 100644 --- a/appsec/appsec.go +++ b/appsec/appsec.go @@ -4,22 +4,29 @@ // Copyright 2022 Datadog, Inc. // Package appsec provides application security features in the form of SDK -// functions that can be manually called to monitor specific code paths and data +// functions that can be manually called to monitor specific code paths and data. +// Application Security is currently transparently integrated into the APM tracer +// and cannot be used nor started alone at the moment. +// You can read more on how to enable and start Application Security for Go at +// https://docs.datadoghq.com/security_platform/application_security/getting_started/go package appsec import ( "golang.org/x/net/context" + "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo/instrumentation/httpsec" ) -// MonitorParsedHTTPBody performs rule matching on the *parsed* HTTP request body. -// - body: the parsed HTTP body value to be used for rule matching. -// - ctx: the context of the http.Request for which the matching is to be performed. -// Note that some HTTP frameworks implement their own version of context.Context, -// however this function expects this exact type to be used. -// Also note that passing the HTTP request raw body instead of parsed will result -// in inaccurate attack detection +// MonitorParsedHTTPBody runs the security monitoring rules on the given *parsed* +// HTTP request body. The given context must be the HTTP request context as returned +// by the Context() method of an HTTP request. Calls to this function are ignored if +// AppSec is disabled or the given context is incorrect. +// Note that passing the raw bytes of the HTTP request body is not expected and would +// result in inaccurate attack detection. func MonitorParsedHTTPBody(ctx context.Context, body interface{}) { - httpsec.MonitorParsedBody(ctx, body) + if appsec.Enabled() { + httpsec.MonitorParsedBody(ctx, body) + } + // bonus: use sync.Once to log a debug message once if AppSec is disabled } diff --git a/appsec/example_test.go b/appsec/example_test.go index 82a1c8be0d..c4f7695d0f 100644 --- a/appsec/example_test.go +++ b/appsec/example_test.go @@ -6,8 +6,8 @@ package appsec_test import ( + "encoding/json" "io" - "io/ioutil" "net/http" "github.com/labstack/echo/v4" @@ -17,12 +17,15 @@ import ( httptrace "gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http" ) -func customBodyParser(body io.ReadCloser) string { - b, err := ioutil.ReadAll(body) - if err != nil { - return "Ill formed http body" - } - return string(b) +type parsedBodyType struct { + Value string `json:"value"` +} + +func customBodyParser(body io.ReadCloser) (*parsedBodyType, error) { + var parsedBody parsedBodyType + err := json.NewDecoder(body).Decode(&parsedBody) + + return &parsedBody, err } // Monitor HTTP request parsed body @@ -30,19 +33,29 @@ func ExampleMonitorParsedHTTPBody() { mux := httptrace.NewServeMux() mux.HandleFunc("/body", func(w http.ResponseWriter, r *http.Request) { // Use the SDK to monitor the request's parsed body - appsec.MonitorParsedHTTPBody(r.Context(), customBodyParser(r.Body)) + body, err := customBodyParser(r.Body) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + appsec.MonitorParsedHTTPBody(r.Context(), body) w.Write([]byte("Body monitored using AppSec SDK\n")) }) http.ListenAndServe(":8080", mux) } // Monitor HTTP request parsed body with a framework customized context type -func ExampleMonitorParsedHTTPBodyCustomContext() { +func ExampleMonitorParsedHTTPBody_CustomContext() { r := echo.New() r.Use(echotrace.Middleware()) - r.POST("/body", func(c echo.Context) error { + r.POST("/body", func(c echo.Context) (e error) { + req := c.Request() + body, err := customBodyParser(req.Body) + if err != nil { + return c.String(http.StatusInternalServerError, err.Error()) + } // Use the SDK to monitor the request's parsed body - appsec.MonitorParsedHTTPBody(c.Request().Context(), customBodyParser(c.Request().Body)) + appsec.MonitorParsedHTTPBody(c.Request().Context(), body) return c.String(http.StatusOK, "Body monitored using AppSec SDK") }) diff --git a/contrib/gin-gonic/gin/appsec.go b/contrib/gin-gonic/gin/appsec.go index e5a2e2f112..ddbef4f7b5 100644 --- a/contrib/gin-gonic/gin/appsec.go +++ b/contrib/gin-gonic/gin/appsec.go @@ -27,8 +27,8 @@ func useAppSec(c *gin.Context, span tracer.Span) func() { } } args := httpsec.MakeHandlerOperationArgs(req, params) - ctx, op := httpsec.StartOperation(c.Request.Context(), args) - *req = *req.WithContext(ctx) + ctx, op := httpsec.StartOperation(req.Context(), args) + c.Request = req.WithContext(ctx) return func() { events := op.Finish(httpsec.HandlerOperationRes{Status: c.Writer.Status()}) if len(events) > 0 { diff --git a/contrib/labstack/echo.v4/appsec.go b/contrib/labstack/echo.v4/appsec.go index a7058047c7..aa6a2cbbe9 100644 --- a/contrib/labstack/echo.v4/appsec.go +++ b/contrib/labstack/echo.v4/appsec.go @@ -28,7 +28,7 @@ func withAppSec(next echo.HandlerFunc) echo.HandlerFunc { } args := httpsec.MakeHandlerOperationArgs(req, params) ctx, op := httpsec.StartOperation(req.Context(), args) - *req = *req.WithContext(ctx) + c.SetRequest(req.WithContext(ctx)) defer func() { events := op.Finish(httpsec.HandlerOperationRes{Status: c.Response().Status}) if len(events) > 0 { diff --git a/internal/appsec/dyngo/instrumentation/httpsec/http.go b/internal/appsec/dyngo/instrumentation/httpsec/http.go index aef1d0e04a..b7b647bbf1 100644 --- a/internal/appsec/dyngo/instrumentation/httpsec/http.go +++ b/internal/appsec/dyngo/instrumentation/httpsec/http.go @@ -47,22 +47,23 @@ type ( // SDKBodyOperationArgs is the SDK body operation arguments. SDKBodyOperationArgs struct { + // Body corresponds to the address `server.request.body`. Body interface{} } // SDKBodyOperationRes is the SDK body operation results. - SDKBodyOperationRes struct { - } + SDKBodyOperationRes struct{} ) // MonitorParsedBody starts and finishes the SDK body operation. +// This function should not be called when AppSec is disabled in order to +// get preciser error logs. func MonitorParsedBody(ctx context.Context, body interface{}) { if parent := fromContext(ctx); parent != nil { op := StartSDKBodyOperation(parent, SDKBodyOperationArgs{Body: body}) op.Finish() } else { - log.Debug("appsec: no valid parent HTTP context upon SDKBody operation start" + - " AppSec may be disabled") + log.Error("appsec: parsed http body monitoring ignored: could not find the http handler instrumentation metadata in the request context: the request handler is not being monitored by a middleware function or the provided context is not the expected request context") } } @@ -135,14 +136,13 @@ type ( // StartSDKBodyOperation() and finished with its Finish() method. SDKBodyOperation struct { dyngo.Operation - events json.RawMessage } contextKey struct{} ) // StartOperation starts an HTTP handler operation, along with the given -// context and arguments and emits a start event up in the operation stack +// context and arguments and emits a start event up in the operation stack. // The operation is linked to the global root operation since an HTTP operation // is always expected to be first in the operation stack. func StartOperation(ctx context.Context, args HandlerOperationArgs) (context.Context, *Operation) { @@ -153,7 +153,7 @@ func StartOperation(ctx context.Context, args HandlerOperationArgs) (context.Con } func fromContext(ctx context.Context) *Operation { - // Avoid a runtime panic in case of error by collecting the 2 return values + // Avoid a runtime panic in case of type-assertion error by collecting the 2 return values op, _ := ctx.Value(contextKey{}).(*Operation) return op } @@ -173,9 +173,8 @@ func StartSDKBodyOperation(parent *Operation, args SDKBodyOperationArgs) *SDKBod } // Finish finishes the SDKBody operation and emits a finish event -func (op *SDKBodyOperation) Finish() json.RawMessage { +func (op *SDKBodyOperation) Finish() { dyngo.FinishOperation(op, SDKBodyOperationRes{}) - return op.events } // AddSecurityEvent adds the security event to the list of events observed diff --git a/internal/appsec/waf.go b/internal/appsec/waf.go index c28bde8e6d..b08343bfeb 100644 --- a/internal/appsec/waf.go +++ b/internal/appsec/waf.go @@ -84,9 +84,7 @@ func newHTTPWAFEventListener(handle *waf.Handle, addresses []string, timeout tim return httpsec.OnHandlerOperationStart(func(op *httpsec.Operation, args httpsec.HandlerOperationArgs) { var body interface{} op.On(httpsec.OnSDKBodyOperationStart(func(op *httpsec.SDKBodyOperation, args httpsec.SDKBodyOperationArgs) { - op.On(httpsec.OnSDKBodyOperationFinish(func(operation *httpsec.SDKBodyOperation, res httpsec.SDKBodyOperationRes) { - body = args.Body - })) + body = args.Body })) // At the moment, AppSec doesn't block the requests, and so we can use the fact we are in monitoring-only mode // to call the WAF only once at the end of the handler operation.