search

ergochat / ergo

A modern IRC server (daemon/ircd) written in Go.
https://ergo.chat/
MIT License
2.26k stars 179 forks source link

consider relaxing PRECIS constraints for RELAYMSG relay nicks? #1441

Open slingamn opened 3 years ago

slingamn commented 3 years ago

See #1437. The / separator character can cause identifiers that are not obviously malformed to fail PRECIS.

Since we are not enforcing any kind of uniqueness or non-confusability requirement on spoofed RELAYMSG nicks, it may not actually be necessary for them to pass PRECIS. I'm thinking instead of making them pass CasefoldName, we can have them pass a different check:

  1. If casemapping is PRECIS or permissive, just check for protocol-breaking characters
  2. If casemapping is ASCII, do the normal foldASCII check
slingamn commented 3 years ago

I have some qualms since channel operators can send these in the default/recommended configuration...

Mikaela commented 3 years ago

Example problem as seen by Matterbridge

tammi 01 09:45:50 etro matterbridge[9445]: time="2021-01-01T09:45:50Z" level=debug msg="Sending RELAYMSG to channel #mikaela: nick=Leon🏳️‍🌈-[he/him/his]/m" func=doSend file="bridge/irc/irc.go:241" p refix=irc 
tammi 01 09:44:17 etro matterbridge[9445]: time="2021-01-01T09:44:17Z" level=debug msg=""@time=2021-01-01T09:44:17.580Z :tuusula-fi.pirateirc.net FAIL RELAYMSG INVALID_NICK :Invalid nickname"" func=handleOther file="bridge/irc/handlers.go:163" prefix=irc
DanielOaks commented 3 years ago

Oh how jlu5's net handles these characters: the matterbridge code just replaces all invalid/reserved chars with a -

I'm not sure how allowing identifiers that can't pass PRECIS will work for clients - I get the feeling we may already do it with how relaymsg works right now, but if clients are expecting to be able to run PRECIS casefolding on all incoming identifiers they may run into issues. Might be worth adding a note to our unicode identifiers spec explicitly stating this case and telling clients that "hey if you can't run it through precis then just, um, don't"

slingamn commented 3 years ago

Do we have a unicode identifiers spec?

slingamn commented 3 years ago

Some discussion on #1083 re. whether clients should be able to treat our identifiers as opaque byte strings.

slingamn commented 3 years ago

This interacts weirdly with #1502; if we can't casefold the identifier then it's not clear how we're going to match it against bans.

Mikaela commented 3 years ago

Regarding this and #1684 splitting would it be possible to also strips @s from RELAYMSGed nicknames? It would be another line I could remove from my matterbridge.tomls as I have it to format MatrixIDs for non-RELAYMSG-capable networks.

I understand that not all stripping should be done on server side and some should possibly be left for clients.

Mikaela commented 2 years ago

I am still using Matrix IDs instead of display names in matterbridge so messages won't just disappear when Ergo cannot parse them and I have two awkward situations with it:

slingamn commented 1 year ago
  1. If casemapping is ASCII, enforce all normal restrictions, else
  2. Enforce protocol-breaking characters
  3. Replace disfavored characters with a replacement (maybe -?)
  4. Attempt to casefold with PRECIS
  5. If this fails, apply foldPermissive; if this fails, reject the message
  6. The folded identifier from steps 4 or 5 is evaluated against bans/mutes