diff --git a/contrib/net/http/roundtripper.go b/contrib/net/http/roundtripper.go index 77e664ba19..083d32908a 100644 --- a/contrib/net/http/roundtripper.go +++ b/contrib/net/http/roundtripper.go @@ -6,12 +6,14 @@ package http import ( + "errors" "fmt" "math" "net/http" "os" "strconv" + "gopkg.in/DataDog/dd-trace-go.v1/appsec/events" "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" @@ -59,7 +61,7 @@ func (rt *roundTripper) RoundTrip(req *http.Request) (res *http.Response, err er if rt.cfg.after != nil { rt.cfg.after(res, span) } - if rt.cfg.errCheck == nil || rt.cfg.errCheck(err) { + if !errors.Is(err, &events.SecurityBlockingEvent{}) && (rt.cfg.errCheck == nil || rt.cfg.errCheck(err)) { span.Finish(tracer.WithError(err)) } else { span.Finish() @@ -77,11 +79,15 @@ func (rt *roundTripper) RoundTrip(req *http.Request) (res *http.Response, err er fmt.Fprintf(os.Stderr, "contrib/net/http.Roundtrip: failed to inject http headers: %v\n", err) } } + if appsec.Enabled() { - res, err = httpsec.RoundTrip(ctx, r2, rt.base) - } else { - res, err = rt.base.RoundTrip(r2) + if err := httpsec.ProtectRoundTrip(ctx, r2.URL.String()); err != nil { + return nil, err + } } + + res, err = rt.base.RoundTrip(r2) + if err != nil { span.SetTag("http.errors", err.Error()) if rt.cfg.errCheck == nil || rt.cfg.errCheck(err) { diff --git a/contrib/net/http/roundtripper_test.go b/contrib/net/http/roundtripper_test.go index 725f666e97..5544ec3c6a 100644 --- a/contrib/net/http/roundtripper_test.go +++ b/contrib/net/http/roundtripper_test.go @@ -8,7 +8,6 @@ package http import ( "encoding/base64" "fmt" - "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/listener/httpsec" "net/http" "net/http/httptest" "net/url" @@ -17,12 +16,14 @@ import ( "testing" "time" + "gopkg.in/DataDog/dd-trace-go.v1/appsec/events" "gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/namingschematest" "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/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/appsec/listener/httpsec" "gopkg.in/DataDog/dd-trace-go.v1/internal/globalconfig" "github.com/stretchr/testify/assert" @@ -653,8 +654,10 @@ func TestAppsec(t *testing.T) { require.NoError(t, err) resp, err := client.RoundTrip(req.WithContext(r.Context())) - defer require.NoError(t, err) - defer resp.Body.Close() + require.ErrorIs(t, err, &events.SecurityBlockingEvent{}) + if resp != nil { + defer resp.Body.Close() + } }), w, r, &ServeConfig{ Service: "service", Resource: "resource", @@ -665,10 +668,11 @@ func TestAppsec(t *testing.T) { serviceSpan := spans[1] require.Contains(t, serviceSpan.Tags(), "_dd.appsec.json") - appsecJSON := serviceSpan.Tag("_dd.appsec.json") require.Contains(t, appsecJSON, httpsec.ServerIoNetURLAddr) + require.Contains(t, serviceSpan.Tags(), "_dd.stack") + // This is a nested event so it should contain the child span id in the service entry span // TODO(eliott.bouhana): uncomment this once we have the child span id in the service entry span // require.Contains(t, appsecJSON, `"span_id":`+strconv.FormatUint(requestSpan.SpanID(), 10)) diff --git a/internal/appsec/emitter/httpsec/roundtripper.go b/internal/appsec/emitter/httpsec/roundtripper.go index 7c97af382b..a495276b37 100644 --- a/internal/appsec/emitter/httpsec/roundtripper.go +++ b/internal/appsec/emitter/httpsec/roundtripper.go @@ -7,7 +7,6 @@ package httpsec import ( "context" - "net/http" "sync" "gopkg.in/DataDog/dd-trace-go.v1/appsec/events" @@ -19,8 +18,7 @@ import ( var badInputContextOnce sync.Once -func RoundTrip(ctx context.Context, request *http.Request, rt http.RoundTripper) (*http.Response, error) { - url := request.URL.String() +func ProtectRoundTrip(ctx context.Context, url string) error { opArgs := types.RoundTripOperationArgs{ URL: url, } @@ -32,7 +30,7 @@ func RoundTrip(ctx context.Context, request *http.Request, rt http.RoundTripper) "instrumentation metadata in the request context: the request handler is not being monitored by a " + "middleware function or the incoming request context has not be forwarded correctly to the roundtripper") }) - return rt.RoundTrip(request) + return nil } op := &types.RoundTripOperation{ @@ -49,8 +47,8 @@ func RoundTrip(ctx context.Context, request *http.Request, rt http.RoundTripper) if err != nil { log.Debug("appsec: outgoing http request blocked by the WAF on URL: %s", url) - return nil, err + return err } - return rt.RoundTrip(request) + return nil } diff --git a/internal/appsec/listener/httpsec/roundtripper.go b/internal/appsec/listener/httpsec/roundtripper.go index afc4c2fbeb..93b7867b87 100644 --- a/internal/appsec/listener/httpsec/roundtripper.go +++ b/internal/appsec/listener/httpsec/roundtripper.go @@ -17,7 +17,7 @@ import ( // RegisterRoundTripperListener registers a listener on outgoing requests to run the WAF. func RegisterRoundTripperListener(op dyngo.Operation, events *trace.SecurityEventsHolder, wafCtx *waf.Context, limiter limiter.Limiter) { - dyngo.On(op, func(operation *types.RoundTripOperation, args types.RoundTripOperationArgs) { + dyngo.On(op, func(op *types.RoundTripOperation, args types.RoundTripOperationArgs) { wafResult := sharedsec.RunWAF(wafCtx, waf.RunAddressData{Persistent: map[string]any{ServerIoNetURLAddr: args.URL}}) if !wafResult.HasEvents() { return diff --git a/internal/appsec/listener/sharedsec/shared.go b/internal/appsec/listener/sharedsec/shared.go index dc8fe5e775..6c3dcb4de2 100644 --- a/internal/appsec/listener/sharedsec/shared.go +++ b/internal/appsec/listener/sharedsec/shared.go @@ -79,6 +79,7 @@ func AddWAFMonitoringTags(th trace.TagSetter, rulesVersion string, stats map[str // When SDKError is not nil, this error is sent to the op with EmitData so that the invoked SDK can return it func ProcessActions(op dyngo.Operation, actions map[string]any) (interrupt bool) { for aType, params := range actions { + log.Debug("appsec: processing %s action with params %v", aType, params) actionArray := ActionsFromEntry(aType, params) if actionArray == nil { log.Debug("cannot process %s action with params %v", aType, params) @@ -86,12 +87,16 @@ func ProcessActions(op dyngo.Operation, actions map[string]any) (interrupt bool) } for _, a := range actionArray { a.EmitData(op) - if a.Blocking() { // Send the error to be returned by the SDK - interrupt = true - dyngo.EmitData(op, &events.SecurityBlockingEvent{}) // Send error - } + interrupt = interrupt || a.Blocking() } } + + // If any of the actions are supposed to interrupt the request, emit a blocking event for the SDK operations + // to return an error. + if interrupt { + dyngo.EmitData(op, &events.SecurityBlockingEvent{}) + } + return interrupt } diff --git a/internal/appsec/testdata/rasp.json b/internal/appsec/testdata/rasp.json index fe2db6742b..ecbaf0d16f 100644 --- a/internal/appsec/testdata/rasp.json +++ b/internal/appsec/testdata/rasp.json @@ -49,7 +49,8 @@ ], "transformers": [], "on_match": [ - "stack_trace" + "stack_trace", + "block" ] }, { @@ -97,7 +98,8 @@ ], "transformers": [], "on_match": [ - "stack_trace" + "stack_trace", + "block" ] }, { @@ -147,7 +149,8 @@ ], "transformers": [], "on_match": [ - "stack_trace" + "stack_trace", + "block" ] } ], diff --git a/internal/stacktrace/event.go b/internal/stacktrace/event.go index feb1e2c581..ac292f13f6 100644 --- a/internal/stacktrace/event.go +++ b/internal/stacktrace/event.go @@ -88,14 +88,16 @@ func AddToSpan(span ddtrace.Span, events ...*Event) { return } - groupByCategory := map[EventCategory][]*Event{ - ExceptionEvent: {}, - VulnerabilityEvent: {}, - ExploitEvent: {}, - } + // TODO(eliott.bouhana): switch to a map[EventCategory][]*Event type when the tinylib/msgp@1.1.10 is out + groupByCategory := make(map[string]any, 3) for _, event := range events { - groupByCategory[event.Category] = append(groupByCategory[event.Category], event) + if _, ok := groupByCategory[string(event.Category)]; !ok { + groupByCategory[string(event.Category)] = []*Event{event} + continue + } + + groupByCategory[string(event.Category)] = append(groupByCategory[string(event.Category)].([]*Event), event) } type rooter interface { diff --git a/internal/stacktrace/event_test.go b/internal/stacktrace/event_test.go index 5da7febd9c..3ebdb554c3 100644 --- a/internal/stacktrace/event_test.go +++ b/internal/stacktrace/event_test.go @@ -6,6 +6,7 @@ package stacktrace import ( + "github.com/tinylib/msgp/msgp" "testing" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer" @@ -38,11 +39,29 @@ func TestEventToSpan(t *testing.T) { require.Len(t, spans, 1) require.Equal(t, "op", spans[0].OperationName()) - eventsMap := spans[0].Tag("_dd.stack").(internal.MetaStructValue).Value.(map[EventCategory][]*Event) - require.Len(t, eventsMap, 3) + eventsMap := spans[0].Tag("_dd.stack").(internal.MetaStructValue).Value.(map[string]any) + require.Len(t, eventsMap, 1) - eventsCat := eventsMap[ExceptionEvent] + eventsCat := eventsMap[string(ExceptionEvent)].([]*Event) require.Len(t, eventsCat, 1) require.Equal(t, *event, *eventsCat[0]) } + +func TestMsgPackSerialization(t *testing.T) { + mt := mocktracer.Start() + defer mt.Stop() + + span := ddtracer.StartSpan("op") + event := NewEvent(ExceptionEvent, WithMessage("message"), WithType("type"), WithID("id")) + AddToSpan(span, event) + span.Finish() + + spans := mt.FinishedSpans() + require.Len(t, spans, 1) + + eventsMap := spans[0].Tag("_dd.stack").(internal.MetaStructValue).Value + + _, err := msgp.AppendIntf(nil, eventsMap) + require.NoError(t, err) +}