From 74434a0c75759824f5533acd5fe507597a6497d3c67d931464a468966f16aaba Mon Sep 17 00:00:00 2001 From: Lixen Wraith Date: Sat, 18 Jul 2026 08:48:48 -0400 Subject: [PATCH] v0.3.0 security and edge case improvement all around --- README.md | 14 +++- argon2.go | 128 ++++++++++++++++++----------- argon2_test.go | 21 ++++- doc.go | 4 +- error.go | 46 +++++------ go.mod | 8 +- go.sum | 14 ++++ http.go | 4 +- http_test.go | 4 +- jwt.go | 23 ++++-- jwt_test.go | 4 +- scram.go | 215 +++++++++++++++++++++++++++++++------------------ scram_test.go | 33 +++++--- token.go | 28 +++---- token_test.go | 4 +- 15 files changed, 350 insertions(+), 200 deletions(-) diff --git a/README.md b/README.md index 56e5174..56c6e68 100644 --- a/README.md +++ b/README.md @@ -23,11 +23,23 @@ 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) ``` +### SCRAM contract notes + +- 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. + ## Package Structure - `doc.go` - Overview and package documentation @@ -42,4 +54,4 @@ server.AddCredential(cred) ```bash go test -v ./ -``` \ No newline at end of file +``` diff --git a/argon2.go b/argon2.go index 9670587..50089c7 100644 --- a/argon2.go +++ b/argon2.go @@ -1,9 +1,9 @@ -// FILE: auth/argon2.go package auth import ( + "crypto/hmac" "crypto/rand" - "crypto/subtle" + "crypto/sha256" "encoding/base64" "fmt" "strings" @@ -18,6 +18,11 @@ const ( DefaultArgonThreads = 4 DefaultArgonSaltLen = 16 DefaultArgonKeyLen = 32 + MaxPasswordLen = 1024 + // upper bounds for untrusted PHC input + MaxArgonSaltLen = 64 + MaxArgonKeyLen = 64 + MaxPHCHashLen = 256 ) // argonParams holds configurable Argon2id parameters @@ -64,6 +69,9 @@ func HashPassword(password string, opts ...Option) (string, error) { if len(password) < 8 { return "", ErrWeakPassword } + if len(password) > MaxPasswordLen { + return "", ErrPasswordTooLong + } params := &argonParams{ time: DefaultArgonTime, @@ -92,62 +100,50 @@ func HashPassword(password string, opts ...Option) (string, error) { // VerifyPassword checks password against PHC-format hash (standalone) func VerifyPassword(password, phcHash string) error { - parts := strings.Split(phcHash, "$") - if len(parts) != 6 || parts[1] != "argon2id" { - return ErrPHCInvalidFormat - } - - var memory, time uint32 - var threads uint8 - fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &memory, &time, &threads) - - salt, err := base64.RawStdEncoding.DecodeString(parts[4]) - if err != nil { - return fmt.Errorf("%w: %v", ErrPHCInvalidSalt, err) - } - - expectedHash, err := base64.RawStdEncoding.DecodeString(parts[5]) - if err != nil { - return fmt.Errorf("%w: %v", ErrPHCInvalidHash, err) - } - - computedHash := argon2.IDKey([]byte(password), salt, time, memory, threads, uint32(len(expectedHash))) - - if subtle.ConstantTimeCompare(computedHash, expectedHash) != 1 { - return ErrInvalidCredentials - } - - return nil + _, err := verifyPHC(password, phcHash) + return err } // MigrateFromPHC converts PHC hash to SCRAM credential func MigrateFromPHC(username, password, phcHash string) (*Credential, error) { - parts := strings.Split(phcHash, "$") - if len(parts) != 6 || parts[1] != "argon2id" { - return nil, ErrPHCInvalidFormat - } - - var memory, time uint32 - var threads uint8 - fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &memory, &time, &threads) - - salt, err := base64.RawStdEncoding.DecodeString(parts[4]) + r, err := verifyPHC(password, phcHash) if err != nil { - return nil, ErrPHCInvalidSalt - } - - // Use standalone function for verification - if err := VerifyPassword(password, phcHash); err != nil { return nil, err } + if len(r.derived) == DefaultArgonKeyLen { + return credentialFromSaltedPassword(username, r.derived, r.salt, r.time, r.memory, r.threads), nil + } + // Non-standard digest length: derive at the required key length. + return DeriveCredential(username, password, r.salt, r.time, r.memory, r.threads) +} - return DeriveCredential(username, password, salt, time, memory, threads) +// key derivation split from the KDF so callers holding a salted +// 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")) + serverKey := computeHMAC(saltedPassword, []byte("Server Key")) + storedKey := sha256.Sum256(clientKey) + + return &Credential{ + Username: username, + Salt: salt, + ArgonTime: time, + ArgonMemory: memory, + ArgonThreads: threads, + StoredKey: storedKey[:], + ServerKey: serverKey, + } } // ValidatePHCHashFormat checks if a hash string has a valid and complete // PHC format for Argon2id. It validates structure, parameters, and encoding, // but does not verify a password against the hash. func ValidatePHCHashFormat(phcHash string) error { + // Cap total input before any splitting or base64 decoding + if len(phcHash) > MaxPHCHashLen { + return fmt.Errorf("%w: encoded hash exceeds %d bytes", ErrPHCInvalidFormat, MaxPHCHashLen) + } + parts := strings.Split(phcHash, "$") if len(parts) != 6 { return fmt.Errorf("%w: expected 6 parts, got %d", ErrPHCInvalidFormat, len(parts)) @@ -203,6 +199,9 @@ func ValidatePHCHashFormat(phcHash string) error { if len(salt) < 8 { // Minimum safe salt length return fmt.Errorf("%w: salt too short (%d bytes)", ErrPHCInvalidSalt, len(salt)) } + if len(salt) > MaxArgonSaltLen { + return fmt.Errorf("%w: salt too long (%d bytes)", ErrPHCInvalidSalt, len(salt)) + } // Validate hash encoding hash, err := base64.RawStdEncoding.DecodeString(parts[5]) @@ -212,6 +211,45 @@ func ValidatePHCHashFormat(phcHash string) error { if len(hash) < 16 { // Minimum hash length return fmt.Errorf("%w: hash too short (%d bytes)", ErrPHCInvalidHash, len(hash)) } + if len(hash) > MaxArgonKeyLen { + return fmt.Errorf("%w: hash too long (%d bytes)", ErrPHCInvalidHash, len(hash)) + } return nil -} \ No newline at end of file +} + +// parsed + verified PHC material, reused to avoid a second KDF pass +type phcResult struct { + derived []byte // argon2.IDKey output; == SCRAM salted password when len == DefaultArgonKeyLen + salt []byte + time uint32 + memory uint32 + threads uint8 +} + +// verifyPHC validates format, bounds the password, runs the KDF once, and +// constant-time compares against the encoded digest. +func verifyPHC(password, phcHash string) (*phcResult, error) { + if err := ValidatePHCHashFormat(phcHash); err != nil { + return nil, err + } + if len(password) > MaxPasswordLen { + return nil, ErrPasswordTooLong + } + + parts := strings.Split(phcHash, "$") + + r := &phcResult{} + // Parse is guaranteed well-formed by ValidatePHCHashFormat above. + fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &r.memory, &r.time, &r.threads) + + // Encodings validated above; errors are unreachable. + r.salt, _ = base64.RawStdEncoding.DecodeString(parts[4]) + expected, _ := base64.RawStdEncoding.DecodeString(parts[5]) + + r.derived = argon2.IDKey([]byte(password), r.salt, r.time, r.memory, r.threads, uint32(len(expected))) + if !hmac.Equal(r.derived, expected) { + return nil, ErrInvalidCredentials + } + return r, nil +} diff --git a/argon2_test.go b/argon2_test.go index b14c10b..3c0c00a 100644 --- a/argon2_test.go +++ b/argon2_test.go @@ -1,4 +1,3 @@ -// FILE: auth/argon2_test.go package auth import ( @@ -154,6 +153,14 @@ func TestValidatePHCHashFormat(t *testing.T) { base64.RawStdEncoding.EncodeToString([]byte("short")), ErrPHCInvalidHash}, {"too few parts", "$argon2id$v=19$m=65536,t=3,p=4", ErrPHCInvalidFormat}, {"too many parts", "$argon2id$v=19$m=65536,t=3,p=4$salt$hash$extra", ErrPHCInvalidFormat}, + {"oversized salt", "$argon2id$v=19$m=65536,t=3,p=4$" + + base64.RawStdEncoding.EncodeToString(make([]byte, 128)) + "$" + + base64.RawStdEncoding.EncodeToString([]byte("hash1234567890123456")), ErrPHCInvalidSalt}, + {"oversized hash", "$argon2id$v=19$m=65536,t=3,p=4$" + + base64.RawStdEncoding.EncodeToString([]byte("salt12345678")) + "$" + + base64.RawStdEncoding.EncodeToString(make([]byte, 128)), ErrPHCInvalidHash}, + {"oversized input", "$argon2id$v=19$m=65536,t=3,p=4$" + + strings.Repeat("A", 512) + "$hash", ErrPHCInvalidFormat}, } for _, tc := range testCases { @@ -172,4 +179,14 @@ func TestValidatePHCHashFormat(t *testing.T) { require.NoError(t, err) err = VerifyPassword("testPassword123", validHash) assert.NoError(t, err, "Validated hash should still work for password verification") -} \ No newline at end of file +} + +func TestVerifyPassword_MalformedParamsNoPanic(t *testing.T) { + for _, h := range []string{ + "$argon2id$v=19$m=65536,t=0,p=4$c2FsdHNhbHRzYWx0MTI$aGFzaGhhc2hoYXNoaGFzaA", + "$argon2id$v=19$m=65536,t=3,p=0$c2FsdHNhbHRzYWx0MTI$aGFzaGhhc2hoYXNoaGFzaA", + "$argon2id$v=19$garbage$c2FsdHNhbHRzYWx0MTI$aGFzaGhhc2hoYXNoaGFzaA", + } { + assert.Error(t, VerifyPassword("whatever", h)) + } +} diff --git a/doc.go b/doc.go index ad50a02..97fd786 100644 --- a/doc.go +++ b/doc.go @@ -1,4 +1,3 @@ -// FILE: auth/doc.go package auth /* @@ -37,6 +36,7 @@ Server and client implementation for SCRAM: // Server server := auth.NewScramServer() + defer server.Stop() server.AddCredential(credential) // Client @@ -50,4 +50,4 @@ Utility functions for HTTP headers: token, _ := auth.ParseBearerToken(header) Each module can be used independently without initializing other components. -*/ \ No newline at end of file +*/ diff --git a/error.go b/error.go index b4ca76c..d416a8f 100644 --- a/error.go +++ b/error.go @@ -1,4 +1,3 @@ -// FILE: auth/errors.go package auth import ( @@ -10,19 +9,19 @@ import ( 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") ) // JWT-specific errors var ( - ErrTokenMalformed = errors.New("token: malformed structure") - ErrTokenExpired = errors.New("token: expired") - ErrTokenNotYetValid = errors.New("token: not yet valid") - ErrTokenInvalidSignature = errors.New("token: invalid signature") - ErrTokenAlgorithmMismatch = errors.New("token: algorithm mismatch") - ErrTokenMissingClaim = errors.New("token: missing required claim") - 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") + ErrTokenMalformed = errors.New("token: malformed structure") + ErrTokenExpired = errors.New("token: expired") + ErrTokenNotYetValid = errors.New("token: not yet valid") + ErrTokenInvalidSignature = errors.New("token: invalid signature") + ErrTokenMissingClaim = errors.New("token: missing required claim") + 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") ) // JWT secret errors @@ -47,17 +46,17 @@ var ( // SCRAM-specific errors var ( - ErrSCRAMInvalidNonce = errors.New("scram: invalid nonce or expired handshake") - ErrSCRAMTimeout = errors.New("scram: handshake timeout") - ErrSCRAMVerifyInProgress = errors.New("scram: verification already in progress") - ErrSCRAMInvalidProof = errors.New("scram: invalid proof encoding") - ErrSCRAMInvalidProofLen = errors.New("scram: invalid proof length") - ErrSCRAMServerAuthFailed = errors.New("scram: server authentication failed") - ErrSCRAMInvalidState = errors.New("scram: invalid handshake state") - ErrSCRAMInvalidSalt = errors.New("scram: invalid salt encoding") - ErrSCRAMZeroParams = errors.New("scram: invalid Argon2 parameters") - ErrSCRAMSaltTooShort = errors.New("scram: salt must be at least 16 bytes") - ErrSCRAMNonceGenFailed = errors.New("scram: failed to generate nonce") + ErrSCRAMInvalidNonce = errors.New("scram: invalid nonce or expired handshake") + ErrSCRAMTimeout = errors.New("scram: handshake timeout") + ErrSCRAMVerifyInProgress = errors.New("scram: verification already in progress") + ErrSCRAMInvalidProof = errors.New("scram: invalid proof encoding") + ErrSCRAMInvalidProofLen = errors.New("scram: invalid proof length") + ErrSCRAMServerAuthFailed = errors.New("scram: server authentication failed") + ErrSCRAMInvalidState = errors.New("scram: invalid handshake state") + ErrSCRAMInvalidSalt = errors.New("scram: invalid salt encoding") + ErrSCRAMZeroParams = errors.New("scram: invalid Argon2 parameters") + ErrSCRAMSaltTooShort = errors.New("scram: salt must be at least 16 bytes") + ErrSCRAMTooManyHandshakes = errors.New("scram: handshake capacity exceeded") ) // Credential import/export errors @@ -88,8 +87,3 @@ var ( var ( ErrSaltGenerationFailed = errors.New("failed to generate salt") ) - -// Key generation errors -var ( - ErrRSAKeyGenFailed = errors.New("failed to generate RSA key") -) \ No newline at end of file diff --git a/go.mod b/go.mod index a4a8ef3..1b631d1 100644 --- a/go.mod +++ b/go.mod @@ -1,16 +1,16 @@ module github.com/lixenwraith/auth -go 1.25.3 +go 1.26.0 require ( - github.com/golang-jwt/jwt/v5 v5.3.0 + github.com/golang-jwt/jwt/v5 v5.3.1 github.com/stretchr/testify v1.11.1 - golang.org/x/crypto v0.43.0 + golang.org/x/crypto v0.54.0 ) require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - golang.org/x/sys v0.37.0 // indirect + golang.org/x/sys v0.47.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index d143a61..87ca3fd 100644 --- a/go.sum +++ b/go.sum @@ -2,14 +2,28 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/golang-jwt/jwt/v5 v5.3.0 h1:pv4AsKCKKZuqlgs5sUmn4x8UlGa0kEVt/puTpKx9vvo= github.com/golang-jwt/jwt/v5 v5.3.0/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= +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= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= golang.org/x/crypto v0.43.0 h1:dduJYIi3A3KOfdGOHX8AVZ/jGiyPa3IbBozJ5kNuE04= golang.org/x/crypto v0.43.0/go.mod h1:BFbav4mRNlXJL4wNeejLpWxB7wMbc79PdRGhWKncxR0= +golang.org/x/crypto v0.52.0 h1:RMs7fP2rXdep0CftQlK8Uf+kibLm7qkCcradZWYz988= +golang.org/x/crypto v0.52.0/go.mod h1:1QgfPxDqh0T2M/elOJtp9RvuR95kVjir0e6/BvEmGbc= +golang.org/x/crypto v0.53.0 h1:QZ4Muo8THX6CizN2vPPd5fBGHyogrdK9fG4wLPFUsto= +golang.org/x/crypto v0.53.0/go.mod h1:DNLU434OwVakk9PzuwV8w62mAJpRJL3vsgcfp4Qnsio= +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.37.0 h1:fdNQudmxPjkdUTPnLn5mdQv7Zwvbvpaxqs831goi9kQ= golang.org/x/sys v0.37.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= +golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw= +golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/http.go b/http.go index c8b7a16..474167b 100644 --- a/http.go +++ b/http.go @@ -1,4 +1,3 @@ -// FILE: auth/http.go package auth import ( @@ -58,4 +57,5 @@ func ExtractAuthType(header string) string { return header[:idx] } return "" -} \ No newline at end of file +} + diff --git a/http_test.go b/http_test.go index 984e590..0dbb42e 100644 --- a/http_test.go +++ b/http_test.go @@ -1,4 +1,3 @@ -// FILE: auth/http_test.go package auth import ( @@ -51,4 +50,5 @@ func TestHTTPAuthParsing(t *testing.T) { _, err = ParseBearerToken("Bearer ") assert.Error(t, err) assert.Equal(t, ErrAuthEmptyBearerToken, err) -} \ No newline at end of file +} + diff --git a/jwt.go b/jwt.go index ab10899..6c342c7 100644 --- a/jwt.go +++ b/jwt.go @@ -1,4 +1,3 @@ -// FILE: auth/jwt.go package auth import ( @@ -206,10 +205,7 @@ func mapJWTError(err error) error { case errors.Is(err, jwt.ErrTokenInvalidIssuer): return fmt.Errorf("%w : %w", ErrTokenMissingClaim, err) default: - // Check for algorithm mismatch in error message - if errors.Is(err, jwt.ErrTokenSignatureInvalid) { - return fmt.Errorf("%w : %w", ErrTokenAlgorithmMismatch, err) - } + // Alg rejection (WithValidMethods) surfaces as ErrTokenSignatureInvalid. return fmt.Errorf("%w : %w", ErrTokenMalformed, err) } } @@ -221,12 +217,16 @@ func GenerateHS256Token(secret []byte, userID string, claims map[string]any, lif if len(secret) < 32 { return "", ErrSecretTooShort } + if userID == "" { + return "", ErrTokenEmptyUserID + } now := time.Now() token := jwt.NewWithClaims(jwt.SigningMethodHS256, customClaims{ RegisteredClaims: jwt.RegisteredClaims{ Subject: userID, IssuedAt: jwt.NewNumericDate(now), + NotBefore: jwt.NewNumericDate(now), ExpiresAt: jwt.NewNumericDate(now.Add(lifetime)), }, Extra: claims, @@ -244,6 +244,7 @@ func ValidateHS256Token(secret []byte, tokenString string) (string, map[string]a parser := jwt.NewParser( jwt.WithValidMethods([]string{"HS256"}), jwt.WithLeeway(DefaultLeeway), + jwt.WithExpirationRequired(), ) token, err := parser.ParseWithClaims(tokenString, &customClaims{}, func(token *jwt.Token) (any, error) { @@ -290,10 +291,18 @@ func parseRSAPrivateKey(pemBytes []byte) (*rsa.PrivateKey, error) { if block == nil { return nil, ErrRSAInvalidPEM } - key, err := x509.ParsePKCS1PrivateKey(block.Bytes) + if key, err := x509.ParsePKCS1PrivateKey(block.Bytes); err == nil { + return key, nil + } + // PKCS8 fallback + keyAny, err := x509.ParsePKCS8PrivateKey(block.Bytes) if err != nil { return nil, ErrRSAInvalidPrivateKey } + key, ok := keyAny.(*rsa.PrivateKey) + if !ok { + return nil, ErrRSAInvalidPrivateKey + } return key, nil } @@ -312,4 +321,4 @@ func parseRSAPublicKey(pemBytes []byte) (*rsa.PublicKey, error) { return nil, ErrRSANotPublicKey } return pubKey, nil -} \ No newline at end of file +} diff --git a/jwt_test.go b/jwt_test.go index c65f28a..3aebc96 100644 --- a/jwt_test.go +++ b/jwt_test.go @@ -1,4 +1,3 @@ -// FILE: auth/jwt_test.go package auth import ( @@ -256,4 +255,5 @@ func TestJWTRSAFromPEM(t *testing.T) { _, err = NewJWTVerifierFromPEM([]byte("invalid pem data")) assert.ErrorIs(t, err, ErrRSAInvalidPEM) -} \ No newline at end of file +} + diff --git a/scram.go b/scram.go index d852e50..1303ab1 100644 --- a/scram.go +++ b/scram.go @@ -1,4 +1,3 @@ -// FILE: auth/scram.go package auth import ( @@ -8,6 +7,7 @@ import ( "crypto/subtle" "encoding/base64" "fmt" + "math" "sync" "sync/atomic" "time" @@ -21,7 +21,11 @@ const ( // ScramHandshakeTimeout defines maximum time for completing SCRAM handshake ScramHandshakeTimeout = 30 * time.Second // ScramCleanupInterval defines how often expired handshakes are cleaned - ScramCleanupInterval = 60 * time.Second + ScramCleanupInterval = 15 * time.Second + // ScramMaxHandshakes bounds concurrent in-flight handshakes. Caps memory, + // not compute: per client-first server cost is one HMAC. Rate limiting + // upstream remains the control for connection floods. + ScramMaxHandshakes = 4096 ) // Credential stores SCRAM authentication data @@ -79,13 +83,20 @@ func ImportCredential(data map[string]any) (*Credential, error) { } 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: - return 0, fmt.Errorf("invalid type for %s", key) + return 0, fmt.Errorf("%w: %s", ErrCredInvalidType, key) } } @@ -106,8 +117,14 @@ func ImportCredential(data map[string]any) (*Credential, error) { 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 @@ -133,6 +150,17 @@ func ImportCredential(data map[string]any) (*Credential, error) { return nil, fmt.Errorf("%w: %v", ErrCredInvalidServerKey, err) } + // Post-decode validation + if argonTime == 0 || argonMemory == 0 || argonThreads == 0 { + return nil, ErrSCRAMZeroParams + } + if len(salt) < 16 { + return nil, ErrSCRAMSaltTooShort + } + if len(storedKey) != sha256.Size || len(serverKey) != sha256.Size { + return nil, ErrCredInvalidStoredKey + } + return &Credential{ Username: username, Salt: salt, @@ -154,23 +182,12 @@ func DeriveCredential(username, password string, salt []byte, time, memory uint3 return nil, ErrSCRAMZeroParams } - // Derive salted password using Argon2id + if len(password) > MaxPasswordLen { + return nil, ErrPasswordTooLong + } + saltedPassword := argon2.IDKey([]byte(password), salt, time, memory, threads, DefaultArgonKeyLen) - - // Derive keys - clientKey := computeHMAC(saltedPassword, []byte("Client Key")) - serverKey := computeHMAC(saltedPassword, []byte("Server Key")) - storedKey := sha256.Sum256(clientKey) - - return &Credential{ - Username: username, - Salt: salt, - ArgonTime: time, - ArgonMemory: memory, - ArgonThreads: threads, - StoredKey: storedKey[:], - ServerKey: serverKey, - }, nil + return credentialFromSaltedPassword(username, saltedPassword, salt, time, memory, threads), nil } // HandshakeState tracks ongoing authentication @@ -181,37 +198,57 @@ type HandshakeState struct { FullNonce string Credential *Credential CreatedAt time.Time - verifying int32 // Atomic flag to prevent race during verification + verifying atomic.Int32 // Atomic flag to prevent race during verification } // ScramServer handles server-side SCRAM authentication type ScramServer struct { credentials map[string]*Credential handshakes map[string]*HandshakeState + decoyKey []byte // HMAC key for stable decoy salts + decoyTemplate Credential // param/salt-length shape mirrored to unknown users mu sync.RWMutex cleanupTicker *time.Ticker cleanupStop chan struct{} + stopOnce sync.Once } // NewScramServer creates SCRAM server 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{}), } - // Start background cleanup goroutine go s.cleanupLoop() return s } +// decoySalt generates stable decoy salt; indistinguishable across repeated probes +func (s *ScramServer) decoySalt(username string) []byte { + n := len(s.decoyTemplate.Salt) + if n < 16 { + n = DefaultArgonSaltLen + } + 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))...) + } + return out[:n] +} + // Stop gracefully shuts down the server and cleanup goroutine func (s *ScramServer) Stop() { - close(s.cleanupStop) - s.cleanupTicker.Stop() + s.stopOnce.Do(func() { + close(s.cleanupStop) + s.cleanupTicker.Stop() + }) } // cleanupLoop runs periodic cleanup of expired handshakes @@ -226,57 +263,86 @@ func (s *ScramServer) cleanupLoop() { } } -// cleanupExpiredHandshakes removes handshakes older than timeout +// locking split from sweep logic func (s *ScramServer) cleanupExpiredHandshakes() { s.mu.Lock() defer s.mu.Unlock() + s.evictExpiredLocked() +} +// evictExpiredLocked removes timed-out handshakes. Caller holds s.mu. +func (s *ScramServer) evictExpiredLocked() { cutoff := time.Now().Add(-ScramHandshakeTimeout) for nonce, state := range s.handshakes { - if state.CreatedAt.Before(cutoff) && atomic.LoadInt32(&state.verifying) == 0 { + if state.CreatedAt.Before(cutoff) && state.verifying.Load() == 0 { delete(s.handshakes, nonce) } } } // ProcessClientFirstMessage processes initial auth request +// +// An unknown username does NOT produce an error here. The server returns +// a deterministic decoy salt and stores a decoy handshake so that failure +// surfaces only at ProcessClientFinalMessage as ErrInvalidCredentials, matching +// the wrong-password path. Callers must not treat a successful return as +// evidence that the account exists, and must not log it as an auth success. +// +// ErrSCRAMTooManyHandshakes is returned when the in-flight handshake cap is +// 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) { s.mu.Lock() defer s.mu.Unlock() - // Check if user exists - cred, exists := s.credentials[username] - if !exists { - // Prevent user enumeration - still generate response - salt := make([]byte, 16) - rand.Read(salt) - serverNonce := generateNonce() - - return ServerFirstMessage{ - FullNonce: clientNonce + serverNonce, - Salt: base64.StdEncoding.EncodeToString(salt), - ArgonTime: DefaultArgonTime, - ArgonMemory: DefaultArgonMemory, - ArgonThreads: DefaultArgonThreads, - }, ErrInvalidCredentials + // 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 { + s.evictExpiredLocked() + if len(s.handshakes) >= ScramMaxHandshakes { + return ServerFirstMessage{}, ErrSCRAMTooManyHandshakes + } } // Generate server nonce - serverNonce := generateNonce() + serverNonce := rand.Text() fullNonce := clientNonce + serverNonce - // Store handshake state - state := &HandshakeState{ - Username: username, - ClientNonce: clientNonce, - ServerNonce: serverNonce, - FullNonce: fullNonce, - Credential: cred, - CreatedAt: time.Now(), - verifying: 0, + // 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 } - s.handshakes[fullNonce] = state + 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), @@ -288,20 +354,21 @@ func (s *ScramServer) ProcessClientFirstMessage(username, clientNonce string) (S // ProcessClientFinalMessage verifies client proof func (s *ScramServer) ProcessClientFinalMessage(fullNonce, clientProof string) (ServerFinalMessage, error) { - s.mu.RLock() + // ookup + CAS under one write lock; closes the sweep race + s.mu.Lock() state, exists := s.handshakes[fullNonce] - s.mu.RUnlock() - if !exists { + s.mu.Unlock() return ServerFinalMessage{}, ErrSCRAMInvalidNonce } - - // Mark as verifying to prevent deletion race - if !atomic.CompareAndSwapInt32(&state.verifying, 0, 1) { + ok := state.verifying.CompareAndSwap(0, 1) + s.mu.Unlock() + if !ok { return ServerFinalMessage{}, ErrSCRAMVerifyInProgress } + defer func() { - atomic.StoreInt32(&state.verifying, 0) + state.verifying.Store(0) // Safe to delete after verification completes s.mu.Lock() delete(s.handshakes, fullNonce) @@ -313,7 +380,6 @@ func (s *ScramServer) ProcessClientFinalMessage(fullNonce, clientProof string) ( return ServerFinalMessage{}, ErrSCRAMTimeout } - // [rest of verification logic unchanged] // Decode client proof clientProofBytes, err := base64.StdEncoding.DecodeString(clientProof) if err != nil { @@ -361,14 +427,11 @@ func (s *ScramServer) AddCredential(cred *Credential) { s.mu.Lock() defer s.mu.Unlock() s.credentials[cred.Username] = cred -} - -func (s *ScramServer) cleanupHandshakes() { - cutoff := time.Now().Add(-60 * time.Second) - for nonce, state := range s.handshakes { - if state.CreatedAt.Before(cutoff) && atomic.LoadInt32(&state.verifying) == 0 { - delete(s.handshakes, nonce) - } + s.decoyTemplate = Credential{ + Salt: make([]byte, len(cred.Salt)), + ArgonTime: cred.ArgonTime, + ArgonMemory: cred.ArgonMemory, + ArgonThreads: cred.ArgonThreads, } } @@ -393,14 +456,15 @@ func NewScramClient(username, password string) *ScramClient { // StartAuthentication generates initial client message func (c *ScramClient) StartAuthentication() (ClientFirstRequest, error) { + // Reject oversized password before the handshake commits to a KDF pass + if len(c.Password) > MaxPasswordLen { + return ClientFirstRequest{}, ErrPasswordTooLong + } + c.startTime = time.Now() // Generate client nonce - nonce := make([]byte, 32) - if _, err := rand.Read(nonce); err != nil { - return ClientFirstRequest{}, ErrSCRAMNonceGenFailed - } - c.clientNonce = base64.StdEncoding.EncodeToString(nonce) + c.clientNonce = rand.Text() return ClientFirstRequest{ Username: c.Username, @@ -460,7 +524,6 @@ func (c *ScramClient) VerifyServerFinalMessage(msg ServerFinalMessage) error { return ErrSCRAMTimeout } - // [rest unchanged] if c.authMessage == "" || c.serverKey == nil { return ErrSCRAMInvalidState } @@ -537,9 +600,3 @@ func xorBytes(a, b []byte) []byte { } return result } - -func generateNonce() string { - b := make([]byte, 32) - rand.Read(b) - return base64.StdEncoding.EncodeToString(b) -} \ No newline at end of file diff --git a/scram_test.go b/scram_test.go index 40c1e4e..e379e23 100644 --- a/scram_test.go +++ b/scram_test.go @@ -1,4 +1,3 @@ -// FILE: auth/scram_test.go package auth import ( @@ -27,6 +26,7 @@ func setupScramTest(t *testing.T) (server *ScramServer, username, password strin // 3. Create a server and add the new credential. server = NewScramServer() + t.Cleanup(server.Stop) server.AddCredential(cred) return server, username, password, cred @@ -64,6 +64,7 @@ func TestScram_FullRoundtrip_Success(t *testing.T) { // TestScram_FullRoundtrip_WrongPassword ensures authentication fails with an incorrect password. func TestScram_FullRoundtrip_WrongPassword(t *testing.T) { server, username, _, _ := setupScramTest(t) + defer server.Stop() // Create a client with the WRONG password client := NewScramClient(username, "WrongPassword!!!") @@ -85,22 +86,36 @@ func TestScram_FullRoundtrip_WrongPassword(t *testing.T) { } // TestScram_FullRoundtrip_UserNotFound tests for user enumeration protection. -// The server should not reveal whether a user exists or not in its first message. +// The server must be indistinguishable from the wrong-password path: no error at +// first message, stable decoy salt across probes, ErrInvalidCredentials at proof. func TestScram_FullRoundtrip_UserNotFound(t *testing.T) { server, _, _, _ := setupScramTest(t) + defer server.Stop() client := NewScramClient("unknown_user", "any_password") clientFirst, err := client.StartAuthentication() require.NoError(t, err) - // --- Step 2: Server should return an error, but also a FAKE response --- - // This prevents an attacker from knowing if the user exists based on the response structure. + // unknown user must not error here serverFirst, err := server.ProcessClientFirstMessage(clientFirst.Username, clientFirst.ClientNonce) - assert.ErrorIs(t, err, ErrInvalidCredentials, "Server should return an error for an unknown user") - assert.NotEmpty(t, serverFirst.FullNonce, "Server must still provide a nonce to prevent enumeration") - assert.NotEmpty(t, serverFirst.Salt, "Server must still provide a salt to prevent enumeration") + require.NoError(t, err, "unknown user must not be signalled at first message") + assert.NotEmpty(t, serverFirst.FullNonce) + assert.NotEmpty(t, serverFirst.Salt) - t.Log("SCRAM correctly protected against user enumeration") + // decoy salt must be stable across repeated probes + second, err := server.ProcessClientFirstMessage(clientFirst.Username, "probe-nonce-2") + require.NoError(t, err) + assert.Equal(t, serverFirst.Salt, second.Salt, "decoy salt must be deterministic") + assert.Equal(t, serverFirst.ArgonTime, second.ArgonTime) + assert.Equal(t, serverFirst.ArgonMemory, second.ArgonMemory) + assert.Equal(t, serverFirst.ArgonThreads, second.ArgonThreads) + + clientFinal, err := client.ProcessServerFirstMessage(serverFirst) + require.NoError(t, err) + + // failure surfaces only here, identical to wrong-password path + _, err = server.ProcessClientFinalMessage(clientFinal.FullNonce, clientFinal.ClientProof) + assert.ErrorIs(t, err, ErrInvalidCredentials, "unknown user must fail like wrong password") } // TestScram_InvalidNonce simulates a replay attack or message mismatch. @@ -316,4 +331,4 @@ func TestScramExplicitTimeout(t *testing.T) { assert.ErrorIs(t, err, ErrSCRAMTimeout, "Client should reject after timeout") _ = originalTimeout // Suppress unused variable warning -} \ No newline at end of file +} diff --git a/token.go b/token.go index b2c23cf..544e926 100644 --- a/token.go +++ b/token.go @@ -1,50 +1,44 @@ -// FILE: auth/token.go package auth import ( - "crypto/subtle" + "crypto/sha256" "sync" ) // SimpleTokenValidator implements in-memory token validation type SimpleTokenValidator struct { - tokens map[string]struct{} + tokens map[[32]byte]struct{} // keyed by SHA-256(token) mu sync.RWMutex } // NewSimpleTokenValidator creates token validator func NewSimpleTokenValidator() *SimpleTokenValidator { return &SimpleTokenValidator{ - tokens: make(map[string]struct{}), + tokens: make(map[[32]byte]struct{}), } } // ValidateToken checks if token is valid func (v *SimpleTokenValidator) ValidateToken(token string) bool { + h := sha256.Sum256([]byte(token)) v.mu.RLock() defer v.mu.RUnlock() - - // Constant-time comparison for each stored token - for storedToken := range v.tokens { - if subtle.ConstantTimeEq(int32(len(token)), int32(len(storedToken))) == 1 { - if subtle.ConstantTimeCompare([]byte(token), []byte(storedToken)) == 1 { - return true - } - } - } - return false + _, ok := v.tokens[h] + return ok } // AddToken adds token to validator func (v *SimpleTokenValidator) AddToken(token string) { + h := sha256.Sum256([]byte(token)) v.mu.Lock() defer v.mu.Unlock() - v.tokens[token] = struct{}{} + v.tokens[h] = struct{}{} } // RemoveToken removes token from validator func (v *SimpleTokenValidator) RemoveToken(token string) { + h := sha256.Sum256([]byte(token)) v.mu.Lock() defer v.mu.Unlock() - delete(v.tokens, token) -} \ No newline at end of file + delete(v.tokens, h) +} diff --git a/token_test.go b/token_test.go index 6d25017..2ace02c 100644 --- a/token_test.go +++ b/token_test.go @@ -1,4 +1,3 @@ -// FILE: auth/token_test.go package auth import ( @@ -78,4 +77,5 @@ func TestConcurrentTokenValidator(t *testing.T) { token := fmt.Sprintf("token-%d", i) assert.True(t, validator.ValidateToken(token)) } -} \ No newline at end of file +} +