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 <e.burkov@adguard.com>
Date:   Tue Apr 27 15:35:49 2021 +0300

    home: fix mistake

commit dfa2ab05e9a8e70ac1bec36c4eb8ef3b02283b92
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Tue Apr 27 15:31:53 2021 +0300

    home: imp code

commit ff4220d3c3d92ae604e92a0c5c274d9527350d99
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Tue Apr 27 15:14:20 2021 +0300

    home: imp authratelimiter

commit c73a407d8652d64957e35046dbae7168aa45202f
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Tue Apr 27 14:20:17 2021 +0300

    home: fix authratelimiter

commit 724db4380928b055f9995006cf421cbe9c5363e7
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Fri Apr 23 12:15:48 2021 +0300

    home: introduce auth blocker

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

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 := &Auth{
-	a.sessionTTL = sessionTTL
+		sessionTTL: sessionTTL,
-	a.sessions = make(map[string]*session)
+		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)

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)
 

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)
+}

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)
+}

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
+	// AuthAttempts is the maximum number of failed login attempts a user
-	Language     string `yaml:"language"`       // two-letter ISO 639-1 language code
+	// can do before being blocked.
-	RlimitNoFile uint   `yaml:"rlimit_nofile"`  // Maximum number of opened fd's per process (0: default)
+	AuthAttempts uint `yaml:"auth_attempts"`
-	DebugPProf   bool   `yaml:"debug_pprof"`    // Enable pprof HTTP server on port 6060
+	// 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,

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")
 	}

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.

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':