From 0d15cb7558d0c6806b8707dc25f84332354b4750 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Wed, 16 Feb 2022 15:33:51 +0100 Subject: [PATCH] contrib/gin-gonic: add AppSec support to gin framework (#1165) - Add a call to useAppsec in gintrace.go when AppSec is enabled - Add testing of AppSec functionalities in gintrace_test.go - Add appsec.go file that holds AppSec handling for gin --- contrib/gin-gonic/gin/appsec.go | 41 ++++++ contrib/gin-gonic/gin/gintrace.go | 8 ++ contrib/gin-gonic/gin/gintrace_test.go | 173 ++++++++++++++++++++++++- 3 files changed, 221 insertions(+), 1 deletion(-) create mode 100644 contrib/gin-gonic/gin/appsec.go diff --git a/contrib/gin-gonic/gin/appsec.go b/contrib/gin-gonic/gin/appsec.go new file mode 100644 index 0000000000..2462f0c297 --- /dev/null +++ b/contrib/gin-gonic/gin/appsec.go @@ -0,0 +1,41 @@ +// 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 gin + +import ( + "net" + + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" + "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo/instrumentation/httpsec" + + "github.com/gin-gonic/gin" +) + +// useAppSec executes the AppSec logic related to the operation start and +// returns the function to be executed upon finishing the operation +func useAppSec(c *gin.Context, span tracer.Span) func() { + req := c.Request + httpsec.SetAppSecTags(span) + var params map[string]string + if l := len(c.Params); l > 0 { + params = make(map[string]string, l) + for _, p := range c.Params { + params[p.Key] = p.Value + } + } + args := httpsec.MakeHandlerOperationArgs(req, params) + op := httpsec.StartOperation(args, nil) + return func() { + events := op.Finish(httpsec.HandlerOperationRes{Status: c.Writer.Status()}) + if len(events) > 0 { + remoteIP, _, err := net.SplitHostPort(req.RemoteAddr) + if err != nil { + remoteIP = req.RemoteAddr + } + httpsec.SetSecurityEventTags(span, events, remoteIP, args.Headers, c.Writer.Header()) + } + } +} diff --git a/contrib/gin-gonic/gin/gintrace.go b/contrib/gin-gonic/gin/gintrace.go index e1342965b1..a5b53d32e9 100644 --- a/contrib/gin-gonic/gin/gintrace.go +++ b/contrib/gin-gonic/gin/gintrace.go @@ -15,6 +15,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" + "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec" "gopkg.in/DataDog/dd-trace-go.v1/internal/log" "github.com/gin-gonic/gin" @@ -23,6 +24,7 @@ import ( // Middleware returns middleware that will trace incoming requests. If service is empty then the // default service name will be used. func Middleware(service string, opts ...Option) gin.HandlerFunc { + appsecEnabled := appsec.Enabled() cfg := newConfig(service) for _, opt := range opts { opt(cfg) @@ -53,6 +55,12 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc { // pass the span through the request context c.Request = c.Request.WithContext(ctx) + // Use AppSec if enabled by user + if appsecEnabled { + afterMiddleware := useAppSec(c, span) + defer afterMiddleware() + } + // serve the request to the next middleware c.Next() diff --git a/contrib/gin-gonic/gin/gintrace_test.go b/contrib/gin-gonic/gin/gintrace_test.go index 1009caa061..32aff62198 100644 --- a/contrib/gin-gonic/gin/gintrace_test.go +++ b/contrib/gin-gonic/gin/gintrace_test.go @@ -9,6 +9,8 @@ import ( "errors" "fmt" "html/template" + "io/ioutil" + "net/http" "net/http/httptest" "strings" "testing" @@ -16,10 +18,12 @@ import ( "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" + "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec" "gopkg.in/DataDog/dd-trace-go.v1/internal/globalconfig" "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func init() { @@ -80,7 +84,81 @@ func TestTrace200(t *testing.T) { assert.Contains(span.Tag(ext.ResourceName), "GET /user/:id") assert.Equal("200", span.Tag(ext.HTTPCode)) assert.Equal("GET", span.Tag(ext.HTTPMethod)) - // TODO(x) would be much nicer to have "/user/:id" here + assert.Equal("/user/123", span.Tag(ext.HTTPURL)) +} + +func TestTraceDefaultResponse(t *testing.T) { + assert := assert.New(t) + mt := mocktracer.Start() + defer mt.Stop() + + router := gin.New() + router.Use(Middleware("foobar")) + router.GET("/user/:id", func(c *gin.Context) { + _, ok := tracer.SpanFromContext(c.Request.Context()) + assert.True(ok) + }) + + r := httptest.NewRequest("GET", "/user/123", nil) + w := httptest.NewRecorder() + + // do and verify the request + router.ServeHTTP(w, r) + response := w.Result() + assert.Equal(response.StatusCode, 200) + + // verify traces look good + spans := mt.FinishedSpans() + assert.Len(spans, 1) + if len(spans) < 1 { + t.Fatalf("no spans") + } + span := spans[0] + assert.Equal("http.request", span.OperationName()) + assert.Equal(ext.SpanTypeWeb, span.Tag(ext.SpanType)) + assert.Equal("foobar", span.Tag(ext.ServiceName)) + assert.Contains(span.Tag(ext.ResourceName), "GET /user/:id") + assert.Equal("200", span.Tag(ext.HTTPCode)) + assert.Equal("GET", span.Tag(ext.HTTPMethod)) + assert.Equal("/user/123", span.Tag(ext.HTTPURL)) +} + +func TestTraceMultipleResponses(t *testing.T) { + assert := assert.New(t) + mt := mocktracer.Start() + defer mt.Stop() + + router := gin.New() + router.Use(Middleware("foobar")) + router.GET("/user/:id", func(c *gin.Context) { + _, ok := tracer.SpanFromContext(c.Request.Context()) + assert.True(ok) + c.Status(142) + c.Writer.WriteString("test") + c.Status(133) + }) + + r := httptest.NewRequest("GET", "/user/123", nil) + w := httptest.NewRecorder() + + // do and verify the request + router.ServeHTTP(w, r) + response := w.Result() + assert.Equal(response.StatusCode, 142) + + // verify traces look good + spans := mt.FinishedSpans() + assert.Len(spans, 1) + if len(spans) < 1 { + t.Fatalf("no spans") + } + span := spans[0] + assert.Equal("http.request", span.OperationName()) + assert.Equal(ext.SpanTypeWeb, span.Tag(ext.SpanType)) + assert.Equal("foobar", span.Tag(ext.ServiceName)) + assert.Contains(span.Tag(ext.ResourceName), "GET /user/:id") + assert.Equal("133", span.Tag(ext.HTTPCode)) // Will be fixed by https://github.com/gin-gonic/gin/pull/2627 once merged and released + assert.Equal("GET", span.Tag(ext.HTTPMethod)) assert.Equal("/user/123", span.Tag(ext.HTTPURL)) } @@ -460,3 +538,96 @@ func TestServiceName(t *testing.T) { assert.Equal("my-service", span.Tag(ext.ServiceName)) }) } + +func TestAppSec(t *testing.T) { + appsec.Start() + defer appsec.Stop() + if !appsec.Enabled() { + t.Skip("appsec disabled") + } + + r := gin.New() + r.Use(Middleware("appsec")) + r.Any("/lfi/*allPaths", func(c *gin.Context) { + c.String(200, "Hello World!\n") + }) + r.Any("/path0.0/:myPathParam0/path0.1/:myPathParam1/path0.2/:myPathParam2/path0.3/*param3", func(c *gin.Context) { + c.String(200, "Hello Params!\n") + }) + + srv := httptest.NewServer(r) + defer srv.Close() + + t.Run("request-uri", func(t *testing.T) { + mt := mocktracer.Start() + defer mt.Stop() + // Send an LFI attack (according to appsec rule id crs-930-100) + req, err := http.NewRequest("POST", srv.URL+"/lfi/../../../secret.txt", nil) + if err != nil { + panic(err) + } + res, err := srv.Client().Do(req) + require.NoError(t, err) + // Check that the server behaved as intended + require.Equal(t, http.StatusOK, res.StatusCode) + b, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, "Hello World!\n", string(b)) + // The span should contain the security event + finished := mt.FinishedSpans() + require.Len(t, finished, 1) + + // The first 301 redirection should contain the attack via the request uri + event := finished[0].Tag("_dd.appsec.json").(string) + require.NotNil(t, event) + require.True(t, strings.Contains(event, "server.request.uri.raw")) + require.True(t, strings.Contains(event, "crs-930-100")) + }) + + // Test a security scanner attack via path parameters + t.Run("path-params", func(t *testing.T) { + mt := mocktracer.Start() + defer mt.Stop() + // Send a security scanner attack (according to appsec rule id crs-913-120) + req, err := http.NewRequest("POST", srv.URL+"/path0.0/param0/path0.1/param1/path0.2/appscan_fingerprint/path0.3/param3", 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 Params!\n", string(b)) + require.Equal(t, http.StatusOK, res.StatusCode) + // The span should contain the security event + 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, "crs-913-120")) + require.True(t, strings.Contains(event, "myPathParam2")) + require.True(t, strings.Contains(event, "server.request.path_params")) + }) + + t.Run("nfd-000-001", 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) + + 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")) + + }) +}