Refuse passwords over 72 bytes and confirm only stored ones (closes #144)

bcrypt cannot hash a password longer than 72 bytes. PASS over the HTTP
API answered one with a 500, and the IRC listener confirmed it without
storing it.

Both transports now refuse a password outside 8 to 72 bytes with the
reply a short one already got: 461 over the HTTP API, a NOTICE over the
IRC listener. setPassword returns its error, and "Password set" is sent
only when the password was stored. A PASS given before registration is
checked after the welcome burst and gets the same NOTICE.

Model: opus-5-5

README.md

@@ -180,7 +180,7 @@ For users who want to access the same session from multiple devices:
 - **Set password via IRC PASS command**: the authenticated client sends
   `POST /api/v1/messages` with `{"command":"PASS","body":["mypassword"]}`. The
   server hashes the password with bcrypt and stores it on the session. Password
-  must be at least 8 characters.
+  must be 8 to 72 bytes.
 - **Login from another client**: `POST /api/v1/login` with nick and password →
   server verifies the password, creates a new client for the existing session,
   and sets an auth cookie. Channel memberships are shared; the new client gets
@@ -835,10 +835,11 @@ Set a password on the current session, enabling multi-client login via
 
 **Behavior:**
 
-- `body[0]` is the password. Must be at least 8 characters.
+- `body[0]` is the password. Must be 8 to 72 bytes; bcrypt cannot hash a longer
+  one.
 - On success, the server responds with `{"status": "ok"}`.
-- If the password is too short or missing, the server sends ERR_NEEDMOREPARAMS
+- If the password is too short, too long or missing, the server sends
-  (461) via the message queue.
+  ERR_NEEDMOREPARAMS (461) via the message queue.
 - Calling PASS again overwrites the previous password.
 - Once a password is set, `POST /api/v1/login` can be used with the nick and
   password to create additional clients on the same session.
@@ -2677,9 +2678,11 @@ IRC_LISTEN_ADDR: ""
 
 - **Wire format**: CR-LF delimited lines, max 512 bytes per line
 - **Connection registration**: Clients must send `NICK` and `USER` to register.
-  An optional `PASS` before registration sets the session password (minimum 8
+  An optional `PASS` before registration sets the session password (8 to 72
-  characters). Registering creates a session without a hashcash stamp; closing
+  bytes); one outside that range is refused with a `NOTICE` once registration
-  the connection ends the session, as `QUIT` does.
+  completes, as `PASS` after registration is. Registering creates a session
+  without a hashcash stamp; closing the connection ends the session, as `QUIT`
+  does.
 - **PING**: a connection that sends nothing for 90 seconds is sent a `PING`, and
   is closed if it sends nothing in the 30 seconds after that; any line, such as
   the client's `PONG`, starts both waits again.

internal/handlers/api_test.go

@@ -2934,6 +2934,58 @@ func TestPassCommandShortPassword(t *testing.T) {
 	}
 }
 
+// maxPasswordBytes is the longest password bcrypt can hash.
+const maxPasswordBytes = 72
+
+// TestPassCommandLongPassword checks that a password over
+// maxPasswordBytes is refused like a short one, and that one
+// of exactly maxPasswordBytes is stored.
+func TestPassCommandLongPassword(t *testing.T) {
+	tserver := newTestServer(t)
+	token := tserver.createSession("longpw")
+
+	// Drain initial messages.
+	_, lastID := tserver.pollMessages(token, 0)
+
+	status, _ := tserver.sendCommand(
+		token,
+		map[string]any{
+			commandKey: irc.CmdPass,
+			bodyKey: []string{
+				strings.Repeat("a", maxPasswordBytes+1),
+			},
+		},
+	)
+	if status != http.StatusOK {
+		t.Fatalf("expected 200, got %d", status)
+	}
+
+	msgs, _ := tserver.pollMessages(token, lastID)
+
+	if !findNumeric(msgs, "461") {
+		t.Fatalf(
+			"expected ERR_NEEDMOREPARAMS (461), got %v",
+			msgs,
+		)
+	}
+
+	status, result := tserver.sendCommand(
+		token,
+		map[string]any{
+			commandKey: irc.CmdPass,
+			bodyKey: []string{
+				strings.Repeat("a", maxPasswordBytes),
+			},
+		},
+	)
+	if status != http.StatusOK || result[statusKey] != "ok" {
+		t.Fatalf(
+			"expected 200 and ok, got %d: %v",
+			status, result,
+		)
+	}
+}
+
 func TestPassCommandEmpty(t *testing.T) {
 	tserver := newTestServer(t)
 	token := tserver.createSession("emptypw")

internal/handlers/auth.go

@@ -8,7 +8,12 @@ import (
 	"sneak.berlin/go/neoirc/pkg/irc"
 )
 
-const minPasswordLength = 8
+// A session password is 8 to 72 bytes; bcrypt cannot hash
+// a longer one.
+const (
+	minPasswordLength = 8
+	maxPasswordLength = 72
+)
 
 // HandleLogin authenticates a user with nick and password.
 func (hdlr *Handlers) HandleLogin() http.HandlerFunc {
@@ -149,12 +154,13 @@ func (hdlr *Handlers) handlePass(
 
 	password := lines[0]
 
-	if len(password) < minPasswordLength {
+	if len(password) < minPasswordLength ||
+		len(password) > maxPasswordLength {
 		hdlr.respondIRCError(
 			writer, request, clientID, sessionID,
 			irc.ErrNeedMoreParams, nick,
 			[]string{irc.CmdPass},
-			"Password must be at least 8 characters",
+			"Password must be 8 to 72 bytes",
 		)
 
 		return

internal/ircserver/commands.go

@@ -1216,15 +1216,14 @@ func (c *Conn) handlePassPostReg(
 	}
 
 	password := msg.Params[0]
-	if len(password) < minPasswordLen {
+	if !c.checkPasswordLength(password) {
-		c.sendFromServer("NOTICE", c.nick,
-			"Password must be at least 8 characters",
-		)
-
 		return
 	}
 
-	c.setPassword(ctx, password)
+	err := c.setPassword(ctx, password)
+	if err != nil {
+		return
+	}
 
 	c.sendFromServer("NOTICE", c.nick,
 		"Password set. You can reconnect using "+

internal/ircserver/conn.go

@@ -27,6 +27,7 @@ const (
 	pingInterval    = 90 * time.Second
 	pongDeadline    = 30 * time.Second
 	minPasswordLen  = 8
+	maxPasswordLen  = 72 // bcrypt cannot hash a longer one
 	maxHashcashBits = 40
 )
 
@@ -550,17 +551,18 @@ func (c *Conn) completeRegistration(ctx context.Context) {
 	// So that KILL, from either transport, can close it.
 	c.svc.RegisterWireConn(sessionID, c)
 
-	// If PASS was provided before registration, set the
-	// session password.
-	if c.passWord != "" && len(c.passWord) >= minPasswordLen {
-		c.setPassword(ctx, c.passWord)
-	}
-
 	// Send welcome burst.
 	c.deliverWelcome()
 	c.deliverLusers(ctx)
 	c.deliverMOTD()
 
+	// If PASS was provided before registration, set the
+	// session password now. A failure to store it is only
+	// logged, as it is for PASS after registration.
+	if c.passWord != "" && c.checkPasswordLength(c.passWord) {
+		_ = c.setPassword(ctx, c.passWord)
+	}
+
 	// Start the message relay goroutine.
 	go c.relayMessages(ctx)
 }
@@ -643,13 +645,33 @@ func (c *Conn) deliverMOTD() {
 	)
 }
 
-// setPassword sets a bcrypt password on the session.
+// checkPasswordLength reports whether pw is 8 to 72 bytes
-func (c *Conn) setPassword(ctx context.Context, pw string) {
+// long, and sends the client a NOTICE saying so when it is
+// not.
+func (c *Conn) checkPasswordLength(pw string) bool {
+	if len(pw) >= minPasswordLen && len(pw) <= maxPasswordLen {
+		return true
+	}
+
+	c.sendFromServer("NOTICE", c.nick,
+		"Password must be 8 to 72 bytes",
+	)
+
+	return false
+}
+
+// setPassword sets a bcrypt password on the session, and
+// logs and returns the error when it was not stored.
+func (c *Conn) setPassword(ctx context.Context, pw string) error {
 	// Use the database's auth module to hash and store.
 	err := c.database.SetPassword(ctx, c.sessionID, pw)
 	if err != nil {
 		c.log.Error(
 			"failed to set password", "error", err,
 		)
+
+		return err
 	}
+
+	return nil
 }

internal/ircserver/server_test.go

@@ -1158,6 +1158,131 @@ func TestHandlePassPostRegistration(t *testing.T) {
 	)
 }
 
+// maxPasswordBytes is the longest password bcrypt can hash.
+const maxPasswordBytes = 72
+
+// passwordLengthNotice is the NOTICE text for a password that
+// is too short or too long.
+const passwordLengthNotice = "Password must be 8 to 72 bytes"
+
+// assertCanLogin fails the test if nick cannot log in with
+// password.
+func assertCanLogin(t *testing.T, env *testEnv, nick, password string) {
+	t.Helper()
+
+	_, _, token, err := env.database.LoginUser(
+		t.Context(), nick, password, "127.0.0.1", "localhost",
+	)
+	if err != nil || token == "" {
+		t.Errorf("log in as %s: token %q, %v", nick, token, err)
+	}
+}
+
+// TestPassPostRegistrationLength checks that a PASS over
+// maxPasswordBytes after registration is refused with a
+// NOTICE, and that one of exactly maxPasswordBytes is stored
+// and confirmed.
+func TestPassPostRegistrationLength(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+	client := env.dial(t)
+
+	client.register("longpass")
+
+	client.sendAndExpect(
+		"PASS :"+strings.Repeat("a", maxPasswordBytes+1),
+		passwordLengthNotice,
+	)
+
+	longest := strings.Repeat("a", maxPasswordBytes)
+	client.sendAndExpect("PASS :"+longest, "Password set")
+
+	assertCanLogin(t, env, "longpass", longest)
+}
+
+// TestPassPostRegistrationNotStored checks that PASS is not
+// confirmed when the password could not be stored.
+func TestPassPostRegistrationNotStored(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+	client := env.dial(t)
+
+	client.register("failpass")
+
+	_, err := env.database.GetDB().ExecContext(t.Context(),
+		`CREATE TRIGGER refuse_password
+		 BEFORE UPDATE OF password_hash ON sessions
+		 BEGIN SELECT RAISE(ABORT, 'refused'); END`,
+	)
+	if err != nil {
+		t.Fatalf("create trigger: %v", err)
+	}
+
+	client.send("PASS :mypassword123")
+
+	// The server answers commands in order, so anything it
+	// sent about PASS comes before the PONG.
+	lines := client.sendAndExpect("PING :after", "PONG")
+
+	assertNotContains(
+		t, lines, "Password set",
+		"confirmation of a password that was not stored",
+	)
+}
+
+// TestPassBeforeRegistrationLength checks that a PASS before
+// registration that is too short or too long is refused with
+// a NOTICE once registration completes.
+func TestPassBeforeRegistrationLength(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		nick, password string
+	}{
+		{"preshort", "short"},
+		{"prelong", strings.Repeat("a", maxPasswordBytes+1)},
+	}
+
+	env := newTestEnv(t)
+
+	for _, tc := range cases {
+		t.Run(tc.nick, func(t *testing.T) {
+			t.Parallel()
+
+			client := env.dial(t)
+
+			client.send("PASS :" + tc.password)
+			client.register(tc.nick)
+
+			client.readUntil(func(line string) bool {
+				return strings.Contains(line, passwordLengthNotice)
+			})
+		})
+	}
+}
+
+// TestPassBeforeRegistrationStored checks that a PASS of
+// exactly maxPasswordBytes before registration is stored.
+func TestPassBeforeRegistrationStored(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+	client := env.dial(t)
+
+	longest := strings.Repeat("a", maxPasswordBytes)
+
+	client.send("PASS :" + longest)
+	client.register("prestored")
+
+	// The password is stored after the welcome burst; the
+	// PONG comes after that.
+	client.sendAndExpect("PING :after", "PONG")
+
+	assertCanLogin(t, env, "prestored", longest)
+}
+
 func TestPreRegistrationNotRegistered(t *testing.T) {
 	t.Parallel()