From 32c4ffdfc8c1064e700f2cf00972a07a44f1df64 Mon Sep 17 00:00:00 2001 From: Eliott Bouhana Date: Fri, 31 May 2024 15:01:46 +0200 Subject: [PATCH] misc suggestions from @JulioGuerra Signed-off-by: Eliott Bouhana --- appsec/events/block.go | 18 ++++++++++-------- .../appsec/emitter/httpsec/roundtripper.go | 2 +- internal/appsec/emitter/httpsec/types/types.go | 3 +++ .../appsec/listener/httpsec/roundtripper.go | 2 +- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/appsec/events/block.go b/appsec/events/block.go index 64458541b7..0d5b518945 100644 --- a/appsec/events/block.go +++ b/appsec/events/block.go @@ -3,18 +3,20 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2022 Datadog, Inc. -// Package events provides the types and interfaces for the appsec event system. -// User-facing events can be returned by the appsec package to signal that a request was blocked. -// Handling these events differently than other errors is crucial to not leak information to an attacker. +// Package events provides security event types that appsec can return in function calls it monitors when blocking them. +// It allows finer-grained integrations of appsec into your Go errors' management logic. package events var _ error = (*BlockingSecurityEvent)(nil) -// BlockingSecurityEvent is an event that signals that a request was blocked by the WAF. -// It should be handled differently than other errors to avoid leaking information to an attacker. -// If this error was returned by native types wrapped by dd-trace-go, it means that a 403 response will be written -// by appsec middleware (or any other status code defined in DataDog's UI). Therefore, the user should not write a -// response in the handler. +// BlockingSecurityEvent is the error type returned by function calls blocked by appsec. +// Even though appsec takes care of responding automatically to the blocked requests, it +// is your duty to abort the request handlers that are calling functions blocked by appsec. +// For instance, if a gRPC handler performs a SQL query blocked by appsec, the SQL query +// function call gets blocked and aborted by returning an error of type SecurityBlockingEvent. +// This allows you to safely abort your request handlers, and to be able to leverage errors.As if +// necessary in your Go error management logic to be able to tell if the error is a blocking security +// event or not (eg. to avoid retrying an HTTP client request). type BlockingSecurityEvent struct{} func (*BlockingSecurityEvent) Error() string { diff --git a/internal/appsec/emitter/httpsec/roundtripper.go b/internal/appsec/emitter/httpsec/roundtripper.go index acd849a0df..072b604a9a 100644 --- a/internal/appsec/emitter/httpsec/roundtripper.go +++ b/internal/appsec/emitter/httpsec/roundtripper.go @@ -26,7 +26,7 @@ func ProtectRoundTrip(ctx context.Context, url string) error { parent, _ := ctx.Value(listener.ContextKey{}).(dyngo.Operation) if parent == nil { // No parent operation => we can't monitor the request badInputContextOnce.Do(func() { - log.Debug("appsec: outgoing http request monitoring ignored: could not find the http handler " + + log.Debug("appsec: outgoing http request monitoring ignored: could not find the handler " + "instrumentation metadata in the request context: the request handler is not being monitored by a " + "middleware function or the incoming request context has not be forwarded correctly to the roundtripper") }) diff --git a/internal/appsec/emitter/httpsec/types/types.go b/internal/appsec/emitter/httpsec/types/types.go index 6f97264cae..2ea8648b7c 100644 --- a/internal/appsec/emitter/httpsec/types/types.go +++ b/internal/appsec/emitter/httpsec/types/types.go @@ -76,10 +76,13 @@ type ( // SDKBodyOperationRes is the SDK body operation results. SDKBodyOperationRes struct{} + // RoundTripOperationArgs is the round trip operation arguments. RoundTripOperationArgs struct { + // URL corresponds to the address `server.io.net.url`. URL string } + // RoundTripOperationRes is the round trip operation results. RoundTripOperationRes struct{} ) diff --git a/internal/appsec/listener/httpsec/roundtripper.go b/internal/appsec/listener/httpsec/roundtripper.go index 93b7867b87..a0c99f6112 100644 --- a/internal/appsec/listener/httpsec/roundtripper.go +++ b/internal/appsec/listener/httpsec/roundtripper.go @@ -15,7 +15,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/log" ) -// RegisterRoundTripperListener registers a listener on outgoing requests to run the WAF. +// 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) { dyngo.On(op, func(op *types.RoundTripOperation, args types.RoundTripOperationArgs) { wafResult := sharedsec.RunWAF(wafCtx, waf.RunAddressData{Persistent: map[string]any{ServerIoNetURLAddr: args.URL}})