From d9dac944cac9b4b4b9f4c6a3bc21b803d514a5c3 Mon Sep 17 00:00:00 2001 From: bsaurusrex <82356519+bsaurusrex@users.noreply.github.com> Date: Fri, 9 Oct 2026 14:11:23 +0800 Subject: [PATCH] fix: reject invalid user password hashes at startup Configured local users (TINYAUTH_AUTH_USERS / users file) were loaded without validating the password field, and bcrypt ignores any bytes after a complete 60-char hash. A malformed, truncated or trailing-garbage hash was accepted silently and only surfaced as a confusing failed login. GetUsers now verifies each password is exactly one well-formed bcrypt hash (prefix, cost and base64 body) and fails startup naming the user, instead of loading an account that can never authenticate. The ParseUser primitive is unchanged, so the verify and generate-totp commands that call it directly keep working. Refs #685 Co-Authored-By: Claude Opus 5.5 --- internal/utils/user_utils.go | 51 +++++++++++++++++++++++++++++++ internal/utils/user_utils_test.go | 44 ++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/internal/utils/user_utils.go b/internal/utils/user_utils.go index d94b3a20d..aa77d54e9 100644 --- a/internal/utils/user_utils.go +++ b/internal/utils/user_utils.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/tinyauthapp/tinyauth/internal/model" + "golang.org/x/crypto/bcrypt" ) func ParseUsers(usersStr []string, userAttributes map[string]model.UserAttributes) (*[]model.LocalUser, error) { @@ -24,6 +25,11 @@ func ParseUsers(usersStr []string, userAttributes map[string]model.UserAttribute if err != nil { return nil, err } + // Validation lives here, at config load, rather than in ParseUser, which the verify and + // generate-totp commands call directly on an already-stored hash. + if !isBcryptHash(parsed.Password) { + return nil, fmt.Errorf("invalid password hash for user %q, expected a single bcrypt hash", parsed.Username) + } if attrs, ok := userAttributes[parsed.Username]; ok { parsed.Attributes = attrs } @@ -73,6 +79,51 @@ func ParseUser(userStr string) (*model.LocalUser, error) { return &user, nil } +// bcryptBase64Alphabet is the (non-standard) base64 alphabet bcrypt encodes its salt and hash with. +const bcryptBase64Alphabet = "./ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789" + +// isBcryptHash reports whether s is exactly one canonical bcrypt hash. bcrypt ignores trailing bytes and +// parses the cost leniently, and bcrypt.CompareHashAndPassword re-encodes a hash before comparing, so a +// value that merely looks close (wrong delimiter, non-canonical base64 tail bits, trailing garbage) would +// pass a loose check yet never authenticate. The full layout is validated to reject those: +// +// $ 2[aby] $ <2-digit cost> $ <22-char salt> <31-char hash> (60 bytes) +func isBcryptHash(s string) bool { + if len(s) != 60 { + return false + } + + // Prefix: "$2[aby]$NN$" where NN is the two-digit cost. + if s[0] != '$' || s[1] != '2' || (s[2] != 'a' && s[2] != 'b' && s[2] != 'y') || s[3] != '$' || s[6] != '$' { + return false + } + if s[4] < '0' || s[4] > '9' || s[5] < '0' || s[5] > '9' { + return false + } + if cost := int(s[4]-'0')*10 + int(s[5]-'0'); cost < bcrypt.MinCost || cost > bcrypt.MaxCost { + return false + } + + // The 22-char salt and 31-char hash must be in the bcrypt base64 alphabet. + for i := 7; i < len(s); i++ { + if strings.IndexByte(bcryptBase64Alphabet, s[i]) < 0 { + return false + } + } + + // 22 base64 chars hold the 16-byte salt (4 excess bits) and 31 hold the 23-byte hash (2 excess bits), + // so the final char of each must carry zero in those excess low bits. Otherwise bcrypt re-encodes it + // to a different canonical string and a password can never match the stored value. + if strings.IndexByte(bcryptBase64Alphabet, s[28])%16 != 0 { + return false + } + if strings.IndexByte(bcryptBase64Alphabet, s[59])%4 != 0 { + return false + } + + return true +} + func CompileUserEmail(username string, domain string) string { _, err := mail.ParseAddress(username) diff --git a/internal/utils/user_utils_test.go b/internal/utils/user_utils_test.go index 973be9183..bed7c010f 100644 --- a/internal/utils/user_utils_test.go +++ b/internal/utils/user_utils_test.go @@ -2,6 +2,7 @@ package utils_test import ( "os" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -10,6 +11,49 @@ import ( "github.com/tinyauthapp/tinyauth/internal/utils" ) +func TestGetUsersRejectsInvalidHash(t *testing.T) { + hash := "$2a$10$Mz5xhkfSJUtPWkzCd/TdaePh9CaXc5QcGII5wIMPLSR46eTwma30G" + noAttrs := map[string]model.UserAttributes{} + + // A plaintext (non-bcrypt) password is rejected at load with the username named + _, err := utils.GetUsers([]string{"alice:not-a-hash"}, "", noAttrs) + assert.ErrorContains(t, err, `invalid password hash for user "alice"`) + + // A 60-char value with the right prefix/cost but a malformed base64 body is rejected + badHash := "$2a$10$" + strings.Repeat("!", 53) + require.Len(t, badHash, 60) + _, err = utils.GetUsers([]string{"bob:" + badHash}, "", noAttrs) + assert.ErrorContains(t, err, `invalid password hash for user "bob"`) + + // bcrypt ignores trailing bytes, so a valid hash with extra bytes is rejected too + _, err = utils.GetUsers([]string{"carol:" + hash + "extra"}, "", noAttrs) + assert.ErrorContains(t, err, `invalid password hash for user "carol"`) + + // A valid hash (with a TOTP secret) still loads + users, err := utils.GetUsers([]string{"dave:" + hash + ":JBSWY3DPEHPK3PXP"}, "", noAttrs) + assert.NoError(t, err) + assert.Len(t, *users, 1) + assert.Equal(t, "dave", (*users)[0].Username) + + // Every valid bcrypt minor version is accepted (the body is identical, only the tag differs) + for _, v := range []string{"$2a$", "$2b$", "$2y$"} { + _, err := utils.GetUsers([]string{"eve:" + v + hash[4:]}, "", noAttrs) + assert.NoError(t, err, "version %s should be accepted", v) + } + + // Values bcrypt would re-encode differently (so the original password could never match) are rejected: + // a wrong cost delimiter, and non-canonical final salt/checksum base64 characters. + mutate := func(i int, c byte) string { b := []byte(hash); b[i] = c; return string(b) } + for name, bad := range map[string]string{ + "wrong delimiter": mutate(6, 'X'), + "noncanonical checksum tail": mutate(59, 'H'), // canonical final char here is 'G' + "noncanonical salt tail": mutate(28, 'f'), // canonical final salt char here is 'e' + } { + _, err := utils.GetUsers([]string{"frank:" + bad}, "", noAttrs) + assert.ErrorContains(t, err, `invalid password hash for user "frank"`, name) + } +} + func TestGetUsers(t *testing.T) { tmpDir := t.TempDir()