From 29b75e7648d5bd181bc67702b8f109c4b698f96b Mon Sep 17 00:00:00 2001 From: Brett Smith Date: Fri, 16 Aug 2024 16:09:36 -0400 Subject: [PATCH] 21901: keep-web logs based on the last X-Forwarded-For address Instead of the first. Arvados-DCO-1.1-Signed-off-by: Brett Smith --- services/keep-web/handler.go | 9 +++++++++ services/keep-web/handler_test.go | 26 +++++++++++++------------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go index d7adb87357..05622aae4c 100644 --- a/services/keep-web/handler.go +++ b/services/keep-web/handler.go @@ -15,6 +15,7 @@ import ( "net/http" "net/url" "os" + "slices" "sort" "strconv" "strings" @@ -982,12 +983,20 @@ func newFileEventLog( return nil } + // We want to log the address of the proxy closest to keep-web—the last + // value in the X-Forwarded-For list—or the client address if there is no + // valid proxy. var clientAddr string + // 1. Build a slice of proxy addresses from X-Forwarded-For. xff := strings.Join(r.Header.Values("X-Forwarded-For"), ",") addrs := strings.Split(xff, ",") + // 2. Reverse the slice so it's in our most preferred order for logging. + slices.Reverse(addrs) + // 3. Append the client address to that slice. if addr, _, err := net.SplitHostPort(r.RemoteAddr); err == nil { addrs = append(addrs, addr) } + // 4. Use the first valid address in the slice. for _, addr := range addrs { if ip := net.ParseIP(strings.TrimSpace(addr)); ip != nil { clientAddr = ip.String() diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go index ae154ac107..0dbace67e0 100644 --- a/services/keep-web/handler_test.go +++ b/services/keep-web/handler_test.go @@ -162,19 +162,19 @@ func (s *UnitSuite) TestLogXForwardedFor(c *check.C) { req := newRequest("GET", collURL+filePath) for xff, expected := range map[string]string{ "10.20.30.55": "10.20.30.55", - "192.168.144.120, 10.20.30.120": "192.168.144.120", - "192.0.2.4, 192.0.2.6, 192.0.2.8": "192.0.2.4", - "192.0.2.4,192.168.2.4": "192.0.2.4", - "10.20.30.60,192.168.144.40,192.0.2.4": "10.20.30.60", + "192.168.144.120, 10.20.30.120": "10.20.30.120", + "192.0.2.4, 192.0.2.6, 192.0.2.8": "192.0.2.8", + "192.0.2.4,192.168.2.4": "192.168.2.4", + "10.20.30.60,192.168.144.40,192.0.2.4": "192.0.2.4", "100::20:30:50": "100::20:30:50", - "2001:db8::80:90, 100::100": "2001:db8::80:90", - "3fff::ff, 3fff::ee, 3fff::fe": "3fff::ff", - "3fff::3f,100::1000": "3fff::3f", - "2001:db8::88,100::88,3fff::88": "2001:db8::88", - "10.20.30.60, 2001:db8::60": "10.20.30.60", - "2001:db8::20,10.20.30.20": "2001:db8::20", - ", 10.20.30.123, 100::123": "10.20.30.123", - ",100::321,10.30.20.10": "100::321", + "2001:db8::80:90, 100::100": "100::100", + "3fff::ff, 3fff::ee, 3fff::fe": "3fff::fe", + "3fff::3f,100::1000": "100::1000", + "2001:db8::88,100::88,3fff::88": "3fff::88", + "10.20.30.60, 2001:db8::60": "2001:db8::60", + "2001:db8::20,10.20.30.20": "10.20.30.20", + ", 10.20.30.123, 100::123": "100::123", + ",100::321,10.30.20.10": "10.30.20.10", } { req.Header.Set("X-Forwarded-For", xff) actual := newFileEventLog(s.handler, req, filePath, nil, nil, "") @@ -208,7 +208,7 @@ func (s *UnitSuite) TestLogXForwardedForMultivalue(c *check.C) { req.Header.Add("X-Forwarded-For", "10.20.30.90") actual := newFileEventLog(s.handler, req, filePath, nil, nil, "") c.Assert(actual, check.NotNil) - c.Check(actual.clientAddr, check.Equals, "2001:db8::db9:dbd") + c.Check(actual.clientAddr, check.Equals, "10.20.30.90") } func (s *UnitSuite) TestLogClientAddressCanonicalization(c *check.C) { -- 2.39.5