internal/appsec: report private IP if no global IP found

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

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 {

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