diff --git a/internal/web/service/panel/user.go b/internal/web/service/panel/user.go index a11402187..5080f51ff 100644 --- a/internal/web/service/panel/user.go +++ b/internal/web/service/panel/user.go @@ -2,6 +2,7 @@ package panel import ( "errors" + "time" "github.com/xlzd/gotp" "gorm.io/gorm" @@ -97,7 +98,7 @@ func (s *UserService) CheckUser(username string, password string, twoFactorCode return nil, err } - if gotp.NewDefaultTOTP(twoFactorToken).Now() != twoFactorCode { + if !verifyTOTPWithSkew(twoFactorToken, twoFactorCode) { return nil, errors.New("invalid 2fa code") } } @@ -105,6 +106,26 @@ func (s *UserService) CheckUser(username string, password string, twoFactorCode return user, nil } +// totpSkewWindows is how many 30s steps around now are accepted. Client and +// server clocks are rarely perfectly in sync, and a code submitted at the end +// of its window may arrive after the server has rolled over — without skew +// the first attempt fails and the immediate retry (in the next window) +// succeeds, see #6535. +const totpSkewWindows = 1 + +// verifyTOTPWithSkew accepts the code for the current step plus/minus +// totpSkewWindows steps, the standard tolerance for TOTP clock drift. +func verifyTOTPWithSkew(secret, code string) bool { + totp := gotp.NewDefaultTOTP(secret) + now := time.Now() + for i := -totpSkewWindows; i <= totpSkewWindows; i++ { + if totp.AtTime(now.Add(time.Duration(i*30)*time.Second)) == code { + return true + } + } + return false +} + func (s *UserService) BumpLoginEpoch() error { db := database.GetDB() return db.Model(model.User{}). diff --git a/internal/web/service/panel/user_totp_test.go b/internal/web/service/panel/user_totp_test.go new file mode 100644 index 000000000..b0501b628 --- /dev/null +++ b/internal/web/service/panel/user_totp_test.go @@ -0,0 +1,33 @@ +package panel + +import ( + "testing" + "time" + + "github.com/xlzd/gotp" +) + +func TestVerifyTOTPWithSkew(t *testing.T) { + secret := "JBSWY3DPEHPK3PXP" + totp := gotp.NewDefaultTOTP(secret) + now := time.Now() + + if !verifyTOTPWithSkew(secret, totp.AtTime(now)) { + t.Fatal("current window code should verify") + } + if !verifyTOTPWithSkew(secret, totp.AtTime(now.Add(-30*time.Second))) { + t.Fatal("previous window code should verify (clock skew)") + } + if !verifyTOTPWithSkew(secret, totp.AtTime(now.Add(30*time.Second))) { + t.Fatal("next window code should verify (clock skew)") + } + if verifyTOTPWithSkew(secret, totp.AtTime(now.Add(-60*time.Second))) { + t.Fatal("code two windows old should not verify") + } + if verifyTOTPWithSkew(secret, totp.AtTime(now.Add(60*time.Second))) { + t.Fatal("code two windows ahead should not verify") + } + if verifyTOTPWithSkew(secret, "000000") { + t.Fatal("wrong code should not verify") + } +}