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

@@ -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())
+		}
+	}
+}

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()
 

contrib/gin-gonic/gin/gintrace_test.go

@@ -9,17 +9,21 @@ import (
 	"errors"
 	"fmt"
 	"html/template"
+	"io/ioutil"
+	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
 
 	"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"))
+
+	})
+}