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 5080f51ff..3c69aa512 100644 --- a/internal/web/service/panel/user.go +++ b/internal/web/service/panel/user.go @@ -4,7 +4,6 @@ import ( "errors" "time" - "github.com/xlzd/gotp" "gorm.io/gorm" "github.com/mhsanaei/3x-ui/v3/internal/database" @@ -12,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" ) @@ -98,7 +98,7 @@ func (s *UserService) CheckUser(username string, password string, twoFactorCode return nil, err } - if !verifyTOTPWithSkew(twoFactorToken, twoFactorCode) { + if !totp.VerifyWithSkew(twoFactorToken, twoFactorCode, time.Now()) { return nil, errors.New("invalid 2fa code") } } @@ -106,26 +106,6 @@ 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 deleted file mode 100644 index b0501b628..000000000 --- a/internal/web/service/panel/user_totp_test.go +++ /dev/null @@ -1,33 +0,0 @@ -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") - } -} 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") }