diff --git a/contrib/gin-gonic/gin/appsec.go b/contrib/gin-gonic/gin/appsec.go index 46dda96700..791462016d 100644 --- a/contrib/gin-gonic/gin/appsec.go +++ b/contrib/gin-gonic/gin/appsec.go @@ -28,7 +28,7 @@ func useAppSec(c *gin.Context, span tracer.Span) { c.Request = r c.Next() }) - httpsec.WrapHandler(httpWrapper, span, params, func() { - c.Abort() + httpsec.WrapHandler(httpWrapper, span, params, &httpsec.Config{ + OnBlock: []func(){func() { c.Abort() }}, }).ServeHTTP(c.Writer, c.Request) } diff --git a/contrib/go-chi/chi.v5/appsec.go b/contrib/go-chi/chi.v5/appsec.go index bf95e49144..4d43e67a23 100644 --- a/contrib/go-chi/chi.v5/appsec.go +++ b/contrib/go-chi/chi.v5/appsec.go @@ -14,10 +14,10 @@ import ( "github.com/go-chi/chi/v5" ) -func withAppsec(next http.Handler, r *http.Request, span tracer.Span, opts ...httpsec.WrapHandlerOption) http.Handler { +func withAppsec(next http.Handler, r *http.Request, span tracer.Span, cfg *httpsec.Config) http.Handler { rctx := chi.RouteContext(r.Context()) if rctx == nil { - return httpsec.WrapHandler(next, span, nil) + return httpsec.WrapHandler(next, span, nil, cfg) } var pathParams map[string]string keys := rctx.URLParams.Keys @@ -28,5 +28,5 @@ func withAppsec(next http.Handler, r *http.Request, span tracer.Span, opts ...ht pathParams[key] = values[i] } } - return httpsec.WrapHandlerOpts(next, span, pathParams, opts...) + return httpsec.WrapHandler(next, span, pathParams, cfg) } diff --git a/contrib/go-chi/chi.v5/chi.go b/contrib/go-chi/chi.v5/chi.go index 4f74f8d9df..8caddf092a 100644 --- a/contrib/go-chi/chi.v5/chi.go +++ b/contrib/go-chi/chi.v5/chi.go @@ -68,7 +68,7 @@ func Middleware(opts ...Option) func(next http.Handler) http.Handler { next := next // avoid modifying the value of next in the outer closure scope if appsec.Enabled() && !cfg.appsecDisabled { - next = withAppsec(next, r, span, cfg.appsecOptions...) + next = withAppsec(next, r, span, &cfg.appsecConfig) // Note that the following response writer passed to the handler // implements the `interface { Status() int }` expected by httpsec. } diff --git a/contrib/go-chi/chi.v5/option.go b/contrib/go-chi/chi.v5/option.go index 6260415c52..e6aa343067 100644 --- a/contrib/go-chi/chi.v5/option.go +++ b/contrib/go-chi/chi.v5/option.go @@ -29,7 +29,7 @@ type config struct { headerTags *internal.LockMap resourceNamer func(r *http.Request) string appsecDisabled bool - appsecOptions []httpsec.WrapHandlerOption + appsecConfig httpsec.Config } // Option represents an option that can be passed to NewRouter. @@ -144,17 +144,21 @@ func WithAppsecDisabled(disabled bool) Option { } } -// WithAppsecOptions allows for specifying options for the AppSec middleware. -func WithAppsecOptions(opts ...httpsec.WrapHandlerOption) Option { +// WithAppsecOnBlock configures callbacks to be invoked when an AppSec blocking decision is made. +// This has no effect if AppSec is not in use. +func WithAppsecOnBlock(fs ...func()) Option { return func(cfg *config) { - cfg.appsecOptions = opts + cfg.appsecConfig.OnBlock = append(cfg.appsecConfig.OnBlock, fs...) } } -// WithResponseHdrFetcher provides a function to fetch the response headers from the http.ResponseWriter. -// This allows for custom implementations as needed if you over-ride the default http.ResponseWriter. -func WithResponseHdrFetcher(f func(http.ResponseWriter) http.Header) httpsec.WrapHandlerOption { - return func(cfg *httpsec.WrapHandlerCfg) { - cfg.ResponseHdrFetcher = f +// WithResponseHeaderCopier provides a function to fetch the response headers from the +// http.ResponseWriter. This allows for custom implementations as needed if you over-ride the +// default http.ResponseWriter, such as to add synchronization. Provided functions may elect to +// return a copy of the http.Header map instead of a reference to the original (e.g: to not risk +// breaking synchronization). +func WithResponseHeaderCopier(f func(http.ResponseWriter) http.Header) Option { + return func(cfg *config) { + cfg.appsecConfig.ResponseHeaderCopier = f } } diff --git a/contrib/go-chi/chi/appsec.go b/contrib/go-chi/chi/appsec.go index ee40901c6e..066b6b176f 100644 --- a/contrib/go-chi/chi/appsec.go +++ b/contrib/go-chi/chi/appsec.go @@ -17,7 +17,7 @@ import ( func withAppsec(next http.Handler, r *http.Request, span tracer.Span) http.Handler { rctx := chi.RouteContext(r.Context()) if rctx == nil { - return httpsec.WrapHandler(next, span, nil) + return httpsec.WrapHandler(next, span, nil, nil) } var pathParams map[string]string keys := rctx.URLParams.Keys @@ -28,5 +28,5 @@ func withAppsec(next http.Handler, r *http.Request, span tracer.Span) http.Handl pathParams[key] = values[i] } } - return httpsec.WrapHandler(next, span, pathParams) + return httpsec.WrapHandler(next, span, pathParams, nil) } diff --git a/contrib/labstack/echo.v4/appsec.go b/contrib/labstack/echo.v4/appsec.go index a241c40d37..2f820551cc 100644 --- a/contrib/labstack/echo.v4/appsec.go +++ b/contrib/labstack/echo.v4/appsec.go @@ -32,7 +32,7 @@ func withAppSec(next echo.HandlerFunc, span tracer.Span) echo.HandlerFunc { } }) // Wrap the echo response to allow monitoring of the response status code in httpsec.WrapHandler() - httpsec.WrapHandler(handler, span, params).ServeHTTP(&statusResponseWriter{Response: c.Response()}, c.Request()) + httpsec.WrapHandler(handler, span, params, nil).ServeHTTP(&statusResponseWriter{Response: c.Response()}, c.Request()) // If an error occurred, wrap it under an echo.HTTPError. We need to do this so that APM doesn't override // the response code tag with 500 in case it doesn't recognize the error type. if _, ok := err.(*echo.HTTPError); !ok && err != nil { diff --git a/contrib/net/http/trace.go b/contrib/net/http/trace.go index bc5d8c6811..a019f5819e 100644 --- a/contrib/net/http/trace.go +++ b/contrib/net/http/trace.go @@ -71,7 +71,7 @@ func TraceAndServe(h http.Handler, w http.ResponseWriter, r *http.Request, cfg * }() if appsec.Enabled() { - h = httpsec.WrapHandler(h, span, cfg.RouteParams) + h = httpsec.WrapHandler(h, span, cfg.RouteParams, nil) } h.ServeHTTP(rw, r.WithContext(ctx)) } diff --git a/internal/appsec/emitter/httpsec/config.go b/internal/appsec/emitter/httpsec/config.go new file mode 100644 index 0000000000..9c64019e89 --- /dev/null +++ b/internal/appsec/emitter/httpsec/config.go @@ -0,0 +1,19 @@ +package httpsec + +import "net/http" + +type Config struct { + // OnBlock is a list of callbacks to be invoked when a block decision is made. + OnBlock []func() + // ResponseHeaderCopier provides a way to access response headers for reading + // purposes (the value may be provided by copy). This allows customers to + // apply synchronization if they allow http.ResponseWriter objects to be + // accessed by multiple goroutines. + ResponseHeaderCopier func(http.ResponseWriter) http.Header +} + +var defaultWrapHandlerConfig = &Config{ + ResponseHeaderCopier: func(w http.ResponseWriter) http.Header { + return w.Header() + }, +} diff --git a/internal/appsec/emitter/httpsec/http.go b/internal/appsec/emitter/httpsec/http.go index 2d718cebbb..d787141c17 100644 --- a/internal/appsec/emitter/httpsec/http.go +++ b/internal/appsec/emitter/httpsec/http.go @@ -55,11 +55,18 @@ func ExecuteSDKBodyOperation(parent dyngo.Operation, args types.SDKBodyOperation return err } -// WrapHandlerOpts is WrapHandler with support for flexible options. -func WrapHandlerOpts(handler http.Handler, span ddtrace.Span, pathParams map[string]string, opts ...WrapHandlerOption) http.Handler { - cfg := defaultWrapHandlerCfg() - for _, opt := range opts { - opt(cfg) +// WrapHandler wraps the given HTTP handler with the abstract HTTP operation defined by HandlerOperationArgs and +// HandlerOperationRes. +// The onBlock params are used to cleanup the context when needed. +// It is a specific patch meant for Gin, for which we must abort the +// context since it uses a queue of handlers and it's the only way to make +// sure other queued handlers don't get executed. +// TODO: this patch must be removed/improved when we rework our actions/operations system +func WrapHandler(handler http.Handler, span ddtrace.Span, pathParams map[string]string, opts *Config) http.Handler { + if opts == nil { + opts = defaultWrapHandlerConfig + } else if opts.ResponseHeaderCopier == nil { + opts.ResponseHeaderCopier = defaultWrapHandlerConfig.ResponseHeaderCopier } trace.SetAppSecEnabledTags(span) @@ -86,7 +93,7 @@ func WrapHandlerOpts(handler http.Handler, span ddtrace.Span, pathParams map[str // in case we are instrumenting the Gin framework if blocking { op.SetTag(trace.BlockedRequestTag, true) - for _, f := range cfg.OnBlock { + for _, f := range opts.OnBlock { f() } } @@ -99,7 +106,7 @@ func WrapHandlerOpts(handler http.Handler, span ddtrace.Span, pathParams map[str // extra headers have been added such as the Host header which is removed from the original Go request headers // map setRequestHeadersTags(span, args.Headers) - setResponseHeadersTags(span, cfg.ResponseHdrFetcher(w)) + setResponseHeadersTags(span, opts.ResponseHeaderCopier(w)) trace.SetTags(span, op.Tags()) if len(events) > 0 { httptrace.SetSecurityEventsTags(span, events) @@ -114,17 +121,6 @@ func WrapHandlerOpts(handler http.Handler, span ddtrace.Span, pathParams map[str }) } -// WrapHandler wraps the given HTTP handler with the abstract HTTP operation defined by HandlerOperationArgs and -// HandlerOperationRes. -// The onBlock params are used to cleanup the context when needed. -// It is a specific patch meant for Gin, for which we must abort the -// context since it uses a queue of handlers and it's the only way to make -// sure other queued handlers don't get executed. -// TODO: this patch must be removed/improved when we rework our actions/operations system -func WrapHandler(handler http.Handler, span ddtrace.Span, pathParams map[string]string, onBlock ...func()) http.Handler { - return WrapHandlerOpts(handler, span, pathParams, WithOnBlock(onBlock...)) -} - // MakeHandlerOperationArgs creates the HandlerOperationArgs value. func MakeHandlerOperationArgs(r *http.Request, clientIP netip.Addr, pathParams map[string]string) types.HandlerOperationArgs { cookies := makeCookies(r) // TODO(Julio-Guerra): avoid actively parsing the cookies thanks to dynamic instrumentation diff --git a/internal/appsec/emitter/httpsec/options.go b/internal/appsec/emitter/httpsec/options.go deleted file mode 100644 index 097e0ad43c..0000000000 --- a/internal/appsec/emitter/httpsec/options.go +++ /dev/null @@ -1,24 +0,0 @@ -package httpsec - -import "net/http" - -type WrapHandlerCfg struct { - OnBlock []func() - ResponseHdrFetcher func(http.ResponseWriter) http.Header -} -type WrapHandlerOption func(*WrapHandlerCfg) - -func defaultWrapHandlerCfg() *WrapHandlerCfg { - return &WrapHandlerCfg{ - OnBlock: []func(){}, - ResponseHdrFetcher: func(w http.ResponseWriter) http.Header { - return w.Header() - }, - } -} - -func WithOnBlock(f ...func()) WrapHandlerOption { - return func(cfg *WrapHandlerCfg) { - cfg.OnBlock = append(cfg.OnBlock, f...) - } -} diff --git a/internal/version/version.go b/internal/version/version.go index e469ae1fc8..3a33490687 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -13,7 +13,7 @@ import ( // Tag specifies the current release tag. It needs to be manually // updated. A test checks that the value of Tag never points to a // git tag that is older than HEAD. -const Tag = "v1.62.0-rc.1" +const Tag = "v1.62.0-dev" // Dissected version number. Filled during init() var (