From d440c2b932e29a014d63888f2a46d051f9725dfa Mon Sep 17 00:00:00 2001 From: sdhfsl Date: Tue, 15 Sep 2026 20:42:30 +0800 Subject: [PATCH] fix(panel): accept 2FA codes from adjacent TOTP windows (#6546) * fix(panel): accept 2FA codes from adjacent TOTP windows CheckUser compared only gotp.Now(), so a code submitted at the end of its 30s window (or with slight client/server clock drift) failed with 'invalid 2fa code', while the immediate retry in the next window succeeded. Accept current +/-1 window, the standard TOTP skew tolerance. Fixes MHSanaei/3x-ui#6535 * fix(panel): share TOTP skew tolerance with VerifyTwoFactorCode Move the +/-1 window helper to internal/util/totp so both 2FA acceptance points use it: login (CheckUser) and disable/rebind plus username/password changes (VerifyTwoFactorCode). Also shrink comments to the 2-line house rule and anchor the unit test mid-window to avoid a step-boundary flake. Addresses review on #6546 (MEDIUM + 2 LOWs). --------- Co-authored-by: sdhfsl --- internal/util/totp/totp.go | 22 ++++++++++++ internal/util/totp/totp_test.go | 34 +++++++++++++++++++ internal/web/service/panel/user.go | 5 +-- internal/web/service/setting.go | 4 +-- internal/web/service/setting_security_test.go | 4 +++ 5 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 internal/util/totp/totp.go create mode 100644 internal/util/totp/totp_test.go diff --git a/internal/util/totp/totp.go b/internal/util/totp/totp.go new file mode 100644 index 000000000..4a25dc42e --- /dev/null +++ b/internal/util/totp/totp.go @@ -0,0 +1,22 @@ +package totp + +import ( + "time" + + "github.com/xlzd/gotp" +) + +// SkewWindows is how many 30s steps around now VerifyWithSkew accepts. +// Standard TOTP clock-drift tolerance, see MHSanaei/3x-ui#6535. +const SkewWindows = 1 + +// VerifyWithSkew accepts the code for the current step plus/minus SkewWindows. +func VerifyWithSkew(secret, code string, now time.Time) bool { + totp := gotp.NewDefaultTOTP(secret) + for i := -SkewWindows; i <= SkewWindows; i++ { + if totp.AtTime(now.Add(time.Duration(i*30)*time.Second)) == code { + return true + } + } + return false +} diff --git a/internal/util/totp/totp_test.go b/internal/util/totp/totp_test.go new file mode 100644 index 000000000..1036af6f2 --- /dev/null +++ b/internal/util/totp/totp_test.go @@ -0,0 +1,34 @@ +package totp + +import ( + "testing" + "time" + + "github.com/xlzd/gotp" +) + +func TestVerifyWithSkew(t *testing.T) { + secret := "JBSWY3DPEHPK3PXP" + totp := gotp.NewDefaultTOTP(secret) + // Anchor mid-window so a step boundary can't fall between sampling and verify. + now := time.Unix((time.Now().Unix()/30)*30+15, 0).UTC() + + if !VerifyWithSkew(secret, totp.AtTime(now), now) { + t.Fatal("current window code should verify") + } + if !VerifyWithSkew(secret, totp.AtTime(now.Add(-30*time.Second)), now) { + t.Fatal("previous window code should verify (clock skew)") + } + if !VerifyWithSkew(secret, totp.AtTime(now.Add(30*time.Second)), now) { + t.Fatal("next window code should verify (clock skew)") + } + if VerifyWithSkew(secret, totp.AtTime(now.Add(-60*time.Second)), now) { + t.Fatal("code two windows old should not verify") + } + if VerifyWithSkew(secret, totp.AtTime(now.Add(60*time.Second)), now) { + t.Fatal("code two windows ahead should not verify") + } + if VerifyWithSkew(secret, "000000", now) { + t.Fatal("wrong code should not verify") + } +} diff --git a/internal/web/service/panel/user.go b/internal/web/service/panel/user.go index a11402187..3c69aa512 100644 --- a/internal/web/service/panel/user.go +++ b/internal/web/service/panel/user.go @@ -2,8 +2,8 @@ package panel import ( "errors" + "time" - "github.com/xlzd/gotp" "gorm.io/gorm" "github.com/mhsanaei/3x-ui/v3/internal/database" @@ -11,6 +11,7 @@ import ( "github.com/mhsanaei/3x-ui/v3/internal/logger" "github.com/mhsanaei/3x-ui/v3/internal/util/crypto" ldaputil "github.com/mhsanaei/3x-ui/v3/internal/util/ldap" + "github.com/mhsanaei/3x-ui/v3/internal/util/totp" "github.com/mhsanaei/3x-ui/v3/internal/web/service" ) @@ -97,7 +98,7 @@ func (s *UserService) CheckUser(username string, password string, twoFactorCode return nil, err } - if gotp.NewDefaultTOTP(twoFactorToken).Now() != twoFactorCode { + if !totp.VerifyWithSkew(twoFactorToken, twoFactorCode, time.Now()) { return nil, errors.New("invalid 2fa code") } } diff --git a/internal/web/service/setting.go b/internal/web/service/setting.go index 64f58d133..6848cd19a 100644 --- a/internal/web/service/setting.go +++ b/internal/web/service/setting.go @@ -15,7 +15,6 @@ import ( "time" "github.com/google/uuid" - "github.com/xlzd/gotp" "gorm.io/gorm" "github.com/mhsanaei/3x-ui/v3/internal/config" @@ -26,6 +25,7 @@ import ( "github.com/mhsanaei/3x-ui/v3/internal/util/netproxy" "github.com/mhsanaei/3x-ui/v3/internal/util/random" "github.com/mhsanaei/3x-ui/v3/internal/util/reflect_util" + "github.com/mhsanaei/3x-ui/v3/internal/util/totp" "github.com/mhsanaei/3x-ui/v3/internal/web/entity" "github.com/mhsanaei/3x-ui/v3/internal/xray" "github.com/mhsanaei/3x-ui/v3/internal/xray/dnsconf" @@ -655,7 +655,7 @@ func (s *SettingService) VerifyTwoFactorCode(code string) error { if err != nil { return err } - if strings.TrimSpace(token) == "" || !gotp.NewDefaultTOTP(token).Verify(strings.TrimSpace(code), time.Now().Unix()) { + if strings.TrimSpace(token) == "" || !totp.VerifyWithSkew(token, strings.TrimSpace(code), time.Now()) { return common.NewError("invalid two factor code") } return nil diff --git a/internal/web/service/setting_security_test.go b/internal/web/service/setting_security_test.go index cb98fa8b4..338aa65aa 100644 --- a/internal/web/service/setting_security_test.go +++ b/internal/web/service/setting_security_test.go @@ -4,6 +4,7 @@ import ( "path/filepath" "regexp" "testing" + "time" "github.com/xlzd/gotp" @@ -230,6 +231,9 @@ func TestVerifyTwoFactorCode(t *testing.T) { if err := s.VerifyTwoFactorCode(gotp.NewDefaultTOTP(token).Now()); err != nil { t.Fatalf("valid code rejected: %v", err) } + if err := s.VerifyTwoFactorCode(gotp.NewDefaultTOTP(token).AtTime(time.Now().Add(-30 * time.Second))); err != nil { + t.Fatalf("previous window code rejected: %v", err) + } if err := s.VerifyTwoFactorCode("000000"); err == nil { t.Fatal("invalid code accepted") }