From e6f290285af317a131ea45dd7c2e77a987b47c70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Wed, 16 Nov 2022 15:26:38 +0100 Subject: [PATCH] internal/appsec/dyngo: clear actions/events before second waf run --- internal/appsec/dyngo/instrumentation/common.go | 15 ++++++++++++--- .../appsec/dyngo/instrumentation/httpsec/http.go | 10 +++++++++- .../appsec/dyngo/instrumentation/httpsec/tags.go | 8 +++++--- .../dyngo/instrumentation/httpsec/tags_test.go | 4 ++-- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/internal/appsec/dyngo/instrumentation/common.go b/internal/appsec/dyngo/instrumentation/common.go index 9d0d66736e..4b47ccdab0 100644 --- a/internal/appsec/dyngo/instrumentation/common.go +++ b/internal/appsec/dyngo/instrumentation/common.go @@ -65,6 +65,13 @@ func (s *SecurityEventsHolder) Events() []json.RawMessage { return s.events } +// ClearEvents clears the list of stored events +func (s *SecurityEventsHolder) ClearEvents() { + s.mu.Lock() + defer s.mu.Unlock() + s.events = []json.RawMessage{} +} + // 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 { @@ -102,9 +109,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{} diff --git a/internal/appsec/dyngo/instrumentation/httpsec/http.go b/internal/appsec/dyngo/instrumentation/httpsec/http.go index 95319cd1a3..311d8ee8ae 100644 --- a/internal/appsec/dyngo/instrumentation/httpsec/http.go +++ b/internal/appsec/dyngo/instrumentation/httpsec/http.go @@ -80,7 +80,7 @@ func WrapHandler(handler http.Handler, span ddtrace.Span, pathParams map[string] switch a := action.(type) { case *BlockRequestAction: handler = a.handler - op.AddTag("appsec.blocked", true) + op.AddTag(tagBlockedRequest, true) default: log.Warn("appsec: unsupported action %v. Ignoring.", a) } @@ -97,6 +97,8 @@ func WrapHandler(handler http.Handler, span ddtrace.Span, pathParams map[string] events := op.Events() if len(events) > 0 { applyActions(op) + op.ClearEvents() + op.ClearActions() } defer func() { var status int @@ -210,10 +212,16 @@ func (op *Operation) Finish(res HandlerOperationRes) []json.RawMessage { return op.Events() } +// Actions returns the actions linked to the operation func (op *Operation) Actions() []Action { return op.actions } +// ClearActions clears all the actions linked to the operation +func (op *Operation) ClearActions() { + op.actions = []Action{} +} + // StartSDKBodyOperation starts the SDKBody operation and emits a start event func StartSDKBodyOperation(parent *Operation, args SDKBodyOperationArgs) *SDKBodyOperation { op := &SDKBodyOperation{Operation: dyngo.NewOperation(parent)} diff --git a/internal/appsec/dyngo/instrumentation/httpsec/tags.go b/internal/appsec/dyngo/instrumentation/httpsec/tags.go index faf300d950..769c9dc194 100644 --- a/internal/appsec/dyngo/instrumentation/httpsec/tags.go +++ b/internal/appsec/dyngo/instrumentation/httpsec/tags.go @@ -21,9 +21,11 @@ import ( const ( // envClientIPHeader is the name of the env var used to specify the IP header to be used for client IP collection. envClientIPHeader = "DD_TRACE_CLIENT_IP_HEADER" - // multipleIPHeaders sets the multiple ip header tag used internally to tell the backend an error occurred when + // tagMultipleIPHeaders sets the multiple ip header tag used internally to tell the backend an error occurred when // retrieving an HTTP request client IP. - multipleIPHeaders = "_dd.multiple-ip-headers" + tagMultipleIPHeaders = "_dd.multiple-ip-headers" + // tagBlockedRequest used to convey whether a request is blocked + tagBlockedRequest = "appsec.blocked" ) var ( @@ -138,7 +140,7 @@ func SetIPTags(span instrumentation.TagSetter, r *http.Request) { for i := range ips { span.SetTag(ext.HTTPRequestHeaders+"."+headers[i], ips[i]) } - span.SetTag(multipleIPHeaders, strings.Join(headers, ",")) + span.SetTag(tagMultipleIPHeaders, strings.Join(headers, ",")) } } diff --git a/internal/appsec/dyngo/instrumentation/httpsec/tags_test.go b/internal/appsec/dyngo/instrumentation/httpsec/tags_test.go index 1dfe92c6fe..dcbcc06a97 100644 --- a/internal/appsec/dyngo/instrumentation/httpsec/tags_test.go +++ b/internal/appsec/dyngo/instrumentation/httpsec/tags_test.go @@ -225,11 +225,11 @@ func TestIPHeaders(t *testing.T) { SetIPTags(&span, &r) if tc.expectedIP.IsValid() { require.Equal(t, tc.expectedIP.String(), span.Tag(ext.HTTPClientIP)) - require.Nil(t, span.Tag(multipleIPHeaders)) + require.Nil(t, span.Tag(tagMultipleIPHeaders)) } else { require.Nil(t, span.Tag(ext.HTTPClientIP)) if tc.multiHeaders != "" { - require.Equal(t, tc.multiHeaders, span.Tag(multipleIPHeaders)) + require.Equal(t, tc.multiHeaders, span.Tag(tagMultipleIPHeaders)) for hdr, ip := range tc.headers { require.Equal(t, ip, span.Tag(ext.HTTPRequestHeaders+"."+hdr)) }