fix(log): record user real ip from headers (#603)

* fix(real_ip): get user real ip from headers of request * fix(real_ip): compatible with those header with multiple IP values separated by commas * test(real_ip): add benchmark for IPv4 and IPv6 private address check * fix(real_ip): check empty, then remove leading and tailing comma char, finally locate first IP field * test(real_ip): move checker logic into utils and add more unit test cases * test(real_ip): write unit tests covering all code branches of the `util-ip` module * refactor(real_ip): use one-line `testify.assert.Panics` to capture intended panic in test case * chore(real_ip): add module private variable `UserRealIpHeaderCandidates` put those headers together, make it easy to manage in one place * doc(real_ip): write docstring for each function in the `utils-ip` module * chore(real_ip): choose more concrete and unambiguous name for test helper function It is to avoid polluting the module name-space with too general names. * chore(naming): change function names according to code style * refactor(real_ip): simplify the code indicated by 'gosimple' and `golangci` * chore(naming): rename the `utils-ip` file to `utils_ip` to match with the rest of the file structure --------- Co-authored-by: Felipe Martin <812088+fmartingr@users.noreply.github.com>
2025-01-15 20:37:44 +08:00 · 2023-06-12 03:25:23 +08:00 · 2023-06-12 03:25:23 +08:00 · dde1b44e77
commit dde1b44e77
parent a4b92504b0
3 changed files with 447 additions and 2 deletions
--- a/internal/webserver/server.go
+++ b/internal/webserver/server.go
@ -88,7 +88,7 @@ func Logger(r *http.Request, statusCode int, size int) {
 	if statusCode == http.StatusOK {
 		logrus.WithFields(logrus.Fields{
 			"proto":  r.Proto,
-			"remote": r.RemoteAddr,
+			"remote": GetUserRealIP(r),
 			"reqlen": r.ContentLength,
 			"size":   size,
 			"status": statusCode,
@ -96,7 +96,7 @@ func Logger(r *http.Request, statusCode int, size int) {
 	} else {
 		logrus.WithFields(logrus.Fields{
 			"proto":  r.Proto,
-			"remote": r.RemoteAddr,
+			"remote": GetUserRealIP(r),
 			"reqlen": r.ContentLength,
 			"size":   size,
 			"status": statusCode,
--- a/internal/webserver/utils_ip.go
+++ b/internal/webserver/utils_ip.go
@ -0,0 +1,216 @@
+package webserver
+
+import (
+	"fmt"
+	"net"
+	"net/http"
+	"strings"
+)
+
+var (
+	userRealIpHeaderCandidates = [...]string{"X-Real-Ip", "X-Forwarded-For"}
+	// From: https://github.com/letsencrypt/boulder/blob/main/bdns/dns.go#L30-L146
+	// Private CIDRs to ignore
+	privateNetworks = []net.IPNet{
+		// RFC1918
+		// 10.0.0.0/8
+		{
+			IP:   []byte{10, 0, 0, 0},
+			Mask: []byte{255, 0, 0, 0},
+		},
+		// 172.16.0.0/12
+		{
+			IP:   []byte{172, 16, 0, 0},
+			Mask: []byte{255, 240, 0, 0},
+		},
+		// 192.168.0.0/16
+		{
+			IP:   []byte{192, 168, 0, 0},
+			Mask: []byte{255, 255, 0, 0},
+		},
+		// RFC5735
+		// 127.0.0.0/8
+		{
+			IP:   []byte{127, 0, 0, 0},
+			Mask: []byte{255, 0, 0, 0},
+		},
+		// RFC1122 Section 3.2.1.3
+		// 0.0.0.0/8
+		{
+			IP:   []byte{0, 0, 0, 0},
+			Mask: []byte{255, 0, 0, 0},
+		},
+		// RFC3927
+		// 169.254.0.0/16
+		{
+			IP:   []byte{169, 254, 0, 0},
+			Mask: []byte{255, 255, 0, 0},
+		},
+		// RFC 5736
+		// 192.0.0.0/24
+		{
+			IP:   []byte{192, 0, 0, 0},
+			Mask: []byte{255, 255, 255, 0},
+		},
+		// RFC 5737
+		// 192.0.2.0/24
+		{
+			IP:   []byte{192, 0, 2, 0},
+			Mask: []byte{255, 255, 255, 0},
+		},
+		// 198.51.100.0/24
+		{
+			IP:   []byte{198, 51, 100, 0},
+			Mask: []byte{255, 255, 255, 0},
+		},
+		// 203.0.113.0/24
+		{
+			IP:   []byte{203, 0, 113, 0},
+			Mask: []byte{255, 255, 255, 0},
+		},
+		// RFC 3068
+		// 192.88.99.0/24
+		{
+			IP:   []byte{192, 88, 99, 0},
+			Mask: []byte{255, 255, 255, 0},
+		},
+		// RFC 2544, Errata 423
+		// 198.18.0.0/15
+		{
+			IP:   []byte{198, 18, 0, 0},
+			Mask: []byte{255, 254, 0, 0},
+		},
+		// RFC 3171
+		// 224.0.0.0/4
+		{
+			IP:   []byte{224, 0, 0, 0},
+			Mask: []byte{240, 0, 0, 0},
+		},
+		// RFC 1112
+		// 240.0.0.0/4
+		{
+			IP:   []byte{240, 0, 0, 0},
+			Mask: []byte{240, 0, 0, 0},
+		},
+		// RFC 919 Section 7
+		// 255.255.255.255/32
+		{
+			IP:   []byte{255, 255, 255, 255},
+			Mask: []byte{255, 255, 255, 255},
+		},
+		// RFC 6598
+		// 100.64.0.0/10
+		{
+			IP:   []byte{100, 64, 0, 0},
+			Mask: []byte{255, 192, 0, 0},
+		},
+	}
+	// Sourced from https://www.iana.org/assignments/iana-ipv6-special-registry/iana-ipv6-special-registry.xhtml
+	// where Global, Source, or Destination is False
+	privateV6Networks = []net.IPNet{
+		parseCIDR("::/128", "RFC 4291: Unspecified Address"),
+		parseCIDR("::1/128", "RFC 4291: Loopback Address"),
+		parseCIDR("::ffff:0:0/96", "RFC 4291: IPv4-mapped Address"),
+		parseCIDR("100::/64", "RFC 6666: Discard Address Block"),
+		parseCIDR("2001::/23", "RFC 2928: IETF Protocol Assignments"),
+		parseCIDR("2001:2::/48", "RFC 5180: Benchmarking"),
+		parseCIDR("2001:db8::/32", "RFC 3849: Documentation"),
+		parseCIDR("2001::/32", "RFC 4380: TEREDO"),
+		parseCIDR("fc00::/7", "RFC 4193: Unique-Local"),
+		parseCIDR("fe80::/10", "RFC 4291: Section 2.5.6 Link-Scoped Unicast"),
+		parseCIDR("ff00::/8", "RFC 4291: Section 2.7"),
+		// We disable validations to IPs under the 6to4 anycase prefix because
+		// there's too much risk of a malicious actor advertising the prefix and
+		// answering validations for a 6to4 host they do not control.
+		// https://community.letsencrypt.org/t/problems-validating-ipv6-against-host-running-6to4/18312/9
+		parseCIDR("2002::/16", "RFC 7526: 6to4 anycast prefix deprecated"),
+	}
+)
+
+// parseCIDR parses the predefined CIDR to `net.IPNet` that consisting of IP and IPMask.
+func parseCIDR(network string, comment string) net.IPNet {
+	_, subNet, err := net.ParseCIDR(network)
+	if err != nil {
+		panic(fmt.Sprintf("error parsing %s (%s): %s", network, comment, err))
+	}
+	return *subNet
+}
+
+// isPrivateV4 checks whether an `ip` is private based on whether the IP is in the private CIDR range.
+func isPrivateV4(ip net.IP) bool {
+	for _, subNet := range privateNetworks {
+		if subNet.Contains(ip) {
+			return true
+		}
+	}
+	return false
+}
+
+// isPrivateV6 checks whether an `ip` is private based on whether the IP is in the private CIDR range.
+func isPrivateV6(ip net.IP) bool {
+	for _, subNet := range privateV6Networks {
+		if subNet.Contains(ip) {
+			return true
+		}
+	}
+	return false
+}
+
+// IsPrivateIP check IPv4 or IPv6 address according to the length of byte array
+func IsPrivateIP(ip net.IP) bool {
+	if ip4 := ip.To4(); ip4 != nil {
+		return isPrivateV4(ip4)
+	}
+	return ip.To16() != nil && isPrivateV6(ip)
+}
+
+// IsIPValidAndPublic is a helper function check if an IP address is valid and public.
+func IsIPValidAndPublic(ipAddr string) bool {
+	if ipAddr == "" {
+		return false
+	}
+	ipAddr = strings.TrimSpace(ipAddr)
+	ip := net.ParseIP(ipAddr)
+	// remote address within public address range
+	if ip != nil && !IsPrivateIP(ip) {
+		return true
+	}
+	return false
+}
+
+// GetUserRealIP Get User Real IP from headers of request `r`
+//  1. First, determine whether the remote addr of request is a private address.
+//     If it is a public network address, return it directly;
+//  2. Otherwise, get and check the real IP from X-REAL-IP and X-Forwarded-For headers in turn.
+//     if the header value contains multiple IP addresses separated by commas, that is,
+//     the request may pass through multiple reverse proxies, we just keep the first one,
+//     which imply it is the user connecting IP.
+//     then we check the value is a valid public IP address using the `IsIPValidAndPublic` function.
+//     If it is, the function returns the value as the client's real IP address.
+//  3. Finally, If the above headers do not exist or are invalid, the remote addr is returned as is.
+func GetUserRealIP(r *http.Request) string {
+	fallbackAddr := r.RemoteAddr
+	connectAddr, _, err := net.SplitHostPort(r.RemoteAddr)
+	if err != nil {
+		return fallbackAddr
+	}
+	if IsIPValidAndPublic(connectAddr) {
+		return connectAddr
+	}
+	// in case that remote address is private(container or internal)
+	for _, hd := range userRealIpHeaderCandidates {
+		val := r.Header.Get(hd)
+		if val == "" {
+			continue
+		}
+		// remove leading or tailing comma, tab, space
+		ipAddr := strings.Trim(val, ",\t ")
+		if idxFirstIP := strings.Index(ipAddr, ","); idxFirstIP >= 0 {
+			ipAddr = ipAddr[:idxFirstIP]
+		}
+		if IsIPValidAndPublic(ipAddr) {
+			return ipAddr
+		}
+	}
+	return fallbackAddr
+}
--- a/internal/webserver/utils_ip_test.go
+++ b/internal/webserver/utils_ip_test.go
@ -0,0 +1,229 @@
+package webserver
+
+import (
+	"fmt"
+	"math/rand"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestParseCidr(t *testing.T) {
+	res := parseCIDR("192.168.0.0/16", "internal 192.168.x.x")
+	assert.Equal(t, res.IP, net.IP([]byte{192, 168, 0, 0}))
+	assert.Equal(t, res.Mask, net.IPMask([]byte{255, 255, 0, 0}))
+}
+
+func TestParseCidrInvalidAddr(t *testing.T) {
+	assert.Panics(t, func() { parseCIDR("192.168.0.0/34", "internal 192.168.x.x") })
+}
+
+func TestIsPrivateIP(t *testing.T) {
+	assert.True(t, IsPrivateIP(net.ParseIP("127.0.0.1")), "should be private")
+	assert.True(t, IsPrivateIP(net.ParseIP("192.168.254.254")), "should be private")
+	assert.True(t, IsPrivateIP(net.ParseIP("10.255.0.3")), "should be private")
+	assert.True(t, IsPrivateIP(net.ParseIP("172.16.255.255")), "should be private")
+	assert.True(t, IsPrivateIP(net.ParseIP("172.31.255.255")), "should be private")
+	assert.True(t, !IsPrivateIP(net.ParseIP("128.0.0.1")), "should be private")
+	assert.True(t, !IsPrivateIP(net.ParseIP("192.169.255.255")), "should not be private")
+	assert.True(t, !IsPrivateIP(net.ParseIP("9.255.0.255")), "should not be private")
+	assert.True(t, !IsPrivateIP(net.ParseIP("172.32.255.255")), "should not be private")
+
+	assert.True(t, IsPrivateIP(net.ParseIP("::0")), "should be private")
+	assert.True(t, IsPrivateIP(net.ParseIP("::1")), "should be private")
+	assert.True(t, !IsPrivateIP(net.ParseIP("::2")), "should not be private")
+
+	assert.True(t, IsPrivateIP(net.ParseIP("fe80::1")), "should be private")
+	assert.True(t, IsPrivateIP(net.ParseIP("febf::1")), "should be private")
+	assert.True(t, !IsPrivateIP(net.ParseIP("fec0::1")), "should not be private")
+	assert.True(t, !IsPrivateIP(net.ParseIP("feff::1")), "should not be private")
+
+	assert.True(t, IsPrivateIP(net.ParseIP("ff00::1")), "should be private")
+	assert.True(t, IsPrivateIP(net.ParseIP("ff10::1")), "should be private")
+	assert.True(t, IsPrivateIP(net.ParseIP("ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff")), "should be private")
+
+	assert.True(t, IsPrivateIP(net.ParseIP("2002::")), "should be private")
+	assert.True(t, IsPrivateIP(net.ParseIP("2002:ffff:ffff:ffff:ffff:ffff:ffff:ffff")), "should be private")
+	assert.True(t, IsPrivateIP(net.ParseIP("0100::")), "should be private")
+	assert.True(t, IsPrivateIP(net.ParseIP("0100::0000:ffff:ffff:ffff:ffff")), "should be private")
+	assert.True(t, !IsPrivateIP(net.ParseIP("0100::0001:0000:0000:0000:0000")), "should be private")
+}
+
+func TestIsIpValidAndPublic(t *testing.T) {
+	// test empty address
+	assert.False(t, IsIPValidAndPublic(""))
+	// test public address
+	assert.True(t, IsIPValidAndPublic("31.41.244.124"))
+	assert.True(t, IsIPValidAndPublic("62.233.50.248"))
+	// trim head or tail space
+	assert.True(t, IsIPValidAndPublic(" 62.233.50.249"))
+	assert.True(t, IsIPValidAndPublic(" 62.233.50.250 "))
+	assert.True(t, IsIPValidAndPublic("62.233.50.251 "))
+	// test private address
+	assert.False(t, IsIPValidAndPublic("10.1.123.52"))
+	assert.False(t, IsIPValidAndPublic("192.168.123.24"))
+	assert.False(t, IsIPValidAndPublic("172.17.0.1"))
+}
+
+func BenchmarkIsPrivateIPv4(b *testing.B) {
+	// range: 2-254
+	n1 := 2 + rand.Intn(252)
+	n2 := 2 + rand.Intn(252)
+	for i := 0; i < b.N; i++ {
+		IsPrivateIP(net.ParseIP(fmt.Sprintf("192.168.%d.%d", n1, n2)))
+	}
+}
+
+func BenchmarkIsPrivateIPv6(b *testing.B) {
+	n1 := 2 + rand.Intn(252)
+	for i := 0; i < b.N; i++ {
+		IsPrivateIP(net.ParseIP(fmt.Sprintf("2002::%d", n1)))
+	}
+}
+
+func testIsPublicHttpRequestAddressHelper(
+	t *testing.T, wantIP string, headers map[string]string, isPublic bool,
+) {
+	testIsPublicHttpRequestAddressHelperWrapped(t, nil, wantIP, headers, isPublic)
+}
+
+func testIsPublicHttpRequestAddressHelperWrapped(
+	t *testing.T, r *http.Request, wantIP string, headers map[string]string, isPublic bool,
+) {
+	var (
+		err    error
+		userIP string
+	)
+	if r == nil {
+		r = httptest.NewRequest("GET", "/", nil)
+	}
+	for k, v := range headers {
+		r.Header.Set(k, v)
+	}
+
+	origVal := GetUserRealIP(r)
+	if strings.Contains(origVal, ":") {
+		userIP, _, err = net.SplitHostPort(origVal)
+		if err != nil {
+			t.Error(err)
+		}
+	} else {
+		userIP = origVal
+	}
+
+	if isPublic {
+		// should equal first ip in list
+		assert.Equal(t, wantIP, userIP)
+		assert.True(t, IsIPValidAndPublic(userIP))
+	} else {
+		assert.Equal(t, origVal, r.RemoteAddr)
+		assert.False(t, IsIPValidAndPublic(userIP))
+	}
+}
+
+func TestGetUserRealIPWithSetRemoteAddr(t *testing.T) {
+	// Test Public RemoteAddr
+	testIsPublicHttpRequestAddressHelper(t, "", nil, false)
+
+	r := httptest.NewRequest("GET", "/", nil)
+	wantIP := "34.23.123.122"
+	r.RemoteAddr = fmt.Sprintf("%s:1234", wantIP)
+	testIsPublicHttpRequestAddressHelperWrapped(t, r, wantIP, nil, true)
+}
+
+func TestGetUserRealIPWithInvalidRemoteAddr(t *testing.T) {
+	// Test Public RemoteAddr
+	testIsPublicHttpRequestAddressHelper(t, "", nil, false)
+
+	r := httptest.NewRequest("GET", "/", nil)
+	wantIP := "34.23.123.122"
+	// without port
+	r.RemoteAddr = wantIP
+	testIsPublicHttpRequestAddressHelperWrapped(t, r, wantIP, nil, true)
+}
+
+func TestGetUserRealIPWithEmptyHeader(t *testing.T) {
+	// Test Empty X-Real-IP
+	testIsPublicHttpRequestAddressHelper(t, "", nil, false)
+}
+
+func TestGetUserRealIPWithInvalidHeaderValue(t *testing.T) {
+	for _, name := range userRealIpHeaderCandidates {
+		// invalid ip
+		m := map[string]string{
+			name: "31.41.24a.12",
+		}
+		testIsPublicHttpRequestAddressHelper(t, "", m, false)
+	}
+}
+
+func TestGetUserRealIPWithXRealIpHeader(t *testing.T) {
+	// Test public Real IP
+	for _, name := range userRealIpHeaderCandidates {
+		wantIP := "31.41.242.12"
+		m := map[string]string{
+			name: wantIP,
+		}
+		testIsPublicHttpRequestAddressHelper(t, wantIP, m, true)
+	}
+}
+
+func TestGetUserRealIPWithPrivateXRealIpHeader(t *testing.T) {
+	for _, name := range userRealIpHeaderCandidates {
+		wantIP := "192.168.123.123"
+		// test private ip in header
+		m := map[string]string{
+			name: wantIP,
+		}
+		testIsPublicHttpRequestAddressHelper(t, wantIP, m, false)
+	}
+}
+
+func TestGetUserRealIPWithXRealIpListHeader(t *testing.T) {
+	// Test Real IP List
+	for _, name := range userRealIpHeaderCandidates {
+		ipList := []string{"34.23.123.122", "34.23.123.123"}
+		// should equal first ip in list
+		wantIP := ipList[0]
+		// test private ip in header
+		m := map[string]string{
+			name: strings.Join(ipList, ", "),
+		}
+		testIsPublicHttpRequestAddressHelper(t, wantIP, m, true)
+	}
+}
+
+func TestGetUserRealIPWithXRealIpHeaderIgnoreComma(t *testing.T) {
+	// Test Real IP List with leading or tailing comma
+	wantIP := "34.23.123.124"
+	ipVariants := []string{
+		",34.23.123.124", " ,34.23.123.124", "\t,34.23.123.124",
+		",34.23.123.124,", " ,34.23.123.124, ", "\t,34.23.123.124,\t",
+		"34.23.123.124,", "34.23.123.124, ", "34.23.123.124,\t"}
+	for _, variant := range ipVariants {
+		for _, name := range userRealIpHeaderCandidates {
+			m := map[string]string{name: variant}
+			testIsPublicHttpRequestAddressHelper(t, wantIP, m, true)
+		}
+	}
+}
+
+func TestGetUserRealIPWithDifferentHeaderOrder(t *testing.T) {
+	var m map[string]string
+	wantIP := "34.23.123.124"
+	m = map[string]string{
+		"X-Real-Ip":       "192.168.123.122",
+		"X-Forwarded-For": wantIP,
+	}
+
+	testIsPublicHttpRequestAddressHelper(t, wantIP, m, true)
+	m = map[string]string{
+		"X-Real-Ip":       wantIP,
+		"X-Forwarded-For": "192.168.123.122",
+	}
+	testIsPublicHttpRequestAddressHelper(t, wantIP, m, true)
+}