From c267cd215ce3e03e1d01cba006061ebc7f068a76 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Wed, 21 Oct 2026 05:02:00 +0200 Subject: [PATCH] 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, an empty one included, is checked after the welcome burst and gets the same NOTICE. Model: opus-5-5 --- README.md | 17 ++-- internal/handlers/api_test.go | 85 +++++++++++----- internal/handlers/auth.go | 12 ++- internal/ircserver/commands.go | 11 +-- internal/ircserver/conn.go | 42 ++++++-- internal/ircserver/server_test.go | 159 ++++++++++++++++++++++++++++++ 6 files changed, 280 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index ebd8708..b94af90 100644 --- a/README.md +++ b/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 - (461) via the message queue. +- If the password is too short, too long or missing, the server sends + 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. @@ -2678,9 +2679,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 - characters). Registering creates a session without a hashcash stamp; closing - the connection ends the session, as `QUIT` does. + An optional `PASS` before registration sets the session password (8 to 72 + bytes); one outside that range is refused with a `NOTICE` once registration + 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. diff --git a/internal/handlers/api_test.go b/internal/handlers/api_test.go index 05b8603..060f3dd 100644 --- a/internal/handlers/api_test.go +++ b/internal/handlers/api_test.go @@ -2906,32 +2906,73 @@ func TestPassCommand(t *testing.T) { } } -func TestPassCommandShortPassword(t *testing.T) { - tserver := newTestServer(t) - token := tserver.createSession("shortpw") +// The shortest and longest password PASS accepts; bcrypt +// cannot hash more than 72 bytes. +const ( + minPasswordBytes = 8 + maxPasswordBytes = 72 +) - // Drain initial messages. - _, lastID := tserver.pollMessages(token, 0) - - // Try short password — should fail. - status, _ := tserver.sendCommand( - token, - map[string]any{ - commandKey: irc.CmdPass, - bodyKey: []string{"short"}, - }, - ) - if status != http.StatusOK { - t.Fatalf("expected 200, got %d", status) +// TestPassCommandLength checks that PASS stores a password of +// 8 to 72 bytes and refuses any other with 461. The limit +// counts bytes: "é" is two bytes in UTF-8, so 72 of them are +// refused. +func TestPassCommandLength(t *testing.T) { + cases := []struct { + name string + password string + stored bool + }{ + {"empty", "", false}, + {"7 bytes", strings.Repeat("a", minPasswordBytes-1), false}, + {"8 bytes", strings.Repeat("a", minPasswordBytes), true}, + {"72 bytes", strings.Repeat("a", maxPasswordBytes), true}, + {"73 bytes", strings.Repeat("a", maxPasswordBytes+1), false}, + {"72 characters", strings.Repeat("é", maxPasswordBytes), false}, } - msgs, _ := tserver.pollMessages(token, lastID) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + tserver := newTestServer(t) + token := tserver.createSession("pwlength") - if !findNumeric(msgs, "461") { - t.Fatalf( - "expected ERR_NEEDMOREPARAMS (461), got %v", - msgs, - ) + _, lastID := tserver.pollMessages(token, 0) + + status, result := tserver.sendCommand( + token, + map[string]any{ + commandKey: irc.CmdPass, + bodyKey: []string{tc.password}, + }, + ) + if status != http.StatusOK { + t.Fatalf("expected 200, got %d: %v", status, result) + } + + if !tc.stored { + msgs, _ := tserver.pollMessages(token, lastID) + if !findNumeric(msgs, "461") { + t.Fatalf( + "expected ERR_NEEDMOREPARAMS (461), got %v", + msgs, + ) + } + + return + } + + if result[statusKey] != "ok" { + t.Fatalf("expected ok, got %v", result) + } + + postJSONExpectStatus( + t, tserver, "/api/v1/login", + map[string]string{ + nickKey: "pwlength", passwordKey: tc.password, + }, + http.StatusOK, + ) + }) } } diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go index 7104c13..21d9bc6 100644 --- a/internal/handlers/auth.go +++ b/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 diff --git a/internal/ircserver/commands.go b/internal/ircserver/commands.go index 960dac7..77d1eb3 100644 --- a/internal/ircserver/commands.go +++ b/internal/ircserver/commands.go @@ -1216,15 +1216,14 @@ func (c *Conn) handlePassPostReg( } password := msg.Params[0] - if len(password) < minPasswordLen { - c.sendFromServer("NOTICE", c.nick, - "Password must be at least 8 characters", - ) - + if !c.checkPasswordLength(password) { 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 "+ diff --git a/internal/ircserver/conn.go b/internal/ircserver/conn.go index bf7c9f4..dbbd0e5 100644 --- a/internal/ircserver/conn.go +++ b/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 ) @@ -62,6 +63,7 @@ type Conn struct { registered bool gotNick bool gotUser bool + gotPass bool passWord string lastQueueID int64 @@ -436,6 +438,7 @@ func (c *Conn) handlePreRegistration( } c.passWord = msg.Params[0] + c.gotPass = true case irc.CmdNick: if len(msg.Params) < 1 { c.sendNumeric( @@ -550,17 +553,20 @@ 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 given before registration, store its + // password now, after the welcome burst, so that a + // NOTICE refusing it reaches a registered client. A + // failure to store it is only logged, as it is for PASS + // after registration. + if c.gotPass && c.checkPasswordLength(c.passWord) { + _ = c.setPassword(ctx, c.passWord) + } + // Start the message relay goroutine. go c.relayMessages(ctx) } @@ -643,13 +649,33 @@ func (c *Conn) deliverMOTD() { ) } -// setPassword sets a bcrypt password on the session. -func (c *Conn) setPassword(ctx context.Context, pw string) { +// checkPasswordLength reports whether pw is 8 to 72 bytes +// 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 } diff --git a/internal/ircserver/server_test.go b/internal/ircserver/server_test.go index b5970af..3b7eabf 100644 --- a/internal/ircserver/server_test.go +++ b/internal/ircserver/server_test.go @@ -1158,6 +1158,165 @@ func TestHandlePassPostRegistration(t *testing.T) { ) } +// The shortest and longest password PASS accepts; bcrypt +// cannot hash more than 72 bytes. +const ( + minPasswordBytes = 8 + 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" + +// passwordCase is a password a PASS test sends, and whether +// the server should store it. +type passwordCase struct { + name string + password string + stored bool +} + +// passwordCases returns the passwords the PASS length tests +// send: one of 8 to 72 bytes is stored and any other is +// refused. The limit counts bytes: "é" is two bytes in UTF-8, +// so 72 of them are refused. +func passwordCases() []passwordCase { + return []passwordCase{ + {"empty", "", false}, + {"7 bytes", strings.Repeat("a", minPasswordBytes-1), false}, + {"8 bytes", strings.Repeat("a", minPasswordBytes), true}, + {"72 bytes", strings.Repeat("a", maxPasswordBytes), true}, + {"73 bytes", strings.Repeat("a", maxPasswordBytes+1), false}, + {"72 characters", strings.Repeat("é", maxPasswordBytes), false}, + } +} + +// assertStored fails the test unless nick can log in with +// password exactly when stored is true. +func assertStored( + t *testing.T, env *testEnv, nick, password string, stored bool, +) { + t.Helper() + + _, _, token, err := env.database.LoginUser( + t.Context(), nick, password, "127.0.0.1", "localhost", + ) + + loggedIn := err == nil && token != "" + if loggedIn != stored { + t.Errorf( + "log in as %s: logged in %v, want %v (error: %v)", + nick, loggedIn, stored, err, + ) + } +} + +// TestPassPostRegistrationLength checks that PASS after +// registration stores and confirms a password of 8 to 72 +// bytes, and refuses any other with a NOTICE. +func TestPassPostRegistrationLength(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + + for i, tc := range passwordCases() { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + nick := fmt.Sprintf("postpass%d", i) + client := env.dial(t) + + client.register(nick) + client.send("PASS :" + tc.password) + + // The server answers commands in order, so its + // reply to PASS comes before the PONG. + lines := client.sendAndExpect("PING :after", "PONG") + + reply := passwordLengthNotice + if tc.stored { + reply = "Password set" + } + + assertContains(t, lines, reply, "reply to PASS") + assertStored(t, env, nick, tc.password, tc.stored) + }) + } +} + +// 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 stores a password of 8 to 72 bytes, and that +// any other is refused with a NOTICE once registration +// completes. +func TestPassBeforeRegistrationLength(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + + for i, tc := range passwordCases() { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + nick := fmt.Sprintf("prepass%d", i) + client := env.dial(t) + + client.send("PASS :" + tc.password) + client.register(nick) + + // The password is checked after the welcome + // burst, which register reads; the PONG comes + // after that. + lines := client.sendAndExpect("PING :after", "PONG") + + if tc.stored { + assertNotContains( + t, lines, passwordLengthNotice, + "refusal of a valid password", + ) + } else { + assertContains( + t, lines, passwordLengthNotice, + "refusal after registration", + ) + } + + assertStored(t, env, nick, tc.password, tc.stored) + }) + } +} + func TestPreRegistrationNotRegistered(t *testing.T) { t.Parallel()