From 431822fea7bca6548853591e55c400746800bedd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Mon, 22 Apr 2024 17:08:45 +0200 Subject: [PATCH 1/8] appsec: add actions package --- internal/appsec/actions/actions.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 internal/appsec/actions/actions.go diff --git a/internal/appsec/actions/actions.go b/internal/appsec/actions/actions.go new file mode 100644 index 0000000000..2511de2e6c --- /dev/null +++ b/internal/appsec/actions/actions.go @@ -0,0 +1,19 @@ +package actions + +type ( + ActionEntry[T any] struct { + ID string `json:"id"` + Type string `json:"type"` + Parameters T `json:"parameters"` + } + + BlockActionParams struct { + GRPCStatusCode *int `json:"grpc_status_code,omitempty"` + StatusCode int `json:"status_code"` + Type string `json:"type,omitempty"` + } + RedirectActionParams struct { + Location string `json:"location,omitempty"` + StatusCode int `json:"status_code"` + } +) From 1af83516744f4a28a09d9bc58f112057dd940b6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Tue, 23 Apr 2024 15:07:50 +0200 Subject: [PATCH 2/8] appsec: config: register default block action --- internal/appsec/config/config.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/internal/appsec/config/config.go b/internal/appsec/config/config.go index d7cf538881..60f58c6969 100644 --- a/internal/appsec/config/config.go +++ b/internal/appsec/config/config.go @@ -13,6 +13,7 @@ import ( internal "github.com/DataDog/appsec-internal-go/appsec" + "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/actions" "gopkg.in/DataDog/dd-trace-go.v1/internal/log" "gopkg.in/DataDog/dd-trace-go.v1/internal/remoteconfig" "gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry" @@ -109,6 +110,16 @@ func NewConfig() (*Config, error) { return nil, err } + // Default blocking action + r.Base.Actions = append(r.Base.Actions, actions.ActionEntry[actions.BlockActionParams]{ + ID: "block", + Type: "block_request", + Parameters: actions.BlockActionParams{ + Type: "auto", + StatusCode: 403, + }, + }) + return &Config{ RulesManager: r, WAFTimeout: internal.WAFTimeoutFromEnv(), From 4e559b39d87abf0bea37f4ef6e68b294ac66c626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Tue, 23 Apr 2024 15:09:35 +0200 Subject: [PATCH 3/8] appsec: adapt RC action test --- internal/appsec/remoteconfig_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/appsec/remoteconfig_test.go b/internal/appsec/remoteconfig_test.go index 9570ccbed7..684831baad 100644 --- a/internal/appsec/remoteconfig_test.go +++ b/internal/appsec/remoteconfig_test.go @@ -807,7 +807,7 @@ func TestWafRCUpdate(t *testing.T) { // Make sure the rule returns a blocking action when matching result = sharedsec.RunWAF(newWafCtx, waf.RunAddressData{Persistent: values}, cfg.WAFTimeout) require.Contains(t, jsonString(t, result.Events), "crs-913-120") - require.Contains(t, result.Actions, "block") + require.Contains(t, result.Actions, "block_request") }) } From 239824823c033e14d86a9dfb04fb9420e08983ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Tue, 23 Apr 2024 15:32:34 +0200 Subject: [PATCH 4/8] appsec: pass actions definitions to the WAF --- internal/appsec/appsec.go | 2 +- internal/appsec/config/rules_manager.go | 28 +++------ internal/appsec/emitter/sharedsec/actions.go | 31 ++++++++-- .../appsec/emitter/sharedsec/actions_test.go | 14 ++--- .../appsec/listener/graphqlsec/graphql.go | 3 +- internal/appsec/listener/grpcsec/grpc.go | 13 ++--- internal/appsec/listener/httpsec/http.go | 35 ++++++------ internal/appsec/listener/sharedsec/shared.go | 25 ++++++-- internal/appsec/waf.go | 57 +++++-------------- 9 files changed, 102 insertions(+), 106 deletions(-) diff --git a/internal/appsec/appsec.go b/internal/appsec/appsec.go index f41373edc7..826d7b7851 100644 --- a/internal/appsec/appsec.go +++ b/internal/appsec/appsec.go @@ -129,7 +129,7 @@ func setActiveAppSec(a *appsec) { type appsec struct { cfg *config.Config limiter *limiter.TokenTicker - wafHandle *wafHandle + wafHandle *waf.Handle started bool } diff --git a/internal/appsec/config/rules_manager.go b/internal/appsec/config/rules_manager.go index 46d22b3899..a61f68e804 100644 --- a/internal/appsec/config/rules_manager.go +++ b/internal/appsec/config/rules_manager.go @@ -29,15 +29,15 @@ type ( // RulesFragment can represent a full ruleset or a fragment of it. RulesFragment struct { Version string `json:"version,omitempty"` - Metadata interface{} `json:"metadata,omitempty"` - Rules []interface{} `json:"rules,omitempty"` - Overrides []interface{} `json:"rules_override,omitempty"` - Exclusions []interface{} `json:"exclusions,omitempty"` + Metadata any `json:"metadata,omitempty"` + Rules []any `json:"rules,omitempty"` + Overrides []any `json:"rules_override,omitempty"` + Exclusions []any `json:"exclusions,omitempty"` RulesData []RuleDataEntry `json:"rules_data,omitempty"` - Actions []ActionEntry `json:"actions,omitempty"` - CustomRules []interface{} `json:"custom_rules,omitempty"` - Processors []interface{} `json:"processors,omitempty"` - Scanners []interface{} `json:"scanners,omitempty"` + Actions []any `json:"actions,omitempty"` + CustomRules []any `json:"custom_rules,omitempty"` + Processors []any `json:"processors,omitempty"` + Scanners []any `json:"scanners,omitempty"` } // RuleDataEntry represents an entry in the "rules_data" top level field of a rules file @@ -46,18 +46,6 @@ type ( RulesData struct { RulesData []RuleDataEntry `json:"rules_data"` } - - // ActionEntry represents an entry in the "actions" top level field of a rules file - ActionEntry struct { - ID string `json:"id"` - Type string `json:"type"` - Parameters struct { - StatusCode int `json:"status_code"` - GRPCStatusCode *int `json:"grpc_status_code,omitempty"` - Type string `json:"type,omitempty"` - Location string `json:"location,omitempty"` - } `json:"parameters,omitempty"` - } ) // DefaultRulesFragment returns a RulesFragment created using the default static recommended rules diff --git a/internal/appsec/emitter/sharedsec/actions.go b/internal/appsec/emitter/sharedsec/actions.go index a248dc0e95..e103e401ac 100644 --- a/internal/appsec/emitter/sharedsec/actions.go +++ b/internal/appsec/emitter/sharedsec/actions.go @@ -12,6 +12,7 @@ import ( "os" "strings" + "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/actions" "gopkg.in/DataDog/dd-trace-go.v1/internal/log" ) @@ -108,8 +109,31 @@ func newGRPCBlockHandler(status int) GRPCWrapper { } } -// NewBlockRequestAction creates an action for the "block" action type -func NewBlockRequestAction(httpStatus, grpcStatus int, template string) *Action { +// NewBlockAction creates an action for the "block_request" action type +func NewBlockAction(params any) *Action { + p, ok := params.(actions.BlockActionParams) + if !ok { + // TODO: log + return nil + } + grpcCode := 10 + if p.GRPCStatusCode != nil { + grpcCode = *p.GRPCStatusCode + } + return newBlockRequestAction(p.StatusCode, grpcCode, p.Type) +} + +// NewRedirectAction creates an action for the "redirect_request" action type +func NewRedirectAction(params any) *Action { + p, ok := params.(actions.RedirectActionParams) + if !ok { + //TODO: log + return nil + } + return newRedirectRequestAction(p.StatusCode, p.Location) +} + +func newBlockRequestAction(httpStatus, grpcStatus int, template string) *Action { return &Action{ http: NewBlockHandler(httpStatus, template), grpc: newGRPCBlockHandler(grpcStatus), @@ -117,8 +141,7 @@ func NewBlockRequestAction(httpStatus, grpcStatus int, template string) *Action } } -// NewRedirectRequestAction creates an action for the "redirect" action type -func NewRedirectRequestAction(status int, loc string) *Action { +func newRedirectRequestAction(status int, loc string) *Action { // Default to 303 if status is out of redirection codes bounds if status < 300 || status >= 400 { status = 303 diff --git a/internal/appsec/emitter/sharedsec/actions_test.go b/internal/appsec/emitter/sharedsec/actions_test.go index b49c83ca56..a86597136d 100644 --- a/internal/appsec/emitter/sharedsec/actions_test.go +++ b/internal/appsec/emitter/sharedsec/actions_test.go @@ -17,9 +17,9 @@ import ( func TestNewBlockRequestAction(t *testing.T) { mux := http.NewServeMux() srv := httptest.NewServer(mux) - mux.HandleFunc("/json", NewBlockRequestAction(403, 10, "json").HTTP().ServeHTTP) - mux.HandleFunc("/html", NewBlockRequestAction(403, 10, "html").HTTP().ServeHTTP) - mux.HandleFunc("/auto", NewBlockRequestAction(403, 10, "auto").HTTP().ServeHTTP) + mux.HandleFunc("/json", newBlockRequestAction(403, 10, "json").HTTP().ServeHTTP) + mux.HandleFunc("/html", newBlockRequestAction(403, 10, "html").HTTP().ServeHTTP) + mux.HandleFunc("/auto", newBlockRequestAction(403, 10, "auto").HTTP().ServeHTTP) defer srv.Close() t.Run("json", func(t *testing.T) { @@ -157,10 +157,10 @@ func TestNewBlockRequestAction(t *testing.T) { func TestNewRedirectRequestAction(t *testing.T) { mux := http.NewServeMux() srv := httptest.NewServer(mux) - mux.HandleFunc("/redirect-default-status", NewRedirectRequestAction(100, "/redirected").HTTP().ServeHTTP) - mux.HandleFunc("/redirect-no-location", NewRedirectRequestAction(303, "").HTTP().ServeHTTP) - mux.HandleFunc("/redirect1", NewRedirectRequestAction(http.StatusFound, "/redirect2").HTTP().ServeHTTP) - mux.HandleFunc("/redirect2", NewRedirectRequestAction(http.StatusFound, "/redirected").HTTP().ServeHTTP) + mux.HandleFunc("/redirect-default-status", newRedirectRequestAction(100, "/redirected").HTTP().ServeHTTP) + mux.HandleFunc("/redirect-no-location", newRedirectRequestAction(303, "").HTTP().ServeHTTP) + mux.HandleFunc("/redirect1", newRedirectRequestAction(http.StatusFound, "/redirect2").HTTP().ServeHTTP) + mux.HandleFunc("/redirect2", newRedirectRequestAction(http.StatusFound, "/redirected").HTTP().ServeHTTP) mux.HandleFunc("/redirected", func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) // Shouldn't matter since we write 302 before arriving here w.Write([]byte("Redirected")) diff --git a/internal/appsec/listener/graphqlsec/graphql.go b/internal/appsec/listener/graphqlsec/graphql.go index 90e6e70ec3..63dd073ce6 100644 --- a/internal/appsec/listener/graphqlsec/graphql.go +++ b/internal/appsec/listener/graphqlsec/graphql.go @@ -14,7 +14,6 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/config" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/emitter/graphqlsec/types" - "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/emitter/sharedsec" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/listener" shared "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/listener/sharedsec" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/trace" @@ -33,7 +32,7 @@ var supportedAddresses = listener.AddressSet{ } // Install registers the GraphQL WAF Event Listener on the given root operation. -func Install(wafHandle *waf.Handle, _ sharedsec.Actions, cfg *config.Config, lim limiter.Limiter, root dyngo.Operation) { +func Install(wafHandle *waf.Handle, cfg *config.Config, lim limiter.Limiter, root dyngo.Operation) { if listener := newWafEventListener(wafHandle, cfg, lim); listener != nil { log.Debug("appsec: registering the GraphQL WAF Event Listener") dyngo.On(root, listener.onEvent) diff --git a/internal/appsec/listener/grpcsec/grpc.go b/internal/appsec/listener/grpcsec/grpc.go index 4d8807748e..ff71fda551 100644 --- a/internal/appsec/listener/grpcsec/grpc.go +++ b/internal/appsec/listener/grpcsec/grpc.go @@ -44,8 +44,8 @@ var supportedAddresses = listener.AddressSet{ } // Install registers the gRPC WAF Event Listener on the given root operation. -func Install(wafHandle *waf.Handle, actions sharedsec.Actions, cfg *config.Config, lim limiter.Limiter, root dyngo.Operation) { - if listener := newWafEventListener(wafHandle, actions, cfg, lim); listener != nil { +func Install(wafHandle *waf.Handle, cfg *config.Config, lim limiter.Limiter, root dyngo.Operation) { + if listener := newWafEventListener(wafHandle, cfg, lim); listener != nil { log.Debug("appsec: registering the gRPC WAF Event Listener") dyngo.On(root, listener.onEvent) } @@ -61,7 +61,7 @@ type wafEventListener struct { once sync.Once } -func newWafEventListener(wafHandle *waf.Handle, actions sharedsec.Actions, cfg *config.Config, limiter limiter.Limiter) *wafEventListener { +func newWafEventListener(wafHandle *waf.Handle, cfg *config.Config, limiter limiter.Limiter) *wafEventListener { if wafHandle == nil { log.Debug("appsec: no WAF Handle available, the gRPC WAF Event Listener will not be registered") return nil @@ -76,7 +76,6 @@ func newWafEventListener(wafHandle *waf.Handle, actions sharedsec.Actions, cfg * return &wafEventListener{ wafHandle: wafHandle, config: cfg, - actions: actions, addresses: addresses, limiter: limiter, wafDiags: wafHandle.Diagnostics(), @@ -114,8 +113,8 @@ func (l *wafEventListener) onEvent(op *types.HandlerOperation, handlerArgs types } wafResult := shared.RunWAF(wafCtx, waf.RunAddressData{Persistent: values}, l.config.WAFTimeout) if wafResult.HasActions() || wafResult.HasEvents() { - for _, id := range wafResult.Actions { - if a, ok := l.actions[id]; ok && a.Blocking() { + for aType, params := range wafResult.Actions { + if a := shared.ActionFromEntry(aType, params); a != nil && a.Blocking() { code, err := a.GRPC()(map[string][]string{}) dyngo.EmitData(userIDOp, types.NewMonitoringError(err.Error(), code)) } @@ -137,7 +136,7 @@ func (l *wafEventListener) onEvent(op *types.HandlerOperation, handlerArgs types wafResult := shared.RunWAF(wafCtx, waf.RunAddressData{Persistent: values}, l.config.WAFTimeout) if wafResult.HasActions() || wafResult.HasEvents() { - interrupt := shared.ProcessActions(op, l.actions, wafResult.Actions) + interrupt := shared.ProcessActions(op, wafResult.Actions) shared.AddSecurityEvents(op, l.limiter, wafResult.Events) log.Debug("appsec: WAF detected an attack before executing the request") if interrupt { diff --git a/internal/appsec/listener/httpsec/http.go b/internal/appsec/listener/httpsec/http.go index 2d821f474e..60d56a4e51 100644 --- a/internal/appsec/listener/httpsec/http.go +++ b/internal/appsec/listener/httpsec/http.go @@ -54,8 +54,8 @@ var supportedAddresses = listener.AddressSet{ } // Install registers the HTTP WAF Event Listener on the given root operation. -func Install(wafHandle *waf.Handle, actions sharedsec.Actions, cfg *config.Config, lim limiter.Limiter, root dyngo.Operation) { - if listener := newWafEventListener(wafHandle, actions, cfg, lim); listener != nil { +func Install(wafHandle *waf.Handle, cfg *config.Config, lim limiter.Limiter, root dyngo.Operation) { + if listener := newWafEventListener(wafHandle, cfg, lim); listener != nil { log.Debug("appsec: registering the HTTP WAF Event Listener") dyngo.On(root, listener.onEvent) } @@ -64,14 +64,13 @@ func Install(wafHandle *waf.Handle, actions sharedsec.Actions, cfg *config.Confi type wafEventListener struct { wafHandle *waf.Handle config *config.Config - actions sharedsec.Actions addresses map[string]struct{} limiter limiter.Limiter wafDiags waf.Diagnostics once sync.Once } -func newWafEventListener(wafHandle *waf.Handle, actions sharedsec.Actions, cfg *config.Config, limiter limiter.Limiter) *wafEventListener { +func newWafEventListener(wafHandle *waf.Handle, cfg *config.Config, limiter limiter.Limiter) *wafEventListener { if wafHandle == nil { log.Debug("appsec: no WAF Handle available, the HTTP WAF Event Listener will not be registered") return nil @@ -86,7 +85,6 @@ func newWafEventListener(wafHandle *waf.Handle, actions sharedsec.Actions, cfg * return &wafEventListener{ wafHandle: wafHandle, config: cfg, - actions: actions, addresses: addresses, limiter: limiter, wafDiags: wafHandle.Diagnostics(), @@ -108,7 +106,7 @@ func (l *wafEventListener) onEvent(op *types.Operation, args types.HandlerOperat dyngo.On(op, func(operation *sharedsec.UserIDOperation, args sharedsec.UserIDOperationArgs) { wafResult := shared.RunWAF(wafCtx, waf.RunAddressData{Persistent: map[string]any{UserIDAddr: args.UserID}}, l.config.WAFTimeout) if wafResult.HasActions() || wafResult.HasEvents() { - processHTTPSDKAction(operation, l.actions, wafResult.Actions) + processHTTPSDKAction(operation, wafResult.Actions) shared.AddSecurityEvents(op, l.limiter, wafResult.Events) log.Debug("appsec: WAF detected a suspicious user: %s", args.UserID) } @@ -155,7 +153,7 @@ func (l *wafEventListener) onEvent(op *types.Operation, args types.HandlerOperat op.AddSerializableTag(tag, value) } if wafResult.HasActions() || wafResult.HasEvents() { - interrupt := shared.ProcessActions(op, l.actions, wafResult.Actions) + interrupt := shared.ProcessActions(op, wafResult.Actions) shared.AddSecurityEvents(op, l.limiter, wafResult.Events) log.Debug("appsec: WAF detected an attack before executing the request") if interrupt { @@ -171,7 +169,7 @@ func (l *wafEventListener) onEvent(op *types.Operation, args types.HandlerOperat op.AddSerializableTag(tag, value) } if wafResult.HasActions() || wafResult.HasEvents() { - processHTTPSDKAction(sdkBodyOp, l.actions, wafResult.Actions) + processHTTPSDKAction(sdkBodyOp, wafResult.Actions) shared.AddSecurityEvents(op, l.limiter, wafResult.Events) log.Debug("appsec: WAF detected a suspicious request body") } @@ -225,15 +223,18 @@ func (l *wafEventListener) canExtractSchemas() bool { // - send actions to the parent operation's data listener, for their handlers to be executed after the user handler // - send an error to the current operation's data listener (created by an SDK call), to signal users to interrupt // their handler. -func processHTTPSDKAction(op dyngo.Operation, actions sharedsec.Actions, actionIds []string) { - for _, id := range actionIds { - if action, ok := actions[id]; ok { - if op.Parent() != nil { - dyngo.EmitData(op, action) // Send the action so that the handler gets executed - } - if action.Blocking() { // Send the error to be returned by the SDK - dyngo.EmitData(op, types.NewMonitoringError("Request blocked")) // Send error - } +func processHTTPSDKAction(op dyngo.Operation, actions map[string]any) { + for aType, params := range actions { + action := shared.ActionFromEntry(aType, params) + if action == nil { + log.Debug("cannot process %s action with params %v", aType, params) + continue + } + if op.Parent() != nil { + dyngo.EmitData(op, action) // Send the action so that the handler gets executed + } + if action.Blocking() { // Send the error to be returned by the SDK + dyngo.EmitData(op, types.NewMonitoringError("Request blocked")) // Send error } } } diff --git a/internal/appsec/listener/sharedsec/shared.go b/internal/appsec/listener/sharedsec/shared.go index b46d1459c4..4e4571830b 100644 --- a/internal/appsec/listener/sharedsec/shared.go +++ b/internal/appsec/listener/sharedsec/shared.go @@ -79,12 +79,27 @@ func AddWAFMonitoringTags(th trace.TagSetter, rulesVersion string, stats map[str // ProcessActions sends the relevant actions to the operation's data listener. // It returns true if at least one of those actions require interrupting the request handler -func ProcessActions(op dyngo.Operation, actions sharedsec.Actions, actionIds []string) (interrupt bool) { - for _, id := range actionIds { - if a, ok := actions[id]; ok { - dyngo.EmitData(op, actions[id]) - interrupt = interrupt || a.Blocking() +func ProcessActions(op dyngo.Operation, actions map[string]any) (interrupt bool) { + for aType, params := range actions { + action := ActionFromEntry(aType, params) + if action == nil { + log.Debug("cannot process %s action with params %v", aType, params) + continue } + dyngo.EmitData(op, action) + interrupt = interrupt || action.Blocking() } return interrupt } + +func ActionFromEntry(actionType string, params any) *sharedsec.Action { + switch actionType { + case "block_request": + return sharedsec.NewBlockAction(params) + case "redirect_request": + return sharedsec.NewRedirectAction(params) + default: + log.Debug("appsec: unknown action type `%s`", actionType) + return nil + } +} diff --git a/internal/appsec/waf.go b/internal/appsec/waf.go index 8e74bfca89..649dbadb9b 100644 --- a/internal/appsec/waf.go +++ b/internal/appsec/waf.go @@ -8,18 +8,11 @@ package appsec import ( "github.com/DataDog/appsec-internal-go/limiter" waf "github.com/DataDog/go-libddwaf/v2" + "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/actions" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/config" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo" - "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/emitter/sharedsec" - "gopkg.in/DataDog/dd-trace-go.v1/internal/log" ) -type wafHandle struct { - *waf.Handle - // actions are tightly link to a ruleset, which is linked to a waf handle - actions sharedsec.Actions -} - func (a *appsec) swapWAF(rules config.RulesFragment) (err error) { // Instantiate a new WAF handle and verify its state newHandle, err := newWAFHandle(rules, a.cfg) @@ -36,7 +29,7 @@ func (a *appsec) swapWAF(rules config.RulesFragment) (err error) { newRoot := dyngo.NewRootOperation() for _, fn := range wafEventListeners { - fn(newHandle.Handle, newHandle.actions, a.cfg, a.limiter, newRoot) + fn(newHandle, a.cfg, a.limiter, newRoot) } // Hot-swap dyngo's root operation @@ -58,42 +51,20 @@ func (a *appsec) swapWAF(rules config.RulesFragment) (err error) { return nil } -func actionFromEntry(e *config.ActionEntry) *sharedsec.Action { - switch e.Type { - case "block_request": - grpcCode := 10 // use the grpc.Codes value for "Aborted" by default - if e.Parameters.GRPCStatusCode != nil { - grpcCode = *e.Parameters.GRPCStatusCode - } - return sharedsec.NewBlockRequestAction(e.Parameters.StatusCode, grpcCode, e.Parameters.Type) - case "redirect_request": - return sharedsec.NewRedirectRequestAction(e.Parameters.StatusCode, e.Parameters.Location) - default: - log.Debug("appsec: unknown action type `%s`", e.Type) - return nil - } -} - -func newWAFHandle(rules config.RulesFragment, cfg *config.Config) (*wafHandle, error) { - handle, err := waf.NewHandle(rules, cfg.Obfuscator.KeyRegex, cfg.Obfuscator.ValueRegex) - actions := sharedsec.Actions{ - // Default built-in block action - "block": sharedsec.NewBlockRequestAction(403, 10, "auto"), - } - - for _, entry := range rules.Actions { - a := actionFromEntry(&entry) - if a != nil { - actions[entry.ID] = a - } - } - return &wafHandle{ - Handle: handle, - actions: actions, - }, err +func newWAFHandle(rules config.RulesFragment, cfg *config.Config) (*waf.Handle, error) { + // Default block action + rules.Actions = append(rules.Actions, actions.ActionEntry[actions.BlockActionParams]{ + ID: "block", + Type: "block_request", + Parameters: actions.BlockActionParams{ + Type: "auto", + StatusCode: 403, + }, + }) + return waf.NewHandle(rules, cfg.Obfuscator.KeyRegex, cfg.Obfuscator.ValueRegex) } -type wafEventListener func(*waf.Handle, sharedsec.Actions, *config.Config, limiter.Limiter, dyngo.Operation) +type wafEventListener func(*waf.Handle, *config.Config, limiter.Limiter, dyngo.Operation) // wafEventListeners is the global list of event listeners registered by contribs at init time. This // is thread-safe assuming all writes (via AddWAFEventListener) are performed within `init` From 97ce5174d0d5ea0964b80fd5f2405ceb7e2a05e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Thu, 25 Apr 2024 15:12:16 +0200 Subject: [PATCH 5/8] appsec: actions: translate all string waf params --- internal/appsec/actions/actions.go | 63 ++++++++++++++++++++ internal/appsec/emitter/sharedsec/actions.go | 20 +++---- internal/appsec/listener/sharedsec/shared.go | 8 ++- 3 files changed, 76 insertions(+), 15 deletions(-) diff --git a/internal/appsec/actions/actions.go b/internal/appsec/actions/actions.go index 2511de2e6c..b6cef4a37b 100644 --- a/internal/appsec/actions/actions.go +++ b/internal/appsec/actions/actions.go @@ -1,5 +1,10 @@ package actions +import ( + "encoding/json" + "strconv" +) + type ( ActionEntry[T any] struct { ID string `json:"id"` @@ -17,3 +22,61 @@ type ( StatusCode int `json:"status_code"` } ) + +func BlockParamsFromMap(params map[string]any) (BlockActionParams, error) { + type blockActionParams struct { + GRPCStatusCode string `json:"grpc_status_code,omitempty"` + StatusCode string `json:"status_code"` + Type string `json:"type,omitempty"` + } + p := BlockActionParams{ + StatusCode: 403, + Type: "auto", + } + var strParams blockActionParams + var err error + data, err := json.Marshal(params) + if err != nil { + return p, err + } + if err := json.Unmarshal(data, &strParams); err != nil { + return p, err + } + + p.Type = strParams.Type + + if p.StatusCode, err = strconv.Atoi(strParams.StatusCode); err != nil { + return p, err + } + if strParams.GRPCStatusCode == "" { + strParams.GRPCStatusCode = "10" + } + + if grpcCode, err := strconv.Atoi(strParams.GRPCStatusCode); err != nil { + return p, err + } else { + p.GRPCStatusCode = &grpcCode + } + return p, nil + +} +func RedirectParamsFromMap(params map[string]any) (RedirectActionParams, error) { + type redirectActionParams struct { + Location string `json:"location,omitempty"` + StatusCode string `json:"status_code"` + } + p := RedirectActionParams{} + var strParams redirectActionParams + var err error + data, err := json.Marshal(params) + if err != nil { + return p, err + } + if err := json.Unmarshal(data, &strParams); err != nil { + return p, err + } + + p.Location = strParams.Location + p.StatusCode, err = strconv.Atoi(strParams.StatusCode) + return p, err +} diff --git a/internal/appsec/emitter/sharedsec/actions.go b/internal/appsec/emitter/sharedsec/actions.go index e103e401ac..7000ae6936 100644 --- a/internal/appsec/emitter/sharedsec/actions.go +++ b/internal/appsec/emitter/sharedsec/actions.go @@ -110,24 +110,18 @@ func newGRPCBlockHandler(status int) GRPCWrapper { } // NewBlockAction creates an action for the "block_request" action type -func NewBlockAction(params any) *Action { - p, ok := params.(actions.BlockActionParams) - if !ok { - // TODO: log +func NewBlockAction(params map[string]any) *Action { + p, err := actions.BlockParamsFromMap(params) + if err != nil { return nil } - grpcCode := 10 - if p.GRPCStatusCode != nil { - grpcCode = *p.GRPCStatusCode - } - return newBlockRequestAction(p.StatusCode, grpcCode, p.Type) + return newBlockRequestAction(p.StatusCode, *p.GRPCStatusCode, p.Type) } // NewRedirectAction creates an action for the "redirect_request" action type -func NewRedirectAction(params any) *Action { - p, ok := params.(actions.RedirectActionParams) - if !ok { - //TODO: log +func NewRedirectAction(params map[string]any) *Action { + p, err := actions.RedirectParamsFromMap(params) + if err != nil { return nil } return newRedirectRequestAction(p.StatusCode, p.Location) diff --git a/internal/appsec/listener/sharedsec/shared.go b/internal/appsec/listener/sharedsec/shared.go index 4e4571830b..3f4df08a4b 100644 --- a/internal/appsec/listener/sharedsec/shared.go +++ b/internal/appsec/listener/sharedsec/shared.go @@ -93,11 +93,15 @@ func ProcessActions(op dyngo.Operation, actions map[string]any) (interrupt bool) } func ActionFromEntry(actionType string, params any) *sharedsec.Action { + p, ok := params.(map[string]any) + if !ok { + return nil + } switch actionType { case "block_request": - return sharedsec.NewBlockAction(params) + return sharedsec.NewBlockAction(p) case "redirect_request": - return sharedsec.NewRedirectAction(params) + return sharedsec.NewRedirectAction(p) default: log.Debug("appsec: unknown action type `%s`", actionType) return nil From 34409e2ed97106a3d58108f4721a93da40db46fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Thu, 25 Apr 2024 16:41:39 +0200 Subject: [PATCH 6/8] appsec: increase WAF test timeouts --- internal/appsec/waf_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/appsec/waf_test.go b/internal/appsec/waf_test.go index aa741f01f4..af95c1e1a3 100644 --- a/internal/appsec/waf_test.go +++ b/internal/appsec/waf_test.go @@ -7,7 +7,6 @@ package appsec_test import ( "encoding/json" - "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/listener/httpsec" "io" "net/http" "net/http/httptest" @@ -22,6 +21,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/config" + "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/listener/httpsec" "github.com/stretchr/testify/require" ) @@ -151,6 +151,7 @@ func TestUserRules(t *testing.T) { // the WAF is properly detecting an LFI attempt and that the corresponding security event is being sent to the agent. // Additionally, verifies that rule matching through SDK body instrumentation works as expected func TestWAF(t *testing.T) { + t.Setenv(internal.EnvWAFTimeout, "1s") appsec.Start() defer appsec.Stop() @@ -435,6 +436,7 @@ func TestBlocking(t *testing.T) { // Test that API Security schemas get collected when API security is enabled func TestAPISecurity(t *testing.T) { // Start and trace an HTTP server + t.Setenv(internal.EnvWAFTimeout, "1s") t.Setenv(config.EnvEnabled, "true") if wafOK, err := waf.Health(); !wafOK { t.Skipf("WAF must be usable for this test to run correctly: %v", err) From 12733ff09f33e5ecf28419aef94757e1e1f2dc49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Thu, 25 Apr 2024 17:13:44 +0200 Subject: [PATCH 7/8] appsec: actions: simplify --- internal/appsec/actions/actions.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/appsec/actions/actions.go b/internal/appsec/actions/actions.go index b6cef4a37b..471c4eb769 100644 --- a/internal/appsec/actions/actions.go +++ b/internal/appsec/actions/actions.go @@ -52,12 +52,11 @@ func BlockParamsFromMap(params map[string]any) (BlockActionParams, error) { strParams.GRPCStatusCode = "10" } - if grpcCode, err := strconv.Atoi(strParams.GRPCStatusCode); err != nil { - return p, err - } else { + grpcCode, err := strconv.Atoi(strParams.GRPCStatusCode) + if err == nil { p.GRPCStatusCode = &grpcCode } - return p, nil + return p, err } func RedirectParamsFromMap(params map[string]any) (RedirectActionParams, error) { From f4b603a64d0b7fe77660f01571f4b5c1c57f398b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Thu, 25 Apr 2024 17:17:55 +0200 Subject: [PATCH 8/8] ci: use go-libddwaf/v3 in smoke tests --- .github/workflows/smoke-tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/smoke-tests.yml b/.github/workflows/smoke-tests.yml index 982411a673..ea79e14067 100644 --- a/.github/workflows/smoke-tests.yml +++ b/.github/workflows/smoke-tests.yml @@ -8,7 +8,7 @@ on: required: true type: string go-libddwaf-ref: - description: A git ref to update github.com/DataDog/go-libddwaf/v2 to. No-op if empty. + description: A git ref to update github.com/DataDog/go-libddwaf/v3 to. No-op if empty. required: false type: string push: @@ -57,7 +57,7 @@ jobs: - name: Install requested go-libddwaf version if: github.event_name == 'workflow_call' && inputs.go-libddwaf-ref != '' run: |- - go get -u -t github.com/DataDog/go-libddwaf/v2@${{ inputs.go-libddwaf-ref }} + go get -u -t github.com/DataDog/go-libddwaf/v3@${{ inputs.go-libddwaf-ref }} go mod tidy - name: Compile dd-trace-go run: go build $PACKAGES