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

Merged
clawbot merged 1 commits from issue-144-password-length into next 2026-10-21 05:02:01 +02:00
6 changed files with 280 additions and 46 deletions
+10 -7
View File
@@ -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.
+63 -22
View File
@@ -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,
)
})
}
}
+9 -3
View File
@@ -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
+5 -6
View File
@@ -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 "+
+34 -8
View File
@@ -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
}
+159
View File
@@ -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()