diff --git a/staging/src/k8s.io/apimachinery/pkg/util/net/http.go b/staging/src/k8s.io/apimachinery/pkg/util/net/http.go index f9540c63bb2..0ba586bfe51 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/net/http.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/net/http.go @@ -206,13 +206,17 @@ func GetHTTPClient(req *http.Request) string { return "unknown" } -// SourceIPs splits the comma separated X-Forwarded-For header or returns the X-Real-Ip header or req.RemoteAddr, -// in that order, ignoring invalid IPs. It returns nil if all of these are empty or invalid. +// SourceIPs splits the comma separated X-Forwarded-For header and joins it with +// the X-Real-Ip header and/or req.RemoteAddr, ignoring invalid IPs. +// The X-Real-Ip is omitted if it's already present in the X-Forwarded-For chain. +// The req.RemoteAddr is always the last IP in the returned list. +// It returns nil if all of these are empty or invalid. func SourceIPs(req *http.Request) []net.IP { + var srcIPs []net.IP + hdr := req.Header // First check the X-Forwarded-For header for requests via proxy. hdrForwardedFor := hdr.Get("X-Forwarded-For") - forwardedForIPs := []net.IP{} if hdrForwardedFor != "" { // X-Forwarded-For can be a csv of IPs in case of multiple proxies. // Use the first valid one. @@ -220,38 +224,49 @@ func SourceIPs(req *http.Request) []net.IP { for _, part := range parts { ip := net.ParseIP(strings.TrimSpace(part)) if ip != nil { - forwardedForIPs = append(forwardedForIPs, ip) + srcIPs = append(srcIPs, ip) } } } - if len(forwardedForIPs) > 0 { - return forwardedForIPs - } // Try the X-Real-Ip header. hdrRealIp := hdr.Get("X-Real-Ip") if hdrRealIp != "" { ip := net.ParseIP(hdrRealIp) - if ip != nil { - return []net.IP{ip} + // Only append the X-Real-Ip if it's not already contained in the X-Forwarded-For chain. + if ip != nil && !containsIP(srcIPs, ip) { + srcIPs = append(srcIPs, ip) } } - // Fallback to Remote Address in request, which will give the correct client IP when there is no proxy. + // Always include the request Remote Address as it cannot be easily spoofed. + var remoteIP net.IP // Remote Address in Go's HTTP server is in the form host:port so we need to split that first. host, _, err := net.SplitHostPort(req.RemoteAddr) if err == nil { - if remoteIP := net.ParseIP(host); remoteIP != nil { - return []net.IP{remoteIP} + remoteIP = net.ParseIP(host) + } + // Fallback if Remote Address was just IP. + if remoteIP == nil { + remoteIP = net.ParseIP(req.RemoteAddr) + } + + // Don't duplicate remote IP if it's already the last address in the chain. + if remoteIP != nil && (len(srcIPs) == 0 || !remoteIP.Equal(srcIPs[len(srcIPs)-1])) { + srcIPs = append(srcIPs, remoteIP) + } + + return srcIPs +} + +// Checks whether the given IP address is contained in the list of IPs. +func containsIP(ips []net.IP, ip net.IP) bool { + for _, v := range ips { + if v.Equal(ip) { + return true } } - - // Fallback if Remote Address was just IP. - if remoteIP := net.ParseIP(req.RemoteAddr); remoteIP != nil { - return []net.IP{remoteIP} - } - - return nil + return false } // Extracts and returns the clients IP from the given request. diff --git a/staging/src/k8s.io/apimachinery/pkg/util/net/http_test.go b/staging/src/k8s.io/apimachinery/pkg/util/net/http_test.go index 51d91dd84d7..da483c3563b 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/net/http_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/net/http_test.go @@ -493,3 +493,128 @@ func TestAllowsHTTP2(t *testing.T) { }) } } + +func TestSourceIPs(t *testing.T) { + tests := []struct { + name string + realIP string + forwardedFor string + remoteAddr string + expected []string + }{{ + name: "no headers, missing remoteAddr", + expected: []string{}, + }, { + name: "no headers, just remoteAddr host:port", + remoteAddr: "1.2.3.4:555", + expected: []string{"1.2.3.4"}, + }, { + name: "no headers, just remoteAddr host", + remoteAddr: "1.2.3.4", + expected: []string{"1.2.3.4"}, + }, { + name: "empty forwarded-for chain", + forwardedFor: " ", + remoteAddr: "1.2.3.4", + expected: []string{"1.2.3.4"}, + }, { + name: "invalid forwarded-for chain", + forwardedFor: "garbage garbage values!", + remoteAddr: "1.2.3.4", + expected: []string{"1.2.3.4"}, + }, { + name: "partially invalid forwarded-for chain", + forwardedFor: "garbage garbage values!,4.5.6.7", + remoteAddr: "1.2.3.4", + expected: []string{"4.5.6.7", "1.2.3.4"}, + }, { + name: "valid forwarded-for chain", + forwardedFor: "120.120.120.126,2.2.2.2,4.5.6.7", + remoteAddr: "1.2.3.4", + expected: []string{"120.120.120.126", "2.2.2.2", "4.5.6.7", "1.2.3.4"}, + }, { + name: "valid forwarded-for chain with redundant remoteAddr", + forwardedFor: "2.2.2.2,1.2.3.4", + remoteAddr: "1.2.3.4", + expected: []string{"2.2.2.2", "1.2.3.4"}, + }, { + name: "invalid Real-Ip", + realIP: "garbage, just garbage!", + remoteAddr: "1.2.3.4", + expected: []string{"1.2.3.4"}, + }, { + name: "invalid Real-Ip with forwarded-for", + realIP: "garbage, just garbage!", + forwardedFor: "2.2.2.2", + remoteAddr: "1.2.3.4", + expected: []string{"2.2.2.2", "1.2.3.4"}, + }, { + name: "valid Real-Ip", + realIP: "2.2.2.2", + remoteAddr: "1.2.3.4", + expected: []string{"2.2.2.2", "1.2.3.4"}, + }, { + name: "redundant Real-Ip", + realIP: "1.2.3.4", + remoteAddr: "1.2.3.4", + expected: []string{"1.2.3.4"}, + }, { + name: "valid Real-Ip with forwarded-for", + realIP: "2.2.2.2", + forwardedFor: "120.120.120.126,4.5.6.7", + remoteAddr: "1.2.3.4", + expected: []string{"120.120.120.126", "4.5.6.7", "2.2.2.2", "1.2.3.4"}, + }, { + name: "redundant Real-Ip with forwarded-for", + realIP: "2.2.2.2", + forwardedFor: "120.120.120.126,2.2.2.2,4.5.6.7", + remoteAddr: "1.2.3.4", + expected: []string{"120.120.120.126", "2.2.2.2", "4.5.6.7", "1.2.3.4"}, + }, { + name: "full redundancy", + realIP: "1.2.3.4", + forwardedFor: "1.2.3.4", + remoteAddr: "1.2.3.4", + expected: []string{"1.2.3.4"}, + }, { + name: "full ipv6", + realIP: "abcd:ef01:2345:6789:abcd:ef01:2345:6789", + forwardedFor: "aaaa:bbbb:cccc:dddd:eeee:ffff:0:1111,0:1111:2222:3333:4444:5555:6666:7777", + remoteAddr: "aaaa:aaaa:aaaa:aaaa:aaaa:aaaa:aaaa:aaaa", + expected: []string{ + "aaaa:bbbb:cccc:dddd:eeee:ffff:0:1111", + "0:1111:2222:3333:4444:5555:6666:7777", + "abcd:ef01:2345:6789:abcd:ef01:2345:6789", + "aaaa:aaaa:aaaa:aaaa:aaaa:aaaa:aaaa:aaaa", + }, + }, { + name: "mixed ipv4 ipv6", + forwardedFor: "aaaa:bbbb:cccc:dddd:eeee:ffff:0:1111,1.2.3.4", + remoteAddr: "0:0:0:0:0:ffff:102:304", // ipv6 equivalent to 1.2.3.4 + expected: []string{ + "aaaa:bbbb:cccc:dddd:eeee:ffff:0:1111", + "1.2.3.4", + }, + }} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + req, _ := http.NewRequest("GET", "https://cluster.k8s.io/apis/foobars/v1/foo/bar", nil) + req.RemoteAddr = test.remoteAddr + if test.forwardedFor != "" { + req.Header.Set("X-Forwarded-For", test.forwardedFor) + } + if test.realIP != "" { + req.Header.Set("X-Real-Ip", test.realIP) + } + + actualIPs := SourceIPs(req) + actual := make([]string, len(actualIPs)) + for i, ip := range actualIPs { + actual[i] = ip.String() + } + + assert.Equal(t, test.expected, actual) + }) + } +}