diff --git a/internal/core/auth/totp/totp.go b/internal/core/auth/totp/totp.go index 19a10ba..48ebb6d 100644 --- a/internal/core/auth/totp/totp.go +++ b/internal/core/auth/totp/totp.go @@ -10,17 +10,24 @@ package totp import ( "context" + "crypto/subtle" "errors" "fmt" + "sync" "time" "github.com/pquerna/otp" "github.com/pquerna/otp/totp" ) -// ErrInvalidCode is returned when a code does not match the stored secret. +// ErrInvalidCode is returned when a code does not match the stored secret — or +// when it does but has already been accepted (a replay, L5s). The two cases are +// deliberately indistinguishable: never tell an attacker which. var ErrInvalidCode = errors.New("totp: invalid code") +// errNoStore is returned when the service was built without a Store. +var errNoStore = errors.New("totp: no store configured") + // Store persists an identity's TOTP secret. It is implemented by the auth // service, which encrypts at rest through the AEAD sealer (A9) — so this // package never touches the ciphertext or the key. @@ -51,6 +58,13 @@ type Generator struct { store Store now func() time.Time opts totp.ValidateOpts + + // mu guards last, the highest counter accepted per identity (L5s). Replay + // defence is in-process: a restart forgets the counters, but the skew + // window is only ±90s, so the exposure is bounded and the alternative (a + // DB round-trip per verification) is not worth it at M0. + mu sync.Mutex + last map[int64]uint64 } var _ Service = (*Generator)(nil) @@ -71,6 +85,7 @@ func New(issuer string, store Store) *Generator { Digits: otp.DigitsSix, Algorithm: otp.AlgorithmSHA1, }, + last: make(map[int64]uint64), } } @@ -82,10 +97,11 @@ func (g *Generator) SetClock(fn func() time.Time) { } // Begin issues a fresh secret, stores it encrypted (A9) and returns the otpauth -// URL for the authenticator app. +// URL for the authenticator app. A fresh secret resets the identity's replay +// counter (L5s): the new secret is a new sequence. func (g *Generator) Begin(ctx context.Context, identityID int64, account string) (Issued, error) { if g.store == nil { - return Issued{}, errors.New("totp: no store configured") + return Issued{}, errNoStore } key, err := totp.Generate(totp.GenerateOpts{Issuer: g.issuer, AccountName: account}) if err != nil { @@ -95,29 +111,82 @@ func (g *Generator) Begin(ctx context.Context, identityID int64, account string) if err := g.store.PutTOTPSecret(ctx, identityID, []byte(key.Secret())); err != nil { return Issued{}, err } + g.forget(identityID) return Issued{URL: key.URL()}, nil } // Verify validates code against the stored secret, then records the // verification. The code and the secret never reach an error string or a log. +// +// A code that matches but whose counter has already been accepted is rejected +// as a replay (L5s), so a code observed (shoulder-surfed, logged, phished) in +// its 90-second validity cannot be used twice. func (g *Generator) Verify(ctx context.Context, identityID int64, code string) error { if code == "" { return ErrInvalidCode } + if g.store == nil { + return errNoStore + } secret, err := g.store.TOTPSecret(ctx, identityID) if err != nil { return err } - ok, err := totp.ValidateCustom(code, string(secret), g.now().UTC(), g.opts) - if err != nil { - // A malformed code is an invalid code, not a server fault. - return ErrInvalidCode - } + counter, ok := g.matchCounter(secret, code) if !ok { return ErrInvalidCode } + if !g.acceptCounter(identityID, counter) { + return ErrInvalidCode // a replay — indistinguishable from a bad code + } if err := g.store.MarkTOTPVerified(ctx, identityID); err != nil { return err } return nil } + +// matchCounter finds the time step whose code equals code, searching the skew +// window newest-first (L5s: newest-first means the returned counter is the +// highest matching step, so an earlier step is never preferred over a later +// one). The comparison is constant-time. +func (g *Generator) matchCounter(secret []byte, code string) (uint64, bool) { + period := int64(g.opts.Period) + if period <= 0 { + period = 30 + } + skew := int64(g.opts.Skew) + base := g.now().UTC().Unix() / period + for d := skew; d >= -skew; d-- { + step := base + d + if step < 0 { + continue + } + want, err := totp.GenerateCodeCustom(string(secret), time.Unix(step*period, 0).UTC(), g.opts) + if err != nil { + continue + } + if subtle.ConstantTimeCompare([]byte(want), []byte(code)) == 1 { + return uint64(step), true + } + } + return 0, false +} + +// acceptCounter records counter as the highest accepted for identityID and +// reports whether it is strictly newer than the last one (L5s). +func (g *Generator) acceptCounter(identityID int64, counter uint64) bool { + g.mu.Lock() + defer g.mu.Unlock() + if prev, ok := g.last[identityID]; ok && counter <= prev { + return false + } + g.last[identityID] = counter + return true +} + +// forget drops an identity's replay counter (a new secret, L5s). +func (g *Generator) forget(identityID int64) { + g.mu.Lock() + defer g.mu.Unlock() + delete(g.last, identityID) +} diff --git a/internal/core/auth/totp/totp_test.go b/internal/core/auth/totp/totp_test.go index 00973a7..fc264a3 100644 --- a/internal/core/auth/totp/totp_test.go +++ b/internal/core/auth/totp/totp_test.go @@ -80,7 +80,7 @@ func TestVerifyAcceptsAValidCodeAndMarksVerified(t *testing.T) { } } -func TestVerifyRejectsBadOrEmptyCode(t *testing.T) { +func TestVerifyRejectsABadOrEmptyCode(t *testing.T) { st := newMapStore() svc := totp.New("MyDay", st) if _, err := svc.Begin(context.Background(), 1, "guid"); err != nil { @@ -96,3 +96,55 @@ func TestVerifyRejectsBadOrEmptyCode(t *testing.T) { t.Error("a bad code marked the secret verified") } } + +// TestVerifyRejectsAReplay covers L5s: a code that already worked must not work +// again within the skew window. +func TestVerifyRejectsAReplay(t *testing.T) { + st := newMapStore() + svc := totp.New("MyDay", st) + ctx := context.Background() + if _, err := svc.Begin(ctx, 1, "guid"); err != nil { + t.Fatalf("Begin: %v", err) + } + now := time.Now() + svc.SetClock(func() time.Time { return now }) + secret := string(st.secrets[1]) + + code, err := pqotp.GenerateCode(secret, now) + if err != nil { + t.Fatalf("GenerateCode: %v", err) + } + if err := svc.Verify(ctx, 1, code); err != nil { + t.Fatalf("first Verify = %v, want nil", err) + } + // The same code, still inside its window, is a replay (L5s). + if err := svc.Verify(ctx, 1, code); !errors.Is(err, totp.ErrInvalidCode) { + t.Errorf("replayed Verify = %v, want ErrInvalidCode (L5s)", err) + } + + // A code from an EARLIER step is refused once a newer step was accepted. + newer, err := pqotp.GenerateCode(secret, now.Add(30*time.Second)) + if err != nil { + t.Fatalf("GenerateCode: %v", err) + } + svc.SetClock(func() time.Time { return now.Add(30 * time.Second) }) + if err := svc.Verify(ctx, 1, newer); err != nil { + t.Fatalf("Verify(newer) = %v, want nil", err) + } + svc.SetClock(func() time.Time { return now }) // the skew window still covers step N + if err := svc.Verify(ctx, 1, code); !errors.Is(err, totp.ErrInvalidCode) { + t.Errorf("an older step after a newer one = %v, want ErrInvalidCode (L5s)", err) + } + + // A fresh secret resets the counter (re-enrollment, L5s). + if _, err := svc.Begin(ctx, 1, "guid"); err != nil { + t.Fatalf("Begin (re-enroll): %v", err) + } + fresh, err := pqotp.GenerateCode(string(st.secrets[1]), now) + if err != nil { + t.Fatalf("GenerateCode: %v", err) + } + if err := svc.Verify(ctx, 1, fresh); err != nil { + t.Errorf("Verify after re-enrollment = %v, want nil", err) + } +}