Skip to content

Implement authentication#7

Open
noctux wants to merge 6 commits into
pawelb0:mainfrom
noctux:implement-authentication
Open

Implement authentication#7
noctux wants to merge 6 commits into
pawelb0:mainfrom
noctux:implement-authentication

Conversation

@noctux

@noctux noctux commented Jun 6, 2026

Copy link
Copy Markdown

This PR adds the ability to optionally set a connection password that is enforced for connections (in IRC via PASS aka the server password, and in HTTP connections via HTTP-Basic Auth (username is arbitrary)).
This allows for a basic level of security when using matrirc on (quasi-)multiusersystems.

For the HTTP-Part, it was a bit surprising to see that matrirc currently implements HTTP1.1 all by itself instead of using one of the typical rust frameworks, but this PR follows suit to keep with this design decision.

N.B.: Currently, the PR uses the default PBKDF2 configuration, which results in a noticable extension of the testsuits execution time.

Comment thread src/irc/conn.rs
if handle_command(&mut write, &peer, &bridge, &msg, &mut s).await? { return Ok(()); }
if !s.registered {
if let (Some(n), Some(_)) = (s.nick.clone(), s.user.clone()) {
if !check_auth(&s.received_password, &s.password_hash) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check itself is spot on — PHC, constant-time verify, and the proxy side enforces it perfectly. The snag is placement: this branch flips the registered/welcome state, but commands run earlier in handle_command (line 280), where PRIVMSG/JOIN gate on s.nick rather than registered (lines 462/468). A client can send NICK eve then PRIVMSG #room :hi and it reaches Matrix with no PASS — s.registered never flips (this block also waits on USER, which never comes), so we never get here.

The read side has a matching gap: the DM branch in handle_matrix_event keys on s.nick instead of registered, so a socket that only sent NICK can also receive incoming DMs.

Your other handlers already gate on s.registered (391/405/410/415/429), so having the dispatcher do the same closes both directions — and your check already controls that flag. A test for NICK-then-PRIVMSG-without-PASS would lock it in.

Comment thread src/proxy.rs
&mut sock,
401,
"Unauthorized",
vec!["WWW-Authenticate: Basic realm=matrirc"],

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

realm should be a quoted-string per RFC 7617 — realm="matrirc".

Comment thread Cargo.toml
libc = "0.2"
rpassword = "7"
unidecode = "0.3.0"
pbkdf2 = { version = "0.13.0", features = [ "getrandom", "phc", "sha2" ] }

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0.13 pulls in the newer RustCrypto generation (crypto-common 0.2, digest 0.11, sha2 0.11, hmac 0.13, hybrid-array, phc) alongside the 0.10/0.12 copies matrix-sdk already uses. Pinning to 0.12 would reuse what's already in the tree — optional, just a build-size thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants