diff --git a/CHANGELOG.md b/CHANGELOG.md
index 029478e..eedae6f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -126,6 +126,16 @@ APIs change between minors without a deprecation period.
agents' long-lived streams); the client port holds at most
`max_connections` (10,000) at once, and a message from an agent at most
16 MB.
+- **The sign-in and signup forms resist scripts.** Ten wrong passwords from
+ anywhere locked an address's owner out for the window; attempts are now
+ counted per address and client, so somebody guessing elsewhere cannot lock
+ the owner out, with a looser count per address alone so guesses spread
+ over many clients are still bounded. The sign-in form carries a token its
+ cookie matches, so another site cannot sign a visitor into an account of
+ its choosing. Posting either form is rationed per client address (sign-in
+ 30 a minute, signup 5), and a password hash that cannot get a turn within
+ ten seconds answers "busy" instead of holding the request. `server init`
+ now writes `signup_open: false`.
### Everything that can be changed on a running cluster, from the console
diff --git a/internal/account/account_test.go b/internal/account/account_test.go
index 9dfa8d3..36ca51d 100644
--- a/internal/account/account_test.go
+++ b/internal/account/account_test.go
@@ -398,3 +398,53 @@ func TestGuessingTheCurrentPasswordIsBounded(t *testing.T) {
t.Errorf("guessing was unbounded: %v", err)
}
}
+
+func TestSomebodyGuessingElsewhereCannotLockTheOwnerOut(t *testing.T) {
+ svc, st := newStoreAndUser(t)
+ ctx := context.Background()
+ must(t, st.ApproveUser(ctx, "usr-1", "proj", "ilker", time.Now()))
+ cfg := DefaultConfig()
+ cfg.MaxAttempts = 3
+ svc = New(st, cfg, slog.New(slog.DiscardHandler))
+
+ attacker, owner := WithClient(ctx, "203.0.113.9"), WithClient(ctx, "198.51.100.7")
+ for range 5 {
+ _, _, _ = svc.SignIn(attacker, "ayse@example.com", "nope")
+ }
+ if _, _, err := svc.SignIn(attacker, "ayse@example.com", "nope"); !errors.Is(err, ErrTooManyTries) {
+ t.Errorf("the guessing client was not held back: %v", err)
+ }
+ if _, _, err := svc.SignIn(owner, "ayse@example.com", goodPassword); err != nil {
+ t.Errorf("the owner, elsewhere, was locked out: %v", err)
+ }
+ // Spread over many clients, guessing is still bounded per address.
+ held := false
+ for i := range 40 {
+ c := WithClient(ctx, "192.0.2."+strings.Repeat("1", 1+i%3)+string(rune('0'+i%10)))
+ if _, _, err := svc.SignIn(c, "ayse@example.com", "nope"); errors.Is(err, ErrTooManyTries) {
+ held = true
+ }
+ }
+ if !held {
+ t.Error("guesses from many clients were not bounded")
+ }
+}
+
+func TestAHashThatCannotGetATurnSaysTheServerIsBusy(t *testing.T) {
+ was := argonWait
+ argonWait = 10 * time.Millisecond
+ t.Cleanup(func() { argonWait = was })
+ // Every turn taken.
+ n := cap(argonSlots)
+ for range n {
+ argonSlots <- struct{}{}
+ }
+ t.Cleanup(func() {
+ for range n {
+ <-argonSlots
+ }
+ })
+ if _, err := HashPassword(goodPassword); !errors.Is(err, ErrBusy) {
+ t.Errorf("a hash with no turn: %v", err)
+ }
+}
diff --git a/internal/account/password.go b/internal/account/password.go
index f19be13..5cc8e64 100644
--- a/internal/account/password.go
+++ b/internal/account/password.go
@@ -17,6 +17,7 @@ import (
"fmt"
"runtime"
"strings"
+ "time"
"golang.org/x/crypto/argon2"
)
@@ -57,7 +58,10 @@ func HashPassword(password string) (string, error) {
if _, err := rand.Read(salt); err != nil {
return "", err
}
- sum := idKey([]byte(password), salt, argonTime, argonMemory, argonThreads, argonKeyLen)
+ sum, err := idKey([]byte(password), salt, argonTime, argonMemory, argonThreads, argonKeyLen)
+ if err != nil {
+ return "", err
+ }
b64 := base64.RawStdEncoding.EncodeToString
return fmt.Sprintf("$argon2id$v=%d$m=%d,t=%d,p=%d$%s$%s",
@@ -94,7 +98,10 @@ func VerifyPassword(password, encoded string) (bool, error) {
return false, ErrBadHash
}
- got := idKey([]byte(password), salt, time, memory, threads, uint32(len(want)))
+ got, err := idKey([]byte(password), salt, time, memory, threads, uint32(len(want)))
+ if err != nil {
+ return false, err
+ }
return subtle.ConstantTimeCompare(got, want) == 1, nil
}
@@ -119,8 +126,19 @@ func init() {
// Beyond the bound, attempts wait their turn instead.
var argonSlots = make(chan struct{}, max(2, runtime.NumCPU()/2))
-func idKey(password, salt []byte, time, memory uint32, threads uint8, keyLen uint32) []byte {
- argonSlots <- struct{}{}
+// ErrBusy is a hash that waited its turn too long: the server is answering
+// more sign-ins than it can hash, and says so rather than hold the request.
+var ErrBusy = errors.New("too many sign-ins at once; try again in a moment")
+
+// argonWait bounds the wait for a turn; a variable so a test can shorten it.
+var argonWait = 10 * time.Second
+
+func idKey(password, salt []byte, t, memory uint32, threads uint8, keyLen uint32) ([]byte, error) {
+ select {
+ case argonSlots <- struct{}{}:
+ case <-time.After(argonWait):
+ return nil, ErrBusy
+ }
defer func() { <-argonSlots }()
- return argon2.IDKey(password, salt, time, memory, threads, keyLen)
+ return argon2.IDKey(password, salt, t, memory, threads, keyLen), nil
}
diff --git a/internal/account/service.go b/internal/account/service.go
index 2c5e345..20ea1e0 100644
--- a/internal/account/service.go
+++ b/internal/account/service.go
@@ -138,7 +138,15 @@ func (s *Service) SignUp(ctx context.Context, email, name, password string) erro
// the CSRF token that must accompany the forms in it.
func (s *Service) SignIn(ctx context.Context, email, password string) (token, csrf string, err error) {
email = store.NormalizeEmail(email)
- if !s.allowAttempt(email) {
+ // Counted per address and the client trying it, so somebody guessing
+ // from elsewhere cannot lock the owner out of their own account; and per
+ // address alone, more loosely, so guesses spread over many clients are
+ // still bounded.
+ byClient := email
+ if c := clientFrom(ctx); c != "" {
+ byClient = email + "|" + c
+ }
+ if !s.allowAttempt(byClient, s.cfg.MaxAttempts) || !s.allowAttempt("all|"+email, 10*s.cfg.MaxAttempts) {
return "", "", ErrTooManyTries
}
@@ -154,11 +162,15 @@ func (s *Service) SignIn(ctx context.Context, email, password string) (token, cs
}
ok, err := VerifyPassword(password, u.PasswordHash)
+ if errors.Is(err, ErrBusy) {
+ // Not an answer about the password: said as what it is.
+ return "", "", err
+ }
if err != nil || !ok {
return "", "", ErrBadCredentials
}
// The password was right, so this was not an attack.
- s.clearAttempts(email)
+ s.clearAttempts(byClient)
// State is checked after the password, on purpose. Checking first would
// tell anyone who guesses an address whether it has a pending account.
@@ -224,7 +236,7 @@ func (s *Service) CheckCSRF(sess store.Session, submitted string) bool {
// Per address rather than per IP: an attacker has many addresses to come from
// and only one to guess against, and limiting by IP would lock out everyone
// behind one office NAT the moment somebody fat-fingers their password.
-func (s *Service) allowAttempt(email string) bool {
+func (s *Service) allowAttempt(email string, limit int) bool {
now := s.now()
s.mu.Lock()
defer s.mu.Unlock()
@@ -251,7 +263,20 @@ func (s *Service) allowAttempt(email string) bool {
return true
}
w.count++
- return w.count <= s.cfg.MaxAttempts
+ return w.count <= limit
+}
+
+type clientKey struct{}
+
+// WithClient says which client a request comes from — its address — for the
+// sign-in throttle.
+func WithClient(ctx context.Context, client string) context.Context {
+ return context.WithValue(ctx, clientKey{}, client)
+}
+
+func clientFrom(ctx context.Context) string {
+ c, _ := ctx.Value(clientKey{}).(string)
+ return c
}
// maxTrackedAddresses bounds the throttle's memory. Unbounded, it grew by one
@@ -307,7 +332,7 @@ type PasswordChanger interface {
func (s *Service) ChangePassword(ctx context.Context, pw PasswordChanger,
u store.User, current, next string,
) error {
- if !s.allowAttempt("pw:" + u.ID) {
+ if !s.allowAttempt("pw:"+u.ID, s.cfg.MaxAttempts) {
return ErrTooManyTries
}
ok, err := VerifyPassword(current, u.PasswordHash)
diff --git a/internal/account/throttle_bound_test.go b/internal/account/throttle_bound_test.go
index 11796b1..3a0003e 100644
--- a/internal/account/throttle_bound_test.go
+++ b/internal/account/throttle_bound_test.go
@@ -12,22 +12,22 @@ func TestSprayingAddressesCannotGrowTheThrottleOrEscapeIt(t *testing.T) {
// The target is being guessed at.
for i := 0; i < s.cfg.MaxAttempts; i++ {
- s.allowAttempt("victim@example.com")
+ s.allowAttempt("victim@example.com", s.cfg.MaxAttempts)
}
// Then the table is flooded with made-up addresses.
for i := 0; i < maxTrackedAddresses+100; i++ {
- s.allowAttempt(fmt.Sprintf("x%d@example.com", i))
+ s.allowAttempt(fmt.Sprintf("x%d@example.com", i), s.cfg.MaxAttempts)
}
if n := len(s.attempts); n > maxTrackedAddresses {
t.Errorf("the table grew to %d", n)
}
- if s.allowAttempt("victim@example.com") {
+ if s.allowAttempt("victim@example.com", s.cfg.MaxAttempts) {
t.Error("flooding the table reset the target's throttle")
}
// Once the window passes, everything expires and sign-ins work again.
now = now.Add(s.cfg.AttemptWindow + time.Second)
- if !s.allowAttempt("new@example.com") {
+ if !s.allowAttempt("new@example.com", s.cfg.MaxAttempts) {
t.Error("a new address was refused after the flood expired")
}
}
diff --git a/internal/cli/serverinit.go b/internal/cli/serverinit.go
index 408a183..08d931a 100644
--- a/internal/cli/serverinit.go
+++ b/internal/cli/serverinit.go
@@ -480,7 +480,9 @@ control:
auth:
enabled: true
store: "%s" # a SQLite path, or @file holding a PostgreSQL address
- signup_open: true
+ # The signup form: accounts it makes wait for an administrator's approval,
+ # but anyone reaching port 8080 can make them. Off until you want it.
+ signup_open: false
# Applied to any key no scope gives a limit to. Without them, a project
# created without limits is metered by nothing at all.
default_rpm: 60
diff --git a/internal/web/account.go b/internal/web/account.go
index 60c36b3..11b85bd 100644
--- a/internal/web/account.go
+++ b/internal/web/account.go
@@ -135,10 +135,12 @@ func NewAccountPages(a Accounts, k KeyOwner, pw account.PasswordChanger,
func (p *AccountPages) Register(mux *http.ServeMux) {
if p.accounts.SignupOpen() {
mux.HandleFunc("GET /signup", p.signupForm)
- mux.HandleFunc("POST /signup", p.signup)
+ // Each signup costs a password hash and a row: a few a minute from
+ // one address is a person, more is a script.
+ mux.HandleFunc("POST /signup", rationed(newRation(5, 10), p.signup))
}
mux.HandleFunc("GET /login", p.loginForm)
- mux.HandleFunc("POST /login", p.login)
+ mux.HandleFunc("POST /login", rationed(newRation(30, 30), p.login))
mux.HandleFunc("POST /logout", p.logout)
mux.HandleFunc("GET /profile", p.profile)
mux.HandleFunc("POST /profile/keys", p.createKey)
@@ -196,25 +198,37 @@ type loginData struct {
Email, Error string
SignupOpen bool
Version string
+ // CSRF is the form's token, matched by a cookie.
+ CSRF string
}
-func (p *AccountPages) loginForm(w http.ResponseWriter, _ *http.Request) {
+func (p *AccountPages) loginForm(w http.ResponseWriter, r *http.Request) {
noStore(w)
- render(w, "login", loginData{SignupOpen: p.accounts.SignupOpen(), Version: p.version})
+ render(w, "login", loginData{SignupOpen: p.accounts.SignupOpen(), Version: p.version, CSRF: setLoginToken(w, r)})
}
func (p *AccountPages) login(w http.ResponseWriter, r *http.Request) {
noStore(w)
email := strings.TrimSpace(r.FormValue("email"))
+ if !loginTokenOK(r) {
+ // Not this site's form: another one posting a visitor into an
+ // account of its choosing, or a form left open past its hour.
+ w.WriteHeader(http.StatusBadRequest)
+ render(w, "login", loginData{Email: email, SignupOpen: p.accounts.SignupOpen(), Version: p.version,
+ Error: "the form had expired; sign in again", CSRF: setLoginToken(w, r)})
+ return
+ }
- token, csrf, err := p.accounts.SignIn(r.Context(), email, r.FormValue("password"))
+ ctx := account.WithClient(r.Context(), clientAddr(r))
+ token, csrf, err := p.accounts.SignIn(ctx, email, r.FormValue("password"))
if err != nil {
- d := loginData{Email: email, SignupOpen: p.accounts.SignupOpen(), Version: p.version}
+ d := loginData{Email: email, SignupOpen: p.accounts.SignupOpen(), Version: p.version, CSRF: setLoginToken(w, r)}
switch {
case errors.Is(err, account.ErrBadCredentials),
errors.Is(err, account.ErrNotApproved),
errors.Is(err, account.ErrSuspended),
- errors.Is(err, account.ErrTooManyTries):
+ errors.Is(err, account.ErrTooManyTries),
+ errors.Is(err, account.ErrBusy):
d.Error = err.Error()
default:
p.log.Error("sign-in failed", "err", err)
diff --git a/internal/web/formguard.go b/internal/web/formguard.go
new file mode 100644
index 0000000..a47aad4
--- /dev/null
+++ b/internal/web/formguard.go
@@ -0,0 +1,111 @@
+package web
+
+import (
+ "crypto/rand"
+ "crypto/subtle"
+ "encoding/hex"
+ "net"
+ "net/http"
+ "sync"
+ "time"
+)
+
+// The sign-in and signup forms are the two a browser posts before it has a
+// session: nothing ties them to the person in front of it, and nothing slows
+// a script posting them. Two guards: a token the form carries and a cookie
+// matches, so another site cannot sign a visitor into an account of its
+// choosing; and a ration per client address, so a script cannot spend the
+// password hasher, the database or an address's attempts at the rate it can
+// send.
+
+// loginCSRFCookie is the sign-in form's token, before there is a session.
+const loginCSRFCookie = "llmgw_login_csrf"
+
+// newFormToken is a token for one form.
+func newFormToken() string {
+ b := make([]byte, 16)
+ _, _ = rand.Read(b)
+ return hex.EncodeToString(b)
+}
+
+// setLoginToken gives the sign-in form its token and the browser its match.
+func setLoginToken(w http.ResponseWriter, r *http.Request) string {
+ t := newFormToken()
+ http.SetCookie(w, &http.Cookie{
+ Name: loginCSRFCookie, Value: t, Path: "/login",
+ HttpOnly: true, Secure: isHTTPS(r), SameSite: http.SameSiteStrictMode, MaxAge: 3600,
+ })
+ return t
+}
+
+// loginTokenOK reports whether a posted sign-in form carries the token its
+// cookie does.
+func loginTokenOK(r *http.Request) bool {
+ c, err := r.Cookie(loginCSRFCookie)
+ if err != nil || c.Value == "" {
+ return false
+ }
+ return subtle.ConstantTimeCompare([]byte(c.Value), []byte(r.FormValue("csrf"))) == 1
+}
+
+// ration is a token bucket per client address.
+type ration struct {
+ mu sync.Mutex
+ rate float64 // a second
+ burst float64
+ by map[string]*allowance
+}
+
+type allowance struct {
+ tokens float64
+ at time.Time
+}
+
+func newRation(perMinute, burst float64) *ration {
+ return &ration{rate: perMinute / 60, burst: burst, by: map[string]*allowance{}}
+}
+
+func (q *ration) allow(client string, now time.Time) bool {
+ q.mu.Lock()
+ defer q.mu.Unlock()
+ if len(q.by) > 50_000 {
+ for k, v := range q.by {
+ if now.Sub(v.at).Seconds()*q.rate >= q.burst {
+ delete(q.by, k)
+ }
+ }
+ }
+ a, ok := q.by[client]
+ if !ok {
+ a = &allowance{tokens: q.burst, at: now}
+ q.by[client] = a
+ }
+ a.tokens = min(q.burst, a.tokens+now.Sub(a.at).Seconds()*q.rate)
+ a.at = now
+ if a.tokens < 1 {
+ return false
+ }
+ a.tokens--
+ return true
+}
+
+// rationed refuses a client past its ration with 429 before next runs.
+func rationed(q *ration, next http.HandlerFunc) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ if !q.allow(clientAddr(r), time.Now()) {
+ w.Header().Set("Retry-After", "60")
+ http.Error(w, "too many attempts from this address; wait a minute", http.StatusTooManyRequests)
+ return
+ }
+ next(w, r)
+ }
+}
+
+// clientAddr is the address a request came from, without its port.
+func clientAddr(r *http.Request) string {
+ host, _, err := net.SplitHostPort(r.RemoteAddr)
+ if err != nil {
+ return r.RemoteAddr
+ }
+ return host
+}
diff --git a/internal/web/signup_test.go b/internal/web/signup_test.go
index 886363f..8370688 100644
--- a/internal/web/signup_test.go
+++ b/internal/web/signup_test.go
@@ -52,3 +52,48 @@ func TestASignupSaysWhatToDoIfTheAddressHadAnAccount(t *testing.T) {
t.Errorf("the done page leaves someone who already had an account stuck: %s", body)
}
}
+
+func TestTheSignInFormWantsItsOwnTokenAndRationsAnAddress(t *testing.T) {
+ mux := http.NewServeMux()
+ NewAccountPages(signupAccounts{}, nil, nil, "v0", slog.New(slog.DiscardHandler)).Register(mux)
+ post := func(form url.Values, cookie *http.Cookie) *httptest.ResponseRecorder {
+ req := httptest.NewRequest(http.MethodPost, "/login", strings.NewReader(form.Encode()))
+ req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+ req.RemoteAddr = "203.0.113.9:1234"
+ if cookie != nil {
+ req.AddCookie(cookie)
+ }
+ rec := httptest.NewRecorder()
+ mux.ServeHTTP(rec, req)
+ return rec
+ }
+ // No token: another site's form.
+ if rec := post(url.Values{"email": {"a@b.c"}, "password": {"x"}}, nil); rec.Code != http.StatusBadRequest {
+ t.Errorf("a sign-in without the form's token: %d", rec.Code)
+ }
+ // The form's token, matched by its cookie, reaches the sign-in.
+ rec := httptest.NewRecorder()
+ mux.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/login", nil))
+ var cookie *http.Cookie
+ for _, c := range rec.Result().Cookies() {
+ if c.Name == loginCSRFCookie {
+ cookie = c
+ }
+ }
+ if cookie == nil || !strings.Contains(rec.Body.String(), cookie.Value) {
+ t.Fatal("the form carries no token")
+ }
+ if rec := post(url.Values{"email": {"a@b.c"}, "password": {"x"}, "csrf": {cookie.Value}}, cookie); rec.Code == http.StatusBadRequest {
+ t.Errorf("the form's own token was refused")
+ }
+ // A script posting it is rationed.
+ limited := false
+ for range 40 {
+ if post(url.Values{"csrf": {cookie.Value}}, cookie).Code == http.StatusTooManyRequests {
+ limited = true
+ }
+ }
+ if !limited {
+ t.Error("forty sign-ins a moment from one address were all taken")
+ }
+}
diff --git a/internal/web/templates/account.html b/internal/web/templates/account.html
index acd9cab..9d023cb 100644
--- a/internal/web/templates/account.html
+++ b/internal/web/templates/account.html
@@ -41,6 +41,7 @@ an operator to reset it.
{{if .Error}}{{.Error}}
{{end}}