diff --git a/CODEOWNERS b/CODEOWNERS index b0c28372e5..7702abe6ef 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -13,5 +13,6 @@ /internal/traceprof @DataDog/profiling-go # appsec +/appsec @DataDog/appsec-go /internal/appsec @DataDog/appsec-go /contrib/**/appsec.go @DataDog/appsec-go diff --git a/appsec/appsec.go b/appsec/appsec.go new file mode 100644 index 0000000000..36833749a9 --- /dev/null +++ b/appsec/appsec.go @@ -0,0 +1,32 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// 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. +// 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 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{}) { + 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 new file mode 100644 index 0000000000..e290e657e0 --- /dev/null +++ b/appsec/example_test.go @@ -0,0 +1,62 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2022 Datadog, Inc. + +package appsec_test + +import ( + "encoding/json" + "io" + "net/http" + + "gopkg.in/DataDog/dd-trace-go.v1/appsec" + echotrace "gopkg.in/DataDog/dd-trace-go.v1/contrib/labstack/echo.v4" + httptrace "gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http" + + "github.com/labstack/echo/v4" +) + +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 +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 + 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 ExampleMonitorParsedHTTPBody_CustomContext() { + r := echo.New() + r.Use(echotrace.Middleware()) + 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(), body) + return c.String(http.StatusOK, "Body monitored using AppSec SDK") + }) + + r.Start(":8080") +} diff --git a/contrib/gin-gonic/gin/appsec.go b/contrib/gin-gonic/gin/appsec.go index 2462f0c297..ddbef4f7b5 100644 --- a/contrib/gin-gonic/gin/appsec.go +++ b/contrib/gin-gonic/gin/appsec.go @@ -27,7 +27,8 @@ func useAppSec(c *gin.Context, span tracer.Span) func() { } } args := httpsec.MakeHandlerOperationArgs(req, params) - op := httpsec.StartOperation(args, nil) + 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/gin-gonic/gin/gintrace_test.go b/contrib/gin-gonic/gin/gintrace_test.go index 32aff62198..243335da17 100644 --- a/contrib/gin-gonic/gin/gintrace_test.go +++ b/contrib/gin-gonic/gin/gintrace_test.go @@ -15,6 +15,7 @@ import ( "strings" "testing" + pappsec "gopkg.in/DataDog/dd-trace-go.v1/appsec" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" @@ -554,6 +555,10 @@ func TestAppSec(t *testing.T) { r.Any("/path0.0/:myPathParam0/path0.1/:myPathParam1/path0.2/:myPathParam2/path0.3/*param3", func(c *gin.Context) { c.String(200, "Hello Params!\n") }) + r.Any("/body", func(c *gin.Context) { + pappsec.MonitorParsedHTTPBody(c.Request.Context(), "$globals") + c.String(200, "Hello Body!\n") + }) srv := httptest.NewServer(r) defer srv.Close() @@ -630,4 +635,29 @@ func TestAppSec(t *testing.T) { require.True(t, strings.Contains(event, "nfd-000-001")) }) + + // Test a PHP injection attack via request parsed body + t.Run("SDK-body", func(t *testing.T) { + mt := mocktracer.Start() + defer mt.Stop() + + req, err := http.NewRequest("POST", srv.URL+"/body", nil) + if err != nil { + panic(err) + } + res, err := srv.Client().Do(req) + require.NoError(t, err) + + // Check that the handler was properly called + b, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, "Hello Body!\n", string(b)) + + finished := mt.FinishedSpans() + require.Len(t, finished, 1) + + event := finished[0].Tag("_dd.appsec.json") + require.NotNil(t, event) + require.True(t, strings.Contains(event.(string), "crs-933-130")) + }) } diff --git a/contrib/go-chi/chi.v4/chi_test.go b/contrib/go-chi/chi.v4/chi_test.go index 8e2448e5f3..09d0df38ce 100644 --- a/contrib/go-chi/chi.v4/chi_test.go +++ b/contrib/go-chi/chi.v4/chi_test.go @@ -14,6 +14,7 @@ import ( "strings" "testing" + pappsec "gopkg.in/DataDog/dd-trace-go.v1/appsec" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" @@ -323,6 +324,11 @@ func TestAppSec(t *testing.T) { _, err := w.Write([]byte("Hello World!\n")) require.NoError(t, err) }) + router.HandleFunc("/body", func(w http.ResponseWriter, r *http.Request) { + pappsec.MonitorParsedHTTPBody(r.Context(), "$globals") + _, err := w.Write([]byte("Hello Body!\n")) + require.NoError(t, err) + }) srv := httptest.NewServer(router) defer srv.Close() @@ -379,4 +385,29 @@ func TestAppSec(t *testing.T) { require.True(t, strings.Contains(event, "myPathParam2")) require.True(t, strings.Contains(event, "server.request.path_params")) }) + + // Test a PHP injection attack via request parsed body + t.Run("SDK-body", func(t *testing.T) { + mt := mocktracer.Start() + defer mt.Stop() + + req, err := http.NewRequest("POST", srv.URL+"/body", nil) + if err != nil { + panic(err) + } + res, err := srv.Client().Do(req) + require.NoError(t, err) + + // Check that the handler was properly called + b, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, "Hello Body!\n", string(b)) + + finished := mt.FinishedSpans() + require.Len(t, finished, 1) + + event := finished[0].Tag("_dd.appsec.json") + require.NotNil(t, event) + require.True(t, strings.Contains(event.(string), "crs-933-130")) + }) } diff --git a/contrib/go-chi/chi.v5/chi_test.go b/contrib/go-chi/chi.v5/chi_test.go index 35ff5b3afb..4201d3076e 100644 --- a/contrib/go-chi/chi.v5/chi_test.go +++ b/contrib/go-chi/chi.v5/chi_test.go @@ -14,6 +14,7 @@ import ( "strings" "testing" + pappsec "gopkg.in/DataDog/dd-trace-go.v1/appsec" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" @@ -323,6 +324,11 @@ func TestAppSec(t *testing.T) { _, err := w.Write([]byte("Hello World!\n")) require.NoError(t, err) }) + router.HandleFunc("/body", func(w http.ResponseWriter, r *http.Request) { + pappsec.MonitorParsedHTTPBody(r.Context(), "$globals") + _, err := w.Write([]byte("Hello Body!\n")) + require.NoError(t, err) + }) srv := httptest.NewServer(router) defer srv.Close() @@ -379,4 +385,29 @@ func TestAppSec(t *testing.T) { require.True(t, strings.Contains(event, "myPathParam2")) require.True(t, strings.Contains(event, "server.request.path_params")) }) + + // Test a PHP injection attack via request parsed body + t.Run("SDK-body", func(t *testing.T) { + mt := mocktracer.Start() + defer mt.Stop() + + req, err := http.NewRequest("POST", srv.URL+"/body", nil) + if err != nil { + panic(err) + } + res, err := srv.Client().Do(req) + require.NoError(t, err) + + // Check that the handler was properly called + b, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, "Hello Body!\n", string(b)) + + finished := mt.FinishedSpans() + require.Len(t, finished, 1) + + event := finished[0].Tag("_dd.appsec.json") + require.NotNil(t, event) + require.True(t, strings.Contains(event.(string), "crs-933-130")) + }) } diff --git a/contrib/go-chi/chi/chi_test.go b/contrib/go-chi/chi/chi_test.go index a77cd8b6d5..20d5dd5c31 100644 --- a/contrib/go-chi/chi/chi_test.go +++ b/contrib/go-chi/chi/chi_test.go @@ -14,6 +14,7 @@ import ( "strings" "testing" + pappsec "gopkg.in/DataDog/dd-trace-go.v1/appsec" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" @@ -323,6 +324,11 @@ func TestAppSec(t *testing.T) { _, err := w.Write([]byte("Hello World!\n")) require.NoError(t, err) }) + router.HandleFunc("/body", func(w http.ResponseWriter, r *http.Request) { + pappsec.MonitorParsedHTTPBody(r.Context(), "$globals") + _, err := w.Write([]byte("Hello Body!\n")) + require.NoError(t, err) + }) srv := httptest.NewServer(router) defer srv.Close() @@ -379,4 +385,29 @@ func TestAppSec(t *testing.T) { require.True(t, strings.Contains(event, "myPathParam2")) require.True(t, strings.Contains(event, "server.request.path_params")) }) + + // Test a PHP injection attack via request parsed body + t.Run("SDK-body", func(t *testing.T) { + mt := mocktracer.Start() + defer mt.Stop() + + req, err := http.NewRequest("POST", srv.URL+"/body", nil) + if err != nil { + panic(err) + } + res, err := srv.Client().Do(req) + require.NoError(t, err) + + // Check that the handler was properly called + b, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, "Hello Body!\n", string(b)) + + finished := mt.FinishedSpans() + require.Len(t, finished, 1) + + event := finished[0].Tag("_dd.appsec.json") + require.NotNil(t, event) + require.True(t, strings.Contains(event.(string), "crs-933-130")) + }) } diff --git a/contrib/gorilla/mux/mux_test.go b/contrib/gorilla/mux/mux_test.go index 2c8774ab59..074d1d33dd 100644 --- a/contrib/gorilla/mux/mux_test.go +++ b/contrib/gorilla/mux/mux_test.go @@ -14,6 +14,7 @@ import ( "strings" "testing" + pappsec "gopkg.in/DataDog/dd-trace-go.v1/appsec" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" @@ -326,6 +327,11 @@ func TestAppSec(t *testing.T) { _, err := w.Write([]byte("Hello World!\n")) require.NoError(t, err) }) + router.HandleFunc("/body", func(w http.ResponseWriter, r *http.Request) { + pappsec.MonitorParsedHTTPBody(r.Context(), "$globals") + _, err := w.Write([]byte("Hello Body!\n")) + require.NoError(t, err) + }) srv := httptest.NewServer(router) defer srv.Close() @@ -404,4 +410,27 @@ func TestAppSec(t *testing.T) { require.True(t, strings.Contains(event, "server.response.status")) require.True(t, strings.Contains(event, "nfd-000-001")) }) + + // Test a PHP injection attack via request parsed body + t.Run("SDK-body", func(t *testing.T) { + mt := mocktracer.Start() + defer mt.Stop() + + req, err := http.NewRequest("POST", srv.URL+"/body", nil) + if err != nil { + panic(err) + } + res, err := srv.Client().Do(req) + require.NoError(t, err) + // Check that the handler was properly called + b, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, "Hello Body!\n", string(b)) + + finished := mt.FinishedSpans() + require.Len(t, finished, 1) + event := finished[0].Tag("_dd.appsec.json") + require.NotNil(t, event) + require.True(t, strings.Contains(event.(string), "crs-933-130")) + }) } diff --git a/contrib/labstack/echo.v4/appsec.go b/contrib/labstack/echo.v4/appsec.go index 1b12f4b8d7..675af4e48b 100644 --- a/contrib/labstack/echo.v4/appsec.go +++ b/contrib/labstack/echo.v4/appsec.go @@ -22,7 +22,8 @@ func useAppSec(c echo.Context, span tracer.Span) func() { params[n] = c.Param(n) } args := httpsec.MakeHandlerOperationArgs(req, params) - op := httpsec.StartOperation(args, nil) + ctx, op := httpsec.StartOperation(req.Context(), args) + c.SetRequest(req.WithContext(ctx)) return func() { events := op.Finish(httpsec.HandlerOperationRes{Status: c.Response().Status}) if len(events) > 0 { diff --git a/contrib/labstack/echo.v4/echotrace_test.go b/contrib/labstack/echo.v4/echotrace_test.go index 334735170b..98b0dbe8d8 100644 --- a/contrib/labstack/echo.v4/echotrace_test.go +++ b/contrib/labstack/echo.v4/echotrace_test.go @@ -13,6 +13,7 @@ import ( "strings" "testing" + pappsec "gopkg.in/DataDog/dd-trace-go.v1/appsec" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" @@ -290,6 +291,10 @@ func TestAppSec(t *testing.T) { e.POST("/", func(c echo.Context) error { return c.String(200, "Hello World!\n") }) + e.POST("/body", func(c echo.Context) error { + pappsec.MonitorParsedHTTPBody(c.Request().Context(), "$globals") + return c.String(200, "Hello Body!\n") + }) srv := httptest.NewServer(e) defer srv.Close() @@ -367,25 +372,48 @@ func TestAppSec(t *testing.T) { require.False(t, strings.Contains(event, "myPathParam3")) require.True(t, strings.Contains(event, "server.request.path_params")) }) + }) - t.Run("response-status", func(t *testing.T) { - mt := mocktracer.Start() - defer mt.Stop() + t.Run("response-status", func(t *testing.T) { + mt := mocktracer.Start() + defer mt.Stop() - req, err := http.NewRequest("POST", srv.URL+"/etc/", nil) - if err != nil { - panic(err) - } - res, err := srv.Client().Do(req) - require.NoError(t, err) - require.Equal(t, 404, res.StatusCode) + req, err := http.NewRequest("POST", srv.URL+"/etc/", nil) + if err != nil { + panic(err) + } + res, err := srv.Client().Do(req) + require.NoError(t, err) + require.Equal(t, 404, res.StatusCode) - finished := mt.FinishedSpans() - require.Len(t, finished, 1) - event := finished[0].Tag("_dd.appsec.json").(string) - require.NotNil(t, event) - require.True(t, strings.Contains(event, "server.response.status")) - require.True(t, strings.Contains(event, "nfd-000-001")) - }) + finished := mt.FinishedSpans() + require.Len(t, finished, 1) + event := finished[0].Tag("_dd.appsec.json").(string) + require.NotNil(t, event) + require.True(t, strings.Contains(event, "server.response.status")) + require.True(t, strings.Contains(event, "nfd-000-001")) + }) + + // Test a PHP injection attack via request parsed body + t.Run("SDK-body", func(t *testing.T) { + mt := mocktracer.Start() + defer mt.Stop() + + req, err := http.NewRequest("POST", srv.URL+"/body", nil) + if err != nil { + panic(err) + } + res, err := srv.Client().Do(req) + require.NoError(t, err) + // Check that the handler was properly called + b, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, "Hello Body!\n", string(b)) + + finished := mt.FinishedSpans() + require.Len(t, finished, 1) + event := finished[0].Tag("_dd.appsec.json") + require.NotNil(t, event) + require.True(t, strings.Contains(event.(string), "crs-933-130")) }) } diff --git a/internal/appsec/dyngo/instrumentation/httpsec/http.go b/internal/appsec/dyngo/instrumentation/httpsec/http.go index 2a0ef1a94d..b7b647bbf1 100644 --- a/internal/appsec/dyngo/instrumentation/httpsec/http.go +++ b/internal/appsec/dyngo/instrumentation/httpsec/http.go @@ -11,6 +11,7 @@ package httpsec import ( + "context" "encoding/json" "net" "net/http" @@ -19,6 +20,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo" + "gopkg.in/DataDog/dd-trace-go.v1/internal/log" ) // Abstract HTTP handler operation definition. @@ -42,18 +44,37 @@ type ( // Status corresponds to the address `server.response.status`. Status int } + + // 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{} ) +// 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.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") + } +} + // WrapHandler wraps the given HTTP handler with the abstract HTTP operation defined by HandlerOperationArgs and // HandlerOperationRes. func WrapHandler(handler http.Handler, span ddtrace.Span, pathParams map[string]string) http.Handler { SetAppSecTags(span) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { args := MakeHandlerOperationArgs(r, pathParams) - op := StartOperation( - args, - nil, - ) + ctx, op := StartOperation(r.Context(), args) + r = r.WithContext(ctx) defer func() { var status int if mw, ok := w.(interface{ Status() int }); ok { @@ -105,28 +126,57 @@ func MakeHandlerOperationArgs(r *http.Request, pathParams map[string]string) Han // Operation type representing an HTTP operation. It must be created with // StartOperation() and finished with its Finish(). -type Operation struct { - dyngo.Operation - events json.RawMessage -} +type ( + Operation struct { + dyngo.Operation + events json.RawMessage + } + + // SDKBodyOperation type representing an SDK body. It must be created with + // StartSDKBodyOperation() and finished with its Finish() method. + SDKBodyOperation struct { + dyngo.Operation + } + + contextKey struct{} +) // StartOperation starts an HTTP handler operation, along with the given -// arguments and parent operation, and emits a start event up in the -// operation stack. When parent is nil, the operation is linked to the global -// root operation. -func StartOperation(args HandlerOperationArgs, parent dyngo.Operation) *Operation { - op := &Operation{Operation: dyngo.NewOperation(parent)} +// 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) { + op := &Operation{Operation: dyngo.NewOperation(nil)} + newCtx := context.WithValue(ctx, contextKey{}, op) dyngo.StartOperation(op, args) + return newCtx, op +} + +func fromContext(ctx context.Context) *Operation { + // Avoid a runtime panic in case of type-assertion error by collecting the 2 return values + op, _ := ctx.Value(contextKey{}).(*Operation) return op } -// Finish the HTTP handler operation, along with the given results, and emits a +// Finish the HTTP handler operation, along with the given results and emits a // finish event up in the operation stack. func (op *Operation) Finish(res HandlerOperationRes) json.RawMessage { dyngo.FinishOperation(op, res) return op.events } +// StartSDKBodyOperation starts the SDKBody operation and emits a start event +func StartSDKBodyOperation(parent *Operation, args SDKBodyOperationArgs) *SDKBodyOperation { + op := &SDKBodyOperation{Operation: dyngo.NewOperation(parent)} + dyngo.StartOperation(op, args) + return op +} + +// Finish finishes the SDKBody operation and emits a finish event +func (op *SDKBodyOperation) Finish() { + dyngo.FinishOperation(op, SDKBodyOperationRes{}) +} + // AddSecurityEvent adds the security event to the list of events observed // during the operation lifetime. func (op *Operation) AddSecurityEvent(event json.RawMessage) { @@ -144,19 +194,27 @@ type ( // OnHandlerOperationFinish function type, called when an HTTP handler // operation finishes. OnHandlerOperationFinish func(*Operation, HandlerOperationRes) + // OnSDKBodyOperationStart function type, called when an SDK body + // operation starts. + OnSDKBodyOperationStart func(*SDKBodyOperation, SDKBodyOperationArgs) + // OnSDKBodyOperationFinish function type, called when an SDK body + // operation finishes. + OnSDKBodyOperationFinish func(*SDKBodyOperation, SDKBodyOperationRes) ) var ( handlerOperationArgsType = reflect.TypeOf((*HandlerOperationArgs)(nil)).Elem() handlerOperationResType = reflect.TypeOf((*HandlerOperationRes)(nil)).Elem() + sdkBodyOperationArgsType = reflect.TypeOf((*SDKBodyOperationArgs)(nil)).Elem() + sdkBodyOperationResType = reflect.TypeOf((*SDKBodyOperationRes)(nil)).Elem() ) // ListenedType returns the type a OnHandlerOperationStart event listener // listens to, which is the HandlerOperationArgs type. func (OnHandlerOperationStart) ListenedType() reflect.Type { return handlerOperationArgsType } -// Call the underlying event listener function by performing the type-assertion -// on v whose type is the one returned by ListenedType(). +// Call calls the underlying event listener function by performing the +// type-assertion on v whose type is the one returned by ListenedType(). func (f OnHandlerOperationStart) Call(op dyngo.Operation, v interface{}) { f(op.(*Operation), v.(HandlerOperationArgs)) } @@ -165,8 +223,28 @@ func (f OnHandlerOperationStart) Call(op dyngo.Operation, v interface{}) { // listens to, which is the HandlerOperationRes type. func (OnHandlerOperationFinish) ListenedType() reflect.Type { return handlerOperationResType } -// Call the underlying event listener function by performing the type-assertion -// on v whose type is the one returned by ListenedType(). +// Call calls the underlying event listener function by performing the +// type-assertion on v whose type is the one returned by ListenedType(). func (f OnHandlerOperationFinish) Call(op dyngo.Operation, v interface{}) { f(op.(*Operation), v.(HandlerOperationRes)) } + +// ListenedType returns the type a OnSDKBodyOperationStart event listener +// listens to, which is the SDKBodyOperationStartArgs type. +func (OnSDKBodyOperationStart) ListenedType() reflect.Type { return sdkBodyOperationArgsType } + +// Call calls the underlying event listener function by performing the +// type-assertion on v whose type is the one returned by ListenedType(). +func (f OnSDKBodyOperationStart) Call(op dyngo.Operation, v interface{}) { + f(op.(*SDKBodyOperation), v.(SDKBodyOperationArgs)) +} + +// ListenedType returns the type a OnSDKBodyOperationFinish event listener +// listens to, which is the SDKBodyOperationRes type. +func (OnSDKBodyOperationFinish) ListenedType() reflect.Type { return sdkBodyOperationResType } + +// Call calls the underlying event listener function by performing the +// type-assertion on v whose type is the one returned by ListenedType(). +func (f OnSDKBodyOperationFinish) Call(op dyngo.Operation, v interface{}) { + f(op.(*SDKBodyOperation), v.(SDKBodyOperationRes)) +} diff --git a/internal/appsec/waf.go b/internal/appsec/waf.go index 8f72ba22b2..b08343bfeb 100644 --- a/internal/appsec/waf.go +++ b/internal/appsec/waf.go @@ -82,6 +82,10 @@ func registerWAF(rules []byte, timeout time.Duration, limiter Limiter) (unreg dy // newWAFEventListener returns the WAF event listener to register in order to enable it. func newHTTPWAFEventListener(handle *waf.Handle, addresses []string, timeout time.Duration, limiter Limiter) dyngo.EventListener { return httpsec.OnHandlerOperationStart(func(op *httpsec.Operation, args httpsec.HandlerOperationArgs) { + var body interface{} + op.On(httpsec.OnSDKBodyOperationStart(func(op *httpsec.SDKBodyOperation, args httpsec.SDKBodyOperationArgs) { + 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. op.On(httpsec.OnHandlerOperationFinish(func(op *httpsec.Operation, res httpsec.HandlerOperationRes) { @@ -114,6 +118,10 @@ func newHTTPWAFEventListener(handle *waf.Handle, addresses []string, timeout tim if pathParams := args.PathParams; pathParams != nil { values[serverRequestPathParams] = pathParams } + case serverRequestBody: + if body != nil { + values[serverRequestBody] = body + } case serverResponseStatusAddr: values[serverResponseStatusAddr] = res.Status } @@ -210,6 +218,7 @@ const ( serverRequestCookiesAddr = "server.request.cookies" serverRequestQueryAddr = "server.request.query" serverRequestPathParams = "server.request.path_params" + serverRequestBody = "server.request.body" serverResponseStatusAddr = "server.response.status" ) @@ -220,6 +229,7 @@ var httpAddresses = []string{ serverRequestCookiesAddr, serverRequestQueryAddr, serverRequestPathParams, + serverRequestBody, serverResponseStatusAddr, } diff --git a/internal/appsec/waf_test.go b/internal/appsec/waf_test.go index 3d9bfa9a5f..404779ebc2 100644 --- a/internal/appsec/waf_test.go +++ b/internal/appsec/waf_test.go @@ -15,6 +15,7 @@ import ( "strings" "testing" + pAppsec "gopkg.in/DataDog/dd-trace-go.v1/appsec" httptrace "gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec" @@ -24,11 +25,8 @@ import ( // TestWAF is a simple validation test of the WAF protecting a net/http server. It only mockups the agent and tests that // the WAF is properly detecting an LFI attempt and that the corresponding security event is being sent to the agent. +// Additionally, verifies that rule matching through SDK body instrumentation works as expected func TestWAF(t *testing.T) { - // Start the tracer along with the fake agent HTTP server - mt := mocktracer.Start() - defer mt.Stop() - appsec.Start() defer appsec.Stop() @@ -41,32 +39,66 @@ func TestWAF(t *testing.T) { mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { w.Write([]byte("Hello World!\n")) }) + mux.HandleFunc("/body", func(w http.ResponseWriter, r *http.Request) { + pAppsec.MonitorParsedHTTPBody(r.Context(), "$globals") + w.Write([]byte("Hello Body!\n")) + }) srv := httptest.NewServer(mux) defer srv.Close() - // Send an LFI attack - req, err := http.NewRequest("POST", srv.URL+"/../../../secret.txt", nil) - if err != nil { - panic(err) - } - res, err := srv.Client().Do(req) - require.NoError(t, err) - - // Check that the handler was properly called - b, err := ioutil.ReadAll(res.Body) - require.NoError(t, err) - require.Equal(t, "Hello World!\n", string(b)) - - finished := mt.FinishedSpans() - require.Len(t, finished, 2) - - // Two requests were performed by the client request (due to the 301 redirection) and the two should have the LFI - // attack attempt event (appsec rule id crs-930-100). - event := finished[0].Tag("_dd.appsec.json") - require.NotNil(t, event) - require.True(t, strings.Contains(event.(string), "crs-930-100")) - - event = finished[1].Tag("_dd.appsec.json") - require.NotNil(t, event) - require.True(t, strings.Contains(event.(string), "crs-930-100")) + t.Run("lfi", func(t *testing.T) { + mt := mocktracer.Start() + defer mt.Stop() + + // Send an LFI attack + req, err := http.NewRequest("POST", srv.URL+"/../../../secret.txt", nil) + if err != nil { + panic(err) + } + res, err := srv.Client().Do(req) + require.NoError(t, err) + + // Check that the handler was properly called + b, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, "Hello World!\n", string(b)) + + finished := mt.FinishedSpans() + require.Len(t, finished, 2) + + // Two requests were performed by the client request (due to the 301 redirection) and the two should have the LFI + // attack attempt event (appsec rule id crs-930-100). + event := finished[0].Tag("_dd.appsec.json") + require.NotNil(t, event) + require.True(t, strings.Contains(event.(string), "crs-930-100")) + + event = finished[1].Tag("_dd.appsec.json") + require.NotNil(t, event) + require.True(t, strings.Contains(event.(string), "crs-930-100")) + }) + + // Test a PHP injection attack via request parsed body + t.Run("SDK-body", func(t *testing.T) { + mt := mocktracer.Start() + defer mt.Stop() + + req, err := http.NewRequest("POST", srv.URL+"/body", nil) + if err != nil { + panic(err) + } + res, err := srv.Client().Do(req) + require.NoError(t, err) + + // Check that the handler was properly called + b, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, "Hello Body!\n", string(b)) + + finished := mt.FinishedSpans() + require.Len(t, finished, 1) + + event := finished[0].Tag("_dd.appsec.json") + require.NotNil(t, event) + require.True(t, strings.Contains(event.(string), "crs-933-130")) + }) }