Pull request: HOFTIX-csrf

Merge in DNS/adguard-home from HOFTIX-csrf to master

Squashed commit of the following:

commit 75ab27bf6c52b80ab4e7347d7c254fa659eac244
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Thu Sep 29 18:45:54 2022 +0300

    all: imp cookie security; rm plain-text apis

internal/home/auth.go

@@ -8,12 +8,14 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"path"
 	"strconv"
 	"strings"
 	"sync"
 	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/golibs/timeutil"
@@ -32,7 +34,8 @@ const sessionTokenSize = 16
 
 type session struct {
 	userName string
-	expire   uint32 // expiration time (in seconds)
+	// expire is the expiration time, in seconds.
+	expire uint32
 }
 
 func (s *session) serialize() []byte {
@@ -64,29 +67,29 @@ 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
-	sessionTTL uint32
+	db          *bbolt.DB
+	raleLimiter *authRateLimiter
+	sessions    map[string]*session
+	users       []webUser
+	lock        sync.Mutex
+	sessionTTL  uint32
 }
 
-// User object
-type User struct {
+// webUser represents a user of the Web UI.
+type webUser struct {
 	Name         string `yaml:"name"`
-	PasswordHash string `yaml:"password"` // bcrypt hash
+	PasswordHash string `yaml:"password"`
 }
 
 // InitAuth - create a global object
-func InitAuth(dbFilename string, users []User, sessionTTL uint32, blocker *authRateLimiter) *Auth {
+func InitAuth(dbFilename string, users []webUser, sessionTTL uint32, rateLimiter *authRateLimiter) *Auth {
 	log.Info("Initializing auth module: %s", dbFilename)
 
 	a := &Auth{
-		sessionTTL: sessionTTL,
-		blocker:    blocker,
-		sessions:   make(map[string]*session),
-		users:      users,
+		sessionTTL:  sessionTTL,
+		raleLimiter: rateLimiter,
+		sessions:    make(map[string]*session),
+		users:       users,
 	}
 	var err error
 	a.db, err = bbolt.Open(dbFilename, 0o644, nil)
@@ -326,35 +329,25 @@ func newSessionToken() (data []byte, err error) {
 	return randData, nil
 }
 
-// cookieTimeFormat is the format to be used in (time.Time).Format for cookie's
-// expiry field.
-const cookieTimeFormat = "Mon, 02 Jan 2006 15:04:05 GMT"
-
-// cookieExpiryFormat returns the formatted exp to be used in cookie string.
-// It's quite simple for now, but probably will be expanded in the future.
-func cookieExpiryFormat(exp time.Time) (formatted string) {
-	return exp.Format(cookieTimeFormat)
-}
-
-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 {
-		if blocker != nil {
-			blocker.inc(addr)
+// newCookie creates a new authentication cookie.
+func (a *Auth) newCookie(req loginJSON, addr string) (c *http.Cookie, err error) {
+	rateLimiter := a.raleLimiter
+	u, ok := a.findUser(req.Name, req.Password)
+	if !ok {
+		if rateLimiter != nil {
+			rateLimiter.inc(addr)
 		}
 
-		return "", err
+		return nil, errors.Error("invalid username or password")
 	}
 
-	if blocker != nil {
-		blocker.remove(addr)
+	if rateLimiter != nil {
+		rateLimiter.remove(addr)
 	}
 
-	var sess []byte
-	sess, err = newSessionToken()
+	sess, err := newSessionToken()
 	if err != nil {
-		return "", err
+		return nil, fmt.Errorf("generating token: %w", err)
 	}
 
 	now := time.Now().UTC()
@@ -364,11 +357,15 @@ func (a *Auth) httpCookie(req loginJSON, addr string) (cookie string, err error)
 		expire:   uint32(now.Unix()) + a.sessionTTL,
 	})
 
-	return fmt.Sprintf(
-		"%s=%s; Path=/; HttpOnly; Expires=%s",
-		sessionCookieName, hex.EncodeToString(sess),
-		cookieExpiryFormat(now.Add(cookieTTL)),
-	), nil
+	return &http.Cookie{
+		Name:    sessionCookieName,
+		Value:   hex.EncodeToString(sess),
+		Path:    "/",
+		Expires: now.Add(cookieTTL),
+
+		HttpOnly: true,
+		SameSite: http.SameSiteLaxMode,
+	}, nil
 }
 
 // realIP extracts the real IP address of the client from an HTTP request using
@@ -436,8 +433,8 @@ func handleLogin(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if blocker := Context.auth.blocker; blocker != nil {
-		if left := blocker.check(remoteAddr); left > 0 {
+	if rateLimiter := Context.auth.raleLimiter; rateLimiter != nil {
+		if left := rateLimiter.check(remoteAddr); left > 0 {
 			w.Header().Set("Retry-After", strconv.Itoa(int(left.Seconds())))
 			aghhttp.Error(r, w, http.StatusTooManyRequests, "auth: blocked for %s", left)
 
@@ -445,10 +442,9 @@ func handleLogin(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	var cookie string
-	cookie, err = Context.auth.httpCookie(req, remoteAddr)
+	cookie, err := Context.auth.newCookie(req, remoteAddr)
 	if err != nil {
-		aghhttp.Error(r, w, http.StatusBadRequest, "crypto rand reader: %s", err)
+		aghhttp.Error(r, w, http.StatusForbidden, "%s", err)
 
 		return
 	}
@@ -462,20 +458,11 @@ func handleLogin(w http.ResponseWriter, r *http.Request) {
 		log.Error("auth: unknown ip")
 	}
 
-	if len(cookie) == 0 {
-		log.Info("auth: failed to login user %q from ip %v", req.Name, ip)
-
-		time.Sleep(1 * time.Second)
-
-		http.Error(w, "invalid username or password", http.StatusBadRequest)
-
-		return
-	}
-
 	log.Info("auth: user %q successfully logged in from ip %v", req.Name, ip)
 
+	http.SetCookie(w, cookie)
+
 	h := w.Header()
-	h.Set("Set-Cookie", cookie)
 	h.Set("Cache-Control", "no-store, no-cache, must-revalidate, proxy-revalidate")
 	h.Set("Pragma", "no-cache")
 	h.Set("Expires", "0")
@@ -484,17 +471,31 @@ func handleLogin(w http.ResponseWriter, r *http.Request) {
 }
 
 func handleLogout(w http.ResponseWriter, r *http.Request) {
-	cookie := r.Header.Get("Cookie")
-	sess := parseCookie(cookie)
+	respHdr := w.Header()
+	c, err := r.Cookie(sessionCookieName)
+	if err != nil {
+		// The only error that is returned from r.Cookie is [http.ErrNoCookie].
+		// The user is already logged out.
+		respHdr.Set("Location", "/login.html")
+		w.WriteHeader(http.StatusFound)
 
-	Context.auth.RemoveSession(sess)
+		return
+	}
 
-	w.Header().Set("Location", "/login.html")
+	Context.auth.RemoveSession(c.Value)
 
-	s := fmt.Sprintf("%s=; Path=/; HttpOnly; Expires=Thu, 01 Jan 1970 00:00:00 GMT",
-		sessionCookieName)
-	w.Header().Set("Set-Cookie", s)
+	c = &http.Cookie{
+		Name:    sessionCookieName,
+		Value:   "",
+		Path:    "/",
+		Expires: time.Unix(0, 0),
 
+		HttpOnly: true,
+		SameSite: http.SameSiteLaxMode,
+	}
+
+	respHdr.Set("Location", "/login.html")
+	respHdr.Set("Set-Cookie", c.String())
 	w.WriteHeader(http.StatusFound)
 }
 
@@ -504,101 +505,108 @@ func RegisterAuthHandlers() {
 	httpRegister(http.MethodGet, "/control/logout", handleLogout)
 }
 
-func parseCookie(cookie string) string {
-	pairs := strings.Split(cookie, ";")
-	for _, pair := range pairs {
-		pair = strings.TrimSpace(pair)
-		kv := strings.SplitN(pair, "=", 2)
-		if len(kv) != 2 {
-			continue
-		}
-		if kv[0] == sessionCookieName {
-			return kv[1]
-		}
-	}
-	return ""
-}
-
 // optionalAuthThird return true if user should authenticate first.
-func optionalAuthThird(w http.ResponseWriter, r *http.Request) (authFirst bool) {
-	authFirst = false
+func optionalAuthThird(w http.ResponseWriter, r *http.Request) (mustAuth bool) {
+	if glProcessCookie(r) {
+		log.Debug("auth: authentication is handled by GL-Inet submodule")
+
+		return false
+	}
 
 	// redirect to login page if not authenticated
-	ok := false
+	isAuthenticated := false
 	cookie, err := r.Cookie(sessionCookieName)
-
-	if glProcessCookie(r) {
-		log.Debug("auth: authentication was handled by GL-Inet submodule")
-		ok = true
-	} else if err == nil {
-		r := Context.auth.checkSession(cookie.Value)
-		if r == checkSessionOK {
-			ok = true
-		} else if r < 0 {
-			log.Debug("auth: invalid cookie value: %s", cookie)
-		}
-	} else {
-		// there's no Cookie, check Basic authentication
-		user, pass, ok2 := r.BasicAuth()
-		if ok2 {
-			u := Context.auth.UserFind(user, pass)
-			if len(u.Name) != 0 {
-				ok = true
-			} else {
+	if err != nil {
+		// The only error that is returned from r.Cookie is [http.ErrNoCookie].
+		// Check Basic authentication.
+		user, pass, hasBasic := r.BasicAuth()
+		if hasBasic {
+			_, isAuthenticated = Context.auth.findUser(user, pass)
+			if !isAuthenticated {
 				log.Info("auth: invalid Basic Authorization value")
 			}
 		}
-	}
-	if !ok {
-		if r.URL.Path == "/" || r.URL.Path == "/index.html" {
-			if glProcessRedirect(w, r) {
-				log.Debug("auth: redirected to login page by GL-Inet submodule")
-			} else {
-				w.Header().Set("Location", "/login.html")
-				w.WriteHeader(http.StatusFound)
-			}
-		} else {
-			w.WriteHeader(http.StatusForbidden)
-			_, _ = w.Write([]byte("Forbidden"))
+	} else {
+		res := Context.auth.checkSession(cookie.Value)
+		isAuthenticated = res == checkSessionOK
+		if !isAuthenticated {
+			log.Debug("auth: invalid cookie value: %s", cookie)
 		}
-		authFirst = true
 	}
 
-	return authFirst
+	if isAuthenticated {
+		return false
+	}
+
+	if p := r.URL.Path; p == "/" || p == "/index.html" {
+		if glProcessRedirect(w, r) {
+			log.Debug("auth: redirected to login page by GL-Inet submodule")
+		} else {
+			log.Debug("auth: redirected to login page")
+			w.Header().Set("Location", "/login.html")
+			w.WriteHeader(http.StatusFound)
+		}
+	} else {
+		log.Debug("auth: responded with forbidden to %s %s", r.Method, p)
+		w.WriteHeader(http.StatusForbidden)
+		_, _ = w.Write([]byte("Forbidden"))
+	}
+
+	return true
 }
 
-func optionalAuth(handler func(http.ResponseWriter, *http.Request)) func(http.ResponseWriter, *http.Request) {
+// TODO(a.garipov): Use [http.Handler] consistently everywhere throughout the
+// project.
+func optionalAuth(
+	h func(http.ResponseWriter, *http.Request),
+) (wrapped func(http.ResponseWriter, *http.Request)) {
 	return func(w http.ResponseWriter, r *http.Request) {
-		if r.URL.Path == "/login.html" {
-			// redirect to dashboard if already authenticated
-			authRequired := Context.auth != nil && Context.auth.AuthRequired()
+		p := r.URL.Path
+		authRequired := Context.auth != nil && Context.auth.AuthRequired()
+		if p == "/login.html" {
 			cookie, err := r.Cookie(sessionCookieName)
 			if authRequired && err == nil {
-				r := Context.auth.checkSession(cookie.Value)
-				if r == checkSessionOK {
+				// Redirect to the dashboard if already authenticated.
+				res := Context.auth.checkSession(cookie.Value)
+				if res == checkSessionOK {
 					w.Header().Set("Location", "/")
 					w.WriteHeader(http.StatusFound)
 
 					return
-				} else if r == checkSessionNotFound {
-					log.Debug("auth: invalid cookie value: %s", cookie)
 				}
-			}
 
-		} else if strings.HasPrefix(r.URL.Path, "/assets/") ||
-			strings.HasPrefix(r.URL.Path, "/login.") {
-			// process as usual
-			// no additional auth requirements
-		} else if Context.auth != nil && Context.auth.AuthRequired() {
+				log.Debug("auth: invalid cookie value: %s", cookie)
+			}
+		} else if isPublicResource(p) {
+			// Process as usual, no additional auth requirements.
+		} else if authRequired {
 			if optionalAuthThird(w, r) {
 				return
 			}
 		}
 
-		handler(w, r)
+		h(w, r)
 	}
 }
 
+// isPublicResource returns true if p is a path to a public resource.
+func isPublicResource(p string) (ok bool) {
+	isAsset, err := path.Match("/assets/*", p)
+	if err != nil {
+		// The only error that is returned from path.Match is
+		// [path.ErrBadPattern].  This is a programmer error.
+		panic(fmt.Errorf("bad asset pattern: %w", err))
+	}
+
+	isLogin, err := path.Match("/login.*", p)
+	if err != nil {
+		// Same as above.
+		panic(fmt.Errorf("bad login pattern: %w", err))
+	}
+
+	return isAsset || isLogin
+}
+
 type authHandler struct {
 	handler http.Handler
 }
@@ -612,7 +620,7 @@ func optionalAuthHandler(handler http.Handler) http.Handler {
 }
 
 // UserAdd - add new user
-func (a *Auth) UserAdd(u *User, password string) {
+func (a *Auth) UserAdd(u *webUser, password string) {
 	if len(password) == 0 {
 		return
 	}
@@ -631,31 +639,35 @@ func (a *Auth) UserAdd(u *User, password string) {
 	log.Debug("auth: added user: %s", u.Name)
 }
 
-// UserFind - find a user
-func (a *Auth) UserFind(login, password string) User {
+// findUser returns a user if there is one.
+func (a *Auth) findUser(login, password string) (u webUser, ok bool) {
 	a.lock.Lock()
 	defer a.lock.Unlock()
-	for _, u := range a.users {
+
+	for _, u = range a.users {
 		if u.Name == login &&
 			bcrypt.CompareHashAndPassword([]byte(u.PasswordHash), []byte(password)) == nil {
-			return u
+			return u, true
 		}
 	}
-	return User{}
+
+	return webUser{}, false
 }
 
 // getCurrentUser returns the current user.  It returns an empty User if the
 // user is not found.
-func (a *Auth) getCurrentUser(r *http.Request) User {
+func (a *Auth) getCurrentUser(r *http.Request) (u webUser) {
 	cookie, err := r.Cookie(sessionCookieName)
 	if err != nil {
 		// There's no Cookie, check Basic authentication.
 		user, pass, ok := r.BasicAuth()
 		if ok {
-			return Context.auth.UserFind(user, pass)
+			u, _ = Context.auth.findUser(user, pass)
+
+			return u
 		}
 
-		return User{}
+		return webUser{}
 	}
 
 	a.lock.Lock()
@@ -663,20 +675,20 @@ func (a *Auth) getCurrentUser(r *http.Request) User {
 
 	s, ok := a.sessions[cookie.Value]
 	if !ok {
-		return User{}
+		return webUser{}
 	}
 
-	for _, u := range a.users {
+	for _, u = range a.users {
 		if u.Name == s.userName {
 			return u
 		}
 	}
 
-	return User{}
+	return webUser{}
 }
 
 // GetUsers - get users
-func (a *Auth) GetUsers() []User {
+func (a *Auth) GetUsers() []webUser {
 	a.lock.Lock()
 	users := a.users
 	a.lock.Unlock()