Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the EDITMSG extension specification. (now #425) #304

Closed
wants to merge 2 commits into from
Closed

Add the EDITMSG extension specification. (now #425) #304

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 16, 2017

@DanielOaks
Copy link
Member

If the server can pass along unvalidated data, then clients will need to do their own validation on every edit/delete message they see incoming anyway. Given that, the "this won't fail" sorta promised to the sending client doesn't hold true, and the "this is valid" guarantee that should be given to the receiving clients isn't given. It's completely unvalidated data, and as completely unvalidated data it should be a client tag rather than a proper tag in my opinion.

If you do make the server validation mandatory, I don't like the required storage of message information on the server end (data which a great deal of them probably aren't storing right now). If you instead make client validation mandatory, there's no reason to involve the server in this imo, may be better placed to just make it a client-tag wholesale.

@ghost
Copy link
Author

ghost commented Mar 16, 2017

Based on current discussions from the IRC channel, there are 3 possible ways of addressing the validation issue:

  1. Keep the spec as-is, including server-side validation

  2. Change validation to client-side and retain the PRIVMSG to read:

Clients MUST verify the authenticity of the edit. If the change is verified, clients SHOULD apply the edit in-place. Unverified changes MUST be supressed and MUST NOT be shown to the user. If a client does not support the editmsg and delete message-tags, the message WILL be displayed as is from the sending user.

This offers the advantage of not requiring any form of server side logging, including metadata. The downside is that unauthorized edits are still passed to the client, as discussed in #304 (comment)

  1. Require client-side validation and change the PRIVMSG to a TAGMSG and send the entirety of the updated message in an edit-text message tag. Users of older clients that do not support TAGMSG would not see any changes.

@jwheare
Copy link
Member

jwheare commented Mar 16, 2017

  1. A hybrid approach: Clients can send either TAGMSG or PRIVMSG (with a slightly different fallback message for older clients instead of the full message) depending on the scale of the edit.

@DanielOaks
Copy link
Member

DanielOaks commented Mar 16, 2017

The main point against (and issue with) client-side validation is that how clients validate messages may not be entirely uniform across all clients implementing this spec. One way to avoid this is to just make it really, really explicit what sorts of validation clients should do.

Rough draft, a specific section going over validation may read something like:

Validating Message Authors

Clients implementing this specification MUST implement the account-tag specification, and SHOULD implement the chghost extension.

For each channel a client is joined to, the client MUST remember which user and account posted each of the last 20 messages in that channel. Clients MAY remember more history than the last 20 messages if desired.

When remembering which user posted a channel, clients MUST track users across nickname changes, and username and hostname changes if the chghost extension is negotiated. It must track this while it shares a common channel with each user. If the client no longer shares a common channel with a user, all tracking information with regards to which messages a user has posted, aside from identical nickmask comparison (nickname!username@hostname) and account comparison MUST no longer be used for validation.


very rough and still not specified super well, but something that's very clear like that will help alleviate the issues around the client-side validation

@DanielOaks
Copy link
Member

Backwards Compatibility

This is the main thing we're worrying about now, exactly how to provide backwards compatibility. Here's my take on the various ways to send messages, and the (main) combos of those types of message sending that we can end up with.

Whether clients can see edits

Some think older clients should always see every edit. Some think that older clients shouldn't be bothered with edits that aren't significant. For instance, if I change a fullstop to a comma and change the first letter of the following sentence, should the old client be bothered with the change? What about if I just change some whitespace around or remove an errant space that made its way in somewhere, or a duped character?

Full PRIVMSG - full content as PRIVMSG body

When edited messages are sent in this way, old clients will ALWAYS see the full message content again. This means they lose no context, ever, but particularly for longer messages it can get very spammy.

PRIVMSG + Digest - full message content as edit-text tag, digest as PRIVMSG body

When edited messages are sent this way, old clients will see the digest (generally, the client's intended to put some meaningful, cut-down version of the full message in here when possible). I'm wary of this because then we need to specify, or at least describe different ways of creating message digests from edited messages.

TAGMSG - full message content as edit-text tag

When edited messages are sent this way, old clients see absolutely no indication that any messages were edited. When the edit is worthless – a trailing space was culled, a duped space or letter was removed, etc, it's not an issue. If dodgy clients start getting messages past other users without the other uesrs seeing, that's dodgy. If clients don't correctly detect when an edit is worthless or worth conveying, this could mean lots of missed context for older clients.

Combinations

  1. Full PRIVMSG, always: Old clients will always see edits, can seem very spammy if edits become popular.
  2. PRIVMSG + Digest, always: Old clients will always see edits, but less spammy than full PRIVMSG always.
  3. PRIVMSG + Digest, plus TAGMSG: Old clients will see relevant edits, and hopefully not be bothered by the minor ones. May provide a back-channel to dodgy people passing dodgy messages along without people who use older clients seeing.
  4. TAGMSG, always: Old clients don't see anything, not feasible.

@DanielOaks
Copy link
Member

As a note, I also wrote up a proposal for message modification around the time @MuffinMedic put this PR in, which you can see here:
https://gist.github.com/DanielOaks/eb88a1a9652f7fd3bbdeebac630ad1c9

Feel free to compare, contrast the differences and take into account the discussions in here.

@lopcode
Copy link

lopcode commented Nov 5, 2017

I read through both of these proposals and I'm liking the way @DanielOaks' looks. Been hoping for a protocol-supported way to hide particularly unsavoury spam for a while.

My immediate thoughts:

  • Allowing only the original author to edit a message, but operators to delete anybody's message, makes sense to me
  • Editable history amount should be conveyed to clients by the server in something like ISUPPORT (though this would still prevent channels having different amounts of editable history in a way that doesn't visually desync people)
  • I'm a little concerned about the assumption of operators having ultimate privilege to perform deletions - is there a sensible way to allow configuration for channels to be conveyed to clients, or would it end up making this extension needlessly complex?

@SinZ163
Copy link

SinZ163 commented Mar 14, 2018

A lot of the discussion has been on how edit-msg will work, and whether it should be a PRIVMSG or a TAGMSG, but not on how deletes will work.

Considering the delete in spec is a PRIVMSG with no content, it makes sense to be a TAGMSG. Regarding backwards compatibility, if the client didn't support delete they would not delete the message, but see an empty message being sent, while using TAGMSG they will see nothing.

@DanielOaks
Copy link
Member

Yeah I think the proposal(s) were written before TAGMSG was a thing, but definitely makes sense to switch them over to use it like you say.

@progval
Copy link
Contributor

progval commented Mar 14, 2018

Regarding backwards compatibility, if the client didn't support delete they would not delete the message, but see an empty message being sent, while using TAGMSG they will see nothing.

If they don't support delete, the server would not send them a deletion message.

@SinZ163
Copy link

SinZ163 commented Mar 19, 2018

I don't see any capabilities in the proposal, so server wouldn't know (other than if they support message-tags in general)

@berndjendrissek
Copy link

  • Channel operators may not want even the capability of deleting other users' messages after the fact. Having the capability and failing to act on it may create unwelcome legal liabilities. I would urge a SHOULD NOT wording here, not MAY, putting it on the same footing as the SHOULD NOT of operator editing of others' messages. I don't think there'd be a similar "duty to kick" (because kicking someone doesn't un-say evil words), so there's no symmetry here with other operator actions.

  • Client-side user tracking can be hairy. InspIRCd for example has channel mode +u which (ordinarily) makes joins/parts invisible (and with it, nick changes). Also there are channel modes that (dis)allow non-joined users to message a channel (-n). They also won't be trackable. A client would struggle to know whether joe!foo@example.com and john!foo@example.com are the same user. The only reliable place to track users is in the server. Consider also users behind a company firewall for example - different users, same host.

@jesopo
Copy link

jesopo commented Feb 15, 2019

There's been a lot of chat on IRC about this today - specifically about how to achieve authentication for editing/deleting without the server having to store any backlog.

One suggestion that got wide support: construct msgid partially out of an account (e.g. nickserv account) identifier so that future attempts to edit/delete this specific msgid can be matched against the account identifier for authentication (checked by the server against what it knows the user's account identifier to be). If a malicious user changes the msgid to falsify the author, that doesn't matter because it would be an entirely new msgid.

There's also been some musing about encrypting the msgid to keep the account identity secret from clients - this seems to not inherently be required but could be useful for servers that don't want to expose how they identify accounts.

The idea to put an account identifier in the msgid also means this spec would need no special functionality other than being able to ask to edit or delete a specific msgid.

@jwheare
Copy link
Member

jwheare commented Jan 17, 2020

A replacement for this PR is planned, based on the kick off document here https://gist.github.com/jesopo/ef44e928aeb1c893e722909608960b1b

@niansa
Copy link

niansa commented Jul 18, 2020

The description is broken.

@jwheare
Copy link
Member

jwheare commented Sep 2, 2020

Updated and replaced by #425

@jwheare jwheare closed this Sep 2, 2020
@DanielOaks DanielOaks changed the title Add the EDITMSG extension specification. Add the EDITMSG extension specification. (now #425) Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants