From f603c21b557c37f4db7afefb3476127ca23d44a9 Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Tue, 27 Apr 2021 18:56:32 +0300 Subject: [PATCH] Pull request: 2826 auth block Merge in DNS/adguard-home from 2826-auth-block to master Updates #2826. Squashed commit of the following: commit ae87360379270012869ad2bc4e528e07eb9af91e Author: Eugene Burkov Date: Tue Apr 27 15:35:49 2021 +0300 home: fix mistake commit dfa2ab05e9a8e70ac1bec36c4eb8ef3b02283b92 Author: Eugene Burkov Date: Tue Apr 27 15:31:53 2021 +0300 home: imp code commit ff4220d3c3d92ae604e92a0c5c274d9527350d99 Author: Eugene Burkov Date: Tue Apr 27 15:14:20 2021 +0300 home: imp authratelimiter commit c73a407d8652d64957e35046dbae7168aa45202f Author: Eugene Burkov Date: Tue Apr 27 14:20:17 2021 +0300 home: fix authratelimiter commit 724db4380928b055f9995006cf421cbe9c5363e7 Author: Eugene Burkov Date: Fri Apr 23 12:15:48 2021 +0300 home: introduce auth blocker --- CHANGELOG.md | 3 + internal/home/auth.go | 63 ++++++-- internal/home/auth_test.go | 10 +- internal/home/authratelimiter.go | 109 ++++++++++++++ internal/home/authratelimiter_test.go | 207 ++++++++++++++++++++++++++ internal/home/config.go | 16 +- internal/home/home.go | 16 +- openapi/CHANGELOG.md | 7 +- openapi/openapi.yaml | 6 + 9 files changed, 415 insertions(+), 22 deletions(-) create mode 100644 internal/home/authratelimiter.go create mode 100644 internal/home/authratelimiter_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b9578c7f..0d52b2e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ and this project adheres to ### Added +- The ability to block user for login after configurable number of unsuccessful + attempts for configurable time ([#2826]). - `$denyallow` modifier for filters ([#2923]). - Hostname uniqueness validation in the DHCP server ([#2952]). - Hostname generating for DHCP clients which don't provide their own ([#2723]). @@ -81,6 +83,7 @@ and this project adheres to [#2704]: https://github.com/AdguardTeam/AdGuardHome/issues/2704 [#2723]: https://github.com/AdguardTeam/AdGuardHome/issues/2723 [#2824]: https://github.com/AdguardTeam/AdGuardHome/issues/2824 +[#2826]: https://github.com/AdguardTeam/AdGuardHome/issues/2826 [#2828]: https://github.com/AdguardTeam/AdGuardHome/issues/2828 [#2835]: https://github.com/AdguardTeam/AdGuardHome/issues/2835 [#2838]: https://github.com/AdguardTeam/AdGuardHome/issues/2838 diff --git a/internal/home/auth.go b/internal/home/auth.go index 63a66a76..80c1a3a0 100644 --- a/internal/home/auth.go +++ b/internal/home/auth.go @@ -8,6 +8,7 @@ import ( "fmt" "net" "net/http" + "strconv" "strings" "sync" "time" @@ -62,6 +63,7 @@ func (s *session) deserialize(data []byte) bool { // Auth - global object type Auth struct { db *bbolt.DB + blocker *authRateLimiter sessions map[string]*session users []User lock sync.Mutex @@ -75,12 +77,15 @@ type User struct { } // InitAuth - create a global object -func InitAuth(dbFilename string, users []User, sessionTTL uint32) *Auth { +func InitAuth(dbFilename string, users []User, sessionTTL uint32, blocker *authRateLimiter) *Auth { log.Info("Initializing auth module: %s", dbFilename) - a := Auth{} - a.sessionTTL = sessionTTL - a.sessions = make(map[string]*session) + a := &Auth{ + sessionTTL: sessionTTL, + blocker: blocker, + sessions: make(map[string]*session), + users: users, + } var err error a.db, err = bbolt.Open(dbFilename, 0o644, nil) if err != nil { @@ -92,10 +97,9 @@ func InitAuth(dbFilename string, users []User, sessionTTL uint32) *Auth { return nil } a.loadSessions() - a.users = users log.Info("auth: initialized. users:%d sessions:%d", len(a.users), len(a.sessions)) - return &a + return a } // Close - close module @@ -330,13 +334,23 @@ func cookieExpiryFormat(exp time.Time) (formatted string) { return exp.Format(cookieTimeFormat) } -func (a *Auth) httpCookie(req loginJSON) (string, error) { +func (a *Auth) httpCookie(req loginJSON, addr string) (cookie string, err error) { + blocker := a.blocker u := a.UserFind(req.Name, req.Password) if len(u.Name) == 0 { - return "", nil + if blocker != nil { + blocker.inc(addr) + } + + return "", err } - sess, err := newSessionToken() + if blocker != nil { + blocker.remove(addr) + } + + var sess []byte + sess, err = newSessionToken() if err != nil { return "", err } @@ -404,10 +418,38 @@ func handleLogin(w http.ResponseWriter, r *http.Request) { err := json.NewDecoder(r.Body).Decode(&req) if err != nil { httpError(w, http.StatusBadRequest, "json decode: %s", err) + return } - cookie, err := Context.auth.httpCookie(req) + var remoteAddr string + // The realIP couldn't be used here due to security issues. + // + // See https://github.com/AdguardTeam/AdGuardHome/issues/2799. + // + // TODO(e.burkov): Use realIP when the issue will be fixed. + if remoteAddr, err = aghnet.SplitHost(r.RemoteAddr); err != nil { + httpError(w, http.StatusBadRequest, "auth: getting remote address: %s", err) + + return + } + + if blocker := Context.auth.blocker; blocker != nil { + if left := blocker.check(remoteAddr); left > 0 { + w.Header().Set("Retry-After", strconv.Itoa(int(left.Seconds()))) + httpError( + w, + http.StatusTooManyRequests, + "auth: blocked for %s", + left, + ) + + return + } + } + + var cookie string + cookie, err = Context.auth.httpCookie(req, remoteAddr) if err != nil { httpError(w, http.StatusBadRequest, "crypto rand reader: %s", err) @@ -425,7 +467,6 @@ func handleLogin(w http.ResponseWriter, r *http.Request) { } else { log.Info("auth: failed to login user %q from ip %q", req.Name, ip) } - time.Sleep(1 * time.Second) http.Error(w, "invalid username or password", http.StatusBadRequest) diff --git a/internal/home/auth_test.go b/internal/home/auth_test.go index 04212450..f6b2b106 100644 --- a/internal/home/auth_test.go +++ b/internal/home/auth_test.go @@ -48,7 +48,7 @@ func TestAuth(t *testing.T) { Name: "name", PasswordHash: "$2y$05$..vyzAECIhJPfaQiOK17IukcQnqEgKJHy0iETyYqxn3YXJl8yZuo2", }} - a := InitAuth(fn, nil, 60) + a := InitAuth(fn, nil, 60, nil) s := session{} user := User{Name: "name"} @@ -76,7 +76,7 @@ func TestAuth(t *testing.T) { a.Close() // load saved session - a = InitAuth(fn, users, 60) + a = InitAuth(fn, users, 60, nil) // the session is still alive assert.Equal(t, checkSessionOK, a.checkSession(sessStr)) @@ -91,7 +91,7 @@ func TestAuth(t *testing.T) { time.Sleep(3 * time.Second) // load and remove expired sessions - a = InitAuth(fn, users, 60) + a = InitAuth(fn, users, 60, nil) assert.Equal(t, checkSessionNotFound, a.checkSession(sessStr)) a.Close() @@ -122,7 +122,7 @@ func TestAuthHTTP(t *testing.T) { users := []User{ {Name: "name", PasswordHash: "$2y$05$..vyzAECIhJPfaQiOK17IukcQnqEgKJHy0iETyYqxn3YXJl8yZuo2"}, } - Context.auth = InitAuth(fn, users, 60) + Context.auth = InitAuth(fn, users, 60, nil) handlerCalled := false handler := func(_ http.ResponseWriter, _ *http.Request) { @@ -151,7 +151,7 @@ func TestAuthHTTP(t *testing.T) { assert.True(t, handlerCalled) // perform login - cookie, err := Context.auth.httpCookie(loginJSON{Name: "name", Password: "password"}) + cookie, err := Context.auth.httpCookie(loginJSON{Name: "name", Password: "password"}, "") assert.Nil(t, err) assert.NotEmpty(t, cookie) diff --git a/internal/home/authratelimiter.go b/internal/home/authratelimiter.go new file mode 100644 index 00000000..c0b3da40 --- /dev/null +++ b/internal/home/authratelimiter.go @@ -0,0 +1,109 @@ +package home + +import ( + "sync" + "time" +) + +// failedAuthTTL is the period of time for which the failed attempt will stay in +// cache. +const failedAuthTTL = 1 * time.Minute + +// failedAuth is an entry of authRateLimiter's cache. +type failedAuth struct { + until time.Time + num uint +} + +// authRateLimiter used to cache failed authentication attempts. +type authRateLimiter struct { + failedAuths map[string]failedAuth + // failedAuthsLock protects failedAuths. + failedAuthsLock sync.Mutex + blockDur time.Duration + maxAttempts uint +} + +// newAuthRateLimiter returns properly initialized *authRateLimiter. +func newAuthRateLimiter(blockDur time.Duration, maxAttempts uint) (ab *authRateLimiter) { + return &authRateLimiter{ + failedAuths: make(map[string]failedAuth), + blockDur: blockDur, + maxAttempts: maxAttempts, + } +} + +// cleanupLocked checks each blocked users removing ones with expired TTL. For +// internal use only. +func (ab *authRateLimiter) cleanupLocked(now time.Time) { + for k, v := range ab.failedAuths { + if now.After(v.until) { + delete(ab.failedAuths, k) + } + } +} + +// checkLocked checks the attempter for it's state. For internal use only. +func (ab *authRateLimiter) checkLocked(usrID string, now time.Time) (left time.Duration) { + a, ok := ab.failedAuths[usrID] + if !ok { + return 0 + } + + if a.num < ab.maxAttempts { + return 0 + } + + return a.until.Sub(now) +} + +// check returns the time left until unblocking. The nonpositive result should +// be interpreted as not blocked attempter. +func (ab *authRateLimiter) check(usrID string) (left time.Duration) { + now := time.Now() + + ab.failedAuthsLock.Lock() + defer ab.failedAuthsLock.Unlock() + + ab.cleanupLocked(now) + return ab.checkLocked(usrID, now) +} + +// incLocked increments the number of unsuccessful attempts for attempter with +// ip and updates it's blocking moment if needed. For internal use only. +func (ab *authRateLimiter) incLocked(usrID string, now time.Time) { + var until time.Time = now.Add(failedAuthTTL) + var attNum uint = 1 + + a, ok := ab.failedAuths[usrID] + if ok { + until = a.until + attNum = a.num + 1 + } + if attNum >= ab.maxAttempts { + until = now.Add(ab.blockDur) + } + + ab.failedAuths[usrID] = failedAuth{ + num: attNum, + until: until, + } +} + +// inc updates the failed attempt in cache. +func (ab *authRateLimiter) inc(usrID string) { + now := time.Now() + + ab.failedAuthsLock.Lock() + defer ab.failedAuthsLock.Unlock() + + ab.incLocked(usrID, now) +} + +// remove stops any tracking and any blocking of the user. +func (ab *authRateLimiter) remove(usrID string) { + ab.failedAuthsLock.Lock() + defer ab.failedAuthsLock.Unlock() + + delete(ab.failedAuths, usrID) +} diff --git a/internal/home/authratelimiter_test.go b/internal/home/authratelimiter_test.go new file mode 100644 index 00000000..b30afd69 --- /dev/null +++ b/internal/home/authratelimiter_test.go @@ -0,0 +1,207 @@ +package home + +import ( + "net" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAuthRateLimiter_Cleanup(t *testing.T) { + const key = "some-key" + now := time.Now() + + testCases := []struct { + name string + att failedAuth + wantExp bool + }{{ + name: "expired", + att: failedAuth{ + until: now.Add(-100 * time.Hour), + }, + wantExp: true, + }, { + name: "nope_yet", + att: failedAuth{ + until: now.Add(failedAuthTTL / 2), + }, + wantExp: false, + }, { + name: "blocked", + att: failedAuth{ + until: now.Add(100 * time.Hour), + }, + wantExp: false, + }} + + for _, tc := range testCases { + ab := &authRateLimiter{ + failedAuths: map[string]failedAuth{ + key: tc.att, + }, + } + t.Run(tc.name, func(t *testing.T) { + ab.cleanupLocked(now) + if tc.wantExp { + assert.Empty(t, ab.failedAuths) + + return + } + + require.Len(t, ab.failedAuths, 1) + + _, ok := ab.failedAuths[key] + require.True(t, ok) + }) + } +} + +func TestAuthRateLimiter_Check(t *testing.T) { + key := string(net.IP{127, 0, 0, 1}) + const maxAtt = 1 + now := time.Now() + + testCases := []struct { + until time.Time + name string + num uint + wantExp bool + }{{ + until: now.Add(-100 * time.Hour), + name: "expired", + num: 0, + wantExp: true, + }, { + until: now.Add(failedAuthTTL), + name: "not_blocked_but_tracked", + num: 0, + wantExp: true, + }, { + until: now, + name: "expired_but_stayed", + num: 2, + wantExp: true, + }, { + until: now.Add(100 * time.Hour), + name: "blocked", + num: 2, + wantExp: false, + }} + + for _, tc := range testCases { + failedAuths := map[string]failedAuth{ + key: { + num: tc.num, + until: tc.until, + }, + } + ab := &authRateLimiter{ + maxAttempts: maxAtt, + failedAuths: failedAuths, + } + t.Run(tc.name, func(t *testing.T) { + until := ab.check(key) + + if tc.wantExp { + assert.LessOrEqual(t, until, time.Duration(0)) + } else { + assert.Greater(t, until, time.Duration(0)) + } + }) + } + + t.Run("non-existent", func(t *testing.T) { + ab := &authRateLimiter{ + failedAuths: map[string]failedAuth{ + key + "smthng": {}, + }, + } + + until := ab.check(key) + + assert.Zero(t, until) + }) +} + +func TestAuthRateLimiter_Inc(t *testing.T) { + ip := net.IP{127, 0, 0, 1} + key := string(ip) + now := time.Now() + const maxAtt = 2 + const blockDur = 15 * time.Minute + + testCases := []struct { + until time.Time + wantUntil time.Time + name string + num uint + wantNum uint + }{{ + name: "only_inc", + until: now, + wantUntil: now, + num: maxAtt - 1, + wantNum: maxAtt, + }, { + name: "inc_and_block", + until: now, + wantUntil: now.Add(failedAuthTTL), + num: maxAtt, + wantNum: maxAtt + 1, + }} + + for _, tc := range testCases { + failedAuths := map[string]failedAuth{ + key: { + num: tc.num, + until: tc.until, + }, + } + ab := &authRateLimiter{ + blockDur: blockDur, + maxAttempts: maxAtt, + failedAuths: failedAuths, + } + t.Run(tc.name, func(t *testing.T) { + ab.inc(key) + + a, ok := ab.failedAuths[key] + require.True(t, ok) + + assert.Equal(t, tc.wantNum, a.num) + assert.LessOrEqual(t, tc.wantUntil.Unix(), a.until.Unix()) + }) + } + + t.Run("non-existent", func(t *testing.T) { + ab := &authRateLimiter{ + blockDur: blockDur, + maxAttempts: maxAtt, + failedAuths: map[string]failedAuth{}, + } + + ab.inc(key) + + a, ok := ab.failedAuths[key] + require.True(t, ok) + assert.EqualValues(t, 1, a.num) + }) +} + +func TestAuthRateLimiter_Remove(t *testing.T) { + const key = "some-key" + + failedAuths := map[string]failedAuth{ + key: {}, + } + ab := &authRateLimiter{ + failedAuths: failedAuths, + } + + ab.remove(key) + + assert.Empty(t, ab.failedAuths) +} diff --git a/internal/home/config.go b/internal/home/config.go index 853e2483..4fafee75 100644 --- a/internal/home/config.go +++ b/internal/home/config.go @@ -47,10 +47,16 @@ type configuration struct { BindPort int `yaml:"bind_port"` // BindPort is the port the HTTP server BetaBindPort int `yaml:"beta_bind_port"` // BetaBindPort is the port for new client Users []User `yaml:"users"` // Users that can access HTTP server - ProxyURL string `yaml:"http_proxy"` // Proxy address for our HTTP client - Language string `yaml:"language"` // two-letter ISO 639-1 language code - RlimitNoFile uint `yaml:"rlimit_nofile"` // Maximum number of opened fd's per process (0: default) - DebugPProf bool `yaml:"debug_pprof"` // Enable pprof HTTP server on port 6060 + // AuthAttempts is the maximum number of failed login attempts a user + // can do before being blocked. + AuthAttempts uint `yaml:"auth_attempts"` + // AuthBlockMin is the duration, in minutes, of the block of new login + // attempts after AuthAttempts unsuccessful login attempts. + AuthBlockMin uint `yaml:"block_auth_min"` + ProxyURL string `yaml:"http_proxy"` // Proxy address for our HTTP client + Language string `yaml:"language"` // two-letter ISO 639-1 language code + RlimitNoFile uint `yaml:"rlimit_nofile"` // Maximum number of opened fd's per process (0: default) + DebugPProf bool `yaml:"debug_pprof"` // Enable pprof HTTP server on port 6060 // TTL for a web session (in hours) // An active session is automatically refreshed once a day. @@ -137,6 +143,8 @@ var config = configuration{ BindPort: 3000, BetaBindPort: 0, BindHost: net.IP{0, 0, 0, 0}, + AuthAttempts: 5, + AuthBlockMin: 15, DNS: dnsConfig{ BindHosts: []net.IP{{0, 0, 0, 0}}, Port: 53, diff --git a/internal/home/home.go b/internal/home/home.go index 21c78fe9..b647b10c 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -282,7 +282,21 @@ func run(args options) { sessFilename := filepath.Join(Context.getDataDir(), "sessions.db") GLMode = args.glinetMode - Context.auth = InitAuth(sessFilename, config.Users, config.WebSessionTTLHours*60*60) + var arl *authRateLimiter + if config.AuthAttempts > 0 && config.AuthBlockMin > 0 { + arl = newAuthRateLimiter( + time.Duration(config.AuthBlockMin)*time.Minute, + config.AuthAttempts, + ) + } else { + log.Info("authratelimiter is disabled") + } + Context.auth = InitAuth( + sessFilename, + config.Users, + config.WebSessionTTLHours*60*60, + arl, + ) if Context.auth == nil { log.Fatalf("Couldn't initialize Auth module") } diff --git a/openapi/CHANGELOG.md b/openapi/CHANGELOG.md index 071c4177..7ee40a5b 100644 --- a/openapi/CHANGELOG.md +++ b/openapi/CHANGELOG.md @@ -4,7 +4,12 @@ ## v0.106: API changes -## New `"private_upstream"` field in `POST /test_upstream_dns` +### New response code in `POST /control/login` + +* `429` is returned when user is out of login attempts. It adds the + `Retry-After` header with the number of seconds of block left in it. + +### New `"private_upstream"` field in `POST /test_upstream_dns` * The new optional field `"private_upstream"` of `UpstreamConfig` contains the upstream servers for resolving locally-served ip addresses to be checked. diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index 7c9aeaea..3a0df747 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -1078,6 +1078,12 @@ 'responses': '200': 'description': 'OK.' + '400': + 'description': > + Invalid username or password. + '429': + 'description': > + Out of login attempts. '/logout': 'get': 'tags':