diff --git a/AUTHORS b/AUTHORS index b9bcad6a..749f0af7 100644 --- a/AUTHORS +++ b/AUTHORS @@ -13,6 +13,8 @@ Frederick F. Kautz IV Josh Hawn Nghia Tran Olivier Gambier +Richard +Shreyas Karnik Stephen J Day Tianon Gravi xiekeyang diff --git a/context/http.go b/context/http.go index 357f0dc3..4f0a7b8a 100644 --- a/context/http.go +++ b/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": diff --git a/context/http_test.go b/context/http_test.go index df3734e8..28d2720c 100644 --- a/context/http_test.go +++ b/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: . 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) + } +} diff --git a/notifications/bridge.go b/notifications/bridge.go index 21d2105d..baa90a5b 100644 --- a/notifications/bridge.go +++ b/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(),