From 7f9ab67d24e2f6298400da264fcb7f642d554ff3 Mon Sep 17 00:00:00 2001 From: Julio Guerra Date: Wed, 5 Jul 2023 21:33:05 +0200 Subject: [PATCH] appsec: integrate the new purego-based go-libddwaf (#2090) --- go.mod | 7 ++--- go.sum | 13 ++++++---- internal/appsec/appsec.go | 38 ++++++++++++++++++++++++++-- internal/appsec/appsec_test.go | 3 ++- internal/appsec/config.go | 6 ++--- internal/appsec/remoteconfig.go | 8 ------ internal/appsec/remoteconfig_test.go | 18 ++++++------- internal/appsec/rule_test.go | 9 +++++-- internal/appsec/waf.go | 16 ++++++------ internal/appsec/waf_unit_test.go | 2 +- 10 files changed, 78 insertions(+), 42 deletions(-) diff --git a/go.mod b/go.mod index 2a65b10688..923007aee4 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/DataDog/datadog-agent/pkg/obfuscate v0.45.0-rc.1 github.com/DataDog/datadog-agent/pkg/remoteconfig/state v0.46.0-rc.4 github.com/DataDog/datadog-go/v5 v5.1.1 - github.com/DataDog/go-libddwaf v1.2.0 + github.com/DataDog/go-libddwaf v1.4.0 github.com/DataDog/gostackparse v0.5.0 github.com/DataDog/sketches-go v1.2.1 github.com/Shopify/sarama v1.22.0 @@ -84,10 +84,10 @@ require ( go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.42.0 go.opentelemetry.io/otel v1.16.0 go.opentelemetry.io/otel/trace v1.16.0 - go.uber.org/atomic v1.10.0 + go.uber.org/atomic v1.11.0 golang.org/x/net v0.10.0 golang.org/x/oauth2 v0.7.0 - golang.org/x/sys v0.8.0 + golang.org/x/sys v0.10.0 golang.org/x/time v0.3.0 golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 google.golang.org/api v0.121.0 @@ -140,6 +140,7 @@ require ( github.com/eapache/go-resiliency v1.1.0 // indirect github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21 // indirect github.com/eapache/queue v1.1.0 // indirect + github.com/ebitengine/purego v0.4.0-alpha.4.0.20230519103000-ee8dcecc618f // indirect github.com/elastic/elastic-transport-go/v8 v8.1.0 // indirect github.com/felixge/httpsnoop v1.0.3 // indirect github.com/gabriel-vasile/mimetype v1.4.2 // indirect diff --git a/go.sum b/go.sum index a62b4b916b..615c6b3503 100644 --- a/go.sum +++ b/go.sum @@ -631,8 +631,8 @@ github.com/DataDog/datadog-agent/pkg/remoteconfig/state v0.46.0-rc.4/go.mod h1:V github.com/DataDog/datadog-go v3.2.0+incompatible/go.mod h1:LButxg5PwREeZtORoXG3tL4fMGNddJ+vMq1mwgfaqoQ= github.com/DataDog/datadog-go/v5 v5.1.1 h1:JLZ6s2K1pG2h9GkvEvMdEGqMDyVLEAccdX5TltWcLMU= github.com/DataDog/datadog-go/v5 v5.1.1/go.mod h1:KhiYb2Badlv9/rofz+OznKoEF5XKTonWyhx5K83AP8E= -github.com/DataDog/go-libddwaf v1.2.0 h1:fKHP5U29E597eV2hU501fcW40bL8zcQ081jEGuRw2kM= -github.com/DataDog/go-libddwaf v1.2.0/go.mod h1:DI5y8obPajk+Tvy2o+nZc2g/5Ria/Rfq5/624k7pHpE= +github.com/DataDog/go-libddwaf v1.4.0 h1:neu9r2KFfn71zHvRrzZgMtRyxb2yYVr3AozIoMj6mf4= +github.com/DataDog/go-libddwaf v1.4.0/go.mod h1:qLZEuaF5amEVMP5NTYtr/6m30m73voPL4i7SK7dnnt4= github.com/DataDog/go-tuf v0.3.0--fix-localmeta-fork h1:yBq5PrAtrM4yVeSzQ+bn050+Ysp++RKF1QmtkL4VqvU= github.com/DataDog/go-tuf v0.3.0--fix-localmeta-fork/go.mod h1:yA5JwkZsHTLuqq3zaRgUQf35DfDkpOZqgtBqHKpwrBs= github.com/DataDog/gostackparse v0.5.0 h1:jb72P6GFHPHz2W0onsN51cS3FkaMDcjb0QzgxxA4gDk= @@ -1046,6 +1046,8 @@ github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21 h1:YEetp8 github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21/go.mod h1:+020luEh2TKB4/GOp8oxxtq0Daoen/Cii55CzbTV6DU= 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.4.0-alpha.4.0.20230519103000-ee8dcecc618f h1:v8f0ADMg0RBM0+5rb8qCFj/XlPkjo+xkyCLuUpBnj9s= +github.com/ebitengine/purego v0.4.0-alpha.4.0.20230519103000-ee8dcecc618f/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= @@ -2122,8 +2124,9 @@ go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= -go.uber.org/atomic v1.10.0 h1:9qC72Qh0+3MqyJbAn8YU5xVq1frD8bn3JtD2oXtafVQ= go.uber.org/atomic v1.10.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0= +go.uber.org/atomic v1.11.0 h1:ZvwS0R+56ePWxUNi+Atn9dWONBPp/AUETXlHW0DxSjE= +go.uber.org/atomic v1.11.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0= go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= go.uber.org/goleak v1.1.12/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= @@ -2513,8 +2516,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.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU= -golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.10.0 h1:SqMFp9UcQJZa+pmYuAKjd9xq1f0j5rLcDIk0mj4qAsA= +golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= 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 3eb5d19e2d..32d234ee4c 100644 --- a/internal/appsec/appsec.go +++ b/internal/appsec/appsec.go @@ -9,6 +9,7 @@ package appsec import ( + "fmt" "sync" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo" @@ -29,16 +30,29 @@ func Enabled() bool { // Start AppSec when enabled is enabled by both using the appsec build tag and // setting the environment variable DD_APPSEC_ENABLED to true. func Start(opts ...StartOption) { + // AppSec can start either: + // 1. Manually thanks to DD_APPSEC_ENABLED + // 2. Remotely when DD_APPSEC_ENABLED is undefined + // Note: DD_APPSEC_ENABLED=false takes precedence over remote configuration + // and enforces to have AppSec disabled. enabled, set, err := isEnabled() if err != nil { logUnexpectedStartError(err) return } + // Check if AppSec is explicitly disabled if set && !enabled { log.Debug("appsec: disabled by the configuration: set the environment variable DD_APPSEC_ENABLED to true to enable it") return } + + // Check whether libddwaf - required for Threats Detection - is supported or not + if supported, err := waf.SupportsTarget(); !supported { + log.Error("appsec: threats detection is not supported: %v\nNo security activities will be collected. Please contact support at https://docs.datadoghq.com/help/ for help.", err) + return + } + // From this point we know that AppSec is either enabled or can be enabled through remote config cfg, err := newConfig() if err != nil { @@ -49,17 +63,21 @@ func Start(opts ...StartOption) { opt(cfg) } appsec := newAppSec(cfg) + + // Start the remote configuration client + log.Debug("appsec: starting the remote configuration client") appsec.startRC() - // If the env var is not set ASM is disabled, but can be enabled through remote config if !set { - log.Debug("appsec: %s is not set. AppSec won't start until activated through remote configuration", enabledEnvVar) + // AppSec is not enforced by the env var and can be enabled through remote config + log.Debug("appsec: %s is not set, appsec won't start until activated through remote configuration", enabledEnvVar) if err := appsec.enableRemoteActivation(); err != nil { // ASM is not enabled and can't be enabled through remote configuration. Nothing more can be done. logUnexpectedStartError(err) appsec.stopRC() return } + log.Debug("appsec: awaiting for possible remote activation") } else if err := appsec.start(); err != nil { // AppSec is specifically enabled logUnexpectedStartError(err) appsec.stopRC() @@ -118,6 +136,18 @@ func newAppSec(cfg *Config) *appsec { // Start AppSec by registering its security protections according to the configured the security rules. func (a *appsec) start() error { + // Load the waf to catch early errors if any + if ok, err := waf.Load(); err != nil { + // 1. If there is an error and the loading is not ok: log as an unexpected error case and quit appsec + // Note that we assume here that the test for the unsupported target has been done before calling + // this method, so it is now considered an error for this method + if !ok { + return fmt.Errorf("error while loading libddwaf: %w", err) + } + // 2. If there is an error and the loading is ok: log as an informative error where appsec can be used + log.Error("appsec: non-critical error while loading libddwaf: %v", err) + } + a.limiter = NewTokenTicker(int64(a.cfg.traceRateLimit), int64(a.cfg.traceRateLimit)) a.limiter.Start() // Register the WAF operation event listener @@ -126,6 +156,9 @@ func (a *appsec) start() error { } a.enableRCBlocking() a.started = true + log.Info("appsec: up and running") + // TODO: log the config like the APM tracer does but we first need to define + // an user-friendly string representation of our config and its sources return nil } @@ -142,6 +175,7 @@ func (a *appsec) stop() { dyngo.SwapRootOperation(nil) if a.wafHandle != nil { a.wafHandle.Close() + a.wafHandle = nil } // TODO: block until no more requests are using dyngo operations diff --git a/internal/appsec/appsec_test.go b/internal/appsec/appsec_test.go index 233360906c..3da30b125d 100644 --- a/internal/appsec/appsec_test.go +++ b/internal/appsec/appsec_test.go @@ -23,7 +23,8 @@ import ( func TestEnabled(t *testing.T) { enabledConfig, _ := strconv.ParseBool(os.Getenv("DD_APPSEC_ENABLED")) - canBeEnabled := enabledConfig && waf.Health() == nil + wafSupported, _ := waf.SupportsTarget() + canBeEnabled := enabledConfig && wafSupported require.False(t, appsec.Enabled()) tracer.Start() diff --git a/internal/appsec/config.go b/internal/appsec/config.go index 5480cfae0a..1aaf419c9b 100644 --- a/internal/appsec/config.go +++ b/internal/appsec/config.go @@ -66,8 +66,8 @@ type ObfuscatorConfig struct { } // isEnabled returns true when appsec is enabled when the environment variable -// It also returns whether the env var is actually set in the env or not // DD_APPSEC_ENABLED is set to true. +// It also returns whether the env var is actually set in the env or not. func isEnabled() (enabled bool, set bool, err error) { enabledStr, set := os.LookupEnv(enabledEnvVar) if enabledStr == "" { @@ -166,7 +166,7 @@ func readRulesConfig() (rules []byte, err error) { rules = []byte(staticRecommendedRules) filepath := os.Getenv(rulesEnvVar) if filepath == "" { - log.Info("appsec: starting with the default recommended security rules") + log.Debug("appsec: using the default built-in recommended security rules") return rules, nil } buf, err := os.ReadFile(filepath) @@ -176,7 +176,7 @@ func readRulesConfig() (rules []byte, err error) { } return nil, err } - log.Info("appsec: starting with the security rules from file %s", filepath) + log.Debug("appsec: using the security rules from file %s", filepath) return buf, nil } diff --git a/internal/appsec/remoteconfig.go b/internal/appsec/remoteconfig.go index e41c0ddec5..8f6af9a950 100644 --- a/internal/appsec/remoteconfig.go +++ b/internal/appsec/remoteconfig.go @@ -18,7 +18,6 @@ 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" ) func genApplyStatus(ack bool, err error) rc.ApplyStatus { @@ -337,13 +336,6 @@ func (a *appsec) enableRemoteActivation() error { if a.rc == nil { return fmt.Errorf("no valid remote configuration client") } - // First verify that the WAF is in good health. We perform this check in order not to falsely "allow" users to - // activate ASM through remote config if activation would fail when trying to register a WAF handle - // (ex: if the service runs on an unsupported platform). - if err := waf.Health(); err != nil { - log.Debug("appsec: WAF health check failed, remote activation will be disabled: %v", err) - return err - } a.registerRCProduct(rc.ProductASMFeatures) a.registerRCCapability(remoteconfig.ASMActivation) a.rc.RegisterCallback(a.onRemoteActivation) diff --git a/internal/appsec/remoteconfig_test.go b/internal/appsec/remoteconfig_test.go index 8b53c44567..2ae9934752 100644 --- a/internal/appsec/remoteconfig_test.go +++ b/internal/appsec/remoteconfig_test.go @@ -25,7 +25,7 @@ import ( ) func TestASMFeaturesCallback(t *testing.T) { - if waf.Health() != nil { + if supported, _ := waf.SupportsTarget(); !supported { t.Skip("WAF cannot be used") } enabledPayload := []byte(`{"asm":{"enabled":true}}`) @@ -321,7 +321,7 @@ func TestMergeRulesDataEntries(t *testing.T) { // This test ensures that the remote activation capabilities are only set if DD_APPSEC_ENABLED is not set in the env. func TestRemoteActivationScenarios(t *testing.T) { - if waf.Health() != nil { + if supported, _ := waf.SupportsTarget(); !supported { t.Skip("WAF cannot be used") } @@ -547,7 +547,7 @@ func TestOnRCUpdate(t *testing.T) { } t.Run("post-stop", func(t *testing.T) { - if waf.Health() != nil { + if supported, _ := waf.SupportsTarget(); !supported { t.Skip("WAF needs to be available for this test (remote activation requirement)") } @@ -626,7 +626,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 rule"))}, + expected: map[string]rc.ApplyStatus{"invalid": genApplyStatus(true, errors.New("could not instantiate the WAF"))}, }, { name: "multiple/ack", @@ -637,8 +637,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 rule")), - "invalid": genApplyStatus(true, errors.New("could not instantiate the waf rule")), + "overrides": genApplyStatus(true, errors.New("could not instantiate the WAF")), + "invalid": genApplyStatus(true, errors.New("could not instantiate the WAF")), }, }, { @@ -682,14 +682,14 @@ func TestWafRCUpdate(t *testing.T) { }, } - if waf.Health() != nil { + if supported, _ := waf.SupportsTarget(); !supported { t.Skip("WAF needs to be available for this test") } t.Run("toggle-blocking", func(t *testing.T) { cfg, err := newConfig() require.NoError(t, err) - wafHandle, err := waf.NewHandleFromRuleSet(cfg.rulesManager.latest, cfg.obfuscator.KeyRegex, cfg.obfuscator.ValueRegex) + wafHandle, err := waf.NewHandle(cfg.rulesManager.latest, cfg.obfuscator.KeyRegex, cfg.obfuscator.ValueRegex) require.NoError(t, err) defer wafHandle.Close() wafCtx := waf.NewContext(wafHandle) @@ -707,7 +707,7 @@ func TestWafRCUpdate(t *testing.T) { require.Equal(t, status.State, rc.ApplyStateAcknowledged) } cfg.rulesManager.compile() - newWafHandle, err := waf.NewHandleFromRuleSet(cfg.rulesManager.latest, cfg.obfuscator.KeyRegex, cfg.obfuscator.ValueRegex) + newWafHandle, err := waf.NewHandle(cfg.rulesManager.latest, cfg.obfuscator.KeyRegex, cfg.obfuscator.ValueRegex) require.NoError(t, err) defer newWafHandle.Close() newWafCtx := waf.NewContext(newWafHandle) diff --git a/internal/appsec/rule_test.go b/internal/appsec/rule_test.go index 9e75883bc8..a7b5737a60 100644 --- a/internal/appsec/rule_test.go +++ b/internal/appsec/rule_test.go @@ -9,6 +9,7 @@ package appsec import ( + "encoding/json" "testing" waf "github.com/DataDog/go-libddwaf" @@ -16,11 +17,15 @@ import ( ) func TestStaticRule(t *testing.T) { - if waf.Health() != nil { + if supported, _ := waf.SupportsTarget(); !supported { t.Skip("waf disabled") return } - waf, err := waf.NewHandle([]byte(staticRecommendedRules), "", "") + + var rules rulesFragment + require.NoError(t, json.Unmarshal([]byte(staticRecommendedRules), &rules)) + waf, err := waf.NewHandle(rules, "", "") require.NoError(t, err) + require.NotNil(t, waf) waf.Close() } diff --git a/internal/appsec/waf.go b/internal/appsec/waf.go index c90c84c154..df8b7bad6e 100644 --- a/internal/appsec/waf.go +++ b/internal/appsec/waf.go @@ -14,7 +14,6 @@ import ( "fmt" "sort" "sync" - "sync/atomic" "time" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" @@ -26,6 +25,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/samplernames" waf "github.com/DataDog/go-libddwaf" + "go.uber.org/atomic" ) const ( @@ -84,7 +84,7 @@ func (a *appsec) swapWAF(rules rulesFragment) (err error) { } func newWAFHandle(rules rulesFragment, cfg *Config) (*waf.Handle, error) { - return waf.NewHandleFromRuleSet(rules, cfg.obfuscator.KeyRegex, cfg.obfuscator.ValueRegex) + return waf.NewHandle(rules, cfg.obfuscator.KeyRegex, cfg.obfuscator.ValueRegex) } func newWAFEventListeners(waf *waf.Handle, cfg *Config, l Limiter) (listeners []dyngo.EventListener, err error) { @@ -251,11 +251,11 @@ func newGRPCWAFEventListener(handle *waf.Handle, addresses map[string]struct{}, // receive unlimited number of messages where we could find security events const maxWAFEventsPerRequest = 10 var ( - nbEvents uint32 + nbEvents atomic.Uint32 logOnce sync.Once // per request - overallRuntimeNs waf.AtomicU64 - internalRuntimeNs waf.AtomicU64 - nbTimeouts waf.AtomicU64 + overallRuntimeNs atomic.Uint64 + internalRuntimeNs atomic.Uint64 + nbTimeouts atomic.Uint64 events []json.RawMessage mu sync.Mutex // events mutex @@ -311,7 +311,7 @@ func newGRPCWAFEventListener(handle *waf.Handle, addresses map[string]struct{}, } op.On(grpcsec.OnReceiveOperationFinish(func(_ grpcsec.ReceiveOperation, res grpcsec.ReceiveOperationRes) { - if atomic.LoadUint32(&nbEvents) == maxWAFEventsPerRequest { + if nbEvents.Load() == maxWAFEventsPerRequest { logOnce.Do(func() { log.Debug("appsec: ignoring the rpc message due to the maximum number of security events per grpc call reached") }) @@ -355,7 +355,7 @@ func newGRPCWAFEventListener(handle *waf.Handle, addresses map[string]struct{}, return } log.Debug("appsec: attack detected by the grpc waf") - atomic.AddUint32(&nbEvents, 1) + nbEvents.Inc() mu.Lock() events = append(events, event) mu.Unlock() diff --git a/internal/appsec/waf_unit_test.go b/internal/appsec/waf_unit_test.go index b7c174d0ce..f7f041226c 100644 --- a/internal/appsec/waf_unit_test.go +++ b/internal/appsec/waf_unit_test.go @@ -24,7 +24,7 @@ func TestTagsTypes(t *testing.T) { Version: "1.3.0", Loaded: 10, Failed: 1, - Errors: map[string]interface{}{"test": []string{"1", "2"}}, + Errors: map[string][]string{"test": {"1", "2"}}, } addRulesMonitoringTags(&th, rInfo)