Get rid of max password length limit

This commit is contained in:
Jonathon Leight committed 2026-07-05 13:48:20 -04:00
1 parent d35f5dbb97
commit 5bb86e2e0c
6 files changed
+147 -19

No files matched your search

+1 -1
View File
@@ -403,7 +403,7 @@ func (s *Handlers) handleChangePassword(w http.ResponseWriter, r *http.Request)
newPassword := r.FormValue("new_password")
if !ValidPassword(newPassword) {
accountRedirect(w, r, "error", "New password must be 8–72 characters.")
accountRedirect(w, r, "error", "New password must be at least 8 characters.")
return
}
// When a password already exists, require the current one to change it.
+6 -10
View File
@@ -409,18 +409,14 @@ func NormalizePasskeyName(s string) string {
return s
}
// Password length bounds. The lower bound is a basic strength floor; the upper
// bound is bcrypt's hard limit — GenerateFromPassword rejects inputs longer than
// 72 bytes, so we reject them at the form boundary with a clear message rather
// than surfacing a generic hashing failure.
const (
MinPasswordLen = 8
MaxPasswordLen = 72
)
// MinPasswordLen is a basic strength floor. There is deliberately no maximum:
// passwords are pre-hashed before bcrypt (see hashPassword), so bcrypt's 72-byte
// input limit never reaches the user.
const MinPasswordLen = 8
// ValidPassword reports whether p is within the accepted length bounds.
// ValidPassword reports whether p meets the minimum length.
func ValidPassword(p string) bool {
return len(p) >= MinPasswordLen && len(p) <= MaxPasswordLen
return len(p) >= MinPasswordLen
}
// ValidUsername reports whether s is 3–32 chars of [a-z0-9_.-].
+1 -1
View File
@@ -14,7 +14,7 @@ func (s *Service) SignupPassword(w http.ResponseWriter, r *http.Request) {
displayName := NormalizeDisplayName(r.FormValue("display_name"))
password := r.FormValue("password")
if !ValidUsername(username) || !ValidPassword(password) {
redirectErr(w, r, "/signup", "Choose a username (3–32 chars: letters, digits, _ . -) and a password of 8–72 characters.")
redirectErr(w, r, "/signup", "Choose a username (3–32 chars: letters, digits, _ . -) and a password of at least 8 characters.")
return
}
+46 -4
View File
@@ -2,6 +2,8 @@ package auth
import (
"context"
"crypto/sha256"
"encoding/base64"
"errors"
"net/http"
"time"
@@ -182,9 +184,15 @@ func (s *Service) VerifyPassword(ctx context.Context, username, password string)
if u.PasswordHash == nil {
return nil, ErrInvalidCredentials
}
if err := bcrypt.CompareHashAndPassword([]byte(*u.PasswordHash), []byte(password)); err != nil {
ok, legacy := comparePassword(*u.PasswordHash, password)
if !ok {
return nil, ErrInvalidCredentials
}
// Transparently migrate a legacy (raw-bcrypt) hash to the pre-hash scheme on
// successful login; best-effort, never blocks the sign-in.
if legacy {
_ = s.SetPassword(ctx, u.ID, password)
}
return u, nil
}
@@ -234,16 +242,50 @@ func (s *Service) PasswordMatches(u *store.User, password string) bool {
if u.PasswordHash == nil {
return false
}
return bcrypt.CompareHashAndPassword([]byte(*u.PasswordHash), []byte(password)) == nil
ok, _ := comparePassword(*u.PasswordHash, password)
return ok
}
// SetPassword hashes and stores a password for a user.
func (s *Service) SetPassword(ctx context.Context, userID int64, password string) error {
hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
hash, err := hashPassword(password)
if err != nil {
return err
}
return s.store.SetPassword(ctx, userID, string(hash))
return s.store.SetPassword(ctx, userID, hash)
}
// bcrypt only considers the first 72 bytes of its input (and stops at the first
// NUL byte), so we can't feed it a raw password without imposing a length limit.
// Pre-hashing with SHA-256 and base64-encoding the digest collapses any-length
// password to a fixed 44-byte, NUL-free token that always fits — so users face
// no visible maximum. (This is the standard bcrypt-pre-hash construction.)
func prehashPassword(password string) []byte {
sum := sha256.Sum256([]byte(password))
enc := base64.StdEncoding.EncodeToString(sum[:])
return []byte(enc)
}
// hashPassword produces the stored bcrypt hash for a password (pre-hash scheme).
func hashPassword(password string) (string, error) {
h, err := bcrypt.GenerateFromPassword(prehashPassword(password), bcrypt.DefaultCost)
return string(h), err
}
// comparePassword checks password against a stored bcrypt hash, accepting both
// the current pre-hash scheme and the legacy raw-bcrypt scheme (hashes written
// before pre-hashing existed). legacy reports which one matched, so the caller
// can upgrade the stored hash. Legacy hashes only ever held passwords ≤72 bytes
// (that limit was enforced then), so the raw comparison is safe and can't be
// confused with a pre-hash match.
func comparePassword(hash, password string) (ok, legacy bool) {
if bcrypt.CompareHashAndPassword([]byte(hash), prehashPassword(password)) == nil {
return true, false
}
if bcrypt.CompareHashAndPassword([]byte(hash), []byte(password)) == nil {
return true, true
}
return false, false
}
// ErrInvalidCredentials is returned for any failed password authentication.
+46 -3
View File
@@ -3,17 +3,19 @@ package auth
import (
"strings"
"testing"
"golang.org/x/crypto/bcrypt"
)
// TestValidPassword pins the accepted length window: an 8-char floor and a
// 72-byte ceiling (bcrypt's hard input limit). Reject just outside both ends.
// TestValidPassword pins the 8-char floor and confirms there is no upper bound:
// pre-hashing removes bcrypt's 72-byte input limit, so long passwords are valid.
func TestValidPassword(t *testing.T) {
t.Parallel()
cases := []struct {
n int
want bool
}{
{7, false}, {8, true}, {72, true}, {73, false},
{7, false}, {8, true}, {72, true}, {73, true}, {5000, true},
}
for _, c := range cases {
if got := ValidPassword(strings.Repeat("a", c.n)); got != c.want {
@@ -22,6 +24,47 @@ func TestValidPassword(t *testing.T) {
}
}
// TestPasswordHashRoundTrip: a password far past bcrypt's 72-byte input limit
// hashes and verifies, and — crucially — two passwords that differ only after
// byte 72 do NOT collide (raw bcrypt would treat them as equal; pre-hashing is
// exactly what prevents that).
func TestPasswordHashRoundTrip(t *testing.T) {
t.Parallel()
long := strings.Repeat("correct horse battery staple ", 10) // ~290 bytes
hash, err := hashPassword(long)
if err != nil {
t.Fatalf("hashPassword: %v", err)
}
if ok, legacy := comparePassword(hash, long); !ok || legacy {
t.Fatalf("comparePassword(correct) = (%v, %v), want (true, false)", ok, legacy)
}
if ok, _ := comparePassword(hash, long+"x"); ok {
t.Fatal("comparePassword accepted a wrong password")
}
a, _ := hashPassword(strings.Repeat("a", 72) + "1")
if ok, _ := comparePassword(a, strings.Repeat("a", 72)+"2"); ok {
t.Fatal("passwords differing only after byte 72 collided (pre-hash not applied?)")
}
}
// TestComparePasswordLegacy: a hash written the old way (raw bcrypt of the
// password) still verifies and is flagged legacy so the caller can upgrade it.
func TestComparePasswordLegacy(t *testing.T) {
t.Parallel()
const pw = "legacy-secret"
raw, err := bcrypt.GenerateFromPassword([]byte(pw), bcrypt.DefaultCost)
if err != nil {
t.Fatalf("bcrypt: %v", err)
}
if ok, legacy := comparePassword(string(raw), pw); !ok || !legacy {
t.Fatalf("legacy compare = (%v, %v), want (true, true)", ok, legacy)
}
if ok, _ := comparePassword(string(raw), "wrong"); ok {
t.Fatal("legacy compare accepted a wrong password")
}
}
func TestSafeLocalPath(t *testing.T) {
t.Parallel()
cases := []struct {
+47
View File
@@ -5,8 +5,11 @@ import (
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
"golang.org/x/crypto/bcrypt"
"github.com/jleight/meshtender/internal/store"
)
@@ -124,6 +127,50 @@ func TestPasskeyBeginDefersAccount(t *testing.T) {
}
}
// TestPasswordNoMaxLengthAndLegacyMigration: a password well past bcrypt's
// 72-byte limit works end to end (pre-hashing removes the ceiling), and a stored
// legacy raw-bcrypt hash still authenticates and is transparently upgraded to the
// pre-hash scheme on login.
func TestPasswordNoMaxLengthAndLegacyMigration(t *testing.T) {
t.Parallel()
st, ctx, ts, h := splitServer(t)
// A 200-char password: sign up, then sign in with it.
longPw := strings.Repeat("MeshTender-", 20) // 220 chars
up := post(t, ts, h.auth, "/signup/password", url.Values{"username": {"longpw"}, "password": {longPw}})
up.Body.Close()
if up.StatusCode != http.StatusSeeOther || cookieByName(up, "meshtender_session") == nil {
t.Fatalf("long-password signup = %d, want 303 + session", up.StatusCode)
}
in := post(t, ts, h.auth, "/login/password", url.Values{"username": {"longpw"}, "password": {longPw}})
in.Body.Close()
if in.StatusCode != http.StatusSeeOther || cookieByName(in, "meshtender_session") == nil {
t.Fatalf("long-password login = %d, want 303 + session", in.StatusCode)
}
// Plant a legacy (raw-bcrypt) hash and confirm login works and upgrades it.
legacyUser, err := st.CreateUser(ctx, "legacypw", "")
if err != nil {
t.Fatalf("create user: %v", err)
}
raw, _ := bcrypt.GenerateFromPassword([]byte("old-password"), bcrypt.DefaultCost)
if err := st.SetPassword(ctx, legacyUser.ID, string(raw)); err != nil {
t.Fatalf("set legacy hash: %v", err)
}
legacyLogin := post(t, ts, h.auth, "/login/password", url.Values{"username": {"legacypw"}, "password": {"old-password"}})
legacyLogin.Body.Close()
if legacyLogin.StatusCode != http.StatusSeeOther || cookieByName(legacyLogin, "meshtender_session") == nil {
t.Fatalf("legacy login = %d, want 303 + session", legacyLogin.StatusCode)
}
after, err := st.GetUserByUsername(ctx, "legacypw")
if err != nil {
t.Fatalf("reload user: %v", err)
}
if after.PasswordHash == nil || *after.PasswordHash == string(raw) {
t.Fatal("legacy hash was not upgraded on login")
}
}
// assertAccountOK asserts a 303 back to /account carrying a success ("ok") flash.
func assertAccountOK(t *testing.T, resp *http.Response, label string) {
t.Helper()