From c667ada63a2fbb7702c0a75480b5c4c2f92d03ec Mon Sep 17 00:00:00 2001 From: bt90 Date: Wed, 23 Apr 2025 08:01:13 +0200 Subject: [PATCH] chore(api): log X-Forwarded-For (#10035) ### Purpose Fix https://github.com/syncthing/syncthing/issues/9336 The `emitLoginAttempt` function now checks for the presence of an `X-Forwarded-For` header. The IP from this header is only used if the connecting host is either on loopback or on the same LAN. In the case of a host pretending to be a proxy, we'd still have both IPs in the logs, which should make this much less critical from a security standpoint. ### Testing 1. directly via localhost 2. via proxy an localhost #### Logs ``` [3JPXJ] 2025/04/11 15:00:40 INFO: Wrong credentials supplied during API authorization from 127.0.0.1 [3JPXJ] 2025/04/11 15:03:04 INFO: Wrong credentials supplied during API authorization from 192.168.178.5 proxied by 127.0.0.1 ``` #### Event API ``` { "id": 23, "globalID": 23, "time": "2025-04-11T15:00:40.578577402+02:00", "type": "LoginAttempt", "data": { "remoteAddress": "127.0.0.1", "success": false, "username": "sdfsd" } }, { "id": 24, "globalID": 24, "time": "2025-04-11T15:03:04.423403976+02:00", "type": "LoginAttempt", "data": { "proxy": "127.0.0.1", "remoteAddress": "192.168.178.5", "success": false, "username": "sdfsd" } } ``` ### Documentation https://github.com/syncthing/docs/pull/907 --------- Co-authored-by: Jakob Borg --- lib/api/api_auth.go | 56 +++++++++++++++++++++++++++++++++------ lib/api/tokenmanager.go | 2 +- lib/osutil/net.go | 12 +++++++++ lib/osutil/osutil_test.go | 32 ++++++++++++++++++++++ 4 files changed, 93 insertions(+), 9 deletions(-) diff --git a/lib/api/api_auth.go b/lib/api/api_auth.go index 791888056..8568394a8 100644 --- a/lib/api/api_auth.go +++ b/lib/api/api_auth.go @@ -18,6 +18,7 @@ import ( ldap "github.com/go-ldap/ldap/v3" "github.com/syncthing/syncthing/lib/config" "github.com/syncthing/syncthing/lib/events" + "github.com/syncthing/syncthing/lib/osutil" "github.com/syncthing/syncthing/lib/rand" ) @@ -27,15 +28,54 @@ const ( randomTokenLength = 64 ) -func emitLoginAttempt(success bool, username, address string, evLogger events.Logger) { - evLogger.Log(events.LoginAttempt, map[string]interface{}{ +func emitLoginAttempt(success bool, username string, r *http.Request, evLogger events.Logger) { + remoteAddress, proxy := remoteAddress(r) + evData := map[string]any{ "success": success, "username": username, - "remoteAddress": address, - }) - if !success { - l.Infof("Wrong credentials supplied during API authorization from %s", address) + "remoteAddress": remoteAddress, } + if proxy != "" { + evData["proxy"] = proxy + } + evLogger.Log(events.LoginAttempt, evData) + + if success { + return + } + if proxy != "" { + l.Infof("Wrong credentials supplied during API authorization from %s proxied by %s", remoteAddress, proxy) + } else { + l.Infof("Wrong credentials supplied during API authorization from %s", remoteAddress) + } +} + +func remoteAddress(r *http.Request) (remoteAddr, proxy string) { + remoteAddr = r.RemoteAddr + remoteIP := osutil.IPFromString(r.RemoteAddr) + + // parse X-Forwarded-For only if the proxy connects via unix socket, localhost or a LAN IP + var localProxy bool + if remoteIP != nil { + remoteAddr = remoteIP.String() + localProxy = remoteIP.IsLoopback() || remoteIP.IsPrivate() || remoteIP.IsLinkLocalUnicast() + } else if remoteAddr == "@" { + localProxy = true + } + + if !localProxy { + return + } + + forwardedAddr, _, _ := strings.Cut(r.Header.Get("X-Forwarded-For"), ",") + forwardedAddr = strings.TrimSpace(forwardedAddr) + forwardedIP := osutil.IPFromString(forwardedAddr) + + if forwardedIP != nil { + proxy = remoteAddr + remoteAddr = forwardedIP.String() + } + return } func antiBruteForceSleep() { @@ -152,7 +192,7 @@ func (m *basicAuthAndSessionMiddleware) passwordAuthHandler(w http.ResponseWrite return } - emitLoginAttempt(false, req.Username, r.RemoteAddr, m.evLogger) + emitLoginAttempt(false, req.Username, r, m.evLogger) antiBruteForceSleep() forbidden(w) } @@ -175,7 +215,7 @@ func attemptBasicAuth(r *http.Request, guiCfg config.GUIConfiguration, ldapCfg c return usernameFromIso, true } - emitLoginAttempt(false, username, r.RemoteAddr, evLogger) + emitLoginAttempt(false, username, r, evLogger) antiBruteForceSleep() return "", false } diff --git a/lib/api/tokenmanager.go b/lib/api/tokenmanager.go index 38cd24415..904b070bd 100644 --- a/lib/api/tokenmanager.go +++ b/lib/api/tokenmanager.go @@ -189,7 +189,7 @@ func (m *tokenCookieManager) createSession(username string, persistent bool, w h Path: "/", }) - emitLoginAttempt(true, username, r.RemoteAddr, m.evLogger) + emitLoginAttempt(true, username, r, m.evLogger) } func (m *tokenCookieManager) hasValidSession(r *http.Request) bool { diff --git a/lib/osutil/net.go b/lib/osutil/net.go index ce308d999..fd7e6b1e2 100644 --- a/lib/osutil/net.go +++ b/lib/osutil/net.go @@ -8,6 +8,7 @@ package osutil import ( "net" + "strings" ) // GetInterfaceAddrs returns the IP networks of all interfaces that are up. @@ -46,6 +47,17 @@ func GetInterfaceAddrs(includePtP bool) ([]*net.IPNet, error) { return nets, nil } +func IPFromString(addr string) net.IP { + // strip the port + host, _, err := net.SplitHostPort(addr) + if err != nil { + host = addr + } + // strip IPv6 zone identifier + host, _, _ = strings.Cut(host, "%") + return net.ParseIP(host) +} + func IPFromAddr(addr net.Addr) (net.IP, error) { switch a := addr.(type) { case *net.TCPAddr: diff --git a/lib/osutil/osutil_test.go b/lib/osutil/osutil_test.go index 6405e150c..81cc4e1c5 100644 --- a/lib/osutil/osutil_test.go +++ b/lib/osutil/osutil_test.go @@ -135,3 +135,35 @@ func TestRenameOrCopy(t *testing.T) { } } } + +func TestIPFromString(t *testing.T) { + t.Parallel() + + cases := []struct { + in string + out string + }{ + {"192.168.178.1", "192.168.178.1"}, + {"192.168.178.1:8384", "192.168.178.1"}, + {"fe80::20c:29ff:fe9a:46d2", "fe80::20c:29ff:fe9a:46d2"}, + {"[fe80::20c:29ff:fe9a:46d2]:8384", "fe80::20c:29ff:fe9a:46d2"}, + {"[fe80::20c:29ff:fe9a:46d2%eno1]:8384", "fe80::20c:29ff:fe9a:46d2"}, + {"google.com", ""}, + {"1.1.1.1.1", ""}, + {"", ""}, + } + + for _, c := range cases { + ip := osutil.IPFromString(c.in) + var address string + if ip != nil { + address = ip.String() + } else { + address = "" + } + + if c.out != address { + t.Fatalf("result should be %s != %s", c.out, address) + } + } +}