From 71a82a9ac94f6002351fead927ed5404bb00a6dc Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Thu, 11 Jun 2026 09:37:40 +0000 Subject: [PATCH] Auth: Refine passcode recovery code comparison --- internal/entity/passcode.go | 6 ++++-- internal/entity/passcode_test.go | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/internal/entity/passcode.go b/internal/entity/passcode.go index 07672df93..ca238a484 100644 --- a/internal/entity/passcode.go +++ b/internal/entity/passcode.go @@ -2,6 +2,7 @@ package entity import ( "bytes" + "crypto/subtle" "errors" "fmt" "image" @@ -254,8 +255,9 @@ func (m *Passcode) Valid(code string) (valid bool, recovery bool, err error) { return false, false, authn.ErrInvalidPasscodeKey } - // Check if recovery code has been used. - if m.RecoveryCode == code { + // Check if recovery code has been used, comparing in constant time so the + // stored secret does not leak through response timing. + if m.RecoveryCode != "" && subtle.ConstantTimeCompare([]byte(m.RecoveryCode), []byte(code)) == 1 { return true, true, nil } diff --git a/internal/entity/passcode_test.go b/internal/entity/passcode_test.go index 69b5b8c71..fb2ffbc0f 100644 --- a/internal/entity/passcode_test.go +++ b/internal/entity/passcode_test.go @@ -381,6 +381,21 @@ func TestPasscode_Verify(t *testing.T) { assert.True(t, recoveryCode) assert.Nil(t, m.VerifiedAt) }) + t.Run("EmptyRecoveryCodeNeverMatches", func(t *testing.T) { + m := &Passcode{ + UID: "uqxc08w3d0ej2283", + KeyURL: "otpauth://totp/Example:alice", + RecoveryCode: "", + } + // An unset recovery code must not match any submitted code (constant-time + // compare returns 0 on a length mismatch, but the guard makes intent explicit). + valid, recoveryCode, err := m.Valid("123456") + if err != nil { + t.Fatal(err) + } + assert.False(t, valid) + assert.False(t, recoveryCode) + }) t.Run("ErrPasscodeRequired", func(t *testing.T) { m := &Passcode{ UID: "uqxc08w3d0ej2283",