Skip to content

Commit c6a53c3

Browse files
Gustedearl-warren
authored andcommitted
[SECURITY] Rework long-term authentication
- This is a 'front-port' of the already existing patch on v1.21 and v1.20, but applied on top of what Gitea has done to rework the LTA mechanism. Forgejo will stick with the reworked mechanism by the Forgejo Security team for the time being. The removal of legacy code (AES-GCM) has been left out. - The current architecture is inherently insecure, because you can construct the 'secret' cookie value with values that are available in the database. Thus provides zero protection when a database is dumped/leaked. - This patch implements a new architecture that's inspired from: [Paragonie Initiative](https://paragonie.com/blog/2015/04/secure-authentication-php-with-long-term-persistence#secure-remember-me-cookies). - Integration testing is added to ensure the new mechanism works. - Removes a setting, because it's not used anymore. (cherry picked from commit e3d6622a63da9c33eed1e3d102cf28a92ff653d6) (cherry picked from commit fef1a6dac5e25579e42d40209c4cfc06879948b9) (cherry picked from commit b0c5165145fa52f2f7bbec1f50b308bdf1d20ef3) (cherry picked from commit 7ad51b9f8d0647eecacd258f6ee26155da3872e1) (cherry picked from commit 64f053f3834e764112cde26bb0d16c5e88d6b2af) (cherry picked from commit f5e78e4) Conflicts: services/auth/auth_token_test.go https://codeberg.org/forgejo/forgejo/pulls/2069 (cherry picked from commit f69fc23d4bbadf388c7857040ee0774b824e418e) (cherry picked from commit d955ab3ab02cbb7f1245a8cddec426d64d3ac500) (cherry picked from commit 9220088f902a25c4690bcabf5a40a8d02e784182) (cherry picked from commit c73ac636962c41c71814c273510146f0533264ab) (cherry picked from commit 747a176048ea93085b406429db0e25bb21912eda) Conflicts: models/user/user.go routers/web/user/setting/account.go https://codeberg.org/forgejo/forgejo/pulls/2295
1 parent 3916357 commit c6a53c3

File tree

15 files changed

+323
-304
lines changed

15 files changed

+323
-304
lines changed

.deadcode-out

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ package "code.gitea.io/gitea/models/asymkey"
3131
func HasDeployKey
3232

3333
package "code.gitea.io/gitea/models/auth"
34-
func DeleteAuthTokenByID
3534
func GetSourceByName
3635
func GetWebAuthnCredentialByID
3736
func WebAuthnCredentials

models/auth/auth_token.go

Lines changed: 63 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,60 +1,96 @@
1-
// Copyright 2023 The Gitea Authors. All rights reserved.
1+
// Copyright 2023 The Forgejo Authors. All rights reserved.
22
// SPDX-License-Identifier: MIT
33

44
package auth
55

66
import (
77
"context"
8+
"crypto/sha256"
9+
"encoding/hex"
10+
"fmt"
11+
"time"
812

913
"code.gitea.io/gitea/models/db"
1014
"code.gitea.io/gitea/modules/timeutil"
1115
"code.gitea.io/gitea/modules/util"
12-
13-
"xorm.io/builder"
1416
)
1517

16-
var ErrAuthTokenNotExist = util.NewNotExistErrorf("auth token does not exist")
18+
// AuthorizationToken represents a authorization token to a user.
19+
type AuthorizationToken struct {
20+
ID int64 `xorm:"pk autoincr"`
21+
UID int64 `xorm:"INDEX"`
22+
LookupKey string `xorm:"INDEX UNIQUE"`
23+
HashedValidator string
24+
Expiry timeutil.TimeStamp
25+
}
1726

18-
type AuthToken struct { //nolint:revive
19-
ID string `xorm:"pk"`
20-
TokenHash string
21-
UserID int64 `xorm:"INDEX"`
22-
ExpiresUnix timeutil.TimeStamp `xorm:"INDEX"`
27+
// TableName provides the real table name.
28+
func (AuthorizationToken) TableName() string {
29+
return "forgejo_auth_token"
2330
}
2431

2532
func init() {
26-
db.RegisterModel(new(AuthToken))
33+
db.RegisterModel(new(AuthorizationToken))
2734
}
2835

29-
func InsertAuthToken(ctx context.Context, t *AuthToken) error {
30-
_, err := db.GetEngine(ctx).Insert(t)
31-
return err
36+
// IsExpired returns if the authorization token is expired.
37+
func (authToken *AuthorizationToken) IsExpired() bool {
38+
return authToken.Expiry.AsLocalTime().Before(time.Now())
3239
}
3340

34-
func GetAuthTokenByID(ctx context.Context, id string) (*AuthToken, error) {
35-
at := &AuthToken{}
41+
// GenerateAuthToken generates a new authentication token for the given user.
42+
// It returns the lookup key and validator values that should be passed to the
43+
// user via a long-term cookie.
44+
func GenerateAuthToken(ctx context.Context, userID int64, expiry timeutil.TimeStamp) (lookupKey, validator string, err error) {
45+
// Request 64 random bytes. The first 32 bytes will be used for the lookupKey
46+
// and the other 32 bytes will be used for the validator.
47+
rBytes, err := util.CryptoRandomBytes(64)
48+
if err != nil {
49+
return "", "", err
50+
}
51+
hexEncoded := hex.EncodeToString(rBytes)
52+
validator, lookupKey = hexEncoded[64:], hexEncoded[:64]
3653

37-
has, err := db.GetEngine(ctx).ID(id).Get(at)
54+
_, err = db.GetEngine(ctx).Insert(&AuthorizationToken{
55+
UID: userID,
56+
Expiry: expiry,
57+
LookupKey: lookupKey,
58+
HashedValidator: HashValidator(rBytes[32:]),
59+
})
60+
return lookupKey, validator, err
61+
}
62+
63+
// FindAuthToken will find a authorization token via the lookup key.
64+
func FindAuthToken(ctx context.Context, lookupKey string) (*AuthorizationToken, error) {
65+
var authToken AuthorizationToken
66+
has, err := db.GetEngine(ctx).Where("lookup_key = ?", lookupKey).Get(&authToken)
3867
if err != nil {
3968
return nil, err
69+
} else if !has {
70+
return nil, fmt.Errorf("lookup key %q: %w", lookupKey, util.ErrNotExist)
4071
}
41-
if !has {
42-
return nil, ErrAuthTokenNotExist
43-
}
44-
return at, nil
72+
return &authToken, nil
4573
}
4674

47-
func UpdateAuthTokenByID(ctx context.Context, t *AuthToken) error {
48-
_, err := db.GetEngine(ctx).ID(t.ID).Cols("token_hash", "expires_unix").Update(t)
75+
// DeleteAuthToken will delete the authorization token.
76+
func DeleteAuthToken(ctx context.Context, authToken *AuthorizationToken) error {
77+
_, err := db.DeleteByBean(ctx, authToken)
4978
return err
5079
}
5180

52-
func DeleteAuthTokenByID(ctx context.Context, id string) error {
53-
_, err := db.GetEngine(ctx).ID(id).Delete(&AuthToken{})
81+
// DeleteAuthTokenByUser will delete all authorization tokens for the user.
82+
func DeleteAuthTokenByUser(ctx context.Context, userID int64) error {
83+
if userID == 0 {
84+
return nil
85+
}
86+
87+
_, err := db.DeleteByBean(ctx, &AuthorizationToken{UID: userID})
5488
return err
5589
}
5690

57-
func DeleteExpiredAuthTokens(ctx context.Context) error {
58-
_, err := db.GetEngine(ctx).Where(builder.Lt{"expires_unix": timeutil.TimeStampNow()}).Delete(&AuthToken{})
59-
return err
91+
// HashValidator will return a hexified hashed version of the validator.
92+
func HashValidator(validator []byte) string {
93+
h := sha256.New()
94+
h.Write(validator)
95+
return hex.EncodeToString(h.Sum(nil))
6096
}

models/forgejo_migrations/migrate.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ var migrations = []*Migration{
4141
NewMigration("Add Forgejo Blocked Users table", forgejo_v1_20.AddForgejoBlockedUser),
4242
// v1 -> v2
4343
NewMigration("create the forgejo_sem_ver table", forgejo_v1_20.CreateSemVerTable),
44+
// v2 -> v3
45+
NewMigration("create the forgejo_auth_token table", forgejo_v1_20.CreateAuthorizationTokenTable),
4446
}
4547

4648
// GetCurrentDBVersion returns the current Forgejo database version.

models/forgejo_migrations/v1_20/v3.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// Copyright 2023 The Forgejo Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package forgejo_v1_20 //nolint:revive
5+
6+
import (
7+
"code.gitea.io/gitea/modules/timeutil"
8+
9+
"xorm.io/xorm"
10+
)
11+
12+
type AuthorizationToken struct {
13+
ID int64 `xorm:"pk autoincr"`
14+
UID int64 `xorm:"INDEX"`
15+
LookupKey string `xorm:"INDEX UNIQUE"`
16+
HashedValidator string
17+
Expiry timeutil.TimeStamp
18+
}
19+
20+
func (AuthorizationToken) TableName() string {
21+
return "forgejo_auth_token"
22+
}
23+
24+
func CreateAuthorizationTokenTable(x *xorm.Engine) error {
25+
return x.Sync(new(AuthorizationToken))
26+
}

models/user/user.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,11 @@ func (u *User) NewGitSig() *git.Signature {
366366
// SetPassword hashes a password using the algorithm defined in the config value of PASSWORD_HASH_ALGO
367367
// change passwd, salt and passwd_hash_algo fields
368368
func (u *User) SetPassword(passwd string) (err error) {
369+
// Invalidate all authentication tokens for this user.
370+
if err := auth.DeleteAuthTokenByUser(db.DefaultContext, u.ID); err != nil {
371+
return err
372+
}
373+
369374
if u.Salt, err = GetUserSalt(); err != nil {
370375
return err
371376
}

modules/context/context_cookie.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ import (
77
"net/http"
88
"strings"
99

10+
auth_model "code.gitea.io/gitea/models/auth"
11+
user_model "code.gitea.io/gitea/models/user"
1012
"code.gitea.io/gitea/modules/setting"
13+
"code.gitea.io/gitea/modules/timeutil"
1114
"code.gitea.io/gitea/modules/web/middleware"
1215
)
1316

@@ -40,3 +43,14 @@ func (ctx *Context) DeleteSiteCookie(name string) {
4043
func (ctx *Context) GetSiteCookie(name string) string {
4144
return middleware.GetSiteCookie(ctx.Req, name)
4245
}
46+
47+
// SetLTACookie will generate a LTA token and add it as an cookie.
48+
func (ctx *Context) SetLTACookie(u *user_model.User) error {
49+
days := 86400 * setting.LogInRememberDays
50+
lookup, validator, err := auth_model.GenerateAuthToken(ctx, u.ID, timeutil.TimeStampNow().Add(int64(days)))
51+
if err != nil {
52+
return err
53+
}
54+
ctx.SetSiteCookie(setting.CookieRememberName, lookup+":"+validator, days)
55+
return nil
56+
}

routers/install/install.go

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,12 @@ import (
2727
"code.gitea.io/gitea/modules/log"
2828
"code.gitea.io/gitea/modules/setting"
2929
"code.gitea.io/gitea/modules/templates"
30-
"code.gitea.io/gitea/modules/timeutil"
3130
"code.gitea.io/gitea/modules/translation"
3231
"code.gitea.io/gitea/modules/user"
3332
"code.gitea.io/gitea/modules/util"
3433
"code.gitea.io/gitea/modules/web"
3534
"code.gitea.io/gitea/modules/web/middleware"
3635
"code.gitea.io/gitea/routers/common"
37-
auth_service "code.gitea.io/gitea/services/auth"
3836
"code.gitea.io/gitea/services/forms"
3937

4038
"gitea.com/go-chi/session"
@@ -549,23 +547,16 @@ func SubmitInstall(ctx *context.Context) {
549547
u, _ = user_model.GetUserByName(ctx, u.Name)
550548
}
551549

552-
nt, token, err := auth_service.CreateAuthTokenForUserID(ctx, u.ID)
553-
if err != nil {
554-
ctx.ServerError("CreateAuthTokenForUserID", err)
550+
if err := ctx.SetLTACookie(u); err != nil {
551+
ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form)
555552
return
556553
}
557554

558-
ctx.SetSiteCookie(setting.CookieRememberName, nt.ID+":"+token, setting.LogInRememberDays*timeutil.Day)
559-
560555
// Auto-login for admin
561556
if err = ctx.Session.Set("uid", u.ID); err != nil {
562557
ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form)
563558
return
564559
}
565-
if err = ctx.Session.Set("uname", u.Name); err != nil {
566-
ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form)
567-
return
568-
}
569560

570561
if err = ctx.Session.Release(); err != nil {
571562
ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form)

routers/web/auth/auth.go

Lines changed: 30 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
package auth
66

77
import (
8+
"crypto/subtle"
9+
"encoding/hex"
810
"errors"
911
"fmt"
1012
"net/http"
@@ -54,23 +56,39 @@ func autoSignIn(ctx *context.Context) (bool, error) {
5456
}
5557
}()
5658

57-
if err := auth.DeleteExpiredAuthTokens(ctx); err != nil {
58-
log.Error("Failed to delete expired auth tokens: %v", err)
59+
authCookie := ctx.GetSiteCookie(setting.CookieRememberName)
60+
if len(authCookie) == 0 {
61+
return false, nil
5962
}
6063

61-
t, err := auth_service.CheckAuthToken(ctx, ctx.GetSiteCookie(setting.CookieRememberName))
64+
lookupKey, validator, found := strings.Cut(authCookie, ":")
65+
if !found {
66+
return false, nil
67+
}
68+
69+
authToken, err := auth.FindAuthToken(ctx, lookupKey)
6270
if err != nil {
63-
switch err {
64-
case auth_service.ErrAuthTokenInvalidFormat, auth_service.ErrAuthTokenExpired:
71+
if errors.Is(err, util.ErrNotExist) {
6572
return false, nil
6673
}
6774
return false, err
6875
}
69-
if t == nil {
76+
77+
if authToken.IsExpired() {
78+
err = auth.DeleteAuthToken(ctx, authToken)
79+
return false, err
80+
}
81+
82+
rawValidator, err := hex.DecodeString(validator)
83+
if err != nil {
84+
return false, err
85+
}
86+
87+
if subtle.ConstantTimeCompare([]byte(authToken.HashedValidator), []byte(auth.HashValidator(rawValidator))) == 0 {
7088
return false, nil
7189
}
7290

73-
u, err := user_model.GetUserByID(ctx, t.UserID)
91+
u, err := user_model.GetUserByID(ctx, authToken.UID)
7492
if err != nil {
7593
if !user_model.IsErrUserNotExist(err) {
7694
return false, fmt.Errorf("GetUserByID: %w", err)
@@ -80,17 +98,9 @@ func autoSignIn(ctx *context.Context) (bool, error) {
8098

8199
isSucceed = true
82100

83-
nt, token, err := auth_service.RegenerateAuthToken(ctx, t)
84-
if err != nil {
85-
return false, err
86-
}
87-
88-
ctx.SetSiteCookie(setting.CookieRememberName, nt.ID+":"+token, setting.LogInRememberDays*timeutil.Day)
89-
90101
if err := updateSession(ctx, nil, map[string]any{
91102
// Set session IDs
92-
"uid": u.ID,
93-
"uname": u.Name,
103+
"uid": u.ID,
94104
}); err != nil {
95105
return false, fmt.Errorf("unable to updateSession: %w", err)
96106
}
@@ -128,10 +138,6 @@ func CheckAutoLogin(ctx *context.Context) bool {
128138
// Check auto-login
129139
isSucceed, err := autoSignIn(ctx)
130140
if err != nil {
131-
if errors.Is(err, auth_service.ErrAuthTokenInvalidHash) {
132-
ctx.Flash.Error(ctx.Tr("auth.remember_me.compromised"), true)
133-
return false
134-
}
135141
ctx.ServerError("autoSignIn", err)
136142
return true
137143
}
@@ -306,13 +312,10 @@ func handleSignIn(ctx *context.Context, u *user_model.User, remember bool) {
306312

307313
func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRedirect bool) string {
308314
if remember {
309-
nt, token, err := auth_service.CreateAuthTokenForUserID(ctx, u.ID)
310-
if err != nil {
311-
ctx.ServerError("CreateAuthTokenForUserID", err)
315+
if err := ctx.SetLTACookie(u); err != nil {
316+
ctx.ServerError("GenerateAuthToken", err)
312317
return setting.AppSubURL + "/"
313318
}
314-
315-
ctx.SetSiteCookie(setting.CookieRememberName, nt.ID+":"+token, setting.LogInRememberDays*timeutil.Day)
316319
}
317320

318321
if err := updateSession(ctx, []string{
@@ -325,8 +328,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe
325328
"twofaRemember",
326329
"linkAccount",
327330
}, map[string]any{
328-
"uid": u.ID,
329-
"uname": u.Name,
331+
"uid": u.ID,
330332
}); err != nil {
331333
ctx.ServerError("RegenerateSession", err)
332334
return setting.AppSubURL + "/"
@@ -745,8 +747,7 @@ func handleAccountActivation(ctx *context.Context, user *user_model.User) {
745747
log.Trace("User activated: %s", user.Name)
746748

747749
if err := updateSession(ctx, nil, map[string]any{
748-
"uid": user.ID,
749-
"uname": user.Name,
750+
"uid": user.ID,
750751
}); err != nil {
751752
log.Error("Unable to regenerate session for user: %-v with email: %s: %v", user, user.Email, err)
752753
ctx.ServerError("ActivateUserEmail", err)

routers/web/auth/oauth.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,8 +1124,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model
11241124
// we can't sign the user in just yet. Instead, redirect them to the 2FA authentication page.
11251125
if !needs2FA {
11261126
if err := updateSession(ctx, nil, map[string]any{
1127-
"uid": u.ID,
1128-
"uname": u.Name,
1127+
"uid": u.ID,
11291128
}); err != nil {
11301129
ctx.ServerError("updateSession", err)
11311130
return

routers/web/user/setting/account.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,15 @@ func AccountPost(ctx *context.Context) {
7979
return
8080
}
8181
} else {
82+
// Re-generate LTA cookie.
83+
if len(ctx.GetSiteCookie(setting.CookieRememberName)) != 0 {
84+
if err := ctx.SetLTACookie(ctx.Doer); err != nil {
85+
ctx.ServerError("SetLTACookie", err)
86+
return
87+
}
88+
}
89+
90+
log.Trace("User password updated: %s", ctx.Doer.Name)
8291
ctx.Flash.Success(ctx.Tr("settings.change_password_success"))
8392
}
8493
}

0 commit comments

Comments
 (0)