From 2aee9a66c70af929e28653245eb73c0f29a46e97 Mon Sep 17 00:00:00 2001 From: Stanislav Chzhen Date: Mon, 11 Mar 2024 18:17:58 +0300 Subject: [PATCH] home: real ip in logs --- internal/home/auth.go | 31 +++++++++------ internal/home/auth_internal_test.go | 6 +-- internal/home/authhttp.go | 50 +++++++++++++++++-------- internal/home/authhttp_internal_test.go | 14 +++---- internal/home/home.go | 4 +- 5 files changed, 67 insertions(+), 38 deletions(-) diff --git a/internal/home/auth.go b/internal/home/auth.go index 72d52a57..00f6b279 100644 --- a/internal/home/auth.go +++ b/internal/home/auth.go @@ -11,6 +11,7 @@ import ( "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" + "github.com/AdguardTeam/golibs/netutil" "go.etcd.io/bbolt" "golang.org/x/crypto/bcrypt" ) @@ -53,12 +54,13 @@ func (s *session) deserialize(data []byte) bool { // Auth - global object type Auth struct { - db *bbolt.DB - rateLimiter *authRateLimiter - sessions map[string]*session - users []webUser - lock sync.Mutex - sessionTTL uint32 + db *bbolt.DB + rateLimiter *authRateLimiter + sessions map[string]*session + users []webUser + trustedProxies []netutil.Prefix + lock sync.Mutex + sessionTTL uint32 } // webUser represents a user of the Web UI. @@ -70,14 +72,21 @@ type webUser struct { } // InitAuth - create a global object -func InitAuth(dbFilename string, users []webUser, sessionTTL uint32, rateLimiter *authRateLimiter) *Auth { +func InitAuth( + dbFilename string, + users []webUser, + sessionTTL uint32, + rateLimiter *authRateLimiter, + trustedProxies []netutil.Prefix, +) *Auth { log.Info("Initializing auth module: %s", dbFilename) a := &Auth{ - sessionTTL: sessionTTL, - rateLimiter: rateLimiter, - sessions: make(map[string]*session), - users: users, + sessionTTL: sessionTTL, + rateLimiter: rateLimiter, + sessions: make(map[string]*session), + users: users, + trustedProxies: trustedProxies, } var err error a.db, err = bbolt.Open(dbFilename, 0o644, nil) diff --git a/internal/home/auth_internal_test.go b/internal/home/auth_internal_test.go index eae8095c..6be17061 100644 --- a/internal/home/auth_internal_test.go +++ b/internal/home/auth_internal_test.go @@ -37,7 +37,7 @@ func TestAuth(t *testing.T) { Name: "name", PasswordHash: "$2y$05$..vyzAECIhJPfaQiOK17IukcQnqEgKJHy0iETyYqxn3YXJl8yZuo2", }} - a := InitAuth(fn, nil, 60, nil) + a := InitAuth(fn, nil, 60, nil, nil) s := session{} user := webUser{Name: "name"} @@ -66,7 +66,7 @@ func TestAuth(t *testing.T) { a.Close() // load saved session - a = InitAuth(fn, users, 60, nil) + a = InitAuth(fn, users, 60, nil, nil) // the session is still alive assert.Equal(t, checkSessionOK, a.checkSession(sessStr)) @@ -82,7 +82,7 @@ func TestAuth(t *testing.T) { time.Sleep(3 * time.Second) // load and remove expired sessions - a = InitAuth(fn, users, 60, nil) + a = InitAuth(fn, users, 60, nil, nil) assert.Equal(t, checkSessionNotFound, a.checkSession(sessStr)) a.Close() diff --git a/internal/home/authhttp.go b/internal/home/authhttp.go index d04e0ca3..a80c1260 100644 --- a/internal/home/authhttp.go +++ b/internal/home/authhttp.go @@ -4,8 +4,8 @@ import ( "encoding/hex" "encoding/json" "fmt" - "net" "net/http" + "net/netip" "path" "strconv" "strings" @@ -78,7 +78,7 @@ func (a *Auth) newCookie(req loginJSON, addr string) (c *http.Cookie, err error) // a well-maintained third-party module. // // TODO(a.garipov): Support header Forwarded from RFC 7329. -func realIP(r *http.Request) (ip net.IP, err error) { +func realIP(r *http.Request) (ip netip.Addr, err error) { proxyHeaders := []string{ httphdr.CFConnectingIP, httphdr.TrueClientIP, @@ -87,8 +87,8 @@ func realIP(r *http.Request) (ip net.IP, err error) { for _, h := range proxyHeaders { v := r.Header.Get(h) - ip = net.ParseIP(v) - if ip != nil { + ip, err = netip.ParseAddr(v) + if err == nil { return ip, nil } } @@ -97,8 +97,8 @@ func realIP(r *http.Request) (ip net.IP, err error) { // from the X-Forwarded-For header. s := r.Header.Get(httphdr.XForwardedFor) ipStrs := strings.SplitN(s, ", ", 2) - ip = net.ParseIP(ipStrs[0]) - if ip != nil { + ip, err = netip.ParseAddr(ipStrs[0]) + if err == nil { return ip, nil } @@ -106,10 +106,10 @@ func realIP(r *http.Request) (ip net.IP, err error) { // by the stdlib. ipStr, err := netutil.SplitHost(r.RemoteAddr) if err != nil { - return nil, fmt.Errorf("getting ip from client addr: %w", err) + return netip.Addr{}, fmt.Errorf("getting ip from client addr: %w", err) } - return net.ParseIP(ipStr), nil + return netip.ParseAddr(ipStr) } // writeErrorWithIP is like [aghhttp.Error], but includes the remote IP address @@ -173,20 +173,25 @@ func handleLogin(w http.ResponseWriter, r *http.Request) { } } - cookie, err := Context.auth.newCookie(req, remoteIP) - if err != nil { - writeErrorWithIP(r, w, http.StatusForbidden, remoteIP, "%s", err) - - return - } - // Use realIP here, since this IP address is only used for logging. ip, err := realIP(r) if err != nil { log.Error("auth: getting real ip from request with remote ip %s: %s", remoteIP, err) } - log.Info("auth: user %q successfully logged in from ip %v", req.Name, ip) + cookie, err := Context.auth.newCookie(req, remoteIP) + if err != nil { + logIP := remoteIP + if isTrustedIP(ip, Context.auth.trustedProxies) { + logIP = ip.String() + } + + writeErrorWithIP(r, w, http.StatusForbidden, logIP, "%s", err) + + return + } + + log.Info("auth: user %q successfully logged in from ip %s", req.Name, ip) http.SetCookie(w, cookie) @@ -198,6 +203,19 @@ func handleLogin(w http.ResponseWriter, r *http.Request) { aghhttp.OK(w) } +// isTrustedIP returns true if the trustedProxies include ip. +func isTrustedIP(ip netip.Addr, trustedProxies []netutil.Prefix) (ok bool) { + ip = ip.Unmap() + + for _, p := range trustedProxies { + if p.Contains(ip) { + return true + } + } + + return false +} + // handleLogout is the handler for the GET /control/logout HTTP API. func handleLogout(w http.ResponseWriter, r *http.Request) { respHdr := w.Header() diff --git a/internal/home/authhttp_internal_test.go b/internal/home/authhttp_internal_test.go index aceefa97..db6ce34e 100644 --- a/internal/home/authhttp_internal_test.go +++ b/internal/home/authhttp_internal_test.go @@ -1,8 +1,8 @@ package home import ( - "net" "net/http" + "net/netip" "net/textproto" "net/url" "path/filepath" @@ -39,7 +39,7 @@ func TestAuthHTTP(t *testing.T) { users := []webUser{ {Name: "name", PasswordHash: "$2y$05$..vyzAECIhJPfaQiOK17IukcQnqEgKJHy0iETyYqxn3YXJl8yZuo2"}, } - Context.auth = InitAuth(fn, users, 60, nil) + Context.auth = InitAuth(fn, users, 60, nil, nil) handlerCalled := false handler := func(_ http.ResponseWriter, _ *http.Request) { @@ -125,13 +125,13 @@ func TestRealIP(t *testing.T) { header http.Header remoteAddr string wantErrMsg string - wantIP net.IP + wantIP netip.Addr }{{ name: "success_no_proxy", header: nil, remoteAddr: remoteAddr, wantErrMsg: "", - wantIP: net.IPv4(1, 2, 3, 4), + wantIP: netip.MustParseAddr("1.2.3.4"), }, { name: "success_proxy", header: http.Header{ @@ -139,7 +139,7 @@ func TestRealIP(t *testing.T) { }, remoteAddr: remoteAddr, wantErrMsg: "", - wantIP: net.IPv4(1, 2, 3, 5), + wantIP: netip.MustParseAddr("1.2.3.5"), }, { name: "success_proxy_multiple", header: http.Header{ @@ -149,14 +149,14 @@ func TestRealIP(t *testing.T) { }, remoteAddr: remoteAddr, wantErrMsg: "", - wantIP: net.IPv4(1, 2, 3, 6), + wantIP: netip.MustParseAddr("1.2.3.6"), }, { name: "error_no_proxy", header: nil, remoteAddr: "1:::2", wantErrMsg: `getting ip from client addr: address 1:::2: ` + `too many colons in address`, - wantIP: nil, + wantIP: netip.Addr{}, }} for _, tc := range testCases { diff --git a/internal/home/home.go b/internal/home/home.go index 4cc91716..bd41950f 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -667,8 +667,10 @@ func initUsers() (auth *Auth, err error) { log.Info("authratelimiter is disabled") } + trustedProxies := slices.Clone(config.DNS.TrustedProxies) + sessionTTL := config.HTTPConfig.SessionTTL.Seconds() - auth = InitAuth(sessFilename, config.Users, uint32(sessionTTL), rateLimiter) + auth = InitAuth(sessFilename, config.Users, uint32(sessionTTL), rateLimiter, trustedProxies) if auth == nil { return nil, errors.Error("initializing auth module failed") }