From 43cb1f1bffc0e128f250bbc134924e80c191ba73 Mon Sep 17 00:00:00 2001 From: exterr2f Date: Fri, 14 Feb 2025 11:41:34 +0100 Subject: [PATCH 1/4] Fix SessionMaxAge condition to correctly apply valid values --- internal/auth/auth.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 15b6532..290463f 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -88,7 +88,7 @@ func Init() { authInstance.sessionStore = sessions.NewCookieStore(bytes) } - if d, err := time.ParseDuration(config.Keys.SessionMaxAge); err != nil { + if d, err := time.ParseDuration(config.Keys.SessionMaxAge); err == nil { authInstance.SessionMaxAge = d } From b6b37ee68bda922bb7bd8a2831debe3b44cd0494 Mon Sep 17 00:00:00 2001 From: exterr2f Date: Fri, 14 Feb 2025 12:41:28 +0100 Subject: [PATCH 2/4] Add Rate Limiting based on IP and username --- go.mod | 1 + go.sum | 2 ++ internal/auth/auth.go | 50 +++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 84cdf7d..67bf615 100644 --- a/go.mod +++ b/go.mod @@ -29,6 +29,7 @@ require ( golang.org/x/crypto v0.32.0 golang.org/x/exp v0.0.0-20240707233637-46b078467d37 golang.org/x/oauth2 v0.21.0 + golang.org/x/time v0.10.0 ) require ( diff --git a/go.sum b/go.sum index 07aaafd..c45aca5 100644 --- a/go.sum +++ b/go.sum @@ -289,6 +289,8 @@ golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo= golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ= +golang.org/x/time v0.10.0 h1:3usCWA8tQn0L8+hFJQNgzpWbd89begxN66o1Ojdn5L4= +golang.org/x/time v0.10.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 290463f..1892b0e 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -10,11 +10,14 @@ import ( "database/sql" "encoding/base64" "errors" + "net" "net/http" "os" "sync" "time" + "golang.org/x/time/rate" + "github.com/ClusterCockpit/cc-backend/internal/config" "github.com/ClusterCockpit/cc-backend/internal/repository" "github.com/ClusterCockpit/cc-backend/pkg/log" @@ -32,6 +35,29 @@ var ( authInstance *Authentication ) +var ( + ipLimiters sync.Map + usernameLimiters sync.Map +) + +func getIPLimiter(ip string) *rate.Limiter { + limiter, ok := ipLimiters.Load(ip) + if !ok { + limiter = rate.NewLimiter(rate.Every(time.Minute/5), 5) + ipLimiters.Store(ip, limiter) + } + return limiter.(*rate.Limiter) +} + +func getUserLimiter(username string) *rate.Limiter { + limiter, ok := usernameLimiters.Load(username) + if !ok { + limiter = rate.NewLimiter(rate.Every(time.Hour/10), 10) + usernameLimiters.Store(username, limiter) + } + return limiter.(*rate.Limiter) +} + type Authentication struct { sessionStore *sessions.CookieStore LdapAuth *LdapAuthenticator @@ -208,9 +234,29 @@ func (auth *Authentication) Login( onfailure func(rw http.ResponseWriter, r *http.Request, loginErr error), ) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { - username := r.FormValue("username") - var dbUser *schema.User + ip, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + ip = r.RemoteAddr + } + username := r.FormValue("username") + + ipLimiter := getIPLimiter(ip) + userLimiter := getUserLimiter(username) + + if !ipLimiter.Allow() { + log.Warnf("AUTH/RATE > Too many login attempts from IP %s", ip) + onfailure(rw, r, errors.New("too many login attempts, please try again later")) + return + } + + if !userLimiter.Allow() { + log.Warnf("AUTH/RATE > Too many failed login attempts for user %s", username) + onfailure(rw, r, errors.New("too many login attempts for this user, please try again later")) + return + } + + var dbUser *schema.User if username != "" { var err error dbUser, err = repository.GetUserRepository().GetUser(username) From e1b992526e4218f0795aaf886c2fa15ce192313b Mon Sep 17 00:00:00 2001 From: exterr2f Date: Fri, 14 Feb 2025 20:20:42 +0100 Subject: [PATCH 3/4] Improve rate limiting to combination of IP and username --- internal/auth/auth.go | 42 ++++++++++++------------------------------ 1 file changed, 12 insertions(+), 30 deletions(-) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 1892b0e..6241585 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -35,25 +35,15 @@ var ( authInstance *Authentication ) -var ( - ipLimiters sync.Map - usernameLimiters sync.Map -) +var ipUserLimiters sync.Map -func getIPLimiter(ip string) *rate.Limiter { - limiter, ok := ipLimiters.Load(ip) +func getIPUserLimiter(ip, username string) *rate.Limiter { + key := ip + ":" + username + limiter, ok := ipUserLimiters.Load(key) if !ok { - limiter = rate.NewLimiter(rate.Every(time.Minute/5), 5) - ipLimiters.Store(ip, limiter) - } - return limiter.(*rate.Limiter) -} - -func getUserLimiter(username string) *rate.Limiter { - limiter, ok := usernameLimiters.Load(username) - if !ok { - limiter = rate.NewLimiter(rate.Every(time.Hour/10), 10) - usernameLimiters.Store(username, limiter) + newLimiter := rate.NewLimiter(rate.Every(time.Hour/10), 10) + ipUserLimiters.Store(key, newLimiter) + return newLimiter } return limiter.(*rate.Limiter) } @@ -241,19 +231,11 @@ func (auth *Authentication) Login( username := r.FormValue("username") - ipLimiter := getIPLimiter(ip) - userLimiter := getUserLimiter(username) - - if !ipLimiter.Allow() { - log.Warnf("AUTH/RATE > Too many login attempts from IP %s", ip) - onfailure(rw, r, errors.New("too many login attempts, please try again later")) - return - } - - if !userLimiter.Allow() { - log.Warnf("AUTH/RATE > Too many failed login attempts for user %s", username) - onfailure(rw, r, errors.New("too many login attempts for this user, please try again later")) - return + limiter := getIPUserLimiter(ip, username) + if !limiter.Allow() { + log.Warnf("AUTH/RATE > Too many login attempts for combination IP: %s, Username: %s", ip, username) + onfailure(rw, r, errors.New("Too many login attempts, try again in 1 hour")) + return } var dbUser *schema.User From 7a61bae471b5b87264948cbdb9fafdf6165df22e Mon Sep 17 00:00:00 2001 From: exterr2f Date: Mon, 17 Feb 2025 09:17:27 +0100 Subject: [PATCH 4/4] clarify error message for blocked user --- internal/auth/auth.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 6241585..262204c 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -234,7 +234,7 @@ func (auth *Authentication) Login( limiter := getIPUserLimiter(ip, username) if !limiter.Allow() { log.Warnf("AUTH/RATE > Too many login attempts for combination IP: %s, Username: %s", ip, username) - onfailure(rw, r, errors.New("Too many login attempts, try again in 1 hour")) + onfailure(rw, r, errors.New("Too many login attempts, try again in a few minutes.")) return }