diff --git a/contrib/google.golang.org/grpc/appsec_test.go b/contrib/google.golang.org/grpc/appsec_test.go index a02c551d5a..6afe314109 100644 --- a/contrib/google.golang.org/grpc/appsec_test.go +++ b/contrib/google.golang.org/grpc/appsec_test.go @@ -7,6 +7,7 @@ package grpc import ( "context" + "encoding/json" "fmt" "net" "strings" @@ -24,7 +25,6 @@ import ( ) func TestAppSec(t *testing.T) { - appsec.Start() defer appsec.Stop() if !appsec.Enabled() { t.Skip("appsec disabled") @@ -81,14 +81,16 @@ func TestAppSec(t *testing.T) { require.Equal(t, "passed", res.Message) require.NoError(t, err) - // Send a SQLi attack - err = stream.Send(&FixtureRequest{Name: "-1' and 1=1 union select * from users--"}) - require.NoError(t, err) + for i := 0; i < 5; i++ { // Fire multiple times, each time should result in a detected event + // Send a SQLi attack + err = stream.Send(&FixtureRequest{Name: fmt.Sprintf("-%[1]d' and %[1]d=%[1]d union select * from users--", i)}) + require.NoError(t, err) - // Check that the handler was properly called - res, err = stream.Recv() - require.Equal(t, "passed", res.Message) - require.NoError(t, err) + // Check that the handler was properly called + res, err = stream.Recv() + require.Equal(t, "passed", res.Message) + require.NoError(t, err) + } err = stream.CloseSend() require.NoError(t, err) @@ -96,14 +98,34 @@ func TestAppSec(t *testing.T) { stream.Recv() finished := mt.FinishedSpans() - require.Len(t, finished, 6) + require.Len(t, finished, 14) // The request should have the attack attempts - event, _ := finished[5].Tag("_dd.appsec.json").(string) - require.NotNil(t, event) - require.True(t, strings.Contains(event, "crs-941-110")) // XSS attack attempt - require.True(t, strings.Contains(event, "crs-942-100")) // SQL-injection attack attempt - require.True(t, strings.Contains(event, "ua0-600-55x")) // canary rule attack attempt + event := finished[len(finished)-1].Tag("_dd.appsec.json") + require.NotNil(t, event, "the _dd.appsec.json tag was not found") + + jsonText := event.(string) + type trigger struct { + Rule struct { + ID string `json:"id"` + } `json:"rule"` + } + var parsed struct { + Triggers []trigger `json:"triggers"` + } + err = json.Unmarshal([]byte(jsonText), &parsed) + require.NoError(t, err) + + histogram := map[string]uint8{} + for _, tr := range parsed.Triggers { + histogram[tr.Rule.ID]++ + } + + require.EqualValues(t, 1, histogram["crs-941-110"]) // XSS attack attempt + require.EqualValues(t, 5, histogram["crs-942-270"]) // SQL-injection attack attempt + require.EqualValues(t, 1, histogram["ua0-600-55x"]) // canary rule attack attempt + + require.Len(t, histogram, 3) }) } diff --git a/go.mod b/go.mod index 0c8ab640cc..c5b563f715 100644 --- a/go.mod +++ b/go.mod @@ -5,11 +5,11 @@ go 1.19 require ( cloud.google.com/go/pubsub v1.33.0 github.com/99designs/gqlgen v0.17.36 - github.com/DataDog/appsec-internal-go v1.0.1 + github.com/DataDog/appsec-internal-go v1.0.2 github.com/DataDog/datadog-agent/pkg/obfuscate v0.48.0 github.com/DataDog/datadog-agent/pkg/remoteconfig/state v0.48.1 github.com/DataDog/datadog-go/v5 v5.3.0 - github.com/DataDog/go-libddwaf v1.5.0 + github.com/DataDog/go-libddwaf/v2 v2.1.0 github.com/DataDog/gostackparse v0.7.0 github.com/DataDog/sketches-go v1.4.2 github.com/IBM/sarama v1.40.0 @@ -89,7 +89,7 @@ require ( go.uber.org/atomic v1.11.0 golang.org/x/net v0.17.0 golang.org/x/oauth2 v0.9.0 - golang.org/x/sys v0.13.0 + golang.org/x/sys v0.14.0 golang.org/x/time v0.3.0 golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 google.golang.org/api v0.128.0 @@ -143,7 +143,7 @@ require ( github.com/eapache/go-resiliency v1.4.0 // indirect github.com/eapache/go-xerial-snappy v0.0.0-20230731223053-c322873962e3 // indirect github.com/eapache/queue v1.1.0 // indirect - github.com/ebitengine/purego v0.5.0-alpha.1 // indirect + github.com/ebitengine/purego v0.5.0 // indirect github.com/elastic/elastic-transport-go/v8 v8.1.0 // indirect github.com/fatih/color v1.15.0 // indirect github.com/felixge/httpsnoop v1.0.3 // indirect diff --git a/go.sum b/go.sum index d1886b1572..63db5554e8 100644 --- a/go.sum +++ b/go.sum @@ -624,8 +624,8 @@ github.com/AzureAD/microsoft-authentication-library-for-go v0.5.1/go.mod h1:Vt9s github.com/AzureAD/microsoft-authentication-library-for-go v0.8.1/go.mod h1:4qFor3D/HDsvBME35Xy9rwW9DecL+M2sNw1ybjPtwA0= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= -github.com/DataDog/appsec-internal-go v1.0.1 h1:j60HUtXEQ2uRIm8SsNnLp1Ummx/EU8iV9IFvEYmSdUM= -github.com/DataDog/appsec-internal-go v1.0.1/go.mod h1:+Y+4klVWKPOnZx6XESG7QHydOaUGEXyH2j/vSg9JiNM= +github.com/DataDog/appsec-internal-go v1.0.2 h1:Z+YWPlkQN+324zIk+BzKlPA1/6guKgGmYbON1/xU7gM= +github.com/DataDog/appsec-internal-go v1.0.2/go.mod h1:+Y+4klVWKPOnZx6XESG7QHydOaUGEXyH2j/vSg9JiNM= github.com/DataDog/datadog-agent/pkg/obfuscate v0.48.0 h1:bUMSNsw1iofWiju9yc1f+kBd33E3hMJtq9GuU602Iy8= github.com/DataDog/datadog-agent/pkg/obfuscate v0.48.0/go.mod h1:HzySONXnAgSmIQfL6gOv9hWprKJkx8CicuXuUbmgWfo= github.com/DataDog/datadog-agent/pkg/remoteconfig/state v0.48.1 h1:5nE6N3JSs2IG3xzMthNFhXfOaXlrsdgqmJ73lndFf8c= @@ -633,8 +633,8 @@ github.com/DataDog/datadog-agent/pkg/remoteconfig/state v0.48.1/go.mod h1:Vc+snp github.com/DataDog/datadog-go v3.2.0+incompatible/go.mod h1:LButxg5PwREeZtORoXG3tL4fMGNddJ+vMq1mwgfaqoQ= github.com/DataDog/datadog-go/v5 v5.3.0 h1:2q2qjFOb3RwAZNU+ez27ZVDwErJv5/VpbBPprz7Z+s8= github.com/DataDog/datadog-go/v5 v5.3.0/go.mod h1:XRDJk1pTc00gm+ZDiBKsjh7oOOtJfYfglVCmFb8C2+Q= -github.com/DataDog/go-libddwaf v1.5.0 h1:lrHP3VrEriy1M5uQuaOcKphf5GU40mBhihMAp6Ik55c= -github.com/DataDog/go-libddwaf v1.5.0/go.mod h1:Fpnmoc2k53h6desQrH1P0/gR52CUzkLNFugE5zWwUBQ= +github.com/DataDog/go-libddwaf/v2 v2.1.0 h1:ODQibem9zg7AC6LbU222gZwuobhnnFz5okJjh+4W3v4= +github.com/DataDog/go-libddwaf/v2 v2.1.0/go.mod h1:X/Kc+PpP1FvvfMJvsmh/YZwGHSnhI40UkKPnDXfdTl4= github.com/DataDog/go-tuf v1.0.2-0.5.2 h1:EeZr937eKAWPxJ26IykAdWA4A0jQXJgkhUjqEI/w7+I= github.com/DataDog/go-tuf v1.0.2-0.5.2/go.mod h1:zBcq6f654iVqmkk8n2Cx81E1JnNTMOAx1UEO/wZR+P0= github.com/DataDog/gostackparse v0.7.0 h1:i7dLkXHvYzHV308hnkvVGDL3BR4FWl7IsXNPz/IGQh4= @@ -1063,8 +1063,8 @@ github.com/eapache/go-xerial-snappy v0.0.0-20230731223053-c322873962e3 h1:Oy0F4A github.com/eapache/go-xerial-snappy v0.0.0-20230731223053-c322873962e3/go.mod h1:YvSRo5mw33fLEx1+DlK6L2VV43tJt5Eyel9n9XBcR+0= github.com/eapache/queue v1.1.0 h1:YOEu7KNc61ntiQlcEeUIoDTJ2o8mQznoNvUhiigpIqc= github.com/eapache/queue v1.1.0/go.mod h1:6eCeP0CKFpHLu8blIFXhExK/dRa7WDZfr6jVFPTqq+I= -github.com/ebitengine/purego v0.5.0-alpha.1 h1:0gVgWGb8GjKYs7cufvfNSleJAD00m2xWC26FMwOjNrw= -github.com/ebitengine/purego v0.5.0-alpha.1/go.mod h1:ah1In8AOtksoNK6yk5z1HTJeUkC1Ez4Wk2idgGslMwQ= +github.com/ebitengine/purego v0.5.0 h1:JrMGKfRIAM4/QVKaesIIT7m/UVjTj5GYhRSQYwfVdpo= +github.com/ebitengine/purego v0.5.0/go.mod h1:ah1In8AOtksoNK6yk5z1HTJeUkC1Ez4Wk2idgGslMwQ= github.com/elastic/elastic-transport-go/v8 v8.1.0 h1:NeqEz1ty4RQz+TVbUrpSU7pZ48XkzGWQj02k5koahIE= github.com/elastic/elastic-transport-go/v8 v8.1.0/go.mod h1:87Tcz8IVNe6rVSLdBux1o/PEItLtyabHU3naC7IoqKI= github.com/elastic/go-elasticsearch/v6 v6.8.5 h1:U2HtkBseC1FNBmDr0TR2tKltL6FxoY+niDAlj5M8TK8= @@ -2534,8 +2534,8 @@ golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.4.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= -golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.14.0 h1:Vz7Qs629MkJkGyHxUlRHizWJRG2j8fbQKjELVSNhy7Q= +golang.org/x/sys v0.14.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= diff --git a/internal/appsec/appsec.go b/internal/appsec/appsec.go index 6ac60bb215..c1e858fa66 100644 --- a/internal/appsec/appsec.go +++ b/internal/appsec/appsec.go @@ -15,7 +15,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo" "gopkg.in/DataDog/dd-trace-go.v1/internal/log" - waf "github.com/DataDog/go-libddwaf" + waf "github.com/DataDog/go-libddwaf/v2" ) // Enabled returns true when AppSec is up and running. Meaning that the appsec build tag is enabled, the env var diff --git a/internal/appsec/appsec_test.go b/internal/appsec/appsec_test.go index 3da30b125d..2a57842d89 100644 --- a/internal/appsec/appsec_test.go +++ b/internal/appsec/appsec_test.go @@ -16,7 +16,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec" - waf "github.com/DataDog/go-libddwaf" + waf "github.com/DataDog/go-libddwaf/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) diff --git a/internal/appsec/dyngo/instrumentation/common.go b/internal/appsec/dyngo/instrumentation/common.go index 6784deff57..c97ad7e6fa 100644 --- a/internal/appsec/dyngo/instrumentation/common.go +++ b/internal/appsec/dyngo/instrumentation/common.go @@ -33,7 +33,7 @@ type ( // used by composition in an Operation to allow said operation to handle security events addition/retrieval. // See httpsec/http.go and grpcsec/grpc.go. SecurityEventsHolder struct { - events []json.RawMessage + events []any mu sync.RWMutex } // ContextKey is used as a key to store operations in the request's context (gRPC/HTTP) @@ -59,14 +59,14 @@ func (m *TagsHolder) Tags() map[string]interface{} { // AddSecurityEvents adds the security events to the collected events list. // Thread safe. -func (s *SecurityEventsHolder) AddSecurityEvents(events ...json.RawMessage) { +func (s *SecurityEventsHolder) AddSecurityEvents(events []any) { s.mu.Lock() defer s.mu.Unlock() s.events = append(s.events, events...) } // Events returns the list of stored events. -func (s *SecurityEventsHolder) Events() []json.RawMessage { +func (s *SecurityEventsHolder) Events() []any { s.mu.RLock() defer s.mu.RUnlock() return s.events @@ -102,7 +102,7 @@ func SetAppSecEnabledTags(span TagSetter) { } // SetEventSpanTags sets the security event span tags into the service entry span. -func SetEventSpanTags(span TagSetter, events []json.RawMessage) error { +func SetEventSpanTags(span TagSetter, events []any) error { // Set the appsec event span tag val, err := makeEventTagValue(events) if err != nil { @@ -123,42 +123,8 @@ 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. -// -// TODO(Julio-Guerra): avoid serializing the json in the request hot path -func makeEventTagValue(events []json.RawMessage) (json.RawMessage, error) { - var v interface{} - if l := len(events); l == 1 { - // eventTag is the structure to use in the `_dd.appsec.json` span tag. - // In this case of 1 event, it already is an array as expected. - type eventTag struct { - Triggers json.RawMessage `json:"triggers"` - } - v = eventTag{Triggers: events[0]} - } else { - // eventTag is the structure to use in the `_dd.appsec.json` span tag. - // With more than one event, we need to concatenate the arrays together - // (ie. convert [][]json.RawMessage into []json.RawMessage). - type eventTag struct { - Triggers []json.RawMessage `json:"triggers"` - } - concatenated := make([]json.RawMessage, 0, l) // at least len(events) - for _, event := range events { - // Unmarshal the top level array - var tmp []json.RawMessage - if err := json.Unmarshal(event, &tmp); err != nil { - return nil, fmt.Errorf("unexpected error while unserializing the appsec event `%s`: %v", string(event), err) - } - concatenated = append(concatenated, tmp...) - } - v = eventTag{Triggers: concatenated} - } - - tag, err := json.Marshal(v) +func makeEventTagValue(events []any) (json.RawMessage, error) { + tag, err := json.Marshal(map[string][]any{"triggers": events}) if err != nil { return nil, fmt.Errorf("unexpected error while serializing the appsec event span tag: %v", err) } diff --git a/internal/appsec/dyngo/instrumentation/grpcsec/grpc.go b/internal/appsec/dyngo/instrumentation/grpcsec/grpc.go index f95e9418ec..f29a9ef130 100644 --- a/internal/appsec/dyngo/instrumentation/grpcsec/grpc.go +++ b/internal/appsec/dyngo/instrumentation/grpcsec/grpc.go @@ -11,7 +11,6 @@ package grpcsec import ( "context" - "encoding/json" "reflect" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo" @@ -117,7 +116,7 @@ func StartHandlerOperation(ctx context.Context, args HandlerOperationArgs, paren // Finish the gRPC handler operation, along with the given results, and emit a // finish event up in the operation stack. -func (op *HandlerOperation) Finish(res HandlerOperationRes) []json.RawMessage { +func (op *HandlerOperation) Finish(res HandlerOperationRes) []any { dyngo.FinishOperation(op, res) return op.Events() } diff --git a/internal/appsec/dyngo/instrumentation/grpcsec/grpc_test.go b/internal/appsec/dyngo/instrumentation/grpcsec/grpc_test.go index 099b556bac..b6868ee907 100644 --- a/internal/appsec/dyngo/instrumentation/grpcsec/grpc_test.go +++ b/internal/appsec/dyngo/instrumentation/grpcsec/grpc_test.go @@ -7,7 +7,6 @@ package grpcsec_test import ( "context" - "encoding/json" "fmt" "testing" @@ -49,7 +48,7 @@ func TestUsage(t *testing.T) { require.Equal(t, expectedMessage, res.Message) recvFinished++ - handlerOp.AddSecurityEvents(json.RawMessage(expectedMessage)) + handlerOp.AddSecurityEvents([]any{expectedMessage}) })) })) @@ -69,7 +68,7 @@ func TestUsage(t *testing.T) { require.Len(t, secEvents, expectedRecvOperation) for i, e := range secEvents { - require.Equal(t, fmt.Sprintf(expectedMessageFormat, i+1), string(e)) + require.Equal(t, fmt.Sprintf(expectedMessageFormat, i+1), e) } } } diff --git a/internal/appsec/dyngo/instrumentation/grpcsec/tags.go b/internal/appsec/dyngo/instrumentation/grpcsec/tags.go index f014610608..306b86e250 100644 --- a/internal/appsec/dyngo/instrumentation/grpcsec/tags.go +++ b/internal/appsec/dyngo/instrumentation/grpcsec/tags.go @@ -6,8 +6,6 @@ package grpcsec import ( - "encoding/json" - "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/httpsec" @@ -15,13 +13,13 @@ import ( ) // SetSecurityEventsTags sets the AppSec events span tags. -func SetSecurityEventsTags(span ddtrace.Span, events []json.RawMessage) { +func SetSecurityEventsTags(span ddtrace.Span, events []any) { if err := setSecurityEventsTags(span, events); err != nil { log.Error("appsec: unexpected error while creating the appsec events tags: %v", err) } } -func setSecurityEventsTags(span ddtrace.Span, events []json.RawMessage) error { +func setSecurityEventsTags(span ddtrace.Span, events []any) error { if events == nil { return nil } diff --git a/internal/appsec/dyngo/instrumentation/grpcsec/tags_test.go b/internal/appsec/dyngo/instrumentation/grpcsec/tags_test.go index 9dda447e4c..534394faa2 100644 --- a/internal/appsec/dyngo/instrumentation/grpcsec/tags_test.go +++ b/internal/appsec/dyngo/instrumentation/grpcsec/tags_test.go @@ -6,7 +6,6 @@ package grpcsec import ( - "encoding/json" "fmt" "net" "testing" @@ -22,7 +21,7 @@ import ( func TestTags(t *testing.T) { for _, eventCase := range []struct { name string - events []json.RawMessage + events []any expectedTag string expectedError bool }{ @@ -32,28 +31,13 @@ func TestTags(t *testing.T) { }, { name: "one-event", - events: []json.RawMessage{json.RawMessage(`["one","two"]`)}, - expectedTag: `{"triggers":["one","two"]}`, - }, - { - name: "one-event-with-json-error", - events: []json.RawMessage{json.RawMessage(`["one",two"]`)}, - expectedError: true, + events: []any{"one"}, + expectedTag: `{"triggers":["one"]}`, }, { name: "two-events", - events: []json.RawMessage{json.RawMessage(`["one","two"]`), json.RawMessage(`["three","four"]`)}, - expectedTag: `{"triggers":["one","two","three","four"]}`, - }, - { - name: "two-events-with-json-error", - events: []json.RawMessage{json.RawMessage(`["one","two"]`), json.RawMessage(`["three,"four"]`)}, - expectedError: true, - }, - { - name: "three-events-with-json-error", - events: []json.RawMessage{json.RawMessage(`["one","two"]`), json.RawMessage(`["three","four"]`), json.RawMessage(`"five"`)}, - expectedError: true, + events: []any{"one", "two"}, + expectedTag: `{"triggers":["one","two"]}`, }, } { eventCase := eventCase diff --git a/internal/appsec/dyngo/instrumentation/httpsec/http.go b/internal/appsec/dyngo/instrumentation/httpsec/http.go index bcb4fb4442..cd3b3184c0 100644 --- a/internal/appsec/dyngo/instrumentation/httpsec/http.go +++ b/internal/appsec/dyngo/instrumentation/httpsec/http.go @@ -14,7 +14,6 @@ import ( "context" // Blank import needed to use embed for the default blocked response payloads _ "embed" - "encoding/json" "net/http" "reflect" "strings" @@ -263,7 +262,7 @@ func fromContext(ctx context.Context) *Operation { // Finish the HTTP handler operation, along with the given results and emits a // finish event up in the operation stack. -func (op *Operation) Finish(res HandlerOperationRes) []json.RawMessage { +func (op *Operation) Finish(res HandlerOperationRes) []any { dyngo.FinishOperation(op, res) return op.Events() } diff --git a/internal/appsec/dyngo/instrumentation/httpsec/tags.go b/internal/appsec/dyngo/instrumentation/httpsec/tags.go index 1fa0349ebb..e70806a75f 100644 --- a/internal/appsec/dyngo/instrumentation/httpsec/tags.go +++ b/internal/appsec/dyngo/instrumentation/httpsec/tags.go @@ -6,7 +6,6 @@ package httpsec import ( - "encoding/json" "os" "sort" "strings" @@ -73,13 +72,13 @@ func init() { } // SetSecurityEventsTags sets the AppSec-specific span tags when a security event occurred into the service entry span. -func SetSecurityEventsTags(span instrumentation.TagSetter, events []json.RawMessage) { +func SetSecurityEventsTags(span instrumentation.TagSetter, events []any) { if err := instrumentation.SetEventSpanTags(span, events); err != nil { log.Error("appsec: unexpected error while creating the appsec events tags: %v", err) } } -func setSecurityEventsTags(span instrumentation.TagSetter, events []json.RawMessage) error { +func setSecurityEventsTags(span instrumentation.TagSetter, events []any) error { if len(events) == 0 { return nil } diff --git a/internal/appsec/dyngo/instrumentation/httpsec/tags_test.go b/internal/appsec/dyngo/instrumentation/httpsec/tags_test.go index 30b90e697d..51368c51b6 100644 --- a/internal/appsec/dyngo/instrumentation/httpsec/tags_test.go +++ b/internal/appsec/dyngo/instrumentation/httpsec/tags_test.go @@ -6,7 +6,6 @@ package httpsec import ( - "encoding/json" "fmt" "testing" @@ -18,7 +17,7 @@ import ( func TestTags(t *testing.T) { for _, eventCase := range []struct { name string - events []json.RawMessage + events []any expectedTag string expectedError bool }{ @@ -28,28 +27,13 @@ func TestTags(t *testing.T) { }, { name: "one-event", - events: []json.RawMessage{json.RawMessage(`["one","two"]`)}, - expectedTag: `{"triggers":["one","two"]}`, - }, - { - name: "one-event-with-json-error", - events: []json.RawMessage{json.RawMessage(`["one",two"]`)}, - expectedError: true, + events: []any{"one"}, + expectedTag: `{"triggers":["one"]}`, }, { name: "two-events", - events: []json.RawMessage{json.RawMessage(`["one","two"]`), json.RawMessage(`["three","four"]`)}, - expectedTag: `{"triggers":["one","two","three","four"]}`, - }, - { - name: "two-events-with-json-error", - events: []json.RawMessage{json.RawMessage(`["one","two"]`), json.RawMessage(`["three,"four"]`)}, - expectedError: true, - }, - { - name: "three-events-with-json-error", - events: []json.RawMessage{json.RawMessage(`["one","two"]`), json.RawMessage(`["three","four"]`), json.RawMessage(`"five"`)}, - expectedError: true, + events: []any{"one", "two"}, + expectedTag: `{"triggers":["one","two"]}`, }, } { eventCase := eventCase diff --git a/internal/appsec/remoteconfig.go b/internal/appsec/remoteconfig.go index 9f433e0330..b35ac26132 100644 --- a/internal/appsec/remoteconfig.go +++ b/internal/appsec/remoteconfig.go @@ -164,10 +164,11 @@ func (a *appsec) onRCRulesUpdate(updates map[string]remoteconfig.ProductUpdate) for k := range statuses { statuses[k] = genApplyStatus(true, err) } - } else { - // Replace the rulesManager with the new one holding the new state - a.cfg.rulesManager = r + return statuses } + // Replace the rulesManager with the new one holding the new state + a.cfg.rulesManager = r + return statuses } diff --git a/internal/appsec/remoteconfig_test.go b/internal/appsec/remoteconfig_test.go index 3da664430a..0c9f720c32 100644 --- a/internal/appsec/remoteconfig_test.go +++ b/internal/appsec/remoteconfig_test.go @@ -10,7 +10,6 @@ package appsec import ( "encoding/json" - "errors" "os" "reflect" "sort" @@ -20,7 +19,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/remoteconfig" rc "github.com/DataDog/datadog-agent/pkg/remoteconfig/state" - waf "github.com/DataDog/go-libddwaf" + waf "github.com/DataDog/go-libddwaf/v2" "github.com/stretchr/testify/require" ) @@ -633,7 +632,7 @@ func TestOnRCUpdateStatuses(t *testing.T) { { name: "single/error", updates: craftRCUpdates(map[string]rulesFragment{"invalid": invalidOverrides}), - expected: map[string]rc.ApplyStatus{"invalid": genApplyStatus(true, errors.New("could not instantiate the WAF"))}, + expected: map[string]rc.ApplyStatus{"invalid": ackStatus}, // Success, as there exists at least 1 usable rule in the whole set }, { name: "multiple/ack", @@ -644,8 +643,8 @@ func TestOnRCUpdateStatuses(t *testing.T) { name: "multiple/single-error", updates: craftRCUpdates(map[string]rulesFragment{"overrides": overrides, "invalid": invalidOverrides}), expected: map[string]rc.ApplyStatus{ - "overrides": genApplyStatus(true, errors.New("could not instantiate the WAF")), - "invalid": genApplyStatus(true, errors.New("could not instantiate the WAF")), + "overrides": ackStatus, // Success, as there exists at least 1 usable rule in the whole set + "invalid": ackStatus, // Success, as there exists at least 1 usable rule in the whole set }, }, { @@ -670,7 +669,7 @@ func TestOnRCUpdateStatuses(t *testing.T) { } } else { require.Len(t, statuses, len(tc.expected)) - require.True(t, reflect.DeepEqual(tc.expected, statuses)) + require.True(t, reflect.DeepEqual(tc.expected, statuses), "expected: %#v\nactual: %#v", tc.expected, statuses) } }) } @@ -705,9 +704,9 @@ func TestWafRCUpdate(t *testing.T) { serverRequestPathParamsAddr: "/rfiinc.txt", } // Make sure the rule matches as expected - matches, actions := runWAF(wafCtx, values, cfg.wafTimeout) - require.Contains(t, string(matches), "crs-913-120") - require.Empty(t, actions) + result := runWAF(wafCtx, waf.RunAddressData{Persistent: values}, cfg.wafTimeout) + require.Contains(t, jsonString(t, result.Events), "crs-913-120") + require.Empty(t, result.Actions) // Simulate an RC update that disables the rule statuses, err := combineRCRulesUpdates(cfg.rulesManager, craftRCUpdates(map[string]rulesFragment{"override": override})) for _, status := range statuses { @@ -720,8 +719,14 @@ func TestWafRCUpdate(t *testing.T) { newWafCtx := waf.NewContext(newWafHandle) defer newWafCtx.Close() // Make sure the rule returns a blocking action when matching - matches, actions = runWAF(newWafCtx, values, cfg.wafTimeout) - require.Contains(t, string(matches), "crs-913-120") - require.Contains(t, actions, "block") + result = runWAF(newWafCtx, waf.RunAddressData{Persistent: values}, cfg.wafTimeout) + require.Contains(t, jsonString(t, result.Events), "crs-913-120") + require.Contains(t, result.Actions, "block") }) } + +func jsonString(t *testing.T, v any) string { + bytes, err := json.Marshal(v) + require.NoError(t, err) + return string(bytes) +} diff --git a/internal/appsec/rule_test.go b/internal/appsec/rule_test.go index 24be0288b1..b1c08cc5a6 100644 --- a/internal/appsec/rule_test.go +++ b/internal/appsec/rule_test.go @@ -13,7 +13,7 @@ import ( "testing" rules "github.com/DataDog/appsec-internal-go/appsec" - waf "github.com/DataDog/go-libddwaf" + waf "github.com/DataDog/go-libddwaf/v2" "github.com/stretchr/testify/require" ) diff --git a/internal/appsec/waf.go b/internal/appsec/waf.go index fa1fb50758..04c606e41b 100644 --- a/internal/appsec/waf.go +++ b/internal/appsec/waf.go @@ -24,7 +24,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/log" "gopkg.in/DataDog/dd-trace-go.v1/internal/samplernames" - waf "github.com/DataDog/go-libddwaf" + waf "github.com/DataDog/go-libddwaf/v2" "go.uber.org/atomic" ) @@ -158,6 +158,8 @@ func newWAFEventListeners(waf *wafHandle, cfg *Config, l Limiter) (listeners []d // newWAFEventListener returns the WAF event listener to register in order to enable it. func newHTTPWAFEventListener(handle *wafHandle, addresses map[string]struct{}, timeout time.Duration, limiter Limiter) dyngo.EventListener { var monitorRulesOnce sync.Once // per instantiation + // TODO: port wafDiags to telemetry metrics and logs instead of span tags (ultimately removing them from here hopefully) + wafDiags := handle.Diagnostics() return httpsec.OnHandlerOperationStart(func(op *httpsec.Operation, args httpsec.HandlerOperationArgs) { wafCtx := waf.NewContext(handle.Handle) @@ -171,16 +173,16 @@ func newHTTPWAFEventListener(handle *wafHandle, addresses map[string]struct{}, t // see if the associated user should be blocked. Since we don't control the execution flow in this case // (SetUser is SDK), we delegate the responsibility of interrupting the handler to the user. op.On(sharedsec.OnUserIDOperationStart(func(operation *sharedsec.UserIDOperation, args sharedsec.UserIDOperationArgs) { - matches, actionIds := runWAF(wafCtx, map[string]interface{}{userIDAddr: args.UserID}, timeout) - if len(matches) > 0 { - processHTTPSDKAction(operation, handle.actions, actionIds) - addSecurityEvents(op, limiter, matches) + wafResult := runWAF(wafCtx, waf.RunAddressData{Persistent: map[string]any{userIDAddr: args.UserID}}, timeout) + if wafResult.HasActions() || wafResult.HasEvents() { + processHTTPSDKAction(operation, handle.actions, wafResult.Actions) + addSecurityEvents(op, limiter, wafResult.Events) log.Debug("appsec: WAF detected a suspicious user: %s", args.UserID) } })) } - values := map[string]interface{}{} + values := make(map[string]any, 7) for addr := range addresses { switch addr { case httpClientIPAddr: @@ -210,10 +212,10 @@ func newHTTPWAFEventListener(handle *wafHandle, addresses map[string]struct{}, t } } - matches, actionIds := runWAF(wafCtx, values, timeout) - if len(matches) > 0 { - interrupt := processActions(op, handle.actions, actionIds) - addSecurityEvents(op, limiter, matches) + wafResult := runWAF(wafCtx, waf.RunAddressData{Persistent: values}, timeout) + if wafResult.HasActions() || wafResult.HasEvents() { + interrupt := processActions(op, handle.actions, wafResult.Actions) + addSecurityEvents(op, limiter, wafResult.Events) log.Debug("appsec: WAF detected an attack before executing the request") if interrupt { wafCtx.Close() @@ -223,10 +225,10 @@ func newHTTPWAFEventListener(handle *wafHandle, addresses map[string]struct{}, t if _, ok := addresses[serverRequestBodyAddr]; ok { op.On(httpsec.OnSDKBodyOperationStart(func(sdkBodyOp *httpsec.SDKBodyOperation, args httpsec.SDKBodyOperationArgs) { - matches, actionIds := runWAF(wafCtx, map[string]interface{}{serverRequestBodyAddr: args.Body}, timeout) - if len(matches) > 0 { - processHTTPSDKAction(sdkBodyOp, handle.actions, actionIds) - addSecurityEvents(op, limiter, matches) + wafResult := runWAF(wafCtx, waf.RunAddressData{Persistent: map[string]any{serverRequestBodyAddr: args.Body}}, timeout) + if wafResult.HasActions() || wafResult.HasEvents() { + processHTTPSDKAction(sdkBodyOp, handle.actions, wafResult.Actions) + addSecurityEvents(op, limiter, wafResult.Events) log.Debug("appsec: WAF detected a suspicious request body") } })) @@ -235,32 +237,31 @@ func newHTTPWAFEventListener(handle *wafHandle, addresses map[string]struct{}, t op.On(httpsec.OnHandlerOperationFinish(func(op *httpsec.Operation, res httpsec.HandlerOperationRes) { defer wafCtx.Close() - values := make(map[string]interface{}, 1) + values := map[string]any{} if _, ok := addresses[serverResponseStatusAddr]; ok { - values[serverResponseStatusAddr] = res.Status + // serverResponseStatusAddr is a string address, so we must format the status code... + values[serverResponseStatusAddr] = fmt.Sprintf("%d", res.Status) } // Run the WAF, ignoring the returned actions - if any - since blocking after the request handler's // response is not supported at the moment. - matches, _ := runWAF(wafCtx, values, timeout) + wafResult := runWAF(wafCtx, waf.RunAddressData{Persistent: values}, timeout) // Add WAF metrics. - rInfo := handle.RulesetInfo() overallRuntimeNs, internalRuntimeNs := wafCtx.TotalRuntime() - addWAFMonitoringTags(op, rInfo.Version, overallRuntimeNs, internalRuntimeNs, wafCtx.TotalTimeouts()) + addWAFMonitoringTags(op, wafDiags.Version, overallRuntimeNs, internalRuntimeNs, wafCtx.TotalTimeouts()) // Add the following metrics once per instantiation of a WAF handle monitorRulesOnce.Do(func() { - addRulesMonitoringTags(op, rInfo) + addRulesMonitoringTags(op, &wafDiags) op.AddTag(ext.ManualKeep, samplernames.AppSec) }) // Log the attacks if any - if len(matches) == 0 { - return + if wafResult.HasEvents() { + log.Debug("appsec: attack detected by the waf") + addSecurityEvents(op, limiter, wafResult.Events) } - log.Debug("appsec: attack detected by the waf") - addSecurityEvents(op, limiter, matches) })) }) } @@ -269,19 +270,17 @@ func newHTTPWAFEventListener(handle *wafHandle, addresses map[string]struct{}, t // to enable it. func newGRPCWAFEventListener(handle *wafHandle, addresses map[string]struct{}, timeout time.Duration, limiter Limiter) dyngo.EventListener { var monitorRulesOnce sync.Once // per instantiation + wafDiags := handle.Diagnostics() return grpcsec.OnHandlerOperationStart(func(op *grpcsec.HandlerOperation, handlerArgs grpcsec.HandlerOperationArgs) { // Limit the maximum number of security events, as a streaming RPC could // receive unlimited number of messages where we could find security events const maxWAFEventsPerRequest = 10 var ( - nbEvents atomic.Uint32 - logOnce sync.Once // per request - overallRuntimeNs atomic.Uint64 - internalRuntimeNs atomic.Uint64 - nbTimeouts atomic.Uint64 + nbEvents atomic.Uint32 + logOnce sync.Once // per request - events []json.RawMessage + events []any mu sync.Mutex // events mutex ) @@ -295,37 +294,37 @@ func newGRPCWAFEventListener(handle *wafHandle, addresses map[string]struct{}, t // see if the associated user should be blocked. Since we don't control the execution flow in this case // (SetUser is SDK), we delegate the responsibility of interrupting the handler to the user. op.On(sharedsec.OnUserIDOperationStart(func(userIDOp *sharedsec.UserIDOperation, args sharedsec.UserIDOperationArgs) { - values := map[string]interface{}{} + values := map[string]any{} for addr := range addresses { if addr == userIDAddr { values[userIDAddr] = args.UserID } } - matches, actionIds := runWAF(wafCtx, values, timeout) - if len(matches) > 0 { - for _, id := range actionIds { + wafResult := runWAF(wafCtx, waf.RunAddressData{Persistent: values}, timeout) + if wafResult.HasActions() || wafResult.HasEvents() { + for _, id := range wafResult.Actions { if a, ok := handle.actions[id]; ok && a.Blocking() { code, err := a.GRPC()(map[string][]string{}) userIDOp.EmitData(grpcsec.NewMonitoringError(err.Error(), code)) } } - addSecurityEvents(op, limiter, matches) + addSecurityEvents(op, limiter, wafResult.Events) log.Debug("appsec: WAF detected an authenticated user attack: %s", args.UserID) } })) // The same address is used for gRPC and http when it comes to client ip - values := map[string]interface{}{} + values := map[string]any{} for addr := range addresses { if addr == httpClientIPAddr && handlerArgs.ClientIP.IsValid() { values[httpClientIPAddr] = handlerArgs.ClientIP.String() } } - matches, actionIds := runWAF(wafCtx, values, timeout) - if len(matches) > 0 { - interrupt := processActions(op, handle.actions, actionIds) - addSecurityEvents(op, limiter, matches) + wafResult := runWAF(wafCtx, waf.RunAddressData{Persistent: values}, timeout) + if wafResult.HasActions() || wafResult.HasEvents() { + interrupt := processActions(op, handle.actions, wafResult.Actions) + addSecurityEvents(op, limiter, wafResult.Events) log.Debug("appsec: WAF detected an attack before executing the request") if interrupt { wafCtx.Close() @@ -340,77 +339,54 @@ func newGRPCWAFEventListener(handle *wafHandle, addresses map[string]struct{}, t }) return } - // The current workaround of the WAF context limitations is to - // simply instantiate and release the WAF context for the operation - // lifetime so that: - // 1. We avoid growing the memory usage of the context every time - // a grpc.server.request.message value is added to it during - // the RPC lifetime. - // 2. We avoid the limitation of 1 event per attack type. - // TODO(Julio-Guerra): a future libddwaf API should solve this out. - wafCtx := waf.NewContext(handle.Handle) - if wafCtx == nil { - // The WAF event listener got concurrently released - return - } - defer wafCtx.Close() + // Run the WAF on the rule addresses available in the args // Note that we don't check if the address is present in the rules // as we only support one at the moment, so this callback cannot be // set when the address is not present. - values := map[string]interface{}{grpcServerRequestMessage: res.Message} + values := waf.RunAddressData{ + Ephemeral: map[string]any{grpcServerRequestMessage: res.Message}, + } if md := handlerArgs.Metadata; len(md) > 0 { - values[grpcServerRequestMetadata] = md + values.Persistent = map[string]any{grpcServerRequestMetadata: md} } // Run the WAF, ignoring the returned actions - if any - since blocking after the request handler's // response is not supported at the moment. - event, _ := runWAF(wafCtx, values, timeout) - - // WAF run durations are WAF context bound. As of now we need to keep track of those externally since - // we use a new WAF context for each callback. When we are able to re-use the same WAF context across - // callbacks, we can get rid of these variables and simply use the WAF bindings in OnHandlerOperationFinish. - overall, internal := wafCtx.TotalRuntime() - overallRuntimeNs.Add(overall) - internalRuntimeNs.Add(internal) - nbTimeouts.Add(wafCtx.TotalTimeouts()) - - if len(event) == 0 { - return + wafResult := runWAF(wafCtx, values, timeout) + + if wafResult.HasEvents() { + log.Debug("appsec: attack detected by the grpc waf") + nbEvents.Inc() + mu.Lock() + defer mu.Unlock() + events = append(events, wafResult.Events...) } - log.Debug("appsec: attack detected by the grpc waf") - nbEvents.Inc() - mu.Lock() - events = append(events, event) - mu.Unlock() })) op.On(grpcsec.OnHandlerOperationFinish(func(op *grpcsec.HandlerOperation, _ grpcsec.HandlerOperationRes) { defer wafCtx.Close() - rInfo := handle.RulesetInfo() - addWAFMonitoringTags(op, rInfo.Version, overallRuntimeNs.Load(), internalRuntimeNs.Load(), nbTimeouts.Load()) + overallRuntimeNs, internalRuntimeNs := wafCtx.TotalRuntime() + addWAFMonitoringTags(op, wafDiags.Version, overallRuntimeNs, internalRuntimeNs, wafCtx.TotalTimeouts()) // Log the following metrics once per instantiation of a WAF handle monitorRulesOnce.Do(func() { - addRulesMonitoringTags(op, rInfo) + addRulesMonitoringTags(op, &wafDiags) op.AddTag(ext.ManualKeep, samplernames.AppSec) }) - addSecurityEvents(op, limiter, events...) + addSecurityEvents(op, limiter, events) })) }) } -func runWAF(wafCtx *waf.Context, values map[string]interface{}, timeout time.Duration) ([]byte, []string) { - matches, actions, err := wafCtx.Run(values, timeout) - if err != nil { - if err == waf.ErrTimeout { - log.Debug("appsec: waf timeout value of %s reached", timeout) - } else { - log.Error("appsec: unexpected waf error: %v", err) - return nil, nil - } +func runWAF(wafCtx *waf.Context, values waf.RunAddressData, timeout time.Duration) waf.Result { + result, err := wafCtx.Run(values, timeout) + if err == waf.ErrTimeout { + log.Debug("appsec: waf timeout value of %s reached", timeout) + } else if err != nil { + log.Error("appsec: unexpected waf error: %v", err) } - return matches, actions + return result } // HTTP rule addresses currently supported by the WAF @@ -487,11 +463,16 @@ func supportedAddresses(ruleAddresses []string) (supportedHTTP, supportedGRPC ma } type tagsHolder interface { - AddTag(string, interface{}) + AddTag(string, any) } // Add the tags related to security rules monitoring -func addRulesMonitoringTags(th tagsHolder, rInfo waf.RulesetInfo) { +func addRulesMonitoringTags(th tagsHolder, wafDiags *waf.Diagnostics) { + rInfo := wafDiags.Rules + if rInfo == nil { + return + } + if len(rInfo.Errors) == 0 { rInfo.Errors = nil } @@ -500,8 +481,8 @@ func addRulesMonitoringTags(th tagsHolder, rInfo waf.RulesetInfo) { log.Error("appsec: could not marshal the waf ruleset info errors to json") } th.AddTag(eventRulesErrorsTag, string(rulesetErrors)) // avoid the tracer's call to fmt.Sprintf on the value - th.AddTag(eventRulesLoadedTag, float64(rInfo.Loaded)) - th.AddTag(eventRulesFailedTag, float64(rInfo.Failed)) + th.AddTag(eventRulesLoadedTag, float64(len(rInfo.Loaded))) + th.AddTag(eventRulesFailedTag, float64(len(rInfo.Failed))) th.AddTag(wafVersionTag, waf.Version()) } @@ -515,13 +496,13 @@ func addWAFMonitoringTags(th tagsHolder, rulesVersion string, overallRuntimeNs, } type securityEventsAdder interface { - AddSecurityEvents(events ...json.RawMessage) + AddSecurityEvents(events []any) } // Helper function to add sec events to an operation taking into account the rate limiter. -func addSecurityEvents(op securityEventsAdder, limiter Limiter, matches ...json.RawMessage) { +func addSecurityEvents(op securityEventsAdder, limiter Limiter, matches []any) { if len(matches) > 0 && limiter.Allow() { - op.AddSecurityEvents(matches...) + op.AddSecurityEvents(matches) } } diff --git a/internal/appsec/waf_unit_test.go b/internal/appsec/waf_unit_test.go index f7f041226c..b96fc54a9c 100644 --- a/internal/appsec/waf_unit_test.go +++ b/internal/appsec/waf_unit_test.go @@ -13,21 +13,23 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo/instrumentation" - waf "github.com/DataDog/go-libddwaf" + waf "github.com/DataDog/go-libddwaf/v2" "github.com/stretchr/testify/require" ) // Test that internal functions used to set span tags use the correct types func TestTagsTypes(t *testing.T) { th := instrumentation.NewTagsHolder() - rInfo := waf.RulesetInfo{ + wafDiags := waf.Diagnostics{ Version: "1.3.0", - Loaded: 10, - Failed: 1, - Errors: map[string][]string{"test": {"1", "2"}}, + Rules: &waf.DiagnosticEntry{ + Loaded: []string{"0", "1", "2", "3", "4", "5", "6", "7", "8", "9"}, + Failed: []string{"1337"}, + Errors: map[string][]string{"test": {"1", "2"}}, + }, } - addRulesMonitoringTags(&th, rInfo) + addRulesMonitoringTags(&th, &wafDiags) addWAFMonitoringTags(&th, "1.2.3", 2, 1, 3) tags := th.Tags()