diff --git a/models/auth/twofactor.go b/models/auth/twofactor.go index 0362879e7e0..ec60e344f59 100644 --- a/models/auth/twofactor.go +++ b/models/auth/twofactor.go @@ -12,8 +12,11 @@ import ( "encoding/base64" "encoding/hex" "fmt" + "strconv" "gitea.dev/models/db" + "gitea.dev/modules/cache" + "gitea.dev/modules/globallock" "gitea.dev/modules/secret" "gitea.dev/modules/setting" "gitea.dev/modules/timeutil" @@ -48,6 +51,11 @@ func (err ErrTwoFactorNotEnrolled) Unwrap() error { return util.ErrNotExist } +const ( + totpMaxAttempts = 5 + totpAttemptsTTL = 15 * 60 // seconds, renewed by every failed attempt +) + // TwoFactor represents a two-factor authentication token. type TwoFactor struct { ID int64 `xorm:"pk autoincr"` @@ -120,26 +128,59 @@ func (t *TwoFactor) validateTOTP(passcode string) (bool, error) { return totp.Validate(passcode, secretStr), nil } +type TwoFactorResult int + +const ( + TwoFactorResultValid TwoFactorResult = iota + 1 + TwoFactorResultInvalid + TwoFactorResultTooManyAttempts +) + // ValidateAndConsumeTOTP validates the passcode and atomically records it as used so that the -// same passcode cannot be redeemed more than once (RFC 6238 §5.2). It returns false for an -// invalid passcode as well as for a replay, including the case where a concurrent request with -// the same passcode won the race first. All TOTP login surfaces must go through this helper. -func (t *TwoFactor) ValidateAndConsumeTOTP(ctx context.Context, passcode string) (bool, error) { - ok, err := t.validateTOTP(passcode) - if err != nil || !ok { - return false, err +// same passcode cannot be redeemed more than once (RFC 6238 §5.2). Invalid passcodes and replays +// return TwoFactorResultInvalid; exhausted attempts return TwoFactorResultTooManyAttempts. +// All TOTP login surfaces must go through this helper. +func (t *TwoFactor) ValidateAndConsumeTOTP(ctx context.Context, passcode string) (result TwoFactorResult, err error) { + if passcode == "" { + // password-only Basic auth clients must not use up the attempts + return TwoFactorResultInvalid, nil } - // Conditional update: only a row whose stored passcode differs from this one is updated, so a - // replay (or a concurrent duplicate) matches zero rows and is rejected. The row lock taken by - // the UPDATE serializes racing requests, closing the read-validate-write TOCTOU window. - t.LastUsedPasscode = passcode - n, err := db.GetEngine(ctx).ID(t.ID). - Where(builder.Or(builder.IsNull{"last_used_passcode"}, builder.Neq{"last_used_passcode": passcode})). - Cols("last_used_passcode").Update(t) - if err != nil { - return false, err - } - return n == 1, nil + // the lock spans check, validation and counting so that concurrent requests cannot exceed the limit + attemptsKey := fmt.Sprintf("twofa_attempts_%d", t.UID) + err = globallock.LockAndDo(ctx, attemptsKey, func(ctx context.Context) error { + value, _ := cache.GetCache().Get(attemptsKey) + attempts, _ := strconv.Atoi(value) + if attempts >= totpMaxAttempts { + result = TwoFactorResultTooManyAttempts + return nil + } + + valid, err := t.validateTOTP(passcode) + if err != nil { + return err + } + if valid { + // Conditional update: only a row whose stored passcode differs from this one is updated, so a + // replay (or a concurrent duplicate) matches zero rows and is rejected. The row lock taken by + // the UPDATE serializes racing requests, closing the read-validate-write TOCTOU window. + t.LastUsedPasscode = passcode + n, err := db.GetEngine(ctx).ID(t.ID). + Where(builder.Or(builder.IsNull{"last_used_passcode"}, builder.Neq{"last_used_passcode": passcode})). + Cols("last_used_passcode").Update(t) + if err != nil { + return err + } + valid = n == 1 + } + if !valid { + result = TwoFactorResultInvalid + return cache.GetCache().Put(attemptsKey, strconv.Itoa(attempts+1), totpAttemptsTTL) + } + cache.Remove(attemptsKey) + result = TwoFactorResultValid + return nil + }) + return result, err } // NewTwoFactor creates a new two-factor authentication token. diff --git a/models/auth/twofactor_test.go b/models/auth/twofactor_test.go index a3ed2187d0f..ad239cb988d 100644 --- a/models/auth/twofactor_test.go +++ b/models/auth/twofactor_test.go @@ -4,11 +4,15 @@ package auth_test import ( + "fmt" + "sync" + "sync/atomic" "testing" "time" auth_model "gitea.dev/models/auth" "gitea.dev/models/unittest" + "gitea.dev/modules/cache" "github.com/go-webauthn/webauthn/webauthn" "github.com/pquerna/otp/totp" @@ -31,21 +35,42 @@ func TestTwoFactorLifecycle(t *testing.T) { require.NoError(t, err) // first use of a valid passcode succeeds - ok, err := tfa.ValidateAndConsumeTOTP(t.Context(), passcode) + result, err := tfa.ValidateAndConsumeTOTP(t.Context(), passcode) require.NoError(t, err) - assert.True(t, ok) + assert.Equal(t, auth_model.TwoFactorResultValid, result) // replaying the same passcode is refused, even when still inside the TOTP validity window reloaded, err := auth_model.GetTwoFactorByUID(t.Context(), tfa.UID) require.NoError(t, err) - ok, err = reloaded.ValidateAndConsumeTOTP(t.Context(), passcode) + result, err = reloaded.ValidateAndConsumeTOTP(t.Context(), passcode) require.NoError(t, err) - assert.False(t, ok) + assert.Equal(t, auth_model.TwoFactorResultInvalid, result) - // an invalid passcode is rejected without consuming anything - ok, err = reloaded.ValidateAndConsumeTOTP(t.Context(), "000000") + var limited atomic.Int32 + var wg sync.WaitGroup + for range 10 { + wg.Go(func() { + result, err := reloaded.ValidateAndConsumeTOTP(t.Context(), "0000000") + assert.NoError(t, err) + if result == auth_model.TwoFactorResultTooManyAttempts { + limited.Add(1) + } else { + assert.Equal(t, auth_model.TwoFactorResultInvalid, result) + } + }) + } + wg.Wait() + assert.EqualValues(t, 6, limited.Load()) + nextPasscode, err := totp.GenerateCode(key.Secret(), time.Now().Add(30*time.Second)) require.NoError(t, err) - assert.False(t, ok) + result, err = reloaded.ValidateAndConsumeTOTP(t.Context(), nextPasscode) + require.NoError(t, err) + assert.Equal(t, auth_model.TwoFactorResultTooManyAttempts, result) + + cache.Remove(fmt.Sprintf("twofa_attempts_%d", tfa.UID)) + result, err = reloaded.ValidateAndConsumeTOTP(t.Context(), nextPasscode) + require.NoError(t, err) + assert.Equal(t, auth_model.TwoFactorResultValid, result) }) t.Run("DisableTwoFactor", func(t *testing.T) { diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 36ff448a385..1b22c13cf10 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -407,6 +407,7 @@ "auth.twofa_scratch_used": "You have used your scratch code. You have been redirected to the two-factor settings page so you may remove your device enrollment or generate a new scratch code.", "auth.twofa_passcode_incorrect": "Your passcode is incorrect. If you misplaced your device, use your scratch code to sign in.", "auth.twofa_scratch_token_incorrect": "Your scratch code is incorrect.", + "auth.twofa_too_many_attempts": "Too many incorrect passcodes. Try again later or use your scratch code.", "auth.twofa_required": "You must set up two-factor authentication to get access to repositories, or try to log in again.", "auth.login_userpass": "Sign In", "auth.oauth_signup_tab": "Register New Account", diff --git a/routers/web/auth/2fa.go b/routers/web/auth/2fa.go index cebc512f3c8..d2884af4b8b 100644 --- a/routers/web/auth/2fa.go +++ b/routers/web/auth/2fa.go @@ -10,9 +10,11 @@ import ( audit_model "gitea.dev/models/audit" "gitea.dev/models/auth" user_model "gitea.dev/models/user" + "gitea.dev/modules/log" "gitea.dev/modules/session" "gitea.dev/modules/setting" "gitea.dev/modules/templates" + "gitea.dev/modules/util" "gitea.dev/modules/web" "gitea.dev/services/audit" "gitea.dev/services/context" @@ -60,13 +62,13 @@ func TwoFactorPost(ctx *context.Context) { } // Validate the passcode and atomically consume it to prevent reuse/replay. - ok, err := twofa.ValidateAndConsumeTOTP(ctx, form.Passcode) + tfaResult, err := twofa.ValidateAndConsumeTOTP(ctx, form.Passcode) if err != nil { ctx.ServerError("UserSignIn", err) return } - if ok { + if tfaResult == auth.TwoFactorResultValid { remember := ctx.Session.Get("twofaRemember").(bool) //nolint:forcetypeassert // must exist u, err := user_model.GetUserByID(ctx, id) if err != nil { @@ -86,9 +88,10 @@ func TwoFactorPost(ctx *context.Context) { if u, err := user_model.GetUserByID(ctx, id); err == nil { audit.RecordAs(ctx, u, audit_model.UserAuthenticationFailTwoFactor, u) + log.Warn("Failed authentication attempt for %s from %s: invalid two-factor passcode", u.Name, ctx.RemoteAddr()) } - - ctx.RenderWithErrDeprecated(ctx.Tr("auth.twofa_passcode_incorrect"), tplTwofa, forms.TwoFactorAuthForm{}) + errMsg := ctx.Tr(util.Iif(tfaResult == auth.TwoFactorResultTooManyAttempts, "auth.twofa_too_many_attempts", "auth.twofa_passcode_incorrect")) + ctx.RenderWithErrDeprecated(errMsg, tplTwofa, forms.TwoFactorAuthForm{}) } // TwoFactorScratch shows the scratch code form for two-factor authentication. diff --git a/routers/web/auth/password.go b/routers/web/auth/password.go index 883f4d6a17b..3f47cae146e 100644 --- a/routers/web/auth/password.go +++ b/routers/web/auth/password.go @@ -16,6 +16,7 @@ import ( "gitea.dev/modules/setting" "gitea.dev/modules/templates" "gitea.dev/modules/timeutil" + "gitea.dev/modules/util" "gitea.dev/modules/web" "gitea.dev/routers/common" "gitea.dev/services/audit" @@ -187,15 +188,17 @@ func ResetPasswdPost(ctx *context.Context) { regenerateScratchToken = true } else { passcode := ctx.FormString("passcode") - ok, err := twofa.ValidateAndConsumeTOTP(ctx, passcode) + tfaResult, err := twofa.ValidateAndConsumeTOTP(ctx, passcode) if err != nil { ctx.HTTPError(http.StatusInternalServerError, "ValidateAndConsumeTOTP", err.Error()) return } - if !ok { + if tfaResult != auth.TwoFactorResultValid { + log.Warn("Failed authentication attempt for %s from %s: invalid two-factor passcode", u.Name, ctx.RemoteAddr()) ctx.Data["IsResetForm"] = true ctx.Data["Err_Passcode"] = true - ctx.RenderWithErrDeprecated(ctx.Tr("auth.twofa_passcode_incorrect"), tplResetPassword, nil) + errMsg := ctx.Tr(util.Iif(tfaResult == auth.TwoFactorResultTooManyAttempts, "auth.twofa_too_many_attempts", "auth.twofa_passcode_incorrect")) + ctx.RenderWithErrDeprecated(errMsg, tplResetPassword, nil) return } } diff --git a/services/auth/basic.go b/services/auth/basic.go index 14c31e4375e..50245c0eac1 100644 --- a/services/auth/basic.go +++ b/services/auth/basic.go @@ -176,11 +176,15 @@ func validateTOTP(req *http.Request, u *user_model.User) error { return err } // Consume the passcode atomically so a captured OTP cannot be replayed within its validity window. - if ok, err := twofa.ValidateAndConsumeTOTP(req.Context(), req.Header.Get("X-Gitea-OTP")); err != nil { + tfaResult, err := twofa.ValidateAndConsumeTOTP(req.Context(), req.Header.Get("X-Gitea-OTP")) + if err != nil { return err - } else if !ok { + } else if tfaResult != auth_model.TwoFactorResultValid { audit.RecordAs(req.Context(), u, audit_model.UserAuthenticationFailTwoFactor, u) - + log.Warn("Failed authentication attempt for %s from %s: invalid provided OTP", u.Name, req.RemoteAddr) + if tfaResult == auth_model.TwoFactorResultTooManyAttempts { + return ErrUserAuthMessage("too many failed OTP attempts, try again later") + } return util.NewInvalidArgumentErrorf("invalid provided OTP") } return nil diff --git a/tests/e2e/login.test.ts b/tests/e2e/login.test.ts index 56047fce7c6..7b1389b30ce 100644 --- a/tests/e2e/login.test.ts +++ b/tests/e2e/login.test.ts @@ -1,6 +1,7 @@ +import {createHmac} from 'node:crypto'; import {env} from 'node:process'; -import {test, expect} from '@playwright/test'; -import {logout} from './utils.ts'; +import {test, expect, type Page} from '@playwright/test'; +import {apiCreateUser, loginUser, logout, randomString, signInWithPassword} from './utils.ts'; test('homepage', async ({page}) => { await page.goto('/'); @@ -25,3 +26,42 @@ test('login form in tab order and logout', async ({page}) => { await expect(page.getByRole('link', {name: 'Sign In'})).toBeHidden(); await logout(page); }); + +function totpCode(secret: string, stepOffset: number) { + const bits = Array.from(secret, (char) => 'ABCDEFGHIJKLMNOPQRSTUVWXYZ234567'.indexOf(char).toString(2).padStart(5, '0')).join(''); + const key = Buffer.from(bits.match(/.{8}/g)!.map((byte) => parseInt(byte, 2))); + const counter = Buffer.alloc(8); + counter.writeBigUInt64BE(BigInt(Math.floor(Date.now() / 30000) + stepOffset)); + const hmac = createHmac('sha1', key).update(counter).digest(); + return String((hmac.readUInt32BE(hmac[19] & 15) & 0x7fffffff) % 1000000).padStart(6, '0'); +} + +async function submitPasscode(page: Page, passcode: string) { + await page.getByLabel('Passcode').fill(passcode); + await page.getByRole('button', {name: 'Verify'}).click(); +} + +test('totp sign-in and attempt limit', async ({page, request}) => { + const username = `e2e-totp-${randomString(8)}`; + await apiCreateUser(request, username); + await loginUser(page, username); + await page.goto('/user/settings/security/two_factor/enroll'); + const secret = (await page.locator('.user-setting-content img').getAttribute('alt'))!; + await submitPasscode(page, totpCode(secret, 0)); + await expect(page.getByText('successfully enrolled')).toBeVisible(); + + await signInWithPassword(page, username); + await page.waitForURL('**/user/two_factor'); + await submitPasscode(page, totpCode(secret, 1)); + await expect(page).not.toHaveURL(/\/user\/(login|two_factor)/); + + await signInWithPassword(page, username); + await page.waitForURL('**/user/two_factor'); + for (let attempt = 0; attempt < 5; attempt++) { + await submitPasscode(page, '0000000'); + await expect(page.getByLabel('Passcode')).toHaveValue(''); + } + await expect(page.getByText('Your passcode is incorrect')).toBeVisible(); + await submitPasscode(page, '0000000'); + await expect(page.getByText('Too many incorrect passcodes')).toBeVisible(); +}); diff --git a/tests/e2e/utils.ts b/tests/e2e/utils.ts index 7d330c51bf4..a0bb1a4babb 100644 --- a/tests/e2e/utils.ts +++ b/tests/e2e/utils.ts @@ -238,6 +238,14 @@ export async function loginUser(page: Page, username: string) { return login(page, username, testUserPassword); } +export async function signInWithPassword(page: Page, username: string) { + await page.context().clearCookies(); + await page.goto('/user/login'); + await page.getByLabel('Username or Email Address').fill(username); + await page.getByLabel('Password').fill(testUserPassword); + await page.getByRole('button', {name: 'Sign In', exact: true}).click(); +} + export async function login(page: Page, username = env.GITEA_TEST_E2E_USER, password = env.GITEA_TEST_E2E_PASSWORD) { const response = await page.request.post('/user/login', { form: {user_name: username, password}, diff --git a/tests/e2e/webauthn.test.ts b/tests/e2e/webauthn.test.ts index 3c04f824ce1..a9feb654258 100644 --- a/tests/e2e/webauthn.test.ts +++ b/tests/e2e/webauthn.test.ts @@ -1,6 +1,6 @@ import {platform} from 'node:process'; import {test, expect, type Page} from '@playwright/test'; -import {apiCreateUser, loginUser, randomString, testUserPassword} from './utils.ts'; +import {apiCreateUser, loginUser, randomString, signInWithPassword} from './utils.ts'; const signedIn = /^(?!.*\/user\/(login|webauthn))/; // the target of a finished login varies @@ -10,14 +10,6 @@ async function registerKey(page: Page, nickname: string) { await page.getByRole('button', {name: 'Add Security Key'}).click(); } -async function signInWithPassword(page: Page, username: string) { - await page.context().clearCookies(); - await page.goto('/user/login'); - await page.getByLabel('Username or Email Address').fill(username); - await page.getByLabel('Password').fill(testUserPassword); - await page.getByRole('button', {name: 'Sign In', exact: true}).click(); -} - // regression: credProtect level 3 hid the credential from the second-factor login test('security key survives credProtect', async ({page, request, browserName}) => { test.skip(browserName !== 'chromium', 'only the CDP authenticator emulates credProtect'); // eslint-disable-line playwright/no-skipped-test -- conditional skip, the reason is in the message diff --git a/tests/integration/api_twofa_test.go b/tests/integration/api_twofa_test.go index 264e04c9240..9b65a08a6ca 100644 --- a/tests/integration/api_twofa_test.go +++ b/tests/integration/api_twofa_test.go @@ -107,5 +107,13 @@ func TestAPITwoFactor(t *testing.T) { AddBasicAuth(user.Name) req.Header.Set("X-Gitea-OTP", passcode) MakeRequest(t, req, http.StatusUnauthorized) + + for range 4 { + MakeRequest(t, NewRequest(t, "GET", "/api/v1/user").AddBasicAuth(user.Name).SetHeader("X-Gitea-OTP", "0000000"), http.StatusUnauthorized) + } + nextPasscode, err := totp.GenerateCode(otpKey.Secret(), time.Now().Add(30*time.Second)) + assert.NoError(t, err) + resp := MakeRequest(t, NewRequest(t, "GET", "/api/v1/user").AddBasicAuth(user.Name).SetHeader("X-Gitea-OTP", nextPasscode), http.StatusUnauthorized) + assert.Contains(t, resp.Body.String(), "too many failed OTP attempts") }) }