fix(coderabbit): address review findings
- auth: prevent legacy .lthn fallback when .hash file exists but is unreadable or has unexpected format (security fix in verifyPassword and Login) - chachapoly: wrap raw error returns in Decrypt with coreerr.E() - trust: reject trailing data in LoadPolicies JSON decoder Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
parent
f4a219816a
commit
36bf16b06e
3 changed files with 43 additions and 24 deletions
48
auth/auth.go
48
auth/auth.go
|
|
@ -420,19 +420,21 @@ func (a *Authenticator) Login(userID, password string) (*Session, error) {
|
||||||
return nil, coreerr.E(op, "failed to read password hash", err)
|
return nil, coreerr.E(op, "failed to read password hash", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
if strings.HasPrefix(storedHash, "$argon2id$") {
|
if !strings.HasPrefix(storedHash, "$argon2id$") {
|
||||||
valid, err := crypt.VerifyPassword(password, storedHash)
|
return nil, coreerr.E(op, "corrupted password hash", nil)
|
||||||
if err != nil {
|
|
||||||
return nil, coreerr.E(op, "failed to verify password", err)
|
|
||||||
}
|
|
||||||
if !valid {
|
|
||||||
return nil, coreerr.E(op, "invalid password", nil)
|
|
||||||
}
|
|
||||||
return a.createSession(userID)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
valid, err := crypt.VerifyPassword(password, storedHash)
|
||||||
|
if err != nil {
|
||||||
|
return nil, coreerr.E(op, "failed to verify password", err)
|
||||||
|
}
|
||||||
|
if !valid {
|
||||||
|
return nil, coreerr.E(op, "invalid password", nil)
|
||||||
|
}
|
||||||
|
return a.createSession(userID)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Fall back to legacy LTHN hash (.lthn file)
|
// Fall back to legacy LTHN hash (.lthn file) — only when no .hash file exists
|
||||||
storedHash, err := a.medium.Read(userPath(userID, ".lthn"))
|
storedHash, err := a.medium.Read(userPath(userID, ".lthn"))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, coreerr.E(op, "user not found", err)
|
return nil, coreerr.E(op, "user not found", err)
|
||||||
|
|
@ -646,19 +648,25 @@ func (a *Authenticator) verifyPassword(userID, password string) error {
|
||||||
// Try Argon2id hash first (.hash file)
|
// Try Argon2id hash first (.hash file)
|
||||||
if a.medium.IsFile(userPath(userID, ".hash")) {
|
if a.medium.IsFile(userPath(userID, ".hash")) {
|
||||||
storedHash, err := a.medium.Read(userPath(userID, ".hash"))
|
storedHash, err := a.medium.Read(userPath(userID, ".hash"))
|
||||||
if err == nil && strings.HasPrefix(storedHash, "$argon2id$") {
|
if err != nil {
|
||||||
valid, verr := crypt.VerifyPassword(password, storedHash)
|
return coreerr.E(op, "failed to read password hash", err)
|
||||||
if verr != nil {
|
|
||||||
return coreerr.E(op, "failed to verify password", nil)
|
|
||||||
}
|
|
||||||
if !valid {
|
|
||||||
return coreerr.E(op, "invalid password", nil)
|
|
||||||
}
|
|
||||||
return nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if !strings.HasPrefix(storedHash, "$argon2id$") {
|
||||||
|
return coreerr.E(op, "corrupted password hash", nil)
|
||||||
|
}
|
||||||
|
|
||||||
|
valid, verr := crypt.VerifyPassword(password, storedHash)
|
||||||
|
if verr != nil {
|
||||||
|
return coreerr.E(op, "failed to verify password", verr)
|
||||||
|
}
|
||||||
|
if !valid {
|
||||||
|
return coreerr.E(op, "invalid password", nil)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Fall back to legacy LTHN hash (.lthn file)
|
// Fall back to legacy LTHN hash (.lthn file) — only when no .hash file exists
|
||||||
storedHash, err := a.medium.Read(userPath(userID, ".lthn"))
|
storedHash, err := a.medium.Read(userPath(userID, ".lthn"))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return coreerr.E(op, "user not found", nil)
|
return coreerr.E(op, "user not found", nil)
|
||||||
|
|
|
||||||
|
|
@ -27,21 +27,23 @@ func Encrypt(plaintext []byte, key []byte) ([]byte, error) {
|
||||||
|
|
||||||
// Decrypt decrypts data using ChaCha20-Poly1305.
|
// Decrypt decrypts data using ChaCha20-Poly1305.
|
||||||
func Decrypt(ciphertext []byte, key []byte) ([]byte, error) {
|
func Decrypt(ciphertext []byte, key []byte) ([]byte, error) {
|
||||||
|
const op = "chachapoly.Decrypt"
|
||||||
|
|
||||||
aead, err := chacha20poly1305.NewX(key)
|
aead, err := chacha20poly1305.NewX(key)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, coreerr.E(op, "failed to create cipher", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
minLen := aead.NonceSize() + aead.Overhead()
|
minLen := aead.NonceSize() + aead.Overhead()
|
||||||
if len(ciphertext) < minLen {
|
if len(ciphertext) < minLen {
|
||||||
return nil, coreerr.E("chachapoly.Decrypt", fmt.Sprintf("ciphertext too short: got %d bytes, need at least %d bytes", len(ciphertext), minLen), nil)
|
return nil, coreerr.E(op, fmt.Sprintf("ciphertext too short: got %d bytes, need at least %d bytes", len(ciphertext), minLen), nil)
|
||||||
}
|
}
|
||||||
|
|
||||||
nonce, ciphertext := ciphertext[:aead.NonceSize()], ciphertext[aead.NonceSize():]
|
nonce, ciphertext := ciphertext[:aead.NonceSize()], ciphertext[aead.NonceSize():]
|
||||||
|
|
||||||
decrypted, err := aead.Open(nil, nonce, ciphertext, nil)
|
decrypted, err := aead.Open(nil, nonce, ciphertext, nil)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, coreerr.E(op, "decryption failed", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
if len(decrypted) == 0 {
|
if len(decrypted) == 0 {
|
||||||
|
|
|
||||||
|
|
@ -34,12 +34,21 @@ func LoadPoliciesFromFile(path string) ([]Policy, error) {
|
||||||
|
|
||||||
// LoadPolicies reads JSON from a reader and returns parsed policies.
|
// LoadPolicies reads JSON from a reader and returns parsed policies.
|
||||||
func LoadPolicies(r io.Reader) ([]Policy, error) {
|
func LoadPolicies(r io.Reader) ([]Policy, error) {
|
||||||
|
const op = "trust.LoadPolicies"
|
||||||
|
|
||||||
var cfg PoliciesConfig
|
var cfg PoliciesConfig
|
||||||
dec := json.NewDecoder(r)
|
dec := json.NewDecoder(r)
|
||||||
dec.DisallowUnknownFields()
|
dec.DisallowUnknownFields()
|
||||||
if err := dec.Decode(&cfg); err != nil {
|
if err := dec.Decode(&cfg); err != nil {
|
||||||
return nil, coreerr.E("trust.LoadPolicies", "failed to decode JSON", err)
|
return nil, coreerr.E(op, "failed to decode JSON", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Reject trailing data after the decoded value
|
||||||
|
var extra json.RawMessage
|
||||||
|
if err := dec.Decode(&extra); err != io.EOF {
|
||||||
|
return nil, coreerr.E(op, "unexpected trailing data in JSON", nil)
|
||||||
|
}
|
||||||
|
|
||||||
return convertPolicies(cfg)
|
return convertPolicies(cfg)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue