mirror of
https://github.com/go-gitea/gitea.git
synced 2026-10-23 19:47:07 +09:00
fix(auth): avoid "compromised account" false-alarm when login with remember-me token (#39686)
1. Remember the last token that has been rotated and skip its validation 2. Don't alarm when the token mismatch occurs in a short window
This commit is contained in:
1 parent
8c7b8d085c
commit
2f1761e3fe
7 files changed
+118
-36
No files matched your search
@@ -434,6 +434,7 @@ func prepareMigrationTasks() []*migration {
|
|||||||
|
|
||||||
newMigration(356, "Add index on action_run commit_sha", v29.AddActionRunCommitSHAIndex),
|
newMigration(356, "Add index on action_run commit_sha", v29.AddActionRunCommitSHAIndex),
|
||||||
newMigration(357, "Normalize legacy team authorize values", v29.NormalizeLegacyTeamAuthorize),
|
newMigration(357, "Normalize legacy team authorize values", v29.NormalizeLegacyTeamAuthorize),
|
||||||
|
newMigration(358, "Track previous remember-me token hashes", v29.AddPreviousAuthTokenHash),
|
||||||
}
|
}
|
||||||
return preparedMigrations
|
return preparedMigrations
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,25 @@
|
|||||||
|
// Copyright 2026 The Gitea Authors. All rights reserved.
|
||||||
|
// SPDX-License-Identifier: MIT
|
||||||
|
|
||||||
|
package v29
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
|
||||||
|
"gitea.dev/modelmigration/base"
|
||||||
|
"gitea.dev/modules/timeutil"
|
||||||
|
|
||||||
|
"xorm.io/xorm"
|
||||||
|
)
|
||||||
|
|
||||||
|
func AddPreviousAuthTokenHash(_ context.Context, x base.EngineMigration) error {
|
||||||
|
type AuthToken struct {
|
||||||
|
PreviousTokenHash string
|
||||||
|
RotatedUnix timeutil.TimeStamp
|
||||||
|
}
|
||||||
|
_, err := x.SyncWithOptions(xorm.SyncOptions{
|
||||||
|
IgnoreDropIndices: true,
|
||||||
|
IgnoreConstrains: true,
|
||||||
|
}, new(AuthToken))
|
||||||
|
return err
|
||||||
|
}
|
||||||
@@ -20,6 +20,9 @@ type AuthToken struct { //nolint:revive // export stutter
|
|||||||
TokenHash string
|
TokenHash string
|
||||||
UserID int64 `xorm:"INDEX"`
|
UserID int64 `xorm:"INDEX"`
|
||||||
ExpiresUnix timeutil.TimeStamp `xorm:"INDEX"`
|
ExpiresUnix timeutil.TimeStamp `xorm:"INDEX"`
|
||||||
|
|
||||||
|
PreviousTokenHash string // Identifies stale cookies without authenticating them.
|
||||||
|
RotatedUnix timeutil.TimeStamp // Bounds the grace window for stale cookies.
|
||||||
}
|
}
|
||||||
|
|
||||||
func init() {
|
func init() {
|
||||||
@@ -44,9 +47,10 @@ func GetAuthTokenByID(ctx context.Context, id string) (*AuthToken, error) {
|
|||||||
return at, nil
|
return at, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func UpdateAuthTokenByID(ctx context.Context, t *AuthToken) error {
|
func UpdateAuthTokenByID(ctx context.Context, t *AuthToken, oldHash string) (bool, error) {
|
||||||
_, err := db.GetEngine(ctx).ID(t.ID).Cols("token_hash", "expires_unix").Update(t)
|
affected, err := db.GetEngine(ctx).ID(t.ID).Where("token_hash = ?", oldHash).
|
||||||
return err
|
Cols("token_hash", "expires_unix", "previous_token_hash", "rotated_unix").Update(t)
|
||||||
|
return affected == 1, err
|
||||||
}
|
}
|
||||||
|
|
||||||
func DeleteAuthTokenByID(ctx context.Context, id string) error {
|
func DeleteAuthTokenByID(ctx context.Context, id string) error {
|
||||||
|
|||||||
+30
-22
@@ -78,49 +78,54 @@ func prepareCommonAuthPageData(ctx *context.Context, opt CommonAuthOptions) {
|
|||||||
|
|
||||||
// autoSignIn reads cookie and try to auto-login.
|
// autoSignIn reads cookie and try to auto-login.
|
||||||
func autoSignIn(ctx *context.Context) (bool, error) {
|
func autoSignIn(ctx *context.Context) (bool, error) {
|
||||||
isSucceed := false
|
cookieTokenValue := ctx.GetSiteCookie(setting.CookieRememberName)
|
||||||
defer func() {
|
if cookieTokenValue == "" {
|
||||||
if !isSucceed {
|
return false, nil
|
||||||
ctx.DeleteSiteCookie(setting.CookieRememberName)
|
}
|
||||||
}
|
|
||||||
}()
|
|
||||||
|
|
||||||
if err := auth.DeleteExpiredAuthTokens(ctx); err != nil {
|
if err := auth.DeleteExpiredAuthTokens(ctx); err != nil {
|
||||||
log.Error("Failed to delete expired auth tokens: %v", err)
|
log.Error("Failed to delete expired auth tokens: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
t, err := auth_service.CheckAuthToken(ctx, ctx.GetSiteCookie(setting.CookieRememberName))
|
deleteCookie := true
|
||||||
if err != nil {
|
defer func() {
|
||||||
switch err {
|
if deleteCookie {
|
||||||
case auth_service.ErrAuthTokenInvalidFormat, auth_service.ErrAuthTokenExpired:
|
ctx.DeleteSiteCookie(setting.CookieRememberName)
|
||||||
return false, nil
|
|
||||||
}
|
}
|
||||||
return false, err
|
}()
|
||||||
}
|
|
||||||
if t == nil {
|
t, err := auth_service.CheckAuthToken(ctx, cookieTokenValue)
|
||||||
|
if errors.Is(err, auth_service.ErrAuthTokenStale) {
|
||||||
|
deleteCookie = false
|
||||||
return false, nil
|
return false, nil
|
||||||
|
} else if errors.Is(err, auth_service.ErrAuthTokenInvalidFormat) || errors.Is(err, auth_service.ErrAuthTokenExpired) {
|
||||||
|
return false, nil
|
||||||
|
} else if err != nil {
|
||||||
|
return false, err
|
||||||
}
|
}
|
||||||
|
|
||||||
u, err := user_model.GetUserByID(ctx, t.UserID)
|
u, err := user_model.GetUserByID(ctx, t.UserID)
|
||||||
if err != nil {
|
if user_model.IsErrUserNotExist(err) {
|
||||||
if !user_model.IsErrUserNotExist(err) {
|
|
||||||
return false, fmt.Errorf("GetUserByID: %w", err)
|
|
||||||
}
|
|
||||||
return false, nil
|
return false, nil
|
||||||
|
} else if err != nil {
|
||||||
|
return false, fmt.Errorf("GetUserByID: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
userHasTwoFactorAuth, err := auth.HasTwoFactorOrWebAuthn(ctx, u.ID)
|
userHasTwoFactorAuth, err := auth.HasTwoFactorOrWebAuthn(ctx, u.ID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return false, fmt.Errorf("HasTwoFactorOrWebAuthn: %w", err)
|
return false, fmt.Errorf("HasTwoFactorOrWebAuthn: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
isSucceed = true
|
|
||||||
|
|
||||||
nt, token, err := auth_service.RegenerateAuthToken(ctx, t)
|
nt, token, err := auth_service.RegenerateAuthToken(ctx, t)
|
||||||
if err != nil {
|
if errors.Is(err, auth_service.ErrAuthTokenStale) {
|
||||||
|
deleteCookie = false
|
||||||
|
return false, nil
|
||||||
|
} else if err != nil {
|
||||||
return false, err
|
return false, err
|
||||||
}
|
}
|
||||||
|
|
||||||
ctx.SetSiteCookie(setting.CookieRememberName, nt.ID+":"+token, setting.LogInRememberDays*timeutil.Day)
|
ctx.SetSiteCookie(setting.CookieRememberName, nt.ID+":"+token, setting.LogInRememberDays*timeutil.Day)
|
||||||
|
deleteCookie = false
|
||||||
|
|
||||||
if err := regenerateSession(ctx, map[string]any{
|
if err := regenerateSession(ctx, map[string]any{
|
||||||
session.KeyUID: u.ID,
|
session.KeyUID: u.ID,
|
||||||
@@ -130,7 +135,7 @@ func autoSignIn(ctx *context.Context) (bool, error) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if err := resetLocale(ctx, u); err != nil {
|
if err := resetLocale(ctx, u); err != nil {
|
||||||
return false, err
|
log.Error("Failed to reset locale for user %d: %v", u.ID, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
return true, nil
|
return true, nil
|
||||||
@@ -374,6 +379,9 @@ func handleSignIn(ctx *context.Context, u *user_model.User, remember bool) {
|
|||||||
|
|
||||||
func handleSignInFull(ctx *context.Context, u *user_model.User, remember bool) {
|
func handleSignInFull(ctx *context.Context, u *user_model.User, remember bool) {
|
||||||
if remember {
|
if remember {
|
||||||
|
if err := auth.DeleteExpiredAuthTokens(ctx); err != nil {
|
||||||
|
log.Error("Failed to delete expired auth tokens: %v", err)
|
||||||
|
}
|
||||||
nt, token, err := auth_service.CreateAuthTokenForUserID(ctx, u.ID)
|
nt, token, err := auth_service.CreateAuthTokenForUserID(ctx, u.ID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
ctx.ServerError("CreateAuthTokenForUserID", err)
|
ctx.ServerError("CreateAuthTokenForUserID", err)
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ import (
|
|||||||
"gitea.dev/modules/test"
|
"gitea.dev/modules/test"
|
||||||
"gitea.dev/modules/util"
|
"gitea.dev/modules/util"
|
||||||
"gitea.dev/modules/web"
|
"gitea.dev/modules/web"
|
||||||
|
auth_service "gitea.dev/services/auth"
|
||||||
"gitea.dev/services/auth/source/oauth2"
|
"gitea.dev/services/auth/source/oauth2"
|
||||||
"gitea.dev/services/contexttest"
|
"gitea.dev/services/contexttest"
|
||||||
"gitea.dev/services/forms"
|
"gitea.dev/services/forms"
|
||||||
@@ -216,3 +217,22 @@ func TestRegisterOpenIDPostRejectsWrongCaptcha(t *testing.T) {
|
|||||||
assert.Equal(t, true, ctx.Data["Err_Captcha"])
|
assert.Equal(t, true, ctx.Data["Err_Captcha"])
|
||||||
unittest.AssertNotExistsBean(t, &user_model.User{LowerName: "openid-captcha-user"})
|
unittest.AssertNotExistsBean(t, &user_model.User{LowerName: "openid-captcha-user"})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestAutoSignInStaleCookie(t *testing.T) {
|
||||||
|
require.NoError(t, unittest.PrepareTestDatabase())
|
||||||
|
at, token, err := auth_service.CreateAuthTokenForUserID(t.Context(), 2)
|
||||||
|
require.NoError(t, err)
|
||||||
|
current, currentToken, err := auth_service.RegenerateAuthToken(t.Context(), at)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
ctx, resp := contexttest.MockContext(t, "/user/login")
|
||||||
|
ctx.Req.AddCookie(&http.Cookie{Name: setting.CookieRememberName, Value: at.ID + ":" + token})
|
||||||
|
succeeded, err := autoSignIn(ctx)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.False(t, succeeded)
|
||||||
|
for _, cookie := range resp.Result().Cookies() {
|
||||||
|
assert.NotEqual(t, setting.CookieRememberName, cookie.Name)
|
||||||
|
}
|
||||||
|
_, err = auth_service.CheckAuthToken(t.Context(), current.ID+":"+currentToken)
|
||||||
|
require.NoError(t, err)
|
||||||
|
}
|
||||||
+18
-10
@@ -25,15 +25,12 @@ import (
|
|||||||
|
|
||||||
var (
|
var (
|
||||||
ErrAuthTokenInvalidFormat = util.NewInvalidArgumentErrorf("auth token has an invalid format")
|
ErrAuthTokenInvalidFormat = util.NewInvalidArgumentErrorf("auth token has an invalid format")
|
||||||
|
ErrAuthTokenStale = util.NewInvalidArgumentErrorf("auth token has been rotated")
|
||||||
ErrAuthTokenExpired = util.NewInvalidArgumentErrorf("auth token has expired")
|
ErrAuthTokenExpired = util.NewInvalidArgumentErrorf("auth token has expired")
|
||||||
ErrAuthTokenInvalidHash = util.NewInvalidArgumentErrorf("auth token is invalid")
|
ErrAuthTokenInvalidHash = util.NewInvalidArgumentErrorf("auth token is invalid")
|
||||||
)
|
)
|
||||||
|
|
||||||
func CheckAuthToken(ctx context.Context, value string) (*auth_model.AuthToken, error) {
|
func CheckAuthToken(ctx context.Context, value string) (*auth_model.AuthToken, error) {
|
||||||
if len(value) == 0 {
|
|
||||||
return nil, nil //nolint:nilnil // the auth method is not applicable
|
|
||||||
}
|
|
||||||
|
|
||||||
parts := strings.SplitN(value, ":", 2)
|
parts := strings.SplitN(value, ":", 2)
|
||||||
if len(parts) != 2 {
|
if len(parts) != 2 {
|
||||||
return nil, ErrAuthTokenInvalidFormat
|
return nil, ErrAuthTokenInvalidFormat
|
||||||
@@ -53,7 +50,12 @@ func CheckAuthToken(ctx context.Context, value string) (*auth_model.AuthToken, e
|
|||||||
|
|
||||||
hashedToken := sha256.Sum256([]byte(parts[1]))
|
hashedToken := sha256.Sum256([]byte(parts[1]))
|
||||||
|
|
||||||
if !util.CryptoConstTimeEqual(t.TokenHash, hex.EncodeToString(hashedToken[:])) {
|
hash := hex.EncodeToString(hashedToken[:])
|
||||||
|
if !util.CryptoConstTimeEqual(t.TokenHash, hash) {
|
||||||
|
// Recently rotated cookies can still be in flight, but must not authenticate.
|
||||||
|
if t.RotatedUnix.AddDuration(time.Minute) >= timeutil.TimeStampNow() && util.CryptoConstTimeEqual(t.PreviousTokenHash, hash) {
|
||||||
|
return nil, ErrAuthTokenStale
|
||||||
|
}
|
||||||
// If an attacker steals a token and uses the token to create a new session the hash gets updated.
|
// If an attacker steals a token and uses the token to create a new session the hash gets updated.
|
||||||
// When the victim uses the old token the hashes don't match anymore and the victim should be notified about the compromised token.
|
// When the victim uses the old token the hashes don't match anymore and the victim should be notified about the compromised token.
|
||||||
// Revoke the token so the attacker's rotated token (which shares this ID) can no longer be used.
|
// Revoke the token so the attacker's rotated token (which shares this ID) can no longer be used.
|
||||||
@@ -70,15 +72,21 @@ func RegenerateAuthToken(ctx context.Context, t *auth_model.AuthToken) (*auth_mo
|
|||||||
token, hash := generateTokenAndHash()
|
token, hash := generateTokenAndHash()
|
||||||
|
|
||||||
newToken := &auth_model.AuthToken{
|
newToken := &auth_model.AuthToken{
|
||||||
ID: t.ID,
|
ID: t.ID,
|
||||||
TokenHash: hash,
|
TokenHash: hash,
|
||||||
UserID: t.UserID,
|
PreviousTokenHash: t.TokenHash,
|
||||||
ExpiresUnix: timeutil.TimeStampNow().AddDuration(time.Duration(setting.LogInRememberDays*24) * time.Hour),
|
RotatedUnix: timeutil.TimeStampNow(),
|
||||||
|
UserID: t.UserID,
|
||||||
|
ExpiresUnix: timeutil.TimeStampNow().AddDuration(time.Duration(setting.LogInRememberDays*24) * time.Hour),
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := auth_model.UpdateAuthTokenByID(ctx, newToken); err != nil {
|
updated, err := auth_model.UpdateAuthTokenByID(ctx, newToken, t.TokenHash)
|
||||||
|
if err != nil {
|
||||||
return nil, "", err
|
return nil, "", err
|
||||||
}
|
}
|
||||||
|
if !updated {
|
||||||
|
return nil, "", ErrAuthTokenStale
|
||||||
|
}
|
||||||
|
|
||||||
return newToken, token, nil
|
return newToken, token, nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -13,6 +13,7 @@ import (
|
|||||||
"gitea.dev/modules/util"
|
"gitea.dev/modules/util"
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestCheckAuthToken(t *testing.T) {
|
func TestCheckAuthToken(t *testing.T) {
|
||||||
@@ -20,7 +21,7 @@ func TestCheckAuthToken(t *testing.T) {
|
|||||||
|
|
||||||
t.Run("Empty", func(t *testing.T) {
|
t.Run("Empty", func(t *testing.T) {
|
||||||
token, err := CheckAuthToken(t.Context(), "")
|
token, err := CheckAuthToken(t.Context(), "")
|
||||||
assert.NoError(t, err)
|
assert.ErrorIs(t, err, ErrAuthTokenInvalidFormat)
|
||||||
assert.Nil(t, token)
|
assert.Nil(t, token)
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -105,5 +106,20 @@ func TestRegenerateAuthToken(t *testing.T) {
|
|||||||
assert.NotEqual(t, token, token2)
|
assert.NotEqual(t, token, token2)
|
||||||
assert.NotEqual(t, at.ExpiresUnix, at2.ExpiresUnix)
|
assert.NotEqual(t, at.ExpiresUnix, at2.ExpiresUnix)
|
||||||
|
|
||||||
|
// A second request validated the same cookie before the first rotation.
|
||||||
|
_, _, err = RegenerateAuthToken(t.Context(), at)
|
||||||
|
require.ErrorIs(t, err, ErrAuthTokenStale)
|
||||||
|
|
||||||
|
_, err = CheckAuthToken(t.Context(), at.ID+":"+token)
|
||||||
|
require.ErrorIs(t, err, ErrAuthTokenStale)
|
||||||
|
current, err := CheckAuthToken(t.Context(), at2.ID+":"+token2)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, at2.TokenHash, current.TokenHash)
|
||||||
|
|
||||||
|
timeutil.MockSet(time.Date(2023, 1, 1, 0, 1, 2, 0, time.UTC))
|
||||||
|
_, err = CheckAuthToken(t.Context(), at.ID+":"+token)
|
||||||
|
require.ErrorIs(t, err, ErrAuthTokenInvalidHash)
|
||||||
|
_, err = auth_model.GetAuthTokenByID(t.Context(), at.ID)
|
||||||
|
require.ErrorIs(t, err, util.ErrNotExist)
|
||||||
assert.NoError(t, auth_model.DeleteAuthTokenByID(t.Context(), at.ID))
|
assert.NoError(t, auth_model.DeleteAuthTokenByID(t.Context(), at.ID))
|
||||||
}
|
}
|
||||||
Reference in new issue
Block a user