From 6f4a6c9b190b2672cecd3e3e31413b03d19f8771 Mon Sep 17 00:00:00 2001
From: Eugene Burkov <e.burkov@adguard.com>
Date: Mon, 1 Mar 2021 18:48:15 +0300
Subject: [PATCH] home: rm user's data from session token

---
 CHANGELOG.md               |  6 ++++++
 internal/home/auth.go      | 43 +++++++++++++++++++++-----------------
 internal/home/auth_test.go | 33 ++++++++++++++++++++++++-----
 3 files changed, 58 insertions(+), 24 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7d078d4e786..46331390709 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,12 @@ and this project adheres to
 ## [v0.105.2] - 2021-03-04
 -->
 
+### Changed
+
+- Session token doesn't contain user's information anymore ([#2470]).
+
+[#2470]: https://github.com/AdguardTeam/AdGuardHome/issues/2470
+
 ### Fixed
 
 - Incomplete OpenWRT detection ([#2757]).
diff --git a/internal/home/auth.go b/internal/home/auth.go
index 26b577870d4..d0d9c22ddcf 100644
--- a/internal/home/auth.go
+++ b/internal/home/auth.go
@@ -2,13 +2,10 @@ package home
 
 import (
 	"crypto/rand"
-	"crypto/sha256"
 	"encoding/binary"
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
-	"math"
-	"math/big"
 	"net/http"
 	"strings"
 	"sync"
@@ -20,8 +17,12 @@ import (
 )
 
 const (
-	cookieTTL         = 365 * 24 // in hours
+	// cookieTTL is given in hours.
+	cookieTTL         = 365 * 24
 	sessionCookieName = "agh_session"
+
+	// sessionTokenSize is the length of session token in bytes.
+	sessionTokenSize = 16
 )
 
 type session struct {
@@ -285,16 +286,19 @@ type loginJSON struct {
 	Password string `json:"password"`
 }
 
-func getSession(u *User) ([]byte, error) {
-	maxSalt := big.NewInt(math.MaxUint32)
-	salt, err := rand.Int(rand.Reader, maxSalt)
+// newSessionToken returns cryptographically secure randomly generated slice of
+// bytes of sessionTokenSize length.
+//
+// TODO(e.burkov): Think about using byte array instead of byte slice.
+func newSessionToken() (data []byte, err error) {
+	randData := make([]byte, sessionTokenSize)
+
+	_, err = rand.Read(randData)
 	if err != nil {
 		return nil, err
 	}
 
-	d := []byte(fmt.Sprintf("%s%s%s", salt, u.Name, u.PasswordHash))
-	hash := sha256.Sum256(d)
-	return hash[:], nil
+	return randData, nil
 }
 
 func (a *Auth) httpCookie(req loginJSON) (string, error) {
@@ -303,21 +307,22 @@ func (a *Auth) httpCookie(req loginJSON) (string, error) {
 		return "", nil
 	}
 
-	sess, err := getSession(&u)
+	sess, err := newSessionToken()
 	if err != nil {
 		return "", err
 	}
 
 	now := time.Now().UTC()
+
+	a.addSession(sess, &session{
+		userName: u.Name,
+		expire:   uint32(now.Unix()) + a.sessionTTL,
+	})
+
 	expire := now.Add(cookieTTL * time.Hour)
-	expstr := expire.Format(time.RFC1123)
-	expstr = expstr[:len(expstr)-len("UTC")] // "UTC" -> "GMT"
-	expstr += "GMT"
-
-	s := session{}
-	s.userName = u.Name
-	s.expire = uint32(now.Unix()) + a.sessionTTL
-	a.addSession(sess, &s)
+	expfmt := expire.Format(time.RFC1123)
+	// "UTC" -> "GMT"
+	expstr := expfmt[:len(expfmt)-len("UTC")] + "GMT"
 
 	return fmt.Sprintf("%s=%s; Path=/; HttpOnly; Expires=%s",
 		sessionCookieName, hex.EncodeToString(sess), expstr), nil
diff --git a/internal/home/auth_test.go b/internal/home/auth_test.go
index 4dbd07b6938..7dbbf3c62f7 100644
--- a/internal/home/auth_test.go
+++ b/internal/home/auth_test.go
@@ -1,6 +1,8 @@
 package home
 
 import (
+	"bytes"
+	"crypto/rand"
 	"encoding/hex"
 	"net/http"
 	"net/url"
@@ -11,6 +13,7 @@ import (
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghtest"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestMain(m *testing.M) {
@@ -24,14 +27,34 @@ func prepareTestDir() string {
 	return dir
 }
 
+func TestNewSessionToken(t *testing.T) {
+	// Successful case.
+	token, err := newSessionToken()
+	require.Nil(t, err)
+	assert.Len(t, token, sessionTokenSize)
+
+	// Break the rand.Reader.
+	prevReader := rand.Reader
+	t.Cleanup(func() {
+		rand.Reader = prevReader
+	})
+	rand.Reader = &bytes.Buffer{}
+
+	// Unsuccessful case.
+	token, err = newSessionToken()
+	require.NotNil(t, err)
+	assert.Empty(t, token)
+}
+
 func TestAuth(t *testing.T) {
 	dir := prepareTestDir()
-	defer func() { _ = os.RemoveAll(dir) }()
+	t.Cleanup(func() { _ = os.RemoveAll(dir) })
 	fn := filepath.Join(dir, "sessions.db")
 
-	users := []User{
-		{Name: "name", PasswordHash: "$2y$05$..vyzAECIhJPfaQiOK17IukcQnqEgKJHy0iETyYqxn3YXJl8yZuo2"},
-	}
+	users := []User{{
+		Name:         "name",
+		PasswordHash: "$2y$05$..vyzAECIhJPfaQiOK17IukcQnqEgKJHy0iETyYqxn3YXJl8yZuo2",
+	}}
 	a := InitAuth(fn, nil, 60)
 	s := session{}
 
@@ -41,7 +64,7 @@ func TestAuth(t *testing.T) {
 	assert.Equal(t, checkSessionNotFound, a.checkSession("notfound"))
 	a.RemoveSession("notfound")
 
-	sess, err := getSession(&users[0])
+	sess, err := newSessionToken()
 	assert.Nil(t, err)
 	sessStr := hex.EncodeToString(sess)