diff --git a/contrib/internal/httptrace/httptrace.go b/contrib/internal/httptrace/httptrace.go index 0017159f62..92069e03d8 100644 --- a/contrib/internal/httptrace/httptrace.go +++ b/contrib/internal/httptrace/httptrace.go @@ -11,7 +11,6 @@ import ( "context" "fmt" "net/http" - "net/url" "strconv" "strings" @@ -28,29 +27,14 @@ var ( ) // StartRequestSpan starts an HTTP request span with the standard list of HTTP request span tags (http.method, http.url, -// http.useragent) with a http.Request object. Any further span start option can be added with opts. -func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer.Span, context.Context) { - return StartHttpSpan( - r.Context(), - r.Header, - r.Host, - r.Method, - urlFromRequest(r), - r.UserAgent(), - r.RemoteAddr, - opts..., - ) -} - -// StartHttpSpan starts an HTTP request span with the standard list of HTTP request span tags (http.method, http.url, // http.useragent). Any further span start option can be added with opts. -func StartHttpSpan(ctx context.Context, headers map[string][]string, host string, method string, url string, userAgent string, remoteAddr string, opts ...ddtrace.StartSpanOption) (tracer.Span, context.Context) { +func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer.Span, context.Context) { // Append our span options before the given ones so that the caller can "overwrite" them. // TODO(): rework span start option handling (https://github.com/DataDog/dd-trace-go/issues/1352) var ipTags map[string]string if cfg.traceClientIP { - ipTags, _ = httpsec.ClientIPTags(headers, true, remoteAddr) + ipTags, _ = httpsec.ClientIPTags(r.Header, true, r.RemoteAddr) } nopts := make([]ddtrace.StartSpanOption, 0, len(opts)+1+len(ipTags)) nopts = append(nopts, @@ -59,23 +43,22 @@ func StartHttpSpan(ctx context.Context, headers map[string][]string, host string cfg.Tags = make(map[string]interface{}) } cfg.Tags[ext.SpanType] = ext.SpanTypeWeb - cfg.Tags[ext.HTTPMethod] = method - cfg.Tags[ext.HTTPURL] = url - cfg.Tags[ext.HTTPUserAgent] = userAgent + cfg.Tags[ext.HTTPMethod] = r.Method + cfg.Tags[ext.HTTPURL] = urlFromRequest(r) + cfg.Tags[ext.HTTPUserAgent] = r.UserAgent() cfg.Tags["_dd.measured"] = 1 - if host != "" { - cfg.Tags["http.host"] = host + if r.Host != "" { + cfg.Tags["http.host"] = r.Host } - if spanctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(headers)); err == nil { + if spanctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(r.Header)); err == nil { cfg.Parent = spanctx } - for k, v := range ipTags { cfg.Tags[k] = v } }) nopts = append(nopts, opts...) - return tracer.StartSpanFromContext(ctx, namingschema.OpName(namingschema.HTTPServer), nopts...) + return tracer.StartSpanFromContext(r.Context(), namingschema.OpName(namingschema.HTTPServer), nopts...) } // FinishRequestSpan finishes the given HTTP request span and sets the expected response-related tags such as the status @@ -114,54 +97,27 @@ func urlFromRequest(r *http.Request) string { // "For most requests, fields other than Path and RawQuery will be // empty. (See RFC 7230, Section 5.3)" // This is why we don't rely on url.URL.String(), url.URL.Host, url.URL.Scheme, etc... + var url string + path := r.URL.EscapedPath() scheme := "http" if r.TLS != nil { scheme = "https" } - - return urlFromArgs( - r.URL.EscapedPath(), - scheme, - r.Host, - r.URL.RawQuery, - r.URL.EscapedFragment(), - ) -} - -// UrlFromUrl returns the full URL from a URL object. If query params are collected, they are obfuscated granted -// obfuscation is not disabled by the user (through DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP) -// See https://docs.datadoghq.com/tracing/configure_data_security#redacting-the-query-in-the-url for more information. -func UrlFromUrl(u *url.URL) string { - scheme := "http" - if u.Scheme != "" { - scheme = u.Scheme - } - return urlFromArgs( - u.EscapedPath(), - scheme, - u.Host, - u.RawQuery, - u.EscapedFragment(), - ) -} - -func urlFromArgs(escapedPath string, scheme string, host string, rawQuery string, escapedFragment string) string { - var url string - if host != "" { - url = strings.Join([]string{scheme, "://", host, escapedPath}, "") + if r.Host != "" { + url = strings.Join([]string{scheme, "://", r.Host, path}, "") } else { - url = escapedPath + url = path } // Collect the query string if we are allowed to report it and obfuscate it if possible/allowed - if cfg.queryString && rawQuery != "" { - query := rawQuery + if cfg.queryString && r.URL.RawQuery != "" { + query := r.URL.RawQuery if cfg.queryStringRegexp != nil { query = cfg.queryStringRegexp.ReplaceAllLiteralString(query, "") } url = strings.Join([]string{url, query}, "?") } - if escapedFragment != "" { - url = strings.Join([]string{url, escapedFragment}, "#") + if frag := r.URL.EscapedFragment(); frag != "" { + url = strings.Join([]string{url, frag}, "#") } return url } diff --git a/internal/appsec/emitter/httpsec/http.go b/internal/appsec/emitter/httpsec/http.go index 1884549e60..41ebfa7e23 100644 --- a/internal/appsec/emitter/httpsec/http.go +++ b/internal/appsec/emitter/httpsec/http.go @@ -12,12 +12,9 @@ package httpsec import ( "context" - "strings" - // Blank import needed to use embed for the default blocked response payloads _ "embed" "net/http" - "net/url" "sync/atomic" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" @@ -200,47 +197,3 @@ func WrapHandler(handler http.Handler, span ddtrace.Span, pathParams map[string] handler.ServeHTTP(tw, tr) }) } - -// MakeHandlerOperationArgs creates the HandlerOperationArgs value. -func MakeHandlerOperationArgs(headers map[string][]string, method string, host string, remoteAddr string, url *url.URL) HandlerOperationArgs { - cookies := filterCookiesFromHeaders(headers) - - args := HandlerOperationArgs{ - Method: method, - RequestURI: url.RequestURI(), - Host: host, - RemoteAddr: remoteAddr, - Headers: headers, - Cookies: cookies, - QueryParams: url.Query(), - PathParams: map[string]string{}, - } - - args.Headers["host"] = []string{host} - return args -} - -// Separate the cookies from the headers, return the parsed cookies and remove in place the cookies from the headers. -// Headers used for `server.request.headers.no_cookies` and `server.response.headers.no_cookies` addresses for the WAF -// Cookies are used for the `server.request.cookies` address -func filterCookiesFromHeaders(headers http.Header) map[string][]string { - cookieHeader, ok := headers["Cookie"] - if !ok { - return make(http.Header) - } - - delete(headers, "Cookie") - - cookies := make(map[string][]string, len(cookieHeader)) - for _, c := range cookieHeader { - parts := strings.Split(c, ";") - for _, part := range parts { - cookie := strings.Split(part, "=") - if len(cookie) == 2 { - cookies[cookie[0]] = append(cookies[cookie[0]], cookie[1]) - } - } - } - - return cookies -} diff --git a/internal/appsec/emitter/waf/actions/block.go b/internal/appsec/emitter/waf/actions/block.go index 40418f4410..ae802b60bd 100644 --- a/internal/appsec/emitter/waf/actions/block.go +++ b/internal/appsec/emitter/waf/actions/block.go @@ -69,11 +69,6 @@ type ( // BlockHTTP are actions that interact with an HTTP request flow BlockHTTP struct { http.Handler - - StatusCode int `mapstructure:"status_code"` - RedirectLocation string - // BlockingTemplate is a function that returns the headers to be added and body to be written to the response - BlockingTemplate func(headers map[string][]string) (map[string][]string, []byte) } ) @@ -130,11 +125,7 @@ func NewBlockAction(params map[string]any) []Action { } func newHTTPBlockRequestAction(status int, template string) *BlockHTTP { - return &BlockHTTP{ - Handler: newBlockHandler(status, template), - BlockingTemplate: newManualBlockHandler(template), - StatusCode: status, - } + return &BlockHTTP{Handler: newBlockHandler(status, template)} } // newBlockHandler creates, initializes and returns a new BlockRequestAction @@ -148,47 +139,16 @@ func newBlockHandler(status int, template string) http.Handler { return htmlHandler default: return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - h := findCorrectTemplate(jsonHandler, htmlHandler, r.Header.Get("Accept")) - h.(http.Handler).ServeHTTP(w, r) - }) - } -} - -func newManualBlockHandler(template string) func(headers map[string][]string) (map[string][]string, []byte) { - htmlHandler := newManualBlockDataHandler("text/html", blockedTemplateHTML) - jsonHandler := newManualBlockDataHandler("application/json", blockedTemplateJSON) - switch template { - case "json": - return jsonHandler - case "html": - return htmlHandler - default: - return func(headers map[string][]string) (map[string][]string, []byte) { - acceptHeader := "" - if hdr, ok := headers["Accept"]; ok && len(hdr) > 0 { - acceptHeader = hdr[0] + h := jsonHandler + hdr := r.Header.Get("Accept") + htmlIdx := strings.Index(hdr, "text/html") + jsonIdx := strings.Index(hdr, "application/json") + // Switch to html handler if text/html comes before application/json in the Accept header + if htmlIdx != -1 && (jsonIdx == -1 || htmlIdx < jsonIdx) { + h = htmlHandler } - h := findCorrectTemplate(jsonHandler, htmlHandler, acceptHeader) - return h.(func(headers map[string][]string) (map[string][]string, []byte))(headers) - } - } -} - -func findCorrectTemplate(jsonHandler interface{}, htmlHandler interface{}, acceptHeader string) interface{} { - h := jsonHandler - hdr := acceptHeader - htmlIdx := strings.Index(hdr, "text/html") - jsonIdx := strings.Index(hdr, "application/json") - // Switch to html handler if text/html comes before application/json in the Accept header - if htmlIdx != -1 && (jsonIdx == -1 || htmlIdx < jsonIdx) { - h = htmlHandler - } - return h -} - -func newManualBlockDataHandler(ct string, template []byte) func(headers map[string][]string) (map[string][]string, []byte) { - return func(headers map[string][]string) (map[string][]string, []byte) { - return map[string][]string{"Content-Type": {ct}}, template + h.ServeHTTP(w, r) + }) } } diff --git a/internal/appsec/emitter/waf/actions/http_redirect.go b/internal/appsec/emitter/waf/actions/http_redirect.go index 25e209d790..3cdca4c818 100644 --- a/internal/appsec/emitter/waf/actions/http_redirect.go +++ b/internal/appsec/emitter/waf/actions/http_redirect.go @@ -7,14 +7,10 @@ package actions import ( "net/http" - "path" - "strings" "github.com/mitchellh/mapstructure" "gopkg.in/DataDog/dd-trace-go.v1/internal/log" - - urlpkg "net/url" ) // redirectActionParams are the dynamic parameters to be provided to a "redirect_request" @@ -42,17 +38,9 @@ func newRedirectRequestAction(status int, loc string) *BlockHTTP { // If location is not set we fall back on a default block action if loc == "" { - return &BlockHTTP{ - Handler: newBlockHandler(http.StatusForbidden, string(blockedTemplateJSON)), - StatusCode: status, - BlockingTemplate: newManualBlockHandler("json"), - } - } - return &BlockHTTP{ - Handler: http.RedirectHandler(loc, status), - StatusCode: status, - RedirectLocation: loc, + return &BlockHTTP{Handler: newBlockHandler(http.StatusForbidden, string(blockedTemplateJSON))} } + return &BlockHTTP{Handler: http.RedirectHandler(loc, status)} } // NewRedirectAction creates an action for the "redirect_request" action type @@ -64,75 +52,3 @@ func NewRedirectAction(params map[string]any) []Action { } return []Action{newRedirectRequestAction(p.StatusCode, p.Location)} } - -// HandleRedirectLocationString returns the headers and body to be written to the response when a redirect is needed -// Vendored from net/http/server.go -func HandleRedirectLocationString(oldpath string, url string, statusCode int, method string, h map[string][]string) (map[string][]string, []byte) { - if u, err := urlpkg.Parse(url); err == nil { - // If url was relative, make its path absolute by - // combining with request path. - // The client would probably do this for us, - // but doing it ourselves is more reliable. - // See RFC 7231, section 7.1.2 - if u.Scheme == "" && u.Host == "" { - if oldpath == "" { // should not happen, but avoid a crash if it does - oldpath = "/" - } - - // no leading http://server - if url == "" || url[0] != '/' { - // make relative path absolute - olddir, _ := path.Split(oldpath) - url = olddir + url - } - - var query string - if i := strings.Index(url, "?"); i != -1 { - url, query = url[:i], url[i:] - } - - // clean up but preserve trailing slash - trailing := strings.HasSuffix(url, "/") - url = path.Clean(url) - if trailing && !strings.HasSuffix(url, "/") { - url += "/" - } - url += query - } - } - - // RFC 7231 notes that a short HTML body is usually included in - // the response because older user agents may not understand 301/307. - // Do it only if the request didn't already have a Content-Type header. - _, hadCT := h["content-type"] - newHeaders := make(map[string][]string, 2) - - newHeaders["location"] = []string{url} - if !hadCT && (method == "GET" || method == "HEAD") { - newHeaders["content-length"] = []string{"text/html; charset=utf-8"} - } - - // Shouldn't send the body for POST or HEAD; that leaves GET. - var body []byte - if !hadCT && method == "GET" { - body = []byte("" + http.StatusText(statusCode) + ".\n") - } - - return newHeaders, body -} - -// Vendored from net/http/server.go -var htmlReplacer = strings.NewReplacer( - "&", "&", - "<", "<", - ">", ">", - // """ is shorter than """. - `"`, """, - // "'" is shorter than "'" and apos was not in HTML until HTML5. - "'", "'", -) - -// htmlEscape escapes special characters like "<" to become "<". -func htmlEscape(s string) string { - return htmlReplacer.Replace(s) -} diff --git a/internal/appsec/listener/httpsec/http.go b/internal/appsec/listener/httpsec/http.go index 804d115fcd..08b9e853dd 100644 --- a/internal/appsec/listener/httpsec/http.go +++ b/internal/appsec/listener/httpsec/http.go @@ -59,7 +59,7 @@ func (feature *Feature) OnRequest(op *httpsec.HandlerOperation, args httpsec.Han headers := headersRemoveCookies(args.Headers) headers["host"] = []string{args.Host} - SetRequestHeadersTags(op, headers) + setRequestHeadersTags(op, headers) op.Run(op, addresses.NewAddressesBuilder(). @@ -76,7 +76,7 @@ func (feature *Feature) OnRequest(op *httpsec.HandlerOperation, args httpsec.Han func (feature *Feature) OnResponse(op *httpsec.HandlerOperation, resp httpsec.HandlerOperationRes) { headers := headersRemoveCookies(resp.Headers) - SetResponseHeadersTags(op, headers) + setResponseHeadersTags(op, headers) builder := addresses.NewAddressesBuilder(). WithResponseHeadersNoCookies(headers). diff --git a/internal/appsec/listener/httpsec/request.go b/internal/appsec/listener/httpsec/request.go index cb181be81e..abd3983183 100644 --- a/internal/appsec/listener/httpsec/request.go +++ b/internal/appsec/listener/httpsec/request.go @@ -149,13 +149,13 @@ func readMonitoredClientIPHeadersConfig() { } } -// SetRequestHeadersTags sets the AppSec-specific request headers span tags. -func SetRequestHeadersTags(span trace.TagSetter, headers map[string][]string) { +// setRequestHeadersTags sets the AppSec-specific request headers span tags. +func setRequestHeadersTags(span trace.TagSetter, headers map[string][]string) { setHeadersTags(span, "http.request.headers.", headers) } -// SetResponseHeadersTags sets the AppSec-specific response headers span tags. -func SetResponseHeadersTags(span trace.TagSetter, headers map[string][]string) { +// setResponseHeadersTags sets the AppSec-specific response headers span tags. +func setResponseHeadersTags(span trace.TagSetter, headers map[string][]string) { setHeadersTags(span, "http.response.headers.", headers) } diff --git a/internal/appsec/listener/httpsec/request_test.go b/internal/appsec/listener/httpsec/request_test.go index badac1891c..38052cbb96 100644 --- a/internal/appsec/listener/httpsec/request_test.go +++ b/internal/appsec/listener/httpsec/request_test.go @@ -215,8 +215,8 @@ func TestTags(t *testing.T) { return } require.NoError(t, err) - SetRequestHeadersTags(&span, reqHeadersCase.headers) - SetResponseHeadersTags(&span, respHeadersCase.headers) + setRequestHeadersTags(&span, reqHeadersCase.headers) + setResponseHeadersTags(&span, respHeadersCase.headers) if eventCase.events != nil { require.Subset(t, span.Tags, map[string]interface{}{ diff --git a/internal/appsec/listener/trace/trace.go b/internal/appsec/listener/trace/trace.go index 709ed31ecf..45fb28e99f 100644 --- a/internal/appsec/listener/trace/trace.go +++ b/internal/appsec/listener/trace/trace.go @@ -6,7 +6,6 @@ package trace import ( - "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" "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/trace" @@ -20,12 +19,6 @@ var staticAppsecTags = map[string]any{ "_dd.runtime_family": "go", } -func SetAppsecStaticTags(span ddtrace.Span) { - for key, value := range staticAppsecTags { - span.SetTag(key, value) - } -} - type AppsecSpanTransport struct{} func (*AppsecSpanTransport) String() string {