Refuse passwords over 72 bytes and confirm only stored ones (closes #144)
check / check (push) Waiting to run
check / check (push) Waiting to run
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
This commit was merged in pull request #173.
This commit is contained in:
@@ -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()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user