Merge branch 'main' into shevchenko/none-ppropagator

CODEOWNERS

@@ -15,7 +15,7 @@
 # appsec
 /appsec                         @DataDog/appsec-go
 /internal/appsec                @DataDog/appsec-go
-/contrib/**/appsec.go           @DataDog/appsec-go
+/contrib/**/*appsec*.go         @DataDog/appsec-go
 /.github/workflows/appsec.yml   @DataDog/appsec-go
 
 # telemetry

contrib/gin-gonic/gin/appsec.go

@@ -6,8 +6,6 @@
 package gin
 
 import (
-	"net"
-
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo/instrumentation"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo/instrumentation/httpsec"
@@ -18,27 +16,29 @@ import (
 // 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
 	instrumentation.SetAppSecEnabledTags(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)
+
+	req := c.Request
+	ipTags, clientIP := httpsec.ClientIPTags(req.Header, req.RemoteAddr)
+	instrumentation.SetStringTags(span, ipTags)
+
+	args := httpsec.MakeHandlerOperationArgs(req, clientIP, params)
 	ctx, op := httpsec.StartOperation(req.Context(), args)
 	c.Request = req.WithContext(ctx)
+
 	return func() {
 		events := op.Finish(httpsec.HandlerOperationRes{Status: c.Writer.Status()})
+		instrumentation.SetTags(span, op.Tags())
 		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())
+			httpsec.SetSecurityEventTags(span, events, args.Headers, c.Writer.Header())
 		}
-		instrumentation.SetTags(span, op.Tags())
 	}
 }

contrib/google.golang.org/grpc/appsec.go

@@ -7,32 +7,46 @@ package grpc
 
 import (
 	"encoding/json"
-	"net"
 
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo/instrumentation"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo/instrumentation/grpcsec"
+	"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo/instrumentation/httpsec"
 
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/metadata"
 	"google.golang.org/grpc/peer"
+	"google.golang.org/grpc/status"
 )
 
 // UnaryHandler wrapper to use when AppSec is enabled to monitor its execution.
 func appsecUnaryHandlerMiddleware(span ddtrace.Span, handler grpc.UnaryHandler) grpc.UnaryHandler {
 	instrumentation.SetAppSecEnabledTags(span)
 	return func(ctx context.Context, req interface{}) (interface{}, error) {
 		md, _ := metadata.FromIncomingContext(ctx)
-		op := grpcsec.StartHandlerOperation(grpcsec.HandlerOperationArgs{Metadata: md}, nil)
+		var remoteAddr string
+		if p, ok := peer.FromContext(ctx); ok {
+			remoteAddr = p.Addr.String()
+		}
+		ipTags, clientIP := httpsec.ClientIPTags(md, remoteAddr)
+		instrumentation.SetStringTags(span, ipTags)
+
+		op := grpcsec.StartHandlerOperation(grpcsec.HandlerOperationArgs{Metadata: md, ClientIP: clientIP}, nil)
 		defer func() {
 			events := op.Finish(grpcsec.HandlerOperationRes{})
 			instrumentation.SetTags(span, op.Tags())
 			if len(events) == 0 {
 				return
 			}
-			setAppSecTags(ctx, span, events)
+			setAppSecEventsTags(ctx, span, events)
 		}()
+
+		if op.BlockedCode != nil {
+			op.AddTag(httpsec.BlockedRequestTag, true)
+			return nil, status.Errorf(*op.BlockedCode, "Request blocked")
+		}
+
 		defer grpcsec.StartReceiveOperation(grpcsec.ReceiveOperationArgs{}, op).Finish(grpcsec.ReceiveOperationRes{Message: req})
 		return handler(ctx, req)
 	}
@@ -43,15 +57,28 @@ func appsecStreamHandlerMiddleware(span ddtrace.Span, handler grpc.StreamHandler
 	instrumentation.SetAppSecEnabledTags(span)
 	return func(srv interface{}, stream grpc.ServerStream) error {
 		md, _ := metadata.FromIncomingContext(stream.Context())
-		op := grpcsec.StartHandlerOperation(grpcsec.HandlerOperationArgs{Metadata: md}, nil)
+		var remoteAddr string
+		if p, ok := peer.FromContext(stream.Context()); ok {
+			remoteAddr = p.Addr.String()
+		}
+		ipTags, clientIP := httpsec.ClientIPTags(md, remoteAddr)
+		instrumentation.SetStringTags(span, ipTags)
+
+		op := grpcsec.StartHandlerOperation(grpcsec.HandlerOperationArgs{Metadata: md, ClientIP: clientIP}, nil)
 		defer func() {
 			events := op.Finish(grpcsec.HandlerOperationRes{})
 			instrumentation.SetTags(span, op.Tags())
 			if len(events) == 0 {
 				return
 			}
-			setAppSecTags(stream.Context(), span, events)
+			setAppSecEventsTags(stream.Context(), span, events)
 		}()
+
+		if op.BlockedCode != nil {
+			op.AddTag(httpsec.BlockedRequestTag, true)
+			return status.Error(*op.BlockedCode, "Request blocked")
+		}
+
 		return handler(srv, appsecServerStream{ServerStream: stream, handlerOperation: op})
 	}
 }
@@ -72,11 +99,7 @@ func (ss appsecServerStream) RecvMsg(m interface{}) error {
 }
 
 // Set the AppSec tags when security events were found.
-func setAppSecTags(ctx context.Context, span ddtrace.Span, events []json.RawMessage) {
+func setAppSecEventsTags(ctx context.Context, span ddtrace.Span, events []json.RawMessage) {
 	md, _ := metadata.FromIncomingContext(ctx)
-	var addr net.Addr
-	if p, ok := peer.FromContext(ctx); ok {
-		addr = p.Addr
-	}
-	grpcsec.SetSecurityEventTags(span, events, addr, md)
+	grpcsec.SetSecurityEventTags(span, events, md)
 }

contrib/google.golang.org/grpc/appsec_test.go

@@ -14,7 +14,9 @@ import (
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec"
 
 	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/metadata"
+	"google.golang.org/grpc/status"
 )
 
 func TestAppSec(t *testing.T) {
@@ -94,3 +96,90 @@ func TestAppSec(t *testing.T) {
 		require.True(t, strings.Contains(event, "ua0-600-55x")) // canary rule attack attempt
 	})
 }
+
+// Test that http blocking works by using custom rules/rules data
+func TestBlocking(t *testing.T) {
+	t.Setenv("DD_APPSEC_RULES", "../../../internal/appsec/testdata/blocking.json")
+	appsec.Start()
+	defer appsec.Stop()
+	if !appsec.Enabled() {
+		t.Skip("appsec disabled")
+	}
+
+	rig, err := newRig(false)
+	require.NoError(t, err)
+	defer rig.Close()
+
+	client := rig.client
+
+	t.Run("unary-block", func(t *testing.T) {
+		mt := mocktracer.Start()
+		defer mt.Stop()
+
+		// Send a XSS attack in the payload along with the canary value in the RPC metadata
+		ctx := metadata.NewOutgoingContext(context.Background(), metadata.Pairs("dd-canary", "dd-test-scanner-log", "x-client-ip", "1.2.3.4"))
+		reply, err := client.Ping(ctx, &FixtureRequest{Name: "<script>alert('xss');</script>"})
+
+		require.Nil(t, reply)
+		require.Equal(t, codes.Aborted, status.Code(err))
+
+		finished := mt.FinishedSpans()
+		require.Len(t, finished, 1)
+		// The request should have the attack attempts
+		event, _ := finished[0].Tag("_dd.appsec.json").(string)
+		require.NotNil(t, event)
+		require.True(t, strings.Contains(event, "blk-001-001"))
+	})
+
+	t.Run("unary-no-block", func(t *testing.T) {
+		mt := mocktracer.Start()
+		defer mt.Stop()
+
+		// Send a XSS attack in the payload along with the canary value in the RPC metadata
+		ctx := metadata.NewOutgoingContext(context.Background(), metadata.Pairs("dd-canary", "dd-test-scanner-log", "x-client-ip", "1.2.3.5"))
+		reply, err := client.Ping(ctx, &FixtureRequest{Name: "<script>alert('xss');</script>"})
+
+		require.Equal(t, "passed", reply.Message)
+		require.Equal(t, codes.OK, status.Code(err))
+	})
+
+	t.Run("stream-block", func(t *testing.T) {
+		mt := mocktracer.Start()
+		defer mt.Stop()
+
+		ctx := metadata.NewOutgoingContext(context.Background(), metadata.Pairs("dd-canary", "dd-test-scanner-log", "x-client-ip", "1.2.3.4"))
+		stream, err := client.StreamPing(ctx)
+		require.NoError(t, err)
+		reply, err := stream.Recv()
+
+		require.Equal(t, codes.Aborted, status.Code(err))
+		require.Nil(t, reply)
+
+		finished := mt.FinishedSpans()
+		require.Len(t, finished, 1)
+		// The request should have the attack attempts
+		event, _ := finished[0].Tag("_dd.appsec.json").(string)
+		require.NotNil(t, event)
+		require.True(t, strings.Contains(event, "blk-001-001"))
+	})
+
+	t.Run("stream-no-block", func(t *testing.T) {
+		mt := mocktracer.Start()
+		defer mt.Stop()
+
+		ctx := metadata.NewOutgoingContext(context.Background(), metadata.Pairs("dd-canary", "dd-test-scanner-log", "x-client-ip", "1.2.3.5"))
+		stream, err := client.StreamPing(ctx)
+		require.NoError(t, err)
+
+		// Send a XSS attack
+		err = stream.Send(&FixtureRequest{Name: "<script>alert('xss');</script>"})
+		require.NoError(t, err)
+		reply, err := stream.Recv()
+		require.Equal(t, codes.OK, status.Code(err))
+		require.Equal(t, "passed", reply.Message)
+
+		err = stream.CloseSend()
+		require.NoError(t, err)
+	})
+
+}

contrib/labstack/echo.v4/appsec.go

@@ -6,8 +6,6 @@
 package echo
 
 import (
-	"net"
-
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo/instrumentation"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo/instrumentation/httpsec"
@@ -16,23 +14,24 @@ import (
 )
 
 func useAppSec(c echo.Context, span tracer.Span) func() {
-	req := c.Request()
 	instrumentation.SetAppSecEnabledTags(span)
+
 	params := make(map[string]string)
 	for _, n := range c.ParamNames() {
 		params[n] = c.Param(n)
 	}
-	args := httpsec.MakeHandlerOperationArgs(req, params)
+
+	req := c.Request()
+	ipTags, clientIP := httpsec.ClientIPTags(req.Header, req.RemoteAddr)
+	instrumentation.SetStringTags(span, ipTags)
+
+	args := httpsec.MakeHandlerOperationArgs(req, clientIP, params)
 	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 {
-			remoteIP, _, err := net.SplitHostPort(req.RemoteAddr)
-			if err != nil {
-				remoteIP = req.RemoteAddr
-			}
-			httpsec.SetSecurityEventTags(span, events, remoteIP, args.Headers, c.Response().Writer.Header())
+			httpsec.SetSecurityEventTags(span, events, args.Headers, c.Response().Writer.Header())
 		}
 		instrumentation.SetTags(span, op.Tags())
 	}

contrib/labstack/echo.v4/echotrace.go

@@ -7,7 +7,11 @@
 package echo
 
 import (
+	"errors"
+	"fmt"
 	"math"
+	"net/http"
+	"strconv"
 
 	"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
@@ -60,23 +64,48 @@ func Middleware(opts ...Option) echo.MiddlewareFunc {
 
 			span, ctx := httptrace.StartRequestSpan(request, opts...)
 			defer func() {
-				httptrace.FinishRequestSpan(span, c.Response().Status, finishOpts...)
+				span.Finish(finishOpts...)
 			}()
 
 			// pass the span through the request context
 			c.SetRequest(request.WithContext(ctx))
-			// serve the request to the next middleware
+
 			if appsecEnabled {
 				afterMiddleware := useAppSec(c, span)
 				defer afterMiddleware()
 			}
+			// serve the request to the next middleware
 			err := next(c)
 			if err != nil {
-				finishOpts = append(finishOpts, tracer.WithError(err))
 				// invokes the registered HTTP error handler
 				c.Error(err)
-			}
 
+				// It is impossible to determine what the final status code of a request is in echo.
+				// This is the best we can do.
+				var echoErr *echo.HTTPError
+				if errors.As(err, &echoErr) {
+					if cfg.isStatusError(echoErr.Code) {
+						finishOpts = append(finishOpts, tracer.WithError(err))
+					}
+					span.SetTag(ext.HTTPCode, strconv.Itoa(echoErr.Code))
+				} else {
+					// Any error that is not an *echo.HTTPError will be treated as an error with 500 status code.
+					if cfg.isStatusError(500) {
+						finishOpts = append(finishOpts, tracer.WithError(err))
+					}
+					span.SetTag(ext.HTTPCode, "500")
+				}
+			} else if status := c.Response().Status; status > 0 {
+				if cfg.isStatusError(status) {
+					finishOpts = append(finishOpts, tracer.WithError(fmt.Errorf("%d: %s", status, http.StatusText(status))))
+				}
+				span.SetTag(ext.HTTPCode, strconv.Itoa(status))
+			} else {
+				if cfg.isStatusError(200) {
+					finishOpts = append(finishOpts, tracer.WithError(fmt.Errorf("%d: %s", 200, http.StatusText(200))))
+				}
+				span.SetTag(ext.HTTPCode, "200")
+			}
 			return err
 		}
 	}