diff --git a/internal/utils/user_utils.go b/internal/utils/user_utils.go index d94b3a20..aa77d54e 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 973be918..bed7c010 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()