diff --git a/README.md b/README.md index 36caadd..5c655b9 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. @@ -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 - 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 66985a3..0e87775 100644 --- a/internal/handlers/api_test.go +++ b/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") 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..6c625bd 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 ) @@ -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. -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..8c6fc08 100644 --- a/internal/ircserver/server_test.go +++ b/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()