Use direct peer socket address

context.request.socket.remote_address should
have the direct peer socket (IP) address, not
the originating client address. This means we
should not parse Forwarded, X-Forwarded-For,
etc. in order to determine this field's value.
That should instead be left to the server.

context.go

@@ -160,14 +160,9 @@ func (c *Context) SetHTTPRequest(req *http.Request) {
 		httpVersion = fmt.Sprintf("%d.%d", req.ProtoMajor, req.ProtoMinor)
 	}
 
-	var forwarded *apmhttputil.ForwardedHeader
-	if fwd := req.Header.Get("Forwarded"); fwd != "" {
-		parsed := apmhttputil.ParseForwarded(fwd)
-		forwarded = &parsed
-	}
 	c.request = model.Request{
 		Body:        c.request.Body,
-		URL:         apmhttputil.RequestURL(req, forwarded),
+		URL:         apmhttputil.RequestURL(req),
 		Method:      truncateString(req.Method),
 		HTTPVersion: httpVersion,
 		Cookies:     req.Cookies(),
@@ -188,7 +183,7 @@ func (c *Context) SetHTTPRequest(req *http.Request) {
 
 	c.requestSocket = model.RequestSocket{
 		Encrypted:     req.TLS != nil,
-		RemoteAddress: apmhttputil.RemoteAddr(req, forwarded),
+		RemoteAddress: apmhttputil.RemoteAddr(req),
 	}
 	if c.requestSocket != (model.RequestSocket{}) {
 		c.request.Socket = &c.requestSocket

internal/apmhttputil/remoteaddr.go

@@ -18,40 +18,11 @@
 package apmhttputil
 
 import (
-	"net"
 	"net/http"
-	"strings"
 )
 
-// RemoteAddr returns the remote address for the HTTP request.
-//
-// In order:
-//  - if the Forwarded header is set, then the first item in the
-//    list's "for" field is used, if it exists. The "for" value
-//    is returned even if it is an obfuscated identifier.
-//  - if the X-Real-Ip header is set, then its value is returned.
-//  - if the X-Forwarded-For header is set, then the first value
-//    in the comma-separated list is returned.
-//  - otherwise, the host portion of req.RemoteAddr is returned.
-func RemoteAddr(req *http.Request, forwarded *ForwardedHeader) string {
-	if forwarded != nil {
-		if forwarded.For != "" {
-			remoteAddr, _, err := net.SplitHostPort(forwarded.For)
-			if err != nil {
-				remoteAddr = forwarded.For
-			}
-			return remoteAddr
-		}
-	}
-	if realIP := req.Header.Get("X-Real-Ip"); realIP != "" {
-		return realIP
-	}
-	if xff := req.Header.Get("X-Forwarded-For"); xff != "" {
-		if sep := strings.IndexRune(xff, ','); sep > 0 {
-			xff = xff[:sep]
-		}
-		return strings.TrimSpace(xff)
-	}
+// RemoteAddr returns the remote (peer) socket address for the HTTP request.
+func RemoteAddr(req *http.Request) string {
 	remoteAddr, _ := splitHost(req.RemoteAddr)
 	return remoteAddr
 }

internal/apmhttputil/remoteaddr_test.go

@@ -31,23 +31,7 @@ func TestRemoteAddr(t *testing.T) {
 		RemoteAddr: "[::1]:1234",
 		Header:     make(http.Header),
 	}
-	assert.Equal(t, "::1", apmhttputil.RemoteAddr(req, nil))
-
-	req.Header.Set("X-Forwarded-For", "client.invalid")
-	assert.Equal(t, "client.invalid", apmhttputil.RemoteAddr(req, nil))
-
 	req.Header.Set("X-Forwarded-For", "client.invalid, proxy.invalid")
-	assert.Equal(t, "client.invalid", apmhttputil.RemoteAddr(req, nil))
-
 	req.Header.Set("X-Real-IP", "127.1.2.3")
-	assert.Equal(t, "127.1.2.3", apmhttputil.RemoteAddr(req, nil))
-
-	// "for" is missing from Forwarded, so fall back to the next thing
-	assert.Equal(t, "127.1.2.3", apmhttputil.RemoteAddr(req, &apmhttputil.ForwardedHeader{}))
-
-	assert.Equal(t, "_secret", apmhttputil.RemoteAddr(req, &apmhttputil.ForwardedHeader{For: "_secret"}))
-
-	assert.Equal(t, "2001:db8:cafe::17", apmhttputil.RemoteAddr(req, &apmhttputil.ForwardedHeader{
-		For: "[2001:db8:cafe::17]:4711",
-	}))
+	assert.Equal(t, "::1", apmhttputil.RemoteAddr(req))
 }

internal/apmhttputil/url.go

@@ -33,7 +33,7 @@ import (
 // requests (i.e. most server-side requests), we reconstruct the
 // URL based on various proxy forwarding headers and other request
 // attributes.
-func RequestURL(req *http.Request, forwarded *ForwardedHeader) model.URL {
+func RequestURL(req *http.Request) model.URL {
 	out := model.URL{
 		Path:   truncateString(req.URL.Path),
 		Search: truncateString(req.URL.RawQuery),
@@ -53,7 +53,8 @@ func RequestURL(req *http.Request, forwarded *ForwardedHeader) model.URL {
 	// We synthesize the full URL by extracting the host and protocol
 	// from headers, or inferring from other properties.
 	var fullHost string
-	if forwarded != nil && forwarded.Host != "" {
+	forwarded := ParseForwarded(req.Header.Get("Forwarded"))
+	if forwarded.Host != "" {
 		fullHost = forwarded.Host
 		out.Protocol = truncateString(forwarded.Proto)
 	} else if xfh := req.Header.Get("X-Forwarded-Host"); xfh != "" {

internal/apmhttputil/url_test.go

@@ -39,7 +39,7 @@ func TestRequestURLClient(t *testing.T) {
 		Path:     "/path",
 		Search:   "query&querier=foo",
 		Hash:     "fragment",
-	}, apmhttputil.RequestURL(req, nil))
+	}, apmhttputil.RequestURL(req))
 }
 
 func TestRequestURLServer(t *testing.T) {
@@ -52,32 +52,31 @@ func TestRequestURLServer(t *testing.T) {
 		Port:     "8080",
 		Path:     "/path",
 		Search:   "query&querier=foo",
-	}, apmhttputil.RequestURL(req, nil))
+	}, apmhttputil.RequestURL(req))
 }
 
 func TestRequestURLServerTLS(t *testing.T) {
 	req := mustNewRequest("/path?query&querier=foo")
 	req.Host = "host.invalid:8080"
 	req.TLS = &tls.ConnectionState{}
-	assert.Equal(t, "https", apmhttputil.RequestURL(req, nil).Protocol)
+	assert.Equal(t, "https", apmhttputil.RequestURL(req).Protocol)
 }
 
 func TestRequestURLHeaders(t *testing.T) {
 	type test struct {
-		name      string
-		full      string
-		header    http.Header
-		forwarded *apmhttputil.ForwardedHeader
+		name   string
+		full   string
+		header http.Header
 	}
 
 	tests := []test{{
-		name:      "Forwarded",
-		full:      "https://forwarded.invalid:443/",
-		forwarded: &apmhttputil.ForwardedHeader{Host: "forwarded.invalid:443", Proto: "HTTPS"},
+		name:   "Forwarded",
+		full:   "https://forwarded.invalid:443/",
+		header: http.Header{"Forwarded": []string{"Host=\"forwarded.invalid:443\"; proto=HTTPS"}},
 	}, {
-		name:      "Forwarded-Empty-Host",
-		full:      "http://host.invalid/", // falls back to the next option
-		forwarded: &apmhttputil.ForwardedHeader{Host: ""},
+		name:   "Forwarded-Empty-Host",
+		full:   "http://host.invalid/", // falls back to the next option
+		header: http.Header{"Forwarded": []string{""}},
 	}, {
 		name:   "X-Forwarded-Host",
 		full:   "http://x-forwarded-host.invalid/",
@@ -110,7 +109,7 @@ func TestRequestURLHeaders(t *testing.T) {
 			req.Host = "host.invalid"
 			req.Header = test.header
 
-			out := apmhttputil.RequestURL(req, test.forwarded)
+			out := apmhttputil.RequestURL(req)
 
 			// Marshal the URL to gets its "full" representation.
 			var w fastjson.Writer

module/apmhttp/handler_http2_test.go

@@ -61,7 +61,6 @@ func TestHandlerHTTP2(t *testing.T) {
 	require.NoError(t, err)
 
 	req, _ := http.NewRequest("GET", srv.URL+"/foo", nil)
-	req.Header.Set("X-Real-IP", "client.testing")
 	resp, err := client.Do(req)
 	require.NoError(t, err)
 	resp.Body.Close()
@@ -73,8 +72,9 @@ func TestHandlerHTTP2(t *testing.T) {
 	assert.Equal(t, &model.Context{
 		Request: &model.Request{
 			Socket: &model.RequestSocket{
-				Encrypted:     true,
-				RemoteAddress: "client.testing",
+				Encrypted: true,
+				// 127.0.0.1 or ::1.
+				RemoteAddress: srvAddr.IP.String(),
 			},
 			URL: model.URL{
 				Full:     srv.URL + "/foo",
@@ -90,9 +90,6 @@ func TestHandlerHTTP2(t *testing.T) {
 			}, {
 				Key:    "User-Agent",
 				Values: []string{"Go-http-client/2.0"},
-			}, {
-				Key:    "X-Real-Ip",
-				Values: []string{"client.testing"},
 			}},
 			HTTPVersion: "2.0",
 		},