diff --git a/contrib/net/http/roundtripper.go b/contrib/net/http/roundtripper.go index 627620625d..7c4e64edde 100644 --- a/contrib/net/http/roundtripper.go +++ b/contrib/net/http/roundtripper.go @@ -26,6 +26,8 @@ type roundTripper struct { cfg *roundTripperConfig } +var securityError = &events.BlockingSecurityEvent{} + func (rt *roundTripper) RoundTrip(req *http.Request) (res *http.Response, err error) { if rt.cfg.ignoreRequest(req) { return rt.base.RoundTrip(req) @@ -61,7 +63,7 @@ func (rt *roundTripper) RoundTrip(req *http.Request) (res *http.Response, err er if rt.cfg.after != nil { rt.cfg.after(res, span) } - if !errors.Is(err, &events.BlockingSecurityEvent{}) && (rt.cfg.errCheck == nil || rt.cfg.errCheck(err)) { + if !errors.Is(err, securityError) && (rt.cfg.errCheck == nil || rt.cfg.errCheck(err)) { span.Finish(tracer.WithError(err)) } else { span.Finish() @@ -80,7 +82,7 @@ func (rt *roundTripper) RoundTrip(req *http.Request) (res *http.Response, err er } } - if appsec.Enabled() { + if appsec.RASPEnabled() { if err := httpsec.ProtectRoundTrip(ctx, r2.URL.String()); err != nil { return nil, err } diff --git a/contrib/net/http/roundtripper_test.go b/contrib/net/http/roundtripper_test.go index 0f9a226733..dd7c9301be 100644 --- a/contrib/net/http/roundtripper_test.go +++ b/contrib/net/http/roundtripper_test.go @@ -632,49 +632,68 @@ func (rt *emptyRoundTripper) RoundTrip(_ *http.Request) (*http.Response, error) } func TestAppsec(t *testing.T) { - mt := mocktracer.Start() - defer mt.Stop() - t.Setenv("DD_APPSEC_RULES", "../../../internal/appsec/testdata/rasp.json") - appsec.Start() - if !appsec.Enabled() { - t.Skip("appsec not enabled") - } - client := WrapRoundTripper(&emptyRoundTripper{}) - t.Run("event", func(t *testing.T) { - w := httptest.NewRecorder() - r, err := http.NewRequest("GET", "?value=169.254.169.254", nil) - require.NoError(t, err) + for _, enabled := range []bool{true, false} { - TraceAndServe(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - req, err := http.NewRequest("GET", "http://169.254.169.254", nil) - require.NoError(t, err) + t.Run(strconv.FormatBool(enabled), func(t *testing.T) { + t.Setenv("DD_APPSEC_RASP_ENABLED", strconv.FormatBool(enabled)) - resp, err := client.RoundTrip(req.WithContext(r.Context())) - require.ErrorIs(t, err, &events.BlockingSecurityEvent{}) - if resp != nil { - defer resp.Body.Close() + mt := mocktracer.Start() + defer mt.Stop() + + appsec.Start() + if !appsec.Enabled() { + t.Skip("appsec not enabled") } - }), w, r, &ServeConfig{ - Service: "service", - Resource: "resource", - }) - spans := mt.FinishedSpans() - require.Len(t, spans, 2) // service entry serviceSpan & http request serviceSpan - serviceSpan := spans[1] + defer appsec.Stop() - require.Contains(t, serviceSpan.Tags(), "_dd.appsec.json") - appsecJSON := serviceSpan.Tag("_dd.appsec.json") - require.Contains(t, appsecJSON, httpsec.ServerIoNetURLAddr) + w := httptest.NewRecorder() + r, err := http.NewRequest("GET", "?value=169.254.169.254", nil) + require.NoError(t, err) - require.Contains(t, serviceSpan.Tags(), "_dd.stack") + TraceAndServe(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + req, err := http.NewRequest("GET", "http://169.254.169.254", nil) + require.NoError(t, err) + + resp, err := client.RoundTrip(req.WithContext(r.Context())) + + if enabled { + require.ErrorIs(t, err, &events.BlockingSecurityEvent{}) + } else { + require.NoError(t, err) + } + + if resp != nil { + defer resp.Body.Close() + } + }), w, r, &ServeConfig{ + Service: "service", + Resource: "resource", + }) + + spans := mt.FinishedSpans() + require.Len(t, spans, 2) // service entry serviceSpan & http request serviceSpan + serviceSpan := spans[1] + + if !enabled { + require.NotContains(t, serviceSpan.Tags(), "_dd.appsec.json") + require.NotContains(t, serviceSpan.Tags(), "_dd.stack") + return + } - // This is a nested event so it should contain the child span id in the service entry span - // TODO(eliott.bouhana): uncomment this once we have the child span id in the service entry span - // require.Contains(t, appsecJSON, `"span_id":`+strconv.FormatUint(requestSpan.SpanID(), 10)) - }) + require.Contains(t, serviceSpan.Tags(), "_dd.appsec.json") + appsecJSON := serviceSpan.Tag("_dd.appsec.json") + require.Contains(t, appsecJSON, httpsec.ServerIoNetURLAddr) + + require.Contains(t, serviceSpan.Tags(), "_dd.stack") + + // This is a nested event so it should contain the child span id in the service entry span + // TODO(eliott.bouhana): uncomment this once we have the child span id in the service entry span + // require.Contains(t, appsecJSON, `"span_id":`+strconv.FormatUint(requestSpan.SpanID(), 10)) + }) + } } diff --git a/internal/appsec/appsec.go b/internal/appsec/appsec.go index 77ab9db35e..ae68d0d668 100644 --- a/internal/appsec/appsec.go +++ b/internal/appsec/appsec.go @@ -26,6 +26,13 @@ func Enabled() bool { return activeAppSec != nil && activeAppSec.started } +// RASPEnabled returns true when DD_APPSEC_RASP_ENABLED=true or is unset. Granted that AppSec is enabled. +func RASPEnabled() bool { + mu.RLock() + defer mu.RUnlock() + return activeAppSec != nil && activeAppSec.started && activeAppSec.cfg.RASP +} + // 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 ...config.StartOption) { diff --git a/internal/appsec/config/config.go b/internal/appsec/config/config.go index 90f077bc10..e2a0b7736a 100644 --- a/internal/appsec/config/config.go +++ b/internal/appsec/config/config.go @@ -11,9 +11,8 @@ import ( "strconv" "time" - appsecInternal "github.com/DataDog/appsec-internal-go/appsec" + internal "github.com/DataDog/appsec-internal-go/appsec" - "gopkg.in/DataDog/dd-trace-go.v1/internal" "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" @@ -63,9 +62,9 @@ type Config struct { // AppSec trace rate limit (traces per second). TraceRateLimit int64 // Obfuscator configuration - Obfuscator appsecInternal.ObfuscatorConfig + Obfuscator internal.ObfuscatorConfig // APISec configuration - APISec appsecInternal.APISecConfig + APISec internal.APISecConfig // RC is the remote configuration client used to receive product configuration updates. Nil if RC is disabled (default) RC *remoteconfig.ClientConfig RASP bool @@ -101,7 +100,7 @@ func parseBoolEnvVar(env string) (enabled bool, set bool, err error) { // NewConfig returns a fresh appsec configuration read from the env func NewConfig() (*Config, error) { - rules, err := appsecInternal.RulesFromEnv() + rules, err := internal.RulesFromEnv() if err != nil { return nil, err } @@ -113,10 +112,10 @@ func NewConfig() (*Config, error) { return &Config{ RulesManager: r, - WAFTimeout: appsecInternal.WAFTimeoutFromEnv(), - TraceRateLimit: int64(appsecInternal.RateLimitFromEnv()), - Obfuscator: appsecInternal.NewObfuscatorConfig(), - APISec: appsecInternal.NewAPISecConfig(), - RASP: internal.BoolEnv("DD_APPSEC_RASP_ENABLED", true), + WAFTimeout: internal.WAFTimeoutFromEnv(), + TraceRateLimit: int64(internal.RateLimitFromEnv()), + Obfuscator: internal.NewObfuscatorConfig(), + APISec: internal.NewAPISecConfig(), + RASP: internal.RASPEnabled(), }, nil } diff --git a/internal/appsec/listener/httpsec/roundtripper.go b/internal/appsec/listener/httpsec/roundtripper.go index a0c99f6112..21d77dc599 100644 --- a/internal/appsec/listener/httpsec/roundtripper.go +++ b/internal/appsec/listener/httpsec/roundtripper.go @@ -8,6 +8,7 @@ package httpsec import ( "github.com/DataDog/appsec-internal-go/limiter" "github.com/DataDog/go-libddwaf/v3" + "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/emitter/httpsec/types" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/listener/sharedsec" @@ -17,6 +18,10 @@ import ( // RegisterRoundTripperListener registers a listener on outgoing HTTP client requests to run the WAF. func RegisterRoundTripperListener(op dyngo.Operation, events *trace.SecurityEventsHolder, wafCtx *waf.Context, limiter limiter.Limiter) { + if !appsec.RASPEnabled() { + return + } + dyngo.On(op, func(op *types.RoundTripOperation, args types.RoundTripOperationArgs) { wafResult := sharedsec.RunWAF(wafCtx, waf.RunAddressData{Persistent: map[string]any{ServerIoNetURLAddr: args.URL}}) if !wafResult.HasEvents() {