From 0fb913388a8cbeb932b16ff24eeaf24306b8e08a Mon Sep 17 00:00:00 2001 From: "F." Date: Sat, 17 Jan 2026 10:09:46 +0100 Subject: [PATCH 1/2] feat(mfa): add backup codes, rate limiting, TOTP step, HOTP resync - Introduce BackupCodeManager for recovery codes: - Generates unique, user-facing codes and Argon2id-hashed values for storage - Input normalization (spaces/hyphens removed, uppercased), grouping, and configurable count/length/alphabet/hasher; returns remaining hashes on use - Add rate limiting to verification paths via RateLimiter interface and WithTOTPRateLimiter/WithHOTPRateLimiter/WithBackupRateLimiter; return ErrMFARateLimited on denial - TOTP: add VerifyWithStep to return the matched time step for replay protection; Verify delegates to the new flow - HOTP: add Resync to recover a drifting counter using two consecutive codes; WithHOTPResyncWindow config and internal generateCode refactor - Extend errors for conflicting options and backup code operations - Docs: update README, usage, and security checklist (backup codes, resync, rate limiting); add examples - Tests: coverage for backup codes, TOTP VerifyWithStep, HOTP resync, and rate-limiting; update cspell dictionary No breaking changes expected. --- README.md | 25 +- cspell.json | 3 + docs/security-checklist.md | 2 + docs/usage.md | 46 ++++ pkg/mfa/backup.go | 511 +++++++++++++++++++++++++++++++++++++ pkg/mfa/backup_test.go | 93 +++++++ pkg/mfa/errors.go | 10 + pkg/mfa/helpers.go | 60 +++-- pkg/mfa/hotp.go | 129 +++++++++- pkg/mfa/hotp_test.go | 84 ++++++ pkg/mfa/rate_limit_test.go | 87 +++++++ pkg/mfa/totp.go | 108 +++++++- pkg/mfa/totp_test.go | 68 +++++ pkg/mfa/types.go | 5 + 14 files changed, 1195 insertions(+), 36 deletions(-) create mode 100644 pkg/mfa/backup.go create mode 100644 pkg/mfa/backup_test.go create mode 100644 pkg/mfa/rate_limit_test.go diff --git a/README.md b/README.md index 1f0ce99..8d909aa 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ Security-focused Go helpers for file I/O, in-memory handling of sensitive data, - Symlink checks and root-scoped file access using `os.OpenRoot` - Secure in-memory buffers with best-effort zeroization - JWT/PASETO helpers with strict validation and safe defaults -- MFA helpers for TOTP/HOTP provisioning and verification +- MFA helpers for TOTP/HOTP provisioning, verification, and backup codes - Password hashing presets for argon2id/bcrypt with rehash detection - Email and URL validation with optional DNS/redirect/reputation checks - Random token generation and validation with entropy/length caps @@ -214,6 +214,29 @@ func main() { } ``` +```go +package main + +import ( + "github.com/hyp3rd/sectools/pkg/mfa" +) + +func main() { + manager, err := mfa.NewBackupCodeManager() + if err != nil { + panic(err) + } + + set, err := manager.Generate() + if err != nil { + panic(err) + } + + // Store set.Hashes and display set.Codes once. + _, _, _ = manager.Verify(set.Codes[0], set.Hashes) +} +``` + ### Password hashing ```go diff --git a/cspell.json b/cspell.json index 9738e52..dc8350a 100644 --- a/cspell.json +++ b/cspell.json @@ -172,6 +172,8 @@ "redaction", "redactor", "Renovate", + "resync", + "Resync", "sanitization", "sanitize", "sanitizer", @@ -210,6 +212,7 @@ "Tracef", "uid", "umask", + "uppercasing", "URLHTTP", "userinfo", "varnamelen", diff --git a/docs/security-checklist.md b/docs/security-checklist.md index ffb93db..82865c5 100644 --- a/docs/security-checklist.md +++ b/docs/security-checklist.md @@ -29,7 +29,9 @@ This checklist is a quick reference for teams using sectools in production. - Use `pkg/mfa` for TOTP/HOTP with drift handling and validated secrets. - Keep TOTP skew and HOTP look-ahead windows small to reduce replay risk. +- Rate limit OTP and backup code verification attempts. - Store MFA secrets securely; avoid logging provisioning URLs or raw secrets. +- Store only hashed backup codes and delete them after first use. ## Random Tokens diff --git a/docs/usage.md b/docs/usage.md index 6445e06..5278fae 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -267,11 +267,13 @@ func GenerateTOTPKey(opts ...TOTPKeyOption) (*otp.Key, error) func NewTOTP(secret string, opts ...TOTPOption) (*TOTP, error) func (t *TOTP) Generate() (string, error) func (t *TOTP) Verify(code string) (bool, error) +func (t *TOTP) VerifyWithStep(code string) (bool, uint64, error) func GenerateHOTPKey(opts ...HOTPKeyOption) (*otp.Key, error) func NewHOTP(secret string, opts ...HOTPOption) (*HOTP, error) func (h *HOTP) Generate(counter uint64) (string, error) func (h *HOTP) Verify(code string, counter uint64) (bool, uint64, error) +func (h *HOTP) Resync(code1 string, code2 string, counter uint64) (bool, uint64, error) ``` Behavior: @@ -283,6 +285,9 @@ Behavior: - `otp.Key` exposes `URL()` and `Image()` for QR provisioning. - Store secrets securely and avoid logging provisioning URLs. - Update the HOTP counter only when `Verify` returns ok. +- Use `VerifyWithStep` to store the last accepted TOTP step and reject replays. +- Use `Resync` with two consecutive HOTP codes to recover a drifting counter. +- Configure rate limiting with `WithTOTPRateLimiter`, `WithHOTPRateLimiter`, and `WithBackupRateLimiter`. Example: @@ -311,6 +316,47 @@ if !ok { } ``` +### Backup codes + +```go +func NewBackupCodeManager(opts ...BackupOption) (*BackupCodeManager, error) +func (m *BackupCodeManager) Generate() (BackupCodeSet, error) +func (m *BackupCodeManager) Verify(code string, hashes []string) (bool, []string, error) +``` + +Behavior: + +- Generates grouped codes for user display and hashed codes for storage. +- Normalizes input by removing spaces/hyphens and uppercasing. +- Uses Argon2id (balanced) for hashing by default; configurable via options. +- `Verify` returns the remaining hashes with the matched entry removed. +- Store only hashes; backup codes are one-time use. + +Example: + +```go +import "github.com/hyp3rd/sectools/pkg/mfa" + +manager, err := mfa.NewBackupCodeManager() +if err != nil { + panic(err) +} + +set, err := manager.Generate() +if err != nil { + panic(err) +} + +// Persist set.Hashes and show set.Codes to the user once. +ok, remaining, err := manager.Verify(set.Codes[0], set.Hashes) +if err != nil { + panic(err) +} + +_ = remaining +_ = ok +``` + ## pkg/password ```go diff --git a/pkg/mfa/backup.go b/pkg/mfa/backup.go new file mode 100644 index 0000000..eceb0b2 --- /dev/null +++ b/pkg/mfa/backup.go @@ -0,0 +1,511 @@ +package mfa + +import ( + "crypto/rand" + "fmt" + "io" + "strings" + + "github.com/hyp3rd/ewrap" + + "github.com/hyp3rd/sectools/pkg/password" +) + +const ( + backupDefaultCount = 10 + backupMinCount = 1 + backupMaxCount = 20 + backupDefaultLength = 12 + backupMinLength = 8 + backupMaxLength = 32 + backupDefaultGroupSize = 4 + backupGroupDisabled = 0 + backupMinAlphabetSize = 16 + backupGenerationAttemptsPerKey = 10 + backupGroupSeparator = "-" + backupDefaultAlphabet = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789" // cspell:disable-line + byteRange = 256 +) + +// BackupHasher hashes and verifies backup codes for storage. +type BackupHasher interface { + Hash(code []byte) (string, error) + Verify(code []byte, hash string) (bool, error) +} + +type argon2idBackupHasher struct { + hasher *password.Argon2idHasher +} + +func (h argon2idBackupHasher) Hash(code []byte) (string, error) { + return h.hasher.Hash(code) +} + +func (h argon2idBackupHasher) Verify(code []byte, hash string) (bool, error) { + ok, _, err := h.hasher.Verify(code, hash) + + return ok, err +} + +type bcryptBackupHasher struct { + hasher *password.BcryptHasher +} + +func (h bcryptBackupHasher) Hash(code []byte) (string, error) { + return h.hasher.Hash(code) +} + +func (h bcryptBackupHasher) Verify(code []byte, hash string) (bool, error) { + ok, _, err := h.hasher.Verify(code, hash) + + return ok, err +} + +// BackupCodeSet contains the generated backup codes and their hashes. +type BackupCodeSet struct { + Codes []string + Hashes []string +} + +// BackupCodeManager generates and verifies recovery codes. +// Instances are immutable and safe for concurrent use. +type BackupCodeManager struct { + cfg backupConfig + hasher BackupHasher + alphabet []byte + allowed [byteRange]bool + reader io.Reader +} + +// BackupOption configures backup code generation and verification. +type BackupOption func(*backupConfig) error + +type backupConfig struct { + count int + length int + groupSize int + alphabet string + hasher BackupHasher + hasherSet bool + reader io.Reader + rateLimiter RateLimiter +} + +// NewBackupCodeManager constructs a backup code manager with safe defaults. +func NewBackupCodeManager(opts ...BackupOption) (*BackupCodeManager, error) { + cfg := defaultBackupConfig() + + for _, opt := range opts { + if opt == nil { + continue + } + + err := opt(&cfg) + if err != nil { + return nil, err + } + } + + err := validateBackupConfig(&cfg) + if err != nil { + return nil, err + } + + normalizedAlphabet, allowed, err := normalizeAlphabet(cfg.alphabet) + if err != nil { + return nil, err + } + + hasher, err := resolveBackupHasher(&cfg) + if err != nil { + return nil, err + } + + reader := cfg.reader + if reader == nil { + reader = rand.Reader + } + + return &BackupCodeManager{ + cfg: cfg, + hasher: hasher, + alphabet: []byte(normalizedAlphabet), + allowed: allowed, + reader: reader, + }, nil +} + +// Generate produces a set of backup codes and hashes for storage. +func (m *BackupCodeManager) Generate() (BackupCodeSet, error) { + target := m.cfg.count + codes := make([]string, 0, target) + hashes := make([]string, 0, target) + seen := make(map[string]struct{}, target) + + maxAttempts := m.cfg.count * backupGenerationAttemptsPerKey + attempts := 0 + + for len(codes) < target { + if attempts >= maxAttempts { + return BackupCodeSet{}, ErrMFABackupGenerationFailed + } + + raw, err := m.generateRawCode() + if err != nil { + return BackupCodeSet{}, fmt.Errorf(mfaWrapFormat, ErrMFABackupGenerationFailed, err) + } + + attempts++ + + if _, exists := seen[raw]; exists { + continue + } + + seen[raw] = struct{}{} + + hash, err := m.hasher.Hash([]byte(raw)) + if err != nil { + return BackupCodeSet{}, fmt.Errorf(mfaWrapFormat, ErrMFABackupHashFailed, err) + } + + codes = append(codes, formatBackupCode(raw, m.cfg.groupSize)) + hashes = append(hashes, hash) + } + + return BackupCodeSet{Codes: codes, Hashes: hashes}, nil +} + +// Verify checks a backup code and returns the remaining hashes if it matched. +func (m *BackupCodeManager) Verify(code string, hashes []string) (bool, []string, error) { + err := checkRateLimiter(m.cfg.rateLimiter) + if err != nil { + return false, nil, err + } + + normalized, err := m.normalizeBackupCode(code) + if err != nil { + return false, nil, err + } + + remaining := make([]string, 0, len(hashes)) + + for _, hash := range hashes { + ok, err := m.hasher.Verify([]byte(normalized), hash) + if err != nil { + return false, nil, fmt.Errorf(mfaWrapFormat, ErrMFABackupVerificationFailed, err) + } + + if ok { + continue + } + + remaining = append(remaining, hash) + } + + if len(remaining) == len(hashes) { + return false, remaining, nil + } + + return true, remaining, nil +} + +// WithBackupCodeCount sets the number of backup codes to generate. +func WithBackupCodeCount(count int) BackupOption { + return func(cfg *backupConfig) error { + if count < backupMinCount || count > backupMaxCount { + return ErrInvalidMFAConfig + } + + cfg.count = count + + return nil + } +} + +// WithBackupCodeLength sets the length of each backup code. +func WithBackupCodeLength(length int) BackupOption { + return func(cfg *backupConfig) error { + if length < backupMinLength || length > backupMaxLength { + return ErrInvalidMFAConfig + } + + cfg.length = length + + return nil + } +} + +// WithBackupCodeGroupSize sets the grouping size for formatting. +func WithBackupCodeGroupSize(size int) BackupOption { + return func(cfg *backupConfig) error { + if size < backupGroupDisabled { + return ErrInvalidMFAConfig + } + + cfg.groupSize = size + + return nil + } +} + +// WithBackupCodeAlphabet sets the alphabet used for backup code generation. +func WithBackupCodeAlphabet(alphabet string) BackupOption { + return func(cfg *backupConfig) error { + trimmed := strings.TrimSpace(alphabet) + if trimmed == "" { + return ErrInvalidMFAConfig + } + + cfg.alphabet = trimmed + + return nil + } +} + +// WithBackupHasher sets a custom hasher for backup codes. +func WithBackupHasher(hasher BackupHasher) BackupOption { + return func(cfg *backupConfig) error { + if hasher == nil { + return ErrInvalidMFAConfig + } + + if cfg.hasherSet { + return ErrMFAConflictingOptions + } + + cfg.hasher = hasher + cfg.hasherSet = true + + return nil + } +} + +// WithBackupHasherArgon2id configures Argon2id hashing for backup codes. +func WithBackupHasherArgon2id(params password.Argon2idParams) BackupOption { + return func(cfg *backupConfig) error { + if cfg.hasherSet { + return ErrMFAConflictingOptions + } + + hasher, err := password.NewArgon2id(params) + if err != nil { + return fmt.Errorf(mfaWrapFormat, ErrInvalidMFAConfig, err) + } + + cfg.hasher = argon2idBackupHasher{hasher: hasher} + cfg.hasherSet = true + + return nil + } +} + +// WithBackupHasherBcrypt configures bcrypt hashing for backup codes. +func WithBackupHasherBcrypt(cost int) BackupOption { + return func(cfg *backupConfig) error { + if cfg.hasherSet { + return ErrMFAConflictingOptions + } + + hasher, err := password.NewBcrypt(cost) + if err != nil { + return fmt.Errorf(mfaWrapFormat, ErrInvalidMFAConfig, err) + } + + cfg.hasher = bcryptBackupHasher{hasher: hasher} + cfg.hasherSet = true + + return nil + } +} + +// WithBackupCodeReader sets the randomness source for backup code generation. +func WithBackupCodeReader(reader io.Reader) BackupOption { + return func(cfg *backupConfig) error { + if reader == nil { + return ErrInvalidMFAConfig + } + + cfg.reader = reader + + return nil + } +} + +// WithBackupRateLimiter sets a rate limiter for backup code verification. +func WithBackupRateLimiter(limiter RateLimiter) BackupOption { + return func(cfg *backupConfig) error { + if limiter == nil { + return ErrInvalidMFAConfig + } + + cfg.rateLimiter = limiter + + return nil + } +} + +func defaultBackupConfig() backupConfig { + return backupConfig{ + count: backupDefaultCount, + length: backupDefaultLength, + groupSize: backupDefaultGroupSize, + alphabet: backupDefaultAlphabet, + reader: rand.Reader, + } +} + +func validateBackupConfig(cfg *backupConfig) error { + if cfg.count < backupMinCount || cfg.count > backupMaxCount { + return ErrInvalidMFAConfig + } + + if cfg.length < backupMinLength || cfg.length > backupMaxLength { + return ErrInvalidMFAConfig + } + + if cfg.groupSize < backupGroupDisabled || cfg.groupSize > cfg.length { + return ErrInvalidMFAConfig + } + + if strings.TrimSpace(cfg.alphabet) == "" { + return ErrInvalidMFAConfig + } + + if cfg.reader == nil { + return ErrInvalidMFAConfig + } + + return nil +} + +func resolveBackupHasher(cfg *backupConfig) (BackupHasher, error) { + if cfg.hasher != nil { + return cfg.hasher, nil + } + + hasher, err := password.NewArgon2id(password.Argon2idBalanced()) + if err != nil { + return nil, fmt.Errorf(mfaWrapFormat, ErrInvalidMFAConfig, err) + } + + return argon2idBackupHasher{hasher: hasher}, nil +} + +func normalizeAlphabet(alphabet string) (string, [byteRange]bool, error) { + var allowed [byteRange]bool + + normalized := strings.ToUpper(strings.TrimSpace(alphabet)) + if normalized == "" { + return "", allowed, ErrInvalidMFAConfig + } + + for i := range len(normalized) { + ch := normalized[i] + if ch < '0' || ch > 'Z' || (ch > '9' && ch < 'A') { + return "", allowed, ErrInvalidMFAConfig + } + + if allowed[ch] { + return "", allowed, ErrInvalidMFAConfig + } + + allowed[ch] = true + } + + if len(normalized) < backupMinAlphabetSize { + return "", allowed, ErrInvalidMFAConfig + } + + return normalized, allowed, nil +} + +func (m *BackupCodeManager) normalizeBackupCode(code string) (string, error) { + trimmed := strings.TrimSpace(code) + if trimmed == "" { + return "", ErrMFAInvalidCode + } + + normalized := make([]byte, 0, len(trimmed)) + + for i := range len(trimmed) { + ch := trimmed[i] + switch ch { + case ' ', '-': + continue + default: + // Continue normal validation for all other characters. + } + + if ch >= 'a' && ch <= 'z' { + ch -= ('a' - 'A') + } + + if !m.allowed[ch] { + return "", ErrMFAInvalidCode + } + + normalized = append(normalized, ch) + } + + if len(normalized) != m.cfg.length { + return "", ErrMFAInvalidCode + } + + return string(normalized), nil +} + +func (m *BackupCodeManager) generateRawCode() (string, error) { + output := make([]byte, m.cfg.length) + + alphabetSize := len(m.alphabet) + if alphabetSize == 0 || alphabetSize > byteRange { + return "", ErrInvalidMFAConfig + } + + limit := byteRange - (byteRange % alphabetSize) + + for i := range output { + for { + var buf [1]byte + + _, err := io.ReadFull(m.reader, buf[:]) + if err != nil { + return "", ewrap.Wrap(err, "failed to read random bytes for backup code") + } + + if int(buf[0]) >= limit { + continue + } + + output[i] = m.alphabet[int(buf[0])%alphabetSize] + + break + } + } + + return string(output), nil +} + +func formatBackupCode(code string, groupSize int) string { + if groupSize <= backupGroupDisabled || groupSize >= len(code) { + return code + } + + separatorCount := len(code) / groupSize + if len(code)%groupSize == 0 { + separatorCount-- + } + + var builder strings.Builder + builder.Grow(len(code) + separatorCount) + + for i := range len(code) { + if i > 0 && i%groupSize == 0 { + builder.WriteString(backupGroupSeparator) + } + + builder.WriteByte(code[i]) + } + + return builder.String() +} diff --git a/pkg/mfa/backup_test.go b/pkg/mfa/backup_test.go new file mode 100644 index 0000000..5bcf71f --- /dev/null +++ b/pkg/mfa/backup_test.go @@ -0,0 +1,93 @@ +package mfa + +import ( + "errors" + "strings" + "testing" + + "github.com/hyp3rd/sectools/pkg/password" +) + +func TestBackupCodeGenerateAndVerify(t *testing.T) { + manager, err := NewBackupCodeManager( + WithBackupCodeCount(3), + WithBackupHasherBcrypt(password.BcryptInteractiveCost), + ) + if err != nil { + t.Fatalf("expected manager, got %v", err) + } + + set, err := manager.Generate() + if err != nil { + t.Fatalf("expected codes, got %v", err) + } + + if len(set.Codes) != 3 || len(set.Hashes) != 3 { + t.Fatalf("expected 3 codes and hashes") + } + + code := set.Codes[0] + ok, remaining, err := manager.Verify(code, set.Hashes) + if err != nil { + t.Fatalf("expected verify, got %v", err) + } + if !ok { + t.Fatalf("expected valid code") + } + if len(remaining) != 2 { + t.Fatalf("expected remaining hashes to shrink") + } + + ok, _, err = manager.Verify(code, remaining) + if err != nil { + t.Fatalf("expected verify, got %v", err) + } + if ok { + t.Fatalf("expected replayed code to fail") + } +} + +func TestBackupCodeVerifyNormalizesInput(t *testing.T) { + manager, err := NewBackupCodeManager( + WithBackupCodeCount(1), + WithBackupHasherBcrypt(password.BcryptInteractiveCost), + ) + if err != nil { + t.Fatalf("expected manager, got %v", err) + } + + set, err := manager.Generate() + if err != nil { + t.Fatalf("expected codes, got %v", err) + } + + code := set.Codes[0] + normalized := strings.ToLower(strings.ReplaceAll(code, "-", " ")) + + ok, remaining, err := manager.Verify(normalized, set.Hashes) + if err != nil { + t.Fatalf("expected verify, got %v", err) + } + if !ok || len(remaining) != 0 { + t.Fatalf("expected normalized code to verify") + } +} + +func TestBackupCodeInvalidInput(t *testing.T) { + manager, err := NewBackupCodeManager() + if err != nil { + t.Fatalf("expected manager, got %v", err) + } + + _, _, err = manager.Verify("invalid", []string{"hash"}) + if !errors.Is(err, ErrMFAInvalidCode) { + t.Fatalf("expected ErrMFAInvalidCode, got %v", err) + } +} + +func TestBackupCodeInvalidAlphabet(t *testing.T) { + _, err := NewBackupCodeManager(WithBackupCodeAlphabet("ABC")) + if !errors.Is(err, ErrInvalidMFAConfig) { + t.Fatalf("expected ErrInvalidMFAConfig, got %v", err) + } +} diff --git a/pkg/mfa/errors.go b/pkg/mfa/errors.go index a588491..12e313a 100644 --- a/pkg/mfa/errors.go +++ b/pkg/mfa/errors.go @@ -5,6 +5,8 @@ import "github.com/hyp3rd/ewrap" var ( // ErrInvalidMFAConfig indicates the MFA configuration is invalid. ErrInvalidMFAConfig = ewrap.New("invalid mfa config") + // ErrMFAConflictingOptions indicates MFA options are conflicting. + ErrMFAConflictingOptions = ewrap.New("mfa options are conflicting") // ErrMFAMissingIssuer indicates the issuer is required. ErrMFAMissingIssuer = ewrap.New("mfa issuer is required") // ErrMFAMissingAccountName indicates the account name is required. @@ -17,6 +19,14 @@ var ( ErrMFASecretTooLong = ewrap.New("mfa secret is too long") // ErrMFAInvalidCode indicates the otp code is invalid. ErrMFAInvalidCode = ewrap.New("mfa otp code is invalid") + // ErrMFARateLimited indicates an MFA verification was rate limited. + ErrMFARateLimited = ewrap.New("mfa rate limit exceeded") + // ErrMFABackupGenerationFailed indicates backup code generation failed. + ErrMFABackupGenerationFailed = ewrap.New("mfa backup code generation failed") + // ErrMFABackupHashFailed indicates backup code hashing failed. + ErrMFABackupHashFailed = ewrap.New("mfa backup code hashing failed") + // ErrMFABackupVerificationFailed indicates backup code verification failed. + ErrMFABackupVerificationFailed = ewrap.New("mfa backup code verification failed") // ErrMFAInvalidCounter indicates the hotp counter is invalid. ErrMFAInvalidCounter = ewrap.New("mfa counter is invalid") ) diff --git a/pkg/mfa/helpers.go b/pkg/mfa/helpers.go index ec46dc8..6012733 100644 --- a/pkg/mfa/helpers.go +++ b/pkg/mfa/helpers.go @@ -3,31 +3,34 @@ package mfa import ( "crypto/subtle" "encoding/base32" + "fmt" "strings" "time" ) const ( - totpDefaultPeriod = 30 * time.Second - totpMinPeriod = 15 * time.Second - totpMaxPeriod = 5 * time.Minute - totpDefaultSkew = 1 - totpMaxSkew = 3 - hotpDefaultLookAhead = 3 - hotpMaxLookAhead = 10 - mfaDefaultSecretSize = 20 - mfaDefaultMinSecret = 16 - mfaAbsoluteMinSecret = 10 - mfaMaxSecret = 64 - digitsSixLength = 6 - digitsEightLength = 8 - secretPaddingCharacter = "=" - mfaWrapFormat = "%w: %w" - digitsInvalidLength int = 0 - zeroDuration time.Duration = 0 - zeroUint64 uint64 = 0 - counterIncrement uint64 = 1 - constantTimeMatch int = 1 + totpDefaultPeriod = 30 * time.Second + totpMinPeriod = 15 * time.Second + totpMaxPeriod = 5 * time.Minute + totpDefaultSkew = 1 + totpMaxSkew = 3 + hotpDefaultLookAhead = 3 + hotpMaxLookAhead = 10 + hotpDefaultResyncWindow = 10 + hotpMaxResyncWindow = 20 + mfaDefaultSecretSize = 20 + mfaDefaultMinSecret = 16 + mfaAbsoluteMinSecret = 10 + mfaMaxSecret = 64 + digitsSixLength = 6 + digitsEightLength = 8 + secretPaddingCharacter = "=" + mfaWrapFormat = "%w: %w" + digitsInvalidLength int = 0 + zeroDuration time.Duration = 0 + zeroUint64 uint64 = 0 + counterIncrement uint64 = 1 + constantTimeMatch int = 1 ) func isValidDigits(digits Digits) bool { @@ -128,3 +131,20 @@ func constantTimeEquals(a, b string) bool { return subtle.ConstantTimeCompare([]byte(a), []byte(b)) == constantTimeMatch } + +func checkRateLimiter(limiter RateLimiter) error { + if limiter == nil { + return nil + } + + allowed, err := limiter.Allow() + if err != nil { + return fmt.Errorf(mfaWrapFormat, ErrMFARateLimited, err) + } + + if !allowed { + return ErrMFARateLimited + } + + return nil +} diff --git a/pkg/mfa/hotp.go b/pkg/mfa/hotp.go index 3d35426..fe4b35e 100644 --- a/pkg/mfa/hotp.go +++ b/pkg/mfa/hotp.go @@ -25,7 +25,9 @@ type hotpConfig struct { digits Digits algorithm Algorithm lookAhead uint + resyncWindow uint minSecretBytes int + rateLimiter RateLimiter } // NewHOTP constructs an HOTP helper using the provided base32 secret. @@ -61,17 +63,17 @@ func NewHOTP(secret string, opts ...HOTPOption) (*HOTP, error) { // Generate returns the HOTP code for the specified counter. func (h *HOTP) Generate(counter uint64) (string, error) { - code, err := hotp.GenerateCodeCustom(h.secret, counter, h.validateOpts()) - if err != nil { - return "", fmt.Errorf(mfaWrapFormat, ErrInvalidMFAConfig, err) - } - - return code, nil + return h.generateCode(counter) } // Verify checks whether the supplied HOTP code is valid for the counter window. // On success, it returns true and the next counter to persist. func (h *HOTP) Verify(code string, counter uint64) (bool, uint64, error) { + err := checkRateLimiter(h.opts.rateLimiter) + if err != nil { + return false, counter, err + } + normalized, err := normalizeCode(code, h.opts.digits) if err != nil { return false, counter, err @@ -83,9 +85,9 @@ func (h *HOTP) Verify(code string, counter uint64) (bool, uint64, error) { return false, counter, ErrMFAInvalidCounter } - candidate, err := hotp.GenerateCodeCustom(h.secret, counter+offset, h.validateOpts()) + candidate, err := h.generateCode(counter + offset) if err != nil { - return false, counter, fmt.Errorf(mfaWrapFormat, ErrInvalidMFAConfig, err) + return false, counter, err } if constantTimeEquals(normalized, candidate) { @@ -96,6 +98,86 @@ func (h *HOTP) Verify(code string, counter uint64) (bool, uint64, error) { return false, counter, nil } +// Resync verifies two consecutive HOTP codes to recover a drifting counter. +// On success, it returns true and the next counter to persist. +func (h *HOTP) Resync(code1, code2 string, counter uint64) (bool, uint64, error) { + err := checkRateLimiter(h.opts.rateLimiter) + if err != nil { + return false, counter, err + } + + normalized1, err := normalizeCode(code1, h.opts.digits) + if err != nil { + return false, counter, err + } + + normalized2, err := normalizeCode(code2, h.opts.digits) + if err != nil { + return false, counter, err + } + + return h.resyncWithWindow(normalized1, normalized2, counter) +} + +func (h *HOTP) resyncWithWindow(code1, code2 string, counter uint64) (bool, uint64, error) { + maxOffset := uint64(h.opts.resyncWindow) + for offset := zeroUint64; offset <= maxOffset; offset++ { + ok, next, err := h.resyncAtOffset(code1, code2, counter, offset) + if err != nil || ok { + return ok, next, err + } + } + + return false, counter, nil +} + +func (h *HOTP) resyncAtOffset(code1, code2 string, counter, offset uint64) (bool, uint64, error) { + if counter > math.MaxUint64-offset { + return false, counter, ErrMFAInvalidCounter + } + + base := counter + offset + + candidate1, err := h.generateCode(base) + if err != nil { + return false, counter, err + } + + if !constantTimeEquals(code1, candidate1) { + return false, counter, nil + } + + if base > math.MaxUint64-counterIncrement { + return false, counter, ErrMFAInvalidCounter + } + + next := base + counterIncrement + + candidate2, err := h.generateCode(next) + if err != nil { + return false, counter, err + } + + if !constantTimeEquals(code2, candidate2) { + return false, counter, nil + } + + if next > math.MaxUint64-counterIncrement { + return false, counter, ErrMFAInvalidCounter + } + + return true, next + counterIncrement, nil +} + +func (h *HOTP) generateCode(counter uint64) (string, error) { + code, err := hotp.GenerateCodeCustom(h.secret, counter, h.validateOpts()) + if err != nil { + return "", fmt.Errorf(mfaWrapFormat, ErrInvalidMFAConfig, err) + } + + return code, nil +} + func (h *HOTP) validateOpts() hotp.ValidateOpts { return hotp.ValidateOpts{ Digits: h.opts.digits, @@ -142,6 +224,19 @@ func WithHOTPWindow(lookAhead uint) HOTPOption { } } +// WithHOTPResyncWindow sets the look-ahead window used for resync. +func WithHOTPResyncWindow(resyncWindow uint) HOTPOption { + return func(cfg *hotpConfig) error { + if resyncWindow > hotpMaxResyncWindow { + return ErrInvalidMFAConfig + } + + cfg.resyncWindow = resyncWindow + + return nil + } +} + // WithHOTPSecretMinBytes sets the minimum secret length in bytes. func WithHOTPSecretMinBytes(minBytes int) HOTPOption { return func(cfg *hotpConfig) error { @@ -155,11 +250,25 @@ func WithHOTPSecretMinBytes(minBytes int) HOTPOption { } } +// WithHOTPRateLimiter sets a rate limiter for HOTP verification. +func WithHOTPRateLimiter(limiter RateLimiter) HOTPOption { + return func(cfg *hotpConfig) error { + if limiter == nil { + return ErrInvalidMFAConfig + } + + cfg.rateLimiter = limiter + + return nil + } +} + func defaultHOTPConfig() hotpConfig { return hotpConfig{ digits: DigitsSix, algorithm: AlgorithmSHA1, lookAhead: hotpDefaultLookAhead, + resyncWindow: hotpDefaultResyncWindow, minSecretBytes: mfaDefaultMinSecret, } } @@ -173,6 +282,10 @@ func validateHOTPConfig(cfg hotpConfig) error { return ErrInvalidMFAConfig } + if cfg.resyncWindow > hotpMaxResyncWindow { + return ErrInvalidMFAConfig + } + if cfg.minSecretBytes < mfaAbsoluteMinSecret || cfg.minSecretBytes > mfaMaxSecret { return ErrInvalidMFAConfig } diff --git a/pkg/mfa/hotp_test.go b/pkg/mfa/hotp_test.go index 717ffef..776ce1c 100644 --- a/pkg/mfa/hotp_test.go +++ b/pkg/mfa/hotp_test.go @@ -66,6 +66,61 @@ func TestHOTPVerifyWindow(t *testing.T) { } } +func TestHOTPResync(t *testing.T) { + helper, err := NewHOTP(hotpTestSecret, WithHOTPResyncWindow(3)) + if err != nil { + t.Fatalf("expected hotp helper, got %v", err) + } + + counter := uint64(5) + code1, err := helper.Generate(counter + 2) + if err != nil { + t.Fatalf("expected code, got %v", err) + } + + code2, err := helper.Generate(counter + 3) + if err != nil { + t.Fatalf("expected code, got %v", err) + } + + ok, next, err := helper.Resync(code1, code2, counter) + if err != nil { + t.Fatalf("expected resync, got %v", err) + } + if !ok { + t.Fatalf("expected valid resync") + } + if next != counter+4 { + t.Fatalf("expected next counter %d, got %d", counter+4, next) + } +} + +func TestHOTPResyncRejectsNonConsecutive(t *testing.T) { + helper, err := NewHOTP(hotpTestSecret, WithHOTPResyncWindow(3)) + if err != nil { + t.Fatalf("expected hotp helper, got %v", err) + } + + counter := uint64(5) + code1, err := helper.Generate(counter + 1) + if err != nil { + t.Fatalf("expected code, got %v", err) + } + + code2, err := helper.Generate(counter + 3) + if err != nil { + t.Fatalf("expected code, got %v", err) + } + + ok, _, err := helper.Resync(code1, code2, counter) + if err != nil { + t.Fatalf("expected resync, got %v", err) + } + if ok { + t.Fatalf("expected resync to fail") + } +} + func TestHOTPInvalidCode(t *testing.T) { helper, err := NewHOTP(hotpTestSecret) if err != nil { @@ -85,6 +140,35 @@ func TestHOTPSecretTooShort(t *testing.T) { } } +func TestHOTPInvalidOptions(t *testing.T) { + tests := []struct { + name string + opts []HOTPOption + }{ + { + name: "invalid-digits", + opts: []HOTPOption{WithHOTPDigits(Digits(99))}, + }, + { + name: "invalid-algorithm", + opts: []HOTPOption{WithHOTPAlgorithm(Algorithm(99))}, + }, + { + name: "invalid-resync-window", + opts: []HOTPOption{WithHOTPResyncWindow(hotpMaxResyncWindow + 1)}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := NewHOTP(hotpTestSecret, tt.opts...) + if !errors.Is(err, ErrInvalidMFAConfig) { + t.Fatalf("expected ErrInvalidMFAConfig, got %v", err) + } + }) + } +} + func TestGenerateHOTPKey(t *testing.T) { key, err := GenerateHOTPKey( WithHOTPKeyIssuer(hotpTestIssuer), diff --git a/pkg/mfa/rate_limit_test.go b/pkg/mfa/rate_limit_test.go new file mode 100644 index 0000000..411ff41 --- /dev/null +++ b/pkg/mfa/rate_limit_test.go @@ -0,0 +1,87 @@ +package mfa + +import ( + "errors" + "testing" + "time" +) + +type testRateLimiter struct { + allow bool + err error + calls int +} + +func (t *testRateLimiter) Allow() (bool, error) { + t.calls++ + return t.allow, t.err +} + +func TestTOTPVerifyRateLimited(t *testing.T) { + limiter := &testRateLimiter{} + + helper, err := NewTOTP(totpTestSecret, WithTOTPRateLimiter(limiter)) + if err != nil { + t.Fatalf("expected totp helper, got %v", err) + } + + _, err = helper.Verify("123456") + if !errors.Is(err, ErrMFARateLimited) { + t.Fatalf("expected ErrMFARateLimited, got %v", err) + } + if limiter.calls != 1 { + t.Fatalf("expected limiter to be called") + } +} + +func TestHOTPVerifyRateLimited(t *testing.T) { + limiter := &testRateLimiter{} + + helper, err := NewHOTP(hotpTestSecret, WithHOTPRateLimiter(limiter)) + if err != nil { + t.Fatalf("expected hotp helper, got %v", err) + } + + _, _, err = helper.Verify("123456", 0) + if !errors.Is(err, ErrMFARateLimited) { + t.Fatalf("expected ErrMFARateLimited, got %v", err) + } + if limiter.calls != 1 { + t.Fatalf("expected limiter to be called") + } +} + +func TestBackupVerifyRateLimited(t *testing.T) { + limiter := &testRateLimiter{} + + manager, err := NewBackupCodeManager(WithBackupRateLimiter(limiter)) + if err != nil { + t.Fatalf("expected manager, got %v", err) + } + + _, _, err = manager.Verify("ABCD", nil) + if !errors.Is(err, ErrMFARateLimited) { + t.Fatalf("expected ErrMFARateLimited, got %v", err) + } + if limiter.calls != 1 { + t.Fatalf("expected limiter to be called") + } +} + +func TestRateLimiterErrorWraps(t *testing.T) { + limiter := &testRateLimiter{allow: false, err: errors.New("backend")} + + helper, err := NewTOTP( + totpTestSecret, + WithTOTPRateLimiter(limiter), + WithTOTPClock(func() time.Time { return time.Now() }), + ) + if err != nil { + t.Fatalf("expected totp helper, got %v", err) + } + + _, err = helper.Verify("123456") + if err == nil || !errors.Is(err, ErrMFARateLimited) { + t.Fatalf("expected ErrMFARateLimited, got %v", err) + } +} diff --git a/pkg/mfa/totp.go b/pkg/mfa/totp.go index 3569d53..2eb4c29 100644 --- a/pkg/mfa/totp.go +++ b/pkg/mfa/totp.go @@ -6,6 +6,7 @@ import ( "time" "github.com/pquerna/otp" + "github.com/pquerna/otp/hotp" "github.com/pquerna/otp/totp" "github.com/hyp3rd/sectools/pkg/converters" @@ -28,6 +29,7 @@ type totpConfig struct { skew uint minSecretBytes int clock func() time.Time + rateLimiter RateLimiter } // NewTOTP constructs a TOTP helper using the provided base32 secret. @@ -68,7 +70,15 @@ func (t *TOTP) Generate() (string, error) { // Verify checks whether the supplied TOTP code is valid for the current time. func (t *TOTP) Verify(code string) (bool, error) { - return t.verifyAt(code, t.opts.clock()) + ok, _, err := t.verifyAtWithStep(code, t.opts.clock()) + + return ok, err +} + +// VerifyWithStep checks a TOTP code and returns the matched time step. +// Use the returned step to prevent replays by rejecting codes <= the last accepted step. +func (t *TOTP) VerifyWithStep(code string) (bool, uint64, error) { + return t.verifyAtWithStep(code, t.opts.clock()) } func (t *TOTP) generateAt(now time.Time) (string, error) { @@ -85,23 +95,94 @@ func (t *TOTP) generateAt(now time.Time) (string, error) { return code, nil } -func (t *TOTP) verifyAt(code string, now time.Time) (bool, error) { +func (t *TOTP) verifyAtWithStep(code string, now time.Time) (bool, uint64, error) { + err := checkRateLimiter(t.opts.rateLimiter) + if err != nil { + return false, 0, err + } + normalized, err := normalizeCode(code, t.opts.digits) if err != nil { - return false, err + return false, 0, err } opts, err := t.validateOpts() if err != nil { - return false, err + return false, 0, err + } + + baseCounter, ok, err := t.baseCounter(now) + if err != nil { + return false, 0, err + } + + if !ok { + return false, 0, nil + } + + return t.verifyWithSkew(normalized, baseCounter, opts) +} + +func (t *TOTP) baseCounter(now time.Time) (int64, bool, error) { + stepSeconds := int64(t.opts.period / time.Second) + if stepSeconds <= 0 { + return 0, false, ErrInvalidMFAConfig + } + + baseCounter := now.Unix() / stepSeconds + if baseCounter < 0 { + return 0, false, nil + } + + return baseCounter, true, nil +} + +func (t *TOTP) verifyWithSkew(normalized string, baseCounter int64, opts totp.ValidateOpts) (bool, uint64, error) { + hotpOpts := hotp.ValidateOpts{ + Digits: opts.Digits, + Algorithm: opts.Algorithm, + } + + ok, step, err := t.verifyAtCounter(normalized, baseCounter, hotpOpts) + if err != nil || ok { + return ok, step, err } - ok, err := totp.ValidateCustom(normalized, t.secret, now, opts) + for offset := uint(1); offset <= opts.Skew; offset++ { + offsetValue, err := converters.SafeInt64FromUint64(uint64(offset)) + if err != nil { + return false, 0, fmt.Errorf(mfaWrapFormat, ErrInvalidMFAConfig, err) + } + + ok, step, err = t.verifyAtCounter(normalized, baseCounter+offsetValue, hotpOpts) + if err != nil || ok { + return ok, step, err + } + + ok, step, err = t.verifyAtCounter(normalized, baseCounter-offsetValue, hotpOpts) + if err != nil || ok { + return ok, step, err + } + } + + return false, 0, nil +} + +func (t *TOTP) verifyAtCounter(normalized string, counter int64, opts hotp.ValidateOpts) (bool, uint64, error) { + if counter < 0 { + return false, 0, nil + } + + codeCandidate, err := hotp.GenerateCodeCustom(t.secret, uint64(counter), opts) if err != nil { - return false, fmt.Errorf(mfaWrapFormat, ErrInvalidMFAConfig, err) + return false, 0, fmt.Errorf(mfaWrapFormat, ErrInvalidMFAConfig, err) } - return ok, nil + if constantTimeEquals(normalized, codeCandidate) { + return true, uint64(counter), nil + } + + return false, 0, nil } func (t *TOTP) validateOpts() (totp.ValidateOpts, error) { @@ -196,6 +277,19 @@ func WithTOTPClock(clock func() time.Time) TOTPOption { } } +// WithTOTPRateLimiter sets a rate limiter for TOTP verification. +func WithTOTPRateLimiter(limiter RateLimiter) TOTPOption { + return func(cfg *totpConfig) error { + if limiter == nil { + return ErrInvalidMFAConfig + } + + cfg.rateLimiter = limiter + + return nil + } +} + func defaultTOTPConfig() totpConfig { return totpConfig{ digits: DigitsSix, diff --git a/pkg/mfa/totp_test.go b/pkg/mfa/totp_test.go index bada9b4..4f13447 100644 --- a/pkg/mfa/totp_test.go +++ b/pkg/mfa/totp_test.go @@ -81,6 +81,37 @@ func TestTOTPVerifySkew(t *testing.T) { } } +func TestTOTPVerifyWithStep(t *testing.T) { + now := time.Date(2024, time.January, 2, 15, 4, 5, 0, time.UTC) + clock := func() time.Time { + return now + } + + helper, err := NewTOTP(totpTestSecret, WithTOTPClock(clock)) + if err != nil { + t.Fatalf("expected totp helper, got %v", err) + } + + code, err := helper.Generate() + if err != nil { + t.Fatalf("expected code, got %v", err) + } + + ok, step, err := helper.VerifyWithStep(code) + if err != nil { + t.Fatalf("expected verify, got %v", err) + } + + if !ok { + t.Fatalf("expected valid code") + } + + expectedStep := uint64(now.Unix() / int64(totpDefaultPeriod/time.Second)) + if step != expectedStep { + t.Fatalf("expected step %d, got %d", expectedStep, step) + } +} + func TestTOTPInvalidCode(t *testing.T) { helper, err := NewTOTP(totpTestSecret) if err != nil { @@ -100,6 +131,43 @@ func TestTOTPSecretTooShort(t *testing.T) { } } +func TestTOTPInvalidOptions(t *testing.T) { + tests := []struct { + name string + opts []TOTPOption + }{ + { + name: "invalid-digits", + opts: []TOTPOption{WithTOTPDigits(Digits(99))}, + }, + { + name: "invalid-algorithm", + opts: []TOTPOption{WithTOTPAlgorithm(Algorithm(99))}, + }, + { + name: "period-too-short", + opts: []TOTPOption{WithTOTPPeriod(totpMinPeriod - time.Second)}, + }, + { + name: "period-not-second-aligned", + opts: []TOTPOption{WithTOTPPeriod(totpMinPeriod + time.Millisecond)}, + }, + { + name: "period-too-long", + opts: []TOTPOption{WithTOTPPeriod(totpMaxPeriod + time.Second)}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := NewTOTP(totpTestSecret, tt.opts...) + if !errors.Is(err, ErrInvalidMFAConfig) { + t.Fatalf("expected ErrInvalidMFAConfig, got %v", err) + } + }) + } +} + func TestGenerateTOTPKey(t *testing.T) { key, err := GenerateTOTPKey( WithTOTPKeyIssuer(totpTestIssuer), diff --git a/pkg/mfa/types.go b/pkg/mfa/types.go index 607825a..dc85eb5 100644 --- a/pkg/mfa/types.go +++ b/pkg/mfa/types.go @@ -23,3 +23,8 @@ const ( // DigitsEight uses 8-digit OTP codes. DigitsEight = otp.DigitsEight ) + +// RateLimiter enforces rate limiting for MFA verification attempts. +type RateLimiter interface { + Allow() (bool, error) +} From 25cc4c7fe573171de915a9c498d791912c9adfe1 Mon Sep 17 00:00:00 2001 From: "F." <62474964+hyp3rd@users.noreply.github.com> Date: Sat, 17 Jan 2026 10:21:28 +0100 Subject: [PATCH 2/2] Update pkg/mfa/backup.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/mfa/backup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/mfa/backup.go b/pkg/mfa/backup.go index eceb0b2..f5fd9fa 100644 --- a/pkg/mfa/backup.go +++ b/pkg/mfa/backup.go @@ -433,7 +433,7 @@ func (m *BackupCodeManager) normalizeBackupCode(code string) (string, error) { case ' ', '-': continue default: - // Continue normal validation for all other characters. + // No-op: non-separator characters are validated in the logic below. } if ch >= 'a' && ch <= 'z' {