internal/appsec: implement WAF actions for http/grpc (#1533)

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

internal/appsec/dyngo/instrumentation/common.go

@@ -49,7 +49,7 @@ func (m *TagsHolder) Tags() map[string]interface{} {
 // See httpsec/http.go and grpcsec/grpc.go.
 type SecurityEventsHolder struct {
 	events []json.RawMessage
-	mu     sync.Mutex
+	mu     sync.RWMutex
 }
 
 // AddSecurityEvents adds the security events to the collected events list.
@@ -62,16 +62,33 @@ func (s *SecurityEventsHolder) AddSecurityEvents(events ...json.RawMessage) {
 
 // Events returns the list of stored events.
 func (s *SecurityEventsHolder) Events() []json.RawMessage {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
 	return s.events
 }
 
+// ClearEvents clears the list of stored events
+func (s *SecurityEventsHolder) ClearEvents() {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.events = s.events[0:0]
+}
+
 // SetTags fills the span tags using the key/value pairs found in `tags`
 func SetTags(span TagSetter, tags map[string]interface{}) {
 	for k, v := range tags {
 		span.SetTag(k, v)
 	}
 }
 
+// SetStringTags fills the span tags using the key/value pairs of strings found
+// in `tags`
+func SetStringTags(span TagSetter, tags map[string]string) {
+	for k, v := range tags {
+		span.SetTag(k, v)
+	}
+}
+
 // SetAppSecEnabledTags sets the AppSec-specific span tags that are expected to be in
 // the web service entry span (span of type `web`) when AppSec is enabled.
 func SetAppSecEnabledTags(span TagSetter) {
@@ -102,9 +119,11 @@ func SetEventSpanTags(span TagSetter, events []json.RawMessage) error {
 
 // Create the value of the security event tag.
 // TODO(Julio-Guerra): a future libddwaf version should return something
-//   avoiding us the following events concatenation logic which currently
-//   involves unserializing the top-level JSON arrays to concatenate them
-//   together.
+//
+//	avoiding us the following events concatenation logic which currently
+//	involves unserializing the top-level JSON arrays to concatenate them
+//	together.
+//
 // TODO(Julio-Guerra): avoid serializing the json in the request hot path
 func makeEventTagValue(events []json.RawMessage) (json.RawMessage, error) {
 	var v interface{}