From c69c14b7831f7d202696d97aebb42b6e0ae75d3dd03d3140d5e51e201349c28c Mon Sep 17 00:00:00 2001 From: Lixen Wraith Date: Wed, 23 Sep 2026 04:14:39 -0400 Subject: [PATCH] v0.4.0 harden jwt, scram, credential handling --- .github/workflows/test.yml | 27 ++ README.md | 234 +++++++++++---- argon2.go | 57 ++-- argon2_test.go | 29 +- doc.go | 71 ++--- doc/security-audit.md | 127 +++++++++ encoding.go | 23 ++ error.go | 35 +-- example_test.go | 44 +++ go.mod | 4 +- go.sum | 8 +- hardening_test.go | 313 ++++++++++++++++++++ http.go | 60 ++-- http_test.go | 136 +++------ jwt.go | 153 +++++++--- jwt_test.go | 37 +-- scram.go | 566 +++++++++++++++++++++++++------------ scram_fuzz_test.go | 62 ++++ scram_test.go | 157 +++++----- token.go | 19 +- token_test.go | 6 +- 21 files changed, 1549 insertions(+), 619 deletions(-) create mode 100644 .github/workflows/test.yml create mode 100644 doc/security-audit.md create mode 100644 encoding.go create mode 100644 example_test.go create mode 100644 hardening_test.go create mode 100644 scram_fuzz_test.go diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..bdc8a1a --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,27 @@ +name: test +on: + push: + branches: [main] + pull_request: + workflow_dispatch: +permissions: + contents: read +jobs: + test: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + - uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5 + with: + go-version: '1.26.x' + - run: go vet ./... + - run: go test -race -count=1 ./... + - name: Bounded fuzz smoke tests + env: + GOMAXPROCS: '2' + run: | + set -eu + for target in FuzzParsePHC FuzzVerifyPassword FuzzImportCredential FuzzValidateHS256Token FuzzParseBearerToken FuzzScramServerMessages FuzzScramClientMessages; do + go test -run '^$' -fuzz "^${target}$" -fuzztime=10s -parallel=2 + done diff --git a/README.md b/README.md index bd2a772..e4f3bc5 100644 --- a/README.md +++ b/README.md @@ -1,66 +1,200 @@ -# Auth Package +# auth -Modular authentication utilities for Go applications. +Small, independent authentication components for Go 1.26+: Argon2id password +hashing, Argon2-SCRAM, HS256/RS256 JWTs, and opaque token validation. -## Features +## Argon2-SCRAM -- **Password Hashing**: Standalone Argon2id hashing with PHC format. -- **JWT**: HS256/RS256 token management via a simple facade over `golang-jwt`. -- **SCRAM-SHA256**: Client/server implementation with Argon2id KDF. -- **HTTP Auth**: Helpers for parsing Basic and Bearer authentication headers. +This is a **package-specific JSON challenge/response protocol using Argon2id and +HMAC-SHA256**, not standard SASL SCRAM-SHA-256. Both peers must implement this +protocol. It does not implement SASLprep or TLS channel binding. Use authenticated +TLS (HTTPS/WSS for HTTP/WebSocket transports), including server certificate and +hostname verification. SCRAM does not encrypt subsequent application traffic. -## Usage +Provision directly; do not create a PHC hash just to convert it: ```go -// Argon2 Password Hashing -hash, _ := auth.HashPassword("password123") -err := auth.VerifyPassword("password123", hash) - -// JWT with HS256 -jwtMgr, _ := auth.NewJWT([]byte("a-very-secure-32-byte-secret-key")) -token, _ := jwtMgr.GenerateToken("user123", map[string]any{"role": "admin"}) -userID, claims, _ := jwtMgr.ValidateToken(token) - -// SCRAM authentication -server := auth.NewScramServer() -defer server.Stop() -phcHash, _ := auth.HashPassword("password123") -cred, _ := auth.MigrateFromPHC("user", "password123", phcHash) -server.AddCredential(cred) +cred, err := auth.NewCredential(username, password) +if err != nil { + return err +} +// Persist cred.Export() in protected storage, then load with ImportCredential. +if err := server.AddCredential(cred); err != nil { + return err +} ``` -### SCRAM contract notes +For a service, construct `server` using +`NewScramServerWithDecoyKey(decoyKey)` and handle its error. Provision and persist +at least 32 random bytes for this key, separately from credentials. Reuse it across +restarts and all replicas sharing the credential database. `NewScramServer()` +remains available for single-instance use, but its random key changes unknown-user +salts on every restart, which can reveal account existence to repeated probes. +Always call `server.Stop()` at shutdown. -- Unknown usernames succeed at `ProcessClientFirstMessage` and fail at - `ProcessClientFinalMessage` with `ErrInvalidCredentials`. This is deliberate - user-enumeration protection. Do not log the first message as an auth success. -- Decoy Argon2 parameters mirror the most recently added credential. Provision - all credentials in a deployment with identical parameters, or the decoy shape - becomes a distinguisher. -- Passwords are bounded by `MaxPasswordLen` (1024 bytes) at every KDF entry - point. +The exchange is: -## Package Structure +1. Client: `NewScramClient(username, password)` then `StartAuthentication()`. +2. Server: `ProcessClientFirstMessage(first.Username, first.ClientNonce)`. +3. Client: `ProcessServerFirstMessage(challenge)`. +4. Server: `ProcessClientFinalMessage(proof.FullNonce, proof.ClientProof)`. +5. Client: `VerifyServerFinalMessage(final)` before accepting login or any token + delivered with it. -- `doc.go` - Overview and package documentation -- `argon2.go` - Standalone Argon2id password hashing -- `jwt.go` - JWT manager (HS256/RS256) wrapping `golang-jwt` -- `scram.go` - SCRAM-SHA256 client/server protocol -- `http.go` - HTTP Basic/Bearer header parsing -- `token.go` - Simple in-memory token validator -- `error.go` - Centralized error definitions +Clients reject costs below 64 MiB and 3 iterations by default, preventing a +hostile server from requesting a cheaply guessable password proof. For a trusted +deployment with a different profile, pass `WithMinArgonCost(iterations, memoryKiB)` +to `NewScramClient`. Choose this policy locally; never copy it from a challenge. +Invalid minimum configuration is rejected by `StartAuthentication`. -## Testing +Check every error and abort the exchange on failure. Only a successful **server +final** authenticates a user; use its `Username`, never a separate identity from +an untrusted request. Bind the exchange to its connection/request context in the +application. See [example_test.go](example_test.go) for a complete exchange. -```bash -go test ./... -race -count=1 -go test ./... -run '^$' -bench . -benchmem +`ScramServer` is safe for concurrent use. Each `ScramClient` belongs to one +exchange and must not be shared concurrently. Starting again discards previous +state. Challenges and final messages are single-use; errors consume client state. +`Reset` clears transient keys while retaining the public username/password fields +for retry. Secret clearing is best effort; Go strings and runtime copies cannot +be reliably erased. -# fuzz targets (run individually) -go test -run '^$' -fuzz FuzzParsePHC -fuzztime 60s -go test -run '^$' -fuzz FuzzVerifyPassword -fuzztime 60s -go test -run '^$' -fuzz FuzzImportCredential -fuzztime 60s -go test -run '^$' -fuzz FuzzValidateHS256Token -fuzztime 60s -go test -run '^$' -fuzz FuzzParseBasicAuth -fuzztime 30s +All credentials in a server must share Argon2 parameters and salt length; +`AddCredential` returns `ErrSCRAMCredentialProfile` otherwise. Load the full +credential set before accepting traffic and use identical profiles on replicas. +Unknown users receive stable decoy challenges and fail at the proof step with +`ErrInvalidCredentials`, as wrong passwords do. This reduces enumeration signals; +it is not a guarantee against statistical timing analysis. Application responses, +registration, logging, and rate limits must not disclose account existence. + +`AddCredential` validates and copies its input; check the returned error. +Replacement and `RemoveCredential(username)` invalidate pending handshakes for +that user. They do not revoke already issued JWTs or application sessions. +`Stop` discards pending handshakes and rejects further authentication. + +## Password hashing and migration + +`HashPassword(password, opts...)` returns an Argon2id PHC record. +`VerifyPassword(password, phc)` verifies it. New passwords must contain 8–1024 +**bytes**; applications should apply their own password policy. Verification and +explicit `DeriveCredential` allow older shorter passwords. Defaults are 64 MiB, +3 iterations, 4 lanes, a random 16-byte salt, and a 32-byte digest. Do not use the +low-cost test parameters in production. + +`WithTime`, `WithMemory` (KiB), and `WithThreads` configure hashing or +`NewCredential`; zero options retain defaults. All KDF entry points reject +invalid or excessive parameters before allocation. Existing short PHC salts of +8–15 bytes remain verifiable but cannot be migrated to SCRAM; reset/re-enroll +those credentials with fresh salts. + +`MigrateFromPHC(username, password, phc)` verifies the password and derives a +SCRAM credential, normally reusing the single KDF result. A non-32-byte PHC +digest requires a second derivation. After migration, remove the old PHC record +unless it is still needed by a separate authentication path: its digest is the +salted password from which both SCRAM keys can be derived. + +`ValidatePHCHashFormat` checks syntax, encoding, and structural parameter limits +without running the KDF. A syntactically valid record may still exceed execution +limits. PHC and SCRAM base64 fields must be canonical, with no CR/LF. + +## JWTs + +Create one manager per key and service policy, then reuse it concurrently: + +```go +manager, err := auth.NewJWT(secret, // at least 32 cryptographically random bytes + auth.WithIssuer("lixen-auth"), + auth.WithAudience([]string{"vi-fighter"}), + auth.WithTokenLifetime(15*time.Minute), + auth.WithLeeway(0)) +if err != nil { + return err +} +token, err := manager.GenerateToken(userID, map[string]any{"session_id": sessionID}) +if err != nil { + return err +} +subject, claims, err := manager.ValidateToken(token) +// Check err, then authorize subject and the application claims for this action. ``` +Generation uses `sub`, `exp`, `iat`, `nbf`, and configured `iss`/`aud`. Application +claims remain nested under `extra` and cannot overwrite registered claims. +Validation requires expiration and a nonempty subject, validates `nbf` and `iat` +when present, pins the signing algorithm, and enforces configured issuer and +**any matching audience**. No identity/claims are returned on failure. +Use `errors.Is` with package errors. `ErrTokenMissingClaim` covers missing or +mismatched issuer/audience; `ErrTokenNotYetValid` also covers future `iat`. + +For logwisp and vi-fighter, use distinct audiences and verify them at each +service; use separate keys where trust differs. With HS256, every verifier can +mint tokens. For separate signing and verification trust, use +`NewJWTRSA(privateKey)` and `NewJWTVerifier(publicKey)` with the same claim policy. +RSA keys must be valid and at least 2048 bits. PEM constructors accept PKCS#1 or +PKCS#8 private keys and PKCS#1 or PKIX public keys. Constructors copy keys and +audiences; create a new manager for rotation and synchronize its replacement in +the application. Options are for construction only. + +The default lifetime is 24 hours and the existing clock leeway is **5 minutes**, +including after expiration. Explicitly set `WithLeeway(0)` or a small justified +skew for new services. Expiry does not revoke an otherwise valid JWT on logout: +check a persisted session or a revocation policy when immediate revocation is +required. Do not place passwords, credential keys, or secrets in JWT claims; +JWT payloads are readable by the holder. + +`GenerateHS256Token` and `ValidateHS256Token` retain chess's API and `extra` JSON +shape. The validator shares the hardened validation path, but intentionally +retains the legacy five-minute leeway and no issuer/audience constraint. Prefer +a configured manager for new integrations. Existing chess hashing and CLI PHC +validation functions also retain their signatures. + +## Opaque tokens and HTTP + +`ParseBearerToken(header)` parses the case-insensitive Bearer scheme and RFC +6750 token alphabet. It rejects whitespace inside tokens, misplaced padding, +and oversized input. It only extracts a token; always validate it afterward. + +`SimpleTokenValidator` is a concurrent, in-memory allowlist keyed by SHA256(token). +Its zero value works. Generate high-entropy opaque tokens with `crypto/rand.Text`, +then use `AddToken`, `ValidateToken`, and `RemoveToken`. Empty and oversized tokens +are never accepted. There is no TTL or persistence; use JWTs or application +session storage when needed. `AddToken` ignores invalid lengths for API stability. + +## Resource limits + +| Input/resource | Limit | +| --- | --- | +| Password | 1024 bytes at every KDF entry | +| SCRAM username | 1–256 UTF-8 bytes; no control characters, comma, or equals | +| Client nonce / combined nonce | 256 / 512 printable ASCII bytes; no comma | +| SCRAM salt / key / proof | 16–64 / 32 / 32 decoded bytes | +| PHC record / salt / digest | 256 encoded / 8–64 / 16–64 decoded bytes | +| Argon2 execution | memory ≤256 MiB, iterations ≤16, lanes ≤16, memory ≥8×lanes KiB | +| SCRAM client minimum | 64 MiB and 3 iterations by default; trusted local policy can override | +| Argon2 combined work | memory×iterations ≤786432 KiB-passes (4×default) | +| Pending server handshakes | 4096, with a 30-second timeout | +| JWT / opaque token | 16 KiB | + +These are per-operation ceilings, not aggregate memory/CPU quotas. Set transport +body/frame limits **before decoding**, use connection/request deadlines, rate +limit by peer/account, and bound concurrent KDF calls for your available RAM. +The server proof path only hashes/HMACs; registration, migration, password checks, +and SCRAM clients run Argon2. Apply limits to all of those routes. Username and +password normalization is application policy and must be identical at both ends. + +## Verification + +```sh +go vet ./... +go test -race -count=1 ./... +go test -run '^$' -bench . -benchmem + +# Run each target separately; keep worker count bounded for KDF fuzzing. +for target in FuzzParsePHC FuzzVerifyPassword FuzzImportCredential FuzzValidateHS256Token FuzzParseBearerToken FuzzScramServerMessages FuzzScramClientMessages; do + GOMAXPROCS=2 go test -run '^$' -fuzz "^${target}$" -fuzztime=60s -parallel=2 +done +``` + +CI runs race tests, vet, and short fuzz campaigns. Fuzz KDF paths use bounded test +costs; deterministic tests cover production limits without allocating at those +limits. See [the audit report](doc/security-audit.md) for findings and scope. diff --git a/argon2.go b/argon2.go index 7e0c900..fed0553 100644 --- a/argon2.go +++ b/argon2.go @@ -25,15 +25,17 @@ const ( MaxPHCHashLen = 256 ) -// Execution budget for KDF parameters taken from an encoded record. +// Execution budget for every Argon2 entry point. // parsePHC accepts the full PHC range (m <= 4 GiB, t <= 1000, p <= 255) because // those values are well-formed; running them is a different decision. One -// crafted record would otherwise allocate 4 GiB. Applies to any Argon2 run -// whose parameters were chosen by a peer rather than by this process. +// crafted record would otherwise allocate 4 GiB. Generation uses the same +// limits so this package cannot create records its verifier refuses to execute. const ( MaxVerifyArgonMemory = 256 * 1024 // KiB MaxVerifyArgonTime = 16 MaxVerifyArgonThreads = 16 + // KiB-passes: prevent simultaneously maximizing memory and iterations. + MaxVerifyArgonWork = 4 * DefaultArgonMemory * DefaultArgonTime ) // argonParams holds configurable Argon2id parameters @@ -93,13 +95,19 @@ func HashPassword(password string, opts ...Option) (string, error) { } for _, opt := range opts { - opt(params) + if opt != nil { + opt(params) + } + } + if err := checkArgonCost(params.memory, params.time, params.threads); err != nil { + return "", err } salt := make([]byte, params.saltLen) rand.Read(salt) // cryptographically secure random bytes hash := argon2.IDKey([]byte(password), salt, params.time, params.memory, params.threads, params.keyLen) + defer clear(hash) saltB64 := base64.RawStdEncoding.EncodeToString(salt) hashB64 := base64.RawStdEncoding.EncodeToString(hash) @@ -109,16 +117,26 @@ func HashPassword(password string, opts ...Option) (string, error) { // VerifyPassword checks password against PHC-format hash (standalone) func VerifyPassword(password, phcHash string) error { - _, err := verifyPHC(password, phcHash) + r, err := verifyPHC(password, phcHash) + if r != nil { + clear(r.derived) + } return err } // MigrateFromPHC converts PHC hash to SCRAM credential func MigrateFromPHC(username, password, phcHash string) (*Credential, error) { + if err := validateUsername(username); err != nil { + return nil, err + } r, err := verifyPHC(password, phcHash) if err != nil { return nil, err } + defer clear(r.derived) + if err := validateSalt(r.salt); err != nil { + return nil, err + } if len(r.derived) == DefaultArgonKeyLen { return credentialFromSaltedPassword(username, r.derived, r.salt, r.time, r.memory, r.threads), nil } @@ -130,12 +148,13 @@ func MigrateFromPHC(username, password, phcHash string) (*Credential, error) { // password can build a credential without re-running Argon2. func credentialFromSaltedPassword(username string, saltedPassword, salt []byte, time, memory uint32, threads uint8) *Credential { clientKey := computeHMAC(saltedPassword, []byte("Client Key")) + defer clear(clientKey) serverKey := computeHMAC(saltedPassword, []byte("Server Key")) storedKey := sha256.Sum256(clientKey) return &Credential{ Username: username, - Salt: salt, + Salt: append([]byte(nil), salt...), ArgonTime: time, ArgonMemory: memory, ArgonThreads: threads, @@ -162,8 +181,7 @@ type phcResult struct { threads uint8 } -// verifyPHC validates format, bounds the password, runs the KDF once, -// and constant-time compares against the encoded digest. +// parsePHC validates the record without executing the KDF. func parsePHC(phcHash string) (*phcResult, error) { if len(phcHash) > MaxPHCHashLen { return nil, fmt.Errorf("%w: encoded hash exceeds %d bytes", ErrPHCInvalidFormat, MaxPHCHashLen) @@ -194,8 +212,8 @@ func parsePHC(phcHash string) (*phcResult, error) { return nil, fmt.Errorf("%w: failed to parse parameters", ErrPHCInvalidFormat) } - if time == 0 || memory == 0 || threads == 0 { - return nil, fmt.Errorf("%w: parameters must be non-zero", ErrPHCInvalidFormat) + if time == 0 || threads == 0 || memory < 8*uint32(threads) { + return nil, fmt.Errorf("%w: invalid Argon2 parameters", ErrPHCInvalidFormat) } if memory > 4*1024*1024 { return nil, fmt.Errorf("%w: memory parameter exceeds maximum (4GB)", ErrPHCInvalidFormat) @@ -203,31 +221,21 @@ func parsePHC(phcHash string) (*phcResult, error) { if time > 1000 { return nil, fmt.Errorf("%w: time parameter exceeds maximum (1000)", ErrPHCInvalidFormat) } - if threads > 255 { - return nil, fmt.Errorf("%w: threads parameter exceeds maximum (255)", ErrPHCInvalidFormat) - } - - salt, err := base64.RawStdEncoding.DecodeString(parts[4]) + salt, err := decodeBase64(parts[4], base64.RawStdEncoding, MaxArgonSaltLen) if err != nil { return nil, fmt.Errorf("%w: %v", ErrPHCInvalidSalt, err) } if len(salt) < 8 { return nil, fmt.Errorf("%w: salt too short (%d bytes)", ErrPHCInvalidSalt, len(salt)) } - if len(salt) > MaxArgonSaltLen { - return nil, fmt.Errorf("%w: salt too long (%d bytes)", ErrPHCInvalidSalt, len(salt)) - } - hash, err := base64.RawStdEncoding.DecodeString(parts[5]) + hash, err := decodeBase64(parts[5], base64.RawStdEncoding, MaxArgonKeyLen) if err != nil { return nil, fmt.Errorf("%w: %v", ErrPHCInvalidHash, err) } if len(hash) < 16 { return nil, fmt.Errorf("%w: hash too short (%d bytes)", ErrPHCInvalidHash, len(hash)) } - if len(hash) > MaxArgonKeyLen { - return nil, fmt.Errorf("%w: hash too long (%d bytes)", ErrPHCInvalidHash, len(hash)) - } return &phcResult{ expectedHash: hash, @@ -257,6 +265,7 @@ func verifyPHC(password, phcHash string) (*phcResult, error) { r.derived = argon2.IDKey([]byte(password), r.salt, r.time, r.memory, r.threads, uint32(len(r.expectedHash))) if subtle.ConstantTimeCompare(r.derived, r.expectedHash) != 1 { + clear(r.derived) return nil, ErrInvalidCredentials } return r, nil @@ -264,12 +273,16 @@ func verifyPHC(password, phcHash string) (*phcResult, error) { func checkArgonCost(memory, time uint32, threads uint8) error { switch { + case time == 0 || threads == 0 || memory < 8*uint32(threads): + return ErrArgonInvalidParams case memory > MaxVerifyArgonMemory: return fmt.Errorf("%w: memory %d KiB exceeds %d", ErrPHCCostTooHigh, memory, MaxVerifyArgonMemory) case time > MaxVerifyArgonTime: return fmt.Errorf("%w: time %d exceeds %d", ErrPHCCostTooHigh, time, MaxVerifyArgonTime) case threads > MaxVerifyArgonThreads: return fmt.Errorf("%w: threads %d exceeds %d", ErrPHCCostTooHigh, threads, MaxVerifyArgonThreads) + case uint64(memory)*uint64(time) > MaxVerifyArgonWork: + return fmt.Errorf("%w: memory*time exceeds %d KiB-passes", ErrPHCCostTooHigh, MaxVerifyArgonWork) } return nil } diff --git a/argon2_test.go b/argon2_test.go index 50392f3..d5ff7c0 100644 --- a/argon2_test.go +++ b/argon2_test.go @@ -214,9 +214,8 @@ func TestValidatePHCHashFormat(t *testing.T) { {"digest at cap", std(params, okSalt, rawB64(MaxArgonKeyLen)), nil}, {"digest over cap", std(params, okSalt, rawB64(MaxArgonKeyLen+1)), ErrPHCInvalidHash}, - // encoding/base64 discards CR and LF, so PHC records are not - // byte-canonical: never compare them as strings. - {"digest with newline", std(params, okSalt, okDigest[:20]+"\n"+okDigest[20:]), nil}, + // Canonical PHC fields cannot contain whitespace. + {"digest with newline", std(params, okSalt, okDigest[:20]+"\n"+okDigest[20:]), ErrPHCInvalidHash}, } for _, tc := range cases { @@ -287,22 +286,14 @@ func TestMigrateFromPHC(t *testing.T) { } func TestMigrateFromPHCShortSalt(t *testing.T) { - // parsePHC accepts 8..64 byte salts; SCRAM requires >= 16. The 32-byte - // digest branch skips that check, the fallback branch enforces it. - // See the ‼️ note on MigrateFromPHC. const pw = "testPassword123" - salt := []byte("12345678") - - cred, err := MigrateFromPHC("u", pw, phcFor(pw, salt, DefaultArgonKeyLen)) - noErr(t, err, "32-byte digest branch") - eq(t, len(cred.Salt), 8, "short salt retained") - - // the credential it produced cannot survive an export/import cycle - _, err = ImportCredential(cred.Export()) - errIs(t, err, ErrSCRAMSaltTooShort, "re-import") - - _, err = MigrateFromPHC("u", pw, phcFor(pw, salt, 20)) - errIs(t, err, ErrSCRAMSaltTooShort, "fallback branch") + for _, keyLen := range []uint32{20, DefaultArgonKeyLen} { + cred, err := MigrateFromPHC("u", pw, phcFor(pw, []byte("12345678"), keyLen)) + errIs(t, err, ErrSCRAMSaltTooShort, "short migration salt") + if cred != nil { + t.Fatal("invalid credential returned") + } + } } func TestConcurrentPasswordOperations(t *testing.T) { @@ -384,7 +375,7 @@ func FuzzVerifyPassword(f *testing.F) { // parsePHC admits m up to 4 GiB from untrusted input; clamp before // letting the fuzzer choose the KDF cost. if r, err := parsePHC(hash); err == nil && - (r.memory > 64*1024 || r.time > 4 || r.threads > 8) { + (r.memory > testArgonMemory || r.time > testArgonTime || r.threads > testArgonThreads) { return } _ = VerifyPassword(password, hash) diff --git a/doc.go b/doc.go index 97fd786..2463af9 100644 --- a/doc.go +++ b/doc.go @@ -1,53 +1,20 @@ +// Package auth provides independent Argon2id password hashing, Argon2-SCRAM +// challenge/response authentication, HS256/RS256 JWTs, and opaque token utilities. +// +// NewCredential provisions a SCRAM credential in one KDF pass. ScramServer holds +// validated credential copies and bounded, single-use handshakes. ScramClient +// performs one exchange at a time and must verify the server's final signature +// before the application trusts the authentication result. +// +// The SCRAM exchange uses a package-specific Argon2id/JSON protocol; it is not +// interoperable with the standard SASL SCRAM-SHA-256 mechanism. Applications must +// supply authenticated TLS, request limits, rate limits, and authorization. +// +// NewJWT and NewJWTRSA create reusable, concurrent token managers. Configure an +// issuer and audience for each service. ValidateToken requires expiration and a +// nonempty subject; it returns only the nested "extra" application claims. The +// one-off HS256 helpers retain their existing API for consumers such as chess. +// +// See the README and doc/security-audit.md for limits, migration, transport +// requirements, persistence, and deployment guidance. package auth - -/* -Package auth provides modular authentication components: - -# Argon2 Password Hashing - -Standalone password hashing using Argon2id: - - hash, err := auth.HashPassword("password123") - err = auth.VerifyPassword("password123", hash) - - // With custom parameters - hash, err := auth.HashPassword("password123", - auth.WithTime(5), - auth.WithMemory(128*1024)) - -# JWT Token Management - -JSON Web Token generation and validation: - - // HS256 (symmetric) - jwtMgr, _ := auth.NewJWT(secret) - token, _ := jwtMgr.GenerateToken("user1", claims) - userID, claims, _ := jwtMgr.ValidateToken(token) - - // RS256 (asymmetric) - jwtMgr, _ := auth.NewJWTRSA(privateKey) - - // One-off operations - token, _ := auth.GenerateHS256Token(secret, "user1", claims, 1*time.Hour) - -# SCRAM-SHA256 Authentication - -Server and client implementation for SCRAM: - - // Server - server := auth.NewScramServer() - defer server.Stop() - server.AddCredential(credential) - - // Client - client := auth.NewScramClient(username, password) - -# HTTP Authentication Parsing - -Utility functions for HTTP headers: - - username, password, _ := auth.ParseBasicAuth(header) - token, _ := auth.ParseBearerToken(header) - -Each module can be used independently without initializing other components. -*/ diff --git a/doc/security-audit.md b/doc/security-audit.md new file mode 100644 index 0000000..0df4501 --- /dev/null +++ b/doc/security-audit.md @@ -0,0 +1,127 @@ +# Security audit and integration notes + +Audit date: 2026-09-23. Baseline: `e4eb41658be8b1d4f46cb695837ebfe564cd2f25`. +Scope: all production code and tests in `lixenwraith/auth`, plus current auth +call sites in chess and planned use in vi-fighter/logwisp. This is a source audit +with regression, race, and bounded fuzz testing, not a formal protocol proof or +a production penetration test. + +## Findings and corrections + +| Area | Finding | Correction | +| --- | --- | --- | +| SCRAM replay | Verification cleared its atomic claim before deleting the handshake. A concurrent request could claim and successfully verify the same proof again. | Retain the claim through deletion; regression requires exactly one success under concurrent replay. | +| SCRAM client | Challenges did not need an active handshake or a nonce that extends the client's nonce. Restart retained old authentication material; final messages were reusable. | Explicit ordered states, nonce binding, one-use challenges/finals, terminal errors, and reset on restart. Recheck exported password length immediately before the KDF. | +| KDF downgrade | A hostile server could choose a tiny but valid KDF and collect a cheaply guessable client proof. | Clients default to a minimum of 64 MiB/3 iterations; an explicit local policy option supports trusted alternate profiles. | +| Resource exhaustion | Username/nonce/salt/proof inputs could allocate or retain unbounded data. Direct derivation and hash options could request unbounded Argon2 work; separate maxima allowed costly combinations. | Pre-decode byte limits, credential validation, common KDF limits at every entry point, and a combined memory×iterations ceiling. Retain the existing handshake-count cap and timeout. | +| Credential ownership | Server retained caller-owned credential pointers/slices without validation. Mutation could change challenges or keys, or race authentication. There was no account-removal API. | Validate and deep-copy credentials; add `Credential.Validate`, error-returning `AddCredential`, and `RemoveCredential`. Replacement/removal invalidates pending handshakes; successful final verification checks the active credential. | +| Account enumeration | Decoy salts changed on restart/replica boundaries. Last-added credential determined the unknown-user profile, allowing mixed profiles to disclose existence. | Add a constructor with a persistent shared decoy key; enforce one profile per server. Perform decoy HMAC work on known-user requests too. Document residual timing/application signals and load-before-serving requirements. | +| PHC migration | The common 32-byte digest branch admitted short salts that SCRAM import rejected. | Validate SCRAM salt length on both migration paths; reject the record consistently. Existing PHC verification remains available for legacy salts. | +| JWT policy | Signed tokens without a subject were accepted; future issued-at values were ignored. Unbounded token parsing and noncanonical signature encodings were accepted. | Require a nonempty subject and expiry; check issued-at; bound tokens; reject CR/LF, nonzero base64 pad bits, and unsupported critical headers. Keep algorithm pinning and configured issuer/audience checks. | +| Key/config ownership | HMAC secrets, RSA keys, and audience slices were borrowed. Invalid/weak RSA keys were not checked at construction. | Copy constructor inputs; validate RSA private material/public parameters and require at least 2048 bits. Accept PKCS#1 public PEM as well as PKIX. | +| Opaque tokens / HTTP | The allowlist could authenticate an explicitly stored empty token and its zero value panicked on insertion. Bearer parsing accepted embedded whitespace and required case-sensitive schemes. | Reject empty/oversized tokens, support the zero value, and enforce RFC 6750 Bearer syntax. Remove the unused password-header parser, its errors/tests/docs, and the generic scheme extractor. | + +Canonical base64 checks also cover PHC records and imported SCRAM credentials. +Import accepts common JSON, YAML, and TOML numeric representations (`float64`, +`json.Number`, `int`, `int64`, unsigned integers) with exact range checks. Explicit +regressions reject NaN, infinities, fractions, negatives, and overflow. Transient +key buffers are cleared where practical; this is not a guarantee of memory erasure. + +## Compatibility and rollout + +Chess at `e3e465c9a2a81e4f5904a0e646c9812caa071c31` uses `HashPassword`, +`VerifyPassword`, `ValidatePHCHashFormat`, `GenerateHS256Token`, and +`ValidateHS256Token`. These signatures and JWT `extra` structure remain intact. +The one-off validator is a compatibility adapter to the common hardened parser; +it retains the existing five-minute leeway and lack of issuer/audience constraints. +Existing well-formed chess tokens remain valid. Its service tests and CLI package +were checked against this checkout using a temporary module replacement; no +changes to chess are included in this PR. + +The inspected vi-fighter (`6fd727f848ac05ff6b76b85c8152aad995d9426e`) and logwisp +(`72c0a43c9f2ba017bf2fd62319077b3655ee4514`) revisions do not yet import auth. +Their future deployment integration is outside this package PR. + +SCRAM message JSON fields and the valid transcript encoding stay unchanged. +`NewScramClient` accepts optional local minimum-cost policy options; ordinary +two-argument calls retain their syntax, with stronger default challenge policy. +`AddCredential` now returns an error; callers should check it. Calls used as +statements still compile, but interfaces requiring the old no-result signature +must be updated (none occur in the inspected consumers). Intentionally rejected +inputs include malformed identities/nonces, mixed SCRAM profiles, noncanonical +base64, oversized tokens, and KDF parameters above the new budget. Short-salt PHC +records must be re-enrolled before SCRAM migration. A server's profile remains +fixed after removing its last credential to keep decoy responses stable. + +## Integration boundaries + +- **Transport:** this Argon2/JSON protocol is not standard SASL SCRAM-SHA-256 and + has no channel binding. Authenticated TLS/WSS and connection-bound exchange + handling belong to the caller. Always verify the server final before trusting + the login result or a token delivered with it. +- **Registration/storage:** generate salts with `NewCredential`; protect both + credential keys and the shared decoy secret. Do not retain the old PHC salted + password after migration unless a separate authentication path requires it. +- **Denial of service:** enforce request/frame limits before JSON decoding, + deadlines, rate limits, and a RAM-appropriate cap on simultaneous KDF calls. + Package ceilings are per operation. Credential count and opaque-token count + are application-controlled and are not capped by the package. +- **Authorization/revocation:** scope JWT issuer/audience to the receiving + service. HS256 verifiers can issue tokens; use RS256 when services should only + verify. Check session state when logout/rotation needs immediate revocation. + Removing a SCRAM credential cannot retract sessions already issued by the app. +- **Privacy:** map authentication failures to suitable public responses; avoid + logging passwords, proofs, keys, or bearer tokens. The decoy mechanism reduces + enumeration signals but does not promise constant-time network behavior. + +## Performance + +`NewCredential` performs one Argon2 derivation; provisioning through +`HashPassword` followed by `MigrateFromPHC` performs two. Existing PHC migration +continues to reuse its verification KDF for the normal 32-byte digest. + +JWT managers now construct their immutable parser once, and the legacy HS256 +helper shares a parser. No password-strength defaults were reduced. RSA key +validation/copying occurs at construction. The server's proof path performs +bounded hash/HMAC operations, with no Argon2 work. + +On Go 1.27.1/linux-amd64, three local benchmark samples measured direct +provisioning at 6.61–7.54 ms and ~8.39 MB allocated, versus 13.10–13.72 ms and +~16.79 MB for hash-then-migrate (8 MiB/1-iteration **test** profile). JWT +sign-plus-verify allocations dropped from 88 to 85 per operation (~5133 to +4973 bytes); median time was 14.67 µs before and 13.73 µs after. These short local +samples are indicative, not a production capacity claim. + +## Validation + +Local validation on Go 1.27.1/linux-amd64 passed `go vet ./...` and +`go test -race -count=1 -coverprofile=... ./...`, with **98.1% statement coverage**. +All seven fuzz targets passed 10-second, two-worker campaigns (over 1.8 million +executions combined); the client target was rerun after adding minimum-cost +policy. These are smoke campaigns, not exhaustive fuzzing. Chess's service tests +and CLI compilation also passed after the dependency update. + +`govulncheck v1.8.0` reported zero vulnerabilities in reachable calls or imported +packages. Its only remaining module-level advisory is **GO-2026-5932** for +`golang.org/x/crypto/openpgp`, which this package does not import and which has no +fixed release. The scan used Go 1.27.1; consumers must also keep their Go runtime +patched. + +The test suite includes concurrent replay, registration/removal, credential ownership, +protocol sequencing, resource ceilings, canonical encoding, RSA validation, +JWT trust scopes, and the compiled end-to-end example. Seven fuzz targets cover +PHC parsing/verification, credential import, JWT verification, Bearer parsing, +and both SCRAM peer inputs. CI runs vet, race tests, and bounded fuzz smoke tests. + +Dependencies were updated to `golang.org/x/crypto v0.57.0` and its +`golang.org/x/sys v0.48.0` requirement without raising Go 1.26. The Argon2 source +is unchanged from v0.54.0. The update includes fixes for SSH advisories +GO-2026-6303, GO-2026-6354, and GO-2026-6355 in unused sibling packages. + +## Primary references + +- [RFC 5802, SCRAM exchange and security considerations](https://www.rfc-editor.org/rfc/rfc5802.html) +- [RFC 9106, Argon2 inputs and recommended parameters](https://www.rfc-editor.org/rfc/rfc9106.html) +- [RFC 8725, JWT best current practices](https://www.rfc-editor.org/rfc/rfc8725.html) +- [RFC 6750, Bearer token syntax and transport](https://www.rfc-editor.org/rfc/rfc6750.html) +- [Go vulnerability database](https://vuln.go.dev/) diff --git a/encoding.go b/encoding.go new file mode 100644 index 0000000..5e52207 --- /dev/null +++ b/encoding.go @@ -0,0 +1,23 @@ +package auth + +import ( + "encoding/base64" + "errors" + "strings" +) + +// Bound allocation before decoding and reject non-canonical encodings, including +// CR/LF (which encoding/base64 accepts even in Strict mode). +func decodeBase64(s string, encoding *base64.Encoding, maxBytes int) ([]byte, error) { + if len(s) > encoding.EncodedLen(maxBytes) || strings.ContainsAny(s, "\r\n") { + return nil, errors.New("invalid base64 length or whitespace") + } + b, err := encoding.Strict().DecodeString(s) + if err != nil { + return nil, err + } + if len(b) > maxBytes { + return nil, errors.New("decoded value exceeds limit") + } + return b, nil +} diff --git a/error.go b/error.go index 964aec2..5cff0d2 100644 --- a/error.go +++ b/error.go @@ -1,15 +1,12 @@ package auth -import ( - "errors" - "fmt" -) +import "errors" // Base authentication errors var ( ErrInvalidCredentials = errors.New("invalid credentials") - ErrWeakPassword = errors.New("password must be at least 8 characters") - ErrPasswordTooLong = errors.New("password must be at most 1024 characters") + ErrWeakPassword = errors.New("password must be at least 8 bytes") + ErrPasswordTooLong = errors.New("password must be at most 1024 bytes") ) // JWT-specific errors @@ -22,6 +19,7 @@ var ( ErrTokenEmptyUserID = errors.New("token: empty user ID") ErrTokenNoPrivateKey = errors.New("token: private key required for signing") ErrTokenNoPublicKey = errors.New("token: public key required for verification") + ErrTokenTooLong = errors.New("token: exceeds size limit") ) // JWT secret errors @@ -35,14 +33,16 @@ var ( ErrRSAInvalidPrivateKey = errors.New("rsa: invalid private key format") ErrRSAInvalidPublicKey = errors.New("rsa: invalid public key format") ErrRSANotPublicKey = errors.New("rsa: not an RSA public key") + ErrRSAWeakKey = errors.New("rsa: key must be at least 2048 bits") ) // PHC format errors var ( - ErrPHCInvalidFormat = errors.New("phc: invalid format") - ErrPHCInvalidSalt = errors.New("phc: invalid salt encoding") - ErrPHCInvalidHash = errors.New("phc: invalid hash encoding") - ErrPHCCostTooHigh = errors.New("phc: cost parameters exceed verification limit") + ErrPHCInvalidFormat = errors.New("phc: invalid format") + ErrPHCInvalidSalt = errors.New("phc: invalid salt encoding") + ErrPHCInvalidHash = errors.New("phc: invalid hash encoding") + ErrPHCCostTooHigh = errors.New("phc: cost parameters exceed verification limit") + ErrArgonInvalidParams = errors.New("argon2: invalid parameters (require t >= 1, p >= 1, m >= 8*p)") ) // SCRAM-specific errors @@ -59,6 +59,12 @@ var ( ErrSCRAMSaltTooShort = errors.New("scram: salt must be at least 16 bytes") ErrSCRAMTooManyHandshakes = errors.New("scram: handshake capacity exceeded") ErrSCRAMParamsTooLarge = errors.New("scram: Argon2 parameters exceed limit") + ErrSCRAMParamsTooSmall = errors.New("scram: Argon2 parameters below client minimum") + ErrSCRAMInvalidUsername = errors.New("scram: invalid username") + ErrSCRAMSaltTooLong = errors.New("scram: salt exceeds 64 bytes") + ErrSCRAMStopped = errors.New("scram: server stopped") + ErrSCRAMDecoyKey = errors.New("scram: decoy key must be at least 32 bytes") + ErrSCRAMCredentialProfile = errors.New("scram: credentials must share Argon2 parameters and salt length") ) // Credential import/export errors @@ -73,16 +79,13 @@ var ( ErrCredInvalidStoredKey = errors.New("credential: invalid stored_key encoding") ErrCredMissingServerKey = errors.New("credential: missing server_key") ErrCredInvalidServerKey = errors.New("credential: invalid server_key encoding") - ErrCredInvalidType = fmt.Errorf("credential: invalid type for field") + ErrCredInvalidType = errors.New("credential: invalid type for field") ) // HTTP auth parsing errors var ( - ErrAuthInvalidBasicFormat = errors.New("auth: invalid Basic auth format") - ErrAuthInvalidBasicEncoding = errors.New("auth: invalid Basic auth base64 encoding") - ErrAuthInvalidBasicCreds = errors.New("auth: invalid Basic auth credentials format") - ErrAuthInvalidBearerFormat = errors.New("auth: invalid Bearer auth format") - ErrAuthEmptyBearerToken = errors.New("auth: empty Bearer token") + ErrAuthInvalidBearerFormat = errors.New("auth: invalid Bearer auth format") + ErrAuthEmptyBearerToken = errors.New("auth: empty Bearer token") ) // Salt generation errors diff --git a/example_test.go b/example_test.go new file mode 100644 index 0000000..32171ca --- /dev/null +++ b/example_test.go @@ -0,0 +1,44 @@ +package auth_test + +import ( + "fmt" + "log" + + "github.com/lixenwraith/auth" +) + +func ExampleNewCredential() { + credential, err := auth.NewCredential("alice", "a-long-example-password") + if err != nil { + log.Fatal(err) + } + // For this local example only. Services should persist a random decoy key + // and use NewScramServerWithDecoyKey, plus authenticated TLS for transport. + server := auth.NewScramServer() + defer server.Stop() + if err := server.AddCredential(credential); err != nil { + log.Fatal(err) + } + client := auth.NewScramClient("alice", "a-long-example-password") + first, err := client.StartAuthentication() + if err != nil { + log.Fatal(err) + } + challenge, err := server.ProcessClientFirstMessage(first.Username, first.ClientNonce) + if err != nil { + log.Fatal(err) + } + proof, err := client.ProcessServerFirstMessage(challenge) + if err != nil { + log.Fatal(err) + } + final, err := server.ProcessClientFinalMessage(proof.FullNonce, proof.ClientProof) + if err != nil { + log.Fatal(err) + } + if err := client.VerifyServerFinalMessage(final); err != nil { + log.Fatal(err) + } + fmt.Println(final.Username) + // Output: alice +} diff --git a/go.mod b/go.mod index ec5a876..ed4c4c5 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.26.0 require ( github.com/golang-jwt/jwt/v5 v5.3.1 - golang.org/x/crypto v0.54.0 + golang.org/x/crypto v0.57.0 ) -require golang.org/x/sys v0.47.0 // indirect +require golang.org/x/sys v0.48.0 // indirect diff --git a/go.sum b/go.sum index 6f26f6f..d460ffc 100644 --- a/go.sum +++ b/go.sum @@ -1,6 +1,6 @@ github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63YCY= github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= -golang.org/x/crypto v0.54.0 h1:YLIA59K4fiNzHzjnZt2tUJQjQtUWfWbeHBqKtk3eScw= -golang.org/x/crypto v0.54.0/go.mod h1:KWL8ny2AZdGR2cWmzeHrp2azQPGogOv+HeQaVEXC2dk= -golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= -golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/crypto v0.57.0 h1:3ZVCjf8Ggz7zneR/EHRVx68Ctf+2pmIMP2UFhh9cC6M= +golang.org/x/crypto v0.57.0/go.mod h1:Fdz0i5U6CoizGwLda9DttjSk6qlZo25zYNtR+ycvuZA= +golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo= +golang.org/x/sys v0.48.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og= diff --git a/hardening_test.go b/hardening_test.go new file mode 100644 index 0000000..d51825f --- /dev/null +++ b/hardening_test.go @@ -0,0 +1,313 @@ +package auth + +import ( + "bytes" + "crypto/rsa" + "crypto/x509" + "encoding/json" + "encoding/pem" + "errors" + "math" + "math/big" + "strings" + "sync" + "sync/atomic" + "testing" + "time" +) + +func TestArgonExecutionBounds(t *testing.T) { + for _, tc := range []struct { + memory, time uint32 + threads uint8 + want error + }{ + {1, 1, 1, ErrArgonInvalidParams}, + {MaxVerifyArgonMemory + 1, 1, 1, ErrPHCCostTooHigh}, + {testArgonMemory, MaxVerifyArgonTime + 1, 1, ErrPHCCostTooHigh}, + {testArgonMemory, 1, MaxVerifyArgonThreads + 1, ErrPHCCostTooHigh}, + {MaxVerifyArgonMemory, MaxVerifyArgonTime, 1, ErrPHCCostTooHigh}, + {math.MaxUint32, math.MaxUint32, math.MaxUint8, ErrPHCCostTooHigh}, + } { + _, err := HashPassword("password123", WithMemory(tc.memory), WithTime(tc.time), WithThreads(tc.threads)) + errIs(t, err, tc.want, "hash bounds") + _, err = DeriveCredential("u", "password123", make([]byte, 16), tc.time, tc.memory, tc.threads) + if tc.want == ErrArgonInvalidParams { + errIs(t, err, ErrSCRAMZeroParams, "derive invalid") + } else { + errIs(t, err, ErrSCRAMParamsTooLarge, "derive bounds") + } + } + _, err := DeriveCredential("u", "password123", make([]byte, MaxArgonSaltLen+1), 1, 8, 1) + errIs(t, err, ErrSCRAMSaltTooLong, "derive salt bound") +} + +func TestNewCredentialAndOwnership(t *testing.T) { + cred, err := NewCredential("u", "password123", cheapArgon...) + noErr(t, err, "new") + noErr(t, cred.Validate(), "validate") + s := newTestServer(t) + noErr(t, s.AddCredential(cred), "add") + // Mutation after registration cannot change the server's credential. + clear(cred.Salt) + clear(cred.StoredKey) + clear(cred.ServerKey) + cred.Username = "changed" + noErr(t, runHandshake(s, newTestScramClient("u", "password123")), "owned credential") + + salt := bytes.Repeat([]byte{1}, 16) + derived, err := DeriveCredential("u", "password123", salt, 1, testArgonMemory, 1) + noErr(t, err, "derive") + clear(salt) + if bytes.Equal(derived.Salt, salt) { + t.Fatal("derived credential aliases salt") + } + _, err = NewCredential("u", "short", cheapArgon...) + errIs(t, err, ErrWeakPassword, "new credential password policy") +} + +func TestCredentialImportNumericTypes(t *testing.T) { + cred, err := NewCredential("u", "password123", cheapArgon...) + noErr(t, err, "new") + for _, values := range [][3]any{ + {int64(1), int64(testArgonMemory), int64(1)}, + {uint64(1), uint64(testArgonMemory), uint64(1)}, + {json.Number("1"), json.Number("8192"), json.Number("1")}, + } { + m := cred.Export() + m["argon_time"], m["argon_memory"], m["argon_threads"] = values[0], values[1], values[2] + _, err := ImportCredential(m) + noErr(t, err, "import numeric types") + } + for _, bad := range []any{math.NaN(), math.Inf(1), math.Inf(-1), json.Number("1.5"), uint64(math.MaxUint64), int64(-1)} { + for _, field := range []string{"argon_time", "argon_memory", "argon_threads"} { + m := cred.Export() + m[field] = bad + _, err := ImportCredential(m) + errIs(t, err, ErrCredInvalidType, "invalid number") + } + } + for _, field := range []string{"salt", "stored_key", "server_key"} { + m := cred.Export() + m[field] = strings.Repeat("A", 10000) + _, err := ImportCredential(m) + hasErr(t, err, "oversized encoded field") + } +} + +func TestScramRegistrationRevocationAndStop(t *testing.T) { + s, user, pw, cred := setupScram(t) + errIs(t, s.AddCredential(nil), ErrInvalidCredentials, "nil credential") + bad := *cred + bad.StoredKey = nil + errIs(t, s.AddCredential(&bad), ErrCredInvalidStoredKey, "missing stored key") + bad = *cred + bad.ArgonTime++ + errIs(t, s.AddCredential(&bad), ErrSCRAMCredentialProfile, "mixed profile") + final := startHandshake(t, s, user, pw) + noErr(t, s.AddCredential(cred), "replace") + _, err := s.ProcessClientFinalMessage(final.FullNonce, final.ClientProof) + errIs(t, err, ErrSCRAMInvalidNonce, "replacement invalidates pending proof") + final = startHandshake(t, s, user, pw) + s.RemoveCredential(user) + _, err = s.ProcessClientFinalMessage(final.FullNonce, final.ClientProof) + errIs(t, err, ErrSCRAMInvalidNonce, "removal invalidates pending proof") + errIs(t, runHandshake(s, newTestScramClient(user, pw)), ErrInvalidCredentials, "removed user") + noErr(t, s.AddCredential(cred), "restore") + final = startHandshake(t, s, user, pw) + s.Stop() + eq(t, handshakeCount(s), 0, "stop discards pending work") + _, err = s.ProcessClientFirstMessage(user, "nonce") + errIs(t, err, ErrSCRAMStopped, "first after stop") + _, err = s.ProcessClientFinalMessage(final.FullNonce, final.ClientProof) + errIs(t, err, ErrSCRAMStopped, "final after stop") + errIs(t, s.AddCredential(cred), ErrSCRAMStopped, "register after stop") +} + +func TestScramStableDecoyKey(t *testing.T) { + key := bytes.Repeat([]byte{1}, 32) + a, err := NewScramServerWithDecoyKey(key) + noErr(t, err, "server a") + defer a.Stop() + b, err := NewScramServerWithDecoyKey(key) + noErr(t, err, "server b") + defer b.Stop() + clear(key) + first, err := a.ProcessClientFirstMessage("unknown", "a") + noErr(t, err, "first a") + second, err := b.ProcessClientFirstMessage("unknown", "b") + noErr(t, err, "first b") + eq(t, first.Salt, second.Salt, "stable salt across instances") + _, err = NewScramServerWithDecoyKey(make([]byte, 31)) + errIs(t, err, ErrSCRAMDecoyKey, "short key") +} + +func TestScramInputBounds(t *testing.T) { + s := newTestServer(t) + for _, user := range []string{"", "a,b", "a=b", "a\x00b", string([]byte{0xff}), strings.Repeat("a", MaxUsernameLen+1)} { + _, err := s.ProcessClientFirstMessage(user, "n") + errIs(t, err, ErrSCRAMInvalidUsername, "username") + _, err = newTestScramClient(user, "password123").StartAuthentication() + errIs(t, err, ErrSCRAMInvalidUsername, "client username") + } + for _, nonce := range []string{"", "a,b", "a\nb", "a b", strings.Repeat("x", MaxClientNonceLen+1)} { + _, err := s.ProcessClientFirstMessage("u", nonce) + errIs(t, err, ErrSCRAMInvalidNonce, "nonce") + } + eq(t, handshakeCount(s), 0, "invalid input creates no state") + c := newTestScramClient("u", "password123") + first, err := c.StartAuthentication() + noErr(t, err, "start") + challenge, err := s.ProcessClientFirstMessage(first.Username, first.ClientNonce) + noErr(t, err, "challenge") + c.Password = strings.Repeat("x", MaxPasswordLen+1) + _, err = c.ProcessServerFirstMessage(challenge) + errIs(t, err, ErrPasswordTooLong, "mutable password rechecked") +} + +func TestScramConcurrentProofConsumedOnce(t *testing.T) { + s, user, pw, _ := setupScram(t) + final := startHandshake(t, s, user, pw) + var successes atomic.Int32 + var wg sync.WaitGroup + start := make(chan struct{}) + for range 64 { + wg.Go(func() { + <-start + _, err := s.ProcessClientFinalMessage(final.FullNonce, final.ClientProof) + if err == nil { + successes.Add(1) + } else if !errors.Is(err, ErrSCRAMVerifyInProgress) && !errors.Is(err, ErrSCRAMInvalidNonce) { + t.Errorf("unexpected error: %v", err) + } + }) + } + close(start) + wg.Wait() + eq(t, successes.Load(), int32(1), "only one proof may succeed") + eq(t, handshakeCount(s), 0, "consumed proof") +} + +func TestJWTConfigurationOwnership(t *testing.T) { + secret := bytes.Clone(testSecret) + audience := []string{"vif"} + j, err := NewJWT(secret, WithIssuer("issuer"), WithAudience(audience)) + noErr(t, err, "new") + clear(secret) + audience[0] = "logwisp" + token, err := j.GenerateToken("u", nil) + noErr(t, err, "sign") + check, err := NewJWT(testSecret, WithIssuer("issuer"), WithAudience([]string{"vif"})) + noErr(t, err, "check") + _, _, err = check.ValidateToken(token) + noErr(t, err, "owned config") + _, _, err = j.ValidateToken(token) + noErr(t, err, "cached parser") + other, err := NewJWT(testSecret, WithIssuer("issuer"), WithAudience([]string{"logwisp"})) + noErr(t, err, "other service") + _, _, err = other.ValidateToken(token) + errIs(t, err, ErrTokenMissingClaim, "cross-service token") +} + +func TestJWTBoundedCanonicalEncoding(t *testing.T) { + j, err := NewJWT(testSecret) + noErr(t, err, "new") + token, err := j.GenerateToken("u", nil) + noErr(t, err, "sign") + // A 32-byte signature has two unused bits in its last base64url character. + const alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_" + last := strings.IndexByte(alphabet, token[len(token)-1]) + malleated := token[:len(token)-1] + string(alphabet[last+1]) + for _, bad := range []string{token + "\n", malleated, strings.Repeat("x", MaxTokenLen+1)} { + _, _, err = j.ValidateToken(bad) + hasErr(t, err, "malformed/oversized token") + _, _, err = ValidateHS256Token(testSecret, bad) + hasErr(t, err, "standalone malformed/oversized") + } + header := defaultHeader() + header["crit"] = []string{"custom"} + critical := signHS256(t, testSecret, header, map[string]any{"sub": "u", "exp": time.Now().Add(time.Hour).Unix()}) + _, _, err = j.ValidateToken(critical) + hasErr(t, err, "unsupported critical extension") + _, err = j.GenerateToken("u", map[string]any{"huge": strings.Repeat("x", MaxTokenLen)}) + errIs(t, err, ErrTokenTooLong, "oversized generated JWT") +} + +func TestRSAValidationAndOwnership(t *testing.T) { + for _, key := range []*rsa.PublicKey{{}, {N: big.NewInt(3), E: 65537}, {N: testRSAKey().N, E: 2}} { + _, err := NewJWTVerifier(key) + hasErr(t, err, "invalid/weak public key") + } + for _, key := range []*rsa.PrivateKey{{}, {PublicKey: testRSAKey().PublicKey}, {PublicKey: testRSAKey().PublicKey, D: big.NewInt(1), Primes: []*big.Int{nil, nil}}} { + _, err := NewJWTRSA(key) + hasErr(t, err, "invalid private key") + } + key, err := x509.ParsePKCS1PrivateKey(x509.MarshalPKCS1PrivateKey(testRSAKey())) + noErr(t, err, "copy") + signer, err := NewJWTRSA(key) + noErr(t, err, "signer") + verifier, err := NewJWTVerifier(&key.PublicKey) + noErr(t, err, "verifier") + key.N.SetInt64(3) + key.D.SetInt64(0) + key.Primes[0].SetInt64(0) + token, err := signer.GenerateToken("u", nil) + noErr(t, err, "sign with copied key") + _, _, err = verifier.ValidateToken(token) + noErr(t, err, "verify with copied key") + pkcs1 := pem.EncodeToMemory(&pem.Block{Type: "RSA PUBLIC KEY", Bytes: x509.MarshalPKCS1PublicKey(&testRSAKey().PublicKey)}) + fromPEM, err := NewJWTVerifierFromPEM(pkcs1) + noErr(t, err, "PKCS1 public PEM") + _, _, err = fromPEM.ValidateToken(token) + noErr(t, err, "PEM verifier") +} + +func TestSimpleTokenZeroValueAndBounds(t *testing.T) { + var v SimpleTokenValidator + v.AddToken("") + v.AddToken(strings.Repeat("x", MaxTokenLen+1)) + if v.ValidateToken("") || v.ValidateToken(strings.Repeat("x", MaxTokenLen+1)) { + t.Fatal("invalid credential accepted") + } + v.AddToken("a-token") + isTrue(t, v.ValidateToken("a-token"), "zero value initialized") + v.RemoveToken("a-token") + isTrue(t, !v.ValidateToken("a-token"), "revoked") +} + +func BenchmarkCredentialProvisioning(b *testing.B) { + b.Run("direct", func(b *testing.B) { + for b.Loop() { + if _, err := NewCredential("u", "password123", cheapArgon...); err != nil { + b.Fatal(err) + } + } + }) + b.Run("via_phc", func(b *testing.B) { + for b.Loop() { + hash, err := HashPassword("password123", cheapArgon...) + if err != nil { + b.Fatal(err) + } + if _, err := MigrateFromPHC("u", "password123", hash); err != nil { + b.Fatal(err) + } + } + }) +} + +func TestScramClientRejectsKDFDowngrade(t *testing.T) { + c := NewScramClient("u", "password123") + first, err := c.StartAuthentication() + noErr(t, err, "start") + msg := ServerFirstMessage{ + FullNonce: first.ClientNonce + "server", Salt: "AAAAAAAAAAAAAAAAAAAAAA==", + ArgonTime: 1, ArgonMemory: 8, ArgonThreads: 1, + } + _, err = c.ProcessServerFirstMessage(msg) + errIs(t, err, ErrSCRAMParamsTooSmall, "hostile KDF downgrade") + for _, option := range []ScramClientOption{WithMinArgonCost(0, 8), WithMinArgonCost(1, 0), WithMinArgonCost(1, MaxVerifyArgonMemory+1)} { + _, err := NewScramClient("u", "password123", option).StartAuthentication() + hasErr(t, err, "invalid client policy") + } +} diff --git a/http.go b/http.go index dd1f6af..8c319ac 100644 --- a/http.go +++ b/http.go @@ -1,47 +1,35 @@ package auth -import ( - "encoding/base64" - "strings" -) +import "strings" -// ParseBasicAuth extracts username/password from Basic auth header -func ParseBasicAuth(header string) (username, password string, err error) { - encoded, ok := strings.CutPrefix(header, "Basic ") - if !ok { - return "", "", ErrAuthInvalidBasicFormat +// ParseBearerToken parses an RFC 6750 Bearer authorization value. The scheme is +// case-insensitive; tokens must use the b64token alphabet and contain no spaces. +// Parsing does not validate the token's signature or grant authorization. +func ParseBearerToken(header string) (string, error) { + // Bound before scanning, including an allowance for separator spaces. + if len(header) > MaxTokenLen+16 { + return "", ErrTokenTooLong } - - decoded, err := base64.StdEncoding.DecodeString(encoded) - if err != nil { - return "", "", ErrAuthInvalidBasicEncoding - } - - username, password, ok = strings.Cut(string(decoded), ":") - if !ok { - return "", "", ErrAuthInvalidBasicCreds - } - - return username, password, nil -} - -// ParseBearerToken extracts token from Bearer auth header -func ParseBearerToken(header string) (token string, err error) { - token, ok := strings.CutPrefix(header, "Bearer ") - if !ok { + scheme, token, ok := strings.Cut(header, " ") + if !ok || !strings.EqualFold(scheme, "Bearer") { return "", ErrAuthInvalidBearerFormat } + token = strings.TrimLeft(token, " ") if token == "" { return "", ErrAuthEmptyBearerToken } - + if len(token) > MaxTokenLen { + return "", ErrTokenTooLong + } + padding := false + for i, ch := range []byte(token) { + if ch == '=' && i > 0 { + padding = true + continue + } + if padding || !(ch >= 'a' && ch <= 'z' || ch >= 'A' && ch <= 'Z' || ch >= '0' && ch <= '9' || strings.ContainsRune("-._~+/", rune(ch))) { + return "", ErrAuthInvalidBearerFormat + } + } return token, nil } - -// ExtractAuthType returns authentication type from header -func ExtractAuthType(header string) string { - if authType, _, ok := strings.Cut(header, " "); ok { - return authType - } - return "" // Matches original behavior if no space is found or string is empty -} diff --git a/http_test.go b/http_test.go index 5e5471c..ec68023 100644 --- a/http_test.go +++ b/http_test.go @@ -1,123 +1,57 @@ package auth import ( - "encoding/base64" - "fmt" "strings" "testing" ) -func basicHeader(payload string) string { - return "Basic " + base64.StdEncoding.EncodeToString([]byte(payload)) -} - -func TestParseBasicAuth(t *testing.T) { - cases := []struct { - name, header, username, password string - want error - }{ - {name: "standard", header: basicHeader("user:pass"), username: "user", password: "pass"}, - {name: "empty password", header: basicHeader("user:"), username: "user"}, - {name: "empty username", header: basicHeader(":pass"), password: "pass"}, - {name: "colon in password", header: basicHeader("user:pa:ss"), username: "user", password: "pa:ss"}, - {name: "unicode", header: basicHeader("üser:pässwörd"), username: "üser", password: "pässwörd"}, - {name: "separator only", header: basicHeader(":")}, - - {name: "missing scheme", header: "Invalid header", want: ErrAuthInvalidBasicFormat}, - {name: "no space", header: "Basic", want: ErrAuthInvalidBasicFormat}, - {name: "empty header", header: "", want: ErrAuthInvalidBasicFormat}, - // ☢ RFC 7235 auth-scheme is case-insensitive; matching here is not - {name: "lowercase scheme", header: "basic dXNlcjpwYXNz", want: ErrAuthInvalidBasicFormat}, - {name: "bad base64", header: "Basic not-base64!", want: ErrAuthInvalidBasicEncoding}, - {name: "no colon", header: basicHeader("no-colon"), want: ErrAuthInvalidBasicCreds}, - {name: "empty payload", header: "Basic ", want: ErrAuthInvalidBasicCreds}, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - username, password, err := ParseBasicAuth(tc.header) - if tc.want != nil { - errIs(t, err, tc.want, tc.name) - eq(t, username, "", "username on failure") - eq(t, password, "", "password on failure") - return - } - noErr(t, err, tc.name) - eq(t, username, tc.username, "username") - eq(t, password, tc.password, "password") - }) - } -} - func TestParseBearerToken(t *testing.T) { - cases := []struct { - name, header, token string - want error + for _, tc := range []struct { + header, token string + want error }{ - {name: "standard", header: "Bearer test-token-xyz", token: "test-token-xyz"}, - {name: "embedded space", header: "Bearer a b", token: "a b"}, - {name: "trailing space", header: "Bearer x ", token: "x "}, - - {name: "empty token", header: "Bearer ", want: ErrAuthEmptyBearerToken}, - {name: "missing scheme", header: "Invalid header", want: ErrAuthInvalidBearerFormat}, - {name: "lowercase scheme", header: "bearer x", want: ErrAuthInvalidBearerFormat}, - {name: "no space", header: "Bearer", want: ErrAuthInvalidBearerFormat}, - {name: "empty header", header: "", want: ErrAuthInvalidBearerFormat}, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - token, err := ParseBearerToken(tc.header) - if tc.want != nil { - errIs(t, err, tc.want, tc.name) - eq(t, token, "", "token on failure") - return - } - noErr(t, err, tc.name) - eq(t, token, tc.token, "token") - }) + {"Bearer abc.def-_/+~==", "abc.def-_/+~==", nil}, + {"bEaReR token", "token", nil}, + {"Bearer ", "", ErrAuthEmptyBearerToken}, + {"Bearer ", "", ErrAuthEmptyBearerToken}, + {"Bearer", "", ErrAuthInvalidBearerFormat}, + {"Bearer x y", "", ErrAuthInvalidBearerFormat}, + {"Bearer x ", "", ErrAuthInvalidBearerFormat}, + {"Bearer x\r\n", "", ErrAuthInvalidBearerFormat}, + {"Bearer x\ty", "", ErrAuthInvalidBearerFormat}, + {"Bearer =", "", ErrAuthInvalidBearerFormat}, + {"Bearer a=b", "", ErrAuthInvalidBearerFormat}, + {"Other x", "", ErrAuthInvalidBearerFormat}, + {"Bearer " + strings.Repeat("x", MaxTokenLen+1), "", ErrTokenTooLong}, + } { + token, err := ParseBearerToken(tc.header) + if tc.want != nil { + errIs(t, err, tc.want, tc.header) + } else { + noErr(t, err, tc.header) + } + eq(t, token, tc.token, "token") } } -func TestExtractAuthType(t *testing.T) { - cases := map[string]string{ - "Basic dXNlcjpwYXNz": "Basic", - "Bearer token": "Bearer", - "Custom somedata": "Custom", - "Basic ": "Basic", - "InvalidHeader": "", - "": "", - " Bearer token": "", // cut at the first space yields an empty scheme +func FuzzParseBearerToken(f *testing.F) { + for _, seed := range []string{"Bearer token", "bearer abc+/==", "Bearer x\n", "Bearer a=b", ""} { + f.Add(seed) } - for header, want := range cases { - eq(t, ExtractAuthType(header), want, fmt.Sprintf("header %q", header)) - } -} - -func FuzzParseBasicAuth(f *testing.F) { - f.Add(basicHeader("user:pass")) - f.Add("Basic ") - f.Add("") - f.Add("Basic !!!") - f.Fuzz(func(t *testing.T, header string) { - username, password, err := ParseBasicAuth(header) + token, err := ParseBearerToken(header) if err != nil { - if username != "" || password != "" { - t.Fatal("values returned alongside error") + if token != "" { + t.Fatal("token with error") } return } - encoded, ok := strings.CutPrefix(header, "Basic ") - if !ok { - t.Fatal("accepted a header without the Basic prefix") + if token == "" || len(token) > MaxTokenLen || strings.ContainsAny(token, " \t\r\n") { + t.Fatal("invalid token accepted") } - decoded, decErr := base64.StdEncoding.DecodeString(encoded) - if decErr != nil { - t.Fatal("accepted an undecodable payload") - } - if string(decoded) != username+":"+password { - t.Fatalf("lossy split: %q vs %q", decoded, username+":"+password) + again, err := ParseBearerToken("Bearer " + token) + if err != nil || again != token { + t.Fatal("noncanonical roundtrip") } }) } diff --git a/jwt.go b/jwt.go index dd48357..1a2d04e 100644 --- a/jwt.go +++ b/jwt.go @@ -1,11 +1,14 @@ package auth import ( + "bytes" "crypto/rsa" "crypto/x509" "encoding/pem" "errors" "fmt" + "math/big" + "strings" "time" "github.com/golang-jwt/jwt/v5" @@ -15,6 +18,8 @@ import ( const ( DefaultTokenLifetime = 24 * time.Hour DefaultLeeway = 5 * time.Minute + MaxTokenLen = 16 * 1024 + MinRSABits = 2048 ) // customClaims extends RegisteredClaims with arbitrary user data @@ -23,8 +28,10 @@ type customClaims struct { Extra map[string]any `json:"extra,omitempty"` } -// JWT manages token generation and validation +// JWT manages token generation and validation. Construct once and reuse; its +// immutable configuration and parser are safe for concurrent use. type JWT struct { + parser *jwt.Parser algorithm jwt.SigningMethod signKey any // []byte for HMAC, *rsa.PrivateKey for RSA verifyKey any // []byte for HMAC, *rsa.PublicKey for RSA @@ -34,7 +41,8 @@ type JWT struct { audience []string } -// JWTOption configures JWT behavior +// JWTOption configures JWT behavior at construction time. Do not apply options +// to an existing manager. type JWTOption func(*JWT) // WithTokenLifetime sets token expiration duration @@ -65,7 +73,7 @@ func WithIssuer(iss string) JWTOption { // WithAudience sets token audience claim func WithAudience(aud []string) JWTOption { return func(j *JWT) { - j.audience = aud + j.audience = append([]string(nil), aud...) } } @@ -75,6 +83,7 @@ func NewJWT(secret []byte, opts ...JWTOption) (*JWT, error) { return nil, ErrSecretTooShort } + secret = bytes.Clone(secret) j := &JWT{ algorithm: jwt.SigningMethodHS256, signKey: secret, @@ -83,10 +92,7 @@ func NewJWT(secret []byte, opts ...JWTOption) (*JWT, error) { leeway: DefaultLeeway, } - for _, opt := range opts { - opt(j) - } - + j.configure(opts) return j, nil } @@ -96,6 +102,10 @@ func NewJWTRSA(privateKey *rsa.PrivateKey, opts ...JWTOption) (*JWT, error) { return nil, ErrTokenNoPrivateKey } + privateKey, err := cloneRSAPrivateKey(privateKey) + if err != nil { + return nil, err + } j := &JWT{ algorithm: jwt.SigningMethodRS256, signKey: privateKey, @@ -104,10 +114,7 @@ func NewJWTRSA(privateKey *rsa.PrivateKey, opts ...JWTOption) (*JWT, error) { leeway: DefaultLeeway, } - for _, opt := range opts { - opt(j) - } - + j.configure(opts) return j, nil } @@ -127,6 +134,10 @@ func NewJWTVerifier(publicKey *rsa.PublicKey, opts ...JWTOption) (*JWT, error) { return nil, ErrTokenNoPublicKey } + if err := validateRSAPublicKey(publicKey); err != nil { + return nil, err + } + publicKey = &rsa.PublicKey{N: new(big.Int).Set(publicKey.N), E: publicKey.E} j := &JWT{ algorithm: jwt.SigningMethodRS256, signKey: nil, // Cannot sign @@ -135,10 +146,7 @@ func NewJWTVerifier(publicKey *rsa.PublicKey, opts ...JWTOption) (*JWT, error) { leeway: DefaultLeeway, } - for _, opt := range opts { - opt(j) - } - + j.configure(opts) return j, nil } @@ -177,33 +185,55 @@ func (j *JWT) GenerateToken(userID string, claims map[string]any) (string, error Extra: claims, }) - return token.SignedString(j.signKey) + signed, err := token.SignedString(j.signKey) + if len(signed) > MaxTokenLen { + return "", ErrTokenTooLong + } + return signed, err } -// ValidateToken verifies JWT and extracts claims -func (j *JWT) ValidateToken(tokenString string) (string, map[string]any, error) { - parser := jwt.NewParser( - jwt.WithLeeway(j.leeway), - jwt.WithAudience(j.audience...), - jwt.WithIssuer(j.issuer), - jwt.WithValidMethods([]string{j.algorithm.Alg()}), - jwt.WithExpirationRequired(), +func (j *JWT) configure(opts []JWTOption) { + for _, opt := range opts { + if opt != nil { + opt(j) + } + } + j.parser = jwt.NewParser( + jwt.WithLeeway(j.leeway), jwt.WithAudience(j.audience...), jwt.WithIssuer(j.issuer), + jwt.WithValidMethods([]string{j.algorithm.Alg()}), jwt.WithExpirationRequired(), + jwt.WithIssuedAt(), jwt.WithStrictDecoding(), ) +} +// ValidateToken verifies JWT and returns a nonempty subject plus application +// claims. Issuer/audience are enforced when configured; claims are nil on error. +func (j *JWT) ValidateToken(tokenString string) (string, map[string]any, error) { + return validateJWT(j.parser, j.verifyKey, tokenString) +} + +func validateJWT(parser *jwt.Parser, key any, tokenString string) (string, map[string]any, error) { + if len(tokenString) > MaxTokenLen { + return "", nil, ErrTokenTooLong + } + if strings.ContainsAny(tokenString, "\r\n") { + return "", nil, ErrTokenMalformed + } token, err := parser.ParseWithClaims(tokenString, &customClaims{}, func(token *jwt.Token) (any, error) { - // Algorithm already validated by WithValidMethods - return j.verifyKey, nil + if _, ok := token.Header["crit"]; ok { + return nil, ErrTokenMalformed + } + return key, nil }) - if err != nil { return "", nil, mapJWTError(err) } - claims, ok := token.Claims.(*customClaims) if !ok || !token.Valid { return "", nil, ErrTokenMalformed } - + if claims.Subject == "" { + return "", nil, ErrTokenEmptyUserID + } return claims.Subject, claims.Extra, nil } @@ -218,7 +248,7 @@ func mapJWTError(err error) error { return fmt.Errorf("%w: %w", ErrTokenInvalidSignature, err) case errors.Is(err, jwt.ErrTokenExpired): return fmt.Errorf("%w: %w", ErrTokenExpired, err) - case errors.Is(err, jwt.ErrTokenNotValidYet): + case errors.Is(err, jwt.ErrTokenNotValidYet), errors.Is(err, jwt.ErrTokenUsedBeforeIssued): return fmt.Errorf("%w: %w", ErrTokenNotYetValid, err) case errors.Is(err, jwt.ErrTokenInvalidAudience): return fmt.Errorf("%w: %w", ErrTokenMissingClaim, err) @@ -251,35 +281,61 @@ func GenerateHS256Token(secret []byte, userID string, claims map[string]any, lif Extra: claims, }) - return token.SignedString(secret) + signed, err := token.SignedString(secret) + if len(signed) > MaxTokenLen { + return "", ErrTokenTooLong + } + return signed, err } -// ValidateHS256Token verifies HS256 JWT without manager instance +// ValidateHS256Token is the legacy unscoped HS256 adapter. It requires exp/sub +// and validates iat/nbf with DefaultLeeway, but does not constrain issuer/audience. +// Prefer a reusable JWT with explicit issuer/audience for new services. func ValidateHS256Token(secret []byte, tokenString string) (string, map[string]any, error) { if len(secret) < 32 { return "", nil, ErrSecretTooShort } - parser := jwt.NewParser( - jwt.WithValidMethods([]string{"HS256"}), - jwt.WithLeeway(DefaultLeeway), - jwt.WithExpirationRequired(), - ) + return validateJWT(standaloneHS256Parser, secret, tokenString) +} - token, err := parser.ParseWithClaims(tokenString, &customClaims{}, func(token *jwt.Token) (any, error) { - return secret, nil - }) +var standaloneHS256Parser = jwt.NewParser( + jwt.WithValidMethods([]string{"HS256"}), jwt.WithLeeway(DefaultLeeway), + jwt.WithExpirationRequired(), jwt.WithIssuedAt(), jwt.WithStrictDecoding(), +) - if err != nil { - return "", nil, mapJWTError(err) +func validateRSAPublicKey(key *rsa.PublicKey) error { + if key.N == nil || key.N.Sign() <= 0 || key.N.Bit(0) == 0 || key.E < 3 || key.E&1 == 0 || key.E > 1<<31-1 { + return ErrRSAInvalidPublicKey } - - claims, ok := token.Claims.(*customClaims) - if !ok || !token.Valid { - return "", nil, ErrTokenMalformed + if key.N.BitLen() < MinRSABits { + return ErrRSAWeakKey } + return nil +} - return claims.Subject, claims.Extra, nil +func cloneRSAPrivateKey(key *rsa.PrivateKey) (*rsa.PrivateKey, error) { + if err := validateRSAPublicKey(&key.PublicKey); err != nil { + return nil, err + } + if key.D == nil || key.D.Sign() <= 0 || len(key.Primes) < 2 { + return nil, ErrRSAInvalidPrivateKey + } + copy := &rsa.PrivateKey{ + PublicKey: rsa.PublicKey{N: new(big.Int).Set(key.N), E: key.E}, + D: new(big.Int).Set(key.D), Primes: make([]*big.Int, len(key.Primes)), + } + for i, prime := range key.Primes { + if prime == nil || prime.Sign() <= 0 { + return nil, ErrRSAInvalidPrivateKey + } + copy.Primes[i] = new(big.Int).Set(prime) + } + if err := copy.Validate(); err != nil { + return nil, ErrRSAInvalidPrivateKey + } + copy.Precompute() + return copy, nil } // parseRSAPrivateKey parses a PEM-encoded RSA private key. @@ -309,6 +365,9 @@ func parseRSAPublicKey(pemBytes []byte) (*rsa.PublicKey, error) { if block == nil { return nil, ErrRSAInvalidPEM } + if key, err := x509.ParsePKCS1PublicKey(block.Bytes); err == nil { + return key, nil + } pubInterface, err := x509.ParsePKIXPublicKey(block.Bytes) if err != nil { return nil, ErrRSAInvalidPublicKey diff --git a/jwt_test.go b/jwt_test.go index befaf2a..1c1af69 100644 --- a/jwt_test.go +++ b/jwt_test.go @@ -215,7 +215,7 @@ func TestJWTAlgorithmEnforcement(t *testing.T) { _, _, err = rs.ValidateToken(hsToken) errIs(t, err, ErrTokenInvalidSignature, "HS256 token to RS256 manager") - // ☢ algorithm confusion: HS256 token keyed with the RSA public key + // Algorithm confusion: HS256 token keyed with the RSA public key pubBytes, err := x509.MarshalPKIXPublicKey(&key.PublicKey) noErr(t, err, "marshal public key") pubPEM := pem.EncodeToMemory(&pem.Block{Type: "PUBLIC KEY", Bytes: pubBytes}) @@ -315,8 +315,7 @@ func TestJWTExpiryAndLeeway(t *testing.T) { _, _, err = lenient.ValidateToken(notYet) noErr(t, err, "nbf inside leeway") - // exp is mandatory. ‼️ mapJWTError has no case for - // jwt.ErrTokenRequiredClaimMissing, so this surfaces as malformed. + // exp is mandatory. noExp := signHS256(t, testSecret, defaultHeader(), map[string]any{"sub": "u", "iat": now.Unix()}) _, _, err = strict.ValidateToken(noExp) errIs(t, err, ErrTokenMissingClaim, "missing exp") @@ -381,20 +380,26 @@ func TestJWTIssuerAudience(t *testing.T) { noErr(t, err, "unconstrained validation") } -func TestJWTUnenforcedClaims(t *testing.T) { - // Documented gaps: sub is not required and iat is not verified. - // Callers must reject an empty user id themselves. - manager, err := NewJWT(testSecret) +func TestJWTRequiredSubjectAndIssuedAt(t *testing.T) { + manager, err := NewJWT(testSecret, WithLeeway(0)) noErr(t, err, "NewJWT") - - token := signHS256(t, testSecret, defaultHeader(), map[string]any{ - "exp": time.Now().Add(time.Hour).Unix(), - "iat": time.Now().Add(24 * time.Hour).Unix(), - }) - userID, claims, err := manager.ValidateToken(token) - noErr(t, err, "token without subject") - eq(t, userID, "", "empty subject accepted") - eq(t, len(claims), 0, "no extra claims") + for _, tc := range []struct { + claims map[string]any + want error + }{ + {map[string]any{"exp": time.Now().Add(time.Hour).Unix()}, ErrTokenEmptyUserID}, + {map[string]any{"sub": "", "exp": time.Now().Add(time.Hour).Unix()}, ErrTokenEmptyUserID}, + {map[string]any{"sub": "u", "exp": time.Now().Add(time.Hour).Unix(), "iat": time.Now().Add(24 * time.Hour).Unix()}, ErrTokenNotYetValid}, + } { + token := signHS256(t, testSecret, defaultHeader(), tc.claims) + for _, validate := range []func(string) (string, map[string]any, error){manager.ValidateToken, func(s string) (string, map[string]any, error) { return ValidateHS256Token(testSecret, s) }} { + id, claims, err := validate(token) + errIs(t, err, tc.want, "invalid claims") + if id != "" || claims != nil { + t.Fatal("claims returned with error") + } + } + } } func TestJWTOptionGuards(t *testing.T) { diff --git a/scram.go b/scram.go index 7e57905..f3a4fed 100644 --- a/scram.go +++ b/scram.go @@ -6,11 +6,16 @@ import ( "crypto/sha256" "crypto/subtle" "encoding/base64" + "encoding/json" "fmt" "math" + "strconv" + "strings" "sync" "sync/atomic" "time" + "unicode" + "unicode/utf8" "golang.org/x/crypto/argon2" ) @@ -26,6 +31,9 @@ const ( // not compute: per client-first server cost is one HMAC. Rate limiting // upstream remains the control for connection floods. ScramMaxHandshakes = 4096 + MaxUsernameLen = 256 + MaxClientNonceLen = 256 + MaxFullNonceLen = 512 ) // Credential stores SCRAM authentication data @@ -63,80 +71,40 @@ func ImportCredential(data map[string]any) (*Credential, error) { if !ok { return nil, ErrCredMissingSalt } - salt, err := base64.StdEncoding.DecodeString(saltStr) + salt, err := decodeBase64(saltStr, base64.StdEncoding, MaxArgonSaltLen) if err != nil { return nil, ErrCredInvalidSalt } - // Handle both float64 (from JSON) and int types - getUint32 := func(key string) (uint32, error) { - val, ok := data[key] + getNumber := func(key string, bits int, missing error) (uint64, error) { + v, ok := data[key] if !ok { - switch key { - case "argon_time": - return 0, ErrCredMissingTime - case "argon_memory": - return 0, ErrCredMissingMemory - default: - return 0, fmt.Errorf("missing %s", key) - } + return 0, missing } - switch v := val.(type) { - case float64: - // out-of-range float→int conversion is undefined in Go - if v < 0 || v > math.MaxUint32 || v != math.Trunc(v) { - return 0, fmt.Errorf("%w: %s", ErrCredInvalidType, key) - } - return uint32(v), nil - case int: - if v < 0 || int64(v) > math.MaxUint32 { - return 0, fmt.Errorf("%w: %s", ErrCredInvalidType, key) - } - return uint32(v), nil - case uint32: - return v, nil - default: + n, err := credentialNumber(v, bits) + if err != nil { return 0, fmt.Errorf("%w: %s", ErrCredInvalidType, key) } + return n, nil } - - argonTime, err := getUint32("argon_time") + t, err := getNumber("argon_time", 32, ErrCredMissingTime) if err != nil { return nil, err } - - argonMemory, err := getUint32("argon_memory") + m, err := getNumber("argon_memory", 32, ErrCredMissingMemory) if err != nil { return nil, err } - - threadsVal, ok := data["argon_threads"] - if !ok { - return nil, ErrCredMissingThreads - } - var argonThreads uint8 - switch v := threadsVal.(type) { - case float64: - if v < 0 || v > math.MaxUint8 || v != math.Trunc(v) { - return nil, fmt.Errorf("%w: argon_threads", ErrCredInvalidType) - } - argonThreads = uint8(v) - case int: - if v < 0 || v > math.MaxUint8 { - return nil, fmt.Errorf("%w: argon_threads", ErrCredInvalidType) - } - argonThreads = uint8(v) - case uint8: - argonThreads = v - default: - return nil, fmt.Errorf("%w: argon_threads", ErrCredInvalidType) + p, err := getNumber("argon_threads", 8, ErrCredMissingThreads) + if err != nil { + return nil, err } storedKeyStr, ok := data["stored_key"].(string) if !ok { return nil, ErrCredMissingStoredKey } - storedKey, err := base64.StdEncoding.DecodeString(storedKeyStr) + storedKey, err := decodeBase64(storedKeyStr, base64.StdEncoding, sha256.Size) if err != nil { return nil, fmt.Errorf("%w: %v", ErrCredInvalidStoredKey, err) } @@ -145,54 +113,164 @@ func ImportCredential(data map[string]any) (*Credential, error) { if !ok { return nil, ErrCredMissingServerKey } - serverKey, err := base64.StdEncoding.DecodeString(serverKeyStr) + serverKey, err := decodeBase64(serverKeyStr, base64.StdEncoding, sha256.Size) if err != nil { return nil, fmt.Errorf("%w: %v", ErrCredInvalidServerKey, err) } - // Post-decode validation - if argonTime == 0 || argonMemory == 0 || argonThreads == 0 { - return nil, ErrSCRAMZeroParams + c := &Credential{ + Username: username, Salt: salt, + ArgonTime: uint32(t), ArgonMemory: uint32(m), ArgonThreads: uint8(p), + StoredKey: storedKey, ServerKey: serverKey, } - if err := checkArgonCost(argonMemory, argonTime, argonThreads); err != nil { - return nil, ErrSCRAMParamsTooLarge - } - if len(salt) < 16 { - return nil, ErrSCRAMSaltTooShort - } - if len(storedKey) != sha256.Size { - return nil, ErrCredInvalidStoredKey - } - if len(serverKey) != sha256.Size { - return nil, ErrCredInvalidServerKey + if err := c.Validate(); err != nil { + return nil, err } + return c, nil +} - return &Credential{ - Username: username, - Salt: salt, - ArgonTime: argonTime, - ArgonMemory: argonMemory, - ArgonThreads: argonThreads, - StoredKey: storedKey, - ServerKey: serverKey, - }, nil +func credentialNumber(value any, bits int) (uint64, error) { + var n uint64 + switch v := value.(type) { + case json.Number: + return strconv.ParseUint(string(v), 10, bits) + case float64: + if math.IsNaN(v) || v < 0 || v > math.MaxUint32 || v != math.Trunc(v) { + return 0, ErrCredInvalidType + } + n = uint64(v) + case int: + if v < 0 { + return 0, ErrCredInvalidType + } + n = uint64(v) + case int64: + if v < 0 { + return 0, ErrCredInvalidType + } + n = uint64(v) + case uint: + n = uint64(v) + case uint64: + n = v + case uint32: + n = uint64(v) + case uint8: + n = uint64(v) + default: + return 0, ErrCredInvalidType + } + if n > (uint64(1)< MaxUsernameLen || !utf8.ValidString(username) { + return ErrSCRAMInvalidUsername + } + for _, r := range username { + if r == ',' || r == '=' || unicode.IsControl(r) { + return ErrSCRAMInvalidUsername + } + } + return nil +} + +func validNonce(nonce string, maxLen int) bool { + if len(nonce) == 0 || len(nonce) > maxLen { + return false + } + for i := range nonce { + if nonce[i] < 0x21 || nonce[i] > 0x7e || nonce[i] == ',' { + return false + } + } + return true +} + +func validateSalt(salt []byte) error { + if len(salt) < DefaultArgonSaltLen { + return ErrSCRAMSaltTooShort + } + if len(salt) > MaxArgonSaltLen { + return ErrSCRAMSaltTooLong + } + return nil +} + +func checkScramCost(memory, time uint32, threads uint8) error { + if time == 0 || threads == 0 || memory < 8*uint32(threads) { + return ErrSCRAMZeroParams + } + if err := checkArgonCost(memory, time, threads); err != nil { + return ErrSCRAMParamsTooLarge + } + return nil +} + +// NewCredential provisions a credential with a fresh random salt and one KDF +// pass. The same options and password policy as HashPassword apply. +func NewCredential(username, password string, opts ...Option) (*Credential, error) { + if err := validateUsername(username); err != nil { + return nil, err + } + if len(password) < 8 { + return nil, ErrWeakPassword + } + params := &argonParams{time: DefaultArgonTime, memory: DefaultArgonMemory, threads: DefaultArgonThreads} + for _, opt := range opts { + if opt != nil { + opt(params) + } + } + salt := make([]byte, DefaultArgonSaltLen) + rand.Read(salt) + return DeriveCredential(username, password, salt, params.time, params.memory, params.threads) } // DeriveCredential creates SCRAM credential from password func DeriveCredential(username, password string, salt []byte, time, memory uint32, threads uint8) (*Credential, error) { - if len(salt) < 16 { - return nil, ErrSCRAMSaltTooShort + if err := validateUsername(username); err != nil { + return nil, err } - - if time == 0 || memory == 0 || threads == 0 { - return nil, ErrSCRAMZeroParams + if err := validateSalt(salt); err != nil { + return nil, err + } + if err := checkScramCost(memory, time, threads); err != nil { + return nil, err } - if len(password) > MaxPasswordLen { return nil, ErrPasswordTooLong } saltedPassword := argon2.IDKey([]byte(password), salt, time, memory, threads, DefaultArgonKeyLen) + defer clear(saltedPassword) return credentialFromSaltedPassword(username, saltedPassword, salt, time, memory, threads), nil } @@ -217,26 +295,35 @@ type ScramServer struct { cleanupTicker *time.Ticker cleanupStop chan struct{} stopOnce sync.Once + stopped bool } -// NewScramServer creates SCRAM server +// NewScramServer uses a random per-instance decoy key. For restarts or replicas, +// use NewScramServerWithDecoyKey with a stable, shared, secret key. func NewScramServer() *ScramServer { - decoyKey := make([]byte, 32) - rand.Read(decoyKey) - s := &ScramServer{ - credentials: make(map[string]*Credential), - handshakes: make(map[string]*HandshakeState), - decoyKey: decoyKey, - cleanupTicker: time.NewTicker(ScramCleanupInterval), - cleanupStop: make(chan struct{}), - } - - go s.cleanupLoop() - + key := make([]byte, 32) + rand.Read(key) + s, _ := NewScramServerWithDecoyKey(key) + clear(key) return s } -// decoySalt generates stable decoy salt; indistinguishable across repeated probes +// NewScramServerWithDecoyKey keeps unknown-user salts stable across restarts and +// replicas. Persist a cryptographically random key separately from credentials. +func NewScramServerWithDecoyKey(key []byte) (*ScramServer, error) { + if len(key) < 32 { + return nil, ErrSCRAMDecoyKey + } + s := &ScramServer{ + credentials: make(map[string]*Credential), handshakes: make(map[string]*HandshakeState), + decoyKey: append([]byte(nil), key...), + cleanupTicker: time.NewTicker(ScramCleanupInterval), cleanupStop: make(chan struct{}), + } + go s.cleanupLoop() + return s, nil +} + +// decoySalt generates a stable, username-specific decoy salt. func (s *ScramServer) decoySalt(username string) []byte { n := len(s.decoyTemplate.Salt) if n < 16 { @@ -244,7 +331,7 @@ func (s *ScramServer) decoySalt(username string) []byte { } out := make([]byte, 0, n) for i := 0; len(out) < n; i++ { - out = append(out, computeHMAC(s.decoyKey, fmt.Appendf(nil, "%s|%d", username, i))...) + out = append(out, computeHMAC(s.decoyKey, fmt.Appendf(nil, "auth/scram/decoy|%s|%d", username, i))...) } return out[:n] } @@ -252,6 +339,12 @@ func (s *ScramServer) decoySalt(username string) []byte { // Stop gracefully shuts down the server and cleanup goroutine func (s *ScramServer) Stop() { s.stopOnce.Do(func() { + s.mu.Lock() + s.stopped = true + clear(s.handshakes) + clear(s.credentials) + clear(s.decoyKey) + s.mu.Unlock() close(s.cleanupStop) s.cleanupTicker.Stop() }) @@ -298,9 +391,19 @@ func (s *ScramServer) evictExpiredLocked() { // reached; the cap is applied before credential lookup so the rejection path is // identical for known and unknown users. func (s *ScramServer) ProcessClientFirstMessage(username, clientNonce string) (ServerFirstMessage, error) { + if err := validateUsername(username); err != nil { + return ServerFirstMessage{}, err + } + if !validNonce(clientNonce, MaxClientNonceLen) { + return ServerFirstMessage{}, ErrSCRAMInvalidNonce + } s.mu.Lock() defer s.mu.Unlock() + if s.stopped { + return ServerFirstMessage{}, ErrSCRAMStopped + } + // opportunistic sweep, then hard cap. Applied before the credential // lookup so the rejection path is identical for known and unknown users. if len(s.handshakes) >= ScramMaxHandshakes { @@ -314,54 +417,42 @@ func (s *ScramServer) ProcessClientFirstMessage(username, clientNonce string) (S serverNonce := rand.Text() fullNonce := clientNonce + serverNonce - // Check if user exists - cred, exists := s.credentials[username] - if !exists { - t := s.decoyTemplate // mirror real parameter shape - if t.ArgonTime == 0 { - t.ArgonTime, t.ArgonMemory, t.ArgonThreads = DefaultArgonTime, DefaultArgonMemory, DefaultArgonThreads - } - // Deterministic salt + stored decoy handshake so the final - // step fails with ErrInvalidCredentials, matching the wrong-password path. - decoy := &Credential{ - Username: username, - Salt: s.decoySalt(username), - ArgonTime: t.ArgonTime, - ArgonMemory: t.ArgonMemory, - ArgonThreads: t.ArgonThreads, - StoredKey: make([]byte, sha256.Size), // never matches a real proof - ServerKey: make([]byte, sha256.Size), - } - s.handshakes[fullNonce] = &HandshakeState{ - Username: username, ClientNonce: clientNonce, ServerNonce: serverNonce, - FullNonce: fullNonce, Credential: decoy, CreatedAt: time.Now(), - } - return ServerFirstMessage{ - FullNonce: fullNonce, - Salt: base64.StdEncoding.EncodeToString(decoy.Salt), - ArgonTime: decoy.ArgonTime, - ArgonMemory: decoy.ArgonMemory, - ArgonThreads: decoy.ArgonThreads, - }, nil // No early error → same control flow as valid user + // Do the decoy work for known users as well, reducing first-message timing + // differences. Uniform response profiles and application rate limits remain + // necessary; this is not a constant-time network service. + t := s.decoyTemplate + if t.ArgonTime == 0 { + t.ArgonTime, t.ArgonMemory, t.ArgonThreads = DefaultArgonTime, DefaultArgonMemory, DefaultArgonThreads + } + cred := &Credential{ + Username: username, Salt: s.decoySalt(username), + ArgonTime: t.ArgonTime, ArgonMemory: t.ArgonMemory, ArgonThreads: t.ArgonThreads, + StoredKey: make([]byte, sha256.Size), ServerKey: make([]byte, sha256.Size), + } + if registered, exists := s.credentials[username]; exists { + cred = registered } - s.handshakes[fullNonce] = &HandshakeState{ Username: username, ClientNonce: clientNonce, ServerNonce: serverNonce, FullNonce: fullNonce, Credential: cred, CreatedAt: time.Now(), } return ServerFirstMessage{ - FullNonce: fullNonce, - Salt: base64.StdEncoding.EncodeToString(cred.Salt), - ArgonTime: cred.ArgonTime, - ArgonMemory: cred.ArgonMemory, - ArgonThreads: cred.ArgonThreads, + FullNonce: fullNonce, Salt: base64.StdEncoding.EncodeToString(cred.Salt), + ArgonTime: cred.ArgonTime, ArgonMemory: cred.ArgonMemory, ArgonThreads: cred.ArgonThreads, }, nil } // ProcessClientFinalMessage verifies client proof func (s *ScramServer) ProcessClientFinalMessage(fullNonce, clientProof string) (ServerFinalMessage, error) { - // ookup + CAS under one write lock; closes the sweep race + if !validNonce(fullNonce, MaxFullNonceLen) { + return ServerFinalMessage{}, ErrSCRAMInvalidNonce + } + // Lookup + CAS under one write lock; closes the sweep race s.mu.Lock() + if s.stopped { + s.mu.Unlock() + return ServerFinalMessage{}, ErrSCRAMStopped + } state, exists := s.handshakes[fullNonce] if !exists { s.mu.Unlock() @@ -374,8 +465,7 @@ func (s *ScramServer) ProcessClientFinalMessage(fullNonce, clientProof string) ( } defer func() { - state.verifying.Store(0) - // Safe to delete after verification completes + // Keep the claim set until deletion: resetting it first permits replay. s.mu.Lock() delete(s.handshakes, fullNonce) s.mu.Unlock() @@ -387,7 +477,10 @@ func (s *ScramServer) ProcessClientFinalMessage(fullNonce, clientProof string) ( } // Decode client proof - clientProofBytes, err := base64.StdEncoding.DecodeString(clientProof) + if len(clientProof) > base64.StdEncoding.EncodedLen(sha256.Size) { + return ServerFinalMessage{}, ErrSCRAMInvalidProofLen + } + clientProofBytes, err := decodeBase64(clientProof, base64.StdEncoding, sha256.Size+1) if err != nil { return ServerFinalMessage{}, ErrSCRAMInvalidProof } @@ -412,6 +505,7 @@ func (s *ScramServer) ProcessClientFinalMessage(fullNonce, clientProof string) ( return ServerFinalMessage{}, ErrSCRAMInvalidProofLen } clientKey := make([]byte, len(clientProofBytes)) + defer clear(clientKey) subtle.XORBytes(clientKey, clientProofBytes, clientSignature) // Verify by computing StoredKey @@ -420,6 +514,18 @@ func (s *ScramServer) ProcessClientFinalMessage(fullNonce, clientProof string) ( return ServerFinalMessage{}, ErrInvalidCredentials } + // Linearize successful authentication against credential replacement/removal. + s.mu.RLock() + current := s.credentials[state.Username] + stopped := s.stopped + s.mu.RUnlock() + if stopped { + return ServerFinalMessage{}, ErrSCRAMStopped + } + if current != state.Credential { + return ServerFinalMessage{}, ErrInvalidCredentials + } + // Generate server signature for mutual auth serverSignature := computeHMAC(state.Credential.ServerKey, []byte(authMessage)) @@ -429,46 +535,123 @@ func (s *ScramServer) ProcessClientFinalMessage(fullNonce, clientProof string) ( }, nil } -// AddCredential registers user credential -func (s *ScramServer) AddCredential(cred *Credential) { +// AddCredential validates and copies a credential. Replacing one invalidates +// that user's pending handshakes. The caller may reuse its slices after return. +func (s *ScramServer) AddCredential(cred *Credential) error { + if err := cred.Validate(); err != nil { + return err + } + copy := *cred + copy.Salt = append([]byte(nil), cred.Salt...) + copy.StoredKey = append([]byte(nil), cred.StoredKey...) + copy.ServerKey = append([]byte(nil), cred.ServerKey...) s.mu.Lock() defer s.mu.Unlock() - s.credentials[cred.Username] = cred - s.decoyTemplate = Credential{ - Salt: make([]byte, len(cred.Salt)), - ArgonTime: cred.ArgonTime, - ArgonMemory: cred.ArgonMemory, - ArgonThreads: cred.ArgonThreads, + if s.stopped { + return ErrSCRAMStopped + } + t := s.decoyTemplate + if t.ArgonTime != 0 && (t.ArgonTime != copy.ArgonTime || t.ArgonMemory != copy.ArgonMemory || t.ArgonThreads != copy.ArgonThreads || len(t.Salt) != len(copy.Salt)) { + return ErrSCRAMCredentialProfile + } + s.invalidateHandshakesLocked(copy.Username) + s.credentials[copy.Username] = © + if t.ArgonTime == 0 { + s.decoyTemplate = Credential{ + Salt: make([]byte, len(copy.Salt)), ArgonTime: copy.ArgonTime, + ArgonMemory: copy.ArgonMemory, ArgonThreads: copy.ArgonThreads, + } + } + return nil +} + +// RemoveCredential revokes future authentication and all pending handshakes. +// Already issued application sessions/tokens must be revoked by the application. +func (s *ScramServer) RemoveCredential(username string) { + s.mu.Lock() + defer s.mu.Unlock() + delete(s.credentials, username) + s.invalidateHandshakesLocked(username) +} + +func (s *ScramServer) invalidateHandshakesLocked(username string) { + for nonce, state := range s.handshakes { + if state.Username == username { + delete(s.handshakes, nonce) + } } } -// ScramClient handles client-side SCRAM authentication +type scramClientState uint8 + +const ( + scramClientIdle scramClientState = iota + scramClientAwaitChallenge + scramClientAwaitFinal +) + +// ScramClient handles one authentication exchange at a time. It is not safe for +// concurrent use; use a separate client per connection. type ScramClient struct { - Username string - Password string - clientNonce string - serverFirst *ServerFirstMessage - authMessage string - serverKey []byte - startTime time.Time // Track handshake start + Username string + Password string + clientNonce string + state scramClientState + handshakeUsername string + authMessage string + serverKey []byte + startTime time.Time // Track handshake start + minArgonTime uint32 + minArgonMemory uint32 } -// NewScramClient creates SCRAM client -func NewScramClient(username, password string) *ScramClient { - return &ScramClient{ - Username: username, - Password: password, +// ScramClientOption configures client policy at construction time. +type ScramClientOption func(*ScramClient) + +// WithMinArgonCost sets the minimum accepted iterations and memory (KiB). +// Configure from trusted deployment policy, never from a server challenge. +// The default is DefaultArgonTime and DefaultArgonMemory. StartAuthentication +// rejects invalid configuration; reducing these values weakens password safety. +func WithMinArgonCost(time, memory uint32) ScramClientOption { + return func(c *ScramClient) { + c.minArgonTime, c.minArgonMemory = time, memory } } +// NewScramClient creates a client with a minimum KDF cost matching the package's +// production defaults. Override only for a trusted, explicitly chosen profile. +func NewScramClient(username, password string, opts ...ScramClientOption) *ScramClient { + c := &ScramClient{ + Username: username, + Password: password, + minArgonTime: DefaultArgonTime, + minArgonMemory: DefaultArgonMemory, + } + for _, opt := range opts { + if opt != nil { + opt(c) + } + } + return c +} + // StartAuthentication generates initial client message func (c *ScramClient) StartAuthentication() (ClientFirstRequest, error) { + c.Reset() + if err := checkScramCost(c.minArgonMemory, c.minArgonTime, 1); err != nil { + return ClientFirstRequest{}, err + } + if err := validateUsername(c.Username); err != nil { + return ClientFirstRequest{}, err + } // Reject oversized password before the handshake commits to a KDF pass if len(c.Password) > MaxPasswordLen { return ClientFirstRequest{}, ErrPasswordTooLong } c.startTime = time.Now() + c.state = scramClientAwaitChallenge + c.handshakeUsername = c.Username // Generate client nonce c.clientNonce = rand.Text() @@ -480,39 +663,50 @@ func (c *ScramClient) StartAuthentication() (ClientFirstRequest, error) { } // ProcessServerFirstMessage handles server challenge -func (c *ScramClient) ProcessServerFirstMessage(msg ServerFirstMessage) (ClientFinalRequest, error) { - // Check timeout - if !c.startTime.IsZero() && time.Since(c.startTime) > ScramHandshakeTimeout { +func (c *ScramClient) ProcessServerFirstMessage(msg ServerFirstMessage) (result ClientFinalRequest, err error) { + defer func() { + if err != nil { + c.Reset() + } + }() + if c.state != scramClientAwaitChallenge { + return ClientFinalRequest{}, ErrSCRAMInvalidState + } + if time.Since(c.startTime) > ScramHandshakeTimeout { return ClientFinalRequest{}, ErrSCRAMTimeout } - - c.serverFirst = &msg - - // Decode salt - salt, err := base64.StdEncoding.DecodeString(msg.Salt) + if !validNonce(msg.FullNonce, MaxFullNonceLen) || !strings.HasPrefix(msg.FullNonce, c.clientNonce) || len(msg.FullNonce) <= len(c.clientNonce) { + return ClientFinalRequest{}, ErrSCRAMInvalidNonce + } + // Password is exported for compatibility; check it again immediately before KDF. + if len(c.Password) > MaxPasswordLen { + return ClientFinalRequest{}, ErrPasswordTooLong + } + salt, err := decodeBase64(msg.Salt, base64.StdEncoding, MaxArgonSaltLen) if err != nil { return ClientFinalRequest{}, ErrSCRAMInvalidSalt } - - // Validate parameters - if msg.ArgonTime == 0 || msg.ArgonMemory == 0 || msg.ArgonThreads == 0 { - return ClientFinalRequest{}, ErrSCRAMZeroParams + if err := validateSalt(salt); err != nil { + return ClientFinalRequest{}, err } - // The peer chooses these values. Unbounded, they are a remote OOM - // against every client that talks to a hostile or compromised server. - if err := checkArgonCost(msg.ArgonMemory, msg.ArgonTime, msg.ArgonThreads); err != nil { - return ClientFinalRequest{}, ErrSCRAMParamsTooLarge + if err := checkScramCost(msg.ArgonMemory, msg.ArgonTime, msg.ArgonThreads); err != nil { + return ClientFinalRequest{}, err + } + if msg.ArgonTime < c.minArgonTime || msg.ArgonMemory < c.minArgonMemory { + return ClientFinalRequest{}, ErrSCRAMParamsTooSmall } // Derive keys using Argon2id saltedPassword := argon2.IDKey([]byte(c.Password), salt, msg.ArgonTime, msg.ArgonMemory, msg.ArgonThreads, 32) + defer clear(saltedPassword) clientKey := computeHMAC(saltedPassword, []byte("Client Key")) + defer clear(clientKey) serverKey := computeHMAC(saltedPassword, []byte("Server Key")) storedKey := sha256.Sum256(clientKey) // Build auth message - clientFirstBare := fmt.Sprintf("u=%s,n=%s", c.Username, c.clientNonce) + clientFirstBare := fmt.Sprintf("u=%s,n=%s", c.handshakeUsername, c.clientNonce) clientFinalBare := fmt.Sprintf("r=%s", msg.FullNonce) c.authMessage = clientFirstBare + "," + msg.Marshal() + "," + clientFinalBare @@ -523,6 +717,10 @@ func (c *ScramClient) ProcessServerFirstMessage(msg ServerFirstMessage) (ClientF // Store server key for verification c.serverKey = serverKey + if time.Since(c.startTime) > ScramHandshakeTimeout { + return ClientFinalRequest{}, ErrSCRAMTimeout + } + c.state = scramClientAwaitFinal return ClientFinalRequest{ FullNonce: msg.FullNonce, @@ -532,20 +730,22 @@ func (c *ScramClient) ProcessServerFirstMessage(msg ServerFirstMessage) (ClientF // VerifyServerFinalMessage validates server signature func (c *ScramClient) VerifyServerFinalMessage(msg ServerFinalMessage) error { - // Check timeout - if !c.startTime.IsZero() && time.Since(c.startTime) > ScramHandshakeTimeout { + defer c.Reset() // consume state on success AND failure + if c.state != scramClientAwaitFinal { + return ErrSCRAMInvalidState + } + if time.Since(c.startTime) > ScramHandshakeTimeout { return ErrSCRAMTimeout } - - if c.authMessage == "" || c.serverKey == nil { - return ErrSCRAMInvalidState + if msg.Username != "" && msg.Username != c.handshakeUsername { + return ErrSCRAMServerAuthFailed } // Compute expected server signature expectedSig := computeHMAC(c.serverKey, []byte(c.authMessage)) // Decode received signature - receivedSig, err := base64.StdEncoding.DecodeString(msg.ServerSignature) + receivedSig, err := decodeBase64(msg.ServerSignature, base64.StdEncoding, sha256.Size) if err != nil { return ErrSCRAMServerAuthFailed } @@ -561,8 +761,10 @@ func (c *ScramClient) VerifyServerFinalMessage(msg ServerFinalMessage) error { // Reset clears client state for retry func (c *ScramClient) Reset() { c.clientNonce = "" - c.serverFirst = nil + c.state = scramClientIdle + c.handshakeUsername = "" c.authMessage = "" + clear(c.serverKey) c.serverKey = nil c.startTime = time.Time{} } diff --git a/scram_fuzz_test.go b/scram_fuzz_test.go new file mode 100644 index 0000000..3ea39a2 --- /dev/null +++ b/scram_fuzz_test.go @@ -0,0 +1,62 @@ +package auth + +import ( + "encoding/base64" + "testing" +) + +func FuzzScramServerMessages(f *testing.F) { + s := NewScramServer() + f.Cleanup(s.Stop) + f.Add("unknown", "nonce", base64.StdEncoding.EncodeToString(make([]byte, 32))) + f.Add("", "", "") + f.Add("a,n=b", "nonce", "!!!") + f.Fuzz(func(t *testing.T, username, nonce, proof string) { + first, err := s.ProcessClientFirstMessage(username, nonce) + if err != nil { + return + } + // No credential is registered, so even syntactically valid proofs fail. + _, err = s.ProcessClientFinalMessage(first.FullNonce, proof) + if err == nil { + t.Fatal("unknown account authenticated") + } + _, err = s.ProcessClientFinalMessage(first.FullNonce, proof) + errIs(t, err, ErrSCRAMInvalidNonce, "failed proof consumed") + if handshakeCount(s) != 0 { + t.Fatal("handshake leaked") + } + }) +} + +func FuzzScramClientMessages(f *testing.F) { + f.Add("server", base64.StdEncoding.EncodeToString(make([]byte, 16)), uint32(1), uint32(8), uint8(1), "") + f.Add("", "!!!", uint32(0), uint32(0), uint8(0), "!!!") + f.Add("server", "", uint32(16), uint32(MaxVerifyArgonMemory), uint8(16), "") + f.Fuzz(func(t *testing.T, suffix, salt string, iterations, memory uint32, threads uint8, signature string) { + // Invalid costs exercise production rejection. Valid costs are restricted + // to tiny test profiles so fuzzing cannot multiply expensive KDF work. + if checkScramCost(memory, iterations, threads) == nil && (memory > 64 || iterations > 2 || threads > 2) { + return + } + c := NewScramClient("u", "password123", WithMinArgonCost(1, 8)) + first, err := c.StartAuthentication() + noErr(t, err, "start") + msg := ServerFirstMessage{FullNonce: first.ClientNonce + suffix, Salt: salt, ArgonTime: iterations, ArgonMemory: memory, ArgonThreads: threads} + proof, err := c.ProcessServerFirstMessage(msg) + if err != nil { + if proof.FullNonce != "" || proof.ClientProof != "" || c.serverKey != nil { + t.Fatal("state/values survived error") + } + return + } + if proof.FullNonce != msg.FullNonce { + t.Fatal("nonce not bound") + } + _ = c.VerifyServerFinalMessage(ServerFinalMessage{ServerSignature: signature}) + errIs(t, c.VerifyServerFinalMessage(ServerFinalMessage{ServerSignature: signature}), ErrSCRAMInvalidState, "final consumed") + if c.serverKey != nil { + t.Fatal("server key survived final") + } + }) +} diff --git a/scram_test.go b/scram_test.go index 01a5721..e95fc2d 100644 --- a/scram_test.go +++ b/scram_test.go @@ -15,6 +15,10 @@ import ( "golang.org/x/crypto/argon2" ) +func newTestScramClient(username, password string) *ScramClient { + return NewScramClient(username, password, WithMinArgonCost(testArgonTime, testArgonMemory)) +} + func newTestServer(t *testing.T) *ScramServer { t.Helper() s := NewScramServer() @@ -36,7 +40,7 @@ func setupScram(t *testing.T) (*ScramServer, string, string, *Credential) { const username, password = "testuser", "SecurePassword123" cred := testCredential(t, username, password) s := newTestServer(t) - s.AddCredential(cred) + noErr(t, s.AddCredential(cred), "AddCredential") return s, username, password, cred } @@ -49,7 +53,7 @@ func handshakeCount(s *ScramServer) int { // startHandshake drives a fresh client to the point where a proof is pending. func startHandshake(t *testing.T, s *ScramServer, username, password string) ClientFinalRequest { t.Helper() - c := NewScramClient(username, password) + c := newTestScramClient(username, password) first, err := c.StartAuthentication() noErr(t, err, "StartAuthentication") serverFirst, err := s.ProcessClientFirstMessage(first.Username, first.ClientNonce) @@ -81,19 +85,19 @@ func runHandshake(s *ScramServer, c *ScramClient) error { func TestScramRoundtrip(t *testing.T) { s, user, pw, _ := setupScram(t) - noErr(t, runHandshake(s, NewScramClient(user, pw)), "handshake") + noErr(t, runHandshake(s, newTestScramClient(user, pw)), "handshake") eq(t, handshakeCount(s), 0, "handshake retained after success") } func TestScramWrongPassword(t *testing.T) { s, user, _, _ := setupScram(t) - errIs(t, runHandshake(s, NewScramClient(user, "WrongPassword!!!")), ErrInvalidCredentials, "wrong password") + errIs(t, runHandshake(s, newTestScramClient(user, "WrongPassword!!!")), ErrInvalidCredentials, "wrong password") eq(t, handshakeCount(s), 0, "handshake retained after failure") } func TestScramUnknownUser(t *testing.T) { s, _, _, cred := setupScram(t) - c := NewScramClient("unknown_user", "any_password") + c := newTestScramClient("unknown_user", "any_password") first, err := c.StartAuthentication() noErr(t, err, "StartAuthentication") @@ -151,7 +155,7 @@ func TestScramDecoySaltMultiBlock(t *testing.T) { s := newTestServer(t) cred := testCredential(t, "u", "SecurePassword123") cred.Salt = make([]byte, 48) // exceeds one HMAC-SHA256 block - s.AddCredential(cred) + noErr(t, s.AddCredential(cred), "AddCredential") msg, err := s.ProcessClientFirstMessage("ghost", "n") noErr(t, err, "first message") @@ -249,7 +253,7 @@ func TestScramTimeouts(t *testing.T) { errIs(t, err, ErrSCRAMInvalidNonce, "expired handshake consumed") // client-side clock - c := NewScramClient(user, pw) + c := newTestScramClient(user, pw) _, err = c.StartAuthentication() noErr(t, err, "StartAuthentication") c.startTime = time.Now().Add(-2 * ScramHandshakeTimeout) @@ -262,6 +266,8 @@ func TestScramTimeouts(t *testing.T) { }) errIs(t, err, ErrSCRAMTimeout, "client timeout on server-first") + c.state = scramClientAwaitFinal + c.startTime = time.Now().Add(-2 * ScramHandshakeTimeout) c.authMessage = "seeded" c.serverKey = make([]byte, sha256.Size) errIs(t, c.VerifyServerFinalMessage(ServerFinalMessage{}), ErrSCRAMTimeout, "client timeout on server-final") @@ -349,67 +355,82 @@ func TestScramStopIdempotent(t *testing.T) { func TestScramClientState(t *testing.T) { s, user, pw, _ := setupScram(t) - c := NewScramClient(user, pw) - - errIs(t, c.VerifyServerFinalMessage(ServerFinalMessage{}), ErrSCRAMInvalidState, "unstarted client") - - first, err := c.StartAuthentication() - noErr(t, err, "StartAuthentication") - serverFirst, err := s.ProcessClientFirstMessage(first.Username, first.ClientNonce) - noErr(t, err, "ProcessClientFirstMessage") - final, err := c.ProcessServerFirstMessage(serverFirst) - noErr(t, err, "ProcessServerFirstMessage") - serverFinal, err := s.ProcessClientFinalMessage(final.FullNonce, final.ClientProof) - noErr(t, err, "ProcessClientFinalMessage") - - tampered := serverFinal - tampered.ServerSignature = base64.StdEncoding.EncodeToString(make([]byte, sha256.Size)) - errIs(t, c.VerifyServerFinalMessage(tampered), ErrSCRAMServerAuthFailed, "forged signature") - tampered.ServerSignature = "!!!" - errIs(t, c.VerifyServerFinalMessage(tampered), ErrSCRAMServerAuthFailed, "malformed signature") - noErr(t, c.VerifyServerFinalMessage(serverFinal), "valid signature") - - c.Reset() - errIs(t, c.VerifyServerFinalMessage(serverFinal), ErrSCRAMInvalidState, "after reset") - next, err := c.StartAuthentication() - noErr(t, err, "restart") - if next.ClientNonce == first.ClientNonce { - t.Fatal("client nonce reused after Reset") + c := newTestScramClient(user, pw) + _, err := c.ProcessServerFirstMessage(ServerFirstMessage{}) + errIs(t, err, ErrSCRAMInvalidState, "challenge before start") + errIs(t, c.VerifyServerFinalMessage(ServerFinalMessage{}), ErrSCRAMInvalidState, "final before start") + for _, mode := range []string{"valid", "forged", "malformed", "wrong identity", "restart"} { + t.Run(mode, func(t *testing.T) { + first, err := c.StartAuthentication() + noErr(t, err, "start") + challenge, err := s.ProcessClientFirstMessage(first.Username, first.ClientNonce) + noErr(t, err, "challenge") + proof, err := c.ProcessServerFirstMessage(challenge) + noErr(t, err, "proof") + final, err := s.ProcessClientFinalMessage(proof.FullNonce, proof.ClientProof) + noErr(t, err, "final") + switch mode { + case "forged": + final.ServerSignature = base64.StdEncoding.EncodeToString(make([]byte, sha256.Size)) + case "malformed": + final.ServerSignature = "!!!" + case "wrong identity": + final.Username = "other" + case "restart": + _, err := c.StartAuthentication() + noErr(t, err, "restart") + } + err = c.VerifyServerFinalMessage(final) + if mode == "valid" { + noErr(t, err, "valid signature") + } else if mode == "restart" { + errIs(t, err, ErrSCRAMInvalidState, "old final after restart") + } else { + errIs(t, err, ErrSCRAMServerAuthFailed, "invalid signature") + } + errIs(t, c.VerifyServerFinalMessage(final), ErrSCRAMInvalidState, "consumed state") + if c.serverKey != nil { + t.Fatal("server key retained") + } + }) } } func TestScramClientRejectsBadServerFirst(t *testing.T) { - c := NewScramClient("u", "SecurePassword123") - _, err := c.StartAuthentication() - noErr(t, err, "StartAuthentication") - - _, err = c.ProcessServerFirstMessage(ServerFirstMessage{ - FullNonce: "n", Salt: "!!!", - ArgonTime: testArgonTime, ArgonMemory: testArgonMemory, ArgonThreads: testArgonThreads, - }) - errIs(t, err, ErrSCRAMInvalidSalt, "salt encoding") - - // ☢ no upper bound is applied to server-supplied cost parameters - good := base64.StdEncoding.EncodeToString(make([]byte, 16)) - for _, msg := range []ServerFirstMessage{ - {FullNonce: "n", Salt: good, ArgonTime: 0, ArgonMemory: testArgonMemory, ArgonThreads: 1}, - {FullNonce: "n", Salt: good, ArgonTime: 1, ArgonMemory: 0, ArgonThreads: 1}, - {FullNonce: "n", Salt: good, ArgonTime: 1, ArgonMemory: testArgonMemory, ArgonThreads: 0}, + for _, tc := range []struct { + name string + mutate func(*ServerFirstMessage) + want error + }{ + {"nonce prefix", func(m *ServerFirstMessage) { m.FullNonce = "unrelated" }, ErrSCRAMInvalidNonce}, + {"nonce no suffix", func(m *ServerFirstMessage) { m.FullNonce = strings.TrimSuffix(m.FullNonce, "server") }, ErrSCRAMInvalidNonce}, + {"nonce delimiter", func(m *ServerFirstMessage) { m.FullNonce += ",r=x" }, ErrSCRAMInvalidNonce}, + {"nonce oversized", func(m *ServerFirstMessage) { m.FullNonce += strings.Repeat("x", MaxFullNonceLen) }, ErrSCRAMInvalidNonce}, + {"salt encoding", func(m *ServerFirstMessage) { m.Salt = "!!!" }, ErrSCRAMInvalidSalt}, + {"salt short", func(m *ServerFirstMessage) { m.Salt = base64.StdEncoding.EncodeToString(make([]byte, 15)) }, ErrSCRAMSaltTooShort}, + {"salt oversized", func(m *ServerFirstMessage) { m.Salt = strings.Repeat("A", 10000) }, ErrSCRAMInvalidSalt}, + {"zero time", func(m *ServerFirstMessage) { m.ArgonTime = 0 }, ErrSCRAMZeroParams}, + {"zero threads", func(m *ServerFirstMessage) { m.ArgonThreads = 0 }, ErrSCRAMZeroParams}, + {"low memory", func(m *ServerFirstMessage) { m.ArgonMemory = 1 }, ErrSCRAMZeroParams}, + {"high memory", func(m *ServerFirstMessage) { m.ArgonMemory = MaxVerifyArgonMemory + 1 }, ErrSCRAMParamsTooLarge}, + {"high work", func(m *ServerFirstMessage) { m.ArgonMemory = MaxVerifyArgonMemory; m.ArgonTime = MaxVerifyArgonTime }, ErrSCRAMParamsTooLarge}, } { - _, err = c.ProcessServerFirstMessage(msg) - errIs(t, err, ErrSCRAMZeroParams, "zero parameter") + t.Run(tc.name, func(t *testing.T) { + c := newTestScramClient("u", "SecurePassword123") + first, err := c.StartAuthentication() + noErr(t, err, "start") + msg := ServerFirstMessage{FullNonce: first.ClientNonce + "server", Salt: base64.StdEncoding.EncodeToString(make([]byte, 16)), ArgonTime: testArgonTime, ArgonMemory: testArgonMemory, ArgonThreads: testArgonThreads} + tc.mutate(&msg) + _, err = c.ProcessServerFirstMessage(msg) + errIs(t, err, tc.want, tc.name) + _, err = c.ProcessServerFirstMessage(msg) + errIs(t, err, ErrSCRAMInvalidState, "failure consumed state") + }) } - - // A hostile server cannot dictate an unbounded KDF - _, err = c.ProcessServerFirstMessage(ServerFirstMessage{ - FullNonce: "n", Salt: good, - ArgonTime: 1, ArgonMemory: MaxVerifyArgonMemory + 1, ArgonThreads: 1, - }) - errIs(t, err, ErrSCRAMParamsTooLarge, "memory over ceiling") } func TestScramClientOversizedPassword(t *testing.T) { - c := NewScramClient("u", strings.Repeat("a", MaxPasswordLen+1)) + c := newTestScramClient("u", strings.Repeat("a", MaxPasswordLen+1)) _, err := c.StartAuthentication() errIs(t, err, ErrPasswordTooLong, "oversized password rejected before the KDF") } @@ -432,8 +453,8 @@ func TestScramMigratedNonStandardDigest(t *testing.T) { eq(t, len(cred.StoredKey), sha256.Size, "stored key length") s := newTestServer(t) - s.AddCredential(cred) - noErr(t, runHandshake(s, NewScramClient(user, pw)), "handshake with migrated credential") + noErr(t, s.AddCredential(cred), "AddCredential") + noErr(t, runHandshake(s, newTestScramClient(user, pw)), "handshake with migrated credential") } func TestDeriveCredential(t *testing.T) { @@ -515,8 +536,8 @@ func TestCredentialExportImportRoundTrip(t *testing.T) { // an imported credential must still authenticate s := newTestServer(t) - s.AddCredential(imported) - noErr(t, runHandshake(s, NewScramClient("roundtrip", "SecurePassword123")), "handshake after import") + noErr(t, s.AddCredential(imported), "AddCredential") + noErr(t, runHandshake(s, newTestScramClient("roundtrip", "SecurePassword123")), "handshake after import") } func TestImportCredentialErrors(t *testing.T) { @@ -590,7 +611,7 @@ func TestScramConcurrentSameUser(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - errs <- runHandshake(s, NewScramClient(user, pw)) + errs <- runHandshake(s, newTestScramClient(user, pw)) }() } wg.Wait() @@ -616,7 +637,7 @@ func TestScramConcurrentMixedTraffic(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - s.AddCredential(cred) + noErr(t, s.AddCredential(cred), "AddCredential") }() } for i := range 16 { @@ -677,10 +698,12 @@ func BenchmarkScramHandshake(b *testing.B) { } s := NewScramServer() defer s.Stop() - s.AddCredential(cred) + if err := s.AddCredential(cred); err != nil { + b.Fatal(err) + } for b.Loop() { - c := NewScramClient(user, pw) + c := newTestScramClient(user, pw) first, err := c.StartAuthentication() if err != nil { b.Fatal(err) diff --git a/token.go b/token.go index 544e926..0dd0c1d 100644 --- a/token.go +++ b/token.go @@ -5,7 +5,9 @@ import ( "sync" ) -// SimpleTokenValidator implements in-memory token validation +// SimpleTokenValidator is a concurrent in-memory allowlist of opaque tokens. +// Its zero value is ready to use. Tokens have no automatic expiration; use JWT +// for signed expiry or RemoveToken for explicit revocation. type SimpleTokenValidator struct { tokens map[[32]byte]struct{} // keyed by SHA-256(token) mu sync.RWMutex @@ -20,6 +22,9 @@ func NewSimpleTokenValidator() *SimpleTokenValidator { // ValidateToken checks if token is valid func (v *SimpleTokenValidator) ValidateToken(token string) bool { + if token == "" || len(token) > MaxTokenLen { + return false + } h := sha256.Sum256([]byte(token)) v.mu.RLock() defer v.mu.RUnlock() @@ -27,16 +32,26 @@ func (v *SimpleTokenValidator) ValidateToken(token string) bool { return ok } -// AddToken adds token to validator +// AddToken adds a token to the allowlist. Empty and oversized tokens are ignored. +// Provision high-entropy tokens, e.g. with crypto/rand.Text; do not use passwords. func (v *SimpleTokenValidator) AddToken(token string) { + if token == "" || len(token) > MaxTokenLen { + return + } h := sha256.Sum256([]byte(token)) v.mu.Lock() defer v.mu.Unlock() + if v.tokens == nil { + v.tokens = make(map[[32]byte]struct{}) + } v.tokens[h] = struct{}{} } // RemoveToken removes token from validator func (v *SimpleTokenValidator) RemoveToken(token string) { + if token == "" || len(token) > MaxTokenLen { + return + } h := sha256.Sum256([]byte(token)) v.mu.Lock() defer v.mu.Unlock() diff --git a/token_test.go b/token_test.go index 63696f1..d04ca66 100644 --- a/token_test.go +++ b/token_test.go @@ -38,9 +38,9 @@ func TestSimpleTokenValidator(t *testing.T) { v.AddToken(second) eq(t, len(v.tokens), 1, "entry count after duplicate adds") - // the empty token is storable and matches only itself + // an empty credential must never grant access v.AddToken("") - isTrue(t, v.ValidateToken(""), "empty token accepted once added") + isTrue(t, !v.ValidateToken(""), "empty token rejected") v.RemoveToken("") isTrue(t, !v.ValidateToken(""), "empty token removed") } @@ -48,7 +48,7 @@ func TestSimpleTokenValidator(t *testing.T) { func TestSimpleTokenValidatorKeying(t *testing.T) { v := NewSimpleTokenValidator() tokens := []string{ - "", "a", "a\x00b", "a\x00c", "🔑", strings.Repeat("a", 1<<16), + "a", "a\x00b", "a\x00c", "🔑", strings.Repeat("a", MaxTokenLen), } for _, tok := range tokens { v.AddToken(tok)