Merge pull request distribution#302 from RichardScothern/richardscoth…

…ern-298

Attempt to identify remote IP addresses for requests which come through proxies.

AUTHORS

@@ -13,6 +13,8 @@ Frederick F. Kautz IV <[email protected]>
 Josh Hawn <[email protected]>
 Nghia Tran <[email protected]>
 Olivier Gambier <[email protected]>
+Richard <[email protected]>
+Shreyas Karnik <[email protected]>
 Stephen J Day <[email protected]>
 Tianon Gravi <[email protected]>
 xiekeyang <[email protected]>

context/http.go

@@ -2,12 +2,14 @@ package context
 
 import (
 	"errors"
+	"net"
 	"net/http"
 	"strings"
 	"sync"
 	"time"
 
 	"code.google.com/p/go-uuid/uuid"
+	log "github.com/Sirupsen/logrus"
 	"github.com/gorilla/mux"
 	"golang.org/x/net/context"
 )
@@ -17,6 +19,37 @@ var (
 	ErrNoRequestContext = errors.New("no http request in context")
 )
 
+func parseIP(ipStr string) net.IP {
+	ip := net.ParseIP(ipStr)
+	if ip == nil {
+		log.Warnf("invalid remote IP address: %q", ipStr)
+	}
+	return ip
+}
+
+// RemoteAddr extracts the remote address of the request, taking into
+// account proxy headers.
+func RemoteAddr(r *http.Request) string {
+	if prior := r.Header.Get("X-Forwarded-For"); prior != "" {
+		proxies := strings.Split(prior, ",")
+		if len(proxies) > 0 {
+			remoteAddr := strings.Trim(proxies[0], " ")
+			if parseIP(remoteAddr) != nil {
+				return remoteAddr
+			}
+		}
+	}
+	// X-Real-Ip is less supported, but worth checking in the
+	// absence of X-Forwarded-For
+	if realIP := r.Header.Get("X-Real-Ip"); realIP != "" {
+		if parseIP(realIP) != nil {
+			return realIP
+		}
+	}
+
+	return r.RemoteAddr
+}
+
 // WithRequest places the request on the context. The context of the request
 // is assigned a unique id, available at "http.request.id". The request itself
 // is available at "http.request". Other common attributes are available under
@@ -147,7 +180,7 @@ func (ctx *httpRequestContext) Value(key interface{}) interface{} {
 		case "uri":
 			return ctx.r.RequestURI
 		case "remoteaddr":
-			return ctx.r.RemoteAddr
+			return RemoteAddr(ctx.r)
 		case "method":
 			return ctx.r.Method
 		case "host":

context/http_test.go

@@ -2,6 +2,9 @@ package context
 
 import (
 	"net/http"
+	"net/http/httptest"
+	"net/http/httputil"
+	"net/url"
 	"reflect"
 	"testing"
 	"time"
@@ -205,3 +208,67 @@ func TestWithVars(t *testing.T) {
 		}
 	}
 }
+
+// SingleHostReverseProxy will insert an X-Forwarded-For header, and can be used to test
+// RemoteAddr().  A fake RemoteAddr cannot be set on the HTTP request - it is overwritten
+// at the transport layer to 127.0.0.1:<port> .  However, as the X-Forwarded-For header
+// just contains the IP address, it is different enough for testing.
+func TestRemoteAddr(t *testing.T) {
+	var expectedRemote string
+	backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		defer r.Body.Close()
+
+		if r.RemoteAddr == expectedRemote {
+			t.Errorf("Unexpected matching remote addresses")
+		}
+
+		actualRemote := RemoteAddr(r)
+		if expectedRemote != actualRemote {
+			t.Errorf("Mismatching remote hosts: %v != %v", expectedRemote, actualRemote)
+		}
+
+		w.WriteHeader(200)
+	}))
+
+	defer backend.Close()
+	backendURL, err := url.Parse(backend.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	proxy := httputil.NewSingleHostReverseProxy(backendURL)
+	frontend := httptest.NewServer(proxy)
+	defer frontend.Close()
+
+	// X-Forwarded-For set by proxy
+	expectedRemote = "127.0.0.1"
+	proxyReq, err := http.NewRequest("GET", frontend.URL, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = http.DefaultClient.Do(proxyReq)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// RemoteAddr in X-Real-Ip
+	getReq, err := http.NewRequest("GET", backend.URL, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expectedRemote = "1.2.3.4"
+	getReq.Header["X-Real-ip"] = []string{expectedRemote}
+	_, err = http.DefaultClient.Do(getReq)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Valid X-Real-Ip and invalid X-Forwarded-For
+	getReq.Header["X-forwarded-for"] = []string{"1.2.3"}
+	_, err = http.DefaultClient.Do(getReq)
+	if err != nil {
+		t.Fatal(err)
+	}
+}

notifications/bridge.go

@@ -6,6 +6,7 @@ import (
 
 	"code.google.com/p/go-uuid/uuid"
 	"github.com/docker/distribution"
+	"github.com/docker/distribution/context"
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
 )
@@ -45,7 +46,7 @@ func NewBridge(ub URLBuilder, source SourceRecord, actor ActorRecord, request Re
 func NewRequestRecord(id string, r *http.Request) RequestRecord {
 	return RequestRecord{
 		ID:        id,
-		Addr:      r.RemoteAddr,
+		Addr:      context.RemoteAddr(r),
 		Host:      r.Host,
 		Method:    r.Method,
 		UserAgent: r.UserAgent(),