feat(security): a TOTP code can be used once
RFC 6238 says the verifier must reject a code it has already accepted: a code stays valid for its whole thirty-second window, so one seen in flight — a phishing page, a proxy, over the shoulder — could be replayed until it expired. Tasks now record the counter of the last accepted step (totp_last_step), and a login accepts a code only when its step is strictly greater. The guard is in the UPDATE, not a read-then-write, so two logins racing with the same stolen code cannot both find the old value and both proceed — exactly one moves the row. auth.TOTPStep is VerifyTOTP that also reports which step matched, walking the same one-step window; the code entered a little early or late spends the step it was issued at, not the step it was checked at, so a late-but-valid code is still single-use. Verified end to end: the same code presented to two logins in one window is accepted once and refused the second time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
b4038356f0
commit
77994eff09
@@ -265,6 +265,39 @@ func TestRecoveryCodes(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TOTPStep returns the step a code matched, and it must be the same step across
|
||||
// the tolerance window — that number is what makes a code single-use, so a code
|
||||
// entered a little early or late has to spend the same step, not a new one.
|
||||
func TestTOTPStepIsStableAcrossTheWindow(t *testing.T) {
|
||||
secret, _, err := NewTOTPSecret("verdande", "k@example.com")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
now := time.Date(2026, 3, 10, 10, 0, 15, 0, time.UTC)
|
||||
code := codeAt(t, secret, now)
|
||||
|
||||
want, err := TOTPStep(secret, code, now)
|
||||
if err != nil {
|
||||
t.Fatalf("the current code did not match: %v", err)
|
||||
}
|
||||
// Verified one step late — the code is still valid, and must report the step it
|
||||
// was issued at, not the step it was checked at.
|
||||
got, err := TOTPStep(secret, code, now.Add(30*time.Second))
|
||||
if err != nil {
|
||||
t.Fatalf("code rejected one step late: %v", err)
|
||||
}
|
||||
if got != want {
|
||||
t.Errorf("same code reported step %d then %d; a replay check keyed on that would not catch it", want, got)
|
||||
}
|
||||
|
||||
if _, err := TOTPStep(secret, "000000", now); err == nil {
|
||||
t.Error("a wrong code returned a step")
|
||||
}
|
||||
if _, err := TOTPStep("", code, now); err != ErrNoTOTPSecret {
|
||||
t.Errorf("empty secret: %v, want ErrNoTOTPSecret", err)
|
||||
}
|
||||
}
|
||||
|
||||
// codeAt produces the code an authenticator app would display at that moment,
|
||||
// using the same settings VerifyTOTP checks against.
|
||||
func codeAt(t *testing.T, secret string, now time.Time) string {
|
||||
|
||||
@@ -2,6 +2,7 @@ package auth
|
||||
|
||||
import (
|
||||
"crypto/rand"
|
||||
"crypto/subtle"
|
||||
"encoding/base32"
|
||||
"errors"
|
||||
"fmt"
|
||||
@@ -82,6 +83,40 @@ func VerifyTOTP(secret, code string, now time.Time) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// TOTPStep verifies a code and returns the counter of the step it matched, so the
|
||||
// caller can refuse to accept the same step twice.
|
||||
//
|
||||
// ValidateCustom answers only yes or no, and "yes" for a code inside the skew
|
||||
// window is not enough to stop a replay: a code seen in flight stays valid for the
|
||||
// rest of its period, and the fix is to record which step was spent and never
|
||||
// accept that step or an earlier one again. That needs the number, so this walks
|
||||
// the same window ValidateCustom would — one step either side — and reports the
|
||||
// counter of the match. The comparison is constant time for the same reason the
|
||||
// password check is: a code is a secret being checked against a computed value.
|
||||
func TOTPStep(secret, code string, now time.Time) (int64, error) {
|
||||
if secret == "" {
|
||||
return 0, ErrNoTOTPSecret
|
||||
}
|
||||
code = strings.ReplaceAll(strings.TrimSpace(code), " ", "")
|
||||
|
||||
for offset := -int64(totpSkew); offset <= int64(totpSkew); offset++ {
|
||||
at := now.Add(time.Duration(offset) * totpPeriod * time.Second)
|
||||
want, err := totp.GenerateCodeCustom(secret, at, totp.ValidateOpts{
|
||||
Period: totpPeriod,
|
||||
Skew: 0,
|
||||
Digits: totpDigits,
|
||||
Algorithm: otp.AlgorithmSHA1,
|
||||
})
|
||||
if err != nil {
|
||||
return 0, ErrInvalidCode
|
||||
}
|
||||
if subtle.ConstantTimeCompare([]byte(want), []byte(code)) == 1 {
|
||||
return at.Unix() / totpPeriod, nil
|
||||
}
|
||||
}
|
||||
return 0, ErrInvalidCode
|
||||
}
|
||||
|
||||
// NewRecoveryCodes returns single-use codes for someone who has lost their phone,
|
||||
// and the hashes to store. Without these, losing a device means losing the account,
|
||||
// and the only fix left is editing the database by hand.
|
||||
|
||||
@@ -218,7 +218,24 @@ func (s *Server) handleLoginTOTP(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
err := auth.VerifyTOTP(user.TOTPSecret, req.Code, time.Now())
|
||||
step, err := auth.TOTPStep(user.TOTPSecret, req.Code, time.Now())
|
||||
if err == nil {
|
||||
// The code is right; now it must also be unused. A TOTP code stays valid for
|
||||
// its whole window, so one observed in flight — a phishing page, a proxy,
|
||||
// over the shoulder — could otherwise be replayed until it expires. Spending
|
||||
// the step here makes it single-use: a second presentation of the same code,
|
||||
// or an earlier one, finds the step already gone.
|
||||
fresh, cerr := s.db.ConsumeTOTPStep(r.Context(), user.ID, step)
|
||||
if cerr != nil {
|
||||
s.internal(w, r, "consume totp step", cerr)
|
||||
return
|
||||
}
|
||||
if !fresh {
|
||||
writeError(w, http.StatusUnauthorized, CodeUnauthorized,
|
||||
"that verification code has already been used")
|
||||
return
|
||||
}
|
||||
}
|
||||
if err != nil {
|
||||
// A recovery code is accepted in the same field. Making somebody find a
|
||||
// different form while locked out of their account is a poor time to
|
||||
|
||||
@@ -372,6 +372,50 @@ func TestTOTPEnrolmentAndTwoStepLogin(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A TOTP code works once. A code seen in flight and replayed inside its window
|
||||
// must be refused the second time, or the second factor is only as good as the
|
||||
// thirty seconds an attacker has to reuse what they captured.
|
||||
func TestTOTPCodeIsSingleUse(t *testing.T) {
|
||||
ts := newTestServer(t)
|
||||
ts.bootstrap(t)
|
||||
|
||||
resp, body := ts.do(t, "POST", "/api/v1/auth/totp/setup", map[string]string{
|
||||
"password": "et langt kodeord",
|
||||
})
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
t.Fatalf("totp setup: %d %v", resp.StatusCode, body)
|
||||
}
|
||||
secret, _ := body["secret"].(string)
|
||||
resp, body = ts.do(t, "POST", "/api/v1/auth/totp/confirm", map[string]string{
|
||||
"code": codeFor(t, secret),
|
||||
})
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
t.Fatalf("totp confirm: %d %v", resp.StatusCode, body)
|
||||
}
|
||||
|
||||
// One code, captured once and presented twice.
|
||||
code := codeFor(t, secret)
|
||||
|
||||
ts.do(t, "POST", "/api/v1/auth/logout", nil)
|
||||
ts.do(t, "POST", "/api/v1/auth/login", map[string]string{
|
||||
"email": "kristian@example.dk", "password": "et langt kodeord",
|
||||
})
|
||||
resp, body = ts.do(t, "POST", "/api/v1/auth/login/totp", map[string]string{"code": code})
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
t.Fatalf("first use of the code failed: %d %v", resp.StatusCode, body)
|
||||
}
|
||||
|
||||
// A second login, same code, same window.
|
||||
ts.do(t, "POST", "/api/v1/auth/logout", nil)
|
||||
ts.do(t, "POST", "/api/v1/auth/login", map[string]string{
|
||||
"email": "kristian@example.dk", "password": "et langt kodeord",
|
||||
})
|
||||
resp, body = ts.do(t, "POST", "/api/v1/auth/login/totp", map[string]string{"code": code})
|
||||
if resp.StatusCode != http.StatusUnauthorized {
|
||||
t.Fatalf("the same code was accepted twice: %d %v", resp.StatusCode, body)
|
||||
}
|
||||
}
|
||||
|
||||
// A recovery code is accepted in the same field as a TOTP code, and works once.
|
||||
func TestRecoveryCodeCompletesLoginOnce(t *testing.T) {
|
||||
ts := newTestServer(t)
|
||||
|
||||
@@ -0,0 +1,11 @@
|
||||
-- A TOTP code is meant to be usable once. RFC 6238 says so in as many words: the
|
||||
-- verifier must reject a code that has already been accepted, or a code observed in
|
||||
-- flight — over the shoulder, off a phishing page, out of a proxy — can be replayed
|
||||
-- for the rest of its thirty-second window.
|
||||
--
|
||||
-- totp_last_step records the counter of the most recently accepted code (Unix time
|
||||
-- divided by the period). A login accepts a code only when its step is strictly
|
||||
-- greater, so the same code cannot be spent twice and time cannot run backwards.
|
||||
-- Null means no code has been accepted yet, which is every account until its owner
|
||||
-- next signs in with one.
|
||||
ALTER TABLE users ADD COLUMN totp_last_step INTEGER;
|
||||
@@ -466,6 +466,26 @@ func (db *DB) SetTOTPSecret(ctx context.Context, userID, secret string, enabled
|
||||
return err
|
||||
}
|
||||
|
||||
// ConsumeTOTPStep records a just-accepted TOTP step and reports whether it was
|
||||
// fresh. It answers false when the step has already been spent — the same code, or
|
||||
// an earlier one, being presented a second time.
|
||||
//
|
||||
// The guard is in the statement, not in a read-then-write: two logins racing with
|
||||
// the same stolen code must not both find the old value and both proceed. The row
|
||||
// moves forward only when the new step is strictly greater, so exactly one of them
|
||||
// updates a row and the other sees nothing change.
|
||||
func (db *DB) ConsumeTOTPStep(ctx context.Context, userID string, step int64) (bool, error) {
|
||||
res, err := db.ExecContext(ctx,
|
||||
`UPDATE users SET totp_last_step = ? WHERE id = ?
|
||||
AND (totp_last_step IS NULL OR totp_last_step < ?)`,
|
||||
step, userID, step)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
n, _ := res.RowsAffected()
|
||||
return n == 1, nil
|
||||
}
|
||||
|
||||
// InboxID returns the user's Inbox project.
|
||||
func (db *DB) InboxID(ctx context.Context, userID string) (string, error) {
|
||||
var id string
|
||||
|
||||
Reference in New Issue
Block a user