Merge pull request #134 from JoshVanL/client-ip-improvments

Set client IP as x-forwarded value is available.

Author: jetstack-bot

docs/tasks/extra-impersonation-headers.md

@@ -15,7 +15,9 @@ extra header:
 Proxied requests will then contain the header
 `Impersonate-Extra-Remote-Client-Ip: <REMOTE_ADDR>` where  `<REMOTE_ADDR>` is
 the address of the source connection of the request. Note that this IP address
-may not be the real client IP address when the request is being proxied.
+may not be the real client IP address when the request is being proxied. If the
+`X-Forwarded-For` header is present in the request then the value will be used
+as the client address.
 
 # Extra User Headers
 

go.mod

@@ -7,6 +7,7 @@ require (
 	github.com/heptiolabs/healthcheck v0.0.0-20180807145615-6ff867650f40
 	github.com/onsi/ginkgo v1.10.1
 	github.com/onsi/gomega v1.7.0
+	github.com/sebest/xff v0.0.0-20160910043805-6c115e0ffa35
 	github.com/sirupsen/logrus v1.4.2
 	github.com/spf13/cobra v0.0.5
 	github.com/spf13/pflag v1.0.5

go.sum

@@ -259,6 +259,8 @@ github.com/prometheus/procfs v0.0.2 h1:6LJUbpNm42llc4HRCuvApCSWB/WfhuNo9K98Q9sNG
 github.com/prometheus/procfs v0.0.2/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA=
 github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg=
 github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g=
+github.com/sebest/xff v0.0.0-20160910043805-6c115e0ffa35 h1:eajwn6K3weW5cd1ZXLu2sJ4pvwlBiCWY4uDejOr73gM=
+github.com/sebest/xff v0.0.0-20160910043805-6c115e0ffa35/go.mod h1:wozgYq9WEBQBaIJe4YZ0qTSFAMxmcwBhQH0fO0R34Z0=
 github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo=
 github.com/sirupsen/logrus v1.4.2 h1:SPIRibHv4MatM3XXNO2BJeFLZwZ2LvZgfQ5+UNI2im4=
 github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE=

pkg/proxy/context/context.go

@@ -2,9 +2,9 @@
 package context
 
 import (
-	"context"
 	"net/http"
 
+	"github.com/sebest/xff"
 	"k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/client-go/transport"
 )
@@ -20,37 +20,55 @@ const (
 
 	// bearerTokenKey is the context key for the bearer token.
 	bearerTokenKey
+
+	// bearerTokenKey is the context key for the client address.
+	clientAddressKey
 )
 
-// WithNoImpersonation returns a copy of parent in which the noImpersonation value is set.
-func WithNoImpersonation(parent context.Context) context.Context {
-	return request.WithValue(parent, noImpersonationKey, true)
+// WithNoImpersonation returns a copy of the request in which the noImpersonation context value is set.
+func WithNoImpersonation(req *http.Request) *http.Request {
+	return req.WithContext(request.WithValue(req.Context(), noImpersonationKey, true))
 }
 
-// NoImpersonation returns whether the noImpersonation key has been set
-func NoImpersonation(ctx context.Context) bool {
-	noImp, _ := ctx.Value(noImpersonationKey).(bool)
+// NoImpersonation returns whether the noImpersonation context key has been set
+func NoImpersonation(req *http.Request) bool {
+	noImp, _ := req.Context().Value(noImpersonationKey).(bool)
 	return noImp
 }
 
 // WithImpersonationConfig returns a copy of parent in which contains the impersonation configuration.
-func WithImpersonationConfig(parent context.Context, conf *transport.ImpersonationConfig) context.Context {
-	return request.WithValue(parent, impersonationConfigKey, conf)
+func WithImpersonationConfig(req *http.Request, conf *transport.ImpersonationConfig) *http.Request {
+	return req.WithContext(request.WithValue(req.Context(), impersonationConfigKey, conf))
 }
 
 // ImpersonationConfig returns the impersonation configuration held in the context if existing.
-func ImpersonationConfig(ctx context.Context) *transport.ImpersonationConfig {
-	conf, _ := ctx.Value(impersonationConfigKey).(*transport.ImpersonationConfig)
+func ImpersonationConfig(req *http.Request) *transport.ImpersonationConfig {
+	conf, _ := req.Context().Value(impersonationConfigKey).(*transport.ImpersonationConfig)
 	return conf
 }
 
-// WithBearerToken will add the bearer token from an http.Header to the context.
-func WithBearerToken(parent context.Context, header http.Header) context.Context {
-	return request.WithValue(parent, bearerTokenKey, header.Get("Authorization"))
+// WithBearerToken will add the bearer token to the request context from an http.Header to the request context.
+func WithBearerToken(req *http.Request, header http.Header) *http.Request {
+	return req.WithContext(request.WithValue(req.Context(), bearerTokenKey, header.Get("Authorization")))
 }
 
-// BearerToken will return the bearer token stored in the context.
-func BearerToken(ctx context.Context) string {
-	token, _ := ctx.Value(bearerTokenKey).(string)
+// BearerToken will return the bearer token stored in the request context.
+func BearerToken(req *http.Request) string {
+	token, _ := req.Context().Value(bearerTokenKey).(string)
 	return token
 }
+
+// RemoteAddress will attempt to return the source client address if available
+// in the request context. If it is not, it will be gathered from the request
+// and entered into the context.
+func RemoteAddr(req *http.Request) (*http.Request, string) {
+	ctx := req.Context()
+
+	clientAddress, ok := ctx.Value(clientAddressKey).(string)
+	if !ok {
+		clientAddress = xff.GetRemoteAddr(req)
+		req = req.WithContext(request.WithValue(ctx, clientAddressKey, clientAddress))
+	}
+
+	return req, clientAddress
+}

pkg/proxy/handlers.go

@@ -39,7 +39,10 @@ func (p *Proxy) withAuthenticateRequest(handler http.Handler) http.Handler {
 			return
 		}
 
-		klog.V(4).Infof("authenticated request: %s", req.RemoteAddr)
+		var remoteAddr string
+		req, remoteAddr = context.RemoteAddr(req)
+
+		klog.V(4).Infof("authenticated request: %s", remoteAddr)
 
 		// Add the user info to the request context
 		req = req.WithContext(genericapirequest.WithUser(req.Context(), info.User))
@@ -65,7 +68,7 @@ func (p *Proxy) withTokenReview(handler http.Handler) http.Handler {
 		}
 
 		// Set no impersonation headers and re-add removed headers.
-		req = req.WithContext(context.WithNoImpersonation(req.Context()))
+		req = context.WithNoImpersonation(req)
 
 		handler.ServeHTTP(rw, req)
 	})
@@ -75,16 +78,19 @@ func (p *Proxy) withTokenReview(handler http.Handler) http.Handler {
 func (p *Proxy) withImpersonateRequest(handler http.Handler) http.Handler {
 	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 		// If no impersonation has already been set, return early
-		if context.NoImpersonation(req.Context()) {
+		if context.NoImpersonation(req) {
 			handler.ServeHTTP(rw, req)
 			return
 		}
 
+		var remoteAddr string
+		req, remoteAddr = context.RemoteAddr(req)
+
 		// If we have disabled impersonation we can forward the request right away
 		if p.config.DisableImpersonation {
-			klog.V(2).Infof("passing on request with no impersonation: %s", req.RemoteAddr)
+			klog.V(2).Infof("passing on request with no impersonation: %s", remoteAddr)
 			// Indicate we need to not use impersonation.
-			req = req.WithContext(context.WithNoImpersonation(req.Context()))
+			req = context.WithNoImpersonation(req)
 			handler.ServeHTTP(rw, req)
 			return
 		}
@@ -124,16 +130,16 @@ func (p *Proxy) withImpersonateRequest(handler http.Handler) http.Handler {
 		// address.
 		if p.config.ExtraUserHeadersClientIPEnabled {
 			klog.V(6).Infof("adding impersonate extra user header %s: %s (%s)",
-				UserHeaderClientIPKey, req.RemoteAddr, req.RemoteAddr)
+				UserHeaderClientIPKey, remoteAddr, remoteAddr)
 
-			extra[UserHeaderClientIPKey] = append(extra[UserHeaderClientIPKey], req.RemoteAddr)
+			extra[UserHeaderClientIPKey] = append(extra[UserHeaderClientIPKey], remoteAddr)
 		}
 
 		// Add custom extra user headers to impersonation request.
 		for k, vs := range p.config.ExtraUserHeaders {
 			for _, v := range vs {
 				klog.V(6).Infof("adding impersonate extra user header %s: %s (%s)",
-					k, v, req.RemoteAddr)
+					k, v, remoteAddr)
 
 				extra[k] = append(extra[k], v)
 			}
@@ -146,7 +152,7 @@ func (p *Proxy) withImpersonateRequest(handler http.Handler) http.Handler {
 		}
 
 		// Add the impersonation configuration to the context.
-		req = req.WithContext(context.WithImpersonationConfig(req.Context(), conf))
+		req = context.WithImpersonationConfig(req, conf)
 		handler.ServeHTTP(rw, req)
 	})
 }

pkg/proxy/proxy.go

@@ -165,14 +165,14 @@ func (p *Proxy) RoundTrip(req *http.Request) (*http.Response, error) {
 
 	// If no impersonation then we return here without setting impersonation
 	// header but re-introduce the token we removed.
-	if context.NoImpersonation(req.Context()) {
-		token := context.BearerToken(req.Context())
+	if context.NoImpersonation(req) {
+		token := context.BearerToken(req)
 		req.Header.Add("Authorization", token)
 		return p.noAuthClientTransport.RoundTrip(req)
 	}
 
 	// Get the impersonation headers from the context.
-	conf := context.ImpersonationConfig(req.Context())
+	conf := context.ImpersonationConfig(req)
 	if conf == nil {
 		return nil, errNoImpersonationConfig
 	}
@@ -183,20 +183,24 @@ func (p *Proxy) RoundTrip(req *http.Request) (*http.Response, error) {
 	// Push request through round trippers to the API server.
 	return rt.RoundTrip(req)
 }
+
 func (p *Proxy) reviewToken(rw http.ResponseWriter, req *http.Request) bool {
+	var remoteAddr string
+	req, remoteAddr = context.RemoteAddr(req)
+
 	klog.V(4).Infof("attempting to validate a token in request using TokenReview endpoint(%s)",
-		req.RemoteAddr)
+		remoteAddr)
 
 	ok, err := p.tokenReviewer.Review(req)
 	if err != nil {
 		klog.Errorf("unable to authenticate the request via TokenReview due to an error (%s): %s",
-			req.RemoteAddr, err)
+			remoteAddr, err)
 		return false
 	}
 
 	if !ok {
 		klog.V(4).Infof("passing request with valid token through (%s)",
-			req.RemoteAddr)
+			remoteAddr)
 
 		return false
 	}