From 7281ec4739fbd87879db6847db405c7bf442cc9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Fri, 9 Dec 2022 16:13:08 +0100 Subject: [PATCH] internal/appsec: report private IP if no global IP found --- .../dyngo/instrumentation/httpsec/http.go | 36 +++---------- .../dyngo/instrumentation/httpsec/tags.go | 54 +++++++++++++------ .../instrumentation/httpsec/tags_test.go | 4 +- 3 files changed, 48 insertions(+), 46 deletions(-) diff --git a/internal/appsec/dyngo/instrumentation/httpsec/http.go b/internal/appsec/dyngo/instrumentation/httpsec/http.go index 8d8d3d2759..3ac42de62d 100644 --- a/internal/appsec/dyngo/instrumentation/httpsec/http.go +++ b/internal/appsec/dyngo/instrumentation/httpsec/http.go @@ -12,6 +12,7 @@ package httpsec import ( "context" + // Blank import needed to use embed for the default blocked response payloads _ "embed" "encoding/json" @@ -23,6 +24,7 @@ import ( "sync" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo/instrumentation" "gopkg.in/DataDog/dd-trace-go.v1/internal/log" @@ -143,13 +145,14 @@ func MakeHandlerOperationArgs(r *http.Request, pathParams map[string]string) Han } cookies := makeCookies(r) // TODO(Julio-Guerra): avoid actively parsing the cookies thanks to dynamic instrumentation headers["host"] = []string{r.Host} + ip := IPFromHeaders(r.Header, r.RemoteAddr) return HandlerOperationArgs{ RequestURI: r.RequestURI, Headers: headers, Cookies: cookies, Query: r.URL.Query(), // TODO(Julio-Guerra): avoid actively parsing the query values thanks to dynamic instrumentation PathParams: pathParams, - ClientIP: IPFromHeaders(headers, r.RemoteAddr), + ClientIP: ip, } } @@ -312,35 +315,10 @@ func (f OnSDKBodyOperationFinish) Call(op dyngo.Operation, v interface{}) { f(op.(*SDKBodyOperation), v.(SDKBodyOperationRes)) } -// IPFromHeaders returns the resolved client IP for set of normalized headers. The returned IP can be invalid. +// IPFromHeaders tries to resolve and return the user IP from request headers func IPFromHeaders(hdrs map[string][]string, remoteAddr string) instrumentation.NetaddrIP { - ipHeaders := defaultIPHeaders - if len(clientIPHeader) > 0 { - ipHeaders = []string{clientIPHeader} - } - var headers []string - var ips []string - for _, hdr := range ipHeaders { - if v, ok := hdrs[hdr]; ok && len(v) >= 1 { - headers = append(headers, hdr) - // We currently only use the first entry for one specific header - ips = append(ips, v[0]) - } - } - if len(ips) == 0 { - if remoteIP := parseIP(remoteAddr); remoteIP.IsValid() && isGlobal(remoteIP) { - return remoteIP - } - } else if len(ips) == 1 { - for _, ipstr := range strings.Split(ips[0], ",") { - ip := parseIP(strings.TrimSpace(ipstr)) - if ip.IsValid() && isGlobal(ip) { - return ip - } - } - } - - return instrumentation.NetaddrIP{} + ip := IPTagsFromHeaders(hdrs, remoteAddr)[ext.HTTPClientIP] + return parseIP(ip) } // blockedTemplateJSON is the default JSON template used to write responses for blocked requests diff --git a/internal/appsec/dyngo/instrumentation/httpsec/tags.go b/internal/appsec/dyngo/instrumentation/httpsec/tags.go index 65dd5c4b5f..d010a15722 100644 --- a/internal/appsec/dyngo/instrumentation/httpsec/tags.go +++ b/internal/appsec/dyngo/instrumentation/httpsec/tags.go @@ -106,42 +106,66 @@ func ippref(s string) *instrumentation.NetaddrIPPrefix { } // SetIPTags sets the IP related span tags for a given request +func SetIPTags(s instrumentation.TagSetter, r *http.Request) { + for k, v := range IPTagsFromHeaders(r.Header, r.RemoteAddr) { + s.SetTag(k, v) + } + +} + +// IPTagsFromHeaders generates the IP related span tags for a given request's headers // See https://docs.datadoghq.com/tracing/configure_data_security#configuring-a-client-ip-header for more information. -func SetIPTags(span instrumentation.TagSetter, r *http.Request) { +func IPTagsFromHeaders(hdrs map[string][]string, remoteAddr string) map[string]string { + tags := map[string]string{} ipHeaders := defaultIPHeaders if len(clientIPHeader) > 0 { ipHeaders = []string{clientIPHeader} } - var ( headers []string ips []string ) + + // Make sure all headers are lower-case + for k, v := range hdrs { + hdrs[strings.ToLower(k)] = v + } for _, hdr := range ipHeaders { - if v := r.Header.Get(hdr); v != "" { + if v := hdrs[hdr]; len(v) >= 1 && v[0] != "" { headers = append(headers, hdr) - ips = append(ips, v) + ips = append(ips, v[0]) } } - if l := len(ips); l == 0 { - if remoteIP := parseIP(r.RemoteAddr); remoteIP.IsValid() && isGlobal(remoteIP) { - span.SetTag(ext.HTTPClientIP, remoteIP.String()) - } - } else if l == 1 { + var privateIP instrumentation.NetaddrIP + // First try to use the IP from the headers if only one IP was found + if l := len(ips); l == 1 { for _, ipstr := range strings.Split(ips[0], ",") { ip := parseIP(strings.TrimSpace(ipstr)) - if ip.IsValid() && isGlobal(ip) { - span.SetTag(ext.HTTPClientIP, ip.String()) - break + if ip.IsValid() { + if isGlobal(ip) { + tags[ext.HTTPClientIP] = ip.String() + return tags + } else if !privateIP.IsValid() { + privateIP = ip + } } } - } else { + } else if l > 1 { // If more than one IP header, report them for i := range ips { - span.SetTag(ext.HTTPRequestHeaders+"."+headers[i], ips[i]) + tags[ext.HTTPRequestHeaders+"."+headers[i]] = ips[i] } - span.SetTag(tagMultipleIPHeaders, strings.Join(headers, ",")) + tags[tagMultipleIPHeaders] = strings.Join(headers, ",") } + + // Try to get a global IP from remoteAddr. If not, try to use any private IP we found + if remoteIP := parseIP(remoteAddr); remoteIP.IsValid() && (isGlobal(remoteIP) || !privateIP.IsValid()) { + tags[ext.HTTPClientIP] = remoteIP.String() + } else if privateIP.IsValid() { + tags[ext.HTTPClientIP] = privateIP.String() + } + + return tags } func parseIP(s string) instrumentation.NetaddrIP { diff --git a/internal/appsec/dyngo/instrumentation/httpsec/tags_test.go b/internal/appsec/dyngo/instrumentation/httpsec/tags_test.go index 06f0c7868c..9992a4448a 100644 --- a/internal/appsec/dyngo/instrumentation/httpsec/tags_test.go +++ b/internal/appsec/dyngo/instrumentation/httpsec/tags_test.go @@ -80,7 +80,7 @@ func genIPTestCases() []ipTestCase { tcs = append(tcs, ipTestCase{ name: "ipv4-private." + header, headers: map[string]string{header: ipv4Private}, - expectedIP: instrumentation.NetaddrIP{}, + expectedIP: instrumentation.NetaddrMustParseIP(ipv4Private), }) } // Simple ipv6 test cases over all headers @@ -93,7 +93,7 @@ func genIPTestCases() []ipTestCase { tcs = append(tcs, ipTestCase{ name: "ipv6-private." + header, headers: map[string]string{header: ipv6Private}, - expectedIP: instrumentation.NetaddrIP{}, + expectedIP: instrumentation.NetaddrMustParseIP(ipv6Private), }) } // private and global in same header