sasl: don't connect unauthenticated when a SASL login fails - #13
Open
rubinlinux wants to merge 1 commit into
Open
sasl: don't connect unauthenticated when a SASL login fails#13rubinlinux wants to merge 1 commit into
rubinlinux wants to merge 1 commit into
Conversation
A network configured for SASL PLAIN that failed to log in still registered, silently, as a stranger: a typo in the password, services being down or an account whose email nefarious2 wants verified all ended in a normal-looking connection with no account. Everything keyed to the account — channel access, host masks, the bouncer session and its catch-up cursor — then quietly does not work. Treat "the user asked to log in and is not logged in" as a failed connect: report why in the lobby, QUIT, and stay down so credentials can be fixed without a reconnect loop. `features.saslDisconnectOnFail` in config.json (default true) turns it back off for deploys that would rather connect anyway; the report is shown either way, so the silent case is gone. "Did not succeed" covers every way out of the exchange: no credentials configured (caught before CAP LS), no usable `sasl` cap (the message names what the server does offer), a CAP NAK, the 902/904-907 numerics with the server's own text, and the 12 s timeout. Safe against the ircd: nefarious2 holds registration while SASL is pending (`AR_SASL_PENDING`, cleared only by auth_sasl_done()) and takes QUIT from an unregistered client (`MFLG_UNREG`), so the abort is a clean pre-registration quit. Its failure paths and numerics are written up in docs/resources/nefarious2-websocket.md § SASL. Verified live: dev ircd (no services, `sasl` never advertised) and the services-backed testnet (Keycloak `904` in 0.1 s) both abort under the default and register under the flag; a correct login still completes registration and offers the PERSISTENCE ATTACH cursor.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A network configured for SASL PLAIN whose login failed still registered — silently, as a stranger. A typo in the password, services being down, or nefarious2 wanting the account's email verified all ended in a normal-looking connection with no account, and everything keyed to that account (channel access, host masks, the bouncer session and its catch-up cursor) quietly did not work.
Now "the user asked to log in and is not logged in" is a failed connect: the lobby gets the reason, the client
QUITs and stays down (no reconnect loop) so credentials can be fixed.The config-time toggle
config.json:{"features": {"saslDisconnectOnFail": false}}Default
true(abort).falserestores the old "connect anyway" behaviour — but the failure is now reported either way, so the silent case is gone. Documented indocs/resources/branding.md§ Failed SASL logins.ConnectOptions.saslDisconnectOnFail(which existed but was never wired to anything) stays as a per-client override; unset means "ask the deploy".What counts as a failure
Every way out of the exchange that isn't
903:onOpen, beforeCAP LS, so nothing is sent;sasl, or offers it withoutPLAIN(the message names what it does offer);CAP NAK :sasl;902/904/905/906/907, quoting the server's own text (: service unavailable,: request timed out,FAIL AUTHENTICATE VERIFICATION_REQUIRED, …);Each shows the reason,
Not connecting to <host> without the login you asked for.and what to try next.handlers/cap.tsdrops whatever the negotiator had queued once we're quitting, so noCAP ENDtrails theQUIT.Why this is safe against the ircd (from nefarious2 source)
cap_end()only clearsAR_CAP_PENDING;AR_SASL_PENDINGis cleared solely byauth_sasl_done()(success/fail/abort/timeout), andcheck_auth_finished()scans every flag — so registration is held throughout the exchange and we're never racing 001.QUITisMFLG_UNREG(parse.c:212), handled bym_quitbefore registration: the abort is a clean pre-registration quit, not a dropped socket.FEAT_SASL_TIMEOUTis 10 s, which is why our own backstop stays at 12 s — the server's more descriptive904 :SASL authentication failed: request timed outwins.Written up in
docs/resources/nefarious2-websocket.md§ SASL, along with the failure texts worth quoting.Test plan
yarn test:mocha: 763 passing, 0 failing.test/irc/client-sasl.tscovers each failure case in both policies, plus the branding wiring;test/irc/branding.tscovers the new flag.sasl): reports "the server does not offer SASL", aborts by default, registers with the flag off.904in 0.1 s → abort; a correct login still completes registration and offers thePERSISTENCE ATTACHcursor (attach-cursor.live.ts, 2 passing).🤖 Generated with Claude Code